In this commit, we add two package-level generic helpers to the actor
module as part of the broader chan error -> Future[error] migration in
the discovery package.
CompleteWith[T](p Promise[T], val T) is a one-liner convenience wrapper
over p.Complete(fn.Ok(val)). It lets callers complete a promise with a
plain value without constructing an fn.Result inline, which cuts noise
at every completion site.
AwaitFuture[T](ctx, f Future[T]) (T, error) provides the symmetric
receive side: it blocks until the future resolves or the context is
cancelled and returns the value and any context error unpacked from
the fn.Result, matching the (val, err) convention callers expect.
Both functions are deliberately thin (no policy, no timeout, no new
state) so they compose freely with higher-level helpers built on top,
e.g. discovery.AwaitGossipResult.
go.mod is updated to pin the actor module via a local replace directive
so the rest of the lnd module picks up these additions without waiting
for a tagged release.
Skip the chain_params network check when startup is explicitly
configured to skip SQL migrations. In that mode the schema is assumed
to already be managed externally, and the chain_params table may not
exist yet. Avoid failing startup on a missing table in this path.
This is in particular important when running with a postgres
backend.
This only works if you run LND with the native sql flag but
people should run it with this flag from 21 on anyways.
Strengthen migration consistency coverage by checking the reverse
mapping from embedded SQL files to migrationConfig entries, deriving
previous schema state from slice order instead of Version, rejecting
schema version regressions, and asserting migration names match the
embedded SQL file stems.
Also fix the graph v2 migration config name to match the embedded
migration filename.
Add the 000014_payments_no_fail_reason_index migration to the
main migration configuration so the global migration list stays in
sync with the embedded SQL schema files.
The channelLink.Stop() teardown had an inverted ordering that could
cause a permanent deadlock of the invoice registry under concurrent
peer disconnect.
The previous order was:
1. HodlUnsubscribeAll -- removes subscriptions
2. hodlQueue.Stop() -- kills the queue's internal goroutine
3. cg.Quit() -- signals htlcManager to stop
4. cg.WgWait() -- waits for htlcManager to exit
The race window between steps 2 and 4 left htlcManager alive. A
RevokeAndAck arriving during that window could drive processRemoteAdds
→ processExitHop → NotifyExitHopHtlc, registering a new hodl
subscription backed by a dead hodlQueue (ChanIn() has no reader).
Any subsequent call to notifyHodlSubscribers (e.g. MPP auto-release
timer, expiry watcher, or explicit settle/cancel) would then block
indefinitely on the unbuffered ChanIn(), holding hodlSubscriptionsMux.
Concurrent NotifyExitHopHtlc calls waiting for that lock, plus callers
holding the invoice-level lock waiting for those, produce a full
deadlock of the invoice registry with no recovery path short of a
daemon restart.
The fix is to stop htlcManager before touching the hodl subscription
state. htlcManager is the sole caller of NotifyExitHopHtlc, so once
cg.WgWait() returns no new subscriptions can be registered, making
HodlUnsubscribeAll and hodlQueue.Stop() race-free.
The zombie fallback in SQLStore.FetchChannelEdgesByID unconditionally
constructed a models.NewV1Channel regardless of the requested gossip
version. Use the passed version to select the correct constructor so
that v2 zombie edges carry the right version.
A new testFetchZombieEdgeVersioning versioned test verifies that
zombie edges returned by FetchChannelEdgesByID have the correct
gossip version for both v1 and v2.
Add an explicit lnwire.GossipVersion parameter to FilterKnownChanIDs
on the Store interface, SQLStore, KVStore, and ChannelGraph. Since
FilterKnownChanIDs is always called from a version-scoped ChanSeries
context, a single version parameter is cleaner than per-item version
reads. A convenience wrapper on VersionedGraph preserves the existing
ChanSeries call-site signature by threading c.v automatically.
Replace the hardcoded GossipVersion1 in forEachChanInSCIDList with an
explicit version parameter so the helper can be used for v2 channel
lookups. The caller in FilterKnownChanIDs now passes the version
through.
Keep SQLite's default idle connection limit aligned with the
open connection limit so the default pool matches v1 behavior.
This is a follow-up regression fix to the restored open-connection
default in e263ea145. After that change, SQLite again defaulted to
SetMaxOpenConns(2), but SetMaxIdleConns still fell back to 6. Go
silently caps idle connections at the open limit, so nothing
crashed, but the configured idle default became misleading and no
longer matched v1.
Use cfg.MaxConns() as the inherited idle default, keep the
explicit MaxIdleConnections override, and add unit coverage for
the default and override cases. The mismatch was easy to miss
because the code still compiled and basic tests did not assert the
effective idle pool sizing.
Scope the safety-net rollback to each retry attempt instead of
storing one deferred rollback per loop iteration.
This keeps cleanup local to the active attempt while
preserving the existing commit and rollback behavior.
Validate migration descriptor ordering before executing a
migration stream so inconsistent metadata fails fast.
In addition to checking contiguous descriptor versions and keeping
LatestMigrationVersion aligned with the last descriptor, reject a
non-zero LatestMigrationVersion when the descriptor list is empty.
Without that guard, a set such as {LatestMigrationVersion: 5}
passed validation silently.
The validation intentionally focuses on version consistency.
Descriptor names remain optional metadata for debugging, and the
checks still run only when migrations execute, which means
SkipMigrations continues to bypass validation by design. Add unit
coverage for the new empty-descriptor case.
Sanitize Docker container names more aggressively and add a
random suffix so concurrent test runs do not collide on the
same fixture name.
Normalize unsupported characters, trim leading and trailing
punctuation, and keep the fallback name for cases that sanitize
down to nothing. Add unit coverage for the sanitizer so the
allowed name surface stays explicit.
Remove the unused txExecutorOptions retry helper so the package
only keeps the backoff logic that is actually used.
This avoids carrying a second retry API with different
semantics from the live exponential backoff path.
Make TransactionExecutor satisfy the BatchedTx contract by
providing Backend() and asserting the interface conformance at
compile time.
This was a latent interface mismatch rather than an immediately
triggered package-wide compile failure. The executor was
instantiated directly, but sqldb/v2 did not yet assert or use it
as a BatchedTx, so the missing method stayed hidden until a caller
tried to rely on the advertised interface.
At the same time, move Backend() onto BatchedQuerier so the lower-
level contract explicitly requires backend identity. That lets the
executor delegate directly instead of probing an anonymous
interface at runtime, which would have weakened the contract and
fell back to BackendTypeUnknown instead of failing at compile
time.
Keep the focused runtime test and the compile-time assertion so
future interface drift is caught immediately.
Route migration skipping through the BaseDB field that each
store already initializes.
This makes the embedded state meaningful and keeps the runtime
migration behavior consistent across both backends.
Use the same NewTestDBWithVersion argument order across the
SQLite and Postgres test helpers.
This was a latent build-tag API mismatch rather than a current
package-wide compile failure. Any shared helper that called
NewTestDBWithVersion(t, set, version) would compile under one
backend tag and fail under the other, but the mismatch stayed
hidden because nothing in sqldb/v2 called the helper yet.
Wrap SQLite programmatic migration setup failures with the
SQLite error helper instead of the Postgres helper.
Before this change, a MakeProgrammaticMigrations failure on the
SQLite path returned an error that mentioned Postgres, which made
backend-specific setup failures needlessly confusing to debug.
The package still compiled and behaved normally unless that narrow
error path was exercised, which is why it slipped through.
Add a targeted unit test that forces the failing constructor path
and asserts the returned error is attributed to SQLite rather
than Postgres.
Restore the low default SQLite connection limit used in v1 so
the v2 store does not default to a Postgres-sized pool.
This is a real v2 regression from v1. The v1 store defines
DefaultSqliteMaxConns = 2 and routes SetMaxOpenConns through
cfg.MaxConns(), while v2 had fallen back to the generic
defaultMaxConns = 25. That change did not break compilation, but
it quietly changed runtime pool sizing in a way that is hostile
to SQLite's single-writer concurrency model and can increase lock
contention.
Add a small config helper and unit test so callers can still
override the limit explicitly while the default remains safe for
SQLite. The regression was easy to miss because existing tests
did not assert the effective default pool sizing.
Apply the RequireSSL config knob when opening Postgres stores
so it cannot be silently ignored.
Before this change, sqldb/v2 exposed PostgresConfig.RequireSSL
but still opened cfg.Dsn verbatim, which meant RequireSSL=true
was a no-op. A caller could set RequireSSL=true together with a
DSN such as sslmode=disable and still establish a non-TLS
connection.
This is a v2-only API contract bug, not a v1 regression in DSN
handling: v1 never offered a separate RequireSSL flag and always
left TLS policy entirely up to the DSN. The fix rewrites the DSN
to use sslmode=require when needed, while preserving stricter
modes such as verify-ca and verify-full.
The bug was easy to miss because no test asserted that the boolean
flag changed the effective DSN or overrode an insecure sslmode.
Keep the focused DSN rewrite test because it proves the contract
without needing a live Postgres instance.
Restore the no_sqlite shim so unsupported SQLite targets still
build the module.
Update the stub to satisfy the current migration interface and
restore the missing Postgres-only SQL error helpers. These
failures were easy to miss because the default developer and CI
paths build native SQLite targets, while the broken code only
showed up on no-SQLite architectures and build tags.
Align the backend-swapped test helper files with the platforms
where their backing SQLite and Postgres helper implementations
actually exist.
This keeps the exported helper surface internally consistent even
on targets that current CI likely does not exercise. Before this
change, the SQLite helper file was still selected on no-SQLite
targets, and the Postgres helper file was still selected on
openbsd and netbsd under test_db_postgres even though the Docker
fixture was compiled out there.
Keep the Postgres schema rewrite keyed on " TIMESTAMP" so
CURRENT_TIMESTAMP is not rewritten while schema files are
adapted.
Add a focused unit test to keep the replacement aligned with the
existing v1 behavior.
Simplify FetchNonTerminalPayments by collapsing the selector down to
two branches: payments that are not failed and have no settled attempt,
and payments that still have unresolved attempts. This keeps the same
non-terminal semantics while making the query easier to reason about.
Also add a partial index on payments(id) where fail_reason IS NULL to
speed up the startup selector branch that scans payments without a
recorded failure reason.
The PR severity classifier only needs to run `gh pr view`, `gh pr edit`
(labels), and `gh pr comment`. All three operations are fully covered by
the built-in GITHUB_TOKEN given the existing permissions block:
permissions:
contents: read
pull-requests: write
issues: write
The workflow uses `pull_request_target`, which runs in the base repo
context, so GITHUB_TOKEN has write access even for fork PRs.
Inspection of the claude-code-action@v1 source confirmed that the only
internal call that would require `contents: write` is branch deletion,
which is never exercised here because Claude's tools are locked down to
`gh pr view/edit/comment` via --allowedTools.
This removes the dependency on the PR_SEVERITY_BOT_TOKEN PAT secret.
RegisterAttempt falls back to the payment identifier when an attempt
hash is nil so legacy data can still round-trip safely. In live router
code, however, a nil attempt hash should never happen for newly
registered attempts.
Add an error log on the fallback path so an unexpected nil attempt hash
is surfaced immediately instead of silently persisting the fallback
value.
Use isSQLDB to explicitly assert the expected outcome per backend:
SQL should succeed with empty results, KV should return
ErrVersionNotSupportedForKVDB.
Rename to testGraphZombieIndex and add it to the versionedTests table
so it runs against both v1 and v2 backends. The assertNumZombies
helper is updated to accept a gossip version parameter.
Rename to testLightningNodeSigVerification and add it to the
versionedTests table so it runs against both v1 and v2. The signing
step is version-specific (ECDSA for v1, Schnorr for v2) while the
verification path is shared.
