We prepare the code for the sql payment backend. However no
payment db interface method for the sql backend is implemented
yet. This will be done in the following commits. They currently
use the embedded KVStore to satify the build environment.
Add testFetchPendingInvoicesAccepted to explicitly verify the state
filtering behaviour of FetchPendingInvoices across all four contract
states:
ContractOpen (state 0) – must be returned
ContractAccepted (state 3) – must be returned
ContractSettled (state 1) – must be excluded
ContractCanceled (state 2) – must be excluded
This directly exercises the `state IN (0, 3)` predicate introduced in
the FetchPendingInvoices SQL query and addresses the review request for
explicit test coverage of the new targeted query variants. The test
runs against the KV, SQLite, and Postgres backends.
All call sites have been migrated to the targeted replacement queries
in the previous commit. Remove FilterInvoices and its associated
params struct from the SQL source and regenerate.
Replace all four FilterInvoices call sites with the focused queries
introduced in the previous commit:
FetchPendingInvoices → FetchPendingInvoices
InvoicesSettledSince → FilterInvoicesBySettleIndex
InvoicesAddedSince → FilterInvoicesByAddIndex
QueryInvoices → FilterInvoicesForward / FilterInvoicesReverse
The first three are straight 1:1 swaps — the new params structs carry
only the fields that are actually used, and the removed fields (Reverse,
PendingOnly, unused index bounds) were always left at their zero values.
QueryInvoices is restructured more substantially. The forward/reverse
branch now selects between FilterInvoicesForward and
FilterInvoicesReverse, each of which takes a concrete id bound that is
always set:
forward: AddIndexGet = IndexOffset + 1 (≥ 1 when IndexOffset = 0)
reverse: AddIndexLet = IndexOffset - 1 (or MaxInt64 when offset = 0)
Timestamp parameters are changed from nullable (sql.NullTime with
OR-based SQL fallbacks) to always-on Go-side defaults, consistent with
the approach used by the payments query:
createdAfter → time.Unix(0, 0).UTC() (epoch, before any invoice)
createdBefore → time.Date(9999, 12, 31, …) (far future, no upper cap)
This ensures the planner always sees plain range predicates on
created_at and can use the invoices_created_at_idx index.
The SQLInvoiceQueries interface is updated to expose the five new
methods and drop FilterInvoices.
The existing FilterInvoices query uses optional parameters via the
pattern `(col >= param OR param IS NULL)` for every filter. SQLite
cannot use indexes with this pattern because the OR prevents the query
planner from determining at plan time which rows satisfy the condition,
resulting in a full table scan regardless of the available indexes
(invoices_state_idx, invoices_settle_index_idx, and the primary-key
clustered index on id).
Additionally, the conditional ORDER BY:
ORDER BY CASE WHEN reverse = FALSE ... THEN id ELSE NULL END ASC,
CASE WHEN reverse = TRUE ... THEN id ELSE NULL END DESC
prevents the planner from using the index ordering and forces an
explicit sort.
Add five focused replacements, each with a plain sargable predicate and
a direct ORDER BY so the planner can always choose an index scan:
- FetchPendingInvoices: WHERE state IN (0, 3)
- FilterInvoicesBySettleIndex: WHERE settle_index >= $1
- FilterInvoicesByAddIndex: WHERE id >= $1
- FilterInvoicesForward: WHERE id >= $1 ... ORDER BY id ASC
- FilterInvoicesReverse: WHERE id <= $1 ... ORDER BY id DESC
FilterInvoicesForward and FilterInvoicesReverse accept non-nullable
timestamp parameters (created_after, created_before) so the planner
always sees plain range predicates on created_at. Callers supply
Go-side defaults when no date filter is needed, following the same
convention already used by the payments query.
FilterInvoices is kept in this commit so all existing call sites
continue to compile. It will be removed once all callers have been
migrated.
Replace curl with wget for downloading release manifests and
signatures in verify-install.sh. wget handles redirects, retries, and
error reporting more robustly by default, which avoids silent download
failures that caused misleading "Invalid signature!" errors.
Also add error checking to all download calls so failures are reported
immediately with the URL that failed, and log which signature file and
user failed gpg verification.
When gpg --verify fails, include the signature filename, username,
and full GPG output in the error message. Previously only a generic
"Invalid signature!" was printed, making it hard to identify which
signer's signature was invalid.
Add a workflow that triggers when a release is published. It runs
verify-install.sh inside the official Docker image to validate
signatures and binary hashes. If verification fails, the release
is automatically set back to draft.
The severity bot was misclassifying CLI client code as HIGH because
filenames like cmd_walletunlocker.go matched the walletunlocker/*
auth/security keyword. Add cmd/* explicitly to the MEDIUM tier and
add a classification rule to prevent filename-based false positives.
Re-adds the old no-experimental-endorsement config option as a hidden,
deprecated alias for no-experimental-accountability. This ensures
backward compatibility for users who have the old option in their
config files after the rename it.
This commit fixes a critical race condition in MarkChanFullyClosed and
pruneLinkNode where link nodes could be incorrectly deleted despite
having pending or open channels.
The race occurred because the check for open channels and the link node
deletion happened in separate database transactions:
Thread A: TX1 checks open channels → [] (empty)
Thread A: TX1 commits
Thread B: Opens new channel with same peer
Thread A: TX2 deletes link node (using stale data)
Result: Link node deleted despite pending channel existing
This creates a TOCTOU (time-of-check to time-of-use) vulnerability where
database state changes between reading the channel count and deleting
the node.
Fix for MarkChanFullyClosed:
- Move link node deletion into the same transaction as the channel
closing check, making the check-and-delete operation atomic
Fix for pruneLinkNode:
- Add double-check within the write transaction to verify no channels
were opened since the caller's initial check
- Maintains performance by keeping early return for common case
- Prevents deletion if channels exist at delete time
This ensures the invariant: "link node exists iff channels exist"
is never violated, preventing database corruption and potential
connection issues.
Before we would always log that the peer was not ready starting
up although it was not the case. We now make sure we still log
this case but only when applicable.
Before this change, CompletePaymentRequestsNoWait would return as
soon as the channel's NumUpdates increased by at least one. When
sending multiple payments, this meant the function could return
while some HTLCs were still in-flight and not yet committed to the
channel state.
The problem occurred when tests captured the channel state
immediately after calling this function. Even though we read the
current NumUpdates from the channel, HTLCs could still be in the
process of being committed. This led to a race where the channel
would progress to a new state after we thought we had correctly
captured it, causing tests to see unexpected commitment heights.
Fix this by waiting for all outgoing HTLCs to appear in
PendingHtlcs before returning. We count outgoing HTLCs before
sending, then wait until exactly len(paymentRequests) new HTLCs
are present. This guarantees all payments have fully completed
their commitment exchange and are locked in on both sides.
Fixes the flaky revokedCloseRetributionRemoteHodlCase test where
backups would capture state at height N+1 instead of the expected
height N.
Fix bug where setSelfNode compared only the seconds component of
timestamps instead of the full timestamp. This caused the node to
attempt persisting an older timestamp than what existed in the
database during restart, resulting in "sql: no rows in result set"
errors.
-Due to a newer version we need to use add_labels instead of just
labels
-The backport PR will now also copy the milestones in case the
milstones were set
This commit adds detailed documentation for the automated backport
workflow and updates the contribution guidelines to reference it.
New documentation (docs/backport-workflow.md):
- Complete overview of the automated backport process
- Step-by-step usage instructions with examples
- Detailed explanation of workflow triggers and label format
- Technical details about workflow implementation
- Conflict resolution procedures and best practices
- Multiple backport scenarios and examples
- Comprehensive troubleshooting guide
Updated contribution guidelines (docs/code_contribution_guidelines.md):
- Replaced detailed backport instructions with brief overview
- Added reference to the new detailed documentation
- Keeps contribution guidelines focused and concise
The detailed documentation provides:
- How to use backport labels correctly
- What happens when labels are added before/after merge
- How the workflow validates branches and handles errors
- Step-by-step conflict resolution instructions
- Solutions for common problems and edge cases
- Examples of valid vs invalid label formats
This documentation ensures contributors and maintainers have clear
guidance on using the automated backport workflow effectively.
This commit introduces an automated GitHub Actions workflow to streamline
the backporting process for merged PRs from master to release branches.
Key features:
- Triggers on merged PRs with labels matching 'backport-v*' pattern
(e.g., backport-v0.20.x-branch)
- Validates that target branches exist before attempting backport
- Creates separate backport PRs for each target branch
- Automatically adds 'no-changelog' label to backport PRs
- Handles merge conflicts by creating draft PRs with conflict markers
- Supports multiple simultaneous backports via multiple labels
Workflow steps:
1. Checkout repository with full git history
2. Validate all target branches exist in the remote repository
3. For each valid backport label:
- Create a new branch (backport-<pr-num>-to-<target-branch>)
- Cherry-pick commits from the master PR
- Create a new PR targeting the release branch
- Link back to the original PR
4. If conflicts occur, create a draft PR for manual resolution
Label format:
- Valid: backport-v0.20.x-branch, backport-v0.19.x-branch
- Invalid: backport candidate, backport-candidate, backport-needed
This automation reduces manual work and ensures consistency in the
backporting process while maintaining full visibility and control
for maintainers.
Fix a race condition where forwarding through a public zero-conf channel
could fail with UnknownNextPeer when using the confirmed SCID. The issue
occurred because ReportShortChanID (which updates the switch's baseIndex
to handle the confirmed SCID) was called AFTER addToGraph (which announces
the confirmed SCID to the network).
With slow backends like postgres, addToGraph takes significant time,
creating a window where other nodes learn about the confirmed SCID from
gossip and attempt to route through it, but the receiving node's switch
hasn't been updated yet to handle forwards using the confirmed SCID.
The fix reorders operations to call ReportShortChanID before addToGraph,
ensuring the switch is ready to handle the confirmed SCID before it's
announced to the network. Forwards using either the alias or confirmed
SCID will work since getLinkByMapping uses baseIndex to map both to the
same link in forwardingIndex.
Fixes flaky test: zero_conf-channel_policy_update_public_zero_conf
We should avoid taking the lock of a mutex inside transaction.
Currently we also take this lock in other places and there is a
chance that in case the application lock aquires the lock but
all transactions are already blocked waiting for the mutex to
unlock, we end up in a deadlock.
We now allow the mission control manager to skip over deserializable
errors. We cannot repair this these results but we just skip over
it so we can startup properly.
When fetchAll() encounters entries that fail to deserialize, in
addition to skipping them, now also:
- Delete the corrupted entries from the database
- Remove them from the in-memory keysMap and keys tracking structures
This prevents corrupted entries from:
- Being counted toward maxRecords, which would cause valid entries
to be pruned prematurely
- Persisting in the database indefinitely
- Causing inaccurate entry counts in startup logs
This commit enhances the integration test to validate the LSP heuristic
end-to-end with real network topology and payment probing.
Network topology additions:
- Added Frank node as a private destination
- Created multi-LSP test scenario with Bob, Eve, and Dave as LSPs
New test cases:
1. "probe based estimate, public target with public hop hints"
- Validates Rule 1: public invoice target routes directly
- Even with public hop hints, direct routing is used
- Expected: standard single-hop fees
2. "probe based estimate, multiple different public LSPs"
- Validates multi-LSP worst-case selection
- Frank has routes through Bob (low fee), Eve (HIGH fee), Dave (medium)
- Expected: Eve's worst-case fees (most expensive)
- Tests griefing protection (max 3 LSP probes)
This commit implements a comprehensive LSP (Lightning Service Provider)
detection heuristic and updates the payment probing logic to handle
multiple LSPs with worst-case fee estimation.
Key changes:
1. LSP Detection Heuristic (isLSP function):
Implements three rules to detect LSP setups:
- Rule 1: If invoice target is public → NOT an LSP (route directly)
- Rule 2: If at least one destination hop is public → IS an LSP
- Rule 3: If all destination hops are private → NOT an LSP
2. LSP Route Preparation (prepareLspRouteHints function):
- Groups route hints by unique public LSP nodes
- Filters out non-LSP routes based on the heuristic
- Tracks worst-case fees and CLTV delays for each LSP
- Returns adjusted route hints with LSP hop stripped
3. Multi-LSP Probing (probePaymentRequest updates):
- Probes up to 3 unique LSPs maximum (griefing protection)
- Selects the WORST-CASE (most expensive) route for conservative
fee estimation
- Adds comprehensive debug logging for worst-case selection process
- Properly formats vertex logging using %v (calls Vertex.String())
The worst-case approach ensures users won't be surprised by higher fees
when the actual payment is sent, providing a more conservative and
reliable fee estimate.
This commit also adds extensive unit test coverage for the LSP detection
heuristic and route preparation logic.
TestIsLsp:
- Edge cases: empty route hints, nil scenarios
- Rule 1: Public invoice target (3 tests)
- Rule 2: All private destination hops (4 tests)
- Rule 3: At least one public destination hop (6 tests)
TestPrepareLspRouteHints:
- LSP grouping and filtering logic
- Worst-case fee selection across route hints
- Worst-case CLTV delta tracking
- Adjusted route hints validation (LSP hop stripped)
- Multi-LSP scenarios with different fees
This commit adds the HasNode function to the RouterBackend struct,
which checks if a node exists in the graph (i.e., has public channels).
This function is needed by the LSP detection heuristic to determine
if a node is publicly reachable.
The function is wired up in rpcserver.go to query the graph database.
Replace hardcoded WithGlobalLock assignment with configurable
options wallet postgres backends. Also add the WithGlobalLock
option to the channeldb table for postgres backends.
Defaults:
- channeldb: false (allow concurrent access)
- wallet: true (maintain safe single-writer behavior)
Users can now override these defaults via:
- db.postgres.channeldb-with-global-lock
- db.postgres.walletdb-with-global-lock
This gives operators flexibility while maintaining safe defaults
until full native SQL migration is complete.
Moreover exclude db.postgres.walletdb-with-global-lock check
in the sample config file script. We cannot easily check the
correct default because we set it later in the LND startup
sequence so we exclude it.
Add two configuration options to control global lock usage for
different postgres database backends:
- ChannelDBWithGlobalLock: for channeldb access (default: false)
- WalletDBWithGlobalLock: for wallet database access (default: true)
These allow fine-grained control over which databases use global
locks, rather than hardcoding the behavior. This is a temporary
measure until the revocation log and wallet are migrated to native
SQL and become fully concurrent-safe.
Fix a bug where channels with both policies disabled were not added to
the graph cache during startup. When a policy update later re-enabled
one of the directions, the update would succeed in the database but fail
to update the graph cache (since the channel structure was never added),
preventing the channel from being used for routing.
We now return an error when blinded and non blinded attempts are
combined. This was theoretically possible to register a legacy
attempt in combination with a blinded payment. This would have
been prevented by other checks in the code because legacy payments
are not split into shards.
The config file format changed. The tool golangci-lint migrate
was used to migrate the old config. However old comments and also
the structure of the disabled linters was preserved.
Moreover the new v2 version introduced new linters, we disable
3 of them because they are very noise and we do not really want
to check for them: funcorder, noinlineerr, embeddedstructfieldcheck.
Older LND versions had a bug which would create HTLCs with
0 locktime. The utxonursery will have problems dealing with such
htlc outputs because we do not allow height hints of 0. Now we
will fetch the closeSummary of the channel and will add a
conservative height for rescanning.
Version v1.4.3 has some important bugfixes. We bump both version
in the same go so we do not need to tag the kvdb package and the
latest version of will be used.
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
we make the index assertion db independant so it is a noop for
a future native sql backend. This allows us to reuse even more
tests for the different db architectures.
This commit starts reusing test cases which are not dependant on
the kv db backend. So they can be later used with the native db
implementation as well.
ziggie