From 3d4ee8b874bd20f97914469fb7b0b6e3aced83dd Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 25 Aug 2025 16:26:02 +0200 Subject: [PATCH 1/7] paymentsdb: rename defaultKVStoreOptions func We rename the modifier to use the same naming as used in the graphDB to align the codebase. --- payments/db/kv_store.go | 9 +-------- payments/db/options.go | 8 ++++++++ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/payments/db/kv_store.go b/payments/db/kv_store.go index bde4b2e92..67b4a920b 100644 --- a/payments/db/kv_store.go +++ b/payments/db/kv_store.go @@ -133,18 +133,11 @@ type KVStore struct { keepFailedPaymentAttempts bool } -// defaultKVStoreOptions returns the default options for the KV store. -func defaultKVStoreOptions() *StoreOptions { - return &StoreOptions{ - KeepFailedPaymentAttempts: false, - } -} - // NewKVStore creates a new KVStore for payments. func NewKVStore(db kvdb.Backend, options ...OptionModifier) (*KVStore, error) { - opts := defaultKVStoreOptions() + opts := DefaultOptions() for _, applyOption := range options { applyOption(opts) } diff --git a/payments/db/options.go b/payments/db/options.go index 4f6fa573e..9e98aafa3 100644 --- a/payments/db/options.go +++ b/payments/db/options.go @@ -10,6 +10,14 @@ type StoreOptions struct { KeepFailedPaymentAttempts bool } +// DefaultOptions returns a StoreOptions populated with default values. +func DefaultOptions() *StoreOptions { + return &StoreOptions{ + KeepFailedPaymentAttempts: false, + NoMigration: false, + } +} + // OptionModifier is a function signature for modifying the default // StoreOptions. type OptionModifier func(*StoreOptions) From 0320dfcc89888f40a88e29844e46afe4a33470f3 Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 25 Aug 2025 16:27:57 +0200 Subject: [PATCH 2/7] paymentsdb: add compile time check for the kv db implementation --- payments/db/kv_store.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/payments/db/kv_store.go b/payments/db/kv_store.go index 67b4a920b..5eb94dad2 100644 --- a/payments/db/kv_store.go +++ b/payments/db/kv_store.go @@ -133,6 +133,9 @@ type KVStore struct { keepFailedPaymentAttempts bool } +// A compile-time constraint to ensure KVStore implements DB. +var _ DB = (*KVStore)(nil) + // NewKVStore creates a new KVStore for payments. func NewKVStore(db kvdb.Backend, options ...OptionModifier) (*KVStore, error) { From e20a34de46c80f35b45f86b5c280a4ae9a905f4d Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 25 Aug 2025 16:29:00 +0200 Subject: [PATCH 3/7] paymentsdb: refactor attempt verification --- payments/db/kv_store.go | 85 ++----------------------------------- payments/db/payment.go | 92 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 82 deletions(-) diff --git a/payments/db/kv_store.go b/payments/db/kv_store.go index 5eb94dad2..e5915a003 100644 --- a/payments/db/kv_store.go +++ b/payments/db/kv_store.go @@ -389,88 +389,9 @@ func (p *KVStore) RegisterAttempt(paymentHash lntypes.Hash, return err } - // If the final hop has encrypted data, then we know this is a - // blinded payment. In blinded payments, MPP records are not set - // for split payments and the recipient is responsible for using - // a consistent PathID across the various encrypted data - // payloads that we received from them for this payment. All we - // need to check is that the total amount field for each HTLC - // in the split payment is correct. - isBlinded := len(attempt.Route.FinalHop().EncryptedData) != 0 - - // Make sure any existing shards match the new one with regards - // to MPP options. - mpp := attempt.Route.FinalHop().MPP - - // MPP records should not be set for attempts to blinded paths. - if isBlinded && mpp != nil { - return ErrMPPRecordInBlindedPayment - } - - for _, h := range payment.InFlightHTLCs() { - hMpp := h.Route.FinalHop().MPP - - // If this is a blinded payment, then no existing HTLCs - // should have MPP records. - if isBlinded && hMpp != nil { - return ErrMPPRecordInBlindedPayment - } - - // If this is a blinded payment, then we just need to - // check that the TotalAmtMsat field for this shard - // is equal to that of any other shard in the same - // payment. - if isBlinded { - if attempt.Route.FinalHop().TotalAmtMsat != - h.Route.FinalHop().TotalAmtMsat { - - //nolint:ll - return ErrBlindedPaymentTotalAmountMismatch - } - - continue - } - - switch { - // We tried to register a non-MPP attempt for a MPP - // payment. - case mpp == nil && hMpp != nil: - return ErrMPPayment - - // We tried to register a MPP shard for a non-MPP - // payment. - case mpp != nil && hMpp == nil: - return ErrNonMPPayment - - // Non-MPP payment, nothing more to validate. - case mpp == nil: - continue - } - - // Check that MPP options match. - if mpp.PaymentAddr() != hMpp.PaymentAddr() { - return ErrMPPPaymentAddrMismatch - } - - if mpp.TotalMsat() != hMpp.TotalMsat() { - return ErrMPPTotalAmountMismatch - } - } - - // If this is a non-MPP attempt, it must match the total amount - // exactly. Note that a blinded payment is considered an MPP - // attempt. - amt := attempt.Route.ReceiverAmt() - if !isBlinded && mpp == nil && amt != payment.Info.Value { - return ErrValueMismatch - } - - // Ensure we aren't sending more than the total payment amount. - sentAmt, _ := payment.SentAmt() - if sentAmt+amt > payment.Info.Value { - return fmt.Errorf("%w: attempted=%v, payment amount="+ - "%v", ErrValueExceedsAmt, - sentAmt+amt, payment.Info.Value) + // Verify the attempt is compatible with the existing payment. + if err := verifyAttempt(payment, attempt); err != nil { + return err } htlcsBucket, err := bucket.CreateBucketIfNotExists( diff --git a/payments/db/payment.go b/payments/db/payment.go index 2c5931a4d..b863cea5e 100644 --- a/payments/db/payment.go +++ b/payments/db/payment.go @@ -712,3 +712,95 @@ func generateSphinxPacket(rt *route.Route, paymentHash []byte, PaymentPath: sphinxPath.NodeKeys(), }, nil } + +// verifyAttempt validates that a new HTLC attempt is compatible with the +// existing payment and its in-flight HTLCs. This function checks: +// 1. MPP (Multi-Path Payment) compatibility between attempts +// 2. Blinded payment consistency +// 3. Amount validation +// 4. Total payment amount limits +func verifyAttempt(payment *MPPayment, attempt *HTLCAttemptInfo) error { + // If the final hop has encrypted data, then we know this is a + // blinded payment. In blinded payments, MPP records are not set + // for split payments and the recipient is responsible for using + // a consistent PathID across the various encrypted data + // payloads that we received from them for this payment. All we + // need to check is that the total amount field for each HTLC + // in the split payment is correct. + isBlinded := len(attempt.Route.FinalHop().EncryptedData) != 0 + + // Make sure any existing shards match the new one with regards + // to MPP options. + mpp := attempt.Route.FinalHop().MPP + + // MPP records should not be set for attempts to blinded paths. + if isBlinded && mpp != nil { + return ErrMPPRecordInBlindedPayment + } + + for _, h := range payment.InFlightHTLCs() { + hMpp := h.Route.FinalHop().MPP + + // If this is a blinded payment, then no existing HTLCs + // should have MPP records. + if isBlinded && hMpp != nil { + return ErrMPPRecordInBlindedPayment + } + + // If this is a blinded payment, then we just need to + // check that the TotalAmtMsat field for this shard + // is equal to that of any other shard in the same + // payment. + if isBlinded { + if attempt.Route.FinalHop().TotalAmtMsat != + h.Route.FinalHop().TotalAmtMsat { + + return ErrBlindedPaymentTotalAmountMismatch + } + + continue + } + + switch { + // We tried to register a non-MPP attempt for a MPP + // payment. + case mpp == nil && hMpp != nil: + return ErrMPPayment + + // We tried to register a MPP shard for a non-MPP + // payment. + case mpp != nil && hMpp == nil: + return ErrNonMPPayment + + // Non-MPP payment, nothing more to validate. + case mpp == nil: + continue + } + + // Check that MPP options match. + if mpp.PaymentAddr() != hMpp.PaymentAddr() { + return ErrMPPPaymentAddrMismatch + } + + if mpp.TotalMsat() != hMpp.TotalMsat() { + return ErrMPPTotalAmountMismatch + } + } + + // If this is a non-MPP attempt, it must match the total amount + // exactly. Note that a blinded payment is considered an MPP + // attempt. + amt := attempt.Route.ReceiverAmt() + if !isBlinded && mpp == nil && amt != payment.Info.Value { + return ErrValueMismatch + } + + // Ensure we aren't sending more than the total payment amount. + sentAmt, _ := payment.SentAmt() + if sentAmt+amt > payment.Info.Value { + return fmt.Errorf("%w: attempted=%v, payment amount=%v", + ErrValueExceedsAmt, sentAmt+amt, payment.Info.Value) + } + + return nil +} From 2d032044f9aa7250a3b69678a4ddbfe7048688ef Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 25 Aug 2025 16:44:31 +0200 Subject: [PATCH 4/7] sqldb: Make sure we do not replace keywords if they have a prefix We don't for example want to replace CURRENT_TIMESTAMP when using it in our tables. --- sqldb/postgres.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sqldb/postgres.go b/sqldb/postgres.go index 8cfd3e25b..70dba82a1 100644 --- a/sqldb/postgres.go +++ b/sqldb/postgres.go @@ -30,7 +30,10 @@ var ( postgresSchemaReplacements = map[string]string{ "BLOB": "BYTEA", "INTEGER PRIMARY KEY": "BIGSERIAL PRIMARY KEY", - "TIMESTAMP": "TIMESTAMP WITHOUT TIME ZONE", + // We need this space in front of the TIMESTAMP keyword to + // avoid replacing words which just have the word "TIMESTAMP" in + // them. + " TIMESTAMP": " TIMESTAMP WITHOUT TIME ZONE", } // Make sure PostgresStore implements the MigrationExecutor interface. From a9c30dacb5c86423529cec215c777f2c1d7cff48 Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 25 Aug 2025 16:47:41 +0200 Subject: [PATCH 5/7] sqldb: add helper for type boolean --- sqldb/sqlutils.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/sqldb/sqlutils.go b/sqldb/sqlutils.go index ce99eb682..56ff953d5 100644 --- a/sqldb/sqlutils.go +++ b/sqldb/sqlutils.go @@ -75,6 +75,15 @@ func SQLStrValid(s string) sql.NullString { } } +// SQLBool turns a boolean into the NullBool that sql/sqlc uses when a boolean +// can be permitted to be NULL. +func SQLBool(b bool) sql.NullBool { + return sql.NullBool{ + Bool: b, + Valid: true, + } +} + // SQLTime turns a time.Time into the NullTime that sql/sqlc uses when a time // can be permitted to be NULL. func SQLTime(t time.Time) sql.NullTime { From 335a9b3f40dc016dbf46807f66dcf5e4fb0b3a23 Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 25 Aug 2025 16:48:26 +0200 Subject: [PATCH 6/7] lnd: reduce max limit when querying for payments --- payments/db/query.go | 6 ++++++ rpcserver.go | 11 ++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/payments/db/query.go b/payments/db/query.go index 5904ab56a..40dfd4321 100644 --- a/payments/db/query.go +++ b/payments/db/query.go @@ -1,5 +1,11 @@ package paymentsdb +const ( + // DefaultMaxPayments is the default maximum number of payments returned + // in the payments query pagination. + DefaultMaxPayments = 100 +) + // Query represents a query to the payments database starting or ending // at a certain offset index. The number of retrieved records can be limited. type Query struct { diff --git a/rpcserver.go b/rpcserver.go index 0c19c876a..6ab323fa3 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -7523,10 +7523,15 @@ func (r *rpcServer) ListPayments(ctx context.Context, CreationDateEnd: int64(req.CreationDateEnd), } - // If the maximum number of payments wasn't specified, then we'll - // default to return the maximal number of payments representable. + // If the maximum number of payments wasn't specified, we default to + // a reasonable number to prevent resource exhaustion. All of the + // payments are fetched into memory. Moreover we don't want our daemon + // to remain stable and do other stuff rather than serving payments. + // + // TODO(ziggie): Choose a more specific default value when results of + // performance testing are available. if req.MaxPayments == 0 { - query.MaxPayments = math.MaxUint64 + query.MaxPayments = paymentsdb.DefaultMaxPayments } paymentsQuerySlice, err := r.server.paymentsDB.QueryPayments( From f540e9c8ec63a1d4873e37b3616fe921dc6a9112 Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 26 Aug 2025 12:24:28 +0200 Subject: [PATCH 7/7] paymentsdb: add helper method to set session key This is only used for testing because we have now a unique contraint on the session key and need to swap it in tests for the sql backend. pick f17baec94 paymentsdb: fix linter --- payments/db/payment.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/payments/db/payment.go b/payments/db/payment.go index b863cea5e..f6d998344 100644 --- a/payments/db/payment.go +++ b/payments/db/payment.go @@ -175,6 +175,20 @@ func (h *HTLCAttemptInfo) SessionKey() *btcec.PrivateKey { return h.cachedSessionKey } +// setSessionKey sets the session key for the htlc attempt. +// +// NOTE: Only used for testing. +// +//nolint:unused +func (h *HTLCAttemptInfo) setSessionKey(sessionKey *btcec.PrivateKey) { + h.cachedSessionKey = sessionKey + + // Also set the session key as a raw bytes. + var scratch [btcec.PrivKeyBytesLen]byte + copy(scratch[:], sessionKey.Serialize()) + h.sessionKey = scratch +} + // OnionBlob returns the onion blob created from the sphinx construction. func (h *HTLCAttemptInfo) OnionBlob() ([lnwire.OnionPacketSize]byte, error) { var zeroBytes [lnwire.OnionPacketSize]byte