fix: proper handling of halted txs

e2e-tests-rust/Cargo.toml

@@ -11,7 +11,7 @@ publish = false
 
 [dependencies]
 alloy-zksync = { git = "https://github.com/itegulov/alloy-zksync.git", rev = "e0e54a5ac9e24c1c32c7a8783bbf3e34dde2e218" }
-alloy = { version = "0.6", features = ["full", "rlp", "serde", "sol-types"] }
+alloy = { version = "0.6", features = ["full", "rlp", "serde", "sol-types", "provider-anvil-api"] }
 anyhow = "1.0"
 fs2 = "0.4.3"
 tokio = { version = "1", features = ["time", "rt", "process"] }

e2e-tests-rust/src/lib.rs

@@ -1,6 +1,4 @@
-use alloy::primitives::{Address, TxHash};
-use alloy::providers::{Provider, ProviderCall};
-use alloy::rpc::client::NoParams;
+use alloy::providers::Provider;
 use alloy::transports::Transport;
 use alloy_zksync::network::Zksync;
 
@@ -10,37 +8,11 @@ pub trait EraTestNodeApiProvider<T>: Provider<T, Zksync>
 where
     T: Transport + Clone,
 {
-    fn get_auto_mine(&self) -> ProviderCall<T, NoParams, bool> {
-        self.client().request_noparams("anvil_getAutomine").into()
-    }
+    // Empty but can be extended with custom RPC methods as below
 
-    fn set_auto_mine(&self, enable: bool) -> ProviderCall<T, (bool,), ()> {
-        self.client().request("anvil_setAutomine", (enable,)).into()
-    }
-
-    fn set_interval_mining(&self, seconds: u64) -> ProviderCall<T, (u64,), ()> {
-        self.client()
-            .request("anvil_setIntervalMining", (seconds,))
-            .into()
-    }
-
-    fn drop_transaction(&self, hash: TxHash) -> ProviderCall<T, (TxHash,), Option<TxHash>> {
-        self.client()
-            .request("anvil_dropTransaction", (hash,))
-            .into()
-    }
-
-    fn drop_all_transactions(&self) -> ProviderCall<T, NoParams, ()> {
-        self.client()
-            .request_noparams("anvil_dropAllTransactions")
-            .into()
-    }
-
-    fn remove_pool_transactions(&self, address: Address) -> ProviderCall<T, (Address,), ()> {
-        self.client()
-            .request("anvil_removePoolTransactions", (address,))
-            .into()
-    }
+    // fn get_auto_mine(&self) -> ProviderCall<T, NoParams, bool> {
+    //     self.client().request_noparams("anvil_getAutomine").into()
+    // }
 }
 
 impl<P, T> EraTestNodeApiProvider<T> for P

e2e-tests-rust/tests/lib.rs

@@ -1,18 +1,22 @@
 use alloy::network::{ReceiptResponse, TransactionBuilder};
 use alloy::primitives::{address, Address, U256};
+use alloy::providers::ext::AnvilApi;
 use alloy::providers::{PendingTransaction, PendingTransactionError, Provider, WalletProvider};
+use alloy::signers::local::PrivateKeySigner;
 use alloy::transports::http::{reqwest, Http};
 use alloy_zksync::network::transaction_request::TransactionRequest;
 use alloy_zksync::network::Zksync;
 use alloy_zksync::node_bindings::EraTestNode;
 use alloy_zksync::provider::{zksync_provider, ProviderBuilderExt};
+use alloy_zksync::wallet::ZksyncWallet;
 use era_test_node_e2e_tests::utils::LockedPort;
-use era_test_node_e2e_tests::EraTestNodeApiProvider;
 use std::time::Duration;
 
 async fn init(
     f: impl FnOnce(EraTestNode) -> EraTestNode,
-) -> anyhow::Result<impl Provider<Http<reqwest::Client>, Zksync> + WalletProvider<Zksync> + Clone> {
+) -> anyhow::Result<
+    impl Provider<Http<reqwest::Client>, Zksync> + WalletProvider<Zksync, Wallet = ZksyncWallet> + Clone,
+> {
     let locked_port = LockedPort::acquire_unused().await?;
     let provider = zksync_provider()
         .with_recommended_fillers()
@@ -138,11 +142,11 @@ async fn no_sealing_timeout() -> anyhow::Result<()> {
 async fn dynamic_sealing_mode() -> anyhow::Result<()> {
     // Test that we can successfully switch between different sealing modes
     let provider = init(|node| node.no_mine()).await?;
-    assert_eq!(provider.get_auto_mine().await?, false);
+    assert_eq!(provider.anvil_get_auto_mine().await?, false);
 
     // Enable immediate block sealing
-    provider.set_auto_mine(true).await?;
-    assert_eq!(provider.get_auto_mine().await?, true);
+    provider.anvil_set_auto_mine(true).await?;
+    assert_eq!(provider.anvil_get_auto_mine().await?, true);
 
     // Check that we can finalize transactions now
     let tx = TransactionRequest::default()
@@ -152,8 +156,8 @@ async fn dynamic_sealing_mode() -> anyhow::Result<()> {
     assert!(receipt.status());
 
     // Enable interval block sealing
-    provider.set_interval_mining(3).await?;
-    assert_eq!(provider.get_auto_mine().await?, false);
+    provider.anvil_set_interval_mining(3).await?;
+    assert_eq!(provider.anvil_get_auto_mine().await?, false);
 
     // Check that we can finalize two txs in the same block now
     test_finalize_two_txs_in_the_same_block(
@@ -163,8 +167,8 @@ async fn dynamic_sealing_mode() -> anyhow::Result<()> {
     .await?;
 
     // Disable block sealing entirely
-    provider.set_auto_mine(false).await?;
-    assert_eq!(provider.get_auto_mine().await?, false);
+    provider.anvil_set_auto_mine(false).await?;
+    assert_eq!(provider.anvil_get_auto_mine().await?, false);
 
     // Check that transactions do not get finalized now
     let tx = TransactionRequest::default()
@@ -197,7 +201,9 @@ async fn drop_transaction() -> anyhow::Result<()> {
     let pending_tx1 = provider.send_transaction(tx1).await?.register().await?;
 
     // Drop first
-    provider.drop_transaction(*pending_tx0.tx_hash()).await?;
+    provider
+        .anvil_drop_transaction(*pending_tx0.tx_hash())
+        .await?;
 
     // Assert first never gets finalized but the second one does
     let finalization_result = tokio::time::timeout(Duration::from_secs(4), pending_tx0).await;
@@ -231,7 +237,7 @@ async fn drop_all_transactions() -> anyhow::Result<()> {
     let pending_tx1 = provider.send_transaction(tx1).await?.register().await?;
 
     // Drop all transactions
-    provider.drop_all_transactions().await?;
+    provider.anvil_drop_all_transactions().await?;
 
     // Neither transaction gets finalized
     let finalization_result = tokio::time::timeout(Duration::from_secs(4), pending_tx0).await;
@@ -262,7 +268,9 @@ async fn remove_pool_transactions() -> anyhow::Result<()> {
     let pending_tx1 = provider.send_transaction(tx1).await?.register().await?;
 
     // Drop first
-    provider.remove_pool_transactions(RICH_WALLET0).await?;
+    provider
+        .anvil_remove_pool_transactions(RICH_WALLET0)
+        .await?;
 
     // Assert first never gets finalized but the second one does
     let finalization_result = tokio::time::timeout(Duration::from_secs(4), pending_tx0).await;
@@ -275,3 +283,63 @@ async fn remove_pool_transactions() -> anyhow::Result<()> {
 
     Ok(())
 }
+
+#[tokio::test]
+async fn seal_block_ignoring_halted_transaction() -> anyhow::Result<()> {
+    // Test that we can submit three transactions (1 and 3 are successful, 2 is halting). And then
+    // observe a block that finalizes 1 and 3 while ignoring 2.
+    let mut provider = init(|node| node.block_time(3)).await?;
+    let signer = PrivateKeySigner::random();
+    let random_account = signer.address();
+    provider.wallet_mut().register_signer(signer);
+
+    // Impersonate random account for now so that gas estimation works as expected
+    provider.anvil_impersonate_account(random_account).await?;
+
+    // Submit three transactions
+    let tx0 = TransactionRequest::default()
+        .with_from(RICH_WALLET0)
+        .with_to(address!("d8dA6BF26964aF9D7eEd9e03E53415D37aA96045"))
+        .with_value(U256::from(100));
+    let pending_tx0 = provider.send_transaction(tx0).await?.register().await?;
+    let tx1 = TransactionRequest::default()
+        .with_from(random_account)
+        .with_to(address!("d8dA6BF26964aF9D7eEd9e03E53415D37aA96045"))
+        .with_value(U256::from(100));
+    let pending_tx1 = provider.send_transaction(tx1).await?.register().await?;
+    let tx2 = TransactionRequest::default()
+        .with_from(RICH_WALLET1)
+        .with_to(address!("d8dA6BF26964aF9D7eEd9e03E53415D37aA96045"))
+        .with_value(U256::from(100));
+    let pending_tx2 = provider.send_transaction(tx2).await?.register().await?;
+
+    // Stop impersonating random account so that tx is going to halt
+    provider
+        .anvil_stop_impersonating_account(random_account)
+        .await?;
+
+    // Fetch their receipts
+    let receipt0 = provider
+        .get_transaction_receipt(pending_tx0.await?)
+        .await?
+        .unwrap();
+    assert!(receipt0.status());
+    let receipt2 = provider
+        .get_transaction_receipt(pending_tx2.await?)
+        .await?
+        .unwrap();
+    assert!(receipt2.status());
+
+    // Assert that they are different txs but executed in the same block
+    assert_eq!(receipt0.from(), RICH_WALLET0);
+    assert_eq!(receipt2.from(), RICH_WALLET1);
+    assert_ne!(receipt0.transaction_hash(), receipt2.transaction_hash());
+    assert_eq!(receipt0.block_hash(), receipt2.block_hash());
+    assert_eq!(receipt0.block_number(), receipt2.block_number());
+
+    // Halted transaction never gets finalized
+    let finalization_result = tokio::time::timeout(Duration::from_secs(4), pending_tx1).await;
+    assert!(finalization_result.is_err());
+
+    Ok(())
+}

src/namespaces/anvil.rs

@@ -236,7 +236,7 @@ pub trait AnvilNamespaceT {
     ///
     /// A `BoxFuture` containing a `Result` with a `bool` representing the success of the operation.
     #[rpc(name = "anvil_impersonateAccount")]
-    fn impersonate_account(&self, address: Address) -> RpcResult<bool>;
+    fn impersonate_account(&self, address: Address) -> RpcResult<()>;
 
     /// Use this method to stop impersonating an account after having previously used `anvil_impersonateAccount`
     /// The method returns `true` if the account was being impersonated and `false` otherwise.
@@ -249,7 +249,7 @@ pub trait AnvilNamespaceT {
     ///
     /// A `BoxFuture` containing a `Result` with a `bool` representing the success of the operation.
     #[rpc(name = "anvil_stopImpersonatingAccount")]
-    fn stop_impersonating_account(&self, address: Address) -> RpcResult<bool>;
+    fn stop_impersonating_account(&self, address: Address) -> RpcResult<()>;
 
     /// Modifies the bytecode stored at an account's address.
     ///

src/node/anvil.rs

@@ -193,17 +193,19 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> AnvilNames
             .into_boxed_future()
     }
 
-    fn impersonate_account(&self, address: Address) -> RpcResult<bool> {
+    fn impersonate_account(&self, address: Address) -> RpcResult<()> {
         self.impersonate_account(address)
+            .map(|_| ())
             .map_err(|err| {
                 tracing::error!("failed impersonating account: {:?}", err);
                 into_jsrpc_error(Web3Error::InternalError(err))
             })
             .into_boxed_future()
     }
 
-    fn stop_impersonating_account(&self, address: Address) -> RpcResult<bool> {
+    fn stop_impersonating_account(&self, address: Address) -> RpcResult<()> {
         InMemoryNode::<S>::stop_impersonating_account(self, address)
+            .map(|_| ())
             .map_err(|err| {
                 tracing::error!("failed stopping to impersonate account: {:?}", err);
                 into_jsrpc_error(Web3Error::InternalError(err))

src/node/in_memory.rs

@@ -6,16 +6,17 @@ use std::{
     sync::{Arc, RwLock},
 };
 
-use anyhow::Context as _;
 use colored::Colorize;
 use indexmap::IndexMap;
 use once_cell::sync::OnceCell;
 use zksync_contracts::BaseSystemContracts;
+use zksync_multivm::vm_latest::HistoryEnabled;
 use zksync_multivm::{
     interface::{
         storage::{ReadStorage, StoragePtr, WriteStorage},
         Call, ExecutionResult, InspectExecutionMode, L1BatchEnv, L2Block, L2BlockEnv, SystemEnv,
         TxExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceExt,
+        VmInterfaceHistoryEnabled,
     },
     tracers::CallTracer,
     utils::{
@@ -1770,8 +1771,7 @@ impl<S: ForkSource + std::fmt::Debug + Clone> InMemoryNode<S> {
         let (batch_env, mut block_ctx) = inner.create_l1_batch_env(time, storage.clone());
         drop(inner);
 
-        let mut vm: Vm<_, HistoryDisabled> =
-            Vm::new(batch_env.clone(), system_env, storage.clone());
+        let mut vm: Vm<_, HistoryEnabled> = Vm::new(batch_env.clone(), system_env, storage.clone());
 
         // Compute block hash. Note that the computed block hash here will be different than that in production.
         let tx_hashes = txs.iter().map(|t| t.hash()).collect::<Vec<_>>();
@@ -1780,7 +1780,15 @@ impl<S: ForkSource + std::fmt::Debug + Clone> InMemoryNode<S> {
 
         // Execute transactions and bootloader
         for tx in txs {
-            self.run_l2_tx(tx, &block_ctx, &batch_env, &mut vm)?;
+            // Executing a next transaction means that a previous transaction was either rolled back (in which case its snapshot
+            // was already removed), or that we build on top of it (in which case, it can be removed now).
+            vm.pop_snapshot_no_rollback();
+            // Save pre-execution VM snapshot.
+            vm.make_snapshot();
+            if let Err(e) = self.run_l2_tx(tx, &block_ctx, &batch_env, &mut vm) {
+                tracing::error!("Error while executing transaction: {e}");
+                vm.rollback_to_the_latest_snapshot();
+            }
         }
         vm.execute(InspectExecutionMode::Bootloader);
 
@@ -1796,10 +1804,10 @@ impl<S: ForkSource + std::fmt::Debug + Clone> InMemoryNode<S> {
         let mut transactions = Vec::new();
         let mut tx_results = Vec::new();
         for tx_hash in &tx_hashes {
-            let tx_result = inner
-                .tx_results
-                .get(tx_hash)
-                .context("tx result was not saved after a successful execution")?;
+            let Some(tx_result) = inner.tx_results.get(tx_hash) else {
+                // Skipping halted transaction
+                continue;
+            };
             tx_results.push(&tx_result.info.result);
 
             let mut transaction = zksync_types::api::Transaction::from(tx_result.info.tx.clone());