refactor(l2): restart processes with the proposed API (#1234)

**Motivation** The error handling should be the same for all processes, unwraps have to be handled with the `?` operator so that the api can restart the process if any `Err` is raised. **Description** - Use the same API proposed by the PR #1230, removing the unwraps. - #1168 - When we have an error we "restart" the process, this handles the `not committed block` error. - #1165 - The order of the require statements in the `verify()` function of the `OnChainProposer` contract was changed to throw the correct error. The error handling with the proposed API does the rest. - #1164. Closes #1168 Closes #1165 Closes #1164 --------- Co-authored-by: Javier Chatruc <jrchatruc@gmail.com>
lambdaclass · Nov 26, 2024 · 91b09a2 · 91b09a2
1 parent 1f3f40d
commit 91b09a2
Show file tree

Hide file tree

Showing 15 changed files with 927 additions and 647 deletions.
diff --git a/cmd/ethrex_l2/src/commands/test.rs b/cmd/ethrex_l2/src/commands/test.rs
@@ -89,6 +89,7 @@ async fn transfer_from(
         let tx = client
             .build_eip1559_transaction(
                 to_address,
+                address,
                 Bytes::new(),
                 Overrides {
                     chain_id: Some(cfg.network.l2_chain_id),
@@ -99,14 +100,12 @@ async fn transfer_from(
                     gas_limit: Some(TX_GAS_COST),
                     ..Default::default()
                 },
+                10,
             )
             .await
             .unwrap();
 
-        while let Err(e) = client
-            .send_eip1559_transaction(tx.clone(), &private_key)
-            .await
-        {
+        while let Err(e) = client.send_eip1559_transaction(&tx, &private_key).await {
             println!("Transaction failed (PK: {pk} - Nonce: {}): {e}", tx.nonce);
             retries += 1;
             sleep(std::time::Duration::from_secs(2));

diff --git a/cmd/ethrex_l2/src/commands/wallet.rs b/cmd/ethrex_l2/src/commands/wallet.rs
@@ -341,16 +341,18 @@ impl Command {
                 let tx = eth_client
                     .build_eip1559_transaction(
                         cfg.contracts.common_bridge,
+                        cfg.wallet.address,
                         claim_withdrawal_data.into(),
                         Overrides {
                             chain_id: Some(cfg.network.l1_chain_id),
                             from: Some(cfg.wallet.address),
                             ..Default::default()
                         },
+                        10,
                     )
                     .await?;
                 let tx_hash = eth_client
-                    .send_eip1559_transaction(tx, &cfg.wallet.private_key)
+                    .send_eip1559_transaction(&tx, &cfg.wallet.private_key)
                     .await?;
 
                 println!("Withdrawal claim sent: {tx_hash:#x}");
@@ -377,6 +379,7 @@ impl Command {
                 let transfer_tx = client
                     .build_eip1559_transaction(
                         to,
+                        cfg.wallet.address,
                         Bytes::new(),
                         Overrides {
                             value: Some(amount),
@@ -390,11 +393,12 @@ impl Command {
                             gas_limit: Some(21000 * 100),
                             ..Default::default()
                         },
+                        10,
                     )
                     .await?;
 
                 let tx_hash = client
-                    .send_eip1559_transaction(transfer_tx, &cfg.wallet.private_key)
+                    .send_eip1559_transaction(&transfer_tx, &cfg.wallet.private_key)
                     .await?;
 
                 println!(
@@ -418,6 +422,7 @@ impl Command {
                     .build_privileged_transaction(
                         PrivilegedTxType::Withdrawal,
                         to.unwrap_or(cfg.wallet.address),
+                        to.unwrap_or(cfg.wallet.address),
                         Bytes::new(),
                         Overrides {
                             nonce,
@@ -427,11 +432,12 @@ impl Command {
                             gas_price: Some(800000000),
                             ..Default::default()
                         },
+                        10,
                     )
                     .await?;
 
                 let tx_hash = rollup_client
-                    .send_privileged_l2_transaction(withdraw_transaction, &cfg.wallet.private_key)
+                    .send_privileged_l2_transaction(&withdraw_transaction, &cfg.wallet.private_key)
                     .await?;
 
                 println!("Withdrawal sent: {tx_hash:#x}");
@@ -470,6 +476,7 @@ impl Command {
                 let tx = client
                     .build_eip1559_transaction(
                         to,
+                        cfg.wallet.address,
                         calldata,
                         Overrides {
                             value: Some(value),
@@ -487,10 +494,11 @@ impl Command {
                             from: Some(cfg.wallet.address),
                             ..Default::default()
                         },
+                        10,
                     )
                     .await?;
                 let tx_hash = client
-                    .send_eip1559_transaction(tx, &cfg.wallet.private_key)
+                    .send_eip1559_transaction(&tx, &cfg.wallet.private_key)
                     .await?;
 
                 println!(

diff --git a/crates/l2/Makefile b/crates/l2/Makefile
@@ -98,7 +98,7 @@ rm_dev_libmdbx_l2: ## 🛑 Removes the Libmdbx DB used by the L2
 
 # CI Testing
 
-test:
+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

diff --git a/crates/l2/contracts/deployer.rs b/crates/l2/contracts/deployer.rs
@@ -1,10 +1,7 @@
 use bytes::Bytes;
 use colored::Colorize;
 use ethereum_types::{Address, H160, H256};
-use ethrex_core::{
-    types::{GAS_LIMIT_ADJUSTMENT_FACTOR, GAS_LIMIT_MINIMUM},
-    U256,
-};
+use ethrex_core::U256;
 use ethrex_l2::utils::{
     config::{read_env_as_lines, read_env_file, write_env},
     eth_client::{eth_sender::Overrides, EthClient},
@@ -220,18 +217,6 @@ async fn deploy_contracts(
     eth_client: &EthClient,
     contracts_path: &Path,
 ) -> (Address, Address) {
-    let gas_price = if eth_client.url.contains("localhost:8545") {
-        Some(1_000_000_000)
-    } else {
-        Some(eth_client.get_gas_price().await.unwrap().as_u64() * 2)
-    };
-
-    let overrides = Overrides {
-        gas_limit: Some(GAS_LIMIT_MINIMUM * GAS_LIMIT_ADJUSTMENT_FACTOR),
-        gas_price,
-        ..Default::default()
-    };
-
     let deploy_frames = spinner!(["📭❱❱", "❱📬❱", "❱❱📫"], 220);
 
     let mut spinner = Spinner::new(
@@ -241,14 +226,7 @@ async fn deploy_contracts(
     );
 
     let (on_chain_proposer_deployment_tx_hash, on_chain_proposer_address) =
-        deploy_on_chain_proposer(
-            deployer,
-            deployer_private_key,
-            overrides.clone(),
-            eth_client,
-            contracts_path,
-        )
-        .await;
+        deploy_on_chain_proposer(deployer, deployer_private_key, eth_client, contracts_path).await;
 
     let msg = format!(
         "OnChainProposer:\n\tDeployed at address {} with tx hash {}",
@@ -258,14 +236,8 @@ async fn deploy_contracts(
     spinner.success(&msg);
 
     let mut spinner = Spinner::new(deploy_frames, "Deploying CommonBridge", Color::Cyan);
-    let (bridge_deployment_tx_hash, bridge_address) = deploy_bridge(
-        deployer,
-        deployer_private_key,
-        overrides,
-        eth_client,
-        contracts_path,
-    )
-    .await;
+    let (bridge_deployment_tx_hash, bridge_address) =
+        deploy_bridge(deployer, deployer_private_key, eth_client, contracts_path).await;
 
     let msg = format!(
         "CommonBridge:\n\tDeployed at address {} with tx hash {}",
@@ -280,7 +252,6 @@ async fn deploy_contracts(
 async fn deploy_on_chain_proposer(
     deployer: Address,
     deployer_private_key: SecretKey,
-    overrides: Overrides,
     eth_client: &EthClient,
     contracts_path: &Path,
 ) -> (H256, Address) {
@@ -295,7 +266,6 @@ async fn deploy_on_chain_proposer(
         deployer,
         deployer_private_key,
         &on_chain_proposer_init_code,
-        overrides,
         eth_client,
     )
     .await;
@@ -306,7 +276,6 @@ async fn deploy_on_chain_proposer(
 async fn deploy_bridge(
     deployer: Address,
     deployer_private_key: SecretKey,
-    overrides: Overrides,
     eth_client: &EthClient,
     contracts_path: &Path,
 ) -> (H256, Address) {
@@ -329,7 +298,6 @@ async fn deploy_bridge(
         deployer,
         deployer_private_key,
         &bridge_init_code.into(),
-        overrides,
         eth_client,
     )
     .await;
@@ -341,24 +309,22 @@ async fn create2_deploy(
     deployer: Address,
     deployer_private_key: SecretKey,
     init_code: &Bytes,
-    overrides: Overrides,
     eth_client: &EthClient,
 ) -> (H256, Address) {
     let calldata = [SALT.lock().unwrap().as_bytes(), init_code].concat();
     let deploy_tx = eth_client
         .build_eip1559_transaction(
             DETERMINISTIC_CREATE2_ADDRESS,
+            deployer,
             calldata.into(),
-            Overrides {
-                from: Some(deployer),
-                ..overrides
-            },
+            Overrides::default(),
+            10,
         )
         .await
         .expect("Failed to build create2 deploy tx");
 
     let deploy_tx_hash = eth_client
-        .send_eip1559_transaction(deploy_tx, &deployer_private_key)
+        .send_eip1559_transaction(&deploy_tx, &deployer_private_key)
         .await
         .expect("Failed to send create2 deploy tx");
 
@@ -493,16 +459,15 @@ async fn initialize_on_chain_proposer(
     let initialize_tx = eth_client
         .build_eip1559_transaction(
             on_chain_proposer,
+            deployer,
             on_chain_proposer_initialization_calldata.into(),
-            Overrides {
-                from: Some(deployer),
-                ..Default::default()
-            },
+            Overrides::default(),
+            10,
         )
         .await
         .expect("Failed to build initialize transaction");
     let initialize_tx_hash = eth_client
-        .send_eip1559_transaction(initialize_tx, &deployer_private_key)
+        .send_eip1559_transaction(&initialize_tx, &deployer_private_key)
         .await
         .expect("Failed to send initialize transaction");
 
@@ -537,16 +502,15 @@ async fn initialize_bridge(
     let initialize_tx = eth_client
         .build_eip1559_transaction(
             bridge,
+            deployer,
             bridge_initialization_calldata.into(),
-            Overrides {
-                from: Some(deployer),
-                ..Default::default()
-            },
+            Overrides::default(),
+            10,
         )
         .await
         .expect("Failed to build initialize transaction");
     let initialize_tx_hash = eth_client
-        .send_eip1559_transaction(initialize_tx, &deployer_private_key)
+        .send_eip1559_transaction(&initialize_tx, &deployer_private_key)
         .await
         .expect("Failed to send initialize transaction");
 

diff --git a/crates/l2/contracts/src/l1/OnChainProposer.sol b/crates/l2/contracts/src/l1/OnChainProposer.sol
@@ -38,7 +38,8 @@ contract OnChainProposer is IOnChainProposer, ReentrancyGuard {
     uint256 public lastCommittedBlock;
 
     /// @dev The sequencer addresses that are authorized to commit and verify blocks.
-    mapping(address _authorizedAddress => bool) public authorizedSequencerAddresses;
+    mapping(address _authorizedAddress => bool)
+        public authorizedSequencerAddresses;
 
     address public BRIDGE;
     address public R0VERIFIER;
@@ -138,21 +139,27 @@ contract OnChainProposer is IOnChainProposer, ReentrancyGuard {
     }
 
     /// @inheritdoc IOnChainProposer
+    /// @notice The first `require` checks that the block number is the subsequent block.
+    /// @notice The second `require` checks if the block has been committed.
+    /// @notice The order of these `require` statements is important.
+    /// Ordering Reason: After the verification process, we delete the `blockCommitments` for `blockNumber - 1`. This means that when checking the block,
+    /// we might get an error indicating that the block hasn’t been committed, even though it was committed but deleted. Therefore, it has already been verified.
     function verify(
         uint256 blockNumber,
         bytes calldata blockProof,
         bytes32 imageId,
         bytes32 journalDigest
     ) external override onlySequencer {
-        require(
-            blockCommitments[blockNumber].commitmentHash != bytes32(0),
-            "OnChainProposer: block not committed"
-        );
         require(
             blockNumber == lastVerifiedBlock + 1,
             "OnChainProposer: block already verified"
         );
 
+        require(
+            blockCommitments[blockNumber].commitmentHash != bytes32(0),
+            "OnChainProposer: block not committed"
+        );
+
         if (R0VERIFIER != DEV_MODE) {
             // If the verification fails, it will revert.
             IRiscZeroVerifier(R0VERIFIER).verify(

diff --git a/crates/l2/proposer/errors.rs b/crates/l2/proposer/errors.rs
@@ -1,9 +1,12 @@
+use std::sync::mpsc::SendError;
+
 use crate::utils::{config::errors::ConfigError, eth_client::errors::EthClientError};
 use ethereum_types::FromStrRadixErr;
 use ethrex_core::types::BlobsBundleError;
 use ethrex_dev::utils::engine_client::errors::EngineClientError;
 use ethrex_storage::error::StoreError;
 use ethrex_vm::EvmError;
+use tokio::task::JoinError;
 
 #[derive(Debug, thiserror::Error)]
 pub enum L1WatcherError {
@@ -29,6 +32,28 @@ pub enum ProverServerError {
     EthClientError(#[from] EthClientError),
     #[error("ProverServer failed to send transaction: {0}")]
     FailedToVerifyProofOnChain(String),
+    #[error("ProverServer failed retrieve block from storage: {0}")]
+    FailedToRetrieveBlockFromStorage(#[from] StoreError),
+    #[error("ProverServer failed retrieve block from storaga, data is None.")]
+    StorageDataIsNone,
+    #[error("ProverServer failed to create ProverInputs: {0}")]
+    FailedToCreateProverInputs(#[from] EvmError),
+    #[error("ProverServer SigIntError: {0}")]
+    SigIntError(#[from] SigIntError),
+    #[error("ProverServer JoinError: {0}")]
+    JoinError(#[from] JoinError),
+    #[error("ProverServer failed: {0}")]
+    Custom(String),
+}
+
+#[derive(Debug, thiserror::Error)]
+pub enum SigIntError {
+    #[error("SigInt sigint.recv() failed")]
+    Recv,
+    #[error("SigInt tx.send(()) failed: {0}")]
+    Send(#[from] SendError<()>),
+    #[error("SigInt shutdown(Shutdown::Both) failed: {0}")]
+    Shutdown(#[from] std::io::Error),
 }
 
 #[derive(Debug, thiserror::Error)]
@@ -39,6 +64,10 @@ pub enum ProposerError {
     FailedToProduceBlock(String),
     #[error("Proposer failed to prepare PayloadAttributes timestamp: {0}")]
     FailedToGetSystemTime(#[from] std::time::SystemTimeError),
+    #[error("Proposer failed retrieve block from storage: {0}")]
+    FailedToRetrieveBlockFromStorage(#[from] StoreError),
+    #[error("Proposer failed retrieve block from storaga, data is None.")]
+    StorageDataIsNone,
 }
 
 #[derive(Debug, thiserror::Error)]
@@ -61,6 +90,8 @@ pub enum CommitterError {
     FailedToReExecuteBlock(#[from] EvmError),
     #[error("Committer failed to send transaction: {0}")]
     FailedToSendCommitment(String),
+    #[error("Withdrawal transaction was invalid")]
+    InvalidWithdrawalTransaction,
 }
 
 #[derive(Debug, thiserror::Error)]