From cff4d3547d06349799827bc49b756b2253e4c13e Mon Sep 17 00:00:00 2001
From: "Johan T. Halseth" <johanth@gmail.com>
Date: Thu, 16 May 2019 15:27:28 +0200
Subject: [PATCH] htlcswitch/switch: clarify paymentID uniqueness

With the following commits, it'll become important to not resuse
paymentIDs, since there is no way to tell whether the HTLC in question
has already been forwarded and settled/failed.

We clarify this in the SendHTLC comments, and alter the tests to not
attempt to resend an HTLC with a duplicate payment ID.
---
 htlcswitch/switch.go      |  3 ++-
 htlcswitch/switch_test.go | 28 +---------------------------
 2 files changed, 3 insertions(+), 28 deletions(-)

diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go
index 27c4e7ca..a41081ce 100644
--- a/htlcswitch/switch.go
+++ b/htlcswitch/switch.go
@@ -348,7 +348,8 @@ func (s *Switch) ProcessContractResolution(msg contractcourt.ResolutionMsg) erro
 
 // SendHTLC is used by other subsystems which aren't belong to htlc switch
 // package in order to send the htlc update. The paymentID used MUST be unique
-// for this HTLC, otherwise the switch might reject it.
+// for this HTLC, and MUST be used only once, otherwise the switch might reject
+// it.
 func (s *Switch) SendHTLC(firstHop lnwire.ShortChannelID, paymentID uint64,
 	htlc *lnwire.UpdateAddHTLC,
 	deobfuscator ErrorDecrypter) ([sha256.Size]byte, error) {
diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go
index c6ea60a2..73aee0ad 100644
--- a/htlcswitch/switch_test.go
+++ b/htlcswitch/switch_test.go
@@ -1748,16 +1748,6 @@ func TestSwitchSendPayment(t *testing.T) {
 		errChan <- err
 	}()
 
-	go func() {
-		// Send the payment with the same payment hash and same
-		// amount and check that it will be propagated successfully
-		_, err := s.SendHTLC(
-			aliceChannelLink.ShortChanID(), 0, update,
-			newMockDeobfuscator(),
-		)
-		errChan <- err
-	}()
-
 	select {
 	case packet := <-aliceChannelLink.packets:
 		if err := aliceChannelLink.completeCircuit(packet); err != nil {
@@ -1765,29 +1755,13 @@ func TestSwitchSendPayment(t *testing.T) {
 		}
 
 	case err := <-errChan:
-		if err != ErrPaymentInFlight {
+		if err != nil {
 			t.Fatalf("unable to send payment: %v", err)
 		}
 	case <-time.After(time.Second):
 		t.Fatal("request was not propagated to destination")
 	}
 
-	select {
-	case packet := <-aliceChannelLink.packets:
-		if err := aliceChannelLink.completeCircuit(packet); err != nil {
-			t.Fatalf("unable to complete payment circuit: %v", err)
-		}
-
-	case err := <-errChan:
-		t.Fatalf("unable to send payment: %v", err)
-	case <-time.After(time.Second):
-		t.Fatal("request was not propagated to destination")
-	}
-
-	if s.numPendingPayments() != 1 {
-		t.Fatal("wrong amount of pending payments")
-	}
-
 	if s.circuits.NumOpen() != 1 {
 		t.Fatal("wrong amount of circuits")
 	}