Merge pull request #3295 from Pana/rpc/eth_getBlockByNumber

op: eth_getBlockByNumber return null when parameter is bigger than latest block

Author: Pana

Cargo.lock

@@ -111,6 +111,7 @@ members = [
     "crates/config",
     "crates/cfxcore/types",
     "crates/cfxcore/pow",
+    "crates/cfxcore/errors",
 ]
 
 resolver = "2"
@@ -185,6 +186,7 @@ network = { path = "./crates/network" }
 cfxcore = { path = "./crates/cfxcore/core" }
 cfxcore-types = { path = "./crates/cfxcore/types" }
 cfxcore-pow = { path = "./crates/cfxcore/pow" }
+cfxcore-errors = { path = "./crates/cfxcore/errors" }
 cfx-parameters = { path = "./crates/parameters" }
 cfx-execute-helper = { path = "./crates/execution/execute-helper" }
 cfx-executor = { path = "./crates/execution/executor" }

Cargo.toml

@@ -18,6 +18,7 @@ Misc changes:
    4. These trace methods now support `SelfDestruct(Suicide)` trace, to access historical selfdestruct transaction data, a resync of the data is required.
 4. eSpace now support geth style `txpool` namespace methods, including: `txpool_status`, `txpool_inspect`, `txpool_content`, `txpool_contentFrom`
 5. `eth_call`, `eth_estimateGas` add support for `stateoverride` feature.
+6. `eth_getBlockByNumber` will return `null` when block number is bigger than latest block, other than return error message.
 
 ## v2.4.1
 

changelogs/JSONRPC.md

@@ -116,6 +116,7 @@ cfx-rpc-utils = { workspace = true }
 cfx-util-macros = { workspace = true }
 cfxcore-types = { workspace = true }
 cfxcore-pow = { workspace = true }
+cfxcore-errors = { workspace = true}
 
 [dev-dependencies]
 cfx-storage = { workspace = true, features = ["testonly_code"] }

crates/cfxcore/core/Cargo.toml

@@ -1,6 +1,7 @@
 use super::ConsensusGraph;
 
 use crate::errors::{invalid_params, Result as CoreResult};
+use cfxcore_errors::ProviderBlockError;
 
 use cfx_parameters::consensus::*;
 
@@ -23,7 +24,7 @@ impl ConsensusGraph {
     /// Convert EpochNumber to height based on the current ConsensusGraph
     pub fn get_height_from_epoch_number(
         &self, epoch_number: EpochNumber,
-    ) -> Result<u64, String> {
+    ) -> Result<u64, ProviderBlockError> {
         Ok(match epoch_number {
             EpochNumber::Earliest => 0,
             EpochNumber::LatestCheckpoint => {
@@ -40,7 +41,7 @@ impl ConsensusGraph {
             EpochNumber::Number(num) => {
                 let epoch_num = num;
                 if epoch_num > self.inner.read_recursive().best_epoch_number() {
-                    return Err("Invalid params: expected a numbers with less than largest epoch number.".to_owned());
+                    return Err(ProviderBlockError::EpochNumberTooLarge);
                 }
                 epoch_num
             }
@@ -61,7 +62,7 @@ impl ConsensusGraph {
 
     pub fn get_block_hashes_by_epoch(
         &self, epoch_number: EpochNumber,
-    ) -> Result<Vec<H256>, String> {
+    ) -> Result<Vec<H256>, ProviderBlockError> {
         self.get_height_from_epoch_number(epoch_number)
             .and_then(|height| {
                 self.inner.read_recursive().block_hashes_by_epoch(height)
@@ -70,7 +71,7 @@ impl ConsensusGraph {
 
     pub fn get_hash_from_epoch_number(
         &self, epoch_number: EpochNumber,
-    ) -> Result<H256, String> {
+    ) -> Result<H256, ProviderBlockError> {
         self.get_height_from_epoch_number(epoch_number)
             .and_then(|height| {
                 self.inner.read().get_pivot_hash_from_epoch_number(height)
@@ -79,7 +80,7 @@ impl ConsensusGraph {
 
     pub fn get_skipped_block_hashes_by_epoch(
         &self, epoch_number: EpochNumber,
-    ) -> Result<Vec<H256>, String> {
+    ) -> Result<Vec<H256>, ProviderBlockError> {
         self.get_height_from_epoch_number(epoch_number)
             .and_then(|height| {
                 self.inner

crates/cfxcore/core/src/consensus/consensus_graph/onchain_blocks_provider.rs

@@ -4,6 +4,7 @@ use cfx_execute_helper::{
 };
 use cfx_rpc_cfx_types::PhantomBlock;
 use cfx_types::{Bloom, Space, H256, U256};
+use cfxcore_errors::ProviderBlockError;
 use primitives::{receipt::Receipt, EpochNumber, TransactionStatus};
 use std::sync::Arc;
 
@@ -12,7 +13,7 @@ use super::super::ConsensusGraph;
 impl ConsensusGraph {
     pub fn get_phantom_block_bloom_filter(
         &self, block_num: EpochNumber, pivot_assumption: H256,
-    ) -> Result<Option<Bloom>, String> {
+    ) -> Result<Option<Bloom>, ProviderBlockError> {
         let hashes = self.get_block_hashes_by_epoch(block_num)?;
 
         // sanity check: epoch is not empty
@@ -66,7 +67,7 @@ impl ConsensusGraph {
     pub fn get_phantom_block_pivot_by_number(
         &self, block_num: EpochNumber, pivot_assumption: Option<H256>,
         include_traces: bool,
-    ) -> Result<Option<PhantomBlock>, String> {
+    ) -> Result<Option<PhantomBlock>, ProviderBlockError> {
         self.get_phantom_block_by_number_inner(
             block_num,
             pivot_assumption,
@@ -78,7 +79,7 @@ impl ConsensusGraph {
     pub fn get_phantom_block_by_number(
         &self, block_num: EpochNumber, pivot_assumption: Option<H256>,
         include_traces: bool,
-    ) -> Result<Option<PhantomBlock>, String> {
+    ) -> Result<Option<PhantomBlock>, ProviderBlockError> {
         self.get_phantom_block_by_number_inner(
             block_num,
             pivot_assumption,
@@ -90,7 +91,7 @@ impl ConsensusGraph {
     fn get_phantom_block_by_number_inner(
         &self, block_num: EpochNumber, pivot_assumption: Option<H256>,
         include_traces: bool, only_pivot: bool,
-    ) -> Result<Option<PhantomBlock>, String> {
+    ) -> Result<Option<PhantomBlock>, ProviderBlockError> {
         let hashes = self.get_block_hashes_by_epoch(block_num)?;
 
         // special handling for genesis block
@@ -297,7 +298,7 @@ impl ConsensusGraph {
 
     pub fn get_phantom_block_by_hash(
         &self, hash: &H256, include_traces: bool,
-    ) -> Result<Option<PhantomBlock>, String> {
+    ) -> Result<Option<PhantomBlock>, ProviderBlockError> {
         let epoch_num = match self.get_block_epoch_number(hash) {
             None => return Ok(None),
             Some(n) => n,

crates/cfxcore/core/src/consensus/consensus_graph/rpc_api/phantom_block_provider.rs

@@ -6,6 +6,7 @@ mod blame_verifier;
 pub mod confirmation_meter;
 pub mod consensus_executor;
 pub mod consensus_new_block_handler;
+use cfxcore_errors::ProviderBlockError;
 use cfxcore_pow as pow;
 
 use pow::{PowComputer, ProofOfWorkConfig};
@@ -2131,7 +2132,7 @@ impl ConsensusGraphInner {
     /// out of the current era.
     pub fn get_pivot_hash_from_epoch_number(
         &self, epoch_number: u64,
-    ) -> Result<EpochId, String> {
+    ) -> Result<EpochId, ProviderBlockError> {
         let height = epoch_number;
         if height >= self.cur_era_genesis_height {
             let pivot_index = (height - self.cur_era_genesis_height) as usize;
@@ -2161,7 +2162,7 @@ impl ConsensusGraphInner {
 
     pub fn block_hashes_by_epoch(
         &self, epoch_number: u64,
-    ) -> Result<Vec<H256>, String> {
+    ) -> Result<Vec<H256>, ProviderBlockError> {
         debug!(
             "block_hashes_by_epoch epoch_number={:?} pivot_chain.len={:?}",
             epoch_number,
@@ -2199,7 +2200,7 @@ impl ConsensusGraphInner {
 
     pub fn skipped_block_hashes_by_epoch(
         &self, epoch_number: u64,
-    ) -> Result<Vec<H256>, String> {
+    ) -> Result<Vec<H256>, ProviderBlockError> {
         debug!(
             "skipped_block_hashes_by_epoch epoch_number={:?} pivot_chain.len={:?}",
             epoch_number,
@@ -2351,11 +2352,11 @@ impl ConsensusGraphInner {
 
     pub fn check_block_pivot_assumption(
         &self, pivot_hash: &H256, epoch: u64,
-    ) -> Result<(), String> {
+    ) -> Result<(), ProviderBlockError> {
         let last_number = self.best_epoch_number();
         let hash = self.get_pivot_hash_from_epoch_number(epoch)?;
         if epoch > last_number || hash != *pivot_hash {
-            return Err("Error: pivot chain assumption failed".to_owned());
+            return Err("Error: pivot chain assumption failed".into());
         }
         Ok(())
     }
@@ -4143,6 +4144,7 @@ impl StateMaintenanceTrait for ConsensusGraphInner {
             self,
             epoch_number,
         )
+        .map_err(|e| e.to_string())
     }
 
     fn get_epoch_execution_commitment_with_db(

crates/cfxcore/core/src/consensus/consensus_inner/mod.rs

@@ -6,6 +6,7 @@ use cfx_rpc_eth_types::Error as EthRpcError;
 pub use cfx_rpc_utils::error::error_codes::EXCEPTION_ERROR;
 use cfx_statedb::Error as StateDbError;
 use cfx_storage::Error as StorageError;
+use cfxcore_errors::ProviderBlockError;
 use jsonrpc_core::{futures::future, Error as JsonRpcError, ErrorCode};
 use jsonrpsee::types::ErrorObjectOwned;
 use primitives::{account::AccountError, filter::FilterError};
@@ -90,6 +91,10 @@ impl From<String> for Error {
     fn from(s: String) -> Error { Error::Msg(s) }
 }
 
+impl From<ProviderBlockError> for Error {
+    fn from(e: ProviderBlockError) -> Error { Error::from(e.to_string()) }
+}
+
 impl From<EthRpcError> for Error {
     fn from(e: EthRpcError) -> Error {
         let e: JsonRpcError = e.into();

crates/cfxcore/core/src/errors.rs

@@ -64,7 +64,10 @@ impl LedgerInfo {
     #[inline]
     fn pivot_hash_of(&self, height: u64) -> Result<H256, Error> {
         let epoch = EpochNumber::Number(height);
-        Ok(self.consensus.get_hash_from_epoch_number(epoch)?)
+        Ok(self
+            .consensus
+            .get_hash_from_epoch_number(epoch)
+            .map_err(|e| e.to_string())?)
     }
 
     /// Get header at `height` on the pivot chain, if it exists.
@@ -79,7 +82,10 @@ impl LedgerInfo {
     #[inline]
     pub fn block_hashes_in(&self, height: u64) -> Result<Vec<H256>, Error> {
         let epoch = EpochNumber::Number(height);
-        Ok(self.consensus.get_block_hashes_by_epoch(epoch)?)
+        Ok(self
+            .consensus
+            .get_block_hashes_by_epoch(epoch)
+            .map_err(|e| e.to_string())?)
     }
 
     /// Get the correct deferred state root of the block at `height` on the

crates/cfxcore/core/src/light_protocol/common/ledger_info.rs

@@ -279,7 +279,10 @@ impl QueryService {
                 break;
             }
 
-            let mut epoch_hashes = inner.read().block_hashes_by_epoch(epoch)?;
+            let mut epoch_hashes = inner
+                .read()
+                .block_hashes_by_epoch(epoch)
+                .map_err(|e| e.to_string())?;
             epoch_hashes.reverse();
 
             let missing = GAS_PRICE_BLOCK_SAMPLE_SIZE - hashes.len();