What the developer wrote
Authored by yyforyongyu
paymentsdb: add AMP hash regression test
SQL writes store the payment identifier in payment_hash for each
attempt. That is wrong for AMP payments, because the payment
identifier is the SetID while each shard carries its own HTLC hash.
Add TestRegisterAttemptPreservesAttemptHash as evidence. It passes on
KV and fails on SQL before the fix because SQL reads the attempt hash
back as the payment identifier.
What changed, and why it matters
This commit only adds a new regression test for the Lightning Network Daemon (LND) payments database. The test checks that for AMP (Atomic Multi-Path) payments, each individual payment attempt keeps its own unique hash rather than being overwritten by the overall payment identifier. The commit message says the test fails on the SQL database backend before a separate fix, but the actual code fix is not included in this commit. So by itself, this change does not fix anything; it only provides a test that demonstrates the bug.
Treat this commit as test-only. Locate the companion commit that changes the SQL persistence logic so the attempt hash is stored correctly, and review that fix for security impact. Do not rely on this commit alone to resolve any issue.
Security signals we found
Data-integrity bug in payment attempt hash storage for AMP payments
Regression test added, not the actual fix
Potential mismatch between stored HTLC hash and actual attempt hash in SQL backend
Evidence from the diff
The diff adds TestRegisterAttemptPreservesAttemptHash in payments/db/payment_test.go. It initializes an AMP payment using a SetID as the payment identifier, registers an attempt with a distinct attemptHash, and asserts that FetchPayment returns the HTLC with Hash equal to attemptHash and not equal to the payment identifier. The commit message states SQL writes were storing the payment identifier in payment_hash for each attempt, which is incorrect for AMP because the SetID differs from per-shard HTLC hashes. The test is described as passing on the KV backend and failing on SQL before the fix, implying the fix exists elsewhere or in an earlier/later commit, but no fix is present here.
Changed components
payments/db/payment_test.goLND payments database (SQL backend, per commit message)AMP (Atomic Multi-Path) payment handlingInspect captured patch +35 / −0
diff --git a/payments/db/payment_test.go b/payments/db/payment_test.go
index 55a5c09..2c547a4 100644
--- a/payments/db/payment_test.go
+++ b/payments/db/payment_test.go
@@ -3118,6 +3118,41 @@ func TestRegisterAttemptWithAMP(t *testing.T) {
require.Equal(t, childIndex, finalHop.AMP.ChildIndex())
}
+// TestRegisterAttemptPreservesAttemptHash tests that an attempt's own hash is
+// preserved independently from the payment identifier. This is especially
+// important for AMP payments where the payment identifier is the SetID and the
+// individual HTLC attempts each use their own payment hash.
+func TestRegisterAttemptPreservesAttemptHash(t *testing.T) {
+ t.Parallel()
+
+ ctx := t.Context()
+
+ paymentDB, _ := NewTestDB(t)
+
+ setID := lntypes.Hash{1, 2, 3, 4}
+ attemptHash := lntypes.Hash{5, 6, 7, 8}
+ info := genPaymentCreationInfo(t, setID)
+
+ err := paymentDB.InitPayment(ctx, info.PaymentIdentifier, info)
+ require.NoError(t, err)
+
+ attempt := genAttemptWithHash(t, 0, genSessionKey(t), attemptHash)
+ finalHopIdx := len(attempt.Route.Hops) - 1
+ attempt.Route.Hops[finalHopIdx].AMP = record.NewAMP(
+ [32]byte{9, 10, 11, 12}, setID, 99,
+ )
+
+ _, err = paymentDB.RegisterAttempt(ctx, info.PaymentIdentifier, attempt)
+ require.NoError(t, err)
+
+ payment, err := paymentDB.FetchPayment(ctx, info.PaymentIdentifier)
+ require.NoError(t, err)
+ require.Len(t, payment.HTLCs, 1)
+ require.NotNil(t, payment.HTLCs[0].Hash)
+ require.Equal(t, attemptHash, *payment.HTLCs[0].Hash)
+ require.NotEqual(t, info.PaymentIdentifier, *payment.HTLCs[0].Hash)
+}
+
// TestRegisterAttemptWithBlindedRoute tests that blinded route data
// (EncryptedData, BlindingPoint, TotalAmtMsat) is correctly stored and
// retrieved.
Why this scored 41/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.