-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
fix(bin): latest block in status message #4856
Conversation
da6e1fd
to
6f9d0a7
Compare
bin/reth/src/node/events.rs
Outdated
@@ -79,6 +79,7 @@ impl NodeState { | |||
result: ExecOutput { checkpoint, done }, | |||
} => { | |||
self.current_checkpoint = checkpoint; | |||
self.latest_block = Some(checkpoint.block_number); |
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.
hmm, I'm skeptical that this is the correct fix, because we only want to do this for the finish stage but this updates it for every stage, right?
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.
hmm, you're right, added a check for Finish
stage
Codecov Report
... and 12 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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, pending q re engine update of latest block
bin/reth/src/node/events.rs
Outdated
@@ -124,7 +127,7 @@ impl NodeState { | |||
); | |||
} | |||
BeaconConsensusEngineEvent::CanonicalBlockAdded(block) => { | |||
self.latest_canonical_engine_block = Some(block.number); | |||
self.latest_block = Some(block.number); |
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.
I think this needs to be moved to the CanonicalChainCommitted
event ?
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.
makes sense, CanonicalBlockAdded
is called on new payloads
Ref #4854 (comment)
Report latest block in
Status
message as the latest reached by either pipeline or blockchain tree.