CI / Run itests (backend=bitcoind dbbackend=postgres nativesql=true tags=test_native_sql, bitcoind-postgres-nativesql-experiment) (push) Has been cancelled
CI / Run itests (backend=bitcoind dbbackend=sqlite nativesql=true tags=test_native_sql, bitcoind-sqlite=nativesql-experiment) (push) Has been cancelled
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.
We add this constructor for an AuxHtlcDescriptor that allows setting
some of the internal fields. This is useful for testing purposes for
code external to this package that may need to extensively test the
AuxHtlcView.
When instantiating the lightning channel we now pass in the created HTLC
validator. This validator simply performs a bandwidth check and errors
out if that is insufficient.
Previously we'd perform aux bandwidth checks during path finding. This
could lead to issues where multiple HTLCs where querying the same
bandwidth but were not accounting for each other before being added to
the commitment log. We now add a new validator function that will serve
as the last point of checks before adding the HTLC to the commitment.
During path finding HTLCs could query channel bandwidth asynchronously.
At this new call site all HTLCs that are about to be added to the
channel have been organised in sequence, so it's safe to query bandwdith
again at this point as we're getting the actual up-to-date values.
We remove the aux bandwidth check from the helper canSendHtlc, which was
called from CheckHTLCTransit and CheckHTLCForward (both are methods of
the htlcswitch).
For forwards we now fail at the link level, following the introduction
of the AuxHtlcValidator.
For payments, we now may fail either at the pathfinding level, or at the
link level. The htlcswitch may no longer fail for aux bandwidth checks.
Finally, when fetching the latest htlc view (for bandwidth checks during
pathfinding) we'd silently set the nextHeight of the view to the default
zero value. We now make sure to set it to the correct nextHeight value.
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.
ziggieXXX