feat(l1): getBlockHeaders eth capability test #989
Conversation
cmd/ethereum_rust/ethereum_rust.rs
Outdated
| if store | ||
| .update_latest_block_number(block.header.number) | ||
| .is_err() | ||
| { | ||
| error!("Fatal: added block {} but could not update the block number -- aborting block import", block.header.number); | ||
| break; | ||
| }; | ||
| if store | ||
| .set_canonical_block(block.header.number, hash) | ||
| .is_err() | ||
| { | ||
| error!("Fatal: added block {} but could not set it as canonical -- aborting block import", block.header.number); | ||
| break; | ||
| }; |
There was a problem hiding this comment.
This is already fixed by apply_fork_choice
There was a problem hiding this comment.
apply_fork_choice in line 157 sets the last block imported as the head, which makes all its ancestor blocks canonical. I'd check removing this as Fede suggests and checking everything still works.
cmd/ethereum_rust/ethereum_rust.rs
Outdated
| if store | ||
| .update_latest_block_number(block.header.number) | ||
| .is_err() | ||
| { | ||
| error!("Fatal: added block {} but could not update the block number -- aborting block import", block.header.number); | ||
| break; | ||
| }; | ||
| if store | ||
| .set_canonical_block(block.header.number, hash) | ||
| .is_err() | ||
| { | ||
| error!("Fatal: added block {} but could not set it as canonical -- aborting block import", block.header.number); | ||
| break; | ||
| }; |
There was a problem hiding this comment.
apply_fork_choice in line 157 sets the last block imported as the head, which makes all its ancestor blocks canonical. I'd check removing this as Fede suggests and checking everything still works.
crates/common/rlp/decode.rs
Outdated
| @@ -330,6 +336,34 @@ impl<T1: RLPDecode, T2: RLPDecode, T3: RLPDecode> RLPDecode for (T1, T2, T3) { | |||
| } | |||
| } | |||
|
|
|||
| impl< | |||
There was a problem hiding this comment.
nit: a small comment stating that this decodes a 4-element list as a tuple would be helpful.
| pub fn fetch_headers(&self, storage: &Store) -> Vec<BlockHeader> { | ||
| let start_block = match self.startblock { | ||
| // Check we have the given block hash and fetch its number | ||
| HashOrNumber::Hash(block_hash) => storage.get_block_number(block_hash).ok().flatten(), |
There was a problem hiding this comment.
Same as the other PR here, we should return an empty response + log if error.
| for _ in 0..limit { | ||
| let Some(block_header) = storage | ||
| .get_block_header(current_block as u64) | ||
| .ok() |
Motivation
We' re still missing support for the eth p2p capability, this adds support for the message "getBlockHeaders",
tested with the hive test suite.
Closes #955.