From 9e26e4e8daf20f2147e4c708ff7dec05bb8adefa Mon Sep 17 00:00:00 2001
From: Joost Jager <joost.jager@gmail.com>
Date: Mon, 10 Jun 2019 12:20:49 +0200
Subject: [PATCH] invoices: check invoice amount even when already accepted or
 settled

---
 invoices/invoiceregistry.go      | 24 ++++++++++++++++------
 invoices/invoiceregistry_test.go | 34 ++++++++++++++++++++++++++------
 2 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go
index 0b0b0bc9..9a7add0e 100644
--- a/invoices/invoiceregistry.go
+++ b/invoices/invoiceregistry.go
@@ -526,15 +526,30 @@ func (i *InvoiceRegistry) LookupInvoice(rHash lntypes.Hash) (channeldb.Invoice,
 func (i *InvoiceRegistry) checkHtlcParameters(invoice *channeldb.Invoice,
 	amtPaid lnwire.MilliSatoshi, htlcExpiry uint32, currentHeight int32) error {
 
+	// If the invoice is already canceled, there is no further checking to
+	// do.
+	if invoice.Terms.State == channeldb.ContractCanceled {
+		return channeldb.ErrInvoiceAlreadyCanceled
+	}
+
+	// If a payment has already been made, we only accept more payments if
+	// the amount is the exact same. This prevents probing with small
+	// amounts on settled invoices to find out the receiver node.
+	if invoice.AmtPaid != 0 && amtPaid != invoice.AmtPaid {
+		return ErrInvoiceAmountTooLow
+	}
+
+	// Return early in case the invoice was already accepted or settled. We
+	// don't want to check the expiry again, because it may be that we are
+	// just restarting.
 	switch invoice.Terms.State {
 	case channeldb.ContractAccepted:
 		return channeldb.ErrInvoiceAlreadyAccepted
 	case channeldb.ContractSettled:
 		return channeldb.ErrInvoiceAlreadySettled
-	case channeldb.ContractCanceled:
-		return channeldb.ErrInvoiceAlreadyCanceled
 	}
 
+	// The invoice is still open. Check the expiry.
 	expiry, err := i.decodeFinalCltvExpiry(string(invoice.PaymentRequest))
 	if err != nil {
 		return err
@@ -548,10 +563,7 @@ func (i *InvoiceRegistry) checkHtlcParameters(invoice *channeldb.Invoice,
 		return ErrInvoiceExpiryTooSoon
 	}
 
-	// If an invoice amount is specified, check that enough is paid. This
-	// check is only performed for open invoices. Once a sufficiently large
-	// payment has been made and the invoice is in the accepted or settled
-	// state, any amount will be accepted on top of that.
+	// If an invoice amount is specified, check that enough is paid.
 	if invoice.Terms.Value > 0 && amtPaid < invoice.Terms.Value {
 		return ErrInvoiceAmountTooLow
 	}
diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go
index cb554faa..71f6bfee 100644
--- a/invoices/invoiceregistry_test.go
+++ b/invoices/invoiceregistry_test.go
@@ -148,21 +148,43 @@ func TestSettleInvoice(t *testing.T) {
 		t.Fatal("no update received")
 	}
 
-	// Try to settle again.
-	_, err = registry.NotifyExitHopHtlc(
+	// Try to settle again. We need this idempotent behaviour after a
+	// restart.
+	event, err := registry.NotifyExitHopHtlc(
 		hash, amtPaid, testInvoiceExpiry, testCurrentHeight, hodlChan,
 	)
 	if err != nil {
-		t.Fatal("expected duplicate settle to succeed")
+		t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err)
+	}
+	if event.Preimage == nil {
+		t.Fatal("expected settle event")
 	}
 
-	// Try to settle again with a different amount.
-	_, err = registry.NotifyExitHopHtlc(
+	// Try to settle again with a higher amount. This should result in a
+	// cancel event because after a restart the amount should still be the
+	// same. New HTLCs with a different amount should be rejected.
+	event, err = registry.NotifyExitHopHtlc(
 		hash, amtPaid+600, testInvoiceExpiry, testCurrentHeight,
 		hodlChan,
 	)
 	if err != nil {
-		t.Fatal("expected duplicate settle to succeed")
+		t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err)
+	}
+	if event.Preimage != nil {
+		t.Fatal("expected cancel event")
+	}
+
+	// Try to settle again with a lower amount. This should show the same
+	// behaviour as settling with a higher amount.
+	event, err = registry.NotifyExitHopHtlc(
+		hash, amtPaid-600, testInvoiceExpiry, testCurrentHeight,
+		hodlChan,
+	)
+	if err != nil {
+		t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err)
+	}
+	if event.Preimage != nil {
+		t.Fatal("expected cancel event")
 	}
 
 	// Check that settled amount remains unchanged.