Address review comments

madara-alliance · Nov 25, 2024 · 4922188 · 4922188
1 parent 425b4ee
commit 4922188
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 27 deletions.
diff --git a/crates/client/db/src/l1_db.rs b/crates/client/db/src/l1_db.rs
@@ -132,11 +132,11 @@ impl MadaraBackend {
 
     pub fn get_l1_handler_tx_hashes(&self, l1_tx_hash: TxHash) -> Result<Vec<Felt>, DbError> {
         let l1_l2_mappings_column = self.db.get_column(Column::L1MessagingHandlerTxHashes);
-        let mut l1_handler_tx_hashes = vec![];
-        for kv_bytes in self.db.prefix_iterator_cf(&l1_l2_mappings_column, l1_tx_hash.as_slice()) {
-            let l1_handler_tx_hash = Felt::from_bytes_be_slice(&kv_bytes?.1);
-            l1_handler_tx_hashes.push(l1_handler_tx_hash);
-        }
+        let l1_handler_tx_hashes = self
+            .db
+            .prefix_iterator_cf(&l1_l2_mappings_column, l1_tx_hash.as_slice())
+            .map(|kv_bytes| Ok(Felt::from_bytes_be_slice(&kv_bytes?.1)))
+            .collect::<Result<_, rocksdb::Error>>()?;
         Ok(l1_handler_tx_hashes)
     }
 

diff --git a/crates/client/eth/src/l1_messaging.rs b/crates/client/eth/src/l1_messaging.rs
@@ -5,7 +5,7 @@ use alloy::eips::BlockNumberOrTag;
 use alloy::primitives::{keccak256, FixedBytes, U256};
 use alloy::rpc::types::Log;
 use alloy::sol_types::SolValue;
-use anyhow::{anyhow, Context};
+use anyhow::Context;
 use blockifier::transaction::transaction_execution::Transaction as BlockifierTransation;
 use futures::StreamExt;
 use mc_db::{l1_db::LastSyncedEventBlock, MadaraBackend};
@@ -130,9 +130,9 @@ async fn process_l1_to_l2_msg(
     )?;
     mempool.accept_l1_handler_tx(blockifier_transaction)?;
 
-    let l1_tx_hash = log.transaction_hash.ok_or_else(|| anyhow!("Missing transaction hash"))?;
-    let block_number = log.block_number.ok_or_else(|| anyhow!("Event missing block number"))?;
-    let log_index = log.log_index.ok_or_else(|| anyhow!("Event missing log index"))?;
+    let l1_tx_hash = log.transaction_hash.context("Missing transaction hash")?;
+    let block_number = log.block_number.context("Event missing block number")?;
+    let log_index = log.log_index.context("Event missing log index")?;
 
     // We use the log index for the order to ensure any L1 txs which have multiple messages are
     // retrieved in the order they occured.

diff --git a/crates/client/rpc/src/versions/user/v0_8_0/methods/read/get_messages_status.rs b/crates/client/rpc/src/versions/user/v0_8_0/methods/read/get_messages_status.rs
@@ -15,24 +15,26 @@ pub fn get_messages_status(starknet: &Starknet, transaction_hash: TxHash) -> Sta
     if l1_handler_tx_hashes.is_empty() {
         return Err(StarknetRpcApiError::TxnHashNotFound);
     }
-    let mut message_statuses = vec![];
-    for l1_handler_tx_hash in l1_handler_tx_hashes {
-        let finality_status = match get_transaction_status(starknet, l1_handler_tx_hash) {
-            Ok(tx_status) => tx_status.finality_status(),
-            Err(StarknetRpcApiError::TxnHashNotFound) => {
-                tracing::error!("L1 handler tx {l1_handler_tx_hash:?} for L1 tx {transaction_hash:?} not found");
-                return Err(StarknetRpcApiError::InternalServerError);
-            }
-            Err(e) => return Err(e),
-        };
-        message_statuses.push(MessageStatus {
-            transaction_hash: l1_handler_tx_hash,
-            finality_status,
-            // TODO Update this once get_transaction_status supports rejections
-            failure_reason: None,
-        })
-    }
-    Ok(message_statuses)
+    l1_handler_tx_hashes.iter().try_fold(
+        Vec::with_capacity(l1_handler_tx_hashes.len()),
+        |mut acc, l1_handler_tx_hash| {
+            let finality_status = match get_transaction_status(starknet, *l1_handler_tx_hash) {
+                Ok(tx_status) => tx_status.finality_status(),
+                Err(StarknetRpcApiError::TxnHashNotFound) => {
+                    tracing::error!("L1 handler tx {l1_handler_tx_hash:?} for L1 tx {transaction_hash:?} not found");
+                    return Err(StarknetRpcApiError::InternalServerError);
+                }
+                Err(e) => return Err(e),
+            };
+            acc.push(MessageStatus {
+                transaction_hash: *l1_handler_tx_hash,
+                finality_status,
+                // TODO Update this once get_transaction_status supports rejections
+                failure_reason: None,
+            });
+            Ok(acc)
+        },
+    )
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]