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

feat: Conditionally enable EVM emulation #355

Merged
merged 7 commits into from
Oct 22, 2024

Conversation

slowli
Copy link
Contributor

@slowli slowli commented Oct 16, 2024

What 💻

  • Updates zksync-era dependencies. Remove zksync_state and zksync_node_fee_model deps as unnecessary.
  • Conditionally enables EVM emulation using --emulate-evm flag. For now, this requires local contracts with EVM emulation support.

Why ✋

  • Local node can assist with EVM emulator development.

Evidence 📷

  • CI tests pass.
  • Tested locally.

@slowli
Copy link
Contributor Author

slowli commented Oct 17, 2024

@mm-zk Rust cache in CI looks broken; is this a known issue? It could be related to bumping the commit for the zksync-era deps; as you can see from this CI run, one job for which I've manually disabled caching works fine, and the other ones fail with esoteric errors ("can't find crate"). Weirdly enough, CI worked fine at first, and the "run" jobs that use the same caching approach work fine as well.

@slowli slowli force-pushed the aov-evm-794-adapt-era-test-node-to-evm-emulator branch from 9ca70c3 to 63d7f29 Compare October 17, 2024 12:13
Cargo.lock Outdated Show resolved Hide resolved
@slowli slowli marked this pull request as ready for review October 17, 2024 12:48
@slowli slowli requested a review from a team as a code owner October 17, 2024 12:48
@0xVolosnikov
Copy link

LGTM, but I propose to update it after this PR: matter-labs/zksync-era#3127

@popzxc
Copy link
Member

popzxc commented Oct 21, 2024

@dutterbutter could you PTAL?

dutterbutter
dutterbutter previously approved these changes Oct 21, 2024
Copy link
Collaborator

@dutterbutter dutterbutter left a comment

Choose a reason for hiding this comment

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

LGTM !

@0xVolosnikov
Copy link

Have you tried running this with a EVM emulator?

@dutterbutter
Copy link
Collaborator

dutterbutter commented Oct 21, 2024

@0xVolosnikov Yes. Well perhaps I will be more specific on the steps I used to test.

To reproduce:

  • cd contracts
  • git checkout 8bc5e7a2aee82490e107668e400fe0ae9831d625
  • cd ..
  • cargo build --release

Command to start era-test-node:

./target/release/era_test_node --dev-system-contracts=local --emulate-evm run

Tested forge usage by running the following:

  • forge init testing_evm
  • cd testing_evm
  • forge script script/Counter.s.sol:CounterScript --rpc-url http://127.0.0.1:8011 --broadcast --sender 0x87d6ab9fE5Adef46228fB490810f0F5CB16D6d04 --private-key <ERA-TEST-NODE-RICH-WALLETS> -vvvv --zksync

Note: I was still required to make use of --zksync flag.

Copy link

@0xVolosnikov 0xVolosnikov left a comment

Choose a reason for hiding this comment

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

Can't deploy EVM bytecode: must have recipient address

@dutterbutter dutterbutter self-requested a review October 21, 2024 18:01
@dutterbutter
Copy link
Collaborator

dutterbutter commented Oct 21, 2024

Apologies I did not go deeper in initial review testing as with --zksync flag things worked as expected but its apparent that they should be working without --zksync flag.

@slowli you need to adjust the DebugCall's to field usage in src/utils.rs. If evm emulator is used it does not need to be known.

I was able to successfully use forge create with below change:

Deployer: 0x78cAD996530109838eb016619f5931a03250489A
Deployed to: 0x6d6964656d480bAADA75790519f171fCd1dd3956
Transaction hash: 0xdf38929ee34dde84a30abf305410f855cec5ded64b78024a09279b784f027589

In src/utils.rs update to field with unwrap_or_default() and unwrap() - see below:

          ExecutionResult::Success { output } => Ok(DebugCall {
            gas_used: result.statistics.gas_used.into(),
            output: output.clone().into(),
            r#type: calltype,
            from: l2_tx.initiator_account(),
            to: l2_tx.recipient_account().unwrap_or_default(),
            gas: l2_tx.common_data.fee.gas_limit,
            value: l2_tx.execute.value,
            input: l2_tx.execute.calldata().into(),
            error: None,
            revert_reason: None,
            calls: traces.into_iter().map(call_to_debug_call).collect(),
        }),
        ExecutionResult::Revert { output } => Ok(DebugCall {
            gas_used: result.statistics.gas_used.into(),
            output: Default::default(),
            r#type: calltype,
            from: l2_tx.initiator_account(),
            to: l2_tx.recipient_account().unwrap(),
            gas: l2_tx.common_data.fee.gas_limit,
            value: l2_tx.execute.value,
            input: l2_tx.execute.calldata().into(),
            error: None,
            revert_reason: Some(output.to_string()),
            calls: traces.into_iter().map(call_to_debug_call).collect(),
        }),

@dutterbutter dutterbutter self-requested a review October 21, 2024 18:33
@dutterbutter dutterbutter dismissed their stale review October 21, 2024 18:34

Inaccurate testing assumptions used.

@slowli slowli force-pushed the aov-evm-794-adapt-era-test-node-to-evm-emulator branch from 077ca13 to 17e0d31 Compare October 22, 2024 10:02
@slowli
Copy link
Contributor Author

slowli commented Oct 22, 2024

JFYI, as I've mentioned in comments above, I've been able to deploy a test forge Counter contract successfully with forge script (although it requires specifying an insane gas multiplier – order of 500x; caused by not using eth_estimateGas and not accounting for large amount of pubdata produced by the transaction), and using forge create (this one does use eth_estimateGas, so it works out of the box).

@0xVolosnikov 0xVolosnikov self-requested a review October 22, 2024 11:41
@slowli slowli merged commit 56c4e92 into main Oct 22, 2024
12 checks passed
@slowli slowli deleted the aov-evm-794-adapt-era-test-node-to-evm-emulator branch October 22, 2024 12:43
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.

4 participants