refactor(l2): l1_watcher make deposits unique

.github/workflows/ci_l2.yaml

@@ -35,4 +35,4 @@ jobs:
         run: |
           cd crates/l2
           cp .env.example .env
-          make test
+          make ci_test

crates/common/types/transaction.rs

@@ -1139,7 +1139,7 @@ impl PrivilegedL2Transaction {
 
     /// Returns the formated hash of the deposit transaction,
     /// or None if the transaction is not a deposit.
-    /// The hash is computed as keccak256(to || value)
+    /// The hash is computed as keccak256(to || value || deposit_id == nonce)
     pub fn get_deposit_hash(&self) -> Option<H256> {
         match self.tx_type {
             PrivilegedTxType::Deposit => {
@@ -1151,7 +1151,13 @@ impl PrivilegedL2Transaction {
                 let value = &mut [0u8; 32];
                 self.value.to_big_endian(value);
 
-                Some(keccak_hash::keccak([to.as_bytes(), value].concat()))
+                // The nonce should be a U256,
+                // in solidity the nonce is a U256.
+                let u256_nonce = U256::from(self.nonce);
+                let nonce = &mut [0u8; 32];
+                u256_nonce.to_big_endian(nonce);
+
+                Some(keccak_hash::keccak([to.as_bytes(), value, nonce].concat()))
             }
             _ => None,
         }

crates/l2/.env.example

@@ -11,7 +11,6 @@ DEPLOYER_PRIVATE_KEY=0x385c546456b6a603a1cfcaa9ec9494ba4832da08dd6bcf4de9a71e4a0
 # If set to false, the salt will be randomized.
 DEPLOYER_SALT_IS_ZERO=true
 L1_WATCHER_BRIDGE_ADDRESS=0x266ffef34e21a7c4ce2e0e42dc780c2c273ca440
-L1_WATCHER_TOPICS=0x6f65d68a35457dd88c1f8641be5da191aa122bc76de22ab0789dcc71929d7d37
 L1_WATCHER_CHECK_INTERVAL_MS=1000
 L1_WATCHER_MAX_BLOCK_STEP=5000
 L1_WATCHER_L2_PROPOSER_PRIVATE_KEY=0x385c546456b6a603a1cfcaa9ec9494ba4832da08dd6bcf4de9a71e4a01b74924

crates/l2/Makefile

@@ -1,6 +1,6 @@
 .DEFAULT_GOAL := help
 
-.PHONY: help init down clean init-local-l1 down-local-l1 clean-local-l1 init-l2 down-l2 deploy-l1 deploy-block-executor deploy-inbox setup-prover test
+.PHONY: help init down clean init-local-l1 down-local-l1 clean-local-l1 init-l2 down-l2 deploy-l1 deploy-block-executor deploy-inbox setup-prover ci_test test update-cli update-cli-contracts
 
 L2_GENESIS_FILE_PATH=../../test_data/genesis-l2.json
 L1_GENESIS_FILE_PATH=../../test_data/genesis-l1.json
@@ -95,11 +95,23 @@ init-l2-prover-gpu: ## 🚀 Initializes the Prover with GPU support
 rm_db_l2: ## 🛑 Removes the DB used by the L2
 	cargo run --release --manifest-path ../../Cargo.toml --bin ethrex -- removedb --datadir ${ethrex_L2_DEV_LIBMDBX}
 
+update-cli-contracts: ## 📜 Update the CLI's config contracts
+	@if [ -z "$$C" ]; then \
+		echo "Error: CONFIG_NAME (C) is missing.\nPlease provide it as an argument:\nmake update-cli-contracts C=<config_name>."; \
+		exit 1; \
+	fi && \
+	CB=$$(grep 'L1_WATCHER_BRIDGE_ADDRESS' .env | cut -d= -f2) && \
+	ethrex_l2 config edit --common-bridge $$CB $$C && \
+	OP=$$(grep 'COMMITTER_ON_CHAIN_PROPOSER_ADDRESS' .env | cut -d= -f2) && \
+	ethrex_l2 config edit --on-chain-proposer $$OP $$C
 
-# CI Testing
+# Testing
 
-test: ## 🚧 Runs the L2's integration test
+ci_test: ## 🚧 Runs the L2's integration test
 	docker compose -f ${ethrex_L2_DOCKER_COMPOSE_PATH} down
 	docker compose -f ${ethrex_L2_DOCKER_COMPOSE_PATH} up -d --build
 	BRIDGE_ADDRESS=$$(grep 'L1_WATCHER_BRIDGE_ADDRESS' .env | cut -d= -f2) ON_CHAIN_PROPOSER_ADDRESS=$$(grep 'COMMITTER_ON_CHAIN_PROPOSER_ADDRESS' .env | cut -d= -f2) cargo test --release testito -- --nocapture
 	docker compose -f ${ethrex_L2_DOCKER_COMPOSE_PATH} down
+
+test:
+	BRIDGE_ADDRESS=$$(grep 'L1_WATCHER_BRIDGE_ADDRESS' .env | cut -d= -f2) ON_CHAIN_PROPOSER_ADDRESS=$$(grep 'COMMITTER_ON_CHAIN_PROPOSER_ADDRESS' .env | cut -d= -f2) cargo test --release testito -- --nocapture

crates/l2/contracts/src/l1/CommonBridge.sol

@@ -28,6 +28,10 @@ contract CommonBridge is ICommonBridge, Ownable, ReentrancyGuard {
 
     address public ON_CHAIN_PROPOSER;
 
+    uint256 public lastFetchedL1Block;
+
+    uint256 public depositId;
+
     modifier onlyOnChainProposer() {
         require(
             msg.sender == ON_CHAIN_PROPOSER,
@@ -53,6 +57,9 @@ contract CommonBridge is ICommonBridge, Ownable, ReentrancyGuard {
             "CommonBridge: onChainProposer is the contract address"
         );
         ON_CHAIN_PROPOSER = onChainProposer;
+
+        lastFetchedL1Block = block.number;
+        depositId = 0;
     }
 
     /// @inheritdoc ICommonBridge
@@ -67,9 +74,16 @@ contract CommonBridge is ICommonBridge, Ownable, ReentrancyGuard {
         // TODO: Build the tx.
         bytes32 l2MintTxHash = keccak256(abi.encodePacked("dummyl2MintTxHash"));
         depositLogs.push(
-            keccak256(bytes.concat(bytes20(to), bytes32(msg.value)))
+            keccak256(
+                bytes.concat(
+                    bytes20(to),
+                    bytes32(msg.value),
+                    bytes32(depositId)
+                )
+            )
         );
-        emit DepositInitiated(msg.value, to, l2MintTxHash);
+        emit DepositInitiated(msg.value, to, depositId, l2MintTxHash);
+        depositId += 1;
     }
 
     receive() external payable {

crates/l2/contracts/src/l1/interfaces/ICommonBridge.sol

@@ -14,10 +14,13 @@ interface ICommonBridge {
     /// deposit in L2. Could be used to track the status of the deposit finalization
     /// on L2. You can use this hash to retrive the tx data.
     /// It is the result of keccak(abi.encode(transaction)).
+    /// @param depositId Id used to differentiate deposits with same amount and recipient.
+
     event DepositInitiated(
         uint256 indexed amount,
         address indexed to,
-        bytes32 indexed l2MintTxHash
+        uint256 indexed depositId,
+        bytes32 l2MintTxHash
     );
 
     /// @notice L2 withdrawals have been published on L1.

crates/l2/proposer/errors.rs

@@ -16,8 +16,6 @@ pub enum L1WatcherError {
     FailedToDeserializeLog(String),
     #[error("L1Watcher failed to parse private key: {0}")]
     FailedToDeserializePrivateKey(String),
-    #[error("L1Watcher failed to retrieve depositor account info: {0}")]
-    FailedToRetrieveDepositorAccountInfo(String),
     #[error("L1Watcher failed to retrieve chain config: {0}")]
     FailedToRetrieveChainConfig(String),
     #[error("L1Watcher failed to get config: {0}")]

crates/l2/proposer/l1_committer.rs

@@ -295,6 +295,7 @@ impl Committer {
                         TxKind::Create => Address::zero(),
                     },
                     amount: tx.value,
+                    nonce: tx.nonce,
                 })
                 .collect(),
         };

crates/l2/proposer/l1_watcher.rs

@@ -2,7 +2,7 @@ use crate::{
     proposer::errors::L1WatcherError,
     utils::{
         config::{errors::ConfigError, eth::EthConfig, l1_watcher::L1WatcherConfig},
-        eth_client::{eth_sender::Overrides, EthClient},
+        eth_client::{errors::EthClientError, eth_sender::Overrides, EthClient},
     },
 };
 use bytes::Bytes;
@@ -21,32 +21,38 @@ use tracing::{debug, error, info, warn};
 pub async fn start_l1_watcher(store: Store) -> Result<(), ConfigError> {
     let eth_config = EthConfig::from_env()?;
     let watcher_config = L1WatcherConfig::from_env()?;
-    let mut l1_watcher = L1Watcher::new_from_config(watcher_config, eth_config);
+    let mut l1_watcher = L1Watcher::new_from_config(watcher_config, eth_config).await?;
     l1_watcher.run(&store).await;
     Ok(())
 }
 
 pub struct L1Watcher {
     eth_client: EthClient,
     address: Address,
-    topics: Vec<H256>,
     max_block_step: U256,
     last_block_fetched: U256,
     l2_proposer_pk: SecretKey,
     check_interval: Duration,
 }
 
 impl L1Watcher {
-    pub fn new_from_config(watcher_config: L1WatcherConfig, eth_config: EthConfig) -> Self {
-        Self {
-            eth_client: EthClient::new_from_config(eth_config),
+    pub async fn new_from_config(
+        watcher_config: L1WatcherConfig,
+        eth_config: EthConfig,
+    ) -> Result<Self, EthClientError> {
+        let eth_client = EthClient::new_from_config(eth_config);
+        let last_block_fetched =
+            EthClient::get_last_fetched_l1_block(&eth_client, watcher_config.bridge_address)
+                .await?
+                .into();
+        Ok(Self {
+            eth_client,
             address: watcher_config.bridge_address,
-            topics: watcher_config.topics,
             max_block_step: watcher_config.max_block_step,
-            last_block_fetched: U256::zero(),
+            last_block_fetched,
             l2_proposer_pk: watcher_config.l2_proposer_private_key,
             check_interval: Duration::from_millis(watcher_config.check_interval_ms),
-        }
+        })
     }
 
     pub async fn run(&mut self, store: &Store) {
@@ -70,20 +76,26 @@ impl L1Watcher {
                 continue;
             }
 
-            let pending_deposits_logs = self.get_pending_deposit_logs().await?;
+            let pending_deposit_logs = self.get_pending_deposit_logs().await?;
             let _deposit_txs = self
-                .process_logs(logs, &pending_deposits_logs, &store)
+                .process_logs(logs, &pending_deposit_logs, &store)
                 .await?;
         }
     }
 
     pub async fn get_pending_deposit_logs(&self) -> Result<Vec<H256>, L1WatcherError> {
+        let selector = keccak(b"getDepositLogs()")
+            .as_bytes()
+            .get(..4)
+            .ok_or(EthClientError::Custom("Failed to get selector.".to_owned()))?
+            .to_vec();
+
         Ok(hex::decode(
             &self
                 .eth_client
                 .call(
                     self.address,
-                    Bytes::copy_from_slice(&[0x35, 0x6d, 0xa2, 0x49]),
+                    Bytes::copy_from_slice(&selector),
                     Overrides::default(),
                 )
                 .await?[2..],
@@ -112,25 +124,31 @@ impl L1Watcher {
             self.last_block_fetched, new_last_block
         );
 
+        // Matches the event DepositInitiated from ICommonBridge.sol
+        let topic = keccak(b"DepositInitiated(uint256,address,uint256,bytes32)");
         let logs = match self
             .eth_client
             .get_logs(
                 self.last_block_fetched + 1,
                 new_last_block,
                 self.address,
-                self.topics[0],
+                topic,
             )
             .await
         {
             Ok(logs) => logs,
             Err(error) => {
+                // We may get an error if the RPC doesn't has the logs for the requested
+                // block interval. For example, Light Nodes.
                 warn!("Error when getting logs from L1: {}", error);
                 vec![]
             }
         };
 
         debug!("Logs: {:#?}", logs);
 
+        // If we have an error adding the tx to the mempool we may assign it to the next
+        // block to fetch, but we may lose a deposit tx.
         self.last_block_fetched = new_last_block;
 
         Ok(logs)
@@ -139,23 +157,10 @@ impl L1Watcher {
     pub async fn process_logs(
         &self,
         logs: Vec<RpcLog>,
-        l1_deposit_logs: &[H256],
+        pending_deposit_logs: &[H256],
         store: &Store,
     ) -> Result<Vec<H256>, L1WatcherError> {
         let mut deposit_txs = Vec::new();
-        let mut operator_nonce = store
-            .get_account_info(
-                store
-                    .get_latest_block_number()
-                    .map_err(|e| L1WatcherError::FailedToRetrieveChainConfig(e.to_string()))?
-                    .ok_or(L1WatcherError::FailedToRetrieveChainConfig(
-                        "Last block is None".to_string(),
-                    ))?,
-                Address::zero(),
-            )
-            .map_err(|e| L1WatcherError::FailedToRetrieveDepositorAccountInfo(e.to_string()))?
-            .map(|info| info.nonce)
-            .unwrap_or_default();
 
         for log in logs {
             let mint_value = format!("{:#x}", log.log.topics[1])
@@ -173,14 +178,27 @@ impl L1Watcher {
                     ))
                 })?;
 
+            let deposit_id = format!("{:#x}", log.log.topics[3])
+                .parse::<U256>()
+                .map_err(|e| {
+                    L1WatcherError::FailedToDeserializeLog(format!(
+                        "Failed to parse depositId value from log: {e:#?}"
+                    ))
+                })?;
+
             let mut value_bytes = [0u8; 32];
             mint_value.to_big_endian(&mut value_bytes);
-            if !l1_deposit_logs.contains(&keccak([beneficiary.as_bytes(), &value_bytes].concat())) {
-                warn!("Deposit already processed (to: {beneficiary:#x}, value: {mint_value}), skipping.");
+
+            let mut id_bytes = [0u8; 32];
+            deposit_id.to_big_endian(&mut id_bytes);
+            if !pending_deposit_logs.contains(&keccak(
+                [beneficiary.as_bytes(), &value_bytes, &id_bytes].concat(),
+            )) {
+                warn!("Deposit already processed (to: {beneficiary:#x}, value: {mint_value}, depositId: {deposit_id}), skipping.");
                 continue;
             }
 
-            info!("Initiating mint transaction for {beneficiary:#x} with value {mint_value:#x}",);
+            info!("Initiating mint transaction for {beneficiary:#x} with value {mint_value:#x} and depositId: {deposit_id:#}",);
 
             let mut mint_transaction = self
                 .eth_client
@@ -198,7 +216,10 @@ impl L1Watcher {
                                 })?
                                 .chain_id,
                         ),
-                        nonce: Some(operator_nonce),
+                        // Using the deposit_id as nonce.
+                        // If we make a transaction on the L2 with this address, we may break the
+                        // deposit workflow.
+                        nonce: Some(deposit_id.as_u64()),
                         value: Some(mint_value),
                         // TODO(IMPORTANT): gas_limit should come in the log and must
                         // not be calculated in here. The reason for this is that the
@@ -212,8 +233,6 @@ impl L1Watcher {
                 .await?;
             mint_transaction.sign_inplace(&self.l2_proposer_pk);
 
-            operator_nonce += 1;
-
             match mempool::add_transaction(
                 Transaction::PrivilegedL2Transaction(mint_transaction),
                 store,
@@ -229,6 +248,7 @@ impl L1Watcher {
                 }
             }
         }
+
         Ok(deposit_txs)
     }
 }

crates/l2/proposer/state_diff.rs

@@ -33,6 +33,7 @@ pub struct WithdrawalLog {
 pub struct DepositLog {
     pub address: Address,
     pub amount: U256,
+    pub nonce: u64,
 }
 
 #[derive(Clone)]

crates/l2/utils/eth_client/mod.rs

@@ -972,6 +972,13 @@ impl EthClient {
         .await
     }
 
+    pub async fn get_last_fetched_l1_block(
+        eth_client: &EthClient,
+        common_bridge_address: Address,
+    ) -> Result<u64, EthClientError> {
+        Self::_call_block_variable(eth_client, b"lastFetchedL1Block()", common_bridge_address).await
+    }
+
     async fn _call_block_variable(
         eth_client: &EthClient,
         selector: &[u8],