Skip to content

Conversation

@tnull
Copy link
Collaborator

@tnull tnull commented Feb 13, 2025

Previously, we would calculate each listener's best block height outside of the loop, which will have us retrying init::synchronize_listeners with stale data, potentially leading to panicking as we might try to connect blocks from the original height to listeners that might have been advanced by the previous itertation of the loop before hitting an error. Here, we mitigate this by simply retrieving the current best block height for each listener at the start of each iteration.

Previously, we would calculate each listener's best block height outside
of the `loop`, which will have us retrying `init::synchronize_listeners`
with stale data, potentially leading to panicking as we might try to
connect blocks from the original height to listeners that might have
been advanced by the previous itertation of the `loop` before hitting an
error. Here, we mitigate this by simply retrieving the current best
block height for each listener at the start of each iteration.
@tnull tnull requested a review from jkczyz February 13, 2025 12:42
@tnull tnull added this to the 0.5 milestone Feb 13, 2025
Comment on lines +296 to +306
let mut chain_listeners = vec![
(
onchain_wallet_best_block_hash,
&**onchain_wallet as &(dyn Listen + Send + Sync),
),
(
channel_manager_best_block_hash,
&*channel_manager as &(dyn Listen + Send + Sync),
),
(sweeper_best_block_hash, &*output_sweeper as &(dyn Listen + Send + Sync)),
];
Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed offline that making current_best_block a method on Listen would be useful. It simplifies the API and would avoid needing to allocate a Vec on each iteration.

@tnull tnull merged commit 6de3500 into lightningdevkit:main Feb 13, 2025
10 of 14 checks passed
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