Merge pull request #3391 from joostjager/always-return-invalid-details

htlcswitch+invoices: always return incorrect_or_unknown_payment_details

htlcswitch/link.go

@@ -1204,29 +1204,15 @@ func (l *channelLink) processHodlEvent(hodlEvent invoices.HodlEvent,
 			)
 		}
 	} else {
-		l.debugf("Received hodl cancel event for %v, reason=%v",
-			hash, hodlEvent.CancelReason)
+		l.debugf("Received hodl cancel event for %v", hash)
 
 		hodlAction = func(htlc hodlHtlc) error {
-			var failure lnwire.FailureMessage
-			switch hodlEvent.CancelReason {
-
-			case invoices.CancelAmountTooLow:
-				fallthrough
-			case invoices.CancelInvoiceUnknown:
-				fallthrough
-			case invoices.CancelInvoiceCanceled:
-				failure = lnwire.NewFailUnknownPaymentHash(
-					htlc.pd.Amount,
-				)
-
-			case invoices.CancelExpiryTooSoon:
-				failure = &lnwire.FailFinalExpiryTooSoon{}
-
-			default:
-				return fmt.Errorf("unknown cancel reason: %v",
-					hodlEvent.CancelReason)
-			}
+			// In case of a cancel, always return
+			// incorrect_or_unknown_payment_details in order to
+			// avoid leaking info.
+			failure := lnwire.NewFailIncorrectDetails(
+				htlc.pd.Amount,
+			)
 
 			l.sendHTLCError(
 				htlc.pd.HtlcIndex, failure, htlc.obfuscator,
@@ -2843,7 +2829,7 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor,
 			"value: expected %v, got %v", pd.RHash,
 			pd.Amount, fwdInfo.AmountToForward)
 
-		failure := lnwire.NewFailUnknownPaymentHash(pd.Amount)
+		failure := lnwire.NewFinalIncorrectHtlcAmount(pd.Amount)
 		l.sendHTLCError(pd.HtlcIndex, failure, obfuscator, pd.SourceRef)
 
 		return true, nil
@@ -2856,9 +2842,7 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor,
 			"time-lock: expected %v, got %v",
 			pd.RHash[:], pd.Timeout, fwdInfo.OutgoingCTLV)
 
-		failure := lnwire.NewFinalIncorrectCltvExpiry(
-			fwdInfo.OutgoingCTLV,
-		)
+		failure := lnwire.NewFinalIncorrectCltvExpiry(pd.Timeout)
 		l.sendHTLCError(pd.HtlcIndex, failure, obfuscator, pd.SourceRef)
 
 		return true, nil
@@ -2878,7 +2862,7 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor,
 
 	// Cancel htlc if we don't have an invoice for it.
 	case channeldb.ErrInvoiceNotFound:
-		failure := lnwire.NewFailUnknownPaymentHash(pd.Amount)
+		failure := lnwire.NewFailIncorrectDetails(pd.Amount)
 		l.sendHTLCError(pd.HtlcIndex, failure, obfuscator, pd.SourceRef)
 
 		return true, nil

htlcswitch/link_test.go

@@ -11,7 +11,6 @@ import (
 	"net"
 	"reflect"
 	"runtime"
-	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -623,12 +622,8 @@ func TestExitNodeAmountPayloadMismatch(t *testing.T) {
 	).Wait(30 * time.Second)
 	if err == nil {
 		t.Fatalf("payment should have failed but didn't")
-	} else if !strings.Contains(err.Error(), lnwire.CodeUnknownPaymentHash.String()) {
-		// TODO(roasbeef): use proper error after error propagation is
-		// in
-		t.Fatalf("expected %v got %v", err,
-			lnwire.CodeUnknownPaymentHash)
 	}
+	assertFailureCode(t, err, lnwire.CodeFinalIncorrectHtlcAmount)
 }
 
 // TestLinkForwardTimelockPolicyMismatch tests that if a node is an
@@ -1025,9 +1020,8 @@ func TestChannelLinkMultiHopInsufficientPayment(t *testing.T) {
 	).Wait(30 * time.Second)
 	if err == nil {
 		t.Fatal("error haven't been received")
-	} else if !strings.Contains(err.Error(), "insufficient capacity") {
-		t.Fatalf("wrong error has been received: %v", err)
 	}
+	assertFailureCode(t, err, lnwire.CodeTemporaryChannelFailure)
 
 	// Wait for Alice to receive the revocation.
 	//
@@ -1136,10 +1130,9 @@ func TestChannelLinkMultiHopUnknownPaymentHash(t *testing.T) {
 		t.Fatalf("no result arrive")
 	}
 
-	fErr := result.Error
-	if !strings.Contains(fErr.Error(), lnwire.CodeUnknownPaymentHash.String()) {
-		t.Fatalf("expected %v got %v", lnwire.CodeUnknownPaymentHash, fErr)
-	}
+	assertFailureCode(
+		t, result.Error, lnwire.CodeIncorrectOrUnknownPaymentDetails,
+	)
 
 	// Wait for Alice to receive the revocation.
 	time.Sleep(100 * time.Millisecond)
@@ -1420,10 +1413,10 @@ func TestChannelLinkExpiryTooSoonExitNode(t *testing.T) {
 	}
 
 	switch ferr.FailureMessage.(type) {
-	case *lnwire.FailFinalExpiryTooSoon:
+	case *lnwire.FailIncorrectDetails:
 	default:
-		t.Fatalf("incorrect error, expected final time lock too "+
-			"early, instead have: %v", err)
+		t.Fatalf("expected incorrect_or_unknown_payment_details, "+
+			"instead have: %v", err)
 	}
 }
 
@@ -5677,7 +5670,7 @@ func TestChannelLinkCanceledInvoice(t *testing.T) {
 	if !ok {
 		t.Fatalf("expected ForwardingError, but got %v", err)
 	}
-	_, ok = fErr.FailureMessage.(*lnwire.FailUnknownPaymentHash)
+	_, ok = fErr.FailureMessage.(*lnwire.FailIncorrectDetails)
 	if !ok {
 		t.Fatalf("expected unknown payment hash, but got %v", err)
 	}
@@ -5846,11 +5839,7 @@ func TestChannelLinkHoldInvoiceCancel(t *testing.T) {
 
 	// Wait for payment to succeed.
 	err = <-ctx.errChan
-	if !strings.Contains(err.Error(),
-		lnwire.CodeUnknownPaymentHash.String()) {
-
-		t.Fatal("expected unknown payment hash")
-	}
+	assertFailureCode(t, err, lnwire.CodeIncorrectOrUnknownPaymentDetails)
 }
 
 // TestChannelLinkHoldInvoiceRestart asserts hodl htlcs are held after blocks
@@ -6081,3 +6070,17 @@ func TestChannelLinkRevocationWindowHodl(t *testing.T) {
 	default:
 	}
 }
+
+// assertFailureCode asserts that an error is of type ForwardingError and that
+// the failure code is as expected.
+func assertFailureCode(t *testing.T, err error, code lnwire.FailCode) {
+	fErr, ok := err.(*ForwardingError)
+	if !ok {
+		t.Fatalf("expected ForwardingError but got %T", err)
+	}
+
+	if fErr.FailureMessage.Code() != code {
+		t.Fatalf("expected %v but got %v",
+			code, fErr.FailureMessage.Code())
+	}
+}

htlcswitch/switch_test.go

@@ -1803,7 +1803,7 @@ func TestSwitchSendPayment(t *testing.T) {
 	// the add htlc request with error and sent the htlc fail request
 	// back. This request should be forwarded back to alice channel link.
 	obfuscator := NewMockObfuscator()
-	failure := lnwire.NewFailUnknownPaymentHash(update.Amount)
+	failure := lnwire.NewFailIncorrectDetails(update.Amount)
 	reason, err := obfuscator.EncryptFirstHop(failure)
 	if err != nil {
 		t.Fatalf("unable obfuscate failure: %v", err)
@@ -1824,14 +1824,9 @@ func TestSwitchSendPayment(t *testing.T) {
 
 	select {
 	case err := <-errChan:
-		fErr, ok := err.(*ForwardingError)
-		if !ok {
-			t.Fatal("expected ForwardingError")
-		}
-
-		if _, ok := fErr.FailureMessage.(*lnwire.FailUnknownPaymentHash); !ok {
-			t.Fatalf("expected UnknownPaymentHash got %v", fErr)
-		}
+		assertFailureCode(
+			t, err, lnwire.CodeIncorrectOrUnknownPaymentDetails,
+		)
 	case <-time.After(time.Second):
 		t.Fatal("err wasn't received")
 	}