Fix review comments

matter-labs · Sep 24, 2024 · e85228b · e85228b
1 parent 774b848
commit e85228b
Show file tree

Hide file tree

Showing 20 changed files with 41 additions and 41 deletions.
diff --git a/core/lib/dal/src/models/storage_transaction.rs b/core/lib/dal/src/models/storage_transaction.rs
@@ -367,7 +367,8 @@ impl From<StorageTransactionReceipt> for TransactionReceipt {
                 .and_then(|addr| {
                     serde_json::from_value::<Option<Address>>(addr)
                         .expect("invalid address value in the database")
-                }),
+                // For better compatibility with various clients, we never return null.
+                }).or_else(|| Some(Address::zero())),
             cumulative_gas_used: Default::default(), // TODO: Should be actually calculated (SMA-1183).
             gas_used: {
                 let refunded_gas: U256 = storage_receipt.refunded_gas.into();

diff --git a/core/lib/multivm/src/glue/types/vm/vm_block_result.rs b/core/lib/multivm/src/glue/types/vm/vm_block_result.rs
@@ -47,7 +47,7 @@ impl GlueFrom<crate::vm_m5::vm_instance::VmBlockResult> for crate::interface::Fi
                     circuit_statistic: Default::default(),
                 },
                 refunds: Refunds::default(),
-                new_known_factory_deps: Default::default(),
+                new_known_factory_deps: None,
             },
             final_execution_state: CurrentExecutionState {
                 events: value.full_result.events,
@@ -104,7 +104,7 @@ impl GlueFrom<crate::vm_m6::vm_instance::VmBlockResult> for crate::interface::Fi
                     circuit_statistic: Default::default(),
                 },
                 refunds: Refunds::default(),
-                new_known_factory_deps: Default::default(),
+                new_known_factory_deps: None,
             },
             final_execution_state: CurrentExecutionState {
                 events: value.full_result.events,
@@ -160,7 +160,7 @@ impl GlueFrom<crate::vm_1_3_2::vm_instance::VmBlockResult> for crate::interface:
                     circuit_statistic: Default::default(),
                 },
                 refunds: Refunds::default(),
-                new_known_factory_deps: Default::default(),
+                new_known_factory_deps: None,
             },
             final_execution_state: CurrentExecutionState {
                 events: value.full_result.events,
@@ -230,7 +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(),
+            new_known_factory_deps: None,
         }
     }
 }
@@ -263,7 +263,7 @@ impl GlueFrom<crate::vm_m5::vm_instance::VmBlockResult>
                 circuit_statistic: Default::default(),
             },
             refunds: Refunds::default(),
-            new_known_factory_deps: Default::default(),
+            new_known_factory_deps: None,
         }
     }
 }
@@ -312,7 +312,7 @@ impl GlueFrom<crate::vm_m6::vm_instance::VmBlockResult>
                 circuit_statistic: Default::default(),
             },
             refunds: Refunds::default(),
-            new_known_factory_deps: Default::default(),
+            new_known_factory_deps: None,
         }
     }
 }
diff --git a/core/lib/multivm/src/glue/types/vm/vm_partial_execution_result.rs b/core/lib/multivm/src/glue/types/vm/vm_partial_execution_result.rs
@@ -22,7 +22,7 @@ impl GlueFrom<crate::vm_m5::vm_instance::VmPartialExecutionResult>
                 gas_refunded: 0,
                 operator_suggested_refund: 0,
             },
-            new_known_factory_deps: Default::default(),
+            new_known_factory_deps: None,
         }
     }
 }
@@ -49,7 +49,7 @@ impl GlueFrom<crate::vm_m6::vm_instance::VmPartialExecutionResult>
                 gas_refunded: 0,
                 operator_suggested_refund: 0,
             },
-            new_known_factory_deps: Default::default(),
+            new_known_factory_deps: None,
         }
     }
 }
@@ -76,7 +76,7 @@ impl GlueFrom<crate::vm_1_3_2::vm_instance::VmPartialExecutionResult>
                 gas_refunded: 0,
                 operator_suggested_refund: 0,
             },
-            new_known_factory_deps: Default::default(),
+            new_known_factory_deps: None,
         }
     }
 }

diff --git a/core/lib/multivm/src/glue/types/vm/vm_tx_execution_result.rs b/core/lib/multivm/src/glue/types/vm/vm_tx_execution_result.rs
@@ -66,14 +66,14 @@ impl GlueFrom<Result<crate::vm_m6::vm_instance::VmTxExecutionResult, crate::vm_m
                         logs: Default::default(),
                         statistics: Default::default(),
                         refunds: Default::default(),
-                        new_known_factory_deps: Default::default(),
+                        new_known_factory_deps: None,
                     },
                     TxRevertReason::Halt(halt) => VmExecutionResultAndLogs {
                         result: ExecutionResult::Halt { reason: halt },
                         logs: Default::default(),
                         statistics: Default::default(),
                         refunds: Default::default(),
-                        new_known_factory_deps: Default::default(),
+                        new_known_factory_deps: None,
                     },
                 }
             }
@@ -102,14 +102,14 @@ impl
                         logs: Default::default(),
                         statistics: Default::default(),
                         refunds: Default::default(),
-                        new_known_factory_deps: Default::default(),
+                        new_known_factory_deps: None,
                     },
                     TxRevertReason::Halt(halt) => VmExecutionResultAndLogs {
                         result: ExecutionResult::Halt { reason: halt },
                         logs: Default::default(),
                         statistics: Default::default(),
                         refunds: Default::default(),
-                        new_known_factory_deps: Default::default(),
+                        new_known_factory_deps: None,
                     },
                 }
             }
@@ -133,7 +133,7 @@ impl GlueFrom<Result<crate::vm_m5::vm_instance::VmTxExecutionResult, crate::vm_m
                         logs: Default::default(),
                         statistics: Default::default(),
                         refunds: Default::default(),
-                        new_known_factory_deps: Default::default(),
+                        new_known_factory_deps: None,
                     },
                     _ => {
                         unreachable!("Halt is the only revert reason for VM 5")

diff --git a/core/lib/multivm/src/versions/vm_1_4_1/implementation/execution.rs b/core/lib/multivm/src/versions/vm_1_4_1/implementation/execution.rs
@@ -99,7 +99,7 @@ impl<S: WriteStorage, H: HistoryMode> Vm<S, H> {
             logs,
             statistics,
             refunds,
-            new_known_factory_deps: Default::default(),
+            new_known_factory_deps: None,
         };
 
         (stop_reason, result)

diff --git a/core/lib/multivm/src/versions/vm_1_4_2/implementation/execution.rs b/core/lib/multivm/src/versions/vm_1_4_2/implementation/execution.rs
@@ -96,7 +96,7 @@ impl<S: WriteStorage, H: HistoryMode> Vm<S, H> {
             logs,
             statistics,
             refunds,
-            new_known_factory_deps: Default::default(),
+            new_known_factory_deps: None,
         };
 
         (stop_reason, result)

diff --git a/core/lib/multivm/src/versions/vm_boojum_integration/implementation/execution.rs b/core/lib/multivm/src/versions/vm_boojum_integration/implementation/execution.rs
@@ -93,7 +93,7 @@ impl<S: WriteStorage, H: HistoryMode> Vm<S, H> {
             logs,
             statistics,
             refunds,
-            new_known_factory_deps: Default::default(),
+            new_known_factory_deps: None,
         };
 
         (stop_reason, result)

diff --git a/core/lib/multivm/src/versions/vm_fast/vm.rs b/core/lib/multivm/src/versions/vm_fast/vm.rs
@@ -601,7 +601,7 @@ impl<S: ReadStorage, Tr: Tracer + Default + 'static> VmInterface for Vm<S, Tr> {
                 circuit_statistic: full_tracer.1.circuit_statistic(),
             },
             refunds,
-            new_known_factory_deps: Default::default(),
+            new_known_factory_deps: None,
         }
     }
 

diff --git a/core/lib/multivm/src/versions/vm_refunds_enhancement/implementation/execution.rs b/core/lib/multivm/src/versions/vm_refunds_enhancement/implementation/execution.rs
@@ -90,7 +90,7 @@ impl<S: WriteStorage, H: HistoryMode> Vm<S, H> {
             logs,
             statistics,
             refunds,
-            new_known_factory_deps: Default::default(),
+            new_known_factory_deps: None,
         };
 
         (stop_reason, result)

diff --git a/core/lib/multivm/src/versions/vm_virtual_blocks/implementation/execution.rs b/core/lib/multivm/src/versions/vm_virtual_blocks/implementation/execution.rs
@@ -88,7 +88,7 @@ impl<S: WriteStorage, H: HistoryMode> Vm<S, H> {
                 .refund_tracer
                 .map(|r| r.get_refunds())
                 .unwrap_or_default(),
-            new_known_factory_deps: Default::default(),
+            new_known_factory_deps: None,
         };
 
         tx_tracer.dispatcher.save_results(&mut result);

diff --git a/core/lib/types/src/l2/mod.rs b/core/lib/types/src/l2/mod.rs
@@ -4,7 +4,9 @@ use anyhow::Context as _;
 use num_enum::TryFromPrimitive;
 use rlp::Rlp;
 use serde::{Deserialize, Serialize};
+use zksync_config::configs::use_evm_simulator;
 use zksync_crypto_primitives::K256PrivateKey;
+use zksync_env_config::FromEnv;
 
 use self::error::SignError;
 use crate::{
@@ -216,7 +218,8 @@ impl L2Tx {
         let raw = req.get_signed_bytes(&sig).context("get_signed_bytes")?;
         let (req, hash) =
             TransactionRequest::from_bytes_unverified(&raw).context("from_bytes_unverified()")?;
-        let mut tx = L2Tx::from_request_unverified(req).context("from_request_unverified()")?;
+        let use_evm_simulator = use_evm_simulator::UseEvmSimulator::from_env().unwrap().use_evm_simulator;
+        let mut tx = L2Tx::from_request_unverified(req, use_evm_simulator).context("from_request_unverified()")?;
         tx.set_input(raw, hash);
         Ok(tx)
     }

diff --git a/core/lib/types/src/lib.rs b/core/lib/types/src/lib.rs
@@ -16,7 +16,9 @@ use serde::{Deserialize, Serialize};
 pub use storage::*;
 pub use tx::Execute;
 pub use zksync_basic_types::{protocol_version::ProtocolVersionId, vm, *};
+use zksync_config::configs::use_evm_simulator;
 pub use zksync_crypto_primitives::*;
+use zksync_env_config::FromEnv;
 use zksync_utils::{
     address_to_u256, bytecode::hash_bytecode, h256_to_u256, u256_to_account_address,
 };
@@ -389,7 +391,8 @@ impl TryFrom<abi::Transaction> for Transaction {
             abi::Transaction::L2(raw) => {
                 let (req, hash) =
                     transaction_request::TransactionRequest::from_bytes_unverified(&raw)?;
-                let mut tx = L2Tx::from_request_unverified(req)?;
+                let use_evm_simulator = use_evm_simulator::UseEvmSimulator::from_env().unwrap().use_evm_simulator;
+                let mut tx = L2Tx::from_request_unverified(req, use_evm_simulator)?;
                 tx.set_input(raw, hash);
                 tx.into()
             }

diff --git a/core/lib/types/src/transaction_request.rs b/core/lib/types/src/transaction_request.rs
@@ -811,6 +811,7 @@ impl TransactionRequest {
 impl L2Tx {
     pub(crate) fn from_request_unverified(
         mut value: TransactionRequest,
+        use_evm_simulator: bool,
     ) -> Result<Self, SerializationTransactionError> {
         let fee = value.get_fee_data_checked()?;
         let nonce = value.get_nonce_checked()?;
@@ -819,10 +820,6 @@ impl L2Tx {
         let meta = value.eip712_meta.take().unwrap_or_default();
         validate_factory_deps(&meta.factory_deps)?;
 
-        let use_evm_simulator = use_evm_simulator::UseEvmSimulator::from_env()
-            .unwrap()
-            .use_evm_simulator;
-
         // TODO: Remove this check when evm equivalence gets enabled
         if value.to.is_none() && !use_evm_simulator {
             return Err(SerializationTransactionError::ToAddressIsNull);
@@ -858,7 +855,8 @@ impl L2Tx {
         value: TransactionRequest,
         max_tx_size: usize,
     ) -> Result<Self, SerializationTransactionError> {
-        let tx = Self::from_request_unverified(value)?;
+        let use_evm_simulator = use_evm_simulator::UseEvmSimulator::from_env().unwrap().use_evm_simulator;
+        let tx = Self::from_request_unverified(value, use_evm_simulator)?;
         tx.check_encoded_size(max_tx_size)?;
         Ok(tx)
     }

diff --git a/core/lib/vm_executor/src/oneshot/mock.rs b/core/lib/vm_executor/src/oneshot/mock.rs
@@ -68,7 +68,7 @@ impl MockOneshotExecutor {
                     logs: Default::default(),
                     statistics: Default::default(),
                     refunds: Default::default(),
-                    new_known_factory_deps: Default::default(),
+                    new_known_factory_deps: None,
                 }
             },
         )

diff --git a/core/lib/vm_interface/src/types/outputs/finished_l1batch.rs b/core/lib/vm_interface/src/types/outputs/finished_l1batch.rs
@@ -26,7 +26,7 @@ impl FinishedL1Batch {
                 logs: VmExecutionLogs::default(),
                 statistics: VmExecutionStatistics::default(),
                 refunds: Refunds::default(),
-                new_known_factory_deps: Default::default(),
+                new_known_factory_deps: None,
             },
             final_execution_state: CurrentExecutionState {
                 events: vec![],

diff --git a/core/node/api_server/src/web3/tests/vm.rs b/core/node/api_server/src/web3/tests/vm.rs
@@ -393,7 +393,7 @@ impl HttpTest for SendTransactionWithDetailedOutputTest {
                 logs: vm_execution_logs.clone(),
                 statistics: Default::default(),
                 refunds: Default::default(),
-                new_known_factory_deps: Default::default(),
+                new_known_factory_deps: None,
             }
         });
         tx_executor

diff --git a/core/node/state_keeper/src/testonly/mod.rs b/core/node/state_keeper/src/testonly/mod.rs
@@ -33,7 +33,7 @@ pub(crate) fn successful_exec() -> BatchTransactionExecutionResult {
             logs: Default::default(),
             statistics: Default::default(),
             refunds: Default::default(),
-            new_known_factory_deps: Default::default(),
+            new_known_factory_deps: None,
         }),
         compressed_bytecodes: vec![],
         call_traces: vec![],

diff --git a/core/node/state_keeper/src/testonly/test_batch_executor.rs b/core/node/state_keeper/src/testonly/test_batch_executor.rs
@@ -264,7 +264,7 @@ pub(crate) fn successful_exec_with_log() -> BatchTransactionExecutionResult {
             },
             statistics: Default::default(),
             refunds: Default::default(),
-            new_known_factory_deps: Default::default(),
+            new_known_factory_deps: None,
         }),
         compressed_bytecodes: vec![],
         call_traces: vec![],
@@ -279,7 +279,7 @@ pub(crate) fn rejected_exec(reason: Halt) -> BatchTransactionExecutionResult {
             logs: Default::default(),
             statistics: Default::default(),
             refunds: Default::default(),
-            new_known_factory_deps: Default::default(),
+            new_known_factory_deps: None,
         }),
         compressed_bytecodes: vec![],
         call_traces: vec![],

diff --git a/core/node/state_keeper/src/tests/mod.rs b/core/node/state_keeper/src/tests/mod.rs
@@ -138,7 +138,7 @@ pub(super) fn create_execution_result(
             circuit_statistic: Default::default(),
         },
         refunds: Refunds::default(),
-        new_known_factory_deps: Default::default(),
+        new_known_factory_deps: None,
     }
 }
 

diff --git a/core/node/state_keeper/src/updates/l2_block_updates.rs b/core/node/state_keeper/src/updates/l2_block_updates.rs
@@ -129,12 +129,7 @@ impl L2BlockUpdates {
             .map(|bytecode| (hash_bytecode(bytecode), bytecode.clone()))
             .collect();
 
-        new_known_factory_deps
-            .into_iter()
-            .for_each(|(hash, bytecode)| {
-                tx_factory_deps.insert(hash, bytecode);
-            });
-
+        tx_factory_deps.extend(new_known_factory_deps.clone());
         // Save all bytecodes that were marked as known on the bootloader
         let known_bytecodes = saved_factory_deps.into_iter().map(|bytecode_hash| {
             let bytecode = tx_factory_deps.get(&bytecode_hash).unwrap_or_else(|| {