Remove sync_wallets from hot paths#35
Conversation
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.
a3c5cce to
0ff5762
Compare
There was a problem hiding this comment.
💡 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".
| let esplora_sync_config = EsploraSyncConfig { background_sync_config: None }; | ||
| builder.set_chain_source_esplora(options.esplora_url, Some(esplora_sync_config)); |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
The point is to depend on the external cron to improve performance
martinsaposnic
left a comment
There was a problem hiding this comment.
this makes sense, although we will need to do some manual testing on staging before releasing
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.