Merge pull request #2853 from Pana/opEstimate

Fix feeHistory reverse order and zero gas_used_ratio issue (#2853)

crates/cfxcore/core/src/transaction_pool/mod.rs

@@ -379,9 +379,6 @@ impl TransactionPool {
         let mut failure = HashMap::new();
         let current_best_info = self.consensus_best_info.lock().clone();
 
-        // filter out invalid transactions.
-        let mut index = 0;
-
         let (chain_id, best_height, best_block_number) = {
             (
                 current_best_info.best_chain_id(),
@@ -395,6 +392,8 @@ impl TransactionPool {
         let vm_spec = self.machine.spec(best_block_number, best_height);
         let transitions = &self.machine.params().transition_heights;
 
+        // filter out invalid transactions.
+        let mut index = 0;
         while let Some(tx) = transactions.get(index) {
             match self.verify_transaction_tx_pool(
                 tx,

crates/client/Cargo.toml

@@ -102,6 +102,7 @@ solidity-abi = {path= "../util/solidity-abi" }
 bls-signatures = {git = "https://github.com/Conflux-Chain/bls-signatures.git", rev = "fb52187df92d27c365642cb7e7b2aaf60437cf9c", default-features = false, features = ["multicore"]}
 alloy-rpc-types-trace = { workspace = true }
 geth-tracer = { path = "../cfxcore/geth-tracer" }
+serde-utils = { path = "../serde_utils" }
 
 [dev-dependencies]
 criterion = "0.3"

crates/client/src/rpc/impls/cfx/cfx_handler.rs

@@ -8,7 +8,7 @@ use crate::rpc::{
         call_request::rpc_call_request_network,
         errors::check_rpc_address_network, pos::PoSEpochReward, FeeHistory,
         PoSEconomics, RpcAddress, SponsorInfo, StatOnGasLoad, TokenSupplyInfo,
-        VoteParamsInfo, WrapTransaction,
+        VoteParamsInfo, WrapTransaction, U64 as HexU64,
     },
 };
 use blockgen::BlockGenerator;
@@ -1359,18 +1359,13 @@ impl RpcImpl {
         };
         let storage_collateralized =
             U64::from(estimation.estimated_storage_limit);
-        let estimated_gas_limit = estimation.estimated_gas_limit;
+        let estimated_gas_used = estimation.estimated_gas_limit;
         let response = EstimateGasAndCollateralResponse {
-            // We multiply the gas_used for 2 reasons:
-            // 1. In each EVM call, the gas passed is at most 63/64 of the
-            // remaining gas, so the gas_limit should be multiplied a factor so
-            // that the gas passed into the sub-call is sufficient. The 4 / 3
-            // factor is sufficient for 18 level of calls.
-            // 2. In Conflux, we recommend setting the gas_limit to (gas_used *
-            // 4) / 3, because the extra gas will be refunded up to
-            // 1/4 of the gas limit.
-            gas_limit: estimation.estimated_gas_limit,
-            gas_used: estimated_gas_limit,
+            gas_limit: estimated_gas_used, /* gas_limit used to be 4/3 of
+                                            * gas_used due to inaccuracy,
+                                            * currently it's the same as gas
+                                            * used as it's more accurate */
+            gas_used: estimated_gas_used,
             storage_collateralized,
         };
         Ok(response)
@@ -2277,7 +2272,7 @@ impl Cfx for CfxHandler {
             fn account_pending_info(&self, addr: RpcAddress) -> BoxFuture<Option<AccountPendingInfo>>;
             fn account_pending_transactions(&self, address: RpcAddress, maybe_start_nonce: Option<U256>, maybe_limit: Option<U64>) -> BoxFuture<AccountPendingTransactions>;
             fn get_pos_reward_by_epoch(&self, epoch: EpochNumber) -> JsonRpcResult<Option<PoSEpochReward>>;
-            fn fee_history(&self, block_count: U64, newest_block: EpochNumber, reward_percentiles: Vec<f64>) -> BoxFuture<FeeHistory>;
+            fn fee_history(&self, block_count: HexU64, newest_block: EpochNumber, reward_percentiles: Vec<f64>) -> BoxFuture<FeeHistory>;
             fn max_priority_fee_per_gas(&self) -> BoxFuture<U256>;
         }
 

crates/client/src/rpc/impls/cfx/common.rs

@@ -17,7 +17,7 @@ use crate::rpc::{
         BlockHashOrEpochNumber, Bytes, CheckBalanceAgainstTransactionResponse,
         EpochNumber, FeeHistory, RpcAddress, Status as RpcStatus,
         Transaction as RpcTransaction, TxPoolPendingNonceRange, TxPoolStatus,
-        TxWithPoolInfo,
+        TxWithPoolInfo, U64 as HexU64,
     },
     RpcErrorKind, RpcResult,
 };
@@ -530,15 +530,15 @@ impl RpcImpl {
     }
 
     pub fn fee_history(
-        &self, block_count: U64, newest_block: EpochNumber,
+        &self, block_count: HexU64, newest_block: EpochNumber,
         reward_percentiles: Vec<f64>,
     ) -> RpcResult<FeeHistory> {
         info!(
             "RPC Request: cfx_feeHistory: block_count={}, newest_block={:?}, reward_percentiles={:?}",
             block_count, newest_block, reward_percentiles
         );
 
-        if block_count == U64::zero() {
+        if block_count.as_u64() == 0 {
             return Ok(FeeHistory::new());
         }
         // keep read lock to ensure consistent view
@@ -585,7 +585,7 @@ impl RpcImpl {
             // Internal error happens only if the fetch header has inconsistent
             // block height
             fee_history
-                .push_back_block(
+                .push_front_block(
                     Space::Native,
                     &reward_percentiles,
                     &block.block_header,
@@ -615,7 +615,7 @@ impl RpcImpl {
         info!("RPC Request: max_priority_fee_per_gas",);
 
         let fee_history = self.fee_history(
-            U64::from(300),
+            HexU64::from(300),
             EpochNumber::LatestState,
             vec![50f64],
         )?;

crates/client/src/rpc/impls/cfx/light.rs

@@ -50,6 +50,7 @@ use crate::{
             SponsorInfo, StatOnGasLoad, Status as RpcStatus,
             StorageCollateralInfo, SyncGraphStates, TokenSupplyInfo,
             Transaction as RpcTransaction, VoteParamsInfo, WrapTransaction,
+            U64 as HexU64,
         },
         RpcBoxFuture, RpcResult,
     },
@@ -1091,15 +1092,15 @@ impl RpcImpl {
     }
 
     fn fee_history(
-        &self, block_count: U64, newest_block: EpochNumber,
+        &self, block_count: HexU64, newest_block: EpochNumber,
         reward_percentiles: Vec<f64>,
     ) -> RpcBoxFuture<FeeHistory> {
         info!(
             "RPC Request: cfx_feeHistory: block_count={}, newest_block={:?}, reward_percentiles={:?}",
             block_count, newest_block, reward_percentiles
         );
 
-        if block_count == U64::zero() {
+        if block_count.as_u64() == 0 {
             return Box::new(async { Ok(FeeHistory::new()) }.boxed().compat());
         }
 
@@ -1135,7 +1136,7 @@ impl RpcImpl {
                 // Internal error happens only if the fetch header has
                 // inconsistent block height
                 fee_history
-                    .push_back_block(
+                    .push_front_block(
                         Space::Native,
                         &reward_percentiles,
                         &block.block_header,
@@ -1239,7 +1240,7 @@ impl Cfx for CfxHandler {
             fn transaction_by_hash(&self, hash: H256) -> BoxFuture<Option<RpcTransaction>>;
             fn transaction_receipt(&self, tx_hash: H256) -> BoxFuture<Option<RpcReceipt>>;
             fn vote_list(&self, address: RpcAddress, num: Option<EpochNumber>) -> BoxFuture<Vec<VoteStakeInfo>>;
-            fn fee_history(&self, block_count: U64, newest_block: EpochNumber, reward_percentiles: Vec<f64>) -> BoxFuture<FeeHistory>;
+            fn fee_history(&self, block_count: HexU64, newest_block: EpochNumber, reward_percentiles: Vec<f64>) -> BoxFuture<FeeHistory>;
         }
     }
 

crates/client/src/rpc/impls/eth/eth_handler.rs

@@ -15,7 +15,7 @@ use crate::rpc::{
             CallRequest, EthRpcLogFilter, Log, Receipt, SyncInfo, SyncStatus,
             Transaction,
         },
-        Bytes, FeeHistory, Index, MAX_GAS_CALL_REQUEST,
+        Bytes, FeeHistory, Index, MAX_GAS_CALL_REQUEST, U64 as HexU64,
     },
 };
 use cfx_execute_helper::estimation::{
@@ -41,8 +41,13 @@ use cfxcore::{
 use clap::crate_version;
 use jsonrpc_core::{Error as RpcError, Result as RpcResult};
 use primitives::{
-    filter::LogFilter, receipt::EVM_SPACE_SUCCESS, Action,
-    BlockHashOrEpochNumber, EpochNumber, SignedTransaction, StorageKey,
+    filter::LogFilter,
+    receipt::EVM_SPACE_SUCCESS,
+    transaction::{
+        Eip1559Transaction, Eip155Transaction, Eip2930Transaction,
+        EthereumTransaction::*, EIP1559_TYPE, EIP2930_TYPE, LEGACY_TX_TYPE,
+    },
+    Action, BlockHashOrEpochNumber, EpochNumber, SignedTransaction, StorageKey,
     StorageValue, TransactionStatus, TransactionWithSignature,
 };
 use rustc_hex::ToHex;
@@ -79,8 +84,6 @@ impl EthHandler {
 pub fn sign_call(
     chain_id: u32, request: CallRequest,
 ) -> RpcResult<SignedTransaction> {
-    use primitives::transaction::*;
-    use EthereumTransaction::*;
     let max_gas = U256::from(MAX_GAS_CALL_REQUEST);
     let gas = min(request.gas.unwrap_or(max_gas), max_gas);
     let nonce = request.nonce.unwrap_or_default();
@@ -90,11 +93,11 @@ pub fn sign_call(
     let default_type_id = if request.max_fee_per_gas.is_some()
         || request.max_priority_fee_per_gas.is_some()
     {
-        2
+        EIP1559_TYPE
     } else if request.access_list.is_some() {
-        1
+        EIP2930_TYPE
     } else {
-        0
+        LEGACY_TX_TYPE
     };
     let transaction_type = request
         .transaction_type
@@ -110,8 +113,8 @@ pub fn sign_call(
     let access_list = request.access_list.unwrap_or(vec![]);
     let data = request.data.unwrap_or_default().into_vec();
 
-    let transaction = match transaction_type.as_usize() {
-        0 => Eip155(Eip155Transaction {
+    let transaction = match transaction_type.as_usize() as u8 {
+        LEGACY_TX_TYPE => Eip155(Eip155Transaction {
             nonce,
             gas_price,
             gas,
@@ -120,7 +123,7 @@ pub fn sign_call(
             chain_id: Some(chain_id),
             data,
         }),
-        1 => Eip2930(Eip2930Transaction {
+        EIP2930_TYPE => Eip2930(Eip2930Transaction {
             chain_id,
             nonce,
             gas_price,
@@ -130,7 +133,7 @@ pub fn sign_call(
             data,
             access_list,
         }),
-        2 => Eip1559(Eip1559Transaction {
+        EIP1559_TYPE => Eip1559(Eip1559Transaction {
             chain_id,
             nonce,
             max_priority_fee_per_gas,
@@ -473,8 +476,11 @@ impl Eth for EthHandler {
             self.tx_pool.machine().params().evm_transaction_block_ratio
                 as usize;
 
-        let fee_history =
-            self.fee_history(U64::from(300), BlockNumber::Latest, vec![50f64])?;
+        let fee_history = self.fee_history(
+            HexU64::from(300),
+            BlockNumber::Latest,
+            vec![50f64],
+        )?;
 
         let total_reward: U256 = fee_history
             .reward()
@@ -901,15 +907,15 @@ impl Eth for EthHandler {
     }
 
     fn fee_history(
-        &self, block_count: U64, newest_block: BlockNumber,
+        &self, block_count: HexU64, newest_block: BlockNumber,
         reward_percentiles: Vec<f64>,
     ) -> jsonrpc_core::Result<FeeHistory> {
         info!(
             "RPC Request: eth_feeHistory: block_count={}, newest_block={:?}, reward_percentiles={:?}",
             block_count, newest_block, reward_percentiles
         );
 
-        if block_count == U64::zero() {
+        if block_count.as_u64() == 0 {
             return Ok(FeeHistory::new());
         }
 
@@ -952,7 +958,7 @@ impl Eth for EthHandler {
             // Internal error happens only if the fetch header has inconsistent
             // block height
             fee_history
-                .push_back_block(
+                .push_front_block(
                     Space::Ethereum,
                     &reward_percentiles,
                     &block.pivot_header,

crates/client/src/rpc/traits/cfx_space/cfx.rs

@@ -10,7 +10,7 @@ use crate::rpc::types::{
     EstimateGasAndCollateralResponse, FeeHistory, Log as RpcLog, PoSEconomics,
     Receipt as RpcReceipt, RewardInfo as RpcRewardInfo, RpcAddress,
     SponsorInfo, Status as RpcStatus, StorageCollateralInfo, TokenSupplyInfo,
-    Transaction, VoteParamsInfo,
+    Transaction, VoteParamsInfo, U64 as HexU64,
 };
 use cfx_types::{H128, H256, U256, U64};
 use jsonrpc_core::{BoxFuture, Result as JsonRpcResult};
@@ -203,7 +203,7 @@ pub trait Cfx {
 
     #[rpc(name = "cfx_feeHistory")]
     fn fee_history(
-        &self, block_count: U64, newest_block: EpochNumber,
+        &self, block_count: HexU64, newest_block: EpochNumber,
         reward_percentiles: Vec<f64>,
     ) -> BoxFuture<FeeHistory>;
 

crates/client/src/rpc/traits/eth_space/eth.rs

@@ -19,6 +19,7 @@
 // along with OpenEthereum.  If not, see <http://www.gnu.org/licenses/>.
 
 //! Eth rpc interface.
+use crate::rpc::types::U64 as HexU64;
 use cfx_types::{H128, H160, H256, U256, U64};
 use jsonrpc_core::Result;
 use jsonrpc_derive::rpc;
@@ -76,7 +77,7 @@ pub trait Eth {
 
     #[rpc(name = "eth_feeHistory")]
     fn fee_history(
-        &self, block_count: U64, newest_block: BlockNumber,
+        &self, block_count: HexU64, newest_block: BlockNumber,
         reward_percentiles: Vec<f64>,
     ) -> Result<FeeHistory>;
 

crates/client/src/rpc/types.rs

@@ -32,6 +32,7 @@ mod trace;
 mod trace_filter;
 mod transaction;
 mod tx_pool;
+mod variadic_u64;
 mod vote_params_info;
 
 pub use self::{
@@ -70,5 +71,6 @@ pub use self::{
         AccountPendingInfo, AccountPendingTransactions,
         TxPoolPendingNonceRange, TxPoolStatus, TxWithPoolInfo,
     },
+    variadic_u64::U64,
     vote_params_info::VoteParamsInfo,
 };