feat(en): Check recipient contract and function selector in consisten…

…cy checker (#1367)

## What ❔

- Checks the recipient contract (should be equal to the main L1 contract
address) and the Solidity function selector (should correspond to
`commitBlocks` / `commitBatches` selector) in the consistency checker.
- Reworks error handling in the consistency checker to be more
maintainable.

## Why ❔

Not checking these values allows to more-or-less easily fool the checker
into accepting a commit transaction which is not really a commit
transaction.

## Checklist

- [x] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [x] Tests for the changes have been added / updated.
- [x] Documentation comments have been added / updated.
- [x] Code has been formatted via `zk fmt` and `zk lint`.
- [x] Spellcheck has been run via `zk spellcheck`.
- [x] Linkcheck has been run via `zk linkcheck`.

core/bin/external_node/src/config/mod.rs

@@ -237,6 +237,11 @@ pub struct OptionalENConfig {
     /// 0 means that sealing is synchronous; this is mostly useful for performance comparison, testing etc.
     #[serde(default = "OptionalENConfig::default_miniblock_seal_queue_capacity")]
     pub miniblock_seal_queue_capacity: usize,
+    /// Address of the L1 diamond proxy contract used by the consistency checker to match with the origin of logs emitted
+    /// by commit transactions. If not set, it will not be verified.
+    // This is intentionally not a part of `RemoteENConfig` because fetching this info from the main node would defeat
+    // its purpose; the consistency checker assumes that the main node may provide false information.
+    pub contracts_diamond_proxy_addr: Option<Address>,
 }
 
 impl OptionalENConfig {

core/bin/external_node/src/main.rs

@@ -237,6 +237,21 @@ async fn init_tasks(
         .context("failed initializing metadata calculator")?;
     app_health.insert_component(metadata_calculator.tree_health_check());
 
+    let remote_diamond_proxy_addr = config.remote.diamond_proxy_addr;
+    let diamond_proxy_addr = if let Some(addr) = config.optional.contracts_diamond_proxy_addr {
+        anyhow::ensure!(
+            addr == remote_diamond_proxy_addr,
+            "Diamond proxy address {addr:?} specified in config doesn't match one returned by main node \
+             ({remote_diamond_proxy_addr:?})"
+        );
+        addr
+    } else {
+        tracing::info!(
+            "Diamond proxy address is not specified in config; will use address returned by main node: {remote_diamond_proxy_addr:?}"
+        );
+        remote_diamond_proxy_addr
+    };
+
     let consistency_checker = ConsistencyChecker::new(
         &config
             .required
@@ -247,7 +262,10 @@ async fn init_tasks(
             .build()
             .await
             .context("failed to build connection pool for ConsistencyChecker")?,
-    );
+    )
+    .context("cannot initialize consistency checker")?
+    .with_diamond_proxy_addr(diamond_proxy_addr);
+
     app_health.insert_component(consistency_checker.health_check().clone());
     let consistency_checker_handle = tokio::spawn(consistency_checker.run(stop_receiver.clone()));
 

core/lib/eth_client/src/clients/mock.rs

@@ -1,6 +1,7 @@
 use std::{
     collections::{BTreeMap, HashMap},
-    sync::RwLock,
+    fmt,
+    sync::{RwLock, RwLockWriteGuard},
 };
 
 use async_trait::async_trait;
@@ -12,7 +13,7 @@ use zksync_types::{
         types::{BlockId, BlockNumber, Filter, Log, Transaction, TransactionReceipt, U64},
         Error as Web3Error,
     },
-    Address, L1ChainId, ProtocolVersionId, H160, H256, U256,
+    Address, L1ChainId, H160, H256, U256,
 };
 
 use crate::{
@@ -22,6 +23,7 @@ use crate::{
 
 #[derive(Debug, Clone)]
 struct MockTx {
+    recipient: Address,
     input: Vec<u8>,
     hash: H256,
     nonce: u64,
@@ -32,6 +34,7 @@ struct MockTx {
 impl From<Vec<u8>> for MockTx {
     fn from(tx: Vec<u8>) -> Self {
         let len = tx.len();
+        let recipient = Address::from_slice(&tx[len - 116..len - 96]);
         let max_fee_per_gas = U256::try_from(&tx[len - 96..len - 64]).unwrap();
         let max_priority_fee_per_gas = U256::try_from(&tx[len - 64..len - 32]).unwrap();
         let nonce = U256::try_from(&tx[len - 32..]).unwrap().as_u64();
@@ -42,7 +45,8 @@ impl From<Vec<u8>> for MockTx {
         };
 
         Self {
-            input: tx[32..len - 96].to_vec(),
+            recipient,
+            input: tx[32..len - 116].to_vec(),
             nonce,
             hash,
             max_fee_per_gas,
@@ -54,6 +58,7 @@ impl From<Vec<u8>> for MockTx {
 impl From<MockTx> for Transaction {
     fn from(tx: MockTx) -> Self {
         Self {
+            to: Some(tx.recipient),
             input: tx.input.into(),
             hash: tx.hash,
             nonce: tx.nonce.into(),
@@ -112,8 +117,21 @@ impl MockEthereumInner {
     }
 }
 
-/// Mock Ethereum client is capable of recording all the incoming requests for the further analysis.
 #[derive(Debug)]
+pub struct MockExecutedTxHandle<'a> {
+    inner: RwLockWriteGuard<'a, MockEthereumInner>,
+    tx_hash: H256,
+}
+
+impl MockExecutedTxHandle<'_> {
+    pub fn with_logs(&mut self, logs: Vec<Log>) -> &mut Self {
+        let status = self.inner.tx_statuses.get_mut(&self.tx_hash).unwrap();
+        status.receipt.logs = logs;
+        self
+    }
+}
+
+/// Mock Ethereum client is capable of recording all the incoming requests for the further analysis.
 pub struct MockEthereum {
     max_fee_per_gas: U256,
     max_priority_fee_per_gas: U256,
@@ -122,8 +140,25 @@ pub struct MockEthereum {
     /// If true, the mock will not check the ordering nonces of the transactions.
     /// This is useful for testing the cases when the transactions are executed out of order.
     non_ordering_confirmations: bool,
-    multicall_address: Address,
     inner: RwLock<MockEthereumInner>,
+    call_handler: Box<dyn Fn(&ContractCall) -> ethabi::Token + Send + Sync>,
+}
+
+impl fmt::Debug for MockEthereum {
+    fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+        formatter
+            .debug_struct("MockEthereum")
+            .field("max_fee_per_gas", &self.max_fee_per_gas)
+            .field("max_priority_fee_per_gas", &self.max_priority_fee_per_gas)
+            .field("base_fee_history", &self.base_fee_history)
+            .field("excess_blob_gas_history", &self.excess_blob_gas_history)
+            .field(
+                "non_ordering_confirmations",
+                &self.non_ordering_confirmations,
+            )
+            .field("inner", &self.inner)
+            .finish_non_exhaustive()
+    }
 }
 
 impl Default for MockEthereum {
@@ -134,8 +169,10 @@ impl Default for MockEthereum {
             base_fee_history: vec![],
             excess_blob_gas_history: vec![],
             non_ordering_confirmations: false,
-            multicall_address: Address::default(),
             inner: RwLock::default(),
+            call_handler: Box::new(|call| {
+                panic!("Unexpected eth_call: {call:?}");
+            }),
         }
     }
 }
@@ -159,18 +196,26 @@ impl MockEthereum {
 
     /// Increments the blocks by a provided `confirmations` and marks the sent transaction
     /// as a success.
-    pub fn execute_tx(&self, tx_hash: H256, success: bool, confirmations: u64) {
-        self.inner.write().unwrap().execute_tx(
+    pub fn execute_tx(
+        &self,
+        tx_hash: H256,
+        success: bool,
+        confirmations: u64,
+    ) -> MockExecutedTxHandle<'_> {
+        let mut inner = self.inner.write().unwrap();
+        inner.execute_tx(
             tx_hash,
             success,
             confirmations,
             self.non_ordering_confirmations,
         );
+        MockExecutedTxHandle { inner, tx_hash }
     }
 
     pub fn sign_prepared_tx(
         &self,
         mut raw_tx: Vec<u8>,
+        contract_addr: Address,
         options: Options,
     ) -> Result<SignedCallResult, Error> {
         let max_fee_per_gas = options.max_fee_per_gas.unwrap_or(self.max_fee_per_gas);
@@ -181,9 +226,10 @@ impl MockEthereum {
 
         // Nonce and `gas_price` are appended to distinguish the same transactions
         // with different gas by their hash in tests.
-        raw_tx.append(&mut ethabi::encode(&max_fee_per_gas.into_tokens()));
-        raw_tx.append(&mut ethabi::encode(&max_priority_fee_per_gas.into_tokens()));
-        raw_tx.append(&mut ethabi::encode(&nonce.into_tokens()));
+        raw_tx.extend_from_slice(contract_addr.as_bytes());
+        raw_tx.extend_from_slice(&ethabi::encode(&max_fee_per_gas.into_tokens()));
+        raw_tx.extend_from_slice(&ethabi::encode(&max_priority_fee_per_gas.into_tokens()));
+        raw_tx.extend_from_slice(&ethabi::encode(&nonce.into_tokens()));
         let hash = Self::fake_sha256(&raw_tx); // Okay for test purposes.
 
         // Concatenate `raw_tx` plus hash for test purposes
@@ -225,9 +271,12 @@ impl MockEthereum {
         }
     }
 
-    pub fn with_multicall_address(self, address: Address) -> Self {
+    pub fn with_call_handler<F>(self, call_handler: F) -> Self
+    where
+        F: 'static + Send + Sync + Fn(&ContractCall) -> ethabi::Token,
+    {
         Self {
-            multicall_address: address,
+            call_handler: Box::new(call_handler),
             ..self
         }
     }
@@ -312,26 +361,8 @@ impl EthInterface for MockEthereum {
         &self,
         call: ContractCall,
     ) -> Result<Vec<ethabi::Token>, Error> {
-        use ethabi::Token;
-
-        if call.contract_address == self.multicall_address {
-            let token = Token::Array(vec![
-                Token::Tuple(vec![Token::Bool(true), Token::Bytes(vec![1u8; 32])]),
-                Token::Tuple(vec![Token::Bool(true), Token::Bytes(vec![2u8; 32])]),
-                Token::Tuple(vec![Token::Bool(true), Token::Bytes(vec![3u8; 96])]),
-                Token::Tuple(vec![Token::Bool(true), Token::Bytes(vec![4u8; 32])]),
-                Token::Tuple(vec![
-                    Token::Bool(true),
-                    Token::Bytes(
-                        H256::from_low_u64_be(ProtocolVersionId::default() as u64)
-                            .0
-                            .to_vec(),
-                    ),
-                ]),
-            ]);
-            return Ok(vec![token]);
-        }
-        Ok(vec![])
+        let response = (self.call_handler)(&call);
+        Ok(vec![response])
     }
 
     async fn get_tx(
@@ -415,11 +446,11 @@ impl BoundEthInterface for MockEthereum {
     async fn sign_prepared_tx_for_addr(
         &self,
         data: Vec<u8>,
-        _contract_addr: H160,
+        contract_addr: H160,
         options: Options,
         _component: &'static str,
     ) -> Result<SignedCallResult, Error> {
-        self.sign_prepared_tx(data, options)
+        self.sign_prepared_tx(data, contract_addr, options)
     }
 
     async fn allowance_on_account(
@@ -474,6 +505,7 @@ mod tests {
         let signed_tx = client
             .sign_prepared_tx(
                 b"test".to_vec(),
+                Address::repeat_byte(1),
                 Options {
                     nonce: Some(1.into()),
                     ..Default::default()
@@ -494,6 +526,7 @@ mod tests {
             .unwrap()
             .expect("no transaction");
         assert_eq!(returned_tx.hash, tx_hash);
+        assert_eq!(returned_tx.to, Some(Address::repeat_byte(1)));
         assert_eq!(returned_tx.input.0, b"test");
         assert_eq!(returned_tx.nonce, 1.into());
         assert!(returned_tx.max_priority_fee_per_gas.is_some());

core/lib/eth_client/src/types.rs

@@ -83,6 +83,20 @@ pub struct ContractCall {
     pub(crate) inner: CallFunctionArgs,
 }
 
+impl ContractCall {
+    pub fn contract_address(&self) -> Address {
+        self.contract_address
+    }
+
+    pub fn function_name(&self) -> &str {
+        &self.inner.name
+    }
+
+    pub fn args(&self) -> &[ethabi::Token] {
+        &self.inner.params.0
+    }
+}
+
 /// Common error type exposed by the crate,
 #[derive(Debug, thiserror::Error)]
 pub enum Error {