fix: return null from getBlockTransactionCount methods for unsynced b…

…lock (#441) * fix: return null from block tx count methods for unsynced block * keep behavior unchanged in evm * update wasm bindgens
a16z · Nov 22, 2024 · fa22bfd · fa22bfd
1 parent f46388e
commit fa22bfd
Show file tree

Hide file tree

Showing 7 changed files with 111 additions and 66 deletions.
diff --git a/core/src/client/mod.rs b/core/src/client/mod.rs
@@ -81,11 +81,14 @@ impl<N: NetworkSpec, C: Consensus<N::TransactionResponse>> Client<N, C> {
         self.node.get_nonce(address, block).await
     }
 
-    pub async fn get_block_transaction_count_by_hash(&self, hash: B256) -> Result<u64> {
+    pub async fn get_block_transaction_count_by_hash(&self, hash: B256) -> Result<Option<u64>> {
         self.node.get_block_transaction_count_by_hash(hash).await
     }
 
-    pub async fn get_block_transaction_count_by_number(&self, block: BlockTag) -> Result<u64> {
+    pub async fn get_block_transaction_count_by_number(
+        &self,
+        block: BlockTag,
+    ) -> Result<Option<u64>> {
         self.node.get_block_transaction_count_by_number(block).await
     }
 

diff --git a/core/src/client/node.rs b/core/src/client/node.rs
@@ -70,18 +70,17 @@ impl<N: NetworkSpec, C: Consensus<N::TransactionResponse>> Node<N, C> {
         Ok(account.nonce)
     }
 
-    pub async fn get_block_transaction_count_by_hash(&self, hash: B256) -> Result<u64> {
-        let block = self.execution.get_block_by_hash(hash, false).await?;
-        let transaction_count = block.transactions.hashes().len();
-
-        Ok(transaction_count as u64)
+    pub async fn get_block_transaction_count_by_hash(&self, hash: B256) -> Result<Option<u64>> {
+        let block = self.execution.get_block_by_hash(hash, false).await;
+        Ok(block.map(|block| block.transactions.hashes().len() as u64))
     }
 
-    pub async fn get_block_transaction_count_by_number(&self, tag: BlockTag) -> Result<u64> {
-        let block = self.execution.get_block(tag, false).await?;
-        let transaction_count = block.transactions.hashes().len();
-
-        Ok(transaction_count as u64)
+    pub async fn get_block_transaction_count_by_number(
+        &self,
+        tag: BlockTag,
+    ) -> Result<Option<u64>> {
+        let block = self.execution.get_block(tag, false).await;
+        Ok(block.map(|block| block.transactions.hashes().len() as u64))
     }
 
     pub async fn get_code(&self, address: Address, tag: BlockTag) -> Result<Bytes> {
@@ -178,7 +177,11 @@ impl<N: NetworkSpec, C: Consensus<N::TransactionResponse>> Node<N, C> {
     pub async fn get_gas_price(&self) -> Result<U256> {
         self.check_head_age().await?;
 
-        let block = self.execution.get_block(BlockTag::Latest, false).await?;
+        let block = self
+            .execution
+            .get_block(BlockTag::Latest, false)
+            .await
+            .ok_or(eyre!(ClientError::BlockNotFound(BlockTag::Latest)))?;
         let base_fee = block.base_fee_per_gas;
         let tip = U256::from(10_u64.pow(9));
         Ok(base_fee + tip)
@@ -193,7 +196,11 @@ impl<N: NetworkSpec, C: Consensus<N::TransactionResponse>> Node<N, C> {
     pub async fn get_block_number(&self) -> Result<U256> {
         self.check_head_age().await?;
 
-        let block = self.execution.get_block(BlockTag::Latest, false).await?;
+        let block = self
+            .execution
+            .get_block(BlockTag::Latest, false)
+            .await
+            .ok_or(eyre!(ClientError::BlockNotFound(BlockTag::Latest)))?;
         Ok(block.number.to())
     }
 
@@ -204,10 +211,8 @@ impl<N: NetworkSpec, C: Consensus<N::TransactionResponse>> Node<N, C> {
     ) -> Result<Option<Block<N::TransactionResponse>>> {
         self.check_blocktag_age(&tag).await?;
 
-        match self.execution.get_block(tag, full_tx).await {
-            Ok(block) => Ok(Some(block)),
-            Err(_) => Ok(None),
-        }
+        let block = self.execution.get_block(tag, full_tx).await;
+        Ok(block)
     }
 
     pub async fn get_block_by_hash(
@@ -216,11 +221,7 @@ impl<N: NetworkSpec, C: Consensus<N::TransactionResponse>> Node<N, C> {
         full_tx: bool,
     ) -> Result<Option<Block<N::TransactionResponse>>> {
         let block = self.execution.get_block_by_hash(hash, full_tx).await;
-
-        match block {
-            Ok(block) => Ok(Some(block)),
-            Err(_) => Ok(None),
-        }
+        Ok(block)
     }
 
     pub fn chain_id(&self) -> u64 {
@@ -246,7 +247,12 @@ impl<N: NetworkSpec, C: Consensus<N::TransactionResponse>> Node<N, C> {
     pub async fn get_coinbase(&self) -> Result<Address> {
         self.check_head_age().await?;
 
-        let block = self.execution.get_block(BlockTag::Latest, false).await?;
+        let block = self
+            .execution
+            .get_block(BlockTag::Latest, false)
+            .await
+            .ok_or(eyre!(ClientError::BlockNotFound(BlockTag::Latest)))?;
+
         Ok(block.miner)
     }
 
@@ -260,7 +266,7 @@ impl<N: NetworkSpec, C: Consensus<N::TransactionResponse>> Node<N, C> {
             .execution
             .get_block(BlockTag::Latest, false)
             .await
-            .map_err(|_| ClientError::OutOfSync(timestamp))?
+            .ok_or_else(|| ClientError::OutOfSync(timestamp))?
             .timestamp
             .to();
 

diff --git a/core/src/client/rpc.rs b/core/src/client/rpc.rs
@@ -66,12 +66,12 @@ trait EthRpc<TX: TransactionResponse + RpcObject, TXR: RpcObject, R: ReceiptResp
     async fn get_block_transaction_count_by_hash(
         &self,
         hash: B256,
-    ) -> Result<U64, ErrorObjectOwned>;
+    ) -> Result<Option<U64>, ErrorObjectOwned>;
     #[method(name = "getBlockTransactionCountByNumber")]
     async fn get_block_transaction_count_by_number(
         &self,
         block: BlockTag,
-    ) -> Result<U64, ErrorObjectOwned>;
+    ) -> Result<Option<U64>, ErrorObjectOwned>;
     #[method(name = "getCode")]
     async fn get_code(&self, address: Address, block: BlockTag) -> Result<Bytes, ErrorObjectOwned>;
     #[method(name = "call")]
@@ -188,15 +188,25 @@ impl<N: NetworkSpec, C: Consensus<N::TransactionResponse>>
     async fn get_block_transaction_count_by_hash(
         &self,
         hash: B256,
-    ) -> Result<U64, ErrorObjectOwned> {
-        convert_err(self.node.get_block_transaction_count_by_hash(hash).await).map(U64::from)
+    ) -> Result<Option<U64>, ErrorObjectOwned> {
+        convert_err(
+            self.node
+                .get_block_transaction_count_by_hash(hash)
+                .await
+                .map(|opt| opt.map(U64::from)),
+        )
     }
 
     async fn get_block_transaction_count_by_number(
         &self,
         block: BlockTag,
-    ) -> Result<U64, ErrorObjectOwned> {
-        convert_err(self.node.get_block_transaction_count_by_number(block).await).map(U64::from)
+    ) -> Result<Option<U64>, ErrorObjectOwned> {
+        convert_err(
+            self.node
+                .get_block_transaction_count_by_number(block)
+                .await
+                .map(|opt| opt.map(U64::from)),
+        )
     }
 
     async fn get_code(&self, address: Address, block: BlockTag) -> Result<Bytes, ErrorObjectOwned> {

diff --git a/core/src/execution/evm.rs b/core/src/execution/evm.rs
@@ -13,7 +13,10 @@ use revm::{
 use tracing::trace;
 
 use crate::execution::{
-    constants::PARALLEL_QUERY_BATCH_SIZE, errors::EvmError, rpc::ExecutionRpc, ExecutionClient,
+    constants::PARALLEL_QUERY_BATCH_SIZE,
+    errors::{EvmError, ExecutionError},
+    rpc::ExecutionRpc,
+    ExecutionClient,
 };
 use crate::network_spec::NetworkSpec;
 use crate::types::BlockTag;
@@ -88,7 +91,12 @@ impl<N: NetworkSpec, R: ExecutionRpc<N>> Evm<N, R> {
         let mut env = Env::default();
         env.tx = N::tx_env(tx);
 
-        let block = self.execution.get_block(tag, false).await.unwrap();
+        let block = self
+            .execution
+            .get_block(tag, false)
+            .await
+            .ok_or(ExecutionError::BlockNotFound(tag))
+            .unwrap();
         env.block = N::block_env(&block);
 
         env.cfg.chain_id = self.chain_id;
@@ -169,10 +177,12 @@ impl<N: NetworkSpec, R: ExecutionRpc<N>> EvmState<N, R> {
                     storage.insert(*slot, value);
                 }
                 StateAccess::BlockHash(number) => {
+                    let tag = BlockTag::Number(*number);
                     let block = self
                         .execution
-                        .get_block(BlockTag::Number(*number), false)
-                        .await?;
+                        .get_block(tag, false)
+                        .await
+                        .ok_or(ExecutionError::BlockNotFound(tag))?;
 
                     self.block_hash.insert(*number, block.hash);
                 }
@@ -233,7 +243,12 @@ impl<N: NetworkSpec, R: ExecutionRpc<N>> EvmState<N, R> {
             storage_keys: Vec::default(),
         };
 
-        let coinbase = self.execution.get_block(self.block, false).await?.miner;
+        let coinbase = self
+            .execution
+            .get_block(self.block, false)
+            .await
+            .ok_or(ExecutionError::BlockNotFound(self.block))?
+            .miner;
         let producer_access_entry = AccessListItem {
             address: coinbase,
             storage_keys: Vec::default(),

diff --git a/core/src/execution/mod.rs b/core/src/execution/mod.rs
@@ -7,6 +7,7 @@ use alloy::rpc::types::{Filter, Log};
 use eyre::Result;
 use futures::future::try_join_all;
 use revm::primitives::KECCAK_EMPTY;
+use tracing::warn;
 use triehash_ethereum::ordered_trie_root;
 
 use crate::network_spec::NetworkSpec;
@@ -134,36 +135,38 @@ impl<N: NetworkSpec, R: ExecutionRpc<N>> ExecutionClient<N, R> {
         &self,
         tag: BlockTag,
         full_tx: bool,
-    ) -> Result<Block<N::TransactionResponse>> {
-        let mut block = self
-            .state
-            .get_block(tag)
-            .await
-            .ok_or(ExecutionError::BlockNotFound(tag))?;
+    ) -> Option<Block<N::TransactionResponse>> {
+        let block = self.state.get_block(tag).await;
+        if block.is_none() {
+            warn!(target: "helios::execution", "requested block not found in state: {}", tag);
+            return None;
+        }
+        let mut block = block.unwrap();
 
         if !full_tx {
             block.transactions = Transactions::Hashes(block.transactions.hashes());
         }
 
-        Ok(block)
+        Some(block)
     }
 
     pub async fn get_block_by_hash(
         &self,
         hash: B256,
         full_tx: bool,
-    ) -> Result<Block<N::TransactionResponse>> {
-        let mut block = self
-            .state
-            .get_block_by_hash(hash)
-            .await
-            .ok_or(eyre::eyre!("block not found"))?;
+    ) -> Option<Block<N::TransactionResponse>> {
+        let block = self.state.get_block_by_hash(hash).await;
+        if block.is_none() {
+            warn!(target: "helios::execution", "requested block not found in state: {}", hash);
+            return None;
+        }
+        let mut block = block.unwrap();
 
         if !full_tx {
             block.transactions = Transactions::Hashes(block.transactions.hashes());
         }
 
-        Ok(block)
+        Some(block)
     }
 
     pub async fn get_transaction_by_block_hash_and_index(

diff --git a/helios-ts/src/ethereum.rs b/helios-ts/src/ethereum.rs
@@ -169,23 +169,27 @@ impl EthereumClient {
     }
 
     #[wasm_bindgen]
-    pub async fn get_block_transaction_count_by_hash(&self, hash: JsValue) -> Result<u32, JsError> {
+    pub async fn get_block_transaction_count_by_hash(
+        &self,
+        hash: JsValue,
+    ) -> Result<Option<u32>, JsError> {
         let hash: B256 = serde_wasm_bindgen::from_value(hash)?;
         let count = map_err(self.inner.get_block_transaction_count_by_hash(hash).await)?;
-        Ok(count as u32)
+        Ok(count.map(|v| v as u32))
     }
 
     #[wasm_bindgen]
     pub async fn get_block_transaction_count_by_number(
         &self,
         block: JsValue,
-    ) -> Result<u32, JsError> {
+    ) -> Result<Option<u32>, JsError> {
         let block: BlockTag = serde_wasm_bindgen::from_value(block)?;
-        let res = self
-            .inner
-            .get_block_transaction_count_by_number(block)
-            .await;
-        Ok(map_err(res)? as u32)
+        let count = map_err(
+            self.inner
+                .get_block_transaction_count_by_number(block)
+                .await,
+        )?;
+        Ok(count.map(|v| v as u32))
     }
 
     #[wasm_bindgen]

diff --git a/helios-ts/src/opstack.rs b/helios-ts/src/opstack.rs
@@ -112,23 +112,27 @@ impl OpStackClient {
     }
 
     #[wasm_bindgen]
-    pub async fn get_block_transaction_count_by_hash(&self, hash: JsValue) -> Result<u32, JsError> {
+    pub async fn get_block_transaction_count_by_hash(
+        &self,
+        hash: JsValue,
+    ) -> Result<Option<u32>, JsError> {
         let hash: B256 = serde_wasm_bindgen::from_value(hash)?;
         let count = map_err(self.inner.get_block_transaction_count_by_hash(hash).await)?;
-        Ok(count as u32)
+        Ok(count.map(|v| v as u32))
     }
 
     #[wasm_bindgen]
     pub async fn get_block_transaction_count_by_number(
         &self,
         block: JsValue,
-    ) -> Result<u32, JsError> {
+    ) -> Result<Option<u32>, JsError> {
         let block: BlockTag = serde_wasm_bindgen::from_value(block)?;
-        let res = self
-            .inner
-            .get_block_transaction_count_by_number(block)
-            .await;
-        Ok(map_err(res)? as u32)
+        let count = map_err(
+            self.inner
+                .get_block_transaction_count_by_number(block)
+                .await,
+        )?;
+        Ok(count.map(|v| v as u32))
     }
 
     #[wasm_bindgen]