Cbf fix block fetch#34
Conversation
| } | ||
| self.chain_listener.block_connected(&ib.block, ib.height); | ||
| self.next_height += 1; | ||
| self.mark_syncing(); |
There was a problem hiding this comment.
Could we mark the state as syncing as soon as a new header/filter is observed, before awaiting the block fetch? As written, synced_to_tip remains true while a matched block is being fetched, so sync_wallets() can still return before that known block is applied.
There was a problem hiding this comment.
What if block fetch fails in the end? We would not apply it all, but sync_wallets() would have returned.
There was a problem hiding this comment.
I think we're concerned about the same case. My suggestion is to set synced_to_tip to false as soon as an IndexedFilter is received, before awaiting request_block(), rather than only after ConnectFull has been applied. Then sync_wallets() will wait while the fetch is pending. If the fetch ultimately fails, the existing ChainOp::Failed(Error::TxSyncFailed) path will return an error; if it succeeds, synced_to_tip will become true again only after the block has been applied and Synced is processed.
Please let me know if I am missing the point!
| // now-durable delta or has already been flushed and cleared by a concurrent writer — | ||
| // we never drop unpersisted changes. | ||
| Ok(_) => { | ||
| let _ = self.inner.lock().expect("lock").take_staged(); |
There was a problem hiding this comment.
Could we drop this workaround and depend on lightningdevkit#980 instead? Once the wallet lock is released, another operation can add to the staged changeset, and this unconditional take_staged() may clear changes that were not persisted by this call.
There was a problem hiding this comment.
yeah, i'll try to cherry-pick upstream solution
There was a problem hiding this comment.
Picked it and tested, it works.
* Add CBF chain source stubs for starting Add stub methods/functions, add basic build and start of the CBF chain source as well as basic struct containing the fields which undoubtedtly are needed. * Add waiting for gossip propagation in tests Previously tests assumed that the chain source of the lightning node and is node which mines. This is not the case with CBF chain source which needs to wait until after mining a new block a new tips propagates to it. `wait_for_block` is made to return a new height and a new function `wait_for_node_tip` is added which waits until the given height is processed (returned via `status.best_block` ) on a given node. * Populate revealed spks for CBF Ask wallet for revealed spks, register them. Implement `Listen` trait ans add register_script method as well as implementation of registered scripts/outputs.
* Add CBF chain source stubs for starting * Add waiting for gossip propagation in tests * Populate revealed spks for CBF * Add sender and listener of `ChainOp`s --------- Co-authored-by: Yeji Han <yeji.hvn@gmail.com>
- Implement CBF requester shutdown and mark runtime status stopped on clean exit. - Route generic chain source stop calls to the CBF backend. AI-assisted-by: OpenAI Codex
- Stop runtime-dependent chain sources before waiting for non-cancellable background tasks. - Allow the CBF requester shutdown to unblock the node run loop during Node::stop.
- Check CBF runtime status before publishing a rebuilt requester after restart backoff. - Shut down the newly built requester and exit the restart loop if stop() ran during the backoff.
* Add CBF chain source stubs for starting * Implement `process_kyoto_events` and `ChainOp` Co-authored-by: febyeji <yeji.han@sf.snu.ac.kr>
* Add CBF chain source stubs for starting * Add waiting for gossip propagation in tests * Populate revealed spks for CBF * Implement `process_kyoto_events` and `ChainOp` Co-authored-by: febyeji <yeji.han@sf.snu.ac.kr>
Co-authored-by: febyeji <yeji.han@sf.snu.ac.kr>
Also added env var for the CBF tests, also waiting for tx gossip for broadcast in some of the tests.
Previously we did not track the `FiltersSynced` kyoto event, so we could not tell when we had applied all blocks up to the tip. For example, when we stop and restart the node, kyoto's tip is 0 at the instant of start (it does not persist its chain), so our applied height trivially matches kyoto's tip and we would falsely conclude we had reached it. That is only actually true once we have received `FiltersSynced`.
Now the function is called `list_watched_scripts`.
Previously stalled fetch would hang indefinitely.
0cb26cf to
0b5fa3d
Compare
|
I made a huge rebase and fixed two above problems (fix sync state and cherry-picked my solution dropped in favor of the upstream one). Actually I've made rebase ontop of |
2b61076 to
4d18bdc
Compare
2 participants
This PR fixes several bugs:
Previously we did not understand whether we really reached the tip: for example on restart of the node the tip from kyoto would be 0, so
sync_walletswould incorrectly return without really waiting for the sync. Right now we only are synced when we have receivedFiltersSyncedkyoto event.Wallet persistence sometime deadlock which affected our setup. While it will be fixed in a more general way in future (Wallet mutex held across
Runtime::block_on(persist_async)can deadlock the entire node runtime lightningdevkit/ldk-node#978 and Avoid wallet persistence deadlocks lightningdevkit/ldk-node#980) I decided to leave it here in place to distinguish tests failing because of that and other failing ones.Added lookahead addresses to
list_revealed_scriptsin wallet and changed its name tolist_watched_scripts, as on a fresh run I encountered missed payments during sync because of that.Added timeout to block fetch (same for the fee fetch and just block fetch for block data).