feat: reconstructing state using CommitBlockInfoV2
#51
Conversation
zeapoz
commented
Jan 2, 2024
zeapoz
commented
- Implements reconstruction of state using Boojum-formatted blocks
zeapoz
force-pushed
the
feat/boojum-support
branch
from
ab123b0 to
cc6e295
Compare
chore: procure l1_block_number outside commit block info ref: introduce separate modules for v1 & v2 feat: commit block v2 parsing chore: correct `BOOJUM_BLOCK` value
zeapoz
force-pushed
the
feat/boojum-support
branch
from
cc6e295 to
4441b38
Compare
because the persistent state can have negative progress after program interruption
| @@ -367,6 +381,7 @@ impl L1Fetcher { | |||
| let contracts = self.contracts.clone(); | |||
| Ok(tokio::spawn({ | |||
| async move { | |||
| let mut boojum_mode = false; | |||
There was a problem hiding this comment.
Nice one! I like this approach more!
There was a problem hiding this comment.
I actually had an error where parsing failed, in v1, at a point where it should have been doing v2 - I'm not sure whether this fixed the problem, but I haven't seen it again... :-)
There was a problem hiding this comment.
Huh, in that case, let's keep that in mind should that be encountered sometime in the future
| // assert_eq!(previous_enumeration_index, tree.next_enumeration_index()); | ||
|
|
||
| // Parse blocks using [`CommitBlockInfoV1`] or [`CommitBlockInfoV2`] | ||
| let mut vec = parse_commit_block_info(&new_blocks_data, boojum_mode).await?; |
There was a problem hiding this comment.
| let mut vec = parse_commit_block_info(&new_blocks_data, boojum_mode).await?; | |
| let mut block_infos = parse_commit_block_info(&new_blocks_data, boojum_mode).await?; |
| self.latest_l2_block_num | ||
| .saturating_sub(self.first_l2_block_num) |
There was a problem hiding this comment.
This is fine for now, but ideally we should make sure that latest_l2_block_num is always defined before calling and thus implicitly latest_l2_block_num >= first_l2_block_num
There was a problem hiding this comment.
AFAIK it wasn't undefined, but too low, after restart - those metrics still have room for improvement...
There was a problem hiding this comment.
I see, I would think that the latest_l2_block_num should be set to the same value as first_l2_block_num on start, but evidently it's not...
| @@ -299,3 +298,33 @@ impl TryFrom<&abi::Token> for ExtractedToken { | |||
| }) | |||
| } | |||
| } | |||
|
|
|||
| #[cfg(test)] | |||
| mod tests { | |||
There was a problem hiding this comment.
Great call making some tests for this!
| @@ -101,15 +101,15 @@ impl TryFrom<&abi::Token> for V1 { | |||
| for initial_calldata in initial_changes_calldata[4..].chunks(64) { | |||
| let mut t = initial_calldata.array_chunks::<32>(); | |||
| let key = *t.next().ok_or_else(|| { | |||
| ParseError::InvalidCommitBlockInfo("initialStorageChanges".to_string()) | |||
| ParseError::InvalidCommitBlockInfo("initialStorageChanges key".to_string()) | |||
There was a problem hiding this comment.
| ParseError::InvalidCommitBlockInfo("initialStorageChanges key".to_string()) | |
| ParseError::InvalidCommitBlockInfo("initialStorageChangesKey".to_string()) |
We should adhere to the pre-defined format. Ditto for the rest of the ParseErrors
There was a problem hiding this comment.
Well, why should the code be a single word - is that some kind of Rust convention?
There was a problem hiding this comment.
Eh, not really. This is just what we have been doing up until this point
There was a problem hiding this comment.
Well OK, just to be consistent...
Co-authored-by: Jonathan <94441036+zeapoz@users.noreply.github.com>
