Skip to content

Remove sync_wallets from hot paths#35

Open
amackillop wants to merge 1 commit intomainfrom
austin_mdk-747_remove-wallet-syncs
Open

Remove sync_wallets from hot paths#35
amackillop wants to merge 1 commit intomainfrom
austin_mdk-747_remove-wallet-syncs

Conversation

@amackillop
Copy link
Copy Markdown
Contributor

sync_wallets() issues serial HTTP requests to Esplora (fee rate estimation, lightning tx sync across multiple block heights, BDK wallet sync). This blocks the NAPI thread for a few seconds via runtime.block_on(), which prevents the JS event loop from polling nextEvent() for incoming HTLCs.

The 10+ second payment latency observed traces directly to this: the sender's HTLC sits unprocessed while the receiver node grinds through chain sync before the webhook handler can start its event loop.

Removed sync_wallets from start_receiving, receive_payment, get_invoice, and execute_payment. Receiving HTLCs only needs a peer connection and channel manager, both provided by node.start(). Sending payments routes via the gossip graph with fee rates already cached by node.start().

Also disabled background wallet syncing (the 30s/80s interval loop). For a webhook-based node that lives 40-60s, the immediate first tick just competes with the peer handler for Esplora bandwidth.

LDK uses highest_seen_timestamp (set via best_block_updated) for inbound payment verification with a 2-hour grace window. A stale chain tip could cause valid payments to be rejected as expired. The platform's 30-minute ping cron calls sync_wallets via the balance handler, which keeps the timestamp well within that window. get_balance() also syncs explicitly before reading UTXOs.

sync_wallets() issues serial HTTP requests to Esplora (fee rate
estimation, lightning tx sync across multiple block heights, BDK
wallet sync). This blocks the NAPI thread for a few seconds
via runtime.block_on(), which prevents the JS event loop from
polling nextEvent() for incoming HTLCs.

The 10+ second payment latency observed traces directly
to this: the sender's HTLC sits unprocessed while the receiver
node grinds through chain sync before the webhook handler can
start its event loop.

Removed sync_wallets from start_receiving, receive_payment,
get_invoice, and execute_payment. Receiving HTLCs only needs a
peer connection and channel manager, both provided by
node.start(). Sending payments routes via the gossip graph with
fee rates already cached by node.start().

Also disabled background wallet syncing (the 30s/80s interval
loop). For a webhook-based node that lives 40-60s, the immediate
first tick just competes with the peer handler for Esplora
bandwidth.

LDK uses highest_seen_timestamp (set via best_block_updated) for
inbound payment verification with a 2-hour grace window. A stale
chain tip could cause valid payments to be rejected as expired.
The platform's 30-minute ping cron calls sync_wallets via the
balance handler, which keeps the timestamp well within that
window. get_balance() also syncs explicitly before reading UTXOs.
@amackillop amackillop force-pushed the austin_mdk-747_remove-wallet-syncs branch from a3c5cce to 0ff5762 Compare April 22, 2026 01:27
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a3c5cce677

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/lib.rs Outdated
Comment on lines +378 to +379
let esplora_sync_config = EsploraSyncConfig { background_sync_config: None };
builder.set_chain_source_esplora(options.esplora_url, Some(esplora_sync_config));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Re-enable chain sync before receiving payments

This change disables Esplora background syncing (background_sync_config: None) and, in the same commit, removes sync_wallets() from the receive startup paths. In ldk-node, disabling background sync means callers must manually sync to keep wallet/chain state fresh; without that, a node that has been idle can run with stale chain data and reject otherwise valid inbound payments (e.g., timestamp-based invoice checks). Keep an explicit sync on the receive flow (or keep background sync enabled) so payment acceptance does not depend on an external cron calling get_balance.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point is to depend on the external cron to improve performance

Copy link
Copy Markdown
Contributor

@martinsaposnic martinsaposnic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes sense, although we will need to do some manual testing on staging before releasing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants