feat(sync): set max block if debug.tip provided
#1522
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #1522 +/- ##
=======================================
Coverage 75.75% 75.75%
=======================================
Files 361 362 +1
Lines 42927 42956 +29
=======================================
+ Hits 32518 32543 +25
- Misses 10409 10413 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@mattsse this PR introduces a code path where we do a redundant download of the sync target. do u think it's worth changing |
bin/reth/src/node/mod.rs
Outdated
| let max_block = if let Some(block) = self.max_block { | ||
| Some(block) | ||
| } else if let Some(tip) = self.tip { | ||
| Some(self.fetch_or_lookup_tip(db.clone(), fetch_client, tip).await?) | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
what's the motivation behind doing this here in the command.
this is a bit convoluted
There was a problem hiding this comment.
because this has to do with pipeline instantiation depending on cli args
crates/stages/src/pipeline/ctrl.rs
Outdated
| @@ -15,11 +15,13 @@ pub(crate) enum ControlFlow { | |||
| /// The progress of the last stage | |||
| progress: BlockNumber, | |||
| }, | |||
| NoProgress, | |||
| NoProgress { | |||
| previous_progress: Option<BlockNumber>, | |||
There was a problem hiding this comment.
I find the naming progress a bit confusing here, because this does not really convey what this precisely means.
is this the block number the previous stage is at? or the number of blocks the stage advanced/made progress? not really clear.
needs docs, can revise naming perhaps separately.
There was a problem hiding this comment.
this is the last progress of the current stage. agree on the renaming, added the docs to clarify
| debug!(target: "reth::cli", ?tip, "Fetching tip header from the network."); | ||
| loop { | ||
| match get_single_header(fetch_client.clone(), BlockHashOrNumber::Hash(tip)).await { | ||
| Ok(tip_header) => { | ||
| debug!(target: "reth::cli", ?tip, number = tip_header.number, "Successfully fetched tip"); | ||
| return Ok(tip_header.number) | ||
| } | ||
| Err(error) => { | ||
| error!(target: "reth::cli", %error, "Failed to fetch the tip. Retrying..."); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
why do we need to fetch this in the first place?
There was a problem hiding this comment.
To be able to instantiate the pipeline with the max_block parameter if debug.tip was provided. otherwise, the pipeline doesn't have a terminating condition
There was a problem hiding this comment.
ah okay, understood, thanks for clarifying this in the docs.
Does this mean that even if we would continue after restarting in for example execution stage we need to wait for the first peer?
There was a problem hiding this comment.
no, because the tip was already stored in the db. First, I attempt the db lookup. If the tip is not there, then i fetch
There was a problem hiding this comment.
i should prob change the fn name from fetch_or_lookup_tip to lookup_or_fetch_tip to reflect the order of operations
Closes #1510
Set
Pipeline::max_blockifdebug.tipis provided. This leads to pipeline termination upon reaching the tip.