-
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: reorgs with forks including invalid blocks #10320
Conversation
755a57d
to
2b7485b
Compare
2b7485b
to
04bd239
Compare
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.
awesome, I really like all of the new tests, and just have some nits
crates/engine/tree/src/tree/mod.rs
Outdated
@@ -253,7 +264,7 @@ impl TreeState { | |||
|
|||
while old_hash != current_hash { | |||
if let Some(block) = self.blocks_by_hash.get(&old_hash) { | |||
old_chain.push(block.clone()); | |||
old_chain.insert(0, block.clone()); |
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.
since we seem to be only inserting at the front, it might be worth doing push
and then reverse
like we do with new_chain
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, done here 3f8a5bc
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.
very cool
pending @Rjected suggestions
d879eda
to
3f8a5bc
Compare
These changes primarily address potential reorg-related failures raised by failing hive tests.
TreeState::on_new_head
includes two fixesTreeState::is_fork
: besides walking back from the given target hash and trying to find the canonical head (which would only address canonical chain extensions), it also checks walking back from the canonical head and tries to find the target hash.newPayload
calls, both in test block creation and in theCancunPayloadFields
passed to the callTestHarness
and refactors