Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement dummy IR padding for blocks with < 2 txns #15

Merged
merged 10 commits into from
Apr 8, 2024
Merged

Conversation

BGluth
Copy link
Member

@BGluth BGluth commented Apr 4, 2024

Recently we realized that this tool will fail on blocks with less than 2 txns due to aggregation being unable to happen in this case. This PR ports a lot of logic written specifically for this from trace_decoder and rpc logic from zero-bin.

This is ready for review, but I want to hold off on merging this back in until v0.3.0 of zk_evm lands since currently we require a newer version of the zk_evm deps than what's currently published on crates.io.

Resolves #14.

Other notes:

  • Also changed the logging style to be less verbose and more readable (previous config required 3 lines per log message).
  • I added a flag specifically for use on IMX's test net that sets the block's miner field from a call to clique instead of what is contained in the response from eth_getBlockByNumber. Ideally, if we add more to this tool down the road, we probably want to move this to a config or similar.
  • A lot of the existing rpc logic was replaced with code from zero-bin. zero-bin has a lot of the rpc calls that we needed for this PR already done, and the logic it replaced is more bullet-proof against some edge cases (eg. blocks with a height < 255 would fail due to attempting to get the previous 256 block hashes).
  • Chain id is now determined from an rpc call (previously was hard-coded).

BGluth and others added 9 commits March 20, 2024 19:56
- The deps were very out of date and we need them to be up to date to do
  padding.
- Now works on block heights <= 256.
- Now pads blocks when there is < 2 txns.
- This is intended to be a short term solution. In the long term, this
  should probably be moved to a more robust config.
@BGluth BGluth requested a review from Nashtare April 4, 2024 04:47
Copy link
Contributor

@Nashtare Nashtare left a comment

Choose a reason for hiding this comment

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

Thanks Brendan! LGTM! (I've ran it against a few blocks of the test chain)

leader/src/rpc.rs Outdated Show resolved Hide resolved
leader/src/rpc.rs Show resolved Hide resolved
@BGluth BGluth merged commit 28ea0ae into main Apr 8, 2024
3 checks passed
@BGluth BGluth deleted the dummy_padding branch April 8, 2024 18:58
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.

Add dummy IR padding
2 participants