Merge pull request #2554 from Roasbeef/deprecate-htlc-amt-error

multi: deprecate IncorrectHtlcAmount

htlcswitch/link.go

@@ -2318,7 +2318,9 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 			if err != nil {
 				log.Errorf("unable to query invoice registry: "+
 					" %v", err)
-				failure := lnwire.FailUnknownPaymentHash{}
+				failure := lnwire.NewFailUnknownPaymentHash(
+					pd.Amount,
+				)
 				l.sendHTLCError(
 					pd.HtlcIndex, failure, obfuscator, pd.SourceRef,
 				)
@@ -2367,7 +2369,9 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 					"amount: expected %v, received %v",
 					invoice.Terms.Value, pd.Amount)
 
-				failure := lnwire.FailIncorrectPaymentAmount{}
+				failure := lnwire.NewFailUnknownPaymentHash(
+					pd.Amount,
+				)
 				l.sendHTLCError(
 					pd.HtlcIndex, failure, obfuscator, pd.SourceRef,
 				)
@@ -2394,7 +2398,9 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 					"got %v", pd.RHash, invoice.Terms.Value,
 					fwdInfo.AmountToForward)
 
-				failure := lnwire.FailIncorrectPaymentAmount{}
+				failure := lnwire.NewFailUnknownPaymentHash(
+					pd.Amount,
+				)
 				l.sendHTLCError(
 					pd.HtlcIndex, failure, obfuscator, pd.SourceRef,
 				)

htlcswitch/link_test.go

@@ -587,11 +587,11 @@ func TestExitNodeAmountPayloadMismatch(t *testing.T) {
 	).Wait(30 * time.Second)
 	if err == nil {
 		t.Fatalf("payment should have failed but didn't")
-	} else if err.Error() != lnwire.CodeIncorrectPaymentAmount.String() {
+	} else if !strings.Contains(err.Error(), lnwire.CodeUnknownPaymentHash.String()) {
 		// TODO(roasbeef): use proper error after error propagation is
 		// in
-		t.Fatalf("incorrect error, expected insufficient value, "+
+		t.Fatalf("expected %v got %v", err,
-			"instead have: %v", err)
+			lnwire.CodeUnknownPaymentHash)
 	}
 }
 
@@ -1017,8 +1017,9 @@ func TestChannelLinkMultiHopUnknownPaymentHash(t *testing.T) {
 		n.firstBobChannelLink.ShortChanID(), htlc,
 		newMockDeobfuscator(),
 	)
-	if err.Error() != lnwire.CodeUnknownPaymentHash.String() {
+	if !strings.Contains(err.Error(), lnwire.CodeUnknownPaymentHash.String()) {
-		t.Fatal("error haven't been received")
+		t.Fatalf("expected %v got %v", err,
+			lnwire.CodeUnknownPaymentHash)
 	}
 
 	// Wait for Alice to receive the revocation.

htlcswitch/switch_test.go

@@ -6,13 +6,13 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/btcsuite/btcutil"
 	"github.com/btcsuite/fastsha256"
 	"github.com/davecgh/go-spew/spew"
-	"github.com/go-errors/errors"
 	"github.com/lightningnetwork/lnd/channeldb"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/ticker"
@@ -1780,7 +1780,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.FailIncorrectPaymentAmount{}
+	failure := lnwire.NewFailUnknownPaymentHash(update.Amount)
 	reason, err := obfuscator.EncryptFirstHop(failure)
 	if err != nil {
 		t.Fatalf("unable obfuscate failure: %v", err)
@@ -1801,8 +1801,9 @@ func TestSwitchSendPayment(t *testing.T) {
 
 	select {
 	case err := <-errChan:
-		if err.Error() != errors.New(lnwire.CodeIncorrectPaymentAmount).Error() {
+		if !strings.Contains(err.Error(), lnwire.CodeUnknownPaymentHash.String()) {
-			t.Fatal("err wasn't received")
+			t.Fatalf("expected %v got %v", err,
+				lnwire.CodeUnknownPaymentHash)
 		}
 	case <-time.After(time.Second):
 		t.Fatal("err wasn't received")

lnd_test.go

@@ -7878,10 +7878,11 @@ out:
 
 	// Next, we'll test the case of a recognized payHash but, an incorrect
 	// value on the extended HTLC.
+	htlcAmt := lnwire.NewMSatFromSatoshis(1000)
 	sendReq = &lnrpc.SendRequest{
 		PaymentHashString: hex.EncodeToString(carolInvoice.RHash),
 		DestString:        hex.EncodeToString(carol.PubKey[:]),
-		Amt:               1000, // 10k satoshis are expected.
+		Amt:               int64(htlcAmt.ToSatoshis()), // 10k satoshis are expected.
 	}
 	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	resp, err = net.Alice.SendPaymentSync(ctxt, sendReq)
@@ -7895,12 +7896,19 @@ out:
 		t.Fatalf("payment should have been rejected due to wrong " +
 			"HTLC amount")
 	}
-	expectedErrorCode = lnwire.CodeIncorrectPaymentAmount.String()
+	expectedErrorCode = lnwire.CodeUnknownPaymentHash.String()
 	if !strings.Contains(resp.PaymentError, expectedErrorCode) {
 		t.Fatalf("payment should have failed due to wrong amount, "+
 			"instead failed due to: %v", resp.PaymentError)
 	}
 
+	// We'll also ensure that the encoded error includes the invlaid HTLC
+	// amount.
+	if !strings.Contains(resp.PaymentError, htlcAmt.String()) {
+		t.Fatalf("error didn't include expected payment amt of %v: "+
+			"%v", htlcAmt, resp.PaymentError)
+	}
+
 	// The balances of all parties should be the same as initially since
 	// the HTLC was cancelled.
 	assertBaseBalance()

lnwire/onion_error.go

@@ -125,6 +125,9 @@ func (c FailCode) String() string {
 	case CodeIncorrectCltvExpiry:
 		return "IncorrectCltvExpiry"
 
+	case CodeIncorrectPaymentAmount:
+		return "IncorrectPaymentAmount"
+
 	case CodeExpiryTooSoon:
 		return "ExpiryTooSoon"
 
@@ -134,9 +137,6 @@ func (c FailCode) String() string {
 	case CodeUnknownPaymentHash:
 		return "UnknownPaymentHash"
 
-	case CodeIncorrectPaymentAmount:
-		return "IncorrectPaymentAmount"
-
 	case CodeFinalExpiryTooSoon:
 		return "FinalExpiryTooSoon"
 
@@ -294,28 +294,6 @@ func (f FailUnknownNextPeer) Error() string {
 	return f.Code().String()
 }
 
-// FailUnknownPaymentHash is returned If the payment hash has already been
-// paid, the final node MAY treat the payment hash as unknown, or may succeed
-// in accepting the HTLC. If the payment hash is unknown, the final node MUST
-// fail the HTLC.
-//
-// NOTE: May only be returned by the final node in the path.
-type FailUnknownPaymentHash struct{}
-
-// Code returns the failure unique code.
-//
-// NOTE: Part of the FailureMessage interface.
-func (f FailUnknownPaymentHash) Code() FailCode {
-	return CodeUnknownPaymentHash
-}
-
-// Returns a human readable string describing the target FailureMessage.
-//
-// NOTE: Implements the error interface.
-func (f FailUnknownPaymentHash) Error() string {
-	return f.Code().String()
-}
-
 // FailIncorrectPaymentAmount is returned if the amount paid is less than the
 // amount expected, the final node MUST fail the HTLC. If the amount paid is
 // more than twice the amount expected, the final node SHOULD fail the HTLC.
@@ -339,6 +317,65 @@ func (f FailIncorrectPaymentAmount) Error() string {
 	return f.Code().String()
 }
 
+// FailUnknownPaymentHash is returned for two reasons:
+//
+// 1) if the payment hash has already been paid, the final node MAY treat the
+// payment hash as unknown, or may succeed in accepting the HTLC. If the
+// payment hash is unknown, the final node MUST fail the HTLC.
+//
+// 2) if the amount paid is less than the amount expected, the final node MUST
+// fail the HTLC. If the amount paid is more than twice the amount expected,
+// the final node SHOULD fail the HTLC. This allows the sender to reduce
+// information leakage by altering the amount, without allowing accidental
+// gross overpayment.
+//
+// NOTE: May only be returned by the final node in the path.
+type FailUnknownPaymentHash struct {
+	// amount is the value of the extended HTLC.
+	amount MilliSatoshi
+}
+
+// NewFailUnknownPaymentHash makes a new instance of the FailUnknownPaymentHash
+// error bound to the specified HTLC amount.
+func NewFailUnknownPaymentHash(amt MilliSatoshi) *FailUnknownPaymentHash {
+	return &FailUnknownPaymentHash{
+		amount: amt,
+	}
+}
+
+// Amount is the value of the extended HTLC.
+func (f FailUnknownPaymentHash) Amount() MilliSatoshi {
+	return f.amount
+}
+
+// Code returns the failure unique code.
+//
+// NOTE: Part of the FailureMessage interface.
+func (f FailUnknownPaymentHash) Code() FailCode {
+	return CodeUnknownPaymentHash
+}
+
+// Returns a human readable string describing the target FailureMessage.
+//
+// NOTE: Implements the error interface.
+func (f FailUnknownPaymentHash) Error() string {
+	return fmt.Sprintf("UnknownPaymentHash(amt=%v)", f.amount)
+}
+
+// Decode decodes the failure from bytes stream.
+//
+// NOTE: Part of the Serializable interface.
+func (f *FailUnknownPaymentHash) Decode(r io.Reader, pver uint32) error {
+	return ReadElement(r, &f.amount)
+}
+
+// Encode writes the failure in bytes stream.
+//
+// NOTE: Part of the Serializable interface.
+func (f *FailUnknownPaymentHash) Encode(w io.Writer, pver uint32) error {
+	return WriteElement(w, f.amount)
+}
+
 // FailFinalExpiryTooSoon is returned if the cltv_expiry is too low, the final
 // node MUST fail the HTLC.
 //

routing/router.go

@@ -1843,8 +1843,9 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 			}
 
 			switch onionErr := fErr.FailureMessage.(type) {
-			// If the end destination didn't know they payment
+			// If the end destination didn't know the payment
-			// hash, then we'll terminate immediately.
+			// hash or we sent the wrong payment amount to the
+			// destination, then we'll terminate immediately.
 			case *lnwire.FailUnknownPaymentHash:
 				return preImage, nil, sendError