fix: hardhat_setCode error handling (#293)

* checking hardhat_setCode arg len; added test

* replaced zksync-era functions implementing hardhat_setCode

to not panic

* silenced warning

* added error output

* prettier indentation

* reformatted

* Update e2e-tests/test/hardhat-apis.test.ts

* passig hardhat_setCode error to client

* added comment to ree-run integration

---------

Co-authored-by: Nicolas Villanueva <nicolasvillanueva@msn.com>

Author: vbar

Cargo.lock

@@ -11,6 +11,7 @@ categories = ["cryptography"]
 publish = false # We don't want to publish our binaries.
 
 [dependencies]
+zkevm_opcode_defs = { git = "https://github.com/matter-labs/era-zkevm_opcode_defs.git", branch = "v1.5.0" }
 zksync_basic_types = { git = "https://github.com/matter-labs/zksync-era.git", rev = "e10bbdd1e863962552f37e768ae6af649353e4ea" }
 zksync_node_fee_model = { git = "https://github.com/matter-labs/zksync-era.git", rev = "e10bbdd1e863962552f37e768ae6af649353e4ea" }
 multivm = { git = "https://github.com/matter-labs/zksync-era.git", rev = "e10bbdd1e863962552f37e768ae6af649353e4ea" }

Cargo.toml

@@ -1,6 +1,6 @@
 import { expect } from "chai";
 import { Wallet } from "zksync-web3";
-import { deployContract, getTestProvider } from "../helpers/utils";
+import { deployContract, expectThrowsAsync, getTestProvider } from "../helpers/utils";
 import { RichAccounts } from "../helpers/constants";
 import { ethers } from "hardhat";
 import { Deployer } from "@matterlabs/hardhat-zksync-deploy";
@@ -118,6 +118,24 @@ describe("hardhat_setCode", function () {
     expect(BigNumber.from(result).toNumber()).to.eq(5);
   });
 
+  it("Should reject invalid code", async function () {
+    const action = async () => {
+      // Arrange
+      const wallet = new Wallet(RichAccounts[0].PrivateKey);
+      const deployer = new Deployer(hre, wallet);
+
+      const address = "0x1000000000000000000000000000000000001111";
+      const artifact = await deployer.loadArtifact("Return5");
+      const contractCode = [...ethers.utils.arrayify(artifact.deployedBytecode)];
+      const shortCode = contractCode.slice(0, contractCode.length - 1);
+
+      // Act
+      await provider.send("hardhat_setCode", [address, shortCode]);
+    };
+
+    await expectThrowsAsync(action, "bytes must be divisible by 32");
+  });
+
   it("Should update code with a different smart contract", async function () {
     // Arrange
     const wallet = new Wallet(RichAccounts[0].PrivateKey);

e2e-tests/test/hardhat-apis.test.ts

@@ -5,7 +5,7 @@ use crate::{
     fork::ForkSource,
     namespaces::{HardhatNamespaceT, RpcResult},
     node::InMemoryNode,
-    utils::{into_jsrpc_error, IntoBoxedFuture},
+    utils::{into_jsrpc_error, into_jsrpc_error_message, IntoBoxedFuture},
 };
 
 impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> HardhatNamespaceT
@@ -60,7 +60,7 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> HardhatNam
         self.set_code(address, code)
             .map_err(|err| {
                 tracing::error!("failed setting code: {:?}", err);
-                into_jsrpc_error(Web3Error::InternalError(err))
+                into_jsrpc_error_message(err.to_string())
             })
             .into_boxed_future()
     }

src/node/hardhat.rs

@@ -1526,11 +1526,17 @@ impl<S: ForkSource + std::fmt::Debug + Clone> InMemoryNode<S> {
             ..Default::default()
         };
 
-        let bytecodes: HashMap<U256, Vec<U256>> = vm
-            .get_last_tx_compressed_bytecodes()
-            .iter()
-            .map(|b| bytecode_to_factory_dep(b.original.clone()))
-            .collect();
+        let mut bytecodes = HashMap::new();
+        for b in vm.get_last_tx_compressed_bytecodes().iter() {
+            let hashcode = match bytecode_to_factory_dep(b.original.clone()) {
+                Ok(hc) => hc,
+                Err(error) => {
+                    tracing::error!("{}", format!("cannot convert bytecode: {}", error).on_red());
+                    return Err(error.to_string());
+                }
+            };
+            bytecodes.insert(hashcode.0, hashcode.1);
+        }
         if execute_bootloader {
             vm.execute(VmExecutionMode::Bootloader);
         }

src/node/in_memory.rs

@@ -96,10 +96,10 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> InMemoryNo
         self.get_inner()
             .write()
             .map_err(|err| anyhow!("failed acquiring lock: {:?}", err))
-            .map(|mut writer| {
-                utils::mine_empty_blocks(&mut writer, 1, 1000);
+            .and_then(|mut writer| {
+                utils::mine_empty_blocks(&mut writer, 1, 1000)?;
                 tracing::info!("👷 Mined block #{}", writer.current_miniblock);
-                "0x0".to_string()
+                Ok("0x0".to_string())
             })
     }
 
@@ -260,7 +260,7 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> InMemoryNo
                         "Number of blocks must be greater than 0".to_string(),
                     ));
                 }
-                utils::mine_empty_blocks(&mut writer, num_blocks.as_u64(), interval_ms.as_u64());
+                utils::mine_empty_blocks(&mut writer, num_blocks.as_u64(), interval_ms.as_u64())?;
                 tracing::info!("👷 Mined {} blocks", num_blocks);
 
                 Ok(true)
@@ -304,22 +304,23 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> InMemoryNo
         self.get_inner()
             .write()
             .map_err(|err| anyhow!("failed acquiring lock: {:?}", err))
-            .map(|mut writer| {
+            .and_then(|mut writer| {
                 let code_key = get_code_key(&address);
                 tracing::info!("set code for address {address:#x}");
-                let (hash, code) = bytecode_to_factory_dep(code);
-                let hash = u256_to_h256(hash);
-                writer.fork_storage.store_factory_dep(
-                    hash,
-                    code.iter()
-                        .flat_map(|entry| {
-                            let mut bytes = vec![0u8; 32];
-                            entry.to_big_endian(&mut bytes);
-                            bytes.to_vec()
-                        })
-                        .collect(),
-                );
+                let hashcode = bytecode_to_factory_dep(code)?;
+                let hash = u256_to_h256(hashcode.0);
+                let code = hashcode
+                    .1
+                    .iter()
+                    .flat_map(|entry| {
+                        let mut bytes = vec![0u8; 32];
+                        entry.to_big_endian(&mut bytes);
+                        bytes.to_vec()
+                    })
+                    .collect();
+                writer.fork_storage.store_factory_dep(hash, code);
                 writer.fork_storage.set_value(code_key, hash);
+                Ok(())
             })
     }
 }

src/node/in_memory_ext.rs

@@ -3,20 +3,22 @@ use std::convert::TryInto;
 use std::fmt;
 use std::pin::Pin;
 
+use anyhow::anyhow;
 use chrono::{DateTime, Utc};
 use futures::Future;
 use jsonrpc_core::{Error, ErrorCode};
 use multivm::interface::{ExecutionResult, VmExecutionResultAndLogs, VmInterface};
 use multivm::vm_latest::HistoryDisabled;
 use multivm::vm_latest::Vm;
+use zkevm_opcode_defs::utils::bytecode_to_code_hash;
 use zksync_basic_types::{H256, U256, U64};
 use zksync_state::WriteStorage;
 use zksync_types::api::{BlockNumber, DebugCall, DebugCallType};
 use zksync_types::l2::L2Tx;
 use zksync_types::vm_trace::Call;
 use zksync_types::CONTRACT_DEPLOYER_ADDRESS;
+use zksync_utils::bytes_to_be_words;
 use zksync_utils::u256_to_h256;
-use zksync_utils::{bytecode::hash_bytecode, bytes_to_be_words};
 use zksync_web3_decl::error::Web3Error;
 
 use crate::deps::storage_view::StorageView;
@@ -55,13 +57,36 @@ pub fn to_human_size(input: U256) -> String {
     tmp.iter().rev().collect()
 }
 
-pub fn bytecode_to_factory_dep(bytecode: Vec<u8>) -> (U256, Vec<U256>) {
-    let bytecode_hash = hash_bytecode(&bytecode);
+pub fn bytes_to_chunks(bytes: &[u8]) -> Vec<[u8; 32]> {
+    bytes
+        .chunks(32)
+        .map(|el| {
+            let mut chunk = [0u8; 32];
+            chunk.copy_from_slice(el);
+            chunk
+        })
+        .collect()
+}
+
+pub fn hash_bytecode(code: &[u8]) -> Result<H256, anyhow::Error> {
+    if code.len() % 32 != 0 {
+        return Err(anyhow!("bytes must be divisible by 32"));
+    }
+
+    let chunked_code = bytes_to_chunks(code);
+    match bytecode_to_code_hash(&chunked_code) {
+        Ok(hash) => Ok(H256(hash)),
+        Err(_) => Err(anyhow!("invalid bytecode")),
+    }
+}
+
+pub fn bytecode_to_factory_dep(bytecode: Vec<u8>) -> Result<(U256, Vec<U256>), anyhow::Error> {
+    let bytecode_hash = hash_bytecode(&bytecode)?;
     let bytecode_hash = U256::from_big_endian(bytecode_hash.as_bytes());
 
     let bytecode_words = bytes_to_be_words(bytecode);
 
-    (bytecode_hash, bytecode_words)
+    Ok((bytecode_hash, bytecode_words))
 }
 
 /// Creates and inserts a given number of empty blocks into the node, with a given interval between them.
@@ -72,7 +97,7 @@ pub fn mine_empty_blocks<S: std::fmt::Debug + ForkSource>(
     node: &mut InMemoryNodeInner<S>,
     num_blocks: u64,
     interval_ms: u64,
-) {
+) -> Result<(), anyhow::Error> {
     // build and insert new blocks
     for i in 0..num_blocks {
         // roll the vm
@@ -98,11 +123,11 @@ pub fn mine_empty_blocks<S: std::fmt::Debug + ForkSource>(
 
             vm.execute(multivm::interface::VmExecutionMode::Bootloader);
 
-            let bytecodes: HashMap<U256, Vec<U256>> = vm
-                .get_last_tx_compressed_bytecodes()
-                .iter()
-                .map(|b| bytecode_to_factory_dep(b.original.clone()))
-                .collect();
+            let mut bytecodes = HashMap::new();
+            for b in vm.get_last_tx_compressed_bytecodes().iter() {
+                let hashcode = bytecode_to_factory_dep(b.original.clone())?;
+                bytecodes.insert(hashcode.0, hashcode.1);
+            }
             let modified_keys = storage.borrow().modified_storage_keys().clone();
             (modified_keys, bytecodes, block_ctx)
         };
@@ -140,6 +165,8 @@ pub fn mine_empty_blocks<S: std::fmt::Debug + ForkSource>(
         node.current_miniblock = block_ctx.miniblock;
         node.current_timestamp = block_ctx.timestamp;
     }
+
+    Ok(())
 }
 
 /// Returns the actual [U64] block number from [BlockNumber].
@@ -265,6 +292,14 @@ pub fn into_jsrpc_error(err: Web3Error) -> Error {
     }
 }
 
+pub fn into_jsrpc_error_message(msg: String) -> Error {
+    Error {
+        code: ErrorCode::InternalError,
+        message: msg,
+        data: None,
+    }
+}
+
 pub fn internal_error(method_name: &'static str, error: impl fmt::Display) -> Web3Error {
     tracing::error!("Internal error in method {method_name}: {error}");
     Web3Error::InternalError(anyhow::Error::msg(error.to_string()))
@@ -362,7 +397,7 @@ mod tests {
 
         {
             let mut writer = inner.write().expect("failed acquiring write lock");
-            mine_empty_blocks(&mut writer, 1, 1000);
+            mine_empty_blocks(&mut writer, 1, 1000).unwrap();
         }
 
         let reader = inner.read().expect("failed acquiring reader");
@@ -396,7 +431,7 @@ mod tests {
 
         {
             let mut writer = inner.write().expect("failed acquiring write lock");
-            mine_empty_blocks(&mut writer, 2, 1000);
+            mine_empty_blocks(&mut writer, 2, 1000).unwrap();
         }
 
         let reader = inner.read().expect("failed acquiring reader");
@@ -439,7 +474,7 @@ mod tests {
 
         {
             let mut writer = inner.write().expect("failed acquiring write lock");
-            mine_empty_blocks(&mut writer, 2, 1000);
+            mine_empty_blocks(&mut writer, 2, 1000).unwrap();
         }
 
         {