Skip to content

Conversation

@jsign
Copy link
Collaborator

@jsign jsign commented Apr 15, 2025

This PR adds a test that creates the current bytecode worst case scenario for provers.

It creates the target bytecodes as part of pre-alloc, which isn't friendly for filling. An alternative would be to generate these bytecodes with txs. That probably requires creating blocks before the attack block since you can't deploy ~300MiB of code in a single block due to gas limits. Sounds doable, but it can take some work.

To fill: uv run fill --from=Cancun --until=Cancun -m "zkevm and blockchain_test"

cc @kevaundray @marioevz

Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

I spoke with kev about the purpose of the test and he explained that it was necessary that the contracts were not part of the pre-alloc since that would make the size of the fixture several gigabytes.

I suggested we create a factory contract that generates random code using keccak with the initcode.

We could:

  • use a first transaction to deploy the factory contract.
  • use N transactions to call the factory and create the desired amount of contracts.
  • lastly send a transaction that uses EXTCODESIZE/HASH on all contracts that were created by the prior txs

@jsign jsign force-pushed the jsign-zkvm-bytecode-worstcase branch 3 times, most recently from c114004 to afc7f13 Compare April 21, 2025 19:56
@raxhvl
Copy link
Member

raxhvl commented Apr 22, 2025

Hey @marioevz could we deploy the factory contract in a separate block? And then call EXTCODESIZE/HASH txs in next block as part of the same blockchain test?

i.e is it possible to persist state between two blocks in a test?

Asking because the contract deployments (especially its high memory usage) will eat into the gas limit of block.

My understanding is that the t8n tool only supports a single block after which it writes the post state to disk.

@jsign
Copy link
Collaborator Author

jsign commented Apr 22, 2025

The factory creation and calling to the factory should be in as many blocks as needed (due to gas limit). After all is ready, do the "attack" in a single separate block so you have full gas limit.

My understanding is that the t8n tool only supports a single block after which it writes the post state to disk.

For these kind of tests, blockchain tests, you can pass multiple blocks, and the state is kept in the entire blockchain.

@raxhvl
Copy link
Member

raxhvl commented Apr 22, 2025

That makes sense. One challenge I think is getting the deployed contract addresses to call in "attack" block. We could write the addresses to the factory's storage, i.e we'll spend some non-trivial amount of gas in the attack block reading from storage.

@marioevz
Copy link
Member

That makes sense. One challenge I think is getting the deployed contract addresses to call in "attack" block. We could write the addresses to the factory's storage, i.e we'll spend some non-trivial amount of gas in the attack block reading from storage.

Passing them through calldata is another option, because the created contract addresses are deterministic and known during the test function execution.

@jsign jsign marked this pull request as ready for review April 23, 2025 18:05
marioevz and others added 2 commits April 30, 2025 22:17
Signed-off-by: Ignacio Hagopian <[email protected]>

lints

Signed-off-by: Ignacio Hagopian <[email protected]>

fix

Signed-off-by: Ignacio Hagopian <[email protected]>

ruff fix

Signed-off-by: Ignacio Hagopian <[email protected]>

simplify

Signed-off-by: Ignacio Hagopian <[email protected]>

fix linter

Signed-off-by: Ignacio Hagopian <[email protected]>

test: Implement dynamic contract deployment for worst bytecode zkevm test

fix: review comments
@marioevz marioevz force-pushed the jsign-zkvm-bytecode-worstcase branch from 55853d2 to a672724 Compare April 30, 2025 22:19
Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

Rebased and should be ready to merge :)

@marioevz marioevz merged commit 448a1a9 into main Apr 30, 2025
22 checks passed
@marioevz marioevz deleted the jsign-zkvm-bytecode-worstcase branch April 30, 2025 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants