From e2fe4c295530634cfb2ba0a5c48c7d21ae60bfea Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 4 Jan 2018 14:23:31 -0600 Subject: [PATCH] htlcswitch: reject duplicate payments to same invoice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In this commit, we modify the way the link handles HTLC’s that it detects is destined for itself. Before this commit if a payment hash came across for an invoice we’d already settled, then we’d gladly accept the payment _again_. As we’d like to enforce the norm that an invoice is NEVER to be used twice, this commit modifies that behavior to instead reject an incoming payment that attempts to re-use an invoice. Fixes #560. --- htlcswitch/link.go | 14 +++++++++ htlcswitch/link_test.go | 70 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 9b65d909..512c76d0 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -1313,6 +1313,20 @@ func (l *channelLink) processLockedInHtlcs( continue } + // If this invoice has already been settled, + // then we'll reject it as we don't allow an + // invoice to be paid twice. + if invoice.Terms.Settled == true { + log.Warnf("Rejecting duplicate "+ + "payment for hash=%x", pd.RHash[:]) + failure := lnwire.FailUnknownPaymentHash{} + l.sendHTLCError( + pd.HtlcIndex, failure, obfuscator, + ) + needUpdate = true + continue + } + // If we're not currently in debug mode, and // the extended htlc doesn't meet the value // requested, then we'll fail the htlc. diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index ef91da6b..168de48d 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -456,7 +456,7 @@ func TestChannelLinkMultiHopPayment(t *testing.T) { // links were changed. invoice, err := receiver.registry.LookupInvoice(rhash) if err != nil { - t.Fatalf("unable to get inveoice: %v", err) + t.Fatalf("unable to get invoice: %v", err) } if !invoice.Terms.Settled { t.Fatal("carol invoice haven't been settled") @@ -912,7 +912,7 @@ func TestChannelLinkMultiHopInsufficientPayment(t *testing.T) { // links hasn't been changed. invoice, err := receiver.registry.LookupInvoice(rhash) if err != nil { - t.Fatalf("unable to get inveoice: %v", err) + t.Fatalf("unable to get invoice: %v", err) } if invoice.Terms.Settled { t.Fatal("carol invoice have been settled") @@ -1076,7 +1076,7 @@ func TestChannelLinkMultiHopUnknownNextHop(t *testing.T) { // links hasn't been changed. invoice, err := receiver.registry.LookupInvoice(rhash) if err != nil { - t.Fatalf("unable to get inveoice: %v", err) + t.Fatalf("unable to get invoice: %v", err) } if invoice.Terms.Settled { t.Fatal("carol invoice have been settled") @@ -1164,7 +1164,7 @@ func TestChannelLinkMultiHopDecodeError(t *testing.T) { // links hasn't been changed. invoice, err := receiver.registry.LookupInvoice(rhash) if err != nil { - t.Fatalf("unable to get inveoice: %v", err) + t.Fatalf("unable to get invoice: %v", err) } if invoice.Terms.Settled { t.Fatal("carol invoice have been settled") @@ -2265,3 +2265,65 @@ func TestChannelLinkUpdateCommitFee(t *testing.T) { aliceFeeRate, bobFeeRate) } } + +// TestChannelLinkRejectDuplicatePayment tests that if a link receives an +// incoming HTLC for a payment we have already settled, then it rejects the +// HTLC. We do this as we want to enforce the fact that invoices are only to be +// used _once. +func TestChannelLinkRejectDuplicatePayment(t *testing.T) { + t.Parallel() + + // First, we'll create our traditional three hop network. We'll only be + // interacting with and asserting the state of two of the end points + // for this test. + channels, cleanUp, _, err := createClusterChannels( + btcutil.SatoshiPerBitcoin*3, + btcutil.SatoshiPerBitcoin*5) + if err != nil { + t.Fatalf("unable to create channel: %v", err) + } + defer cleanUp() + + n := newThreeHopNetwork(t, channels.aliceToBob, channels.bobToAlice, + channels.bobToCarol, channels.carolToBob, testStartingHeight) + if err := n.start(); err != nil { + t.Fatalf("unable to start three hop network: %v", err) + } + defer n.stop() + + amount := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin) + + // We'll start off by making a payment from Alice to Carol. We'll + // manually generate this request so we can control all the parameters. + htlcAmt, totalTimelock, hops := generateHops(amount, testStartingHeight, + n.firstBobChannelLink, n.carolChannelLink) + blob, err := generateRoute(hops...) + if err != nil { + t.Fatal(err) + } + invoice, htlc, err := generatePayment(amount, htlcAmt, totalTimelock, + blob) + if err != nil { + t.Fatal(err) + } + if err := n.carolServer.registry.AddInvoice(*invoice); err != nil { + t.Fatalf("unable to add invoice in carol registry: %v", err) + } + + // With the invoice now added to Carol's registry, we'll send the + // payment. It should succeed w/o any issues as it has been crafted + // properly. + _, err = n.aliceServer.htlcSwitch.SendHTLC(n.bobServer.PubKey(), htlc, + newMockDeobfuscator()) + if err != nil { + t.Fatalf("unable to send payment to carol: %v", err) + } + + // Now, if we attempt to send the payment *again* it should be rejected + // as it's a duplicate request. + _, err = n.aliceServer.htlcSwitch.SendHTLC(n.bobServer.PubKey(), htlc, + newMockDeobfuscator()) + if err.Error() != lnwire.CodeUnknownPaymentHash.String() { + t.Fatal("error haven't been received") + } +}