From e234f88b820896e9210370e76c2aaeccc8a642d8 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Fri, 15 Nov 2019 12:18:06 +0100 Subject: [PATCH 01/10] channeldb: export now function To allow mocking when testing other packages. --- channeldb/db.go | 5 +++-- channeldb/invoice_test.go | 2 +- channeldb/invoices.go | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/channeldb/db.go b/channeldb/db.go index cf811aa6..c3f63234 100644 --- a/channeldb/db.go +++ b/channeldb/db.go @@ -137,7 +137,8 @@ type DB struct { *bbolt.DB dbPath string graph *ChannelGraph - now func() time.Time + + Now func() time.Time } // Open opens an existing channeldb. Any necessary schemas migrations due to @@ -171,7 +172,7 @@ func Open(dbPath string, modifiers ...OptionModifier) (*DB, error) { chanDB := &DB{ DB: bdb, dbPath: dbPath, - now: time.Now, + Now: time.Now, } chanDB.graph = newChannelGraph( chanDB, opts.RejectCacheSize, opts.ChannelCacheSize, diff --git a/channeldb/invoice_test.go b/channeldb/invoice_test.go index 46687e25..82d96082 100644 --- a/channeldb/invoice_test.go +++ b/channeldb/invoice_test.go @@ -337,7 +337,7 @@ func TestDuplicateSettleInvoice(t *testing.T) { if err != nil { t.Fatalf("unable to make test db: %v", err) } - db.now = func() time.Time { return time.Unix(1, 0) } + db.Now = func() time.Time { return time.Unix(1, 0) } // We'll start out by creating an invoice and writing it to the DB. amt := lnwire.NewMSatFromSatoshis(1000) diff --git a/channeldb/invoices.go b/channeldb/invoices.go index d06ef46a..c1fd4533 100644 --- a/channeldb/invoices.go +++ b/channeldb/invoices.go @@ -1244,7 +1244,7 @@ func (d *DB) updateInvoice(hash lntypes.Hash, invoices, settleIndex *bbolt.Bucke // Update invoice state. invoice.State = update.State - now := d.now() + now := d.Now() // Update htlc set. for key, htlcUpdate := range update.Htlcs { From fa010de548c71fb82814e7d5fdf2b9028d576682 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Tue, 12 Nov 2019 20:28:43 +0100 Subject: [PATCH 02/10] invoices/test: add test context This commit adds a test context for invoice registry and additionally passed in a payload object to NotifyExitHopHtlc. This makes the test match the reality better where a payload is always provided. --- invoices/invoiceregistry_test.go | 117 +++++++++++++++++++------------ 1 file changed, 71 insertions(+), 46 deletions(-) diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go index 9e6fc62a..bb14ba12 100644 --- a/invoices/invoiceregistry_test.go +++ b/invoices/invoiceregistry_test.go @@ -9,6 +9,7 @@ import ( "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" + "github.com/lightningnetwork/lnd/record" ) var ( @@ -32,10 +33,13 @@ var ( testFeatures = lnwire.NewFeatureVector( nil, lnwire.Features, ) + + testPayload = &mockPayload{} ) var ( - testInvoice = &channeldb.Invoice{ + testInvoiceAmt = lnwire.MilliSatoshi(100000) + testInvoice = &channeldb.Invoice{ Terms: channeldb.ContractTerm{ PaymentPreimage: preimage, Value: lnwire.MilliSatoshi(100000), @@ -46,19 +50,26 @@ var ( testHodlInvoice = &channeldb.Invoice{ Terms: channeldb.ContractTerm{ PaymentPreimage: channeldb.UnknownPreimage, - Value: lnwire.MilliSatoshi(100000), + Value: testInvoiceAmt, Features: testFeatures, }, } ) -func newTestContext(t *testing.T) (*InvoiceRegistry, func()) { +type testContext struct { + registry *InvoiceRegistry + + cleanup func() + t *testing.T +} + +func newTestContext(t *testing.T) *testContext { cdb, cleanup, err := newDB() if err != nil { t.Fatal(err) } - // Instantiate and start the invoice registry. + // Instantiate and start the invoice ctx.registry. registry := NewRegistry(cdb, testFinalCltvRejectDelta) err = registry.Start() @@ -67,10 +78,16 @@ func newTestContext(t *testing.T) (*InvoiceRegistry, func()) { t.Fatal(err) } - return registry, func() { - registry.Stop() - cleanup() + ctx := testContext{ + registry: registry, + t: t, + cleanup: func() { + registry.Stop() + cleanup() + }, } + + return &ctx } func getCircuitKey(htlcID uint64) channeldb.CircuitKey { @@ -84,14 +101,14 @@ func getCircuitKey(htlcID uint64) channeldb.CircuitKey { // TestSettleInvoice tests settling of an invoice and related notifications. func TestSettleInvoice(t *testing.T) { - registry, cleanup := newTestContext(t) - defer cleanup() + ctx := newTestContext(t) + defer ctx.cleanup() - allSubscriptions := registry.SubscribeNotifications(0, 0) + allSubscriptions := ctx.registry.SubscribeNotifications(0, 0) defer allSubscriptions.Cancel() // Subscribe to the not yet existing invoice. - subscription, err := registry.SubscribeSingleInvoice(hash) + subscription, err := ctx.registry.SubscribeSingleInvoice(hash) if err != nil { t.Fatal(err) } @@ -102,7 +119,7 @@ func TestSettleInvoice(t *testing.T) { } // Add the invoice. - addIdx, err := registry.AddInvoice(testInvoice, hash) + addIdx, err := ctx.registry.AddInvoice(testInvoice, hash) if err != nil { t.Fatal(err) } @@ -137,10 +154,10 @@ 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( + event, err := ctx.registry.NotifyExitHopHtlc( hash, testInvoice.Terms.Value, uint32(testCurrentHeight)+testInvoiceCltvDelta-1, - testCurrentHeight, getCircuitKey(10), hodlChan, nil, + testCurrentHeight, getCircuitKey(10), hodlChan, testPayload, ) if err != nil { t.Fatal(err) @@ -155,9 +172,9 @@ func TestSettleInvoice(t *testing.T) { // Settle invoice with a slightly higher amount. amtPaid := lnwire.MilliSatoshi(100500) - _, err = registry.NotifyExitHopHtlc( + _, err = ctx.registry.NotifyExitHopHtlc( hash, amtPaid, testHtlcExpiry, testCurrentHeight, - getCircuitKey(0), hodlChan, nil, + getCircuitKey(0), hodlChan, testPayload, ) if err != nil { t.Fatal(err) @@ -191,9 +208,9 @@ 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 = ctx.registry.NotifyExitHopHtlc( hash, amtPaid, testHtlcExpiry, testCurrentHeight, - getCircuitKey(0), hodlChan, nil, + getCircuitKey(0), hodlChan, testPayload, ) if err != nil { t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err) @@ -205,9 +222,9 @@ func TestSettleInvoice(t *testing.T) { // Try to settle again with a new higher-valued htlc. This payment // should also be accepted, to prevent any change in behaviour for a // paid invoice that may open up a probe vector. - event, err = registry.NotifyExitHopHtlc( + event, err = ctx.registry.NotifyExitHopHtlc( hash, amtPaid+600, testHtlcExpiry, testCurrentHeight, - getCircuitKey(1), hodlChan, nil, + getCircuitKey(1), hodlChan, testPayload, ) if err != nil { t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err) @@ -218,9 +235,9 @@ func TestSettleInvoice(t *testing.T) { // Try to settle again with a lower amount. This should fail just as it // would have failed if it were the first payment. - event, err = registry.NotifyExitHopHtlc( + event, err = ctx.registry.NotifyExitHopHtlc( hash, amtPaid-600, testHtlcExpiry, testCurrentHeight, - getCircuitKey(2), hodlChan, nil, + getCircuitKey(2), hodlChan, testPayload, ) if err != nil { t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err) @@ -231,7 +248,7 @@ func TestSettleInvoice(t *testing.T) { // Check that settled amount is equal to the sum of values of the htlcs // 0 and 1. - inv, err := registry.LookupInvoice(hash) + inv, err := ctx.registry.LookupInvoice(hash) if err != nil { t.Fatal(err) } @@ -240,7 +257,7 @@ func TestSettleInvoice(t *testing.T) { } // Try to cancel. - err = registry.CancelInvoice(hash) + err = ctx.registry.CancelInvoice(hash) if err != channeldb.ErrInvoiceAlreadySettled { t.Fatal("expected cancelation of a settled invoice to fail") } @@ -255,20 +272,20 @@ func TestSettleInvoice(t *testing.T) { // TestCancelInvoice tests cancelation of an invoice and related notifications. func TestCancelInvoice(t *testing.T) { - registry, cleanup := newTestContext(t) - defer cleanup() + ctx := newTestContext(t) + defer ctx.cleanup() - allSubscriptions := registry.SubscribeNotifications(0, 0) + allSubscriptions := ctx.registry.SubscribeNotifications(0, 0) defer allSubscriptions.Cancel() // Try to cancel the not yet existing invoice. This should fail. - err := registry.CancelInvoice(hash) + err := ctx.registry.CancelInvoice(hash) if err != channeldb.ErrInvoiceNotFound { t.Fatalf("expected ErrInvoiceNotFound, but got %v", err) } // Subscribe to the not yet existing invoice. - subscription, err := registry.SubscribeSingleInvoice(hash) + subscription, err := ctx.registry.SubscribeSingleInvoice(hash) if err != nil { t.Fatal(err) } @@ -280,7 +297,7 @@ func TestCancelInvoice(t *testing.T) { // Add the invoice. amt := lnwire.MilliSatoshi(100000) - _, err = registry.AddInvoice(testInvoice, hash) + _, err = ctx.registry.AddInvoice(testInvoice, hash) if err != nil { t.Fatal(err) } @@ -312,7 +329,7 @@ func TestCancelInvoice(t *testing.T) { } // Cancel invoice. - err = registry.CancelInvoice(hash) + err = ctx.registry.CancelInvoice(hash) if err != nil { t.Fatal(err) } @@ -335,7 +352,7 @@ func TestCancelInvoice(t *testing.T) { // subscribers (backwards compatibility). // Try to cancel again. - err = registry.CancelInvoice(hash) + err = ctx.registry.CancelInvoice(hash) if err != nil { t.Fatal("expected cancelation of a canceled invoice to succeed") } @@ -343,9 +360,9 @@ func TestCancelInvoice(t *testing.T) { // Notify arrival of a new htlc paying to this invoice. This should // result in a cancel event. hodlChan := make(chan interface{}) - event, err := registry.NotifyExitHopHtlc( + event, err := ctx.registry.NotifyExitHopHtlc( hash, amt, testHtlcExpiry, testCurrentHeight, - getCircuitKey(0), hodlChan, nil, + getCircuitKey(0), hodlChan, testPayload, ) if err != nil { t.Fatal("expected settlement of a canceled invoice to succeed") @@ -371,7 +388,7 @@ func TestSettleHoldInvoice(t *testing.T) { } defer cleanup() - // Instantiate and start the invoice registry. + // Instantiate and start the invoice ctx.registry. registry := NewRegistry(cdb, testFinalCltvRejectDelta) err = registry.Start() @@ -423,7 +440,7 @@ func TestSettleHoldInvoice(t *testing.T) { // should be possible. event, err := registry.NotifyExitHopHtlc( hash, amtPaid, testHtlcExpiry, testCurrentHeight, - getCircuitKey(0), hodlChan, nil, + getCircuitKey(0), hodlChan, testPayload, ) if err != nil { t.Fatalf("expected settle to succeed but got %v", err) @@ -435,7 +452,7 @@ func TestSettleHoldInvoice(t *testing.T) { // Test idempotency. event, err = registry.NotifyExitHopHtlc( hash, amtPaid, testHtlcExpiry, testCurrentHeight, - getCircuitKey(0), hodlChan, nil, + getCircuitKey(0), hodlChan, testPayload, ) if err != nil { t.Fatalf("expected settle to succeed but got %v", err) @@ -448,7 +465,7 @@ func TestSettleHoldInvoice(t *testing.T) { // is a replay. event, err = registry.NotifyExitHopHtlc( hash, amtPaid, testHtlcExpiry, testCurrentHeight+10, - getCircuitKey(0), hodlChan, nil, + getCircuitKey(0), hodlChan, testPayload, ) if err != nil { t.Fatalf("expected settle to succeed but got %v", err) @@ -461,7 +478,7 @@ func TestSettleHoldInvoice(t *testing.T) { // requirement. It should be rejected. event, err = registry.NotifyExitHopHtlc( hash, amtPaid, 1, testCurrentHeight, - getCircuitKey(1), hodlChan, nil, + getCircuitKey(1), hodlChan, testPayload, ) if err != nil { t.Fatalf("expected settle to succeed but got %v", err) @@ -539,7 +556,7 @@ func TestCancelHoldInvoice(t *testing.T) { } defer cleanup() - // Instantiate and start the invoice registry. + // Instantiate and start the invoice ctx.registry. registry := NewRegistry(cdb, testFinalCltvRejectDelta) err = registry.Start() @@ -561,7 +578,7 @@ func TestCancelHoldInvoice(t *testing.T) { // should be possible. event, err := registry.NotifyExitHopHtlc( hash, amtPaid, testHtlcExpiry, testCurrentHeight, - getCircuitKey(0), hodlChan, nil, + getCircuitKey(0), hodlChan, testPayload, ) if err != nil { t.Fatalf("expected settle to succeed but got %v", err) @@ -586,7 +603,7 @@ func TestCancelHoldInvoice(t *testing.T) { // accept height. event, err = registry.NotifyExitHopHtlc( hash, amtPaid, testHtlcExpiry, testCurrentHeight+1, - getCircuitKey(0), hodlChan, nil, + getCircuitKey(0), hodlChan, testPayload, ) if err != nil { t.Fatalf("expected settle to succeed but got %v", err) @@ -629,18 +646,26 @@ func newDB() (*channeldb.DB, func(), error) { // the exit hop, but in htlcIncomingContestResolver it is called with forwarded // htlc hashes as well. func TestUnknownInvoice(t *testing.T) { - registry, cleanup := newTestContext(t) - defer cleanup() + ctx := newTestContext(t) + defer ctx.cleanup() // Notify arrival of a new htlc paying to this invoice. This should // succeed. hodlChan := make(chan interface{}) amt := lnwire.MilliSatoshi(100000) - _, err := registry.NotifyExitHopHtlc( + _, err := ctx.registry.NotifyExitHopHtlc( hash, amt, testHtlcExpiry, testCurrentHeight, - getCircuitKey(0), hodlChan, nil, + getCircuitKey(0), hodlChan, testPayload, ) if err != channeldb.ErrInvoiceNotFound { t.Fatal("expected invoice not found error") } } + +type mockPayload struct { + mpp *record.MPP +} + +func (p *mockPayload) MultiPath() *record.MPP { + return p.mpp +} From 823b52802c96f9eb5669efb6c53a1462bfe3820f Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Tue, 3 Sep 2019 11:45:51 +0200 Subject: [PATCH 03/10] record: add mpp String() --- record/mpp.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/record/mpp.go b/record/mpp.go index b28d1085..38877caf 100644 --- a/record/mpp.go +++ b/record/mpp.go @@ -1,6 +1,7 @@ package record import ( + "fmt" "io" "github.com/lightningnetwork/lnd/lnwire" @@ -96,3 +97,8 @@ func (r *MPP) Record() tlv.Record { MPPOnionType, r, size, MPPEncoder, MPPDecoder, ) } + +// String returns a human-readable representation of the mpp payload field. +func (r *MPP) String() string { + return fmt.Sprintf("total=%v, addr=%x", r.totalMsat, r.paymentAddr) +} From 915867e90f3bab800039535e73cf19c8cc0b3921 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 27 Nov 2019 10:05:14 +0100 Subject: [PATCH 04/10] invoiceregistry: promote update closure to method This commit moves the update code into its own function as a preparation for extending the logic further for mpp. In order to make this change cleanly, structured result codes are introduced. This also prepares for a future htlc notifier rpc hook that reports htlc settle decisions to external applications. Furthermore the awkward use of errNoUpdate as a way to signal no update is removed. --- channeldb/invoices.go | 5 + invoices/invoiceregistry.go | 287 +++++++++++++++++++++++------------- 2 files changed, 191 insertions(+), 101 deletions(-) diff --git a/channeldb/invoices.go b/channeldb/invoices.go index c1fd4533..81e27394 100644 --- a/channeldb/invoices.go +++ b/channeldb/invoices.go @@ -1241,6 +1241,11 @@ func (d *DB) updateInvoice(hash lntypes.Hash, invoices, settleIndex *bbolt.Bucke return &invoice, err } + // If there is nothing to update, return early. + if update == nil { + return &invoice, nil + } + // Update invoice state. invoice.State = update.State diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index 51b57c09..3a57eaf8 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -24,9 +24,6 @@ var ( // ErrShuttingDown is returned when an operation failed because the // invoice registry is shutting down. ErrShuttingDown = errors.New("invoice registry shutting down") - - // errNoUpdate is returned when no invoice updated is required. - errNoUpdate = errors.New("no update needed") ) // HodlEvent describes how an htlc should be resolved. If HodlEvent.Preimage is @@ -439,111 +436,48 @@ func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash, rHash[:], s, amtPaid, expiry, circuitKey) } - // Default is to not update subscribers after the invoice update. - updateSubscribers := false - - updateInvoice := func(inv *channeldb.Invoice) ( - *channeldb.InvoiceUpdateDesc, error) { - - // Don't update the invoice when this is a replayed htlc. - htlc, ok := inv.Htlcs[circuitKey] - if ok { - switch htlc.State { - case channeldb.HtlcStateCanceled: - debugLog("replayed htlc to canceled invoice") - - case channeldb.HtlcStateAccepted: - debugLog("replayed htlc to accepted invoice") - - case channeldb.HtlcStateSettled: - debugLog("replayed htlc to settled invoice") - - default: - return nil, errors.New("unexpected htlc state") - } - - return nil, errNoUpdate - } - - // If the invoice is already canceled, there is no further - // checking to do. - if inv.State == channeldb.ContractCanceled { - debugLog("invoice already canceled") - return nil, errNoUpdate - } - - // If an invoice amount is specified, check that enough - // is paid. Also check this for duplicate payments if - // the invoice is already settled or accepted. - if inv.Terms.Value > 0 && amtPaid < inv.Terms.Value { - debugLog("amount too low") - return nil, errNoUpdate - } - - // The invoice is still open. Check the expiry. - if expiry < uint32(currentHeight+i.finalCltvRejectDelta) { - debugLog("expiry too soon") - return nil, errNoUpdate - } - - if expiry < uint32(currentHeight+inv.Terms.FinalCltvDelta) { - debugLog("expiry too soon") - return nil, errNoUpdate - } - - // Record HTLC in the invoice database. - newHtlcs := map[channeldb.CircuitKey]*channeldb.HtlcAcceptDesc{ - circuitKey: { - Amt: amtPaid, - Expiry: expiry, - AcceptHeight: currentHeight, - }, - } - - update := channeldb.InvoiceUpdateDesc{ - Htlcs: newHtlcs, - } - - // Don't update invoice state if we are accepting a duplicate - // payment. We do accept or settle the HTLC. - switch inv.State { - case channeldb.ContractAccepted: - debugLog("accepting duplicate payment to accepted invoice") - update.State = channeldb.ContractAccepted - return &update, nil - - case channeldb.ContractSettled: - debugLog("accepting duplicate payment to settled invoice") - update.State = channeldb.ContractSettled - return &update, nil - } - - // Check to see if we can settle or this is an hold invoice and - // we need to wait for the preimage. - holdInvoice := inv.Terms.PaymentPreimage == channeldb.UnknownPreimage - if holdInvoice { - debugLog("accepted") - update.State = channeldb.ContractAccepted - } else { - debugLog("settled") - update.Preimage = inv.Terms.PaymentPreimage - update.State = channeldb.ContractSettled - } - - updateSubscribers = true - - return &update, nil + // Create the update context containing the relevant details of the + // incoming htlc. + updateCtx := invoiceUpdateCtx{ + circuitKey: circuitKey, + amtPaid: amtPaid, + expiry: expiry, + currentHeight: currentHeight, + finalCltvRejectDelta: i.finalCltvRejectDelta, } // We'll attempt to settle an invoice matching this rHash on disk (if - // one exists). The callback will set the resolution action that is - // returned to the link or contract resolver. - invoice, err := i.cdb.UpdateInvoice(rHash, updateInvoice) - if err != nil && err != errNoUpdate { + // one exists). The callback will update the invoice state and/or htlcs. + var ( + result updateResult + updateSubscribers bool + ) + invoice, err := i.cdb.UpdateInvoice( + rHash, + func(inv *channeldb.Invoice) ( + *channeldb.InvoiceUpdateDesc, error) { + + updateDesc, res, err := updateInvoice(&updateCtx, inv) + if err != nil { + return nil, err + } + + // Only send an update if the invoice state was changed. + updateSubscribers = updateDesc != nil && + inv.State != updateDesc.State + + // Assign result to outer scope variable. + result = res + + return updateDesc, nil + }, + ) + if err != nil { debugLog(err.Error()) return nil, err } + debugLog(result.String()) if updateSubscribers { i.notifyClients(rHash, invoice, invoice.State) @@ -1043,3 +977,154 @@ func (i *InvoiceRegistry) HodlUnsubscribeAll(subscriber chan<- interface{}) { delete(i.hodlReverseSubscriptions, subscriber) } + +// updateResult is the result of the invoice update call. +type updateResult uint8 + +const ( + resultInvalid updateResult = iota + resultReplayToCanceled + resultReplayToAccepted + resultReplayToSettled + resultInvoiceAlreadyCanceled + resultAmountTooLow + resultExpiryTooSoon + resultDuplicateToAccepted + resultDuplicateToSettled + resultAccepted + resultSettled +) + +// String returns a human-readable representation of the invoice update result. +func (u updateResult) String() string { + switch u { + + case resultInvalid: + return "invalid" + + case resultReplayToCanceled: + return "replayed htlc to canceled invoice" + + case resultReplayToAccepted: + return "replayed htlc to accepted invoice" + + case resultReplayToSettled: + return "replayed htlc to settled invoice" + + case resultInvoiceAlreadyCanceled: + return "invoice already canceled" + + case resultAmountTooLow: + return "amount too low" + + case resultExpiryTooSoon: + return "expiry too soon" + + case resultDuplicateToAccepted: + return "accepting duplicate payment to accepted invoice" + + case resultDuplicateToSettled: + return "accepting duplicate payment to settled invoice" + + case resultAccepted: + return "accepted" + + case resultSettled: + return "settled" + + default: + return "unknown" + } +} + +// invoiceUpdateCtx is an object that describes the context for the invoice +// update to be carried out. +type invoiceUpdateCtx struct { + circuitKey channeldb.CircuitKey + amtPaid lnwire.MilliSatoshi + expiry uint32 + currentHeight int32 + finalCltvRejectDelta int32 +} + +// updateInvoice is a callback for DB.UpdateInvoice that contains the invoice +// settlement logic. +func updateInvoice(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( + *channeldb.InvoiceUpdateDesc, updateResult, error) { + + // Don't update the invoice when this is a replayed htlc. + htlc, ok := inv.Htlcs[ctx.circuitKey] + if ok { + switch htlc.State { + case channeldb.HtlcStateCanceled: + return nil, resultReplayToCanceled, nil + + case channeldb.HtlcStateAccepted: + return nil, resultReplayToAccepted, nil + + case channeldb.HtlcStateSettled: + return nil, resultReplayToSettled, nil + + default: + return nil, 0, errors.New("unknown htlc state") + } + } + + // If the invoice is already canceled, there is no further checking to + // do. + if inv.State == channeldb.ContractCanceled { + return nil, resultInvoiceAlreadyCanceled, nil + } + + // If an invoice amount is specified, check that enough is paid. Also + // check this for duplicate payments if the invoice is already settled + // or accepted. + if inv.Terms.Value > 0 && ctx.amtPaid < inv.Terms.Value { + return nil, resultAmountTooLow, nil + } + + // The invoice is still open. Check the expiry. + if ctx.expiry < uint32(ctx.currentHeight+ctx.finalCltvRejectDelta) { + return nil, resultExpiryTooSoon, nil + } + + if ctx.expiry < uint32(ctx.currentHeight+inv.Terms.FinalCltvDelta) { + return nil, resultExpiryTooSoon, nil + } + + // Record HTLC in the invoice database. + newHtlcs := map[channeldb.CircuitKey]*channeldb.HtlcAcceptDesc{ + ctx.circuitKey: { + Amt: ctx.amtPaid, + Expiry: ctx.expiry, + AcceptHeight: ctx.currentHeight, + }, + } + + update := channeldb.InvoiceUpdateDesc{Htlcs: newHtlcs} + + // Don't update invoice state if we are accepting a duplicate payment. + // We do accept or settle the HTLC. + switch inv.State { + case channeldb.ContractAccepted: + update.State = channeldb.ContractAccepted + return &update, resultDuplicateToAccepted, nil + + case channeldb.ContractSettled: + update.State = channeldb.ContractSettled + return &update, resultDuplicateToSettled, nil + } + + // Check to see if we can settle or this is an hold invoice and we need + // to wait for the preimage. + holdInvoice := inv.Terms.PaymentPreimage == channeldb.UnknownPreimage + if holdInvoice { + update.State = channeldb.ContractAccepted + return &update, resultAccepted, nil + } + + update.Preimage = inv.Terms.PaymentPreimage + update.State = channeldb.ContractSettled + + return &update, resultSettled, nil +} From c45891ecf7b9a1f06f6f91f25fccc35a210b4f72 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Tue, 3 Dec 2019 12:22:31 +0100 Subject: [PATCH 05/10] invoices: move update logic into separate file --- invoices/invoiceregistry.go | 151 ---------------------------------- invoices/update.go | 159 ++++++++++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 151 deletions(-) create mode 100644 invoices/update.go diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index 3a57eaf8..62998b74 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -977,154 +977,3 @@ func (i *InvoiceRegistry) HodlUnsubscribeAll(subscriber chan<- interface{}) { delete(i.hodlReverseSubscriptions, subscriber) } - -// updateResult is the result of the invoice update call. -type updateResult uint8 - -const ( - resultInvalid updateResult = iota - resultReplayToCanceled - resultReplayToAccepted - resultReplayToSettled - resultInvoiceAlreadyCanceled - resultAmountTooLow - resultExpiryTooSoon - resultDuplicateToAccepted - resultDuplicateToSettled - resultAccepted - resultSettled -) - -// String returns a human-readable representation of the invoice update result. -func (u updateResult) String() string { - switch u { - - case resultInvalid: - return "invalid" - - case resultReplayToCanceled: - return "replayed htlc to canceled invoice" - - case resultReplayToAccepted: - return "replayed htlc to accepted invoice" - - case resultReplayToSettled: - return "replayed htlc to settled invoice" - - case resultInvoiceAlreadyCanceled: - return "invoice already canceled" - - case resultAmountTooLow: - return "amount too low" - - case resultExpiryTooSoon: - return "expiry too soon" - - case resultDuplicateToAccepted: - return "accepting duplicate payment to accepted invoice" - - case resultDuplicateToSettled: - return "accepting duplicate payment to settled invoice" - - case resultAccepted: - return "accepted" - - case resultSettled: - return "settled" - - default: - return "unknown" - } -} - -// invoiceUpdateCtx is an object that describes the context for the invoice -// update to be carried out. -type invoiceUpdateCtx struct { - circuitKey channeldb.CircuitKey - amtPaid lnwire.MilliSatoshi - expiry uint32 - currentHeight int32 - finalCltvRejectDelta int32 -} - -// updateInvoice is a callback for DB.UpdateInvoice that contains the invoice -// settlement logic. -func updateInvoice(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( - *channeldb.InvoiceUpdateDesc, updateResult, error) { - - // Don't update the invoice when this is a replayed htlc. - htlc, ok := inv.Htlcs[ctx.circuitKey] - if ok { - switch htlc.State { - case channeldb.HtlcStateCanceled: - return nil, resultReplayToCanceled, nil - - case channeldb.HtlcStateAccepted: - return nil, resultReplayToAccepted, nil - - case channeldb.HtlcStateSettled: - return nil, resultReplayToSettled, nil - - default: - return nil, 0, errors.New("unknown htlc state") - } - } - - // If the invoice is already canceled, there is no further checking to - // do. - if inv.State == channeldb.ContractCanceled { - return nil, resultInvoiceAlreadyCanceled, nil - } - - // If an invoice amount is specified, check that enough is paid. Also - // check this for duplicate payments if the invoice is already settled - // or accepted. - if inv.Terms.Value > 0 && ctx.amtPaid < inv.Terms.Value { - return nil, resultAmountTooLow, nil - } - - // The invoice is still open. Check the expiry. - if ctx.expiry < uint32(ctx.currentHeight+ctx.finalCltvRejectDelta) { - return nil, resultExpiryTooSoon, nil - } - - if ctx.expiry < uint32(ctx.currentHeight+inv.Terms.FinalCltvDelta) { - return nil, resultExpiryTooSoon, nil - } - - // Record HTLC in the invoice database. - newHtlcs := map[channeldb.CircuitKey]*channeldb.HtlcAcceptDesc{ - ctx.circuitKey: { - Amt: ctx.amtPaid, - Expiry: ctx.expiry, - AcceptHeight: ctx.currentHeight, - }, - } - - update := channeldb.InvoiceUpdateDesc{Htlcs: newHtlcs} - - // Don't update invoice state if we are accepting a duplicate payment. - // We do accept or settle the HTLC. - switch inv.State { - case channeldb.ContractAccepted: - update.State = channeldb.ContractAccepted - return &update, resultDuplicateToAccepted, nil - - case channeldb.ContractSettled: - update.State = channeldb.ContractSettled - return &update, resultDuplicateToSettled, nil - } - - // Check to see if we can settle or this is an hold invoice and we need - // to wait for the preimage. - holdInvoice := inv.Terms.PaymentPreimage == channeldb.UnknownPreimage - if holdInvoice { - update.State = channeldb.ContractAccepted - return &update, resultAccepted, nil - } - - update.Preimage = inv.Terms.PaymentPreimage - update.State = channeldb.ContractSettled - - return &update, resultSettled, nil -} diff --git a/invoices/update.go b/invoices/update.go new file mode 100644 index 00000000..1b015ecf --- /dev/null +++ b/invoices/update.go @@ -0,0 +1,159 @@ +package invoices + +import ( + "errors" + + "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/lnwire" +) + +// updateResult is the result of the invoice update call. +type updateResult uint8 + +const ( + resultInvalid updateResult = iota + resultReplayToCanceled + resultReplayToAccepted + resultReplayToSettled + resultInvoiceAlreadyCanceled + resultAmountTooLow + resultExpiryTooSoon + resultDuplicateToAccepted + resultDuplicateToSettled + resultAccepted + resultSettled +) + +// String returns a human-readable representation of the invoice update result. +func (u updateResult) String() string { + switch u { + + case resultInvalid: + return "invalid" + + case resultReplayToCanceled: + return "replayed htlc to canceled invoice" + + case resultReplayToAccepted: + return "replayed htlc to accepted invoice" + + case resultReplayToSettled: + return "replayed htlc to settled invoice" + + case resultInvoiceAlreadyCanceled: + return "invoice already canceled" + + case resultAmountTooLow: + return "amount too low" + + case resultExpiryTooSoon: + return "expiry too soon" + + case resultDuplicateToAccepted: + return "accepting duplicate payment to accepted invoice" + + case resultDuplicateToSettled: + return "accepting duplicate payment to settled invoice" + + case resultAccepted: + return "accepted" + + case resultSettled: + return "settled" + + default: + return "unknown" + } +} + +// invoiceUpdateCtx is an object that describes the context for the invoice +// update to be carried out. +type invoiceUpdateCtx struct { + circuitKey channeldb.CircuitKey + amtPaid lnwire.MilliSatoshi + expiry uint32 + currentHeight int32 + finalCltvRejectDelta int32 +} + +// updateInvoice is a callback for DB.UpdateInvoice that contains the invoice +// settlement logic. +func updateInvoice(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( + *channeldb.InvoiceUpdateDesc, updateResult, error) { + + // Don't update the invoice when this is a replayed htlc. + htlc, ok := inv.Htlcs[ctx.circuitKey] + if ok { + switch htlc.State { + case channeldb.HtlcStateCanceled: + return nil, resultReplayToCanceled, nil + + case channeldb.HtlcStateAccepted: + return nil, resultReplayToAccepted, nil + + case channeldb.HtlcStateSettled: + return nil, resultReplayToSettled, nil + + default: + return nil, 0, errors.New("unknown htlc state") + } + } + + // If the invoice is already canceled, there is no further checking to + // do. + if inv.State == channeldb.ContractCanceled { + return nil, resultInvoiceAlreadyCanceled, nil + } + + // If an invoice amount is specified, check that enough is paid. Also + // check this for duplicate payments if the invoice is already settled + // or accepted. + if inv.Terms.Value > 0 && ctx.amtPaid < inv.Terms.Value { + return nil, resultAmountTooLow, nil + } + + // The invoice is still open. Check the expiry. + if ctx.expiry < uint32(ctx.currentHeight+ctx.finalCltvRejectDelta) { + return nil, resultExpiryTooSoon, nil + } + + if ctx.expiry < uint32(ctx.currentHeight+inv.Terms.FinalCltvDelta) { + return nil, resultExpiryTooSoon, nil + } + + // Record HTLC in the invoice database. + newHtlcs := map[channeldb.CircuitKey]*channeldb.HtlcAcceptDesc{ + ctx.circuitKey: { + Amt: ctx.amtPaid, + Expiry: ctx.expiry, + AcceptHeight: ctx.currentHeight, + }, + } + + update := channeldb.InvoiceUpdateDesc{Htlcs: newHtlcs} + + // Don't update invoice state if we are accepting a duplicate payment. + // We do accept or settle the HTLC. + switch inv.State { + case channeldb.ContractAccepted: + update.State = channeldb.ContractAccepted + return &update, resultDuplicateToAccepted, nil + + case channeldb.ContractSettled: + update.State = channeldb.ContractSettled + return &update, resultDuplicateToSettled, nil + } + + // Check to see if we can settle or this is an hold invoice and we need + // to wait for the preimage. + holdInvoice := inv.Terms.PaymentPreimage == channeldb.UnknownPreimage + if holdInvoice { + update.State = channeldb.ContractAccepted + return &update, resultAccepted, nil + } + + update.Preimage = inv.Terms.PaymentPreimage + update.State = channeldb.ContractSettled + + return &update, resultSettled, nil +} From a4a3c4192483a7e3b79bc676cb712b2a3ce55ee7 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 27 Nov 2019 14:19:15 +0100 Subject: [PATCH 06/10] channeldb: split cancel and add htlc updates Previously the cancel and add actions were combined in a single map. Nil values implictly signaled cancel actions. This wasn't very obvious. Furthermore this split prepares for processing the adds and cancels separately, which is more efficient if there are already two maps. --- channeldb/invoice_test.go | 2 +- channeldb/invoices.go | 45 +++++++++++++++++++------------------ invoices/invoiceregistry.go | 8 +++---- invoices/update.go | 4 +++- 4 files changed, 31 insertions(+), 28 deletions(-) diff --git a/channeldb/invoice_test.go b/channeldb/invoice_test.go index 82d96082..ac19025c 100644 --- a/channeldb/invoice_test.go +++ b/channeldb/invoice_test.go @@ -686,7 +686,7 @@ func getUpdateInvoice(amt lnwire.MilliSatoshi) InvoiceUpdateCallback { update := &InvoiceUpdateDesc{ Preimage: invoice.Terms.PaymentPreimage, State: ContractSettled, - Htlcs: map[CircuitKey]*HtlcAcceptDesc{ + AddHtlcs: map[CircuitKey]*HtlcAcceptDesc{ {}: { Amt: amt, }, diff --git a/channeldb/invoices.go b/channeldb/invoices.go index 81e27394..e6472fae 100644 --- a/channeldb/invoices.go +++ b/channeldb/invoices.go @@ -316,10 +316,12 @@ type InvoiceUpdateDesc struct { // State is the new state that this invoice should progress to. State ContractState - // Htlcs describes the changes that need to be made to the invoice htlcs - // in the database. Htlc map entries with their value set should be - // added. If the map value is nil, the htlc should be canceled. - Htlcs map[CircuitKey]*HtlcAcceptDesc + // CancelHtlcs describes the htlcs that need to be canceled. + CancelHtlcs map[CircuitKey]struct{} + + // AddHtlcs describes the newly accepted htlcs that need to be added to + // the invoice. + AddHtlcs map[CircuitKey]*HtlcAcceptDesc // Preimage must be set to the preimage when state is settled. Preimage lntypes.Preimage @@ -1251,26 +1253,25 @@ func (d *DB) updateInvoice(hash lntypes.Hash, invoices, settleIndex *bbolt.Bucke now := d.Now() - // Update htlc set. - for key, htlcUpdate := range update.Htlcs { + // Process cancel actions from update descriptor. + for key := range update.CancelHtlcs { htlc, ok := invoice.Htlcs[key] - - // No update means the htlc needs to be canceled. - if htlcUpdate == nil { - if !ok { - return nil, fmt.Errorf("unknown htlc %v", key) - } - if htlc.State != HtlcStateAccepted { - return nil, fmt.Errorf("can only cancel " + - "accepted htlcs") - } - - htlc.State = HtlcStateCanceled - htlc.ResolveTime = now - invoice.AmtPaid -= htlc.Amt - - continue + if !ok { + return nil, fmt.Errorf("unknown htlc %v", key) } + if htlc.State != HtlcStateAccepted { + return nil, fmt.Errorf("can only cancel " + + "accepted htlcs") + } + + htlc.State = HtlcStateCanceled + htlc.ResolveTime = now + invoice.AmtPaid -= htlc.Amt + } + + // Process add actions from update descriptor. + for key, htlcUpdate := range update.AddHtlcs { + htlc, ok := invoice.Htlcs[key] // Add new htlc paying to the invoice. if ok { diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index 62998b74..e36a961b 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -598,7 +598,7 @@ func (i *InvoiceRegistry) CancelInvoice(payHash lntypes.Hash) error { // Mark individual held htlcs as canceled. canceledHtlcs := make( - map[channeldb.CircuitKey]*channeldb.HtlcAcceptDesc, + map[channeldb.CircuitKey]struct{}, ) for key, htlc := range invoice.Htlcs { switch htlc.State { @@ -615,13 +615,13 @@ func (i *InvoiceRegistry) CancelInvoice(payHash lntypes.Hash) error { continue } - canceledHtlcs[key] = nil + canceledHtlcs[key] = struct{}{} } // Move invoice to the canceled state. return &channeldb.InvoiceUpdateDesc{ - Htlcs: canceledHtlcs, - State: channeldb.ContractCanceled, + CancelHtlcs: canceledHtlcs, + State: channeldb.ContractCanceled, }, nil } diff --git a/invoices/update.go b/invoices/update.go index 1b015ecf..dd438bd8 100644 --- a/invoices/update.go +++ b/invoices/update.go @@ -130,7 +130,9 @@ func updateInvoice(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( }, } - update := channeldb.InvoiceUpdateDesc{Htlcs: newHtlcs} + update := channeldb.InvoiceUpdateDesc{ + AddHtlcs: newHtlcs, + } // Don't update invoice state if we are accepting a duplicate payment. // We do accept or settle the HTLC. From 00d93ed87b5e78fd33d2947bd1e5fc4614e0832f Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 27 Nov 2019 13:20:14 +0100 Subject: [PATCH 07/10] channeldb: stricter validation of invoice updates --- channeldb/invoice_test.go | 6 +- channeldb/invoices.go | 251 ++++++++++++++++++++++++++++-------- invoices/invoiceregistry.go | 46 ++----- invoices/update.go | 12 +- 4 files changed, 221 insertions(+), 94 deletions(-) diff --git a/channeldb/invoice_test.go b/channeldb/invoice_test.go index ac19025c..55ae761e 100644 --- a/channeldb/invoice_test.go +++ b/channeldb/invoice_test.go @@ -684,8 +684,10 @@ func getUpdateInvoice(amt lnwire.MilliSatoshi) InvoiceUpdateCallback { } update := &InvoiceUpdateDesc{ - Preimage: invoice.Terms.PaymentPreimage, - State: ContractSettled, + State: &InvoiceStateUpdateDesc{ + Preimage: invoice.Terms.PaymentPreimage, + NewState: ContractSettled, + }, AddHtlcs: map[CircuitKey]*HtlcAcceptDesc{ {}: { Amt: amt, diff --git a/channeldb/invoices.go b/channeldb/invoices.go index e6472fae..73668cdc 100644 --- a/channeldb/invoices.go +++ b/channeldb/invoices.go @@ -76,6 +76,18 @@ var ( // ErrInvoiceStillOpen is returned when the invoice is still open. ErrInvoiceStillOpen = errors.New("invoice still open") + + // ErrInvoiceCannotOpen is returned when an attempt is made to move an + // invoice to the open state. + ErrInvoiceCannotOpen = errors.New("cannot move invoice to open") + + // ErrInvoiceCannotAccept is returned when an attempt is made to accept + // an invoice while the invoice is not in the open state. + ErrInvoiceCannotAccept = errors.New("cannot accept invoice") + + // ErrInvoicePreimageMismatch is returned when the preimage doesn't + // match the invoice hash. + ErrInvoicePreimageMismatch = errors.New("preimage does not match") ) const ( @@ -313,8 +325,9 @@ type HtlcAcceptDesc struct { // InvoiceUpdateDesc describes the changes that should be applied to the // invoice. type InvoiceUpdateDesc struct { - // State is the new state that this invoice should progress to. - State ContractState + // State is the new state that this invoice should progress to. If nil, + // the state is left unchanged. + State *InvoiceStateUpdateDesc // CancelHtlcs describes the htlcs that need to be canceled. CancelHtlcs map[CircuitKey]struct{} @@ -322,8 +335,14 @@ type InvoiceUpdateDesc struct { // AddHtlcs describes the newly accepted htlcs that need to be added to // the invoice. AddHtlcs map[CircuitKey]*HtlcAcceptDesc +} - // Preimage must be set to the preimage when state is settled. +// InvoiceStateUpdateDesc describes an invoice-level state transition. +type InvoiceStateUpdateDesc struct { + // NewState is the new state that this invoice should progress to. + NewState ContractState + + // Preimage must be set to the preimage when NewState is settled. Preimage lntypes.Preimage } @@ -1231,8 +1250,6 @@ func (d *DB) updateInvoice(hash lntypes.Hash, invoices, settleIndex *bbolt.Bucke return nil, err } - preUpdateState := invoice.State - // Create deep copy to prevent any accidental modification in the // callback. invoiceCopy := copyInvoice(&invoice) @@ -1248,78 +1265,97 @@ func (d *DB) updateInvoice(hash lntypes.Hash, invoices, settleIndex *bbolt.Bucke return &invoice, nil } - // Update invoice state. - invoice.State = update.State - now := d.Now() - // Process cancel actions from update descriptor. - for key := range update.CancelHtlcs { - htlc, ok := invoice.Htlcs[key] - if !ok { - return nil, fmt.Errorf("unknown htlc %v", key) - } - if htlc.State != HtlcStateAccepted { - return nil, fmt.Errorf("can only cancel " + - "accepted htlcs") + // Update invoice state if the update descriptor indicates an invoice + // state change. + if update.State != nil { + err := updateInvoiceState(&invoice, hash, *update.State) + if err != nil { + return nil, err } - htlc.State = HtlcStateCanceled - htlc.ResolveTime = now - invoice.AmtPaid -= htlc.Amt + if update.State.NewState == ContractSettled { + err := setSettleMetaFields( + settleIndex, invoiceNum, &invoice, now, + ) + if err != nil { + return nil, err + } + } } // Process add actions from update descriptor. for key, htlcUpdate := range update.AddHtlcs { - htlc, ok := invoice.Htlcs[key] - - // Add new htlc paying to the invoice. - if ok { - return nil, fmt.Errorf("htlc %v already exists", key) + if _, exists := invoice.Htlcs[key]; exists { + return nil, fmt.Errorf("duplicate add of htlc %v", key) } - htlc = &InvoiceHTLC{ + htlc := &InvoiceHTLC{ Amt: htlcUpdate.Amt, Expiry: htlcUpdate.Expiry, AcceptHeight: uint32(htlcUpdate.AcceptHeight), AcceptTime: now, - } - if preUpdateState == ContractSettled { - htlc.State = HtlcStateSettled - htlc.ResolveTime = now - } else { - htlc.State = HtlcStateAccepted + State: HtlcStateAccepted, } invoice.Htlcs[key] = htlc - invoice.AmtPaid += htlc.Amt } - // If invoice moved to the settled state, update settle index and settle - // time. - if preUpdateState != invoice.State && - invoice.State == ContractSettled { - - if update.Preimage.Hash() != hash { - return nil, fmt.Errorf("preimage does not match") - } - invoice.Terms.PaymentPreimage = update.Preimage - - // Settle all accepted htlcs. - for _, htlc := range invoice.Htlcs { - if htlc.State != HtlcStateAccepted { - continue + // Align htlc states with invoice state and recalculate amount paid. + var ( + amtPaid lnwire.MilliSatoshi + cancelHtlcs = update.CancelHtlcs + ) + for key, htlc := range invoice.Htlcs { + // Check whether this htlc needs to be canceled. If it does, + // update the htlc state to Canceled. + _, cancel := cancelHtlcs[key] + if cancel { + // Consistency check to verify that there is no overlap + // between the add and cancel sets. + if _, added := update.AddHtlcs[key]; added { + return nil, fmt.Errorf("added htlc %v canceled", + key) } - htlc.State = HtlcStateSettled - htlc.ResolveTime = now + err := cancelSingleHtlc(now, htlc, invoice.State) + if err != nil { + return nil, err + } + + // Delete processed cancel action, so that we can check + // later that there are no actions left. + delete(cancelHtlcs, key) + + continue } - err := setSettleFields(settleIndex, invoiceNum, &invoice, now) + // The invoice state may have changed and this could have + // implications for the states of the individual htlcs. Align + // the htlc state with the current invoice state. + err := updateHtlc(now, htlc, invoice.State) if err != nil { return nil, err } + + // Update the running amount paid to this invoice. We don't + // include accepted htlcs when the invoice is still open. + if invoice.State != ContractOpen && + (htlc.State == HtlcStateAccepted || + htlc.State == HtlcStateSettled) { + + amtPaid += htlc.Amt + } + } + invoice.AmtPaid = amtPaid + + // Verify that we didn't get an action for htlcs that are not present on + // the invoice. + if len(cancelHtlcs) > 0 { + return nil, errors.New("cancel action on non-existent htlc(s)") } + // Reserialize and update invoice. var buf bytes.Buffer if err := serializeInvoice(&buf, &invoice); err != nil { return nil, err @@ -1332,7 +1368,119 @@ func (d *DB) updateInvoice(hash lntypes.Hash, invoices, settleIndex *bbolt.Bucke return &invoice, nil } -func setSettleFields(settleIndex *bbolt.Bucket, invoiceNum []byte, +// updateInvoiceState validates and processes an invoice state update. +func updateInvoiceState(invoice *Invoice, hash lntypes.Hash, + update InvoiceStateUpdateDesc) error { + + // Returning to open is never allowed from any state. + if update.NewState == ContractOpen { + return ErrInvoiceCannotOpen + } + + switch invoice.State { + + // Once a contract is accepted, we can only transition to settled or + // canceled. Forbid transitioning back into this state. Otherwise this + // state is identical to ContractOpen, so we fallthrough to apply the + // same checks that we apply to open invoices. + case ContractAccepted: + if update.NewState == ContractAccepted { + return ErrInvoiceCannotAccept + } + + fallthrough + + // If a contract is open, permit a state transition to accepted, settled + // or canceled. The only restriction is on transitioning to settled + // where we ensure the preimage is valid. + case ContractOpen: + if update.NewState == ContractSettled { + // Validate preimage. + if update.Preimage.Hash() != hash { + return ErrInvoicePreimageMismatch + } + invoice.Terms.PaymentPreimage = update.Preimage + } + + // Once settled, we are in a terminal state. + case ContractSettled: + return ErrInvoiceAlreadySettled + + // Once canceled, we are in a terminal state. + case ContractCanceled: + return ErrInvoiceAlreadyCanceled + + default: + return errors.New("unknown state transition") + } + + invoice.State = update.NewState + + return nil +} + +// cancelSingleHtlc validates cancelation of a single htlc and update its state. +func cancelSingleHtlc(resolveTime time.Time, htlc *InvoiceHTLC, + invState ContractState) error { + + // It is only possible to cancel individual htlcs on an open invoice. + if invState != ContractOpen { + return fmt.Errorf("htlc canceled on invoice in "+ + "state %v", invState) + } + + // It is only possible if the htlc is still pending. + if htlc.State != HtlcStateAccepted { + return fmt.Errorf("htlc canceled in state %v", + htlc.State) + } + + htlc.State = HtlcStateCanceled + htlc.ResolveTime = resolveTime + + return nil +} + +// updateHtlc aligns the state of an htlc with the given invoice state. +func updateHtlc(resolveTime time.Time, htlc *InvoiceHTLC, + invState ContractState) error { + + switch invState { + + case ContractSettled: + if htlc.State == HtlcStateAccepted { + htlc.State = HtlcStateSettled + htlc.ResolveTime = resolveTime + } + + case ContractCanceled: + switch htlc.State { + + case HtlcStateAccepted: + htlc.State = HtlcStateCanceled + htlc.ResolveTime = resolveTime + + case HtlcStateSettled: + return fmt.Errorf("cannot have a settled htlc with " + + "invoice in state canceled") + } + + case ContractOpen, ContractAccepted: + if htlc.State == HtlcStateSettled { + return fmt.Errorf("cannot have a settled htlc with "+ + "invoice in state %v", invState) + } + + default: + return errors.New("unknown state transition") + } + + return nil +} + +// setSettleMetaFields updates the metadata associated with settlement of an +// invoice. +func setSettleMetaFields(settleIndex *bbolt.Bucket, invoiceNum []byte, invoice *Invoice, now time.Time) error { // Now that we know the invoice hasn't already been settled, we'll @@ -1349,7 +1497,6 @@ func setSettleFields(settleIndex *bbolt.Bucket, invoiceNum []byte, return err } - invoice.State = ContractSettled invoice.SettleDate = now invoice.SettleIndex = nextSettleSeqNo diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index e36a961b..9f56fe7d 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -464,7 +464,7 @@ func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash, // Only send an update if the invoice state was changed. updateSubscribers = updateDesc != nil && - inv.State != updateDesc.State + updateDesc.State != nil // Assign result to outer scope variable. result = res @@ -541,8 +541,10 @@ func (i *InvoiceRegistry) SettleHodlInvoice(preimage lntypes.Preimage) error { } return &channeldb.InvoiceUpdateDesc{ - State: channeldb.ContractSettled, - Preimage: preimage, + State: &channeldb.InvoiceStateUpdateDesc{ + NewState: channeldb.ContractSettled, + Preimage: preimage, + }, }, nil } @@ -589,39 +591,13 @@ func (i *InvoiceRegistry) CancelInvoice(payHash lntypes.Hash) error { updateInvoice := func(invoice *channeldb.Invoice) ( *channeldb.InvoiceUpdateDesc, error) { - switch invoice.State { - case channeldb.ContractSettled: - return nil, channeldb.ErrInvoiceAlreadySettled - case channeldb.ContractCanceled: - return nil, channeldb.ErrInvoiceAlreadyCanceled - } - - // Mark individual held htlcs as canceled. - canceledHtlcs := make( - map[channeldb.CircuitKey]struct{}, - ) - for key, htlc := range invoice.Htlcs { - switch htlc.State { - - // If we get here, there shouldn't be any settled htlcs. - case channeldb.HtlcStateSettled: - return nil, errors.New("cannot cancel " + - "invoice with settled htlc(s)") - - // Don't cancel htlcs that were already canceled, - // because it would incorrectly modify the invoice paid - // amt. - case channeldb.HtlcStateCanceled: - continue - } - - canceledHtlcs[key] = struct{}{} - } - - // Move invoice to the canceled state. + // Move invoice to the canceled state. Rely on validation in + // channeldb to return an error if the invoice is already + // settled or canceled. return &channeldb.InvoiceUpdateDesc{ - CancelHtlcs: canceledHtlcs, - State: channeldb.ContractCanceled, + State: &channeldb.InvoiceStateUpdateDesc{ + NewState: channeldb.ContractCanceled, + }, }, nil } diff --git a/invoices/update.go b/invoices/update.go index dd438bd8..467f17fc 100644 --- a/invoices/update.go +++ b/invoices/update.go @@ -138,11 +138,9 @@ func updateInvoice(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( // We do accept or settle the HTLC. switch inv.State { case channeldb.ContractAccepted: - update.State = channeldb.ContractAccepted return &update, resultDuplicateToAccepted, nil case channeldb.ContractSettled: - update.State = channeldb.ContractSettled return &update, resultDuplicateToSettled, nil } @@ -150,12 +148,16 @@ func updateInvoice(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( // to wait for the preimage. holdInvoice := inv.Terms.PaymentPreimage == channeldb.UnknownPreimage if holdInvoice { - update.State = channeldb.ContractAccepted + update.State = &channeldb.InvoiceStateUpdateDesc{ + NewState: channeldb.ContractAccepted, + } return &update, resultAccepted, nil } - update.Preimage = inv.Terms.PaymentPreimage - update.State = channeldb.ContractSettled + update.State = &channeldb.InvoiceStateUpdateDesc{ + NewState: channeldb.ContractSettled, + Preimage: inv.Terms.PaymentPreimage, + } return &update, resultSettled, nil } From 6c07902a63299c1fc78ff9ddd933d43dc5f043b5 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Tue, 3 Dec 2019 12:13:10 +0100 Subject: [PATCH 08/10] channeldb/test: add single htlc cancel test --- channeldb/invoice_test.go | 64 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/channeldb/invoice_test.go b/channeldb/invoice_test.go index 55ae761e..d244f31b 100644 --- a/channeldb/invoice_test.go +++ b/channeldb/invoice_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/davecgh/go-spew/spew" + "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" ) @@ -184,6 +185,69 @@ func TestInvoiceWorkflow(t *testing.T) { } } +// TestInvoiceCancelSingleHtlc tests that a single htlc can be canceled on the +// invoice. +func TestInvoiceCancelSingleHtlc(t *testing.T) { + t.Parallel() + + db, cleanUp, err := makeTestDB() + defer cleanUp() + if err != nil { + t.Fatalf("unable to make test db: %v", err) + } + + testInvoice := &Invoice{ + Htlcs: map[CircuitKey]*InvoiceHTLC{}, + } + testInvoice.Terms.Value = lnwire.NewMSatFromSatoshis(10000) + testInvoice.Terms.Features = emptyFeatures + + var paymentHash lntypes.Hash + if _, err := db.AddInvoice(testInvoice, paymentHash); err != nil { + t.Fatalf("unable to find invoice: %v", err) + } + + // Accept an htlc on this invoice. + key := CircuitKey{ChanID: lnwire.NewShortChanIDFromInt(1), HtlcID: 4} + invoice, err := db.UpdateInvoice(paymentHash, + func(invoice *Invoice) (*InvoiceUpdateDesc, error) { + return &InvoiceUpdateDesc{ + AddHtlcs: map[CircuitKey]*HtlcAcceptDesc{ + key: { + Amt: 500, + }, + }, + }, nil + }) + if err != nil { + t.Fatalf("unable to add invoice htlc: %v", err) + } + if len(invoice.Htlcs) != 1 { + t.Fatalf("expected the htlc to be added") + } + if invoice.Htlcs[key].State != HtlcStateAccepted { + t.Fatalf("expected htlc in state accepted") + } + + // Cancel the htlc again. + invoice, err = db.UpdateInvoice(paymentHash, func(invoice *Invoice) (*InvoiceUpdateDesc, error) { + return &InvoiceUpdateDesc{ + CancelHtlcs: map[CircuitKey]struct{}{ + key: {}, + }, + }, nil + }) + if err != nil { + t.Fatalf("unable to cancel htlc: %v", err) + } + if len(invoice.Htlcs) != 1 { + t.Fatalf("expected the htlc to be present") + } + if invoice.Htlcs[key].State != HtlcStateCanceled { + t.Fatalf("expected htlc in state canceled") + } +} + // TestInvoiceTimeSeries tests that newly added invoices invoices, as well as // settled invoices are added to the database are properly placed in the add // add or settle index which serves as an event time series. From a33474ca0e7224ac339b20d6a1e9724cbd32b522 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Tue, 3 Dec 2019 12:33:14 +0100 Subject: [PATCH 09/10] invoices: update NotifyExitHop method comment --- invoices/invoiceregistry.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index 9f56fe7d..f388a666 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -412,9 +412,8 @@ func (i *InvoiceRegistry) LookupInvoice(rHash lntypes.Hash) (channeldb.Invoice, return i.cdb.LookupInvoice(rHash) } -// NotifyExitHopHtlc attempts to mark an invoice as settled. If the invoice is a -// debug invoice, then this method is a noop as debug invoices are never fully -// settled. The return value describes how the htlc should be resolved. +// NotifyExitHopHtlc attempts to mark an invoice as settled. The return value +// describes how the htlc should be resolved. // // When the preimage of the invoice is not yet known (hodl invoice), this // function moves the invoice to the accepted state. When SettleHoldInvoice is From 970187ace4ca39d938d676f901e395986f0a9e3d Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Tue, 3 Dec 2019 20:41:46 +0100 Subject: [PATCH 10/10] invoices: remove unnecessary invoice value check --- invoices/update.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/invoices/update.go b/invoices/update.go index 467f17fc..7a1b0324 100644 --- a/invoices/update.go +++ b/invoices/update.go @@ -107,8 +107,9 @@ func updateInvoice(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( // If an invoice amount is specified, check that enough is paid. Also // check this for duplicate payments if the invoice is already settled - // or accepted. - if inv.Terms.Value > 0 && ctx.amtPaid < inv.Terms.Value { + // or accepted. In case this is a zero-valued invoice, it will always be + // enough. + if ctx.amtPaid < inv.Terms.Value { return nil, resultAmountTooLow, nil }