use update_chain_tip() instead of sync_wallets() for invoice creation#29
use update_chain_tip() instead of sync_wallets() for invoice creation#29martinsaposnic merged 1 commit intomainfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cdf53ec2d2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if let Err(err) = self.node.update_chain_tip() { | ||
| eprintln!("[lightning-js] Failed to update chain tip for get_invoice: {err}"); | ||
| // Non-fatal: invoice will still work if timestamp is recent enough (2h buffer) | ||
| } |
There was a problem hiding this comment.
This code swallows update_chain_tip() failures even though the surrounding comment says the chain tip is needed for correct invoice-expiry calculation; when tip refresh fails after the node has been offline (for example during a temporary chain-source outage), invoice creation can proceed with stale chain time and produce invoices that are already near expiry or invalid for the requested expiry_secs. The previous start()+sync_wallets() flow failed fast in this scenario, so this should return an error (or fall back to a stronger sync) instead of continuing silently.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The 2h buffer means it's fine if the node was recently active, but if esplora is down AND the last persisted state is >2h old, we'd silently issue near-expired or invalid invoices. 🤔
There was a problem hiding this comment.
This should be performed during pings right so it is likely that the state will remain fresh. Maybe we can also leverage that to detect issues here since I think we are using the ping as a healthcheck right? So if the ping syncs are failing we should notify ourselves.
3 participants
Replace the expensive start() + sync_wallets() + stop() cycle in
get_invoice() with the lightweight update_chain_tip() (~200ms vs seconds).
Also add update_chain_tip() calls to get_invoice_with_scid() and
get_variable_amount_jit_invoice_with_scid().
moneydevkit/ldk-node@4974e8f