Switch collect_logs from docker cp to docker logs to reliably capture
container output. The previous approach used docker cp to copy lnd log
files from inside named volumes, which silently fails in CI — the
directory gets created and the success echo prints, but no files are
ever copied, causing upload-artifact to report "No files were found".
docker logs reads directly from Docker's captured stdout/stderr buffer,
bypassing the volume entirely, and works as long as the container exists.
Under parallel Postgres test load, the event loop must complete
sequential DB writes after SetTime fires. Using testTimeout (5s)
is too tight; switch the three blocking hodlChan selects in
testFailPartialAMPPayment to testTimeoutLong (1 minute) to
match the pattern already used elsewhere for slow backends.
The backwards compatibility test was failing intermittently due to two
related timing issues in the test setup.
The issue was that Dave's `wait_graph_sync dave 3` was hanging
for up to 60 minutes. Dave's initial gossip sync with Charlie could
complete before Charlie had forwarded the alice-bob channel
announcement, leaving Dave stuck at 2 channels until lnd's historical
syncer fired at its default interval of 1 hour. After this 1-hour idle,
some routing state had become stale, causing the subsequent payment from
alice to dave to fail with FAILURE_REASON_NO_ROUTE.
This issues is now addressed by setting `--historicalsyncinterval=10s` on
all nodes. This causes nodes to periodically re-sync the full gossip
state from their peers every 10 seconds instead of every hour. Dave
therefore picks up any missed channel announcements and routing policies
within seconds, and alice's routing graph stays up-to-date throughout
the test.
Additionally, lnd debug logs from all containers are now collected
before teardown on failure and uploaded as a CI artifact, making future
failures easier to diagnose.
The GetInvoice query used an OR IS NULL pattern for each filter
parameter:
WHERE (i.hash = $1 OR $1 IS NULL)
AND (i.payment_addr = $2 OR $2 IS NULL)
SQLite's query planner decides on an execution plan at prepare time,
before seeing any parameter values. Because either condition can be
trivially true when its parameter is NULL, the planner conservatively
falls back to a full table scan rather than using the unique indexes on
hash and payment_addr. This caused every invoice lookup and update on
the hot path (HTLC settlement) to scan the entire invoices table.
Replace the single catch-all query with three dedicated queries, each
using a direct equality on a uniquely constrained column:
- GetInvoiceByHash: WHERE hash = $1
- GetInvoiceByAddr: WHERE payment_addr = $1 (new, covers AMP path)
- GetInvoiceBySetID: existing, unchanged
Update getInvoiceByRef to route to the appropriate query based on which
fields are present in the InvoiceRef. When both hash and payment address
are provided, we look up by hash and then verify the returned invoice's
payment address matches.
GetInvoice is now unused and removed from the SQLInvoiceQueries
interface.
Remove four indexes from the invoices table that either duplicate existing
UNIQUE constraint indexes or are never used in WHERE clauses:
- invoices_hash_idx: redundant, UNIQUE constraint on hash already creates
an implicit index
- invoices_payment_addr_idx: redundant, UNIQUE constraint on payment_addr
already creates an implicit index
- invoices_preimage_idx: unused, preimage is NULL on all new invoices and
is never used as a query filter
- invoices_settled_at_idx: unused, settled_at is NULL on all pending
invoices and is never used as a query filter (settle_index is used
instead)
Dropping these reduces B-tree working set size, which improves page cache
utilization as the invoice table grows.
The results suggests that the default of 2 is a good conservative
approach. The higher the connection number and the higher the
workload the performance decreases in WAL mode since readers still
need coordination (WAL index shm for example).
Also freeze the lnwire and record packages used by the migration.
Copy the minimal subset of lnwire files (16) into
payments/db/migration1/lnwire/ and all record files (6) into
payments/db/migration1/record/. Three lnwire files are trimmed to avoid
pulling in the full message-type dispatch tree — all changes are purely
subtractive and can be verified with:
diff lnwire/message.go payments/db/migration1/lnwire/message.go
diff lnwire/writer.go payments/db/migration1/lnwire/writer.go
diff lnwire/lnwire.go payments/db/migration1/lnwire/lnwire.go
All migration1 files now import only the frozen packages, removing the
live dependency on lnwire and record so future changes to those packages
cannot affect migration correctness.
LegacyPayload was a hint used exclusively by the KV store to decide
how to serialize and deserialize the hop payload (legacy format vs
TLV). The SQL store does not serialize hop data at all — every hop
field is persisted natively in its own column — so this flag has no
meaning there and is never stored.
Clear LegacyPayload for all hops inside normalizePaymentForCompare so
that deep-equality checks between KV and SQL payments succeed even when
the KV source data carries LegacyPayload=true.
A dedicated test (TestMigrationLegacyPayloadNormalized) is added to
verify that a payment with LegacyPayload=true hops migrates and
compares correctly.
The defer closure checked a local err variable for commit/rollback
decisions, but err remained nil after a successful BeginTx. When
txBody failed, the error was returned directly without assigning to
err, so the defer always committed instead of rolling back.
Additionally, since err was not a named return value, the defer's
Commit error assignment was silently swallowed.
Replace the error-prone defer pattern with explicit rollback on
txBody failure and a direct Commit return.
When duplicatePaymentSequenceKey is missing from a duplicate payment
sub-bucket, the code returned the outer function's err variable which
is nil at that point. This caused corrupted duplicate entries to be
silently treated as "not found" instead of failing loudly.
Return a new dedicated ErrNoDuplicateSequenceNumber error so malformed
data is detected immediately.
For legacy payments, the HTLC Hash field may be nil in the bbolt
backend. Previously, the migration would fail with "HTLC attempt X
missing payment hash" when encountering such payments.
This commit fixes the migration by falling back to the parent payment
hash when the HTLC-specific hash is nil. This is consistent with how
the router handles legacy payments (see patchLegacyPaymentHash in
payment_lifecycle.go).
The validation logic is also updated to apply the same fallback when
comparing bbolt data with migrated SQL data, ensuring the comparison
succeeds.
Hook the payments KV→SQL migration into the SQL migration config.
The migration is still only available when building with the build tag
"test_native_sql".
Moreover a tombstone protection similar to the invoice migration is added
to prevent re-runningi with the KV backend once migration completes.
Add a developer-facing migration_external_test that allows
running the KV→SQL payments migration against a real channel.db
backend to debug migration failures on actual data. The accompanying
testdata README documents how to supply a database file and configure
the test, so users can validate their data and confirm the migration
completes successfully.
The test is skipped by default and meant for manual diagnostics.
Add test helpers plus sql_migration_test coverage for KV→SQL migration.
Basic migration, sequence ordering, data integrity, and feature-specific cases
(MPP/AMP, custom records, blinded routes, metadata, failure messages). Also
cover duplicate payment migration to payment_duplicates, including missing
attempt info to ensure terminal failure is recorded.
This gives broad regression coverage for the migration path and its edge-cases.
Implement the KV→SQL payment migration and add an in-migration
validation pass that deep-compares KV and SQL payment data in batches.
Duplicate payments are migrated into the payment_duplicates table,
and duplicates without attempt info or explicit resolution are marked
failed to ensure terminal state. Validation checks those rows as well.
Copy the core payments/db code into payments/db/migration1 and
add the required sqlc-generated types/queries from sqldb/sqlc.
This effectively freezes the migration code so it stays robust
against future query or schema changes in the main payments package.
Replace the delegation to channeldb.ReadElement/WriteElement with
self-contained, frozen implementations that only handle the exact types
required by this migration package. This removes the dependency on the
live channeldb codec so that future changes to channeldb serialization
cannot silently corrupt or break the migration.
UnknownElementType is also defined locally for the same reason.
Older LND versions could create multiple payments for the same hash.
We need to preserve those historical records during KV→SQL migration,
but they don’t fit the normal payment schema because we enforce a
unique payment hash constraint. Introduce a lean payment_duplicates
table to store only the essential fields (identifier, amount,
timestamps, settle/fail data).
This keeps the primary payment records stable and makes the migration
deterministic even when duplicate records lack attempt info. The table
is intentionally minimal and can be dropped after migration if no
duplicate payments exist.
For now there is no logic in place which allows the noderunner to
fetch duplicate payments after the migration.
- Remove duplicate compile-time interface assertion for SQLStore.
- Fix the down migration to drop payment_intents before payments to
respect the foreign key dependency order. This was not a bug in the
first place bc we have the CASCADE when deleting payments.
The SQL implementation collects payments into a map before converting
to a slice, resulting in non-deterministic iteration order due to Go's
intentional map randomisation. Sort the result by SequenceNum to produce
a deterministic, insertion-ordered output.
Note that the current sole caller (resumePayments in router.go) processes
each payment independently, so this ordering does not affect any existing
behaviour.
The SQL backend introduced in this PR was ignoring the SettleTime and
FailTime fields provided in HTLCSettleInfo and HTLCFailInfo, instead
always recording time.Now() as the resolution timestamp. The KV backend
correctly serializes and deserializes these fields.
The timestamps are set by the caller using a mockable clock
(p.router.cfg.Clock.Now() in payment_lifecycle.go), so ignoring them
means the stored timestamp reflects when the DB write happened rather
than when the event occurred, breaking deterministic testing.
This commit also extends the test assertions in assertPaymentInfo to
verify that SettleTime and FailTime are correctly stored and retrieved
by the SQL backend, and updates the relevant call sites to pass explicit
timestamps so regressions are caught.
The 000009 schema version slot is now taken by 000009_graph_v2_columns
which was merged ahead of the payments schema. Bump the payments schema
file number to 000010 to avoid the collision.
Moreover update the migration_dev.go file to reflect this change and
update the order of migration.
We add a couple of additional tests to increase the unit test
coverage of the sql store but also the kv store. We only create
db agnostic unit tests so both backends are tested effectively.
Previously we had db and application logic mixed on the db level.
We now move the config option KeepFailedPaymentAttempts to the
ChannelRouter level and move it out of the db level.
We make the TestDeleteNonInFlight and separate all the logic out
for the duplicate payment test case. The deletion of duplicate
payments is now tested in isolation only for the kv backend.
The design of the sql and kv db are a bit different. A harness
interface is introduced which allows us to unit most of the test
and keep the backend specific tests at a minimum.
In commit adds the harness which will be used to run db agnostic
tests against the kv and sql backend. We have adopted all the
unit tests so far so that with this commit all the payment tests
not specifically put into the kv_store_test.go should all pass
for all backends.
We are now not supporting the LegacyPayload for the onion packet
anymore. All payments and their onion payload need to be tlv
encoded. The sql backend assumes tlv so we have to always set the
in memory presentation of a hop where the legacy parameter is still
available but deprecated to false, otherwise the hops will not be
equal and unit tests for the sql backend will fail when switched
on in the next commits.
Since now the sql backend is more strict in using the same
session key we refactor the helper so that we can easily change
the session key for every new attempt.
Now that every method of the interface was implemented we can
remove the embedded reference we put into place for the sql store
implementation so that the interface would succeed. This is now
removed.
We rename this variable to paymentsDetailsData because we will
also need to batch load the core payment and intent data in
future commits and this renaming should make it clear that this
does match payment related data but not the core data which is
in the payment and in the intent table.
We remove the SQLStore from most of the helper functions. This
also makes sure we do not accidentally create a new db tx but use
the provided db SQLQueries parameter.
Previously a one(intent)-to-many(payment) relationship it is now
changed to a one-to-one relationship because a payment request
only can have 1 payment related to it. Looking into the future
with BOLT12 offers, the fetched invoice from the offer could be
stored here as well and the relationship would still hold.
ziggie