chore(l2): Remove index slicing usage

cmd/ethrex_l2/src/commands/wallet.rs

@@ -245,6 +245,12 @@ async fn get_withdraw_merkle_proof(
             .collect(),
         tx_withdrawal_hash,
     )
+    .map_err(|err| {
+        eyre::eyre!(
+            "Failed to generate merkle proof in get_withdraw_merkle_proof: {:?}",
+            err
+        )
+    })?
     .ok_or_eyre("Transaction's WithdrawalData is not in block's WithdrawalDataMerkleRoot")?;
 
     Ok((index, path))

crates/l2/proposer/errors.rs

@@ -1,5 +1,6 @@
 use std::sync::mpsc::SendError;
 
+use crate::utils::merkle_tree::MerkleError;
 use crate::utils::{config::errors::ConfigError, eth_client::errors::EthClientError};
 use ethereum_types::FromStrRadixErr;
 use ethrex_core::types::BlobsBundleError;
@@ -92,6 +93,10 @@ pub enum CommitterError {
     FailedToReExecuteBlock(#[from] EvmError),
     #[error("Committer failed to send transaction: {0}")]
     FailedToSendCommitment(String),
+    #[error("Commiter failed to decode deposit hash")]
+    FailedToDecodeDepositHash,
+    #[error("Commiter failed to merkelize: {0}")]
+    FailedToMerkelize(#[from] MerkleError),
     #[error("Withdrawal transaction was invalid")]
     InvalidWithdrawalTransaction,
     #[error("Blob estimation failed: {0}")]

crates/l2/proposer/l1_committer.rs

@@ -117,13 +117,13 @@ impl Committer {
                 }
 
                 let withdrawal_logs_merkle_root =
-                    self.get_withdrawals_merkle_root(withdrawal_hashes);
+                    self.get_withdrawals_merkle_root(withdrawal_hashes)?;
                 let deposit_logs_hash = self.get_deposit_hash(
                     deposits
                         .iter()
                         .filter_map(|tx| tx.get_deposit_hash())
                         .collect(),
-                );
+                )?;
 
                 let state_diff = self.prepare_state_diff(
                     &block_to_commit,
@@ -180,11 +180,14 @@ impl Committer {
         Ok(withdrawals)
     }
 
-    pub fn get_withdrawals_merkle_root(&self, withdrawals_hashes: Vec<H256>) -> H256 {
+    pub fn get_withdrawals_merkle_root(
+        &self,
+        withdrawals_hashes: Vec<H256>,
+    ) -> Result<H256, CommitterError> {
         if !withdrawals_hashes.is_empty() {
-            merkelize(withdrawals_hashes)
+            merkelize(withdrawals_hashes).map_err(CommitterError::FailedToMerkelize)
         } else {
-            H256::zero()
+            Ok(H256::zero())
         }
     }
 
@@ -206,25 +209,27 @@ impl Committer {
         deposits
     }
 
-    pub fn get_deposit_hash(&self, deposit_hashes: Vec<H256>) -> H256 {
+    pub fn get_deposit_hash(&self, deposit_hashes: Vec<H256>) -> Result<H256, CommitterError> {
         if !deposit_hashes.is_empty() {
-            H256::from_slice(
+            Ok(H256::from_slice(
                 [
                     &(deposit_hashes.len() as u16).to_be_bytes(),
-                    &keccak(
+                    keccak(
                         deposit_hashes
                             .iter()
                             .map(H256::as_bytes)
                             .collect::<Vec<&[u8]>>()
                             .concat(),
                     )
-                    .as_bytes()[2..32],
+                    .as_bytes()
+                    .get(2..32)
+                    .ok_or(CommitterError::FailedToDecodeDepositHash)?,
                 ]
                 .concat()
                 .as_slice(),
-            )
+            ))
         } else {
-            H256::zero()
+            Ok(H256::zero())
         }
     }
     /// Prepare the state diff for the block.

crates/l2/proposer/l1_watcher.rs

@@ -79,14 +79,17 @@ impl L1Watcher {
 
     pub async fn get_pending_deposit_logs(&self) -> Result<Vec<H256>, L1WatcherError> {
         Ok(hex::decode(
-            &self
-                .eth_client
+            self.eth_client
                 .call(
                     self.address,
                     Bytes::copy_from_slice(&[0x35, 0x6d, 0xa2, 0x49]),
                     Overrides::default(),
                 )
-                .await?[2..],
+                .await?
+                .get(2..)
+                .ok_or(L1WatcherError::FailedToDeserializeLog(
+                    "Not a valid hex string".to_string(),
+                ))?,
         )
         .map_err(|_| L1WatcherError::FailedToDeserializeLog("Not a valid hex string".to_string()))?
         .chunks(32)
@@ -112,22 +115,28 @@ impl L1Watcher {
             self.last_block_fetched, new_last_block
         );
 
-        let logs = match self
-            .eth_client
-            .get_logs(
-                self.last_block_fetched + 1,
-                new_last_block,
-                self.address,
-                self.topics[0],
-            )
-            .await
-        {
-            Ok(logs) => logs,
-            Err(error) => {
-                warn!("Error when getting logs from L1: {}", error);
-                vec![]
-            }
-        };
+        let logs;
+        if let Some(first_topic) = self.topics.first() {
+            logs = match self
+                .eth_client
+                .get_logs(
+                    self.last_block_fetched + 1,
+                    new_last_block,
+                    self.address,
+                    *first_topic,
+                )
+                .await
+            {
+                Ok(logs) => logs,
+                Err(error) => {
+                    warn!("Error when getting logs from L1: {}", error);
+                    vec![]
+                }
+            };
+        } else {
+            warn!("Error when getting logs from L1: topics vector is empty");
+            logs = vec![];
+        }
 
         debug!("Logs: {:#?}", logs);
 
@@ -158,14 +167,31 @@ impl L1Watcher {
             .unwrap_or_default();
 
         for log in logs {
-            let mint_value = format!("{:#x}", log.log.topics[1])
-                .parse::<U256>()
-                .map_err(|e| {
-                    L1WatcherError::FailedToDeserializeLog(format!(
-                        "Failed to parse mint value from log: {e:#?}"
-                    ))
-                })?;
-            let beneficiary = format!("{:#x}", log.log.topics[2].into_uint())
+            let mint_value = format!(
+                "{:#x}",
+                log.log
+                    .topics
+                    .get(1)
+                    .ok_or(L1WatcherError::FailedToDeserializeLog(
+                        "Failed to parse mint value from log: log.topics[1] out of bounds"
+                            .to_owned()
+                    ))?
+            )
+            .parse::<U256>()
+            .map_err(|e| {
+                L1WatcherError::FailedToDeserializeLog(format!(
+                    "Failed to parse mint value from log: {e:#?}"
+                ))
+            })?;
+            let beneficiary_uint = log
+                .log
+                .topics
+                .get(2)
+                .ok_or(L1WatcherError::FailedToDeserializeLog(
+                    "Failed to parse beneficiary from log: log.topics[2] out of bounds".to_owned(),
+                ))?
+                .into_uint();
+            let beneficiary = format!("{:#x}", beneficiary_uint)
                 .parse::<Address>()
                 .map_err(|e| {
                     L1WatcherError::FailedToDeserializeLog(format!(

crates/l2/sdk/src/sdk.rs

@@ -163,7 +163,14 @@ pub async fn claim_withdraw(
         let mut calldata = Vec::new();
 
         // Function selector
-        calldata.extend_from_slice(&keccak(CLAIM_WITHDRAWAL_SIGNATURE).as_bytes()[..4]);
+        calldata.extend_from_slice(
+            keccak(CLAIM_WITHDRAWAL_SIGNATURE)
+                .as_bytes()
+                .get(..4)
+                .ok_or(EthClientError::Custom(
+                    "failed to slice into the claim withdrawal signature".to_owned(),
+                ))?,
+        );
 
         // bytes32 l2WithdrawalTxHash
         calldata.extend_from_slice(l2_withdrawal_tx_hash.as_fixed_bytes());
@@ -271,6 +278,7 @@ pub async fn get_withdraw_merkle_proof(
             .collect(),
         tx_withdrawal_hash,
     )
+    .map_err(|err| EthClientError::Custom(format!("Failed to generate merkle proof: {err}")))?
     .ok_or(EthClientError::Custom(
         "Failed to generate merkle proof, element is not on the tree".to_string(),
     ))?;

crates/l2/tests/tests.rs

@@ -226,7 +226,7 @@ async fn testito() {
     println!("Claiming funds on L1");
 
     while u64::from_str_radix(
-        &eth_client
+        eth_client
             .call(
                 Address::from_str(
                     &std::env::var("ON_CHAIN_PROPOSER_ADDRESS")
@@ -238,7 +238,9 @@ async fn testito() {
                 Overrides::default(),
             )
             .await
-            .unwrap()[2..],
+            .unwrap()
+            .get(2..)
+            .unwrap(),
         16,
     )
     .unwrap()

crates/l2/utils/eth_client/mod.rs

@@ -29,7 +29,7 @@ use keccak_hash::keccak;
 use reqwest::Client;
 use secp256k1::SecretKey;
 use serde::{Deserialize, Serialize};
-use serde_json::json;
+use serde_json::{json, Value};
 use std::ops::Div;
 use tokio::time::{sleep, Instant};
 use tracing::warn;
@@ -365,7 +365,9 @@ impl EthClient {
 
         // Add the nonce just if present, otherwise the RPC will use the latest nonce
         if let Some(nonce) = transaction.nonce {
-            data["nonce"] = json!(format!("{:#x}", nonce));
+            if let Value::Object(ref mut map) = data {
+                map.insert("nonce".to_owned(), json!(format!("{:#x}", nonce)));
+            }
         }
 
         let request = RpcRequest {
@@ -377,8 +379,12 @@ impl EthClient {
 
         match self.send_request(request).await {
             Ok(RpcResponse::Success(result)) => u64::from_str_radix(
-                &serde_json::from_value::<String>(result.result)
-                    .map_err(EstimateGasPriceError::SerdeJSONError)?[2..],
+                serde_json::from_value::<String>(result.result)
+                    .map_err(EstimateGasPriceError::SerdeJSONError)?
+                    .get(2..)
+                    .ok_or(EstimateGasPriceError::Custom(
+                        "Failed to slice index response in estimate_gas".to_owned(),
+                    ))?,
                 16,
             )
             .map_err(EstimateGasPriceError::ParseIntError)
@@ -398,9 +404,20 @@ impl EthClient {
                                 "Failed to hex::decode in estimate_gas".to_owned(),
                             )
                         })?;
-                        let string_length = U256::from_big_endian(&abi_decoded_error_data[36..68]);
-                        let string_data =
-                            &abi_decoded_error_data[68..68 + string_length.as_usize()];
+                        let string_length = U256::from_big_endian(
+                            abi_decoded_error_data
+                                .get(36..68)
+                                .ok_or(EthClientError::Custom(
+                                    "Failed to slice index abi_decoded_error_data in estimate_gas"
+                                        .to_owned(),
+                                ))?,
+                        );
+                        let string_data = abi_decoded_error_data
+                            .get(68..68 + string_length.as_usize())
+                            .ok_or(EthClientError::Custom(
+                                "Failed to slice index abi_decoded_error_data in estimate_gas"
+                                    .to_owned(),
+                            ))?;
                         String::from_utf8(string_data.to_vec()).map_err(|_| {
                             EthClientError::Custom(
                                 "Failed to String::from_utf8 in estimate_gas".to_owned(),
@@ -449,8 +466,12 @@ impl EthClient {
 
         match self.send_request(request).await {
             Ok(RpcResponse::Success(result)) => u64::from_str_radix(
-                &serde_json::from_value::<String>(result.result)
-                    .map_err(GetNonceError::SerdeJSONError)?[2..],
+                serde_json::from_value::<String>(result.result)
+                    .map_err(GetNonceError::SerdeJSONError)?
+                    .get(2..)
+                    .ok_or(EthClientError::Custom(
+                        "Failed to deserialize get_nonce request".to_owned(),
+                    ))?,
                 16,
             )
             .map_err(GetNonceError::ParseIntError)

crates/l2/utils/merkle_tree.rs

@@ -1,27 +1,38 @@
 use keccak_hash::{keccak, H256};
+use serde::{Deserialize, Serialize};
 use tracing::info;
 
-pub fn merkelize(data: Vec<H256>) -> H256 {
+#[derive(Debug, thiserror::Error, Clone, Serialize, Deserialize)]
+pub enum MerkleError {
+    #[error("Left element is None")]
+    LeftElementIsNone(),
+    #[error("Data vector is empty")]
+    DataVectorIsEmpty(),
+}
+
+pub fn merkelize(data: Vec<H256>) -> Result<H256, MerkleError> {
     info!("Merkelizing {:?}", data);
     let mut data = data;
     let mut first = true;
     while data.len() > 1 || first {
         first = false;
         data = data
             .chunks(2)
-            .map(|chunk| {
-                let left = chunk[0];
-                let right = *chunk.get(1).unwrap_or(&left);
-                keccak([left.as_bytes(), right.as_bytes()].concat())
+            .flat_map(|chunk| -> Result<H256, MerkleError> {
+                let left = chunk.first().ok_or(MerkleError::LeftElementIsNone())?;
+                let right = *chunk.get(1).unwrap_or(left);
+                Ok(keccak([left.as_bytes(), right.as_bytes()].concat()))
             })
             .collect();
     }
-    data[0]
+    data.first()
+        .copied()
+        .ok_or(MerkleError::DataVectorIsEmpty())
 }
 
-pub fn merkle_proof(data: Vec<H256>, base_element: H256) -> Option<Vec<H256>> {
+pub fn merkle_proof(data: Vec<H256>, base_element: H256) -> Result<Option<Vec<H256>>, MerkleError> {
     if !data.contains(&base_element) {
-        return None;
+        return Ok(None);
     }
 
     let mut proof = vec![];
@@ -34,9 +45,12 @@ pub fn merkle_proof(data: Vec<H256>, base_element: H256) -> Option<Vec<H256>> {
         let current_target = target_hash;
         data = data
             .chunks(2)
-            .map(|chunk| {
-                let left = chunk[0];
-                let right = *chunk.get(1).unwrap_or(&left);
+            .flat_map(|chunk| -> Result<H256, MerkleError> {
+                let left = chunk
+                    .first()
+                    .copied()
+                    .ok_or(MerkleError::LeftElementIsNone())?;
+                let right = chunk.get(1).copied().unwrap_or(left);
                 let result = keccak([left.as_bytes(), right.as_bytes()].concat());
                 if left == current_target {
                     proof.push(right);
@@ -45,10 +59,10 @@ pub fn merkle_proof(data: Vec<H256>, base_element: H256) -> Option<Vec<H256>> {
                     proof.push(left);
                     target_hash = result;
                 }
-                result
+                Ok(result)
             })
             .collect();
     }
 
-    Some(proof)
+    Ok(Some(proof))
 }