test: live sync test to check make canonical #10131
Conversation
e9f015f to
65a9afe
Compare
6fe0ab7 to
33a4a1a
Compare
crates/engine/tree/src/tree/mod.rs
Outdated
| if self.is_sync_target_head(child_num_hash.hash) { | ||
| self.make_canonical(child_num_hash.hash); | ||
| } |
There was a problem hiding this comment.
this is the main change in the PR, without this the additional blocks after backfill sync were not being made canonical.
There was a problem hiding this comment.
oh very nice, yeah I actually noticed this and was wondering
| } | ||
|
|
||
| self.provider.extend_blocks(block_data); | ||
| self.provider.extend_headers(headers_data); |
There was a problem hiding this comment.
this allows us to properly query the provider for data persisted.
crates/engine/tree/src/tree/mod.rs
Outdated
| if self.is_sync_target_head(child_num_hash.hash) { | ||
| self.make_canonical(child_num_hash.hash); | ||
| } |
There was a problem hiding this comment.
oh very nice, yeah I actually noticed this and was wondering
crates/engine/tree/src/tree/mod.rs
Outdated
| if self.is_sync_target_head(child_num_hash.hash) { | ||
| self.make_canonical(child_num_hash.hash); | ||
| } |
There was a problem hiding this comment.
this makes perfect sense, but I think we should add an additional check here that ensures, this is a new block by checking the InsertPayloadOk
reth/crates/blockchain-tree-api/src/lib.rs
Lines 256 to 257 in 0173725
separately, we should consider not reusing this from legacy tree but roll our own that no longer needs the BlockAttachment
There was a problem hiding this comment.
this makes perfect sense, but I think we should add an additional check here that ensures, this is a new block by checking the InsertPayloadOk
makes sense, done here b5b1b34
separately, we should consider not reusing this from legacy tree but roll our own that no longer needs the
BlockAttachment
sure I can work on this in a follow-up
… canonical chain commited event
…nical and recovered test_engine_tree_live_sync_transition_required_blocks_requested
…BlockStatus::Valid
0173725 to
b5b1b34
Compare
Towards: #10015
Adds a test to verify that the live synced blocks after a backfill sync are eventually made canonical.
These are the steps of the test
To make it work I needed to call
make_canonicalintry_connect_buffered_blockson successful insertion, otherwise, this was only done on the code path for downloaded blocks, pls let me know if this is advisable.There's also additional refactors and improvements in
TestHarness, like setting up the inserted headers in mock provider to properly simulate a persisted chain.