diff --git a/htlcswitch/link.go b/htlcswitch/link.go index f0b60c18..8d9aa458 100644 --- a/htlcswitch/link.go +++ b/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, ) diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index d4fe2a30..d9e77698 100644 --- a/htlcswitch/link_test.go +++ b/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, "+ - "instead have: %v", err) + t.Fatalf("expected %v got %v", err, + lnwire.CodeUnknownPaymentHash) } } @@ -1017,8 +1017,9 @@ func TestChannelLinkMultiHopUnknownPaymentHash(t *testing.T) { n.firstBobChannelLink.ShortChanID(), htlc, newMockDeobfuscator(), ) - if err.Error() != lnwire.CodeUnknownPaymentHash.String() { - t.Fatal("error haven't been received") + if !strings.Contains(err.Error(), lnwire.CodeUnknownPaymentHash.String()) { + t.Fatalf("expected %v got %v", err, + lnwire.CodeUnknownPaymentHash) } // Wait for Alice to receive the revocation. diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 6a0fd0d6..3733534b 100644 --- a/htlcswitch/switch_test.go +++ b/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() { - t.Fatal("err wasn't received") + if !strings.Contains(err.Error(), lnwire.CodeUnknownPaymentHash.String()) { + t.Fatalf("expected %v got %v", err, + lnwire.CodeUnknownPaymentHash) } case <-time.After(time.Second): t.Fatal("err wasn't received") diff --git a/lnd_test.go b/lnd_test.go index 3881e84c..b6eab25c 100644 --- a/lnd_test.go +++ b/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() diff --git a/lnwire/onion_error.go b/lnwire/onion_error.go index ffc7e1af..f6719ea7 100644 --- a/lnwire/onion_error.go +++ b/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. // diff --git a/routing/router.go b/routing/router.go index f69d817a..e752b065 100644 --- a/routing/router.go +++ b/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 - // hash, then we'll terminate immediately. + // If the end destination didn't know the payment + // hash or we sent the wrong payment amount to the + // destination, then we'll terminate immediately. case *lnwire.FailUnknownPaymentHash: return preImage, nil, sendError