From d3e206ef958cf2d427af86c5e9d521aade0b628a Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 14 Aug 2019 21:21:39 +0200 Subject: [PATCH] invoices: return accept height in hodl event This is a preparation for passing back the accept height in the incorrect payment details failure message to the sender. --- channeldb/invoices.go | 2 +- invoices/invoiceregistry.go | 28 ++++++-- invoices/invoiceregistry_test.go | 115 +++++++++++++++++++++++++++++-- 3 files changed, 133 insertions(+), 12 deletions(-) diff --git a/channeldb/invoices.go b/channeldb/invoices.go index 18f00da65..3f3dbfd9a 100644 --- a/channeldb/invoices.go +++ b/channeldb/invoices.go @@ -272,7 +272,7 @@ type InvoiceHTLC struct { // State indicates the state the invoice htlc is currently in. A // cancelled htlc isn't just removed from the invoice htlcs map, because - // we need AcceptedHeight to properly cancel the htlc back. + // we need AcceptHeight to properly cancel the htlc back. State HtlcState } diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index 39d6dec06..7efb92443 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -39,6 +39,9 @@ type HodlEvent struct { // CircuitKey is the key of the htlc for which we have a resolution // decision. CircuitKey channeldb.CircuitKey + + // AcceptHeight is the original height at which the htlc was accepted. + AcceptHeight int32 } // InvoiceRegistry is a central registry of all the outstanding invoices @@ -552,20 +555,29 @@ func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash, // If it isn't recorded, cancel htlc. if !ok { return &HodlEvent{ - CircuitKey: circuitKey, + CircuitKey: circuitKey, + AcceptHeight: currentHeight, }, nil } + // Determine accepted height of this htlc. If the htlc reached the + // invoice database (possibly in a previous call to the invoice + // registry), we'll take the original accepted height as it was recorded + // in the database. + acceptHeight := int32(invoiceHtlc.AcceptHeight) + switch invoiceHtlc.State { case channeldb.HtlcStateCancelled: return &HodlEvent{ - CircuitKey: circuitKey, + CircuitKey: circuitKey, + AcceptHeight: acceptHeight, }, nil case channeldb.HtlcStateSettled: return &HodlEvent{ - CircuitKey: circuitKey, - Preimage: &invoice.Terms.PaymentPreimage, + CircuitKey: circuitKey, + Preimage: &invoice.Terms.PaymentPreimage, + AcceptHeight: acceptHeight, }, nil case channeldb.HtlcStateAccepted: @@ -622,8 +634,9 @@ func (i *InvoiceRegistry) SettleHodlInvoice(preimage lntypes.Preimage) error { } i.notifyHodlSubscribers(HodlEvent{ - CircuitKey: key, - Preimage: &preimage, + CircuitKey: key, + Preimage: &preimage, + AcceptHeight: int32(htlc.AcceptHeight), }) } i.notifyClients(hash, invoice, invoice.Terms.State) @@ -703,7 +716,8 @@ func (i *InvoiceRegistry) CancelInvoice(payHash lntypes.Hash) error { } i.notifyHodlSubscribers(HodlEvent{ - CircuitKey: key, + CircuitKey: key, + AcceptHeight: int32(htlc.AcceptHeight), }) } i.notifyClients(payHash, invoice, channeldb.ContractCanceled) diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go index ee457c5d1..8145d3fe0 100644 --- a/invoices/invoiceregistry_test.go +++ b/invoices/invoiceregistry_test.go @@ -23,6 +23,8 @@ var ( testHtlcExpiry = uint32(5) + testInvoiceCltvDelta = uint32(4) + testFinalCltvRejectDelta = int32(4) testCurrentHeight = int32(1) @@ -121,6 +123,23 @@ func TestSettleInvoice(t *testing.T) { hodlChan := make(chan interface{}, 1) + // Try to settle invoice with an htlc that expires too soon. + event, err := registry.NotifyExitHopHtlc( + hash, testInvoice.Terms.Value, + uint32(testCurrentHeight)+testInvoiceCltvDelta-1, + testCurrentHeight, getCircuitKey(10), hodlChan, nil, + ) + if err != nil { + t.Fatal(err) + } + if event.Preimage != nil { + t.Fatal("expected cancel event") + } + if event.AcceptHeight != testCurrentHeight { + t.Fatalf("expected acceptHeight %v, but got %v", + testCurrentHeight, event.AcceptHeight) + } + // Settle invoice with a slightly higher amount. amtPaid := lnwire.MilliSatoshi(100500) _, err = registry.NotifyExitHopHtlc( @@ -159,7 +178,7 @@ func TestSettleInvoice(t *testing.T) { // Try to settle again with the same htlc id. We need this idempotent // behaviour after a restart. - event, err := registry.NotifyExitHopHtlc( + event, err = registry.NotifyExitHopHtlc( hash, amtPaid, testHtlcExpiry, testCurrentHeight, getCircuitKey(0), hodlChan, nil, ) @@ -309,7 +328,7 @@ func TestCancelInvoice(t *testing.T) { } // Notify arrival of a new htlc paying to this invoice. This should - // succeed. + // result in a cancel event. hodlChan := make(chan interface{}) event, err := registry.NotifyExitHopHtlc( hash, amt, testHtlcExpiry, testCurrentHeight, @@ -322,10 +341,15 @@ func TestCancelInvoice(t *testing.T) { if event.Preimage != nil { t.Fatal("expected cancel hodl event") } + if event.AcceptHeight != testCurrentHeight { + t.Fatalf("expected acceptHeight %v, but got %v", + testCurrentHeight, event.AcceptHeight) + } } -// TestHoldInvoice tests settling of a hold invoice and related notifications. -func TestHoldInvoice(t *testing.T) { +// TestSettleHoldInvoice tests settling of a hold invoice and related +// notifications. +func TestSettleHoldInvoice(t *testing.T) { defer timeout(t)() cdb, cleanup, err := newDB() @@ -462,6 +486,10 @@ func TestHoldInvoice(t *testing.T) { if *hodlEvent.Preimage != preimage { t.Fatal("unexpected preimage in hodl event") } + if hodlEvent.AcceptHeight != testCurrentHeight { + t.Fatalf("expected acceptHeight %v, but got %v", + testCurrentHeight, event.AcceptHeight) + } // We expect a settled notification to be sent out for both all and // single invoice subscribers. @@ -494,6 +522,85 @@ func TestHoldInvoice(t *testing.T) { } } +// TestCancelHoldInvoice tests canceling of a hold invoice and related +// notifications. +func TestCancelHoldInvoice(t *testing.T) { + defer timeout(t)() + + cdb, cleanup, err := newDB() + if err != nil { + t.Fatal(err) + } + defer cleanup() + + // Instantiate and start the invoice registry. + registry := NewRegistry(cdb, testFinalCltvRejectDelta) + + err = registry.Start() + if err != nil { + t.Fatal(err) + } + defer registry.Stop() + + // Add the invoice. + invoice := &channeldb.Invoice{ + Terms: channeldb.ContractTerm{ + PaymentPreimage: channeldb.UnknownPreimage, + Value: lnwire.MilliSatoshi(100000), + }, + } + + _, err = registry.AddInvoice(invoice, hash) + if err != nil { + t.Fatal(err) + } + + amtPaid := lnwire.MilliSatoshi(100000) + hodlChan := make(chan interface{}, 1) + + // NotifyExitHopHtlc without a preimage present in the invoice registry + // should be possible. + event, err := registry.NotifyExitHopHtlc( + hash, amtPaid, testHtlcExpiry, testCurrentHeight, + getCircuitKey(0), hodlChan, nil, + ) + if err != nil { + t.Fatalf("expected settle to succeed but got %v", err) + } + if event != nil { + t.Fatalf("expected htlc to be held") + } + + // Cancel invoice. + err = registry.CancelInvoice(hash) + if err != nil { + t.Fatal("cancel invoice failed") + } + + hodlEvent := (<-hodlChan).(HodlEvent) + if hodlEvent.Preimage != nil { + t.Fatal("expected cancel hodl event") + } + + // Offering the same htlc again at a higher height should still result + // in a rejection. The accept height is expected to be the original + // accept height. + event, err = registry.NotifyExitHopHtlc( + hash, amtPaid, testHtlcExpiry, testCurrentHeight+1, + getCircuitKey(0), hodlChan, nil, + ) + if err != nil { + t.Fatalf("expected settle to succeed but got %v", err) + } + if event.Preimage != nil { + t.Fatalf("expected htlc to be canceled") + } + if event.AcceptHeight != testCurrentHeight { + t.Fatalf("expected acceptHeight %v, but got %v", + testCurrentHeight, event.AcceptHeight) + } +} + func newDB() (*channeldb.DB, func(), error) { // First, create a temporary directory to be used for the duration of // this test.