Skip to content

Conversation

@julio4
Copy link
Collaborator

@julio4 julio4 commented Nov 6, 2025

Modified checkpoint's tests to improve overall coverage.

One thing is I modified the mocked StateRootProvider for GenesisStateProvider to provide empty state root (instead of random state root), to ensure predictability in tests (for example comparing two payloads built out of the same checkpoint, so they should be equal)

/// Helper test function to apply multiple transactions on a checkpoint
fn apply_multiple<P: PlatformWithRpcTypes>(
root: Checkpoint<P>,
txs: &[Recovered<types::Transaction<P>>],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe txs: impl Iterator<Item=types::Transaction<P>>>?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, this can be written as something along those lines:

let c1 = block.start();
let many_txs = vec![100 txs];
let c100 = many_txs.into_iter().fold(c1, Checkpoint::apply)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: also maybe apply_many, or we can make apply itself work with individual or collections of txs.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, was thinking that this is at the Checkpoint type, not in a test helper. Forget everything I said ;-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep I'm not sure if it's a good idea to add a apply_many at the checkpoint type level but we can discuss this separately

@julio4 julio4 merged commit d52ed99 into flashbots:main Nov 7, 2025
4 checks passed
@julio4 julio4 deleted the test/checkpoints branch November 7, 2025 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants