feat: Evm Equivalence #2929

jrchatruc · 2024-09-19T16:45:04Z

What ❔

This PR contains the rest of the changes required for EVM Equivalence. Changes have been made to make sure it can be merged to main currently without having to point to an era-contracts branch containing the EVM interpreter and the EVM Gas Manager. The changes required for this are all here. Also, the evm simulator hash and bytecode are currently set to be the same as the default AA contract as otherwise things will not work until the evm interpreter is present in the era-contracts branch.

The EVM equivalence changes are:

A use_evm_simulator (defaulted to false) config value to set whether the zk stack is initialized with the evm equivalence feature or not. Most changes below are conditional on whether this flag is set or not.
A new Postgres migration is added with an evm_simulator_code_hash column added to the following tables:
- protocol_versions
- l1_batches
- miniblocks
  The corresponding dal modules were updated to account for these new columns.
The EvmInterpreter is added as a base system contract.
The decommiter present in the vm_latest now has separate hash calculations for each version, ZKEvm and EVM. In particular the length of the pre-image differs (EVM is in bytes, zkEVM in words, i.e. 32 bytes)
An EvmDeployTracer is added as another default tracer. Its purpose is to add EVM contracts to the decommiter once an evm deployment ends succesfully.
For the vm_latest, the TransactionData now checks if the address is absent and in the afirmative case it writes the corresponding flag on the reserved bit to signal an EVM contract deployment.
The decommitment processor for the vm_latest now has the hash and code of the EVM interpreter.
The EthTxAggregator now adds the call to the getters of the hash and code of the evm interpreter to the multicall.
The main_executor calculates a new_known_factory_deps with all the new known EVM bytecodes with the help of the decommiter. These are then added to the regular factory deps.

Why ❔

Checklist

PR title corresponds to the body of PR (we generate changelog entries from PRs).
Tests for the changes have been added / updated.
Documentation comments have been added / updated.
Code has been formatted via zk fmt and zk lint.

…t address

* Add evm simulator as optional for the server * Changes to make it work against era-contracts `main` * fmt * Calculate bytecode of evm gas manager only if evm simulator flag is on * Add multicall for get evm simulator bytecode hash * Use bytecode of default account for the evm simulator if not present * Remove unnecessary comments * Use bytecode of default account for evm simulator in gas test sc * Remove tests for the evm simulator --------- Co-authored-by: Javier Chatruc <[email protected]>

…ontract address

core/bin/system-constants-generator/src/utils.rs

core/lib/contracts/src/lib.rs

0xVolosnikov · 2024-09-20T13:07:39Z

core/lib/multivm/src/versions/vm_fast/tests/nonce_holder.rs

@@ -92,7 +93,7 @@ fn test_nonce_holder() {
    run_nonce_test(
        1u32,
        NonceHolderTestMode::SetValueUnderNonce,
-        Some("Previous nonce has not been used".to_string()),
+        Some("Error function_selector = 0x13595475, data = 0x13595475".to_string()),


Do we really need these changes?

Not really. These changes originate from this PR, which includes protocol defense updates. We needed them for the CI to pass before, but we can remove them now and check if they're still necessary.

0xVolosnikov · 2024-09-20T13:10:13Z

core/lib/multivm/src/versions/vm_latest/types/internals/transaction_data.rs

@@ -62,6 +62,12 @@ impl From<Transaction> for TransactionData {
                    U256::zero()
                };

+                let should_deploy_contract = if execute_tx.execute.contract_address.is_none() {


We should do it only if EVM-eq is enabled

We can add the check to ensure it only happens with EVM equivalence enabled, but it might not be necessary due to the restriction added here. Given this, no transaction should have a None contract address unless the EVM is enabled.

slowli

I'd consider this way too dirty for even the local deployment, but that may be just me.

slowli · 2024-09-23T13:46:53Z

core/bin/system-constants-generator/src/utils.rs

+                hash_bytecode(&evm_simulator_bytecode),
+            )
+        } else {
+            (bytecode.clone(), hash)


This is blatantly incorrect; this is not an EVM simulator bytecode / hash. Why can't the EVM bytecode be defined unconditionally correctly as above?

slowli · 2024-09-23T13:48:09Z

core/bin/system-constants-generator/Cargo.toml

+zksync_config.workspace = true
+zksync_env_config.workspace = true


This breaks abstraction boundaries; node configuration is downstream of system constants, not the other way around. If the EVM simulator is loaded unconditionally (which I don't see any harm in, although I'm not a domain expert), it wouldn't be necessary. If this doesn't work for some reason, it would make sense to pass the "EVM simulator enabled" flag directly instead of parsing it from env.

slowli · 2024-09-23T13:49:58Z

core/lib/config/src/configs/chain.rs

+    #[deprecated(note = "Use GenesisConfig::evm_simulator_hash instead")]
+    pub evm_simulator_hash: Option<H256>,


??? Why create an immediately deprecated field?

It seems to be deprecated for other libraries to access the hashes of the base system contracts, but it is still used to create the GenesisConfig, which is loaded when running a zk server.

slowli · 2024-09-23T13:51:01Z

core/lib/config/src/configs/use_evm_simulator.rs

@@ -0,0 +1,7 @@
+use serde::Deserialize;
+
+/// Configuration for the use evm simulator


Nit: I'm (obviously) not a native English speaker, but this doesn't look parseable.

slowli · 2024-09-23T13:52:53Z

core/lib/contracts/Cargo.toml

+zksync_config.workspace = true
+zksync_env_config.workspace = true


Same concern as for the system constants generator: this breaks abstraction boundaries and doesn't work in the general case (there's no guarantee that code from this crate will necessarily execute in the node context).

slowli · 2024-09-23T14:11:44Z

core/lib/multivm/src/versions/vm_latest/tracers/evm_deploy_tracer.rs

+            .unwrap()
+            .decode_input(data)
+        else {
+            // Not interested


Why is ignoring an error OK here?

slowli · 2024-09-23T14:16:54Z

core/lib/types/src/transaction_request.rs

+        let use_evm_simulator = use_evm_simulator::UseEvmSimulator::from_env()
+            .unwrap()
+            .use_evm_simulator;


Please use a dedicated argument provided to this method instead.

slowli · 2024-09-23T14:17:53Z

core/node/state_keeper/src/updates/l2_block_updates.rs

+        new_known_factory_deps
+            .into_iter()
+            .for_each(|(hash, bytecode)| {
+                tx_factory_deps.insert(hash, bytecode);
+            });


Nit: This is not JavaScript; please use a for cycle, or tx_factory_deps.extend(_).

slowli · 2024-09-23T14:19:43Z

core/lib/multivm/src/glue/types/vm/vm_block_result.rs

@@ -227,6 +230,7 @@ impl GlueFrom<crate::vm_1_3_2::vm_instance::VmBlockResult>
                circuit_statistic: Default::default(),
            },
            refunds: Refunds::default(),
+            new_known_factory_deps: Default::default(),


Please don't use Default::default() instead of None; this is overly verbose and unnecessarily opaque.

slowli · 2024-09-23T14:20:35Z

core/lib/vm_interface/src/types/outputs/execution_result.rs

@@ -118,6 +118,7 @@ pub struct VmExecutionResultAndLogs {
    pub logs: VmExecutionLogs,
    pub statistics: VmExecutionStatistics,
    pub refunds: Refunds,
+    pub new_known_factory_deps: Option<Vec<(H256, Vec<u8>)>>,


Dumb question: What does this field mean? Could you explain in a doc comment for this field what it's necessary for?

IAvecilla · 2024-09-25T18:42:47Z

Close in favor of #2966

IAvecilla and others added 29 commits September 11, 2024 12:48

Make address optional in transaction data

bbd7db9

Add migrations and update queries with evm simulator

4e6bed2

Add evm simulator as base system contract and update callers

723c5e3

Add ts tests and contracts for evm simulator

3a3a300

Update multivm bootloaders bin files for vm1_5_0

8132132

Update lock file

6cae61a

Update contracts submodule

dfef5ea

Improve get contract address for sql queries

ddc35de

Merge branch 'main' into tx-to-optional-address

cf454e2

Make recipient address optional for L2Tx

a5c22ea

Fix error lints

c075fb4

Insert null in sql for contract address instead of empty bytes

6c94de9

Remove require and support None value for protobuff Transaction

b280eb3

Make address optional for Transaction recipient account

d49f3f8

Optionally add contract address to transaction signature

5e9179d

Check address is not null and reject transaction

0034e3e

Merge branch 'main' into tx-to-optional-address

d9e2b9a

Make test only Execute to always have a valid address instead of null

9c3dad1

Update execute use for tests in multivm crate to have a valid contrac…

9652368

…t address

Update comment for optional field in execute proto message

189e591

Unwrap contract address for l1 tx since it should always be present

2ac399e

Merge branch 'main' into tx-to-optional-address

6be103f

Merge branch 'tx-to-optional-address' into evm-simulator-base-sc

9700ff3

Remove tests for the evm simulator

be77938

Merge remote-tracking branch 'upstream/main' into evm-simulator-base-sc

f8816cf

Delete contracts and aux functions used for evm tests

7608ace

Fix fmt

ed97d24

Revert multivm bootloader change

2388ae8

github-actions bot added the external-contribution label Sep 19, 2024

IAvecilla added 3 commits September 19, 2024 14:52

Check for evm simulator to avoid failing for transactions with null c…

00f0324

…ontract address

Fix multicall with evm simulator on

c14b5aa

Fix multicall when evm simulator is absent

c8d1896

0xVolosnikov reviewed Sep 20, 2024

View reviewed changes

core/bin/system-constants-generator/src/utils.rs Outdated Show resolved Hide resolved

0xVolosnikov reviewed Sep 20, 2024

View reviewed changes

core/bin/system-constants-generator/src/utils.rs Outdated Show resolved Hide resolved

0xVolosnikov reviewed Sep 20, 2024

View reviewed changes

core/lib/contracts/src/lib.rs Outdated Show resolved Hide resolved

0xVolosnikov reviewed Sep 20, 2024

View reviewed changes

IAvecilla added 3 commits September 20, 2024 13:16

Remove hardcoded hash for evm simulator when its not enabled

072222d

Fix format

a08ea6c

Check for evm simulator in execute_internal_transfer_test function

5bdabe3

slowli requested changes Sep 23, 2024

View reviewed changes

IAvecilla added 10 commits September 24, 2024 11:24

Merge branch 'main' into evm-simulator-base-sc

1385cb6

Fix doc comment for UseEvmSimulator config

9010c56

Remove evm simulator code hash from miniblocks table

774b848

Fix review comments

e85228b

Update sqlx cached queries

0f7c5da

Fix lints and format

b5c7544

Merge branch 'main' into evm-simulator-base-sc

9b0590d

Add evm simulator config to load config function

653b543

Add evm simulator to general test config and protobuf message

d2d61a4

Add use evm simulator config to general config file

44e714b

IAvecilla closed this Sep 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Evm Equivalence #2929

feat: Evm Equivalence #2929

jrchatruc commented Sep 19, 2024 •

edited

Loading

0xVolosnikov Sep 20, 2024

IAvecilla Sep 20, 2024

0xVolosnikov Sep 20, 2024

IAvecilla Sep 20, 2024

slowli left a comment

slowli Sep 23, 2024

slowli Sep 23, 2024

slowli Sep 23, 2024

IAvecilla Sep 24, 2024

slowli Sep 23, 2024

IAvecilla Sep 24, 2024

slowli Sep 23, 2024

slowli Sep 23, 2024

slowli Sep 23, 2024

slowli Sep 23, 2024

slowli Sep 23, 2024

slowli Sep 23, 2024

IAvecilla commented Sep 25, 2024

		zksync_config.workspace = true
		zksync_env_config.workspace = true

		#[deprecated(note = "Use GenesisConfig::evm_simulator_hash instead")]
		pub evm_simulator_hash: Option<H256>,

		@@ -0,0 +1,7 @@
		use serde::Deserialize;

		/// Configuration for the use evm simulator

feat: Evm Equivalence #2929

feat: Evm Equivalence #2929

Conversation

jrchatruc commented Sep 19, 2024 • edited Loading

What ❔

Why ❔

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slowli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IAvecilla commented Sep 25, 2024

jrchatruc commented Sep 19, 2024 •

edited

Loading