-
Notifications
You must be signed in to change notification settings - Fork 784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add lcli command for manual rescue sync #5458
Conversation
|
good point I might just close this PR for now, I'm not convinced it's worth merging. Was just chucking it here in case others wanted to use it |
The tool has value IMO, it's a plausible sync method if p2p breaks down I'm okay with merging it |
Yep agree with Lion, I was desperately looking for this last night - looks like we might need it again 🙈 |
# Conflicts: # lcli/src/main.rs
Merged latest |
lcli/src/main.rs
Outdated
@@ -552,6 +553,58 @@ fn main() { | |||
.display_order(0) | |||
) | |||
) | |||
.subcommand( | |||
Command::new("rescue") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth adding a note that this tool only works if the distance is within the data availability windows (4096 epochs ~ 18 days) otherwise blobs will become unavailable.
lcli/src/rescue.rs
Outdated
for (slot, block) in blocks.iter().rev() { | ||
println!("posting block at slot {slot}"); | ||
if let Err(e) = target.post_beacon_blocks(block).await { | ||
println!("error posting {slot}: {e:?}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is a non 200 status code, we should just break out of the loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it could be a 202 (or other error) for block-is-already-known
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but actually, that probably shouldn't happen, so yeah, I agree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep It was a 202, I added a check for this
I've address the comments and added two optional flags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michaelsproul I've reviewed your earlier changes. Feel free to mark this as ready to merge once you've review my recent changes!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@mergify queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at 781c5ec |
* Rescue CLI * Allow tweaking start block * More caching * Merge branch 'unstable' into rescue-cli # Conflicts: # lcli/src/main.rs * Add `--known–common-ancestor` flag to optimise for download speed. * Rename rescue command to `http-sync` * Add logging * Add optional `--block-cache-dir` cli arg and create directory if it doesn't already exist. * Lint fix. * Merge branch 'unstable' into rescue-cli
* Rescue CLI * Allow tweaking start block * More caching * Merge branch 'unstable' into rescue-cli # Conflicts: # lcli/src/main.rs * Add `--known–common-ancestor` flag to optimise for download speed. * Rename rescue command to `http-sync` * Add logging * Add optional `--block-cache-dir` cli arg and create directory if it doesn't already exist. * Lint fix. * Merge branch 'unstable' into rescue-cli
* Rescue CLI * Allow tweaking start block * More caching * Merge branch 'unstable' into rescue-cli # Conflicts: # lcli/src/main.rs * Add `--known–common-ancestor` flag to optimise for download speed. * Rename rescue command to `http-sync` * Add logging * Add optional `--block-cache-dir` cli arg and create directory if it doesn't already exist. * Lint fix. * Merge branch 'unstable' into rescue-cli
Proposed Changes
Add an
lcli rescue
subcommand which can be used to manually sync blocks from one beacon node to another. I've been cheekily using to muck around on Goerli in the presence of non-finality, streaming blocks from the public Nimbus endpoint to ours (with seemingly no impact on the head).