From 9f824fe1ee8cb866b27d29e7a62bf968c0e080f5 Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 12 Aug 2025 18:14:09 +0200 Subject: [PATCH 01/16] multi: introduce interface for payment database --- config_builder.go | 6 +-- payments/db/interface.go | 91 ++++++++++++++++++++++++++++++++++++++++ routing/control_tower.go | 4 +- rpcserver.go | 6 +-- server.go | 10 ++--- 5 files changed, 103 insertions(+), 14 deletions(-) create mode 100644 payments/db/interface.go diff --git a/config_builder.go b/config_builder.go index be56f962b59..af4a06ed38c 100644 --- a/config_builder.go +++ b/config_builder.go @@ -925,9 +925,9 @@ type DatabaseInstances struct { // InvoiceDB is the database that stores information about invoices. InvoiceDB invoices.InvoiceDB - // KVPaymentsDB is the database that stores all payment related + // PaymentsDB is the database that stores all payment related // information. - KVPaymentsDB *paymentsdb.KVPaymentsDB + PaymentsDB paymentsdb.DB // MacaroonDB is the database that stores macaroon root keys. MacaroonDB kvdb.Backend @@ -1237,7 +1237,7 @@ func (d *DefaultDatabaseBuilder) BuildDatabase( return nil, nil, err } - dbs.KVPaymentsDB = kvPaymentsDB + dbs.PaymentsDB = kvPaymentsDB // Wrap the watchtower client DB and make sure we clean up. if cfg.WtClient.Active { diff --git a/payments/db/interface.go b/payments/db/interface.go new file mode 100644 index 00000000000..d3a7e7671fb --- /dev/null +++ b/payments/db/interface.go @@ -0,0 +1,91 @@ +package paymentsdb + +import ( + "context" + + "github.com/lightningnetwork/lnd/lntypes" +) + +// DB represents the interface to the underlying payments database. +type DB interface { + PaymentReader + PaymentWriter +} + +// PaymentReader represents the interface to read operations from the payments +// database. +type PaymentReader interface { + // QueryPayments queries the payments database and should support + // pagination. + QueryPayments(ctx context.Context, query Query) (Response, error) + + // FetchPayment fetches the payment corresponding to the given payment + // hash. + FetchPayment(paymentHash lntypes.Hash) (*MPPayment, error) + + // FetchInFlightPayments returns all payments with status InFlight. + FetchInFlightPayments() ([]*MPPayment, error) +} + +// PaymentWriter represents the interface to write operations to the payments +// database. +type PaymentWriter interface { + // DeletePayment deletes a payment from the DB given its payment hash. + DeletePayment(paymentHash lntypes.Hash, failedAttemptsOnly bool) error + + // DeletePayments deletes all payments from the DB given the specified + // flags. + DeletePayments(failedOnly, failedAttemptsOnly bool) (int, error) + + PaymentControl +} + +// PaymentControl represents the interface to control the payment lifecycle and +// its database operations. This interface represents the control flow of how +// a payment should be handled in the database. They are not just writing +// operations but they inherently represent the flow of a payment. The methods +// are called in the following order: +// +// 1. InitPayment +// 2. RegisterAttempt (a payment can have multiple attempts) +// 3. SettleAttempt or FailAttempt (attempts can also fail as long as the +// sending amount will be eventually settled). +// 4. Payment succeeds or "Fail" is called. +// 5. DeleteFailedAttempts is called which will delete all failed attempts +// for a payment to clean up the database. +type PaymentControl interface { + // InitPayment checks that no other payment with the same payment hash + // exists in the database before creating a new payment. However, it + // should allow the user making a subsequent payment if the payment is + // in a Failed state. + InitPayment(lntypes.Hash, *PaymentCreationInfo) error + + // RegisterAttempt atomically records the provided HTLCAttemptInfo. + RegisterAttempt(lntypes.Hash, *HTLCAttemptInfo) (*MPPayment, error) + + // SettleAttempt marks the given attempt settled with the preimage. If + // this is a multi shard payment, this might implicitly mean the + // full payment succeeded. + // + // After invoking this method, InitPayment should always return an + // error to prevent us from making duplicate payments to the same + // payment hash. The provided preimage is atomically saved to the DB + // for record keeping. + SettleAttempt(lntypes.Hash, uint64, *HTLCSettleInfo) (*MPPayment, error) + + // FailAttempt marks the given payment attempt failed. + FailAttempt(lntypes.Hash, uint64, *HTLCFailInfo) (*MPPayment, error) + + // Fail transitions a payment into the Failed state, and records + // the ultimate reason the payment failed. Note that this should only + // be called when all active attempts are already failed. After + // invoking this method, InitPayment should return nil on its next call + // for this payment hash, allowing the user to make a subsequent + // payment. + Fail(lntypes.Hash, FailureReason) (*MPPayment, error) + + // DeleteFailedAttempts removes all failed HTLCs from the db. It should + // be called for a given payment whenever all inflight htlcs are + // completed, and the payment has reached a final terminal state. + DeleteFailedAttempts(lntypes.Hash) error +} diff --git a/routing/control_tower.go b/routing/control_tower.go index c5af230f7ac..dd984c912b6 100644 --- a/routing/control_tower.go +++ b/routing/control_tower.go @@ -151,7 +151,7 @@ func (s *controlTowerSubscriberImpl) Updates() <-chan interface{} { // controlTower is persistent implementation of ControlTower to restrict // double payment sending. type controlTower struct { - db *paymentsdb.KVPaymentsDB + db paymentsdb.DB // subscriberIndex is used to provide a unique id for each subscriber // to all payments. This is used to easily remove the subscriber when @@ -168,7 +168,7 @@ type controlTower struct { } // NewControlTower creates a new instance of the controlTower. -func NewControlTower(db *paymentsdb.KVPaymentsDB) ControlTower { +func NewControlTower(db paymentsdb.DB) ControlTower { return &controlTower{ db: db, subscribersAllPayments: make( diff --git a/rpcserver.go b/rpcserver.go index 8b4c1b1b566..7add215799f 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -7529,7 +7529,7 @@ func (r *rpcServer) ListPayments(ctx context.Context, query.MaxPayments = math.MaxUint64 } - paymentsQuerySlice, err := r.server.kvPaymentsDB.QueryPayments( + paymentsQuerySlice, err := r.server.paymentsDB.QueryPayments( ctx, query, ) if err != nil { @@ -7612,7 +7612,7 @@ func (r *rpcServer) DeletePayment(ctx context.Context, rpcsLog.Infof("[DeletePayment] payment_identifier=%v, "+ "failed_htlcs_only=%v", hash, req.FailedHtlcsOnly) - err = r.server.kvPaymentsDB.DeletePayment(hash, req.FailedHtlcsOnly) + err = r.server.paymentsDB.DeletePayment(hash, req.FailedHtlcsOnly) if err != nil { return nil, err } @@ -7652,7 +7652,7 @@ func (r *rpcServer) DeleteAllPayments(ctx context.Context, "failed_htlcs_only=%v", req.FailedPaymentsOnly, req.FailedHtlcsOnly) - numDeletedPayments, err := r.server.kvPaymentsDB.DeletePayments( + numDeletedPayments, err := r.server.paymentsDB.DeletePayments( req.FailedPaymentsOnly, req.FailedHtlcsOnly, ) if err != nil { diff --git a/server.go b/server.go index b45c5b82303..cfbc55da7be 100644 --- a/server.go +++ b/server.go @@ -336,11 +336,9 @@ type server struct { invoicesDB invoices.InvoiceDB - // kvPaymentsDB is the DB that contains all functions for managing + // paymentsDB is the DB that contains all functions for managing // payments. - // - // TODO(ziggie): Replace with interface. - kvPaymentsDB *paymentsdb.KVPaymentsDB + paymentsDB paymentsdb.DB aliasMgr *aliasmgr.Manager @@ -683,7 +681,7 @@ func newServer(ctx context.Context, cfg *Config, listenAddrs []net.Addr, addrSource: addrSource, miscDB: dbs.ChanStateDB, invoicesDB: dbs.InvoiceDB, - kvPaymentsDB: dbs.KVPaymentsDB, + paymentsDB: dbs.PaymentsDB, cc: cc, sigPool: lnwallet.NewSigPool(cfg.Workers.Sig, cc.Signer), writePool: writePool, @@ -1007,7 +1005,7 @@ func newServer(ctx context.Context, cfg *Config, listenAddrs []net.Addr, PathFindingConfig: pathFindingConfig, } - s.controlTower = routing.NewControlTower(dbs.KVPaymentsDB) + s.controlTower = routing.NewControlTower(dbs.PaymentsDB) strictPruning := cfg.Bitcoin.Node == "neutrino" || cfg.Routing.StrictZombiePruning From 46500f94e09085b9e86dc122dab0cb13b4f7016a Mon Sep 17 00:00:00 2001 From: ziggie Date: Wed, 13 Aug 2025 13:45:10 +0200 Subject: [PATCH 02/16] multi: fix comment of InitPayment method --- routing/control_tower.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routing/control_tower.go b/routing/control_tower.go index dd984c912b6..fcbfbe80ed9 100644 --- a/routing/control_tower.go +++ b/routing/control_tower.go @@ -47,8 +47,8 @@ type DBMPPayment interface { // restarts. Payments are transitioned through various payment states, and the // ControlTower interface provides access to driving the state transitions. type ControlTower interface { - // This method checks that no succeeded payment exist for this payment - // hash. + // InitPayment initializes a new payment with the given payment hash and + // also notifies subscribers of the payment creation. InitPayment(lntypes.Hash, *paymentsdb.PaymentCreationInfo) error // DeleteFailedAttempts removes all failed HTLCs from the db. It should From 39b7417797be6258ef6132c8b482a962c54ba2f9 Mon Sep 17 00:00:00 2001 From: ziggie Date: Wed, 13 Aug 2025 14:56:49 +0200 Subject: [PATCH 03/16] paymentsdb: use querypayments method to make test db agnostic --- payments/db/kv_store_test.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/payments/db/kv_store_test.go b/payments/db/kv_store_test.go index 49e80df136a..04cd7264ca8 100644 --- a/payments/db/kv_store_test.go +++ b/payments/db/kv_store_test.go @@ -2,11 +2,13 @@ package paymentsdb import ( "bytes" + "context" "crypto/rand" "crypto/sha256" "errors" "fmt" "io" + "math" "reflect" "testing" "time" @@ -1372,9 +1374,21 @@ func assertPayments(t *testing.T, paymentDB *KVPaymentsDB, t.Helper() - dbPayments, err := paymentDB.FetchPayments() + ctx := context.Background() + + // We use the query method to fetch payments from the database which + // allows us to use this method db agnostic. We fetch all payments in + // one go. + queryResp, err := paymentDB.QueryPayments(ctx, Query{ + IndexOffset: 0, + MaxPayments: math.MaxUint64, + Reversed: false, + IncludeIncomplete: true, + }) require.NoError(t, err, "could not fetch payments from db") + dbPayments := queryResp.Payments + // Make sure that the number of fetched payments is the same // as expected. require.Len( From e22b898c1e26ec3c6e8a5a0d9934f0f602171b09 Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 12 Aug 2025 18:14:57 +0200 Subject: [PATCH 04/16] paymentsdb: move db interface dependant tests to different file This commit starts reusing test cases which are not dependant on the kv db backend. So they can be later used with the native db implementation as well. --- payments/db/kv_store_test.go | 997 ++++++++++++++--------------------- payments/db/payment_test.go | 958 ++++++++++++++++++++------------- payments/db/test_kvdb.go | 19 +- 3 files changed, 1000 insertions(+), 974 deletions(-) diff --git a/payments/db/kv_store_test.go b/payments/db/kv_store_test.go index 04cd7264ca8..e2a7d552b38 100644 --- a/payments/db/kv_store_test.go +++ b/payments/db/kv_store_test.go @@ -3,11 +3,8 @@ package paymentsdb import ( "bytes" "context" - "crypto/rand" - "crypto/sha256" "errors" "fmt" - "io" "math" "reflect" "testing" @@ -25,47 +22,13 @@ import ( "github.com/stretchr/testify/require" ) -func genPreimage() ([32]byte, error) { - var preimage [32]byte - if _, err := io.ReadFull(rand.Reader, preimage[:]); err != nil { - return preimage, err - } - return preimage, nil -} - -func genInfo(t *testing.T) (*PaymentCreationInfo, *HTLCAttemptInfo, - lntypes.Preimage, error) { - - preimage, err := genPreimage() - if err != nil { - return nil, nil, preimage, fmt.Errorf("unable to "+ - "generate preimage: %v", err) - } - - rhash := sha256.Sum256(preimage[:]) - var hash lntypes.Hash - copy(hash[:], rhash[:]) - - attempt, err := NewHtlcAttempt( - 0, priv, *testRoute.Copy(), time.Time{}, &hash, - ) - require.NoError(t, err) - - return &PaymentCreationInfo{ - PaymentIdentifier: rhash, - Value: testRoute.ReceiverAmt(), - CreationTime: time.Unix(time.Now().Unix(), 0), - PaymentRequest: []byte("hola"), - }, &attempt.HTLCAttemptInfo, preimage, nil -} - // TestKVPaymentsDBSwitchFail checks that payment status returns to Failed // status after failing, and that InitPayment allows another HTLC for the // same payment hash. func TestKVPaymentsDBSwitchFail(t *testing.T) { t.Parallel() - paymentDB := NewTestDB(t) + paymentDB := NewKVTestDB(t) info, attempt, preimg, err := genInfo(t) require.NoError(t, err, "unable to generate htlc message") @@ -202,7 +165,7 @@ func TestKVPaymentsDBSwitchFail(t *testing.T) { func TestKVPaymentsDBSwitchDoubleSend(t *testing.T) { t.Parallel() - paymentDB := NewTestDB(t) + paymentDB := NewKVTestDB(t) info, attempt, preimg, err := genInfo(t) require.NoError(t, err, "unable to generate htlc message") @@ -270,49 +233,12 @@ func TestKVPaymentsDBSwitchDoubleSend(t *testing.T) { } } -// TestKVPaymentsDBSuccessesWithoutInFlight checks that the payment -// control will disallow calls to Success when no payment is in flight. -func TestKVPaymentsDBSuccessesWithoutInFlight(t *testing.T) { - t.Parallel() - - paymentDB := NewTestDB(t) - - info, _, preimg, err := genInfo(t) - require.NoError(t, err, "unable to generate htlc message") - - // Attempt to complete the payment should fail. - _, err = paymentDB.SettleAttempt( - info.PaymentIdentifier, 0, - &HTLCSettleInfo{ - Preimage: preimg, - }, - ) - require.ErrorIs(t, err, ErrPaymentNotInitiated) -} - -// TestKVPaymentsDBFailsWithoutInFlight checks that a strict payment -// control will disallow calls to Fail when no payment is in flight. -func TestKVPaymentsDBFailsWithoutInFlight(t *testing.T) { - t.Parallel() - - paymentDB := NewTestDB(t) - - info, _, _, err := genInfo(t) - require.NoError(t, err, "unable to generate htlc message") - - // Calling Fail should return an error. - _, err = paymentDB.Fail( - info.PaymentIdentifier, FailureReasonNoRoute, - ) - require.ErrorIs(t, err, ErrPaymentNotInitiated) -} - // TestKVPaymentsDBDeleteNonInFlight checks that calling DeletePayments only // deletes payments from the database that are not in-flight. func TestKVPaymentsDBDeleteNonInFlight(t *testing.T) { t.Parallel() - paymentDB := NewTestDB(t) + paymentDB := NewKVTestDB(t) // Create a sequence number for duplicate payments that will not collide // with the sequence numbers for the payments we create. These values @@ -370,7 +296,8 @@ func TestKVPaymentsDBDeleteNonInFlight(t *testing.T) { HTLCAttemptInfo: attempt, } - if p.failed { + switch { + case p.failed: // Fail the payment attempt. htlcFailure := HTLCFailUnreadable _, err := paymentDB.FailAttempt( @@ -403,7 +330,8 @@ func TestKVPaymentsDBDeleteNonInFlight(t *testing.T) { t, paymentDB, info.PaymentIdentifier, info, &failReason, htlc, ) - } else if p.success { + + case p.success: // Verifies that status was changed to StatusSucceeded. _, err := paymentDB.SettleAttempt( info.PaymentIdentifier, attempt.AttemptID, @@ -428,7 +356,8 @@ func TestKVPaymentsDBDeleteNonInFlight(t *testing.T) { ) numSuccess++ - } else { + + default: assertPaymentStatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, @@ -527,148 +456,6 @@ func TestKVPaymentsDBDeleteNonInFlight(t *testing.T) { require.Equal(t, 1, indexCount) } -// TestKVPaymentsDBDeletePayments tests that DeletePayments correctly deletes -// information about completed payments from the database. -func TestKVPaymentsDBDeletePayments(t *testing.T) { - t.Parallel() - - paymentDB := NewTestDB(t) - - // Register three payments: - // 1. A payment with two failed attempts. - // 2. A payment with one failed and one settled attempt. - // 3. A payment with one failed and one in-flight attempt. - payments := []*payment{ - {status: StatusFailed}, - {status: StatusSucceeded}, - {status: StatusInFlight}, - } - - // Use helper function to register the test payments in the data and - // populate the data to the payments slice. - createTestPayments(t, paymentDB, payments) - - // Check that all payments are there as we added them. - assertPayments(t, paymentDB, payments) - - // Delete HTLC attempts for failed payments only. - numPayments, err := paymentDB.DeletePayments(true, true) - require.NoError(t, err) - require.EqualValues(t, 0, numPayments) - - // The failed payment is the only altered one. - payments[0].htlcs = 0 - assertPayments(t, paymentDB, payments) - - // Delete failed attempts for all payments. - numPayments, err = paymentDB.DeletePayments(false, true) - require.NoError(t, err) - require.EqualValues(t, 0, numPayments) - - // The failed attempts should be deleted, except for the in-flight - // payment, that shouldn't be altered until it has completed. - payments[1].htlcs = 1 - assertPayments(t, paymentDB, payments) - - // Now delete all failed payments. - numPayments, err = paymentDB.DeletePayments(true, false) - require.NoError(t, err) - require.EqualValues(t, 1, numPayments) - - assertPayments(t, paymentDB, payments[1:]) - - // Finally delete all completed payments. - numPayments, err = paymentDB.DeletePayments(false, false) - require.NoError(t, err) - require.EqualValues(t, 1, numPayments) - - assertPayments(t, paymentDB, payments[2:]) -} - -// TestKVPaymentsDBDeleteSinglePayment tests that DeletePayment correctly -// deletes information about a completed payment from the database. -func TestKVPaymentsDBDeleteSinglePayment(t *testing.T) { - t.Parallel() - - paymentDB := NewTestDB(t) - - // Register four payments: - // All payments will have one failed HTLC attempt and one HTLC attempt - // according to its final status. - // 1. A payment with two failed attempts. - // 2. Another payment with two failed attempts. - // 3. A payment with one failed and one settled attempt. - // 4. A payment with one failed and one in-flight attempt. - - // Initiate payments, which is a slice of payment that is used as - // template to create the corresponding test payments in the database. - // - // Note: The payment id and number of htlc attempts of each payment will - // be added to this slice when creating the payments below. - // This allows the slice to be used directly for testing purposes. - payments := []*payment{ - {status: StatusFailed}, - {status: StatusFailed}, - {status: StatusSucceeded}, - {status: StatusInFlight}, - } - - // Use helper function to register the test payments in the data and - // populate the data to the payments slice. - createTestPayments(t, paymentDB, payments) - - // Check that all payments are there as we added them. - assertPayments(t, paymentDB, payments) - - // Delete HTLC attempts for first payment only. - require.NoError(t, paymentDB.DeletePayment(payments[0].id, true)) - - // The first payment is the only altered one as its failed HTLC should - // have been removed but is still present as payment. - payments[0].htlcs = 0 - assertPayments(t, paymentDB, payments) - - // Delete the first payment completely. - require.NoError(t, paymentDB.DeletePayment(payments[0].id, false)) - - // The first payment should have been deleted. - assertPayments(t, paymentDB, payments[1:]) - - // Now delete the second payment completely. - require.NoError(t, paymentDB.DeletePayment(payments[1].id, false)) - - // The Second payment should have been deleted. - assertPayments(t, paymentDB, payments[2:]) - - // Delete failed HTLC attempts for the third payment. - require.NoError(t, paymentDB.DeletePayment(payments[2].id, true)) - - // Only the successful HTLC attempt should be left for the third - // payment. - payments[2].htlcs = 1 - assertPayments(t, paymentDB, payments[2:]) - - // Now delete the third payment completely. - require.NoError(t, paymentDB.DeletePayment(payments[2].id, false)) - - // Only the last payment should be left. - assertPayments(t, paymentDB, payments[3:]) - - // Deleting HTLC attempts from InFlight payments should not work and an - // error returned. - require.Error(t, paymentDB.DeletePayment(payments[3].id, true)) - - // The payment is InFlight and therefore should not have been altered. - assertPayments(t, paymentDB, payments[3:]) - - // Finally deleting the InFlight payment should also not work and an - // error returned. - require.Error(t, paymentDB.DeletePayment(payments[3].id, false)) - - // The payment is InFlight and therefore should not have been altered. - assertPayments(t, paymentDB, payments[3:]) -} - // TestKVPaymentsDBMultiShard checks the ability of payment control to // have multiple in-flight HTLCs for a single payment. func TestKVPaymentsDBMultiShard(t *testing.T) { @@ -691,7 +478,7 @@ func TestKVPaymentsDBMultiShard(t *testing.T) { } runSubTest := func(t *testing.T, test testCase) { - paymentDB := NewTestDB(t) + paymentDB := NewKVTestDB(t) info, attempt, preimg, err := genInfo(t) if err != nil { @@ -965,75 +752,6 @@ func TestKVPaymentsDBMultiShard(t *testing.T) { } } -func TestKVPaymentsDBMPPRecordValidation(t *testing.T) { - t.Parallel() - - paymentDB := NewTestDB(t) - - info, attempt, _, err := genInfo(t) - require.NoError(t, err, "unable to generate htlc message") - - // Init the payment. - err = paymentDB.InitPayment(info.PaymentIdentifier, info) - require.NoError(t, err, "unable to send htlc message") - - // Create three unique attempts we'll use for the test, and - // register them with the payment control. We set each - // attempts's value to one third of the payment amount, and - // populate the MPP options. - shardAmt := info.Value / 3 - attempt.Route.FinalHop().AmtToForward = shardAmt - attempt.Route.FinalHop().MPP = record.NewMPP( - info.Value, [32]byte{1}, - ) - - _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, attempt) - require.NoError(t, err, "unable to send htlc message") - - // Now try to register a non-MPP attempt, which should fail. - b := *attempt - b.AttemptID = 1 - b.Route.FinalHop().MPP = nil - _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, &b) - require.ErrorIs(t, err, ErrMPPayment) - - // Try to register attempt one with a different payment address. - b.Route.FinalHop().MPP = record.NewMPP( - info.Value, [32]byte{2}, - ) - _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, &b) - require.ErrorIs(t, err, ErrMPPPaymentAddrMismatch) - - // Try registering one with a different total amount. - b.Route.FinalHop().MPP = record.NewMPP( - info.Value/2, [32]byte{1}, - ) - _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, &b) - require.ErrorIs(t, err, ErrMPPTotalAmountMismatch) - - // Create and init a new payment. This time we'll check that we cannot - // register an MPP attempt if we already registered a non-MPP one. - info, attempt, _, err = genInfo(t) - require.NoError(t, err, "unable to generate htlc message") - - err = paymentDB.InitPayment(info.PaymentIdentifier, info) - require.NoError(t, err, "unable to send htlc message") - - attempt.Route.FinalHop().MPP = nil - _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, attempt) - require.NoError(t, err, "unable to send htlc message") - - // Attempt to register an MPP attempt, which should fail. - b = *attempt - b.AttemptID = 1 - b.Route.FinalHop().MPP = record.NewMPP( - info.Value, [32]byte{1}, - ) - - _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, &b) - require.ErrorIs(t, err, ErrNonMPPayment) -} - // TestDeleteFailedAttempts checks that DeleteFailedAttempts properly removes // failed HTLCs from finished payments. func TestDeleteFailedAttempts(t *testing.T) { @@ -1047,180 +765,12 @@ func TestDeleteFailedAttempts(t *testing.T) { }) } -func testDeleteFailedAttempts(t *testing.T, keepFailedPaymentAttempts bool) { - paymentDB := NewTestDB( - t, WithKeepFailedPaymentAttempts(keepFailedPaymentAttempts), - ) - - // Register three payments: - // All payments will have one failed HTLC attempt and one HTLC attempt - // according to its final status. - // 1. A payment with two failed attempts. - // 2. A payment with one failed and one in-flight attempt. - // 3. A payment with one failed and one settled attempt. - - // Initiate payments, which is a slice of payment that is used as - // template to create the corresponding test payments in the database. - // - // Note: The payment id and number of htlc attempts of each payment will - // be added to this slice when creating the payments below. - // This allows the slice to be used directly for testing purposes. - payments := []*payment{ - {status: StatusFailed}, - {status: StatusInFlight}, - {status: StatusSucceeded}, - } - - // Use helper function to register the test payments in the data and - // populate the data to the payments slice. - createTestPayments(t, paymentDB, payments) - - // Check that all payments are there as we added them. - assertPayments(t, paymentDB, payments) - - // Calling DeleteFailedAttempts on a failed payment should delete all - // HTLCs. - require.NoError(t, paymentDB.DeleteFailedAttempts(payments[0].id)) - - // Expect all HTLCs to be deleted if the config is set to delete them. - if !keepFailedPaymentAttempts { - payments[0].htlcs = 0 - } - assertPayments(t, paymentDB, payments) - - // Calling DeleteFailedAttempts on an in-flight payment should return - // an error. - if keepFailedPaymentAttempts { - require.NoError( - t, paymentDB.DeleteFailedAttempts(payments[1].id), - ) - } else { - require.Error(t, paymentDB.DeleteFailedAttempts(payments[1].id)) - } - - // Since DeleteFailedAttempts returned an error, we should expect the - // payment to be unchanged. - assertPayments(t, paymentDB, payments) - - // Cleaning up a successful payment should remove failed htlcs. - require.NoError(t, paymentDB.DeleteFailedAttempts(payments[2].id)) - // Expect all HTLCs except for the settled one to be deleted if the - // config is set to delete them. - if !keepFailedPaymentAttempts { - payments[2].htlcs = 1 - } - assertPayments(t, paymentDB, payments) - - if keepFailedPaymentAttempts { - // DeleteFailedAttempts is ignored, even for non-existent - // payments, if the control tower is configured to keep failed - // HTLCs. - require.NoError( - t, paymentDB.DeleteFailedAttempts(lntypes.ZeroHash), - ) - } else { - // Attempting to cleanup a non-existent payment returns an error. - require.Error( - t, paymentDB.DeleteFailedAttempts(lntypes.ZeroHash), - ) - } -} - -// assertPaymentStatus retrieves the status of the payment referred to by hash -// and compares it with the expected state. -func assertPaymentStatus(t *testing.T, p *KVPaymentsDB, - hash lntypes.Hash, expStatus PaymentStatus) { - - t.Helper() - - payment, err := p.FetchPayment(hash) - if errors.Is(err, ErrPaymentNotInitiated) { - return - } - if err != nil { - t.Fatal(err) - } - - if payment.Status != expStatus { - t.Fatalf("payment status mismatch: expected %v, got %v", - expStatus, payment.Status) - } -} - type htlcStatus struct { *HTLCAttemptInfo settle *lntypes.Preimage failure *HTLCFailReason } -// assertPaymentInfo retrieves the payment referred to by hash and verifies the -// expected values. -func assertPaymentInfo(t *testing.T, p *KVPaymentsDB, hash lntypes.Hash, - c *PaymentCreationInfo, f *FailureReason, - a *htlcStatus) { - - t.Helper() - - payment, err := p.FetchPayment(hash) - if err != nil { - t.Fatal(err) - } - - if !reflect.DeepEqual(payment.Info, c) { - t.Fatalf("PaymentCreationInfos don't match: %v vs %v", - spew.Sdump(payment.Info), spew.Sdump(c)) - } - - if f != nil { - if *payment.FailureReason != *f { - t.Fatal("unexpected failure reason") - } - } else { - if payment.FailureReason != nil { - t.Fatal("unexpected failure reason") - } - } - - if a == nil { - if len(payment.HTLCs) > 0 { - t.Fatal("expected no htlcs") - } - return - } - - htlc := payment.HTLCs[a.AttemptID] - if err := assertRouteEqual(&htlc.Route, &a.Route); err != nil { - t.Fatal("routes do not match") - } - - if htlc.AttemptID != a.AttemptID { - t.Fatalf("unnexpected attempt ID %v, expected %v", - htlc.AttemptID, a.AttemptID) - } - - if a.failure != nil { - if htlc.Failure == nil { - t.Fatalf("expected HTLC to be failed") - } - - if htlc.Failure.Reason != *a.failure { - t.Fatalf("expected HTLC failure %v, had %v", - *a.failure, htlc.Failure.Reason) - } - } else if htlc.Failure != nil { - t.Fatalf("expected no HTLC failure") - } - - if a.settle != nil { - if htlc.Settle.Preimage != *a.settle { - t.Fatalf("Preimages don't match: %x vs %x", - htlc.Settle.Preimage, a.settle) - } - } else if htlc.Settle != nil { - t.Fatal("expected no settle info") - } -} - // fetchPaymentIndexEntry gets the payment hash for the sequence number provided // from our payment indexes bucket. func fetchPaymentIndexEntry(_ *testing.T, p *KVPaymentsDB, @@ -1242,6 +792,7 @@ func fetchPaymentIndexEntry(_ *testing.T, p *KVPaymentsDB, var err error hash, err = deserializePaymentIndex(r) + return err }, func() { hash = lntypes.Hash{} @@ -1274,141 +825,6 @@ func assertNoIndex(t *testing.T, p *KVPaymentsDB, seqNr uint64) { require.Equal(t, ErrNoSequenceNrIndex, err) } -// payment is a helper structure that holds basic information on a test payment, -// such as the payment id, the status and the total number of HTLCs attempted. -type payment struct { - id lntypes.Hash - status PaymentStatus - htlcs int -} - -// createTestPayments registers payments depending on the provided statuses in -// the payments slice. Each payment will receive one failed HTLC and another -// HTLC depending on the final status of the payment provided. -func createTestPayments(t *testing.T, p *KVPaymentsDB, payments []*payment) { - attemptID := uint64(0) - - for i := 0; i < len(payments); i++ { - info, attempt, preimg, err := genInfo(t) - require.NoError(t, err, "unable to generate htlc message") - - // Set the payment id accordingly in the payments slice. - payments[i].id = info.PaymentIdentifier - - attempt.AttemptID = attemptID - attemptID++ - - // Init the payment. - err = p.InitPayment(info.PaymentIdentifier, info) - require.NoError(t, err, "unable to send htlc message") - - // Register and fail the first attempt for all payments. - _, err = p.RegisterAttempt(info.PaymentIdentifier, attempt) - require.NoError(t, err, "unable to send htlc message") - - htlcFailure := HTLCFailUnreadable - _, err = p.FailAttempt( - info.PaymentIdentifier, attempt.AttemptID, - &HTLCFailInfo{ - Reason: htlcFailure, - }, - ) - require.NoError(t, err, "unable to fail htlc") - - // Increase the HTLC counter in the payments slice for the - // failed attempt. - payments[i].htlcs++ - - // Depending on the test case, fail or succeed the next - // attempt. - attempt.AttemptID = attemptID - attemptID++ - - _, err = p.RegisterAttempt(info.PaymentIdentifier, attempt) - require.NoError(t, err, "unable to send htlc message") - - switch payments[i].status { - // Fail the attempt and the payment overall. - case StatusFailed: - htlcFailure := HTLCFailUnreadable - _, err = p.FailAttempt( - info.PaymentIdentifier, attempt.AttemptID, - &HTLCFailInfo{ - Reason: htlcFailure, - }, - ) - require.NoError(t, err, "unable to fail htlc") - - failReason := FailureReasonNoRoute - _, err = p.Fail(info.PaymentIdentifier, - failReason) - require.NoError(t, err, "unable to fail payment hash") - - // Settle the attempt - case StatusSucceeded: - _, err := p.SettleAttempt( - info.PaymentIdentifier, attempt.AttemptID, - &HTLCSettleInfo{ - Preimage: preimg, - }, - ) - require.NoError(t, err, "no error should have been "+ - "received from settling a htlc attempt") - - // We leave the attempt in-flight by doing nothing. - case StatusInFlight: - } - - // Increase the HTLC counter in the payments slice for any - // attempt above. - payments[i].htlcs++ - } -} - -// assertPayments is a helper function that given a slice of payment and -// indices for the slice asserts that exactly the same payments in the -// slice for the provided indices exist when fetching payments from the -// database. -func assertPayments(t *testing.T, paymentDB *KVPaymentsDB, - payments []*payment) { - - t.Helper() - - ctx := context.Background() - - // We use the query method to fetch payments from the database which - // allows us to use this method db agnostic. We fetch all payments in - // one go. - queryResp, err := paymentDB.QueryPayments(ctx, Query{ - IndexOffset: 0, - MaxPayments: math.MaxUint64, - Reversed: false, - IncludeIncomplete: true, - }) - require.NoError(t, err, "could not fetch payments from db") - - dbPayments := queryResp.Payments - - // Make sure that the number of fetched payments is the same - // as expected. - require.Len( - t, dbPayments, len(payments), "unexpected number of payments", - ) - - // Convert fetched payments of type MPPayment to our helper structure. - p := make([]*payment, len(dbPayments)) - for i, dbPayment := range dbPayments { - p[i] = &payment{ - id: dbPayment.Info.PaymentIdentifier, - status: dbPayment.Status, - htlcs: len(dbPayment.HTLCs), - } - } - - // Check that each payment we want to assert exists in the database. - require.Equal(t, payments, p) -} - func makeFakeInfo(t *testing.T) (*PaymentCreationInfo, *HTLCAttemptInfo) { @@ -1559,7 +975,7 @@ func deletePayment(t *testing.T, db kvdb.Backend, paymentHash lntypes.Hash, // case where a specific duplicate is not found and the duplicates bucket is not // present when we expect it to be. func TestFetchPaymentWithSequenceNumber(t *testing.T) { - paymentDB := NewTestDB(t) + paymentDB := NewKVTestDB(t) // Generate a test payment which does not have duplicates. noDuplicates, _, _, err := genInfo(t) @@ -1767,3 +1183,392 @@ func putDuplicatePayment(t *testing.T, duplicateBucket kvdb.RwBucket, err = paymentBucket.Put(duplicatePaymentSettleInfoKey, preImg[:]) require.NoError(t, err) } + +// TestQueryPayments tests retrieval of payments with forwards and reversed +// queries. +func TestQueryPayments(t *testing.T) { + // Define table driven test for QueryPayments. + // Test payments have sequence indices [1, 3, 4, 5, 6, 7]. + // Note that the payment with index 7 has the same payment hash as 6, + // and is stored in a nested bucket within payment 6 rather than being + // its own entry in the payments bucket. We do this to test retrieval + // of legacy payments. + tests := []struct { + name string + query Query + firstIndex uint64 + lastIndex uint64 + + // expectedSeqNrs contains the set of sequence numbers we expect + // our query to return. + expectedSeqNrs []uint64 + }{ + { + name: "IndexOffset at the end of the payments range", + query: Query{ + IndexOffset: 7, + MaxPayments: 7, + Reversed: false, + IncludeIncomplete: true, + }, + firstIndex: 0, + lastIndex: 0, + expectedSeqNrs: nil, + }, + { + name: "query in forwards order, start at beginning", + query: Query{ + IndexOffset: 0, + MaxPayments: 2, + Reversed: false, + IncludeIncomplete: true, + }, + firstIndex: 1, + lastIndex: 3, + expectedSeqNrs: []uint64{1, 3}, + }, + { + name: "query in forwards order, start at end, overflow", + query: Query{ + IndexOffset: 6, + MaxPayments: 2, + Reversed: false, + IncludeIncomplete: true, + }, + firstIndex: 7, + lastIndex: 7, + expectedSeqNrs: []uint64{7}, + }, + { + name: "start at offset index outside of payments", + query: Query{ + IndexOffset: 20, + MaxPayments: 2, + Reversed: false, + IncludeIncomplete: true, + }, + firstIndex: 0, + lastIndex: 0, + expectedSeqNrs: nil, + }, + { + name: "overflow in forwards order", + query: Query{ + IndexOffset: 4, + MaxPayments: math.MaxUint64, + Reversed: false, + IncludeIncomplete: true, + }, + firstIndex: 5, + lastIndex: 7, + expectedSeqNrs: []uint64{5, 6, 7}, + }, + { + name: "start at offset index outside of payments, " + + "reversed order", + query: Query{ + IndexOffset: 9, + MaxPayments: 2, + Reversed: true, + IncludeIncomplete: true, + }, + firstIndex: 6, + lastIndex: 7, + expectedSeqNrs: []uint64{6, 7}, + }, + { + name: "query in reverse order, start at end", + query: Query{ + IndexOffset: 0, + MaxPayments: 2, + Reversed: true, + IncludeIncomplete: true, + }, + firstIndex: 6, + lastIndex: 7, + expectedSeqNrs: []uint64{6, 7}, + }, + { + name: "query in reverse order, starting in middle", + query: Query{ + IndexOffset: 4, + MaxPayments: 2, + Reversed: true, + IncludeIncomplete: true, + }, + firstIndex: 1, + lastIndex: 3, + expectedSeqNrs: []uint64{1, 3}, + }, + { + name: "query in reverse order, starting in middle, " + + "with underflow", + query: Query{ + IndexOffset: 4, + MaxPayments: 5, + Reversed: true, + IncludeIncomplete: true, + }, + firstIndex: 1, + lastIndex: 3, + expectedSeqNrs: []uint64{1, 3}, + }, + { + name: "all payments in reverse, order maintained", + query: Query{ + IndexOffset: 0, + MaxPayments: 7, + Reversed: true, + IncludeIncomplete: true, + }, + firstIndex: 1, + lastIndex: 7, + expectedSeqNrs: []uint64{1, 3, 4, 5, 6, 7}, + }, + { + name: "exclude incomplete payments", + query: Query{ + IndexOffset: 0, + MaxPayments: 7, + Reversed: false, + IncludeIncomplete: false, + }, + firstIndex: 7, + lastIndex: 7, + expectedSeqNrs: []uint64{7}, + }, + { + name: "query payments at index gap", + query: Query{ + IndexOffset: 1, + MaxPayments: 7, + Reversed: false, + IncludeIncomplete: true, + }, + firstIndex: 3, + lastIndex: 7, + expectedSeqNrs: []uint64{3, 4, 5, 6, 7}, + }, + { + name: "query payments reverse before index gap", + query: Query{ + IndexOffset: 3, + MaxPayments: 7, + Reversed: true, + IncludeIncomplete: true, + }, + firstIndex: 1, + lastIndex: 1, + expectedSeqNrs: []uint64{1}, + }, + { + name: "query payments reverse on index gap", + query: Query{ + IndexOffset: 2, + MaxPayments: 7, + Reversed: true, + IncludeIncomplete: true, + }, + firstIndex: 1, + lastIndex: 1, + expectedSeqNrs: []uint64{1}, + }, + { + name: "query payments forward on index gap", + query: Query{ + IndexOffset: 2, + MaxPayments: 2, + Reversed: false, + IncludeIncomplete: true, + }, + firstIndex: 3, + lastIndex: 4, + expectedSeqNrs: []uint64{3, 4}, + }, + { + name: "query in forwards order, with start creation " + + "time", + query: Query{ + IndexOffset: 0, + MaxPayments: 2, + Reversed: false, + IncludeIncomplete: true, + CreationDateStart: 5, + }, + firstIndex: 5, + lastIndex: 6, + expectedSeqNrs: []uint64{5, 6}, + }, + { + name: "query in forwards order, with start creation " + + "time at end, overflow", + query: Query{ + IndexOffset: 0, + MaxPayments: 2, + Reversed: false, + IncludeIncomplete: true, + CreationDateStart: 7, + }, + firstIndex: 7, + lastIndex: 7, + expectedSeqNrs: []uint64{7}, + }, + { + name: "query with start and end creation time", + query: Query{ + IndexOffset: 9, + MaxPayments: math.MaxUint64, + Reversed: true, + IncludeIncomplete: true, + CreationDateStart: 3, + CreationDateEnd: 5, + }, + firstIndex: 3, + lastIndex: 5, + expectedSeqNrs: []uint64{3, 4, 5}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + paymentDB := NewKVTestDB(t) + + // Initialize the payment database. + paymentDB, err := NewKVPaymentsDB(paymentDB.db) + require.NoError(t, err) + + // Make a preliminary query to make sure it's ok to + // query when we have no payments. + resp, err := paymentDB.QueryPayments(ctx, tt.query) + require.NoError(t, err) + require.Len(t, resp.Payments, 0) + + // Populate the database with a set of test payments. + // We create 6 original payments, deleting the payment + // at index 2 so that we cover the case where sequence + // numbers are missing. We also add a duplicate payment + // to the last payment added to test the legacy case + // where we have duplicates in the nested duplicates + // bucket. + nonDuplicatePayments := 6 + + for i := 0; i < nonDuplicatePayments; i++ { + // Generate a test payment. + info, _, preimg, err := genInfo(t) + if err != nil { + t.Fatalf("unable to create test "+ + "payment: %v", err) + } + // Override creation time to allow for testing + // of CreationDateStart and CreationDateEnd. + info.CreationTime = time.Unix(int64(i+1), 0) + + // Create a new payment entry in the database. + err = paymentDB.InitPayment( + info.PaymentIdentifier, info, + ) + require.NoError(t, err) + + // Immediately delete the payment with index 2. + if i == 1 { + pmt, err := paymentDB.FetchPayment( + info.PaymentIdentifier, + ) + require.NoError(t, err) + + deletePayment( + t, paymentDB.db, + info.PaymentIdentifier, + pmt.SequenceNum, + ) + } + + // If we are on the last payment entry, add a + // duplicate payment with sequence number equal + // to the parent payment + 1. Note that + // duplicate payments will always be succeeded. + if i == (nonDuplicatePayments - 1) { + pmt, err := paymentDB.FetchPayment( + info.PaymentIdentifier, + ) + require.NoError(t, err) + + appendDuplicatePayment( + t, paymentDB.db, + info.PaymentIdentifier, + pmt.SequenceNum+1, + preimg, + ) + } + } + + // Fetch all payments in the database. + allPayments, err := paymentDB.FetchPayments() + if err != nil { + t.Fatalf("payments could not be fetched from "+ + "database: %v", err) + } + + if len(allPayments) != 6 { + t.Fatalf("Number of payments received does "+ + "not match expected one. Got %v, "+ + "want %v.", len(allPayments), 6) + } + + querySlice, err := paymentDB.QueryPayments( + ctx, tt.query, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tt.firstIndex != querySlice.FirstIndexOffset || + tt.lastIndex != querySlice.LastIndexOffset { + + t.Errorf("First or last index does not match "+ + "expected index. Want (%d, %d), "+ + "got (%d, %d).", + tt.firstIndex, tt.lastIndex, + querySlice.FirstIndexOffset, + querySlice.LastIndexOffset) + } + + if len(querySlice.Payments) != len(tt.expectedSeqNrs) { + t.Errorf("expected: %v payments, got: %v", + len(tt.expectedSeqNrs), + len(querySlice.Payments)) + } + + for i, seqNr := range tt.expectedSeqNrs { + q := querySlice.Payments[i] + if seqNr != q.SequenceNum { + t.Errorf("sequence numbers do not "+ + "match, got %v, want %v", + q.SequenceNum, seqNr) + } + } + }) + } +} + +// TestLazySessionKeyDeserialize tests that we can read htlc attempt session +// keys that were previously serialized as a private key as raw bytes. +func TestLazySessionKeyDeserialize(t *testing.T) { + var b bytes.Buffer + + // Serialize as a private key. + err := WriteElements(&b, priv) + require.NoError(t, err) + + // Deserialize into [btcec.PrivKeyBytesLen]byte. + attempt := HTLCAttemptInfo{} + err = ReadElements(&b, &attempt.sessionKey) + require.NoError(t, err) + require.Zero(t, b.Len()) + + sessionKey := attempt.SessionKey() + require.Equal(t, priv, sessionKey) +} diff --git a/payments/db/payment_test.go b/payments/db/payment_test.go index 9889f9d88ed..6d1cf131462 100644 --- a/payments/db/payment_test.go +++ b/payments/db/payment_test.go @@ -1,10 +1,12 @@ package paymentsdb import ( - "bytes" "context" + "crypto/rand" + "crypto/sha256" + "errors" "fmt" - "math" + "io" "reflect" "testing" "time" @@ -98,6 +100,97 @@ var ( } ) +// payment is a helper structure that holds basic information on a test payment, +// such as the payment id, the status and the total number of HTLCs attempted. +type payment struct { + id lntypes.Hash + status PaymentStatus + htlcs int +} + +// createTestPayments registers payments depending on the provided statuses in +// the payments slice. Each payment will receive one failed HTLC and another +// HTLC depending on the final status of the payment provided. +func createTestPayments(t *testing.T, p DB, payments []*payment) { + attemptID := uint64(0) + + for i := 0; i < len(payments); i++ { + info, attempt, preimg, err := genInfo(t) + require.NoError(t, err, "unable to generate htlc message") + + // Set the payment id accordingly in the payments slice. + payments[i].id = info.PaymentIdentifier + + attempt.AttemptID = attemptID + attemptID++ + + // Init the payment. + err = p.InitPayment(info.PaymentIdentifier, info) + require.NoError(t, err, "unable to send htlc message") + + // Register and fail the first attempt for all payments. + _, err = p.RegisterAttempt(info.PaymentIdentifier, attempt) + require.NoError(t, err, "unable to send htlc message") + + htlcFailure := HTLCFailUnreadable + _, err = p.FailAttempt( + info.PaymentIdentifier, attempt.AttemptID, + &HTLCFailInfo{ + Reason: htlcFailure, + }, + ) + require.NoError(t, err, "unable to fail htlc") + + // Increase the HTLC counter in the payments slice for the + // failed attempt. + payments[i].htlcs++ + + // Depending on the test case, fail or succeed the next + // attempt. + attempt.AttemptID = attemptID + attemptID++ + + _, err = p.RegisterAttempt(info.PaymentIdentifier, attempt) + require.NoError(t, err, "unable to send htlc message") + + switch payments[i].status { + // Fail the attempt and the payment overall. + case StatusFailed: + htlcFailure := HTLCFailUnreadable + _, err = p.FailAttempt( + info.PaymentIdentifier, attempt.AttemptID, + &HTLCFailInfo{ + Reason: htlcFailure, + }, + ) + require.NoError(t, err, "unable to fail htlc") + + failReason := FailureReasonNoRoute + _, err = p.Fail(info.PaymentIdentifier, + failReason) + require.NoError(t, err, "unable to fail payment hash") + + // Settle the attempt + case StatusSucceeded: + _, err := p.SettleAttempt( + info.PaymentIdentifier, attempt.AttemptID, + &HTLCSettleInfo{ + Preimage: preimg, + }, + ) + require.NoError(t, err, "no error should have been "+ + "received from settling a htlc attempt") + + // We leave the attempt in-flight by doing nothing. + case StatusInFlight: + } + + // Increase the HTLC counter in the payments slice for any + // attempt above. + payments[i].htlcs++ + } +} + // assertRouteEquals compares to routes for equality and returns an error if // they are not equal. func assertRouteEqual(a, b *route.Route) error { @@ -109,398 +202,414 @@ func assertRouteEqual(a, b *route.Route) error { return nil } -// TestQueryPayments tests retrieval of payments with forwards and reversed -// queries. -func TestQueryPayments(t *testing.T) { - // Define table driven test for QueryPayments. - // Test payments have sequence indices [1, 3, 4, 5, 6, 7]. - // Note that the payment with index 7 has the same payment hash as 6, - // and is stored in a nested bucket within payment 6 rather than being - // its own entry in the payments bucket. We do this to test retrieval - // of legacy payments. - tests := []struct { - name string - query Query - firstIndex uint64 - lastIndex uint64 - - // expectedSeqNrs contains the set of sequence numbers we expect - // our query to return. - expectedSeqNrs []uint64 - }{ - { - name: "IndexOffset at the end of the payments range", - query: Query{ - IndexOffset: 7, - MaxPayments: 7, - Reversed: false, - IncludeIncomplete: true, - }, - firstIndex: 0, - lastIndex: 0, - expectedSeqNrs: nil, - }, - { - name: "query in forwards order, start at beginning", - query: Query{ - IndexOffset: 0, - MaxPayments: 2, - Reversed: false, - IncludeIncomplete: true, - }, - firstIndex: 1, - lastIndex: 3, - expectedSeqNrs: []uint64{1, 3}, - }, - { - name: "query in forwards order, start at end, overflow", - query: Query{ - IndexOffset: 6, - MaxPayments: 2, - Reversed: false, - IncludeIncomplete: true, - }, - firstIndex: 7, - lastIndex: 7, - expectedSeqNrs: []uint64{7}, - }, - { - name: "start at offset index outside of payments", - query: Query{ - IndexOffset: 20, - MaxPayments: 2, - Reversed: false, - IncludeIncomplete: true, - }, - firstIndex: 0, - lastIndex: 0, - expectedSeqNrs: nil, - }, - { - name: "overflow in forwards order", - query: Query{ - IndexOffset: 4, - MaxPayments: math.MaxUint64, - Reversed: false, - IncludeIncomplete: true, - }, - firstIndex: 5, - lastIndex: 7, - expectedSeqNrs: []uint64{5, 6, 7}, - }, - { - name: "start at offset index outside of payments, " + - "reversed order", - query: Query{ - IndexOffset: 9, - MaxPayments: 2, - Reversed: true, - IncludeIncomplete: true, - }, - firstIndex: 6, - lastIndex: 7, - expectedSeqNrs: []uint64{6, 7}, - }, - { - name: "query in reverse order, start at end", - query: Query{ - IndexOffset: 0, - MaxPayments: 2, - Reversed: true, - IncludeIncomplete: true, - }, - firstIndex: 6, - lastIndex: 7, - expectedSeqNrs: []uint64{6, 7}, - }, - { - name: "query in reverse order, starting in middle", - query: Query{ - IndexOffset: 4, - MaxPayments: 2, - Reversed: true, - IncludeIncomplete: true, - }, - firstIndex: 1, - lastIndex: 3, - expectedSeqNrs: []uint64{1, 3}, - }, - { - name: "query in reverse order, starting in middle, " + - "with underflow", - query: Query{ - IndexOffset: 4, - MaxPayments: 5, - Reversed: true, - IncludeIncomplete: true, - }, - firstIndex: 1, - lastIndex: 3, - expectedSeqNrs: []uint64{1, 3}, - }, - { - name: "all payments in reverse, order maintained", - query: Query{ - IndexOffset: 0, - MaxPayments: 7, - Reversed: true, - IncludeIncomplete: true, - }, - firstIndex: 1, - lastIndex: 7, - expectedSeqNrs: []uint64{1, 3, 4, 5, 6, 7}, - }, - { - name: "exclude incomplete payments", - query: Query{ - IndexOffset: 0, - MaxPayments: 7, - Reversed: false, - IncludeIncomplete: false, - }, - firstIndex: 7, - lastIndex: 7, - expectedSeqNrs: []uint64{7}, - }, - { - name: "query payments at index gap", - query: Query{ - IndexOffset: 1, - MaxPayments: 7, - Reversed: false, - IncludeIncomplete: true, - }, - firstIndex: 3, - lastIndex: 7, - expectedSeqNrs: []uint64{3, 4, 5, 6, 7}, - }, - { - name: "query payments reverse before index gap", - query: Query{ - IndexOffset: 3, - MaxPayments: 7, - Reversed: true, - IncludeIncomplete: true, - }, - firstIndex: 1, - lastIndex: 1, - expectedSeqNrs: []uint64{1}, - }, - { - name: "query payments reverse on index gap", - query: Query{ - IndexOffset: 2, - MaxPayments: 7, - Reversed: true, - IncludeIncomplete: true, - }, - firstIndex: 1, - lastIndex: 1, - expectedSeqNrs: []uint64{1}, - }, - { - name: "query payments forward on index gap", - query: Query{ - IndexOffset: 2, - MaxPayments: 2, - Reversed: false, - IncludeIncomplete: true, - }, - firstIndex: 3, - lastIndex: 4, - expectedSeqNrs: []uint64{3, 4}, - }, - { - name: "query in forwards order, with start creation " + - "time", - query: Query{ - IndexOffset: 0, - MaxPayments: 2, - Reversed: false, - IncludeIncomplete: true, - CreationDateStart: 5, - }, - firstIndex: 5, - lastIndex: 6, - expectedSeqNrs: []uint64{5, 6}, - }, - { - name: "query in forwards order, with start creation " + - "time at end, overflow", - query: Query{ - IndexOffset: 0, - MaxPayments: 2, - Reversed: false, - IncludeIncomplete: true, - CreationDateStart: 7, - }, - firstIndex: 7, - lastIndex: 7, - expectedSeqNrs: []uint64{7}, - }, - { - name: "query with start and end creation time", - query: Query{ - IndexOffset: 9, - MaxPayments: math.MaxUint64, - Reversed: true, - IncludeIncomplete: true, - CreationDateStart: 3, - CreationDateEnd: 5, - }, - firstIndex: 3, - lastIndex: 5, - expectedSeqNrs: []uint64{3, 4, 5}, - }, +// assertPaymentInfo retrieves the payment referred to by hash and verifies the +// expected values. +func assertPaymentInfo(t *testing.T, p DB, hash lntypes.Hash, + c *PaymentCreationInfo, f *FailureReason, + a *htlcStatus) { + + t.Helper() + + payment, err := p.FetchPayment(hash) + if err != nil { + t.Fatal(err) } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() + if !reflect.DeepEqual(payment.Info, c) { + t.Fatalf("PaymentCreationInfos don't match: %v vs %v", + spew.Sdump(payment.Info), spew.Sdump(c)) + } - ctx := context.Background() - - paymentDB := NewTestDB(t) - - // Initialize the payment database. - paymentDB, err := NewKVPaymentsDB(paymentDB.db) - require.NoError(t, err) - - // Make a preliminary query to make sure it's ok to - // query when we have no payments. - resp, err := paymentDB.QueryPayments(ctx, tt.query) - require.NoError(t, err) - require.Len(t, resp.Payments, 0) - - // Populate the database with a set of test payments. - // We create 6 original payments, deleting the payment - // at index 2 so that we cover the case where sequence - // numbers are missing. We also add a duplicate payment - // to the last payment added to test the legacy case - // where we have duplicates in the nested duplicates - // bucket. - nonDuplicatePayments := 6 - - for i := 0; i < nonDuplicatePayments; i++ { - // Generate a test payment. - info, _, preimg, err := genInfo(t) - if err != nil { - t.Fatalf("unable to create test "+ - "payment: %v", err) - } - // Override creation time to allow for testing - // of CreationDateStart and CreationDateEnd. - info.CreationTime = time.Unix(int64(i+1), 0) - - // Create a new payment entry in the database. - err = paymentDB.InitPayment( - info.PaymentIdentifier, info, - ) - require.NoError(t, err) - - // Immediately delete the payment with index 2. - if i == 1 { - pmt, err := paymentDB.FetchPayment( - info.PaymentIdentifier, - ) - require.NoError(t, err) - - deletePayment( - t, paymentDB.db, - info.PaymentIdentifier, - pmt.SequenceNum, - ) - } - - // If we are on the last payment entry, add a - // duplicate payment with sequence number equal - // to the parent payment + 1. Note that - // duplicate payments will always be succeeded. - if i == (nonDuplicatePayments - 1) { - pmt, err := paymentDB.FetchPayment( - info.PaymentIdentifier, - ) - require.NoError(t, err) - - appendDuplicatePayment( - t, paymentDB.db, - info.PaymentIdentifier, - pmt.SequenceNum+1, - preimg, - ) - } - } + if f != nil { + if *payment.FailureReason != *f { + t.Fatal("unexpected failure reason") + } + } else { + if payment.FailureReason != nil { + t.Fatal("unexpected failure reason") + } + } - // Fetch all payments in the database. - allPayments, err := paymentDB.FetchPayments() - if err != nil { - t.Fatalf("payments could not be fetched from "+ - "database: %v", err) - } + if a == nil { + if len(payment.HTLCs) > 0 { + t.Fatal("expected no htlcs") + } - if len(allPayments) != 6 { - t.Fatalf("Number of payments received does "+ - "not match expected one. Got %v, "+ - "want %v.", len(allPayments), 6) - } + return + } - querySlice, err := paymentDB.QueryPayments( - ctx, tt.query, - ) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if tt.firstIndex != querySlice.FirstIndexOffset || - tt.lastIndex != querySlice.LastIndexOffset { - - t.Errorf("First or last index does not match "+ - "expected index. Want (%d, %d), "+ - "got (%d, %d).", - tt.firstIndex, tt.lastIndex, - querySlice.FirstIndexOffset, - querySlice.LastIndexOffset) - } + htlc := payment.HTLCs[a.AttemptID] + if err := assertRouteEqual(&htlc.Route, &a.Route); err != nil { + t.Fatal("routes do not match") + } - if len(querySlice.Payments) != len(tt.expectedSeqNrs) { - t.Errorf("expected: %v payments, got: %v", - len(tt.expectedSeqNrs), - len(querySlice.Payments)) - } + if htlc.AttemptID != a.AttemptID { + t.Fatalf("unnexpected attempt ID %v, expected %v", + htlc.AttemptID, a.AttemptID) + } - for i, seqNr := range tt.expectedSeqNrs { - q := querySlice.Payments[i] - if seqNr != q.SequenceNum { - t.Errorf("sequence numbers do not "+ - "match, got %v, want %v", - q.SequenceNum, seqNr) - } - } - }) + if a.failure != nil { + if htlc.Failure == nil { + t.Fatalf("expected HTLC to be failed") + } + + if htlc.Failure.Reason != *a.failure { + t.Fatalf("expected HTLC failure %v, had %v", + *a.failure, htlc.Failure.Reason) + } + } else if htlc.Failure != nil { + t.Fatalf("expected no HTLC failure") + } + + if a.settle != nil { + if htlc.Settle.Preimage != *a.settle { + t.Fatalf("Preimages don't match: %x vs %x", + htlc.Settle.Preimage, a.settle) + } + } else if htlc.Settle != nil { + t.Fatal("expected no settle info") } } -// TestLazySessionKeyDeserialize tests that we can read htlc attempt session -// keys that were previously serialized as a private key as raw bytes. -func TestLazySessionKeyDeserialize(t *testing.T) { - var b bytes.Buffer +// assertPaymentStatus retrieves the status of the payment referred to by hash +// and compares it with the expected state. +func assertPaymentStatus(t *testing.T, p DB, hash lntypes.Hash, + expStatus PaymentStatus) { - // Serialize as a private key. - err := WriteElements(&b, priv) - require.NoError(t, err) + t.Helper() + + payment, err := p.FetchPayment(hash) + if errors.Is(err, ErrPaymentNotInitiated) { + return + } + if err != nil { + t.Fatal(err) + } + + if payment.Status != expStatus { + t.Fatalf("payment status mismatch: expected %v, got %v", + expStatus, payment.Status) + } +} + +// assertPayments is a helper function that given a slice of payment and +// indices for the slice asserts that exactly the same payments in the +// slice for the provided indices exist when fetching payments from the +// database. +func assertPayments(t *testing.T, paymentDB DB, payments []*payment) { + t.Helper() + + response, err := paymentDB.QueryPayments( + context.Background(), Query{ + IndexOffset: 0, + MaxPayments: uint64(len(payments)), + IncludeIncomplete: true, + }, + ) + require.NoError(t, err, "could not fetch payments from db") + + dbPayments := response.Payments + + // Make sure that the number of fetched payments is the same + // as expected. + require.Len( + t, dbPayments, len(payments), "unexpected number of payments", + ) + + // Convert fetched payments of type MPPayment to our helper structure. + p := make([]*payment, len(dbPayments)) + for i, dbPayment := range dbPayments { + p[i] = &payment{ + id: dbPayment.Info.PaymentIdentifier, + status: dbPayment.Status, + htlcs: len(dbPayment.HTLCs), + } + } + + // Check that each payment we want to assert exists in the database. + require.Equal(t, payments, p) +} + +func genPreimage() ([32]byte, error) { + var preimage [32]byte + if _, err := io.ReadFull(rand.Reader, preimage[:]); err != nil { + return preimage, err + } + + return preimage, nil +} + +func genInfo(t *testing.T) (*PaymentCreationInfo, *HTLCAttemptInfo, + lntypes.Preimage, error) { + + preimage, err := genPreimage() + if err != nil { + return nil, nil, preimage, fmt.Errorf("unable to "+ + "generate preimage: %v", err) + } + + rhash := sha256.Sum256(preimage[:]) + var hash lntypes.Hash + copy(hash[:], rhash[:]) - // Deserialize into [btcec.PrivKeyBytesLen]byte. - attempt := HTLCAttemptInfo{} - err = ReadElements(&b, &attempt.sessionKey) + attempt, err := NewHtlcAttempt( + 0, priv, *testRoute.Copy(), time.Time{}, &hash, + ) require.NoError(t, err) - require.Zero(t, b.Len()) - sessionKey := attempt.SessionKey() - require.Equal(t, priv, sessionKey) + return &PaymentCreationInfo{ + PaymentIdentifier: rhash, + Value: testRoute.ReceiverAmt(), + CreationTime: time.Unix(time.Now().Unix(), 0), + PaymentRequest: []byte("hola"), + }, &attempt.HTLCAttemptInfo, preimage, nil +} + +func testDeleteFailedAttempts(t *testing.T, keepFailedPaymentAttempts bool) { + paymentDB := NewTestDB( + t, WithKeepFailedPaymentAttempts(keepFailedPaymentAttempts), + ) + + // Register three payments: + // All payments will have one failed HTLC attempt and one HTLC attempt + // according to its final status. + // 1. A payment with two failed attempts. + // 2. A payment with one failed and one in-flight attempt. + // 3. A payment with one failed and one settled attempt. + + // Initiate payments, which is a slice of payment that is used as + // template to create the corresponding test payments in the database. + // + // Note: The payment id and number of htlc attempts of each payment will + // be added to this slice when creating the payments below. + // This allows the slice to be used directly for testing purposes. + payments := []*payment{ + {status: StatusFailed}, + {status: StatusInFlight}, + {status: StatusSucceeded}, + } + + // Use helper function to register the test payments in the data and + // populate the data to the payments slice. + createTestPayments(t, paymentDB, payments) + + // Check that all payments are there as we added them. + assertPayments(t, paymentDB, payments) + + // Calling DeleteFailedAttempts on a failed payment should delete all + // HTLCs. + require.NoError(t, paymentDB.DeleteFailedAttempts(payments[0].id)) + + // Expect all HTLCs to be deleted if the config is set to delete them. + if !keepFailedPaymentAttempts { + payments[0].htlcs = 0 + } + assertPayments(t, paymentDB, payments) + + // Calling DeleteFailedAttempts on an in-flight payment should return + // an error. + // + // NOTE: In case the option keepFailedPaymentAttempts is set no delete + // operation are performed in general therefore we do NOT expect an + // error in this case. + if keepFailedPaymentAttempts { + require.NoError( + t, paymentDB.DeleteFailedAttempts(payments[1].id), + ) + } else { + require.Error(t, paymentDB.DeleteFailedAttempts(payments[1].id)) + } + + // Since DeleteFailedAttempts returned an error, we should expect the + // payment to be unchanged. + assertPayments(t, paymentDB, payments) + + // Cleaning up a successful payment should remove failed htlcs. + require.NoError(t, paymentDB.DeleteFailedAttempts(payments[2].id)) + + // Expect all HTLCs except for the settled one to be deleted if the + // config is set to delete them. + if !keepFailedPaymentAttempts { + payments[2].htlcs = 1 + } + assertPayments(t, paymentDB, payments) + + // NOTE: In case the option keepFailedPaymentAttempts is set no delete + // operation are performed in general therefore we do NOT expect an + // error in this case. + if keepFailedPaymentAttempts { + // DeleteFailedAttempts is ignored, even for non-existent + // payments, if the control tower is configured to keep failed + // HTLCs. + require.NoError( + t, paymentDB.DeleteFailedAttempts(lntypes.ZeroHash), + ) + } else { + // Attempting to cleanup a non-existent payment returns an + // error. + require.Error( + t, paymentDB.DeleteFailedAttempts(lntypes.ZeroHash), + ) + } +} + +// TestKVPaymentsDBMPPRecordValidation tests MPP record validation. +func TestKVPaymentsDBMPPRecordValidation(t *testing.T) { + t.Parallel() + + paymentDB := NewTestDB(t) + + info, attempt, _, err := genInfo(t) + require.NoError(t, err, "unable to generate htlc message") + + // Init the payment. + err = paymentDB.InitPayment(info.PaymentIdentifier, info) + require.NoError(t, err, "unable to send htlc message") + + // Create three unique attempts we'll use for the test, and + // register them with the payment control. We set each + // attempts's value to one third of the payment amount, and + // populate the MPP options. + shardAmt := info.Value / 3 + attempt.Route.FinalHop().AmtToForward = shardAmt + attempt.Route.FinalHop().MPP = record.NewMPP( + info.Value, [32]byte{1}, + ) + + _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, attempt) + require.NoError(t, err, "unable to send htlc message") + + // Now try to register a non-MPP attempt, which should fail. + b := *attempt + b.AttemptID = 1 + b.Route.FinalHop().MPP = nil + _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, &b) + require.ErrorIs(t, err, ErrMPPayment) + + // Try to register attempt one with a different payment address. + b.Route.FinalHop().MPP = record.NewMPP( + info.Value, [32]byte{2}, + ) + _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, &b) + require.ErrorIs(t, err, ErrMPPPaymentAddrMismatch) + + // Try registering one with a different total amount. + b.Route.FinalHop().MPP = record.NewMPP( + info.Value/2, [32]byte{1}, + ) + _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, &b) + require.ErrorIs(t, err, ErrMPPTotalAmountMismatch) + + // Create and init a new payment. This time we'll check that we cannot + // register an MPP attempt if we already registered a non-MPP one. + info, attempt, _, err = genInfo(t) + require.NoError(t, err, "unable to generate htlc message") + + err = paymentDB.InitPayment(info.PaymentIdentifier, info) + require.NoError(t, err, "unable to send htlc message") + + attempt.Route.FinalHop().MPP = nil + _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, attempt) + require.NoError(t, err, "unable to send htlc message") + + // Attempt to register an MPP attempt, which should fail. + b = *attempt + b.AttemptID = 1 + b.Route.FinalHop().MPP = record.NewMPP( + info.Value, [32]byte{1}, + ) + + _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, &b) + require.ErrorIs(t, err, ErrNonMPPayment) +} + +// TestDeleteSinglePayment tests that DeletePayment correctly +// deletes information about a completed payment from the database. +func TestDeleteSinglePayment(t *testing.T) { + t.Parallel() + + paymentDB := NewTestDB(t) + + // Register four payments: + // All payments will have one failed HTLC attempt and one HTLC attempt + // according to its final status. + // 1. A payment with two failed attempts. + // 2. Another payment with two failed attempts. + // 3. A payment with one failed and one settled attempt. + // 4. A payment with one failed and one in-flight attempt. + + // Initiate payments, which is a slice of payment that is used as + // template to create the corresponding test payments in the database. + // + // Note: The payment id and number of htlc attempts of each payment will + // be added to this slice when creating the payments below. + // This allows the slice to be used directly for testing purposes. + payments := []*payment{ + {status: StatusFailed}, + {status: StatusFailed}, + {status: StatusSucceeded}, + {status: StatusInFlight}, + } + + // Use helper function to register the test payments in the data and + // populate the data to the payments slice. + createTestPayments(t, paymentDB, payments) + + // Check that all payments are there as we added them. + assertPayments(t, paymentDB, payments) + + // Delete HTLC attempts for first payment only. + require.NoError(t, paymentDB.DeletePayment(payments[0].id, true)) + + // The first payment is the only altered one as its failed HTLC should + // have been removed but is still present as payment. + payments[0].htlcs = 0 + assertPayments(t, paymentDB, payments) + + // Delete the first payment completely. + require.NoError(t, paymentDB.DeletePayment(payments[0].id, false)) + + // The first payment should have been deleted. + assertPayments(t, paymentDB, payments[1:]) + + // Now delete the second payment completely. + require.NoError(t, paymentDB.DeletePayment(payments[1].id, false)) + + // The Second payment should have been deleted. + assertPayments(t, paymentDB, payments[2:]) + + // Delete failed HTLC attempts for the third payment. + require.NoError(t, paymentDB.DeletePayment(payments[2].id, true)) + + // Only the successful HTLC attempt should be left for the third + // payment. + payments[2].htlcs = 1 + assertPayments(t, paymentDB, payments[2:]) + + // Now delete the third payment completely. + require.NoError(t, paymentDB.DeletePayment(payments[2].id, false)) + + // Only the last payment should be left. + assertPayments(t, paymentDB, payments[3:]) + + // Deleting HTLC attempts from InFlight payments should not work and an + // error returned. + require.Error(t, paymentDB.DeletePayment(payments[3].id, true)) + + // The payment is InFlight and therefore should not have been altered. + assertPayments(t, paymentDB, payments[3:]) + + // Finally deleting the InFlight payment should also not work and an + // error returned. + require.Error(t, paymentDB.DeletePayment(payments[3].id, false)) + + // The payment is InFlight and therefore should not have been altered. + assertPayments(t, paymentDB, payments[3:]) } -// TestRegistrable checks the method `Registrable` behaves as expected for ALL -// possible payment statuses. -func TestRegistrable(t *testing.T) { +// TestPaymentRegistrable checks the method `Registrable` behaves as expected +// for ALL possible payment statuses. +func TestPaymentRegistrable(t *testing.T) { t.Parallel() testCases := []struct { @@ -1058,3 +1167,98 @@ func TestEmptyRoutesGenerateSphinxPacket(t *testing.T) { _, _, err := generateSphinxPacket(emptyRoute, testHash[:], sessionKey) require.ErrorIs(t, err, route.ErrNoRouteHopsProvided) } + +// TestKVPaymentsDBSuccessesWithoutInFlight tests that the payment control will +// disallow calls to Success when no payment is in flight. +func TestKVPaymentsDBSuccessesWithoutInFlight(t *testing.T) { + t.Parallel() + + paymentDB := NewTestDB(t) + + info, _, preimg, err := genInfo(t) + require.NoError(t, err, "unable to generate htlc message") + + // Attempt to complete the payment should fail. + _, err = paymentDB.SettleAttempt( + info.PaymentIdentifier, 0, + &HTLCSettleInfo{ + Preimage: preimg, + }, + ) + require.ErrorIs(t, err, ErrPaymentNotInitiated) +} + +// TestKVPaymentsDBFailsWithoutInFlight checks that a strict payment control +// will disallow calls to Fail when no payment is in flight. +func TestKVPaymentsDBFailsWithoutInFlight(t *testing.T) { + t.Parallel() + + paymentDB := NewTestDB(t) + + info, _, _, err := genInfo(t) + require.NoError(t, err, "unable to generate htlc message") + + // Calling Fail should return an error. + _, err = paymentDB.Fail( + info.PaymentIdentifier, FailureReasonNoRoute, + ) + require.ErrorIs(t, err, ErrPaymentNotInitiated) +} + +// TestKVPaymentsDBDeletePayments tests that DeletePayments correctly deletes +// information about completed payments from the database. +func TestKVPaymentsDBDeletePayments(t *testing.T) { + t.Parallel() + + paymentDB := NewTestDB(t) + + // Register three payments: + // 1. A payment with two failed attempts. + // 2. A payment with one failed and one settled attempt. + // 3. A payment with one failed and one in-flight attempt. + payments := []*payment{ + {status: StatusFailed}, + {status: StatusSucceeded}, + {status: StatusInFlight}, + } + + // Use helper function to register the test payments in the data and + // populate the data to the payments slice. + createTestPayments(t, paymentDB, payments) + + // Check that all payments are there as we added them. + assertPayments(t, paymentDB, payments) + + // Delete HTLC attempts for failed payments only. + numPayments, err := paymentDB.DeletePayments(true, true) + require.NoError(t, err) + require.EqualValues(t, 0, numPayments) + + // The failed payment is the only altered one. + payments[0].htlcs = 0 + assertPayments(t, paymentDB, payments) + + // Delete failed attempts for all payments. + numPayments, err = paymentDB.DeletePayments(false, true) + require.NoError(t, err) + require.EqualValues(t, 0, numPayments) + + // The failed attempts should be deleted, except for the in-flight + // payment, that shouldn't be altered until it has completed. + payments[1].htlcs = 1 + assertPayments(t, paymentDB, payments) + + // Now delete all failed payments. + numPayments, err = paymentDB.DeletePayments(true, false) + require.NoError(t, err) + require.EqualValues(t, 1, numPayments) + + assertPayments(t, paymentDB, payments[1:]) + + // Finally delete all completed payments. + numPayments, err = paymentDB.DeletePayments(false, false) + require.NoError(t, err) + require.EqualValues(t, 1, numPayments) + + assertPayments(t, paymentDB, payments[2:]) +} diff --git a/payments/db/test_kvdb.go b/payments/db/test_kvdb.go index 25e236e48d4..78c33725b4a 100644 --- a/payments/db/test_kvdb.go +++ b/payments/db/test_kvdb.go @@ -8,7 +8,24 @@ import ( ) // NewTestDB is a helper function that creates an BBolt database for testing. -func NewTestDB(t *testing.T, opts ...OptionModifier) *KVPaymentsDB { +func NewTestDB(t *testing.T, opts ...OptionModifier) DB { + backend, backendCleanup, err := kvdb.GetTestBackend( + t.TempDir(), "paymentsDB", + ) + require.NoError(t, err) + + t.Cleanup(backendCleanup) + + paymentDB, err := NewKVPaymentsDB(backend, opts...) + require.NoError(t, err) + + return paymentDB +} + +// NewKVTestDB is a helper function that creates an BBolt database for testing +// and there is no need to convert the interface to the KVPaymentsDB because for +// some unit tests we still need access to the kvdb interface. +func NewKVTestDB(t *testing.T, opts ...OptionModifier) *KVPaymentsDB { backend, backendCleanup, err := kvdb.GetTestBackend( t.TempDir(), "kvPaymentDB", ) From 8726ba3d7cbae462bf40881d4db659fd8eb8d341 Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 12 Aug 2025 18:55:18 +0200 Subject: [PATCH 05/16] paymentsdb: move more tests we make the index assertion db independant so it is a noop for a future native sql backend. This allows us to reuse even more tests for the different db architectures. --- payments/db/kv_store_test.go | 551 ++--------------------------------- payments/db/payment_test.go | 519 +++++++++++++++++++++++++++++++++ 2 files changed, 540 insertions(+), 530 deletions(-) diff --git a/payments/db/kv_store_test.go b/payments/db/kv_store_test.go index e2a7d552b38..8af8914452e 100644 --- a/payments/db/kv_store_test.go +++ b/payments/db/kv_store_test.go @@ -3,236 +3,21 @@ package paymentsdb import ( "bytes" "context" - "errors" - "fmt" "math" "reflect" "testing" "time" "github.com/btcsuite/btcwallet/walletdb" - "github.com/davecgh/go-spew/spew" "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" - "github.com/lightningnetwork/lnd/record" "github.com/lightningnetwork/lnd/routing/route" "github.com/lightningnetwork/lnd/tlv" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// TestKVPaymentsDBSwitchFail checks that payment status returns to Failed -// status after failing, and that InitPayment allows another HTLC for the -// same payment hash. -func TestKVPaymentsDBSwitchFail(t *testing.T) { - t.Parallel() - - paymentDB := NewKVTestDB(t) - - info, attempt, preimg, err := genInfo(t) - require.NoError(t, err, "unable to generate htlc message") - - // Sends base htlc message which initiate StatusInFlight. - err = paymentDB.InitPayment(info.PaymentIdentifier, info) - require.NoError(t, err, "unable to send htlc message") - - assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) - assertPaymentStatus( - t, paymentDB, info.PaymentIdentifier, StatusInitiated, - ) - assertPaymentInfo( - t, paymentDB, info.PaymentIdentifier, info, nil, nil, - ) - - // Fail the payment, which should moved it to Failed. - failReason := FailureReasonNoRoute - _, err = paymentDB.Fail(info.PaymentIdentifier, failReason) - require.NoError(t, err, "unable to fail payment hash") - - // Verify the status is indeed Failed. - assertPaymentStatus(t, paymentDB, info.PaymentIdentifier, StatusFailed) - assertPaymentInfo( - t, paymentDB, info.PaymentIdentifier, info, &failReason, nil, - ) - - // Lookup the payment so we can get its old sequence number before it is - // overwritten. - payment, err := paymentDB.FetchPayment(info.PaymentIdentifier) - require.NoError(t, err) - - // Sends the htlc again, which should succeed since the prior payment - // failed. - err = paymentDB.InitPayment(info.PaymentIdentifier, info) - require.NoError(t, err, "unable to send htlc message") - - // Check that our index has been updated, and the old index has been - // removed. - assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) - assertNoIndex(t, paymentDB, payment.SequenceNum) - - assertPaymentStatus( - t, paymentDB, info.PaymentIdentifier, StatusInitiated, - ) - assertPaymentInfo( - t, paymentDB, info.PaymentIdentifier, info, nil, nil, - ) - - // Record a new attempt. In this test scenario, the attempt fails. - // However, this is not communicated to control tower in the current - // implementation. It only registers the initiation of the attempt. - _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, attempt) - require.NoError(t, err, "unable to register attempt") - - htlcReason := HTLCFailUnreadable - _, err = paymentDB.FailAttempt( - info.PaymentIdentifier, attempt.AttemptID, - &HTLCFailInfo{ - Reason: htlcReason, - }, - ) - if err != nil { - t.Fatal(err) - } - assertPaymentStatus( - t, paymentDB, info.PaymentIdentifier, StatusInFlight, - ) - - htlc := &htlcStatus{ - HTLCAttemptInfo: attempt, - failure: &htlcReason, - } - - assertPaymentInfo(t, paymentDB, info.PaymentIdentifier, info, nil, htlc) - - // Record another attempt. - attempt.AttemptID = 1 - _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, attempt) - require.NoError(t, err, "unable to send htlc message") - assertPaymentStatus( - t, paymentDB, info.PaymentIdentifier, StatusInFlight, - ) - - htlc = &htlcStatus{ - HTLCAttemptInfo: attempt, - } - - assertPaymentInfo( - t, paymentDB, info.PaymentIdentifier, info, nil, htlc, - ) - - // Settle the attempt and verify that status was changed to - // StatusSucceeded. - payment, err = paymentDB.SettleAttempt( - info.PaymentIdentifier, attempt.AttemptID, - &HTLCSettleInfo{ - Preimage: preimg, - }, - ) - require.NoError(t, err, "error shouldn't have been received, got") - - if len(payment.HTLCs) != 2 { - t.Fatalf("payment should have two htlcs, got: %d", - len(payment.HTLCs)) - } - - err = assertRouteEqual(&payment.HTLCs[0].Route, &attempt.Route) - if err != nil { - t.Fatalf("unexpected route returned: %v vs %v: %v", - spew.Sdump(attempt.Route), - spew.Sdump(payment.HTLCs[0].Route), err) - } - - assertPaymentStatus( - t, paymentDB, info.PaymentIdentifier, StatusSucceeded, - ) - - htlc.settle = &preimg - assertPaymentInfo( - t, paymentDB, info.PaymentIdentifier, info, nil, htlc, - ) - - // Attempt a final payment, which should now fail since the prior - // payment succeed. - err = paymentDB.InitPayment(info.PaymentIdentifier, info) - if !errors.Is(err, ErrAlreadyPaid) { - t.Fatalf("unable to send htlc message: %v", err) - } -} - -// TestKVPaymentsDBSwitchDoubleSend checks the ability of payment control to -// prevent double sending of htlc message, when message is in StatusInFlight. -func TestKVPaymentsDBSwitchDoubleSend(t *testing.T) { - t.Parallel() - - paymentDB := NewKVTestDB(t) - - info, attempt, preimg, err := genInfo(t) - require.NoError(t, err, "unable to generate htlc message") - - // Sends base htlc message which initiate base status and move it to - // StatusInFlight and verifies that it was changed. - err = paymentDB.InitPayment(info.PaymentIdentifier, info) - require.NoError(t, err, "unable to send htlc message") - - assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) - assertPaymentStatus( - t, paymentDB, info.PaymentIdentifier, StatusInitiated, - ) - assertPaymentInfo( - t, paymentDB, info.PaymentIdentifier, info, nil, nil, - ) - - // Try to initiate double sending of htlc message with the same - // payment hash, should result in error indicating that payment has - // already been sent. - err = paymentDB.InitPayment(info.PaymentIdentifier, info) - require.ErrorIs(t, err, ErrPaymentExists) - - // Record an attempt. - _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, attempt) - require.NoError(t, err, "unable to send htlc message") - assertPaymentStatus( - t, paymentDB, info.PaymentIdentifier, StatusInFlight, - ) - - htlc := &htlcStatus{ - HTLCAttemptInfo: attempt, - } - assertPaymentInfo( - t, paymentDB, info.PaymentIdentifier, info, nil, htlc, - ) - - // Sends base htlc message which initiate StatusInFlight. - err = paymentDB.InitPayment(info.PaymentIdentifier, info) - if !errors.Is(err, ErrPaymentInFlight) { - t.Fatalf("payment control wrong behaviour: " + - "double sending must trigger ErrPaymentInFlight error") - } - - // After settling, the error should be ErrAlreadyPaid. - _, err = paymentDB.SettleAttempt( - info.PaymentIdentifier, attempt.AttemptID, - &HTLCSettleInfo{ - Preimage: preimg, - }, - ) - require.NoError(t, err, "error shouldn't have been received, got") - assertPaymentStatus( - t, paymentDB, info.PaymentIdentifier, StatusSucceeded, - ) - - htlc.settle = &preimg - assertPaymentInfo( - t, paymentDB, info.PaymentIdentifier, info, nil, htlc, - ) - - err = paymentDB.InitPayment(info.PaymentIdentifier, info) - if !errors.Is(err, ErrAlreadyPaid) { - t.Fatalf("unable to send htlc message: %v", err) - } -} - // TestKVPaymentsDBDeleteNonInFlight checks that calling DeletePayments only // deletes payments from the database that are not in-flight. func TestKVPaymentsDBDeleteNonInFlight(t *testing.T) { @@ -456,315 +241,6 @@ func TestKVPaymentsDBDeleteNonInFlight(t *testing.T) { require.Equal(t, 1, indexCount) } -// TestKVPaymentsDBMultiShard checks the ability of payment control to -// have multiple in-flight HTLCs for a single payment. -func TestKVPaymentsDBMultiShard(t *testing.T) { - t.Parallel() - - // We will register three HTLC attempts, and always fail the second - // one. We'll generate all combinations of settling/failing the first - // and third HTLC, and assert that the payment status end up as we - // expect. - type testCase struct { - settleFirst bool - settleLast bool - } - - var tests []testCase - for _, f := range []bool{true, false} { - for _, l := range []bool{true, false} { - tests = append(tests, testCase{f, l}) - } - } - - runSubTest := func(t *testing.T, test testCase) { - paymentDB := NewKVTestDB(t) - - info, attempt, preimg, err := genInfo(t) - if err != nil { - t.Fatalf("unable to generate htlc message: %v", err) - } - - // Init the payment, moving it to the StatusInFlight state. - err = paymentDB.InitPayment(info.PaymentIdentifier, info) - if err != nil { - t.Fatalf("unable to send htlc message: %v", err) - } - - assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) - assertPaymentStatus( - t, paymentDB, info.PaymentIdentifier, StatusInitiated, - ) - assertPaymentInfo( - t, paymentDB, info.PaymentIdentifier, info, nil, nil, - ) - - // Create three unique attempts we'll use for the test, and - // register them with the payment control. We set each - // attempts's value to one third of the payment amount, and - // populate the MPP options. - shardAmt := info.Value / 3 - attempt.Route.FinalHop().AmtToForward = shardAmt - attempt.Route.FinalHop().MPP = record.NewMPP( - info.Value, [32]byte{1}, - ) - - var attempts []*HTLCAttemptInfo - for i := uint64(0); i < 3; i++ { - a := *attempt - a.AttemptID = i - attempts = append(attempts, &a) - - _, err = paymentDB.RegisterAttempt( - info.PaymentIdentifier, &a, - ) - if err != nil { - t.Fatalf("unable to send htlc message: %v", err) - } - assertPaymentStatus( - t, paymentDB, info.PaymentIdentifier, - StatusInFlight, - ) - - htlc := &htlcStatus{ - HTLCAttemptInfo: &a, - } - assertPaymentInfo( - t, paymentDB, info.PaymentIdentifier, info, nil, - htlc, - ) - } - - // For a fourth attempt, check that attempting to - // register it will fail since the total sent amount - // will be too large. - b := *attempt - b.AttemptID = 3 - _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, &b) - require.ErrorIs(t, err, ErrValueExceedsAmt) - - // Fail the second attempt. - a := attempts[1] - htlcFail := HTLCFailUnreadable - _, err = paymentDB.FailAttempt( - info.PaymentIdentifier, a.AttemptID, - &HTLCFailInfo{ - Reason: htlcFail, - }, - ) - if err != nil { - t.Fatal(err) - } - - htlc := &htlcStatus{ - HTLCAttemptInfo: a, - failure: &htlcFail, - } - assertPaymentInfo( - t, paymentDB, info.PaymentIdentifier, info, nil, htlc, - ) - - // Payment should still be in-flight. - assertPaymentStatus( - t, paymentDB, info.PaymentIdentifier, StatusInFlight, - ) - - // Depending on the test case, settle or fail the first attempt. - a = attempts[0] - htlc = &htlcStatus{ - HTLCAttemptInfo: a, - } - - var firstFailReason *FailureReason - if test.settleFirst { - _, err := paymentDB.SettleAttempt( - info.PaymentIdentifier, a.AttemptID, - &HTLCSettleInfo{ - Preimage: preimg, - }, - ) - if err != nil { - t.Fatalf("error shouldn't have been "+ - "received, got: %v", err) - } - - // Assert that the HTLC has had the preimage recorded. - htlc.settle = &preimg - assertPaymentInfo( - t, paymentDB, info.PaymentIdentifier, info, nil, - htlc, - ) - } else { - _, err := paymentDB.FailAttempt( - info.PaymentIdentifier, a.AttemptID, - &HTLCFailInfo{ - Reason: htlcFail, - }, - ) - if err != nil { - t.Fatalf("error shouldn't have been "+ - "received, got: %v", err) - } - - // Assert the failure was recorded. - htlc.failure = &htlcFail - assertPaymentInfo( - t, paymentDB, info.PaymentIdentifier, info, nil, - htlc, - ) - - // We also record a payment level fail, to move it into - // a terminal state. - failReason := FailureReasonNoRoute - _, err = paymentDB.Fail( - info.PaymentIdentifier, failReason, - ) - if err != nil { - t.Fatalf("unable to fail payment hash: %v", err) - } - - // Record the reason we failed the payment, such that - // we can assert this later in the test. - firstFailReason = &failReason - - // The payment is now considered pending fail, since - // there is still an active HTLC. - assertPaymentStatus( - t, paymentDB, info.PaymentIdentifier, - StatusInFlight, - ) - } - - // Try to register yet another attempt. This should fail now - // that the payment has reached a terminal condition. - b = *attempt - b.AttemptID = 3 - _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, &b) - if test.settleFirst { - require.ErrorIs( - t, err, ErrPaymentPendingSettled, - ) - } else { - require.ErrorIs( - t, err, ErrPaymentPendingFailed, - ) - } - - assertPaymentStatus( - t, paymentDB, info.PaymentIdentifier, StatusInFlight, - ) - - // Settle or fail the remaining attempt based on the testcase. - a = attempts[2] - htlc = &htlcStatus{ - HTLCAttemptInfo: a, - } - if test.settleLast { - // Settle the last outstanding attempt. - _, err = paymentDB.SettleAttempt( - info.PaymentIdentifier, a.AttemptID, - &HTLCSettleInfo{ - Preimage: preimg, - }, - ) - require.NoError(t, err, "unable to settle") - - htlc.settle = &preimg - assertPaymentInfo( - t, paymentDB, info.PaymentIdentifier, - info, firstFailReason, htlc, - ) - } else { - // Fail the attempt. - _, err := paymentDB.FailAttempt( - info.PaymentIdentifier, a.AttemptID, - &HTLCFailInfo{ - Reason: htlcFail, - }, - ) - if err != nil { - t.Fatalf("error shouldn't have been "+ - "received, got: %v", err) - } - - // Assert the failure was recorded. - htlc.failure = &htlcFail - assertPaymentInfo( - t, paymentDB, info.PaymentIdentifier, info, - firstFailReason, htlc, - ) - - // Check that we can override any perevious terminal - // failure. This is to allow multiple concurrent shard - // write a terminal failure to the database without - // syncing. - failReason := FailureReasonPaymentDetails - _, err = paymentDB.Fail( - info.PaymentIdentifier, failReason, - ) - require.NoError(t, err, "unable to fail") - } - - var ( - finalStatus PaymentStatus - registerErr error - ) - - switch { - // If one of the attempts settled but the other failed with - // terminal error, we would still consider the payment is - // settled. - case test.settleFirst && !test.settleLast: - finalStatus = StatusSucceeded - registerErr = ErrPaymentAlreadySucceeded - - case !test.settleFirst && test.settleLast: - finalStatus = StatusSucceeded - registerErr = ErrPaymentAlreadySucceeded - - // If both failed, we end up in a failed status. - case !test.settleFirst && !test.settleLast: - finalStatus = StatusFailed - registerErr = ErrPaymentAlreadyFailed - - // Otherwise, the payment has a succeed status. - case test.settleFirst && test.settleLast: - finalStatus = StatusSucceeded - registerErr = ErrPaymentAlreadySucceeded - } - - assertPaymentStatus( - t, paymentDB, info.PaymentIdentifier, finalStatus, - ) - - // Finally assert we cannot register more attempts. - _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, &b) - require.Equal(t, registerErr, err) - } - - for _, test := range tests { - subTest := fmt.Sprintf("first=%v, second=%v", - test.settleFirst, test.settleLast) - - t.Run(subTest, func(t *testing.T) { - runSubTest(t, test) - }) - } -} - -// TestDeleteFailedAttempts checks that DeleteFailedAttempts properly removes -// failed HTLCs from finished payments. -func TestDeleteFailedAttempts(t *testing.T) { - t.Parallel() - - t.Run("keep failed payment attempts", func(t *testing.T) { - testDeleteFailedAttempts(t, true) - }) - t.Run("remove failed payment attempts", func(t *testing.T) { - testDeleteFailedAttempts(t, false) - }) -} - type htlcStatus struct { *HTLCAttemptInfo settle *lntypes.Preimage @@ -805,23 +281,38 @@ func fetchPaymentIndexEntry(_ *testing.T, p *KVPaymentsDB, // assertPaymentIndex looks up the index for a payment in the db and checks // that its payment hash matches the expected hash passed in. -func assertPaymentIndex(t *testing.T, p *KVPaymentsDB, - expectedHash lntypes.Hash) { +func assertPaymentIndex(t *testing.T, p DB, expectedHash lntypes.Hash) { + t.Helper() + + // Only the kv implementation uses the index so we exit early if the + // payment db is not a kv implementation. This helps us to reuse the + // same test for both implementations. + kvPaymentDB, ok := p.(*KVPaymentsDB) + if !ok { + return + } // Lookup the payment so that we have its sequence number and check // that is has correctly been indexed in the payment indexes bucket. - pmt, err := p.FetchPayment(expectedHash) + pmt, err := kvPaymentDB.FetchPayment(expectedHash) require.NoError(t, err) - hash, err := fetchPaymentIndexEntry(t, p, pmt.SequenceNum) + hash, err := fetchPaymentIndexEntry(t, kvPaymentDB, pmt.SequenceNum) require.NoError(t, err) assert.Equal(t, expectedHash, *hash) } // assertNoIndex checks that an index for the sequence number provided does not // exist. -func assertNoIndex(t *testing.T, p *KVPaymentsDB, seqNr uint64) { - _, err := fetchPaymentIndexEntry(t, p, seqNr) +func assertNoIndex(t *testing.T, p DB, seqNr uint64) { + t.Helper() + + kvPaymentDB, ok := p.(*KVPaymentsDB) + if !ok { + return + } + + _, err := fetchPaymentIndexEntry(t, kvPaymentDB, seqNr) require.Equal(t, ErrNoSequenceNrIndex, err) } diff --git a/payments/db/payment_test.go b/payments/db/payment_test.go index 6d1cf131462..d246087e899 100644 --- a/payments/db/payment_test.go +++ b/payments/db/payment_test.go @@ -365,6 +365,19 @@ func genInfo(t *testing.T) (*PaymentCreationInfo, *HTLCAttemptInfo, }, &attempt.HTLCAttemptInfo, preimage, nil } +// TestDeleteFailedAttempts checks that DeleteFailedAttempts properly removes +// failed HTLCs from finished payments. +func TestDeleteFailedAttempts(t *testing.T) { + t.Parallel() + + t.Run("keep failed payment attempts", func(t *testing.T) { + testDeleteFailedAttempts(t, true) + }) + t.Run("remove failed payment attempts", func(t *testing.T) { + testDeleteFailedAttempts(t, false) + }) +} + func testDeleteFailedAttempts(t *testing.T, keepFailedPaymentAttempts bool) { paymentDB := NewTestDB( t, WithKeepFailedPaymentAttempts(keepFailedPaymentAttempts), @@ -1262,3 +1275,509 @@ func TestKVPaymentsDBDeletePayments(t *testing.T) { assertPayments(t, paymentDB, payments[2:]) } + +// TestSwitchDoubleSend checks the ability of payment control to +// prevent double sending of htlc message, when message is in StatusInFlight. +func TestSwitchDoubleSend(t *testing.T) { + t.Parallel() + + paymentDB := NewTestDB(t) + + info, attempt, preimg, err := genInfo(t) + require.NoError(t, err, "unable to generate htlc message") + + // Sends base htlc message which initiate base status and move it to + // StatusInFlight and verifies that it was changed. + err = paymentDB.InitPayment(info.PaymentIdentifier, info) + require.NoError(t, err, "unable to send htlc message") + + assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) + assertPaymentStatus( + t, paymentDB, info.PaymentIdentifier, StatusInitiated, + ) + assertPaymentInfo( + t, paymentDB, info.PaymentIdentifier, info, nil, nil, + ) + + // Try to initiate double sending of htlc message with the same + // payment hash, should result in error indicating that payment has + // already been sent. + err = paymentDB.InitPayment(info.PaymentIdentifier, info) + require.ErrorIs(t, err, ErrPaymentExists) + + // Record an attempt. + _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, attempt) + require.NoError(t, err, "unable to send htlc message") + assertPaymentStatus( + t, paymentDB, info.PaymentIdentifier, StatusInFlight, + ) + + htlc := &htlcStatus{ + HTLCAttemptInfo: attempt, + } + assertPaymentInfo( + t, paymentDB, info.PaymentIdentifier, info, nil, htlc, + ) + + // Sends base htlc message which initiate StatusInFlight. + err = paymentDB.InitPayment(info.PaymentIdentifier, info) + if !errors.Is(err, ErrPaymentInFlight) { + t.Fatalf("payment control wrong behaviour: " + + "double sending must trigger ErrPaymentInFlight error") + } + + // After settling, the error should be ErrAlreadyPaid. + _, err = paymentDB.SettleAttempt( + info.PaymentIdentifier, attempt.AttemptID, + &HTLCSettleInfo{ + Preimage: preimg, + }, + ) + require.NoError(t, err, "error shouldn't have been received, got") + assertPaymentStatus( + t, paymentDB, info.PaymentIdentifier, StatusSucceeded, + ) + + htlc.settle = &preimg + assertPaymentInfo( + t, paymentDB, info.PaymentIdentifier, info, nil, htlc, + ) + + err = paymentDB.InitPayment(info.PaymentIdentifier, info) + if !errors.Is(err, ErrAlreadyPaid) { + t.Fatalf("unable to send htlc message: %v", err) + } +} + +// TestSwitchFail checks that payment status returns to Failed status after +// failing, and that InitPayment allows another HTLC for the same payment hash. +func TestSwitchFail(t *testing.T) { + t.Parallel() + + paymentDB := NewTestDB(t) + + info, attempt, preimg, err := genInfo(t) + require.NoError(t, err, "unable to generate htlc message") + + // Sends base htlc message which initiate StatusInFlight. + err = paymentDB.InitPayment(info.PaymentIdentifier, info) + require.NoError(t, err, "unable to send htlc message") + + assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) + assertPaymentStatus( + t, paymentDB, info.PaymentIdentifier, StatusInitiated, + ) + assertPaymentInfo( + t, paymentDB, info.PaymentIdentifier, info, nil, nil, + ) + + // Fail the payment, which should moved it to Failed. + failReason := FailureReasonNoRoute + _, err = paymentDB.Fail(info.PaymentIdentifier, failReason) + require.NoError(t, err, "unable to fail payment hash") + + // Verify the status is indeed Failed. + assertPaymentStatus(t, paymentDB, info.PaymentIdentifier, StatusFailed) + assertPaymentInfo( + t, paymentDB, info.PaymentIdentifier, info, &failReason, nil, + ) + + // Lookup the payment so we can get its old sequence number before it is + // overwritten. + payment, err := paymentDB.FetchPayment(info.PaymentIdentifier) + require.NoError(t, err) + + // Sends the htlc again, which should succeed since the prior payment + // failed. + err = paymentDB.InitPayment(info.PaymentIdentifier, info) + require.NoError(t, err, "unable to send htlc message") + + // Check that our index has been updated, and the old index has been + // removed. + assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) + assertNoIndex(t, paymentDB, payment.SequenceNum) + + assertPaymentStatus( + t, paymentDB, info.PaymentIdentifier, StatusInitiated, + ) + assertPaymentInfo( + t, paymentDB, info.PaymentIdentifier, info, nil, nil, + ) + + // Record a new attempt. In this test scenario, the attempt fails. + // However, this is not communicated to control tower in the current + // implementation. It only registers the initiation of the attempt. + _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, attempt) + require.NoError(t, err, "unable to register attempt") + + htlcReason := HTLCFailUnreadable + _, err = paymentDB.FailAttempt( + info.PaymentIdentifier, attempt.AttemptID, + &HTLCFailInfo{ + Reason: htlcReason, + }, + ) + if err != nil { + t.Fatal(err) + } + assertPaymentStatus( + t, paymentDB, info.PaymentIdentifier, StatusInFlight, + ) + + htlc := &htlcStatus{ + HTLCAttemptInfo: attempt, + failure: &htlcReason, + } + + assertPaymentInfo(t, paymentDB, info.PaymentIdentifier, info, nil, htlc) + + // Record another attempt. + attempt.AttemptID = 1 + _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, attempt) + require.NoError(t, err, "unable to send htlc message") + assertPaymentStatus( + t, paymentDB, info.PaymentIdentifier, StatusInFlight, + ) + + htlc = &htlcStatus{ + HTLCAttemptInfo: attempt, + } + + assertPaymentInfo( + t, paymentDB, info.PaymentIdentifier, info, nil, htlc, + ) + + // Settle the attempt and verify that status was changed to + // StatusSucceeded. + payment, err = paymentDB.SettleAttempt( + info.PaymentIdentifier, attempt.AttemptID, + &HTLCSettleInfo{ + Preimage: preimg, + }, + ) + require.NoError(t, err, "error shouldn't have been received, got") + + if len(payment.HTLCs) != 2 { + t.Fatalf("payment should have two htlcs, got: %d", + len(payment.HTLCs)) + } + + err = assertRouteEqual(&payment.HTLCs[0].Route, &attempt.Route) + if err != nil { + t.Fatalf("unexpected route returned: %v vs %v: %v", + spew.Sdump(attempt.Route), + spew.Sdump(payment.HTLCs[0].Route), err) + } + + assertPaymentStatus( + t, paymentDB, info.PaymentIdentifier, StatusSucceeded, + ) + + htlc.settle = &preimg + assertPaymentInfo( + t, paymentDB, info.PaymentIdentifier, info, nil, htlc, + ) + + // Attempt a final payment, which should now fail since the prior + // payment succeed. + err = paymentDB.InitPayment(info.PaymentIdentifier, info) + if !errors.Is(err, ErrAlreadyPaid) { + t.Fatalf("unable to send htlc message: %v", err) + } +} + +// TestMultiShard checks the ability of payment control to have multiple in- +// flight HTLCs for a single payment. +func TestMultiShard(t *testing.T) { + t.Parallel() + + // We will register three HTLC attempts, and always fail the second + // one. We'll generate all combinations of settling/failing the first + // and third HTLC, and assert that the payment status end up as we + // expect. + type testCase struct { + settleFirst bool + settleLast bool + } + + var tests []testCase + for _, f := range []bool{true, false} { + for _, l := range []bool{true, false} { + tests = append(tests, testCase{f, l}) + } + } + + runSubTest := func(t *testing.T, test testCase) { + paymentDB := NewTestDB(t) + + info, attempt, preimg, err := genInfo(t) + if err != nil { + t.Fatalf("unable to generate htlc message: %v", err) + } + + // Init the payment, moving it to the StatusInFlight state. + err = paymentDB.InitPayment(info.PaymentIdentifier, info) + if err != nil { + t.Fatalf("unable to send htlc message: %v", err) + } + + assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) + assertPaymentStatus( + t, paymentDB, info.PaymentIdentifier, StatusInitiated, + ) + assertPaymentInfo( + t, paymentDB, info.PaymentIdentifier, info, nil, nil, + ) + + // Create three unique attempts we'll use for the test, and + // register them with the payment control. We set each + // attempts's value to one third of the payment amount, and + // populate the MPP options. + shardAmt := info.Value / 3 + attempt.Route.FinalHop().AmtToForward = shardAmt + attempt.Route.FinalHop().MPP = record.NewMPP( + info.Value, [32]byte{1}, + ) + + var attempts []*HTLCAttemptInfo + for i := uint64(0); i < 3; i++ { + a := *attempt + a.AttemptID = i + attempts = append(attempts, &a) + + _, err = paymentDB.RegisterAttempt( + info.PaymentIdentifier, &a, + ) + if err != nil { + t.Fatalf("unable to send htlc message: %v", err) + } + assertPaymentStatus( + t, paymentDB, info.PaymentIdentifier, + StatusInFlight, + ) + + htlc := &htlcStatus{ + HTLCAttemptInfo: &a, + } + assertPaymentInfo( + t, paymentDB, info.PaymentIdentifier, info, nil, + htlc, + ) + } + + // For a fourth attempt, check that attempting to + // register it will fail since the total sent amount + // will be too large. + b := *attempt + b.AttemptID = 3 + _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, &b) + require.ErrorIs(t, err, ErrValueExceedsAmt) + + // Fail the second attempt. + a := attempts[1] + htlcFail := HTLCFailUnreadable + _, err = paymentDB.FailAttempt( + info.PaymentIdentifier, a.AttemptID, + &HTLCFailInfo{ + Reason: htlcFail, + }, + ) + if err != nil { + t.Fatal(err) + } + + htlc := &htlcStatus{ + HTLCAttemptInfo: a, + failure: &htlcFail, + } + assertPaymentInfo( + t, paymentDB, info.PaymentIdentifier, info, nil, htlc, + ) + + // Payment should still be in-flight. + assertPaymentStatus( + t, paymentDB, info.PaymentIdentifier, StatusInFlight, + ) + + // Depending on the test case, settle or fail the first attempt. + a = attempts[0] + htlc = &htlcStatus{ + HTLCAttemptInfo: a, + } + + var firstFailReason *FailureReason + if test.settleFirst { + _, err := paymentDB.SettleAttempt( + info.PaymentIdentifier, a.AttemptID, + &HTLCSettleInfo{ + Preimage: preimg, + }, + ) + if err != nil { + t.Fatalf("error shouldn't have been "+ + "received, got: %v", err) + } + + // Assert that the HTLC has had the preimage recorded. + htlc.settle = &preimg + assertPaymentInfo( + t, paymentDB, info.PaymentIdentifier, info, nil, + htlc, + ) + } else { + _, err := paymentDB.FailAttempt( + info.PaymentIdentifier, a.AttemptID, + &HTLCFailInfo{ + Reason: htlcFail, + }, + ) + if err != nil { + t.Fatalf("error shouldn't have been "+ + "received, got: %v", err) + } + + // Assert the failure was recorded. + htlc.failure = &htlcFail + assertPaymentInfo( + t, paymentDB, info.PaymentIdentifier, info, nil, + htlc, + ) + + // We also record a payment level fail, to move it into + // a terminal state. + failReason := FailureReasonNoRoute + _, err = paymentDB.Fail( + info.PaymentIdentifier, failReason, + ) + if err != nil { + t.Fatalf("unable to fail payment hash: %v", err) + } + + // Record the reason we failed the payment, such that + // we can assert this later in the test. + firstFailReason = &failReason + + // The payment is now considered pending fail, since + // there is still an active HTLC. + assertPaymentStatus( + t, paymentDB, info.PaymentIdentifier, + StatusInFlight, + ) + } + + // Try to register yet another attempt. This should fail now + // that the payment has reached a terminal condition. + b = *attempt + b.AttemptID = 3 + _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, &b) + if test.settleFirst { + require.ErrorIs( + t, err, ErrPaymentPendingSettled, + ) + } else { + require.ErrorIs( + t, err, ErrPaymentPendingFailed, + ) + } + + assertPaymentStatus( + t, paymentDB, info.PaymentIdentifier, StatusInFlight, + ) + + // Settle or fail the remaining attempt based on the testcase. + a = attempts[2] + htlc = &htlcStatus{ + HTLCAttemptInfo: a, + } + if test.settleLast { + // Settle the last outstanding attempt. + _, err = paymentDB.SettleAttempt( + info.PaymentIdentifier, a.AttemptID, + &HTLCSettleInfo{ + Preimage: preimg, + }, + ) + require.NoError(t, err, "unable to settle") + + htlc.settle = &preimg + assertPaymentInfo( + t, paymentDB, info.PaymentIdentifier, + info, firstFailReason, htlc, + ) + } else { + // Fail the attempt. + _, err := paymentDB.FailAttempt( + info.PaymentIdentifier, a.AttemptID, + &HTLCFailInfo{ + Reason: htlcFail, + }, + ) + if err != nil { + t.Fatalf("error shouldn't have been "+ + "received, got: %v", err) + } + + // Assert the failure was recorded. + htlc.failure = &htlcFail + assertPaymentInfo( + t, paymentDB, info.PaymentIdentifier, info, + firstFailReason, htlc, + ) + + // Check that we can override any perevious terminal + // failure. This is to allow multiple concurrent shard + // write a terminal failure to the database without + // syncing. + failReason := FailureReasonPaymentDetails + _, err = paymentDB.Fail( + info.PaymentIdentifier, failReason, + ) + require.NoError(t, err, "unable to fail") + } + + var ( + finalStatus PaymentStatus + registerErr error + ) + + switch { + // If one of the attempts settled but the other failed with + // terminal error, we would still consider the payment is + // settled. + case test.settleFirst && !test.settleLast: + finalStatus = StatusSucceeded + registerErr = ErrPaymentAlreadySucceeded + + case !test.settleFirst && test.settleLast: + finalStatus = StatusSucceeded + registerErr = ErrPaymentAlreadySucceeded + + // If both failed, we end up in a failed status. + case !test.settleFirst && !test.settleLast: + finalStatus = StatusFailed + registerErr = ErrPaymentAlreadyFailed + + // Otherwise, the payment has a succeed status. + case test.settleFirst && test.settleLast: + finalStatus = StatusSucceeded + registerErr = ErrPaymentAlreadySucceeded + } + + assertPaymentStatus( + t, paymentDB, info.PaymentIdentifier, finalStatus, + ) + + // Finally assert we cannot register more attempts. + _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, &b) + require.Equal(t, registerErr, err) + } + + for _, test := range tests { + subTest := fmt.Sprintf("first=%v, second=%v", + test.settleFirst, test.settleLast) + + t.Run(subTest, func(t *testing.T) { + runSubTest(t, test) + }) + } +} From 68a8cf199d59ffd690fc353dafd2522d86bae02b Mon Sep 17 00:00:00 2001 From: ziggie Date: Fri, 15 Aug 2025 21:10:26 +0200 Subject: [PATCH 06/16] paymentsdb: rename assertPayments --- payments/db/kv_store_test.go | 6 +-- payments/db/payment_test.go | 72 ++++++++++++++++++------------------ 2 files changed, 39 insertions(+), 39 deletions(-) diff --git a/payments/db/kv_store_test.go b/payments/db/kv_store_test.go index 8af8914452e..c6b1192da60 100644 --- a/payments/db/kv_store_test.go +++ b/payments/db/kv_store_test.go @@ -105,7 +105,7 @@ func TestKVPaymentsDBDeleteNonInFlight(t *testing.T) { } // Verify the status is indeed Failed. - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusFailed, ) @@ -129,7 +129,7 @@ func TestKVPaymentsDBDeleteNonInFlight(t *testing.T) { " got: %v", err) } - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusSucceeded, ) @@ -143,7 +143,7 @@ func TestKVPaymentsDBDeleteNonInFlight(t *testing.T) { numSuccess++ default: - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) diff --git a/payments/db/payment_test.go b/payments/db/payment_test.go index d246087e899..0ed83112c90 100644 --- a/payments/db/payment_test.go +++ b/payments/db/payment_test.go @@ -271,9 +271,9 @@ func assertPaymentInfo(t *testing.T, p DB, hash lntypes.Hash, } } -// assertPaymentStatus retrieves the status of the payment referred to by hash +// assertPaymentstatus retrieves the status of the payment referred to by hash // and compares it with the expected state. -func assertPaymentStatus(t *testing.T, p DB, hash lntypes.Hash, +func assertPaymentstatus(t *testing.T, p DB, hash lntypes.Hash, expStatus PaymentStatus) { t.Helper() @@ -292,11 +292,11 @@ func assertPaymentStatus(t *testing.T, p DB, hash lntypes.Hash, } } -// assertPayments is a helper function that given a slice of payment and +// assertDBPayments is a helper function that given a slice of payment and // indices for the slice asserts that exactly the same payments in the // slice for the provided indices exist when fetching payments from the // database. -func assertPayments(t *testing.T, paymentDB DB, payments []*payment) { +func assertDBPayments(t *testing.T, paymentDB DB, payments []*payment) { t.Helper() response, err := paymentDB.QueryPayments( @@ -407,7 +407,7 @@ func testDeleteFailedAttempts(t *testing.T, keepFailedPaymentAttempts bool) { createTestPayments(t, paymentDB, payments) // Check that all payments are there as we added them. - assertPayments(t, paymentDB, payments) + assertDBPayments(t, paymentDB, payments) // Calling DeleteFailedAttempts on a failed payment should delete all // HTLCs. @@ -417,7 +417,7 @@ func testDeleteFailedAttempts(t *testing.T, keepFailedPaymentAttempts bool) { if !keepFailedPaymentAttempts { payments[0].htlcs = 0 } - assertPayments(t, paymentDB, payments) + assertDBPayments(t, paymentDB, payments) // Calling DeleteFailedAttempts on an in-flight payment should return // an error. @@ -435,7 +435,7 @@ func testDeleteFailedAttempts(t *testing.T, keepFailedPaymentAttempts bool) { // Since DeleteFailedAttempts returned an error, we should expect the // payment to be unchanged. - assertPayments(t, paymentDB, payments) + assertDBPayments(t, paymentDB, payments) // Cleaning up a successful payment should remove failed htlcs. require.NoError(t, paymentDB.DeleteFailedAttempts(payments[2].id)) @@ -445,7 +445,7 @@ func testDeleteFailedAttempts(t *testing.T, keepFailedPaymentAttempts bool) { if !keepFailedPaymentAttempts { payments[2].htlcs = 1 } - assertPayments(t, paymentDB, payments) + assertDBPayments(t, paymentDB, payments) // NOTE: In case the option keepFailedPaymentAttempts is set no delete // operation are performed in general therefore we do NOT expect an @@ -569,7 +569,7 @@ func TestDeleteSinglePayment(t *testing.T) { createTestPayments(t, paymentDB, payments) // Check that all payments are there as we added them. - assertPayments(t, paymentDB, payments) + assertDBPayments(t, paymentDB, payments) // Delete HTLC attempts for first payment only. require.NoError(t, paymentDB.DeletePayment(payments[0].id, true)) @@ -577,19 +577,19 @@ func TestDeleteSinglePayment(t *testing.T) { // The first payment is the only altered one as its failed HTLC should // have been removed but is still present as payment. payments[0].htlcs = 0 - assertPayments(t, paymentDB, payments) + assertDBPayments(t, paymentDB, payments) // Delete the first payment completely. require.NoError(t, paymentDB.DeletePayment(payments[0].id, false)) // The first payment should have been deleted. - assertPayments(t, paymentDB, payments[1:]) + assertDBPayments(t, paymentDB, payments[1:]) // Now delete the second payment completely. require.NoError(t, paymentDB.DeletePayment(payments[1].id, false)) // The Second payment should have been deleted. - assertPayments(t, paymentDB, payments[2:]) + assertDBPayments(t, paymentDB, payments[2:]) // Delete failed HTLC attempts for the third payment. require.NoError(t, paymentDB.DeletePayment(payments[2].id, true)) @@ -597,27 +597,27 @@ func TestDeleteSinglePayment(t *testing.T) { // Only the successful HTLC attempt should be left for the third // payment. payments[2].htlcs = 1 - assertPayments(t, paymentDB, payments[2:]) + assertDBPayments(t, paymentDB, payments[2:]) // Now delete the third payment completely. require.NoError(t, paymentDB.DeletePayment(payments[2].id, false)) // Only the last payment should be left. - assertPayments(t, paymentDB, payments[3:]) + assertDBPayments(t, paymentDB, payments[3:]) // Deleting HTLC attempts from InFlight payments should not work and an // error returned. require.Error(t, paymentDB.DeletePayment(payments[3].id, true)) // The payment is InFlight and therefore should not have been altered. - assertPayments(t, paymentDB, payments[3:]) + assertDBPayments(t, paymentDB, payments[3:]) // Finally deleting the InFlight payment should also not work and an // error returned. require.Error(t, paymentDB.DeletePayment(payments[3].id, false)) // The payment is InFlight and therefore should not have been altered. - assertPayments(t, paymentDB, payments[3:]) + assertDBPayments(t, paymentDB, payments[3:]) } // TestPaymentRegistrable checks the method `Registrable` behaves as expected @@ -1240,7 +1240,7 @@ func TestKVPaymentsDBDeletePayments(t *testing.T) { createTestPayments(t, paymentDB, payments) // Check that all payments are there as we added them. - assertPayments(t, paymentDB, payments) + assertDBPayments(t, paymentDB, payments) // Delete HTLC attempts for failed payments only. numPayments, err := paymentDB.DeletePayments(true, true) @@ -1249,7 +1249,7 @@ func TestKVPaymentsDBDeletePayments(t *testing.T) { // The failed payment is the only altered one. payments[0].htlcs = 0 - assertPayments(t, paymentDB, payments) + assertDBPayments(t, paymentDB, payments) // Delete failed attempts for all payments. numPayments, err = paymentDB.DeletePayments(false, true) @@ -1259,21 +1259,21 @@ func TestKVPaymentsDBDeletePayments(t *testing.T) { // The failed attempts should be deleted, except for the in-flight // payment, that shouldn't be altered until it has completed. payments[1].htlcs = 1 - assertPayments(t, paymentDB, payments) + assertDBPayments(t, paymentDB, payments) // Now delete all failed payments. numPayments, err = paymentDB.DeletePayments(true, false) require.NoError(t, err) require.EqualValues(t, 1, numPayments) - assertPayments(t, paymentDB, payments[1:]) + assertDBPayments(t, paymentDB, payments[1:]) // Finally delete all completed payments. numPayments, err = paymentDB.DeletePayments(false, false) require.NoError(t, err) require.EqualValues(t, 1, numPayments) - assertPayments(t, paymentDB, payments[2:]) + assertDBPayments(t, paymentDB, payments[2:]) } // TestSwitchDoubleSend checks the ability of payment control to @@ -1292,7 +1292,7 @@ func TestSwitchDoubleSend(t *testing.T) { require.NoError(t, err, "unable to send htlc message") assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInitiated, ) assertPaymentInfo( @@ -1308,7 +1308,7 @@ func TestSwitchDoubleSend(t *testing.T) { // Record an attempt. _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, attempt) require.NoError(t, err, "unable to send htlc message") - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) @@ -1334,7 +1334,7 @@ func TestSwitchDoubleSend(t *testing.T) { }, ) require.NoError(t, err, "error shouldn't have been received, got") - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusSucceeded, ) @@ -1364,7 +1364,7 @@ func TestSwitchFail(t *testing.T) { require.NoError(t, err, "unable to send htlc message") assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInitiated, ) assertPaymentInfo( @@ -1377,7 +1377,7 @@ func TestSwitchFail(t *testing.T) { require.NoError(t, err, "unable to fail payment hash") // Verify the status is indeed Failed. - assertPaymentStatus(t, paymentDB, info.PaymentIdentifier, StatusFailed) + assertPaymentstatus(t, paymentDB, info.PaymentIdentifier, StatusFailed) assertPaymentInfo( t, paymentDB, info.PaymentIdentifier, info, &failReason, nil, ) @@ -1397,7 +1397,7 @@ func TestSwitchFail(t *testing.T) { assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) assertNoIndex(t, paymentDB, payment.SequenceNum) - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInitiated, ) assertPaymentInfo( @@ -1420,7 +1420,7 @@ func TestSwitchFail(t *testing.T) { if err != nil { t.Fatal(err) } - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) @@ -1435,7 +1435,7 @@ func TestSwitchFail(t *testing.T) { attempt.AttemptID = 1 _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, attempt) require.NoError(t, err, "unable to send htlc message") - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) @@ -1469,7 +1469,7 @@ func TestSwitchFail(t *testing.T) { spew.Sdump(payment.HTLCs[0].Route), err) } - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusSucceeded, ) @@ -1522,7 +1522,7 @@ func TestMultiShard(t *testing.T) { } assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInitiated, ) assertPaymentInfo( @@ -1551,7 +1551,7 @@ func TestMultiShard(t *testing.T) { if err != nil { t.Fatalf("unable to send htlc message: %v", err) } - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) @@ -1595,7 +1595,7 @@ func TestMultiShard(t *testing.T) { ) // Payment should still be in-flight. - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) @@ -1659,7 +1659,7 @@ func TestMultiShard(t *testing.T) { // The payment is now considered pending fail, since // there is still an active HTLC. - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) @@ -1680,7 +1680,7 @@ func TestMultiShard(t *testing.T) { ) } - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) @@ -1763,7 +1763,7 @@ func TestMultiShard(t *testing.T) { registerErr = ErrPaymentAlreadySucceeded } - assertPaymentStatus( + assertPaymentstatus( t, paymentDB, info.PaymentIdentifier, finalStatus, ) From a1fc8a3eeeeb408f883c6f73e484edc09e128cdc Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 19 Aug 2025 17:21:44 +0200 Subject: [PATCH 07/16] paymentsdb: rename db agnostic tests to highlight their behaviour --- payments/db/payment_test.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/payments/db/payment_test.go b/payments/db/payment_test.go index 0ed83112c90..413db4140bf 100644 --- a/payments/db/payment_test.go +++ b/payments/db/payment_test.go @@ -466,8 +466,8 @@ func testDeleteFailedAttempts(t *testing.T, keepFailedPaymentAttempts bool) { } } -// TestKVPaymentsDBMPPRecordValidation tests MPP record validation. -func TestKVPaymentsDBMPPRecordValidation(t *testing.T) { +// TestMPPRecordValidation tests MPP record validation. +func TestMPPRecordValidation(t *testing.T) { t.Parallel() paymentDB := NewTestDB(t) @@ -1181,9 +1181,9 @@ func TestEmptyRoutesGenerateSphinxPacket(t *testing.T) { require.ErrorIs(t, err, route.ErrNoRouteHopsProvided) } -// TestKVPaymentsDBSuccessesWithoutInFlight tests that the payment control will -// disallow calls to Success when no payment is in flight. -func TestKVPaymentsDBSuccessesWithoutInFlight(t *testing.T) { +// TestSuccessesWithoutInFlight tests that the payment control will disallow +// calls to Success when no payment is in flight. +func TestSuccessesWithoutInFlight(t *testing.T) { t.Parallel() paymentDB := NewTestDB(t) @@ -1201,9 +1201,9 @@ func TestKVPaymentsDBSuccessesWithoutInFlight(t *testing.T) { require.ErrorIs(t, err, ErrPaymentNotInitiated) } -// TestKVPaymentsDBFailsWithoutInFlight checks that a strict payment control -// will disallow calls to Fail when no payment is in flight. -func TestKVPaymentsDBFailsWithoutInFlight(t *testing.T) { +// TestFailsWithoutInFlight checks that a strict payment control will disallow +// calls to Fail when no payment is in flight. +func TestFailsWithoutInFlight(t *testing.T) { t.Parallel() paymentDB := NewTestDB(t) @@ -1218,9 +1218,9 @@ func TestKVPaymentsDBFailsWithoutInFlight(t *testing.T) { require.ErrorIs(t, err, ErrPaymentNotInitiated) } -// TestKVPaymentsDBDeletePayments tests that DeletePayments correctly deletes -// information about completed payments from the database. -func TestKVPaymentsDBDeletePayments(t *testing.T) { +// TestDeletePayments tests that DeletePayments correctly deletes information +// about completed payments from the database. +func TestDeletePayments(t *testing.T) { t.Parallel() paymentDB := NewTestDB(t) From 7423bfece9c9f5ca4c9fefa70cec7951c2222c92 Mon Sep 17 00:00:00 2001 From: ziggie Date: Fri, 15 Aug 2025 21:17:11 +0200 Subject: [PATCH 08/16] paymentsdb: rename assertPaymentstatus --- payments/db/kv_store_test.go | 6 +++--- payments/db/payment_test.go | 34 +++++++++++++++++----------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/payments/db/kv_store_test.go b/payments/db/kv_store_test.go index c6b1192da60..547273fd0ab 100644 --- a/payments/db/kv_store_test.go +++ b/payments/db/kv_store_test.go @@ -105,7 +105,7 @@ func TestKVPaymentsDBDeleteNonInFlight(t *testing.T) { } // Verify the status is indeed Failed. - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusFailed, ) @@ -129,7 +129,7 @@ func TestKVPaymentsDBDeleteNonInFlight(t *testing.T) { " got: %v", err) } - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusSucceeded, ) @@ -143,7 +143,7 @@ func TestKVPaymentsDBDeleteNonInFlight(t *testing.T) { numSuccess++ default: - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) diff --git a/payments/db/payment_test.go b/payments/db/payment_test.go index 413db4140bf..ba17dc9a63c 100644 --- a/payments/db/payment_test.go +++ b/payments/db/payment_test.go @@ -271,9 +271,9 @@ func assertPaymentInfo(t *testing.T, p DB, hash lntypes.Hash, } } -// assertPaymentstatus retrieves the status of the payment referred to by hash +// assertDBPaymentstatus retrieves the status of the payment referred to by hash // and compares it with the expected state. -func assertPaymentstatus(t *testing.T, p DB, hash lntypes.Hash, +func assertDBPaymentstatus(t *testing.T, p DB, hash lntypes.Hash, expStatus PaymentStatus) { t.Helper() @@ -1292,7 +1292,7 @@ func TestSwitchDoubleSend(t *testing.T) { require.NoError(t, err, "unable to send htlc message") assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInitiated, ) assertPaymentInfo( @@ -1308,7 +1308,7 @@ func TestSwitchDoubleSend(t *testing.T) { // Record an attempt. _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, attempt) require.NoError(t, err, "unable to send htlc message") - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) @@ -1334,7 +1334,7 @@ func TestSwitchDoubleSend(t *testing.T) { }, ) require.NoError(t, err, "error shouldn't have been received, got") - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusSucceeded, ) @@ -1364,7 +1364,7 @@ func TestSwitchFail(t *testing.T) { require.NoError(t, err, "unable to send htlc message") assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInitiated, ) assertPaymentInfo( @@ -1377,7 +1377,7 @@ func TestSwitchFail(t *testing.T) { require.NoError(t, err, "unable to fail payment hash") // Verify the status is indeed Failed. - assertPaymentstatus(t, paymentDB, info.PaymentIdentifier, StatusFailed) + assertDBPaymentstatus(t, paymentDB, info.PaymentIdentifier, StatusFailed) assertPaymentInfo( t, paymentDB, info.PaymentIdentifier, info, &failReason, nil, ) @@ -1397,7 +1397,7 @@ func TestSwitchFail(t *testing.T) { assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) assertNoIndex(t, paymentDB, payment.SequenceNum) - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInitiated, ) assertPaymentInfo( @@ -1420,7 +1420,7 @@ func TestSwitchFail(t *testing.T) { if err != nil { t.Fatal(err) } - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) @@ -1435,7 +1435,7 @@ func TestSwitchFail(t *testing.T) { attempt.AttemptID = 1 _, err = paymentDB.RegisterAttempt(info.PaymentIdentifier, attempt) require.NoError(t, err, "unable to send htlc message") - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) @@ -1469,7 +1469,7 @@ func TestSwitchFail(t *testing.T) { spew.Sdump(payment.HTLCs[0].Route), err) } - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusSucceeded, ) @@ -1522,7 +1522,7 @@ func TestMultiShard(t *testing.T) { } assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInitiated, ) assertPaymentInfo( @@ -1551,7 +1551,7 @@ func TestMultiShard(t *testing.T) { if err != nil { t.Fatalf("unable to send htlc message: %v", err) } - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) @@ -1595,7 +1595,7 @@ func TestMultiShard(t *testing.T) { ) // Payment should still be in-flight. - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) @@ -1659,7 +1659,7 @@ func TestMultiShard(t *testing.T) { // The payment is now considered pending fail, since // there is still an active HTLC. - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) @@ -1680,7 +1680,7 @@ func TestMultiShard(t *testing.T) { ) } - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInFlight, ) @@ -1763,7 +1763,7 @@ func TestMultiShard(t *testing.T) { registerErr = ErrPaymentAlreadySucceeded } - assertPaymentstatus( + assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, finalStatus, ) From 8245e356e5e941839229137244ea4513bf03aeb8 Mon Sep 17 00:00:00 2001 From: ziggie Date: Wed, 13 Aug 2025 15:06:44 +0200 Subject: [PATCH 09/16] paymentsdb: move serialization methods to kv_store file --- payments/db/kv_store.go | 89 ++++++++++++++++++++++++++++++++++++++++ payments/db/payment.go | 91 ----------------------------------------- 2 files changed, 89 insertions(+), 91 deletions(-) diff --git a/payments/db/kv_store.go b/payments/db/kv_store.go index afc7dec0ac0..343f77eeaef 100644 --- a/payments/db/kv_store.go +++ b/payments/db/kv_store.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "math" "sort" "sync" "time" @@ -2107,3 +2108,91 @@ func DeserializeRoute(r io.Reader) (route.Route, error) { return rt, nil } + +// serializeHTLCSettleInfo serializes the details of a settled htlc. +func serializeHTLCSettleInfo(w io.Writer, s *HTLCSettleInfo) error { + if _, err := w.Write(s.Preimage[:]); err != nil { + return err + } + + if err := serializeTime(w, s.SettleTime); err != nil { + return err + } + + return nil +} + +// deserializeHTLCSettleInfo deserializes the details of a settled htlc. +func deserializeHTLCSettleInfo(r io.Reader) (*HTLCSettleInfo, error) { + s := &HTLCSettleInfo{} + if _, err := io.ReadFull(r, s.Preimage[:]); err != nil { + return nil, err + } + + var err error + s.SettleTime, err = deserializeTime(r) + if err != nil { + return nil, err + } + + return s, nil +} + +// serializeHTLCFailInfo serializes the details of a failed htlc including the +// wire failure. +func serializeHTLCFailInfo(w io.Writer, f *HTLCFailInfo) error { + if err := serializeTime(w, f.FailTime); err != nil { + return err + } + + // Write failure. If there is no failure message, write an empty + // byte slice. + var messageBytes bytes.Buffer + if f.Message != nil { + err := lnwire.EncodeFailureMessage(&messageBytes, f.Message, 0) + if err != nil { + return err + } + } + if err := wire.WriteVarBytes(w, 0, messageBytes.Bytes()); err != nil { + return err + } + + return WriteElements(w, byte(f.Reason), f.FailureSourceIndex) +} + +// deserializeHTLCFailInfo deserializes the details of a failed htlc including +// the wire failure. +func deserializeHTLCFailInfo(r io.Reader) (*HTLCFailInfo, error) { + f := &HTLCFailInfo{} + var err error + f.FailTime, err = deserializeTime(r) + if err != nil { + return nil, err + } + + // Read failure. + failureBytes, err := wire.ReadVarBytes( + r, 0, math.MaxUint16, "failure", + ) + if err != nil { + return nil, err + } + if len(failureBytes) > 0 { + f.Message, err = lnwire.DecodeFailureMessage( + bytes.NewReader(failureBytes), 0, + ) + if err != nil { + return nil, err + } + } + + var reason byte + err = ReadElements(r, &reason, &f.FailureSourceIndex) + if err != nil { + return nil, err + } + f.Reason = HTLCFailReason(reason) + + return f, nil +} diff --git a/payments/db/payment.go b/payments/db/payment.go index 72338a4e50e..2c5931a4de4 100644 --- a/payments/db/payment.go +++ b/payments/db/payment.go @@ -4,12 +4,9 @@ import ( "bytes" "errors" "fmt" - "io" - "math" "time" "github.com/btcsuite/btcd/btcec/v2" - "github.com/btcsuite/btcd/wire" "github.com/davecgh/go-spew/spew" sphinx "github.com/lightningnetwork/lightning-onion" "github.com/lightningnetwork/lnd/lntypes" @@ -650,94 +647,6 @@ func (m *MPPayment) AllowMoreAttempts() (bool, error) { return true, nil } -// serializeHTLCSettleInfo serializes the details of a settled htlc. -func serializeHTLCSettleInfo(w io.Writer, s *HTLCSettleInfo) error { - if _, err := w.Write(s.Preimage[:]); err != nil { - return err - } - - if err := serializeTime(w, s.SettleTime); err != nil { - return err - } - - return nil -} - -// deserializeHTLCSettleInfo deserializes the details of a settled htlc. -func deserializeHTLCSettleInfo(r io.Reader) (*HTLCSettleInfo, error) { - s := &HTLCSettleInfo{} - if _, err := io.ReadFull(r, s.Preimage[:]); err != nil { - return nil, err - } - - var err error - s.SettleTime, err = deserializeTime(r) - if err != nil { - return nil, err - } - - return s, nil -} - -// serializeHTLCFailInfo serializes the details of a failed htlc including the -// wire failure. -func serializeHTLCFailInfo(w io.Writer, f *HTLCFailInfo) error { - if err := serializeTime(w, f.FailTime); err != nil { - return err - } - - // Write failure. If there is no failure message, write an empty - // byte slice. - var messageBytes bytes.Buffer - if f.Message != nil { - err := lnwire.EncodeFailureMessage(&messageBytes, f.Message, 0) - if err != nil { - return err - } - } - if err := wire.WriteVarBytes(w, 0, messageBytes.Bytes()); err != nil { - return err - } - - return WriteElements(w, byte(f.Reason), f.FailureSourceIndex) -} - -// deserializeHTLCFailInfo deserializes the details of a failed htlc including -// the wire failure. -func deserializeHTLCFailInfo(r io.Reader) (*HTLCFailInfo, error) { - f := &HTLCFailInfo{} - var err error - f.FailTime, err = deserializeTime(r) - if err != nil { - return nil, err - } - - // Read failure. - failureBytes, err := wire.ReadVarBytes( - r, 0, math.MaxUint16, "failure", - ) - if err != nil { - return nil, err - } - if len(failureBytes) > 0 { - f.Message, err = lnwire.DecodeFailureMessage( - bytes.NewReader(failureBytes), 0, - ) - if err != nil { - return nil, err - } - } - - var reason byte - err = ReadElements(r, &reason, &f.FailureSourceIndex) - if err != nil { - return nil, err - } - f.Reason = HTLCFailReason(reason) - - return f, nil -} - // generateSphinxPacket generates then encodes a sphinx packet which encodes // the onion route specified by the passed layer 3 route. The blob returned // from this function can immediately be included within an HTLC add packet to From 6abd539a2d326d4a4adcab791e9c39584c01c3b4 Mon Sep 17 00:00:00 2001 From: ziggie Date: Wed, 13 Aug 2025 14:34:46 +0200 Subject: [PATCH 10/16] paymentsdb: add missing function comments --- payments/db/payment_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/payments/db/payment_test.go b/payments/db/payment_test.go index ba17dc9a63c..ea5fcb8df4d 100644 --- a/payments/db/payment_test.go +++ b/payments/db/payment_test.go @@ -330,6 +330,7 @@ func assertDBPayments(t *testing.T, paymentDB DB, payments []*payment) { require.Equal(t, payments, p) } +// genPreimage generates a random preimage. func genPreimage() ([32]byte, error) { var preimage [32]byte if _, err := io.ReadFull(rand.Reader, preimage[:]); err != nil { @@ -339,6 +340,7 @@ func genPreimage() ([32]byte, error) { return preimage, nil } +// genInfo generates a payment creation info, an attempt info and a preimage. func genInfo(t *testing.T) (*PaymentCreationInfo, *HTLCAttemptInfo, lntypes.Preimage, error) { @@ -378,6 +380,8 @@ func TestDeleteFailedAttempts(t *testing.T) { }) } +// testDeleteFailedAttempts tests the DeleteFailedAttempts method with the +// given keepFailedPaymentAttempts flag as argument. func testDeleteFailedAttempts(t *testing.T, keepFailedPaymentAttempts bool) { paymentDB := NewTestDB( t, WithKeepFailedPaymentAttempts(keepFailedPaymentAttempts), From 82242f5342a6f3a853fd13a2d955e361389b18bf Mon Sep 17 00:00:00 2001 From: ziggie Date: Wed, 13 Aug 2025 14:42:34 +0200 Subject: [PATCH 11/16] multi: rename KVPaymentDB to KVStore This matches the same naming as used in the graph package. --- config_builder.go | 2 +- payments/db/kv_store.go | 44 +++++++++++++++++------------------ payments/db/kv_store_test.go | 12 +++++----- payments/db/test_kvdb.go | 8 +++---- routing/control_tower_test.go | 38 +++++++++++++++--------------- 5 files changed, 52 insertions(+), 52 deletions(-) diff --git a/config_builder.go b/config_builder.go index af4a06ed38c..aecd5c18036 100644 --- a/config_builder.go +++ b/config_builder.go @@ -1225,7 +1225,7 @@ func (d *DefaultDatabaseBuilder) BuildDatabase( cfg.KeepFailedPaymentAttempts, ), } - kvPaymentsDB, err := paymentsdb.NewKVPaymentsDB( + kvPaymentsDB, err := paymentsdb.NewKVStore( dbs.ChanStateDB, paymentsDBOptions..., ) diff --git a/payments/db/kv_store.go b/payments/db/kv_store.go index 343f77eeaef..bde4b2e923b 100644 --- a/payments/db/kv_store.go +++ b/payments/db/kv_store.go @@ -118,8 +118,8 @@ var ( paymentsIndexBucket = []byte("payments-index-bucket") ) -// KVPaymentsDB implements persistence for payments and payment attempts. -type KVPaymentsDB struct { +// KVStore implements persistence for payments and payment attempts. +type KVStore struct { // Sequence management for the kv store. seqMu sync.Mutex currSeq uint64 @@ -140,9 +140,9 @@ func defaultKVStoreOptions() *StoreOptions { } } -// NewKVPaymentsDB creates a new KVStore for payments. -func NewKVPaymentsDB(db kvdb.Backend, - options ...OptionModifier) (*KVPaymentsDB, error) { +// NewKVStore creates a new KVStore for payments. +func NewKVStore(db kvdb.Backend, + options ...OptionModifier) (*KVStore, error) { opts := defaultKVStoreOptions() for _, applyOption := range options { @@ -155,7 +155,7 @@ func NewKVPaymentsDB(db kvdb.Backend, } } - return &KVPaymentsDB{ + return &KVStore{ db: db, keepFailedPaymentAttempts: opts.KeepFailedPaymentAttempts, }, nil @@ -190,7 +190,7 @@ func initKVStore(db kvdb.Backend) error { // making sure it does not already exist as an in-flight payment. When this // method returns successfully, the payment is guaranteed to be in the InFlight // state. -func (p *KVPaymentsDB) InitPayment(paymentHash lntypes.Hash, +func (p *KVStore) InitPayment(paymentHash lntypes.Hash, info *PaymentCreationInfo) error { // Obtain a new sequence number for this payment. This is used @@ -293,8 +293,8 @@ func (p *KVPaymentsDB) InitPayment(paymentHash lntypes.Hash, } // DeleteFailedAttempts deletes all failed htlcs for a payment if configured -// by the KVPaymentsDB db. -func (p *KVPaymentsDB) DeleteFailedAttempts(hash lntypes.Hash) error { +// by the KVStore db. +func (p *KVStore) DeleteFailedAttempts(hash lntypes.Hash) error { if !p.keepFailedPaymentAttempts { const failedHtlcsOnly = true err := p.DeletePayment(hash, failedHtlcsOnly) @@ -361,7 +361,7 @@ func deserializePaymentIndex(r io.Reader) (lntypes.Hash, error) { // RegisterAttempt atomically records the provided HTLCAttemptInfo to the // DB. -func (p *KVPaymentsDB) RegisterAttempt(paymentHash lntypes.Hash, +func (p *KVStore) RegisterAttempt(paymentHash lntypes.Hash, attempt *HTLCAttemptInfo) (*MPPayment, error) { // Serialize the information before opening the db transaction. @@ -511,7 +511,7 @@ func (p *KVPaymentsDB) RegisterAttempt(paymentHash lntypes.Hash, // After invoking this method, InitPayment should always return an error to // prevent us from making duplicate payments to the same payment hash. The // provided preimage is atomically saved to the DB for record keeping. -func (p *KVPaymentsDB) SettleAttempt(hash lntypes.Hash, +func (p *KVStore) SettleAttempt(hash lntypes.Hash, attemptID uint64, settleInfo *HTLCSettleInfo) (*MPPayment, error) { var b bytes.Buffer @@ -524,7 +524,7 @@ func (p *KVPaymentsDB) SettleAttempt(hash lntypes.Hash, } // FailAttempt marks the given payment attempt failed. -func (p *KVPaymentsDB) FailAttempt(hash lntypes.Hash, +func (p *KVStore) FailAttempt(hash lntypes.Hash, attemptID uint64, failInfo *HTLCFailInfo) (*MPPayment, error) { var b bytes.Buffer @@ -537,7 +537,7 @@ func (p *KVPaymentsDB) FailAttempt(hash lntypes.Hash, } // updateHtlcKey updates a database key for the specified htlc. -func (p *KVPaymentsDB) updateHtlcKey(paymentHash lntypes.Hash, +func (p *KVStore) updateHtlcKey(paymentHash lntypes.Hash, attemptID uint64, key, value []byte) (*MPPayment, error) { aid := make([]byte, 8) @@ -609,7 +609,7 @@ func (p *KVPaymentsDB) updateHtlcKey(paymentHash lntypes.Hash, // payment failed. After invoking this method, InitPayment should return nil on // its next call for this payment hash, allowing the switch to make a // subsequent payment. -func (p *KVPaymentsDB) Fail(paymentHash lntypes.Hash, +func (p *KVStore) Fail(paymentHash lntypes.Hash, reason FailureReason) (*MPPayment, error) { var ( @@ -633,7 +633,7 @@ func (p *KVPaymentsDB) Fail(paymentHash lntypes.Hash, // We mark the payment as failed as long as it is known. This // lets the last attempt to fail with a terminal write its - // failure to the KVPaymentsDB without synchronizing with + // failure to the KVStore without synchronizing with // other attempts. _, err = fetchPaymentStatus(bucket) if errors.Is(err, ErrPaymentNotInitiated) { @@ -666,7 +666,7 @@ func (p *KVPaymentsDB) Fail(paymentHash lntypes.Hash, } // FetchPayment returns information about a payment from the database. -func (p *KVPaymentsDB) FetchPayment(paymentHash lntypes.Hash) ( +func (p *KVStore) FetchPayment(paymentHash lntypes.Hash) ( *MPPayment, error) { var payment *MPPayment @@ -761,7 +761,7 @@ func fetchPaymentBucketUpdate(tx kvdb.RwTx, paymentHash lntypes.Hash) ( // nextPaymentSequence returns the next sequence number to store for a new // payment. -func (p *KVPaymentsDB) nextPaymentSequence() ([]byte, error) { +func (p *KVStore) nextPaymentSequence() ([]byte, error) { p.seqMu.Lock() defer p.seqMu.Unlock() @@ -822,7 +822,7 @@ func fetchPaymentStatus(bucket kvdb.RBucket) (PaymentStatus, error) { } // FetchInFlightPayments returns all payments with status InFlight. -func (p *KVPaymentsDB) FetchInFlightPayments() ([]*MPPayment, error) { +func (p *KVStore) FetchInFlightPayments() ([]*MPPayment, error) { var ( inFlights []*MPPayment start = time.Now() @@ -895,7 +895,7 @@ func htlcBucketKey(prefix, id []byte) []byte { } // FetchPayments returns all sent payments found in the DB. -func (p *KVPaymentsDB) FetchPayments() ([]*MPPayment, error) { +func (p *KVStore) FetchPayments() ([]*MPPayment, error) { var payments []*MPPayment err := kvdb.View(p.db, func(tx kvdb.RTx) error { @@ -1135,7 +1135,7 @@ func fetchFailedHtlcKeys(bucket kvdb.RBucket) ([][]byte, error) { // QueryPayments is a query to the payments database which is restricted // to a subset of payments by the payments query, containing an offset // index and a maximum number of returned payments. -func (p *KVPaymentsDB) QueryPayments(_ context.Context, +func (p *KVStore) QueryPayments(_ context.Context, query Query) (Response, error) { var resp Response @@ -1356,7 +1356,7 @@ func fetchPaymentWithSequenceNumber(tx kvdb.RTx, paymentHash lntypes.Hash, // DeletePayment deletes a payment from the DB given its payment hash. If // failedHtlcsOnly is set, only failed HTLC attempts of the payment will be // deleted. -func (p *KVPaymentsDB) DeletePayment(paymentHash lntypes.Hash, +func (p *KVStore) DeletePayment(paymentHash lntypes.Hash, failedHtlcsOnly bool) error { return kvdb.Update(p.db, func(tx kvdb.RwTx) error { @@ -1453,7 +1453,7 @@ func (p *KVPaymentsDB) DeletePayment(paymentHash lntypes.Hash, // failedHtlcsOnly is set, the payment itself won't be deleted, only failed HTLC // attempts. The method returns the number of deleted payments, which is always // 0 if failedHtlcsOnly is set. -func (p *KVPaymentsDB) DeletePayments(failedOnly, +func (p *KVStore) DeletePayments(failedOnly, failedHtlcsOnly bool) (int, error) { var numPayments int diff --git a/payments/db/kv_store_test.go b/payments/db/kv_store_test.go index 547273fd0ab..b3d7887bd5f 100644 --- a/payments/db/kv_store_test.go +++ b/payments/db/kv_store_test.go @@ -18,9 +18,9 @@ import ( "github.com/stretchr/testify/require" ) -// TestKVPaymentsDBDeleteNonInFlight checks that calling DeletePayments only +// TestKVStoreDeleteNonInFlight checks that calling DeletePayments only // deletes payments from the database that are not in-flight. -func TestKVPaymentsDBDeleteNonInFlight(t *testing.T) { +func TestKVStoreDeleteNonInFlight(t *testing.T) { t.Parallel() paymentDB := NewKVTestDB(t) @@ -249,7 +249,7 @@ type htlcStatus struct { // fetchPaymentIndexEntry gets the payment hash for the sequence number provided // from our payment indexes bucket. -func fetchPaymentIndexEntry(_ *testing.T, p *KVPaymentsDB, +func fetchPaymentIndexEntry(_ *testing.T, p *KVStore, sequenceNumber uint64) (*lntypes.Hash, error) { var hash lntypes.Hash @@ -287,7 +287,7 @@ func assertPaymentIndex(t *testing.T, p DB, expectedHash lntypes.Hash) { // Only the kv implementation uses the index so we exit early if the // payment db is not a kv implementation. This helps us to reuse the // same test for both implementations. - kvPaymentDB, ok := p.(*KVPaymentsDB) + kvPaymentDB, ok := p.(*KVStore) if !ok { return } @@ -307,7 +307,7 @@ func assertPaymentIndex(t *testing.T, p DB, expectedHash lntypes.Hash) { func assertNoIndex(t *testing.T, p DB, seqNr uint64) { t.Helper() - kvPaymentDB, ok := p.(*KVPaymentsDB) + kvPaymentDB, ok := p.(*KVStore) if !ok { return } @@ -929,7 +929,7 @@ func TestQueryPayments(t *testing.T) { paymentDB := NewKVTestDB(t) // Initialize the payment database. - paymentDB, err := NewKVPaymentsDB(paymentDB.db) + paymentDB, err := NewKVStore(paymentDB.db) require.NoError(t, err) // Make a preliminary query to make sure it's ok to diff --git a/payments/db/test_kvdb.go b/payments/db/test_kvdb.go index 78c33725b4a..e0ee1738d7a 100644 --- a/payments/db/test_kvdb.go +++ b/payments/db/test_kvdb.go @@ -16,16 +16,16 @@ func NewTestDB(t *testing.T, opts ...OptionModifier) DB { t.Cleanup(backendCleanup) - paymentDB, err := NewKVPaymentsDB(backend, opts...) + paymentDB, err := NewKVStore(backend, opts...) require.NoError(t, err) return paymentDB } // NewKVTestDB is a helper function that creates an BBolt database for testing -// and there is no need to convert the interface to the KVPaymentsDB because for +// and there is no need to convert the interface to the KVStore because for // some unit tests we still need access to the kvdb interface. -func NewKVTestDB(t *testing.T, opts ...OptionModifier) *KVPaymentsDB { +func NewKVTestDB(t *testing.T, opts ...OptionModifier) *KVStore { backend, backendCleanup, err := kvdb.GetTestBackend( t.TempDir(), "kvPaymentDB", ) @@ -33,7 +33,7 @@ func NewKVTestDB(t *testing.T, opts ...OptionModifier) *KVPaymentsDB { t.Cleanup(backendCleanup) - paymentDB, err := NewKVPaymentsDB(backend, opts...) + paymentDB, err := NewKVStore(backend, opts...) require.NoError(t, err) return paymentDB diff --git a/routing/control_tower_test.go b/routing/control_tower_test.go index 3e01065962d..de0aacf880b 100644 --- a/routing/control_tower_test.go +++ b/routing/control_tower_test.go @@ -50,7 +50,7 @@ func TestControlTowerSubscribeUnknown(t *testing.T) { db := initDB(t) - paymentDB, err := paymentsdb.NewKVPaymentsDB( + paymentDB, err := paymentsdb.NewKVStore( db, paymentsdb.WithKeepFailedPaymentAttempts(true), ) @@ -70,7 +70,7 @@ func TestControlTowerSubscribeSuccess(t *testing.T) { db := initDB(t) - paymentDB, err := paymentsdb.NewKVPaymentsDB(db) + paymentDB, err := paymentsdb.NewKVStore(db) require.NoError(t, err) pControl := NewControlTower(paymentDB) @@ -174,33 +174,33 @@ func TestControlTowerSubscribeSuccess(t *testing.T) { } } -// TestKVPaymentsDBSubscribeFail tests that payment updates for a +// TestKVStoreSubscribeFail tests that payment updates for a // failed payment are properly sent to subscribers. -func TestKVPaymentsDBSubscribeFail(t *testing.T) { +func TestKVStoreSubscribeFail(t *testing.T) { t.Parallel() t.Run("register attempt, keep failed payments", func(t *testing.T) { - testKVPaymentsDBSubscribeFail(t, true, true) + testKVStoreSubscribeFail(t, true, true) }) t.Run("register attempt, delete failed payments", func(t *testing.T) { - testKVPaymentsDBSubscribeFail(t, true, false) + testKVStoreSubscribeFail(t, true, false) }) t.Run("no register attempt, keep failed payments", func(t *testing.T) { - testKVPaymentsDBSubscribeFail(t, false, true) + testKVStoreSubscribeFail(t, false, true) }) t.Run("no register attempt, delete failed payments", func(t *testing.T) { - testKVPaymentsDBSubscribeFail(t, false, false) + testKVStoreSubscribeFail(t, false, false) }) } -// TestKVPaymentsDBSubscribeAllSuccess tests that multiple payments are +// TestKVStoreSubscribeAllSuccess tests that multiple payments are // properly sent to subscribers of TrackPayments. -func TestKVPaymentsDBSubscribeAllSuccess(t *testing.T) { +func TestKVStoreSubscribeAllSuccess(t *testing.T) { t.Parallel() db := initDB(t) - paymentDB, err := paymentsdb.NewKVPaymentsDB( + paymentDB, err := paymentsdb.NewKVStore( db, paymentsdb.WithKeepFailedPaymentAttempts(true), ) @@ -318,14 +318,14 @@ func TestKVPaymentsDBSubscribeAllSuccess(t *testing.T) { require.Equal(t, attempt2.Route, htlc2.Route, "unexpected htlc route.") } -// TestKVPaymentsDBSubscribeAllImmediate tests whether already inflight +// TestKVStoreSubscribeAllImmediate tests whether already inflight // payments are reported at the start of the SubscribeAllPayments subscription. -func TestKVPaymentsDBSubscribeAllImmediate(t *testing.T) { +func TestKVStoreSubscribeAllImmediate(t *testing.T) { t.Parallel() db := initDB(t) - paymentDB, err := paymentsdb.NewKVPaymentsDB( + paymentDB, err := paymentsdb.NewKVStore( db, paymentsdb.WithKeepFailedPaymentAttempts(true), ) @@ -367,14 +367,14 @@ func TestKVPaymentsDBSubscribeAllImmediate(t *testing.T) { } } -// TestKVPaymentsDBUnsubscribeSuccess tests that when unsubscribed, there are +// TestKVStoreUnsubscribeSuccess tests that when unsubscribed, there are // no more notifications to that specific subscription. -func TestKVPaymentsDBUnsubscribeSuccess(t *testing.T) { +func TestKVStoreUnsubscribeSuccess(t *testing.T) { t.Parallel() db := initDB(t) - paymentDB, err := paymentsdb.NewKVPaymentsDB( + paymentDB, err := paymentsdb.NewKVStore( db, paymentsdb.WithKeepFailedPaymentAttempts(true), ) @@ -444,12 +444,12 @@ func TestKVPaymentsDBUnsubscribeSuccess(t *testing.T) { require.Len(t, subscription2.Updates(), 0) } -func testKVPaymentsDBSubscribeFail(t *testing.T, registerAttempt, +func testKVStoreSubscribeFail(t *testing.T, registerAttempt, keepFailedPaymentAttempts bool) { db := initDB(t) - paymentDB, err := paymentsdb.NewKVPaymentsDB( + paymentDB, err := paymentsdb.NewKVStore( db, paymentsdb.WithKeepFailedPaymentAttempts( keepFailedPaymentAttempts, From f87841d638fb53391469ab07860f30e24743fbb4 Mon Sep 17 00:00:00 2001 From: ziggie Date: Wed, 13 Aug 2025 15:05:11 +0200 Subject: [PATCH 12/16] paymentsdb: declare helper functions and add comments --- payments/db/kv_store_test.go | 10 +++++++++- payments/db/payment_test.go | 16 +++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/payments/db/kv_store_test.go b/payments/db/kv_store_test.go index b3d7887bd5f..caa46b5d0e7 100644 --- a/payments/db/kv_store_test.go +++ b/payments/db/kv_store_test.go @@ -20,6 +20,8 @@ import ( // TestKVStoreDeleteNonInFlight checks that calling DeletePayments only // deletes payments from the database that are not in-flight. +// +// TODO(ziggie): Make this test db agnostic. func TestKVStoreDeleteNonInFlight(t *testing.T) { t.Parallel() @@ -249,9 +251,11 @@ type htlcStatus struct { // fetchPaymentIndexEntry gets the payment hash for the sequence number provided // from our payment indexes bucket. -func fetchPaymentIndexEntry(_ *testing.T, p *KVStore, +func fetchPaymentIndexEntry(t *testing.T, p *KVStore, sequenceNumber uint64) (*lntypes.Hash, error) { + t.Helper() + var hash lntypes.Hash if err := kvdb.View(p.db, func(tx walletdb.ReadTx) error { @@ -319,6 +323,8 @@ func assertNoIndex(t *testing.T, p DB, seqNr uint64) { func makeFakeInfo(t *testing.T) (*PaymentCreationInfo, *HTLCAttemptInfo) { + t.Helper() + var preimg lntypes.Preimage copy(preimg[:], rev[:]) @@ -677,6 +683,8 @@ func putDuplicatePayment(t *testing.T, duplicateBucket kvdb.RwBucket, // TestQueryPayments tests retrieval of payments with forwards and reversed // queries. +// +// TODO(ziggie): Make this test db agnostic. func TestQueryPayments(t *testing.T) { // Define table driven test for QueryPayments. // Test payments have sequence indices [1, 3, 4, 5, 6, 7]. diff --git a/payments/db/payment_test.go b/payments/db/payment_test.go index ea5fcb8df4d..fcd82af4a16 100644 --- a/payments/db/payment_test.go +++ b/payments/db/payment_test.go @@ -112,6 +112,8 @@ type payment struct { // the payments slice. Each payment will receive one failed HTLC and another // HTLC depending on the final status of the payment provided. func createTestPayments(t *testing.T, p DB, payments []*payment) { + t.Helper() + attemptID := uint64(0) for i := 0; i < len(payments); i++ { @@ -193,7 +195,9 @@ func createTestPayments(t *testing.T, p DB, payments []*payment) { // assertRouteEquals compares to routes for equality and returns an error if // they are not equal. -func assertRouteEqual(a, b *route.Route) error { +func assertRouteEqual(t *testing.T, a, b *route.Route) error { + t.Helper() + if !reflect.DeepEqual(a, b) { return fmt.Errorf("HTLCAttemptInfos don't match: %v vs %v", spew.Sdump(a), spew.Sdump(b)) @@ -239,7 +243,7 @@ func assertPaymentInfo(t *testing.T, p DB, hash lntypes.Hash, } htlc := payment.HTLCs[a.AttemptID] - if err := assertRouteEqual(&htlc.Route, &a.Route); err != nil { + if err := assertRouteEqual(t, &htlc.Route, &a.Route); err != nil { t.Fatal("routes do not match") } @@ -331,7 +335,9 @@ func assertDBPayments(t *testing.T, paymentDB DB, payments []*payment) { } // genPreimage generates a random preimage. -func genPreimage() ([32]byte, error) { +func genPreimage(t *testing.T) ([32]byte, error) { + t.Helper() + var preimage [32]byte if _, err := io.ReadFull(rand.Reader, preimage[:]); err != nil { return preimage, err @@ -344,7 +350,7 @@ func genPreimage() ([32]byte, error) { func genInfo(t *testing.T) (*PaymentCreationInfo, *HTLCAttemptInfo, lntypes.Preimage, error) { - preimage, err := genPreimage() + preimage, err := genPreimage(t) if err != nil { return nil, nil, preimage, fmt.Errorf("unable to "+ "generate preimage: %v", err) @@ -1466,7 +1472,7 @@ func TestSwitchFail(t *testing.T) { len(payment.HTLCs)) } - err = assertRouteEqual(&payment.HTLCs[0].Route, &attempt.Route) + err = assertRouteEqual(t, &payment.HTLCs[0].Route, &attempt.Route) if err != nil { t.Fatalf("unexpected route returned: %v vs %v: %v", spew.Sdump(attempt.Route), From 9ac93e75acfa59b9b89c1e9cf8bcd8238d7e28bd Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 19 Aug 2025 17:05:47 +0200 Subject: [PATCH 13/16] paymentsdb: fix linter --- payments/db/interface.go | 6 +++--- payments/db/payment_test.go | 5 ++++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/payments/db/interface.go b/payments/db/interface.go index d3a7e7671fb..9b91de9bb81 100644 --- a/payments/db/interface.go +++ b/payments/db/interface.go @@ -44,10 +44,10 @@ type PaymentWriter interface { // its database operations. This interface represents the control flow of how // a payment should be handled in the database. They are not just writing // operations but they inherently represent the flow of a payment. The methods -// are called in the following order: +// are called in the following order. // -// 1. InitPayment -// 2. RegisterAttempt (a payment can have multiple attempts) +// 1. InitPayment. +// 2. RegisterAttempt (a payment can have multiple attempts). // 3. SettleAttempt or FailAttempt (attempts can also fail as long as the // sending amount will be eventually settled). // 4. Payment succeeds or "Fail" is called. diff --git a/payments/db/payment_test.go b/payments/db/payment_test.go index fcd82af4a16..c07d0e9cf23 100644 --- a/payments/db/payment_test.go +++ b/payments/db/payment_test.go @@ -1387,7 +1387,10 @@ func TestSwitchFail(t *testing.T) { require.NoError(t, err, "unable to fail payment hash") // Verify the status is indeed Failed. - assertDBPaymentstatus(t, paymentDB, info.PaymentIdentifier, StatusFailed) + assertDBPaymentstatus( + t, paymentDB, info.PaymentIdentifier, StatusFailed, + ) + assertPaymentInfo( t, paymentDB, info.PaymentIdentifier, info, &failReason, nil, ) From b16782caeb046b169eab69be54bed3b6428d77ff Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 19 Aug 2025 17:53:56 +0200 Subject: [PATCH 14/16] multi: move DBMPPayment to paymentsdb package --- payments/db/interface.go | 32 ++++++++++++++++++++++++++++++++ routing/control_tower.go | 36 ++---------------------------------- routing/mock_test.go | 6 +++--- routing/payment_lifecycle.go | 8 +++++--- 4 files changed, 42 insertions(+), 40 deletions(-) diff --git a/payments/db/interface.go b/payments/db/interface.go index 9b91de9bb81..c41dc371f89 100644 --- a/payments/db/interface.go +++ b/payments/db/interface.go @@ -89,3 +89,35 @@ type PaymentControl interface { // completed, and the payment has reached a final terminal state. DeleteFailedAttempts(lntypes.Hash) error } + +// DBMPPayment is an interface that represents the payment state during a +// payment lifecycle. +type DBMPPayment interface { + // GetState returns the current state of the payment. + GetState() *MPPaymentState + + // Terminated returns true if the payment is in a final state. + Terminated() bool + + // GetStatus returns the current status of the payment. + GetStatus() PaymentStatus + + // NeedWaitAttempts specifies whether the payment needs to wait for the + // outcome of an attempt. + NeedWaitAttempts() (bool, error) + + // GetHTLCs returns all HTLCs of this payment. + GetHTLCs() []HTLCAttempt + + // InFlightHTLCs returns all HTLCs that are in flight. + InFlightHTLCs() []HTLCAttempt + + // AllowMoreAttempts is used to decide whether we can safely attempt + // more HTLCs for a given payment state. Return an error if the payment + // is in an unexpected state. + AllowMoreAttempts() (bool, error) + + // TerminalInfo returns the settled HTLC attempt or the payment's + // failure reason. + TerminalInfo() (*HTLCAttempt, *FailureReason) +} diff --git a/routing/control_tower.go b/routing/control_tower.go index fcbfbe80ed9..c5d91e85603 100644 --- a/routing/control_tower.go +++ b/routing/control_tower.go @@ -9,38 +9,6 @@ import ( "github.com/lightningnetwork/lnd/queue" ) -// DBMPPayment is an interface derived from channeldb.MPPayment that is used by -// the payment lifecycle. -type DBMPPayment interface { - // GetState returns the current state of the payment. - GetState() *paymentsdb.MPPaymentState - - // Terminated returns true if the payment is in a final state. - Terminated() bool - - // GetStatus returns the current status of the payment. - GetStatus() paymentsdb.PaymentStatus - - // NeedWaitAttempts specifies whether the payment needs to wait for the - // outcome of an attempt. - NeedWaitAttempts() (bool, error) - - // GetHTLCs returns all HTLCs of this payment. - GetHTLCs() []paymentsdb.HTLCAttempt - - // InFlightHTLCs returns all HTLCs that are in flight. - InFlightHTLCs() []paymentsdb.HTLCAttempt - - // AllowMoreAttempts is used to decide whether we can safely attempt - // more HTLCs for a given payment state. Return an error if the payment - // is in an unexpected state. - AllowMoreAttempts() (bool, error) - - // TerminalInfo returns the settled HTLC attempt or the payment's - // failure reason. - TerminalInfo() (*paymentsdb.HTLCAttempt, *paymentsdb.FailureReason) -} - // ControlTower tracks all outgoing payments made, whose primary purpose is to // prevent duplicate payments to the same payment hash. In production, a // persistent implementation is preferred so that tracking can survive across @@ -76,7 +44,7 @@ type ControlTower interface { // FetchPayment fetches the payment corresponding to the given payment // hash. - FetchPayment(paymentHash lntypes.Hash) (DBMPPayment, error) + FetchPayment(paymentHash lntypes.Hash) (paymentsdb.DBMPPayment, error) // FailPayment transitions a payment into the Failed state, and records // the ultimate reason the payment failed. Note that this should only @@ -273,7 +241,7 @@ func (p *controlTower) FailAttempt(paymentHash lntypes.Hash, // FetchPayment fetches the payment corresponding to the given payment hash. func (p *controlTower) FetchPayment(paymentHash lntypes.Hash) ( - DBMPPayment, error) { + paymentsdb.DBMPPayment, error) { return p.db.FetchPayment(paymentHash) } diff --git a/routing/mock_test.go b/routing/mock_test.go index 9cb938f74f2..19a76ee9010 100644 --- a/routing/mock_test.go +++ b/routing/mock_test.go @@ -510,7 +510,7 @@ func (m *mockControlTowerOld) FailPayment(phash lntypes.Hash, } func (m *mockControlTowerOld) FetchPayment(phash lntypes.Hash) ( - DBMPPayment, error) { + paymentsdb.DBMPPayment, error) { m.Lock() defer m.Unlock() @@ -787,7 +787,7 @@ func (m *mockControlTower) FailPayment(phash lntypes.Hash, } func (m *mockControlTower) FetchPayment(phash lntypes.Hash) ( - DBMPPayment, error) { + paymentsdb.DBMPPayment, error) { args := m.Called(phash) @@ -825,7 +825,7 @@ type mockMPPayment struct { mock.Mock } -var _ DBMPPayment = (*mockMPPayment)(nil) +var _ paymentsdb.DBMPPayment = (*mockMPPayment)(nil) func (m *mockMPPayment) GetState() *paymentsdb.MPPaymentState { args := m.Called() diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 245e3b3c151..cc92a8be1ba 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -130,7 +130,7 @@ const ( // wait for results, the method will exit with `stepExit` such that the payment // lifecycle loop will terminate. func (p *paymentLifecycle) decideNextStep( - payment DBMPPayment) (stateStep, error) { + payment paymentsdb.DBMPPayment) (stateStep, error) { // Check whether we could make new HTLC attempts. allow, err := payment.AllowMoreAttempts() @@ -1110,7 +1110,9 @@ func (p *paymentLifecycle) patchLegacyPaymentHash( // reloadInflightAttempts is called when the payment lifecycle is resumed after // a restart. It reloads all inflight attempts from the control tower and // collects the results of the attempts that have been sent before. -func (p *paymentLifecycle) reloadInflightAttempts() (DBMPPayment, error) { +func (p *paymentLifecycle) reloadInflightAttempts() (paymentsdb.DBMPPayment, + error) { + payment, err := p.router.cfg.Control.FetchPayment(p.identifier) if err != nil { return nil, err @@ -1133,7 +1135,7 @@ func (p *paymentLifecycle) reloadInflightAttempts() (DBMPPayment, error) { } // reloadPayment returns the latest payment found in the db (control tower). -func (p *paymentLifecycle) reloadPayment() (DBMPPayment, +func (p *paymentLifecycle) reloadPayment() (paymentsdb.DBMPPayment, *paymentsdb.MPPaymentState, error) { // Read the db to get the latest state of the payment. From 039b5994f341fd64fac77e28f2ea2eec2286e58c Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 19 Aug 2025 18:17:27 +0200 Subject: [PATCH 15/16] routing: add more comments to the ControlTower interface --- routing/control_tower.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/routing/control_tower.go b/routing/control_tower.go index c5d91e85603..2b9e7dd9d28 100644 --- a/routing/control_tower.go +++ b/routing/control_tower.go @@ -17,6 +17,8 @@ import ( type ControlTower interface { // InitPayment initializes a new payment with the given payment hash and // also notifies subscribers of the payment creation. + // + // NOTE: Subscribers should be notified by the new state of the payment. InitPayment(lntypes.Hash, *paymentsdb.PaymentCreationInfo) error // DeleteFailedAttempts removes all failed HTLCs from the db. It should @@ -25,6 +27,8 @@ type ControlTower interface { DeleteFailedAttempts(lntypes.Hash) error // RegisterAttempt atomically records the provided HTLCAttemptInfo. + // + // NOTE: Subscribers should be notified by the new state of the payment. RegisterAttempt(lntypes.Hash, *paymentsdb.HTLCAttemptInfo) error // SettleAttempt marks the given attempt settled with the preimage. If @@ -35,10 +39,14 @@ type ControlTower interface { // error to prevent us from making duplicate payments to the same // payment hash. The provided preimage is atomically saved to the DB // for record keeping. + // + // NOTE: Subscribers should be notified by the new state of the payment. SettleAttempt(lntypes.Hash, uint64, *paymentsdb.HTLCSettleInfo) ( *paymentsdb.HTLCAttempt, error) // FailAttempt marks the given payment attempt failed. + // + // NOTE: Subscribers should be notified by the new state of the payment. FailAttempt(lntypes.Hash, uint64, *paymentsdb.HTLCFailInfo) ( *paymentsdb.HTLCAttempt, error) @@ -52,6 +60,8 @@ type ControlTower interface { // invoking this method, InitPayment should return nil on its next call // for this payment hash, allowing the user to make a subsequent // payment. + // + // NOTE: Subscribers should be notified by the new state of the payment. FailPayment(lntypes.Hash, paymentsdb.FailureReason) error // FetchInFlightPayments returns all payments with status InFlight. From bf6131ba02baa4d59f143b5a6e1b4969066cc982 Mon Sep 17 00:00:00 2001 From: ziggie Date: Wed, 20 Aug 2025 09:06:21 +0200 Subject: [PATCH 16/16] routing: Add comment to DeleteFailedAttempts func call --- routing/payment_lifecycle.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index cc92a8be1ba..7c9c0418a1e 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -326,7 +326,9 @@ lifecycle: // terminal condition. We either return the settled preimage or the // payment's failure reason. // - // Optionally delete the failed attempts from the database. + // Optionally delete the failed attempts from the database. Depends on + // the database options deleting attempts is not allowed so this will + // just be a no-op. err = p.router.cfg.Control.DeleteFailedAttempts(p.identifier) if err != nil { log.Errorf("Error deleting failed htlc attempts for payment "+