refactor(fast-inbox): some cleanups - #25422
Conversation
… API and the accepted limitations
…etless node cutover
… checks, stale heights and the leaf-index derivation
…ecks and height refresh precisely
…en-tip assumption at an epoch boundary
| self.bundle_applied = true; | ||
|
|
||
| // Real messages occupy the leading lanes; everything past `num_msgs` is zero padding. | ||
| assert_trailing_zeros(bundle.messages, bundle.num_msgs); |
There was a problem hiding this comment.
Removing this as the same check is also done in append_only_tree::append_leaves_to_snapshot below.
|
|
||
| // Load sync point for blocks defaulting to start block | ||
| const { blocksSynchedTo = this.l1Constants.l1StartBlock } = await getArchiverSynchPoint(this.stores); | ||
| const blocksSynchedTo = (await this.stores.blocks.getSynchedL1BlockNumber()) ?? this.l1Constants.l1StartBlock; |
There was a problem hiding this comment.
I find it easier to see that blocksSynchedTo is the synched l1 block number by calling the block store api directly, and it also avoids another unnecessary async call in getArchiverSynchPoint.
|
|
||
| // Accumulate the streaming bundle now that the block is fully built, so a mid-build throw above leaves the | ||
| // checkpoint's message list (and thus its rolling hash) consistent with the blocks actually built. | ||
| this.l1ToL2Messages.push(...l1ToL2Messages); |
There was a problem hiding this comment.
I didn't feel comfortable that it was changing the array passed to the constructor directly. And while changing it, I found that it also made sense to replace the array with a rolling hash instead, since it's what's been built for each block added.
| const l1ToL2LeafCount = (header: BlockHeader) => header.state.l1ToL2MessageTree.nextAvailableLeafIndex; | ||
| const checkpointStartCount = l1ToL2LeafCount(this.previousBlockHeader); | ||
| const checkpointEndCount = l1ToL2LeafCount(this.checkpoint.blocks.at(-1)!.header); | ||
| if (this.l1ToL2Messages.length !== checkpointEndCount - checkpointStartCount) { |
There was a problem hiding this comment.
Added this simple check to throw an error early if the number of messages doesn't match, instead of waiting until later to find out when the hash doesn't match with the value in the built block's header.
b3f5d87 to
48043e7
Compare
| blocksInCheckpoint: L2Block[]; | ||
| /** The last block's proposal, held back to travel with the checkpoint proposal instead of being gossiped. */ | ||
| blockPendingBroadcast: BlockProposal | undefined; | ||
| streamingState: StreamingCheckpointState; |
There was a problem hiding this comment.
Changes made in this file around streamingState is so that it doesn't get modified under the hood when being passed around.
145907b to
f0d1f1b
Compare
2 participants
Some small cleanups I did while reading the codebase. Details are left in the comments.