From 02820733d5b618a72b04db34e6de135b2fa306b5 Mon Sep 17 00:00:00 2001 From: James Date: Thu, 21 Mar 2024 19:11:06 -0700 Subject: [PATCH 1/4] fix: remove app-layer usage of transport error --- crates/json-rpc/src/error.rs | 9 +++++++ crates/provider/src/heart.rs | 4 +++- crates/provider/src/provider.rs | 42 ++++++++++----------------------- crates/provider/src/utils.rs | 17 +++++++++---- crates/transport/src/error.rs | 9 ------- 5 files changed, 36 insertions(+), 45 deletions(-) diff --git a/crates/json-rpc/src/error.rs b/crates/json-rpc/src/error.rs index 6843b3047c1..075c3245366 100644 --- a/crates/json-rpc/src/error.rs +++ b/crates/json-rpc/src/error.rs @@ -8,6 +8,10 @@ pub enum RpcError> { #[error("server returned an error response: {0}")] ErrorResp(ErrorPayload), + /// Server returned a null response when a non-null response was expected. + #[error("server returned a null response when a non-null response was expected")] + NullResp, + /// JSON serialization error. #[error("serialization error: {0}")] SerError( @@ -94,6 +98,11 @@ impl RpcError { matches!(self, Self::ErrorResp(_)) } + /// Check if the error is a null response. + pub const fn is_null_resp(&self) -> bool { + matches!(self, Self::NullResp) + } + /// Fallible conversion to an error response. pub const fn as_error_resp(&self) -> Option<&ErrorPayload> { match self { diff --git a/crates/provider/src/heart.rs b/crates/provider/src/heart.rs index 4395e84e4fc..66071598c49 100644 --- a/crates/provider/src/heart.rs +++ b/crates/provider/src/heart.rs @@ -1,6 +1,7 @@ //! Block heartbeat and pending transaction watcher. use crate::{Provider, RootProvider}; +use alloy_json_rpc::RpcError; use alloy_network::Network; use alloy_primitives::{B256, U256}; use alloy_rpc_types::Block; @@ -182,7 +183,8 @@ impl<'a, N: Network, T: Transport + Clone> PendingTransactionBuilder<'a, N, T> { let pending_tx = self.provider.watch_pending_transaction(self.config).await?; let hash = pending_tx.await?; let receipt = self.provider.get_transaction_receipt(hash).await?; - receipt.ok_or_else(TransportErrorKind::missing_receipt) + + receipt.ok_or(RpcError::NullResp) } } diff --git a/crates/provider/src/provider.rs b/crates/provider/src/provider.rs index 8a987cb0ec2..e96fd7e949a 100644 --- a/crates/provider/src/provider.rs +++ b/crates/provider/src/provider.rs @@ -6,7 +6,7 @@ use crate::{ utils::{self, Eip1559Estimation, EstimatorFunction}, PendingTransactionBuilder, }; -use alloy_json_rpc::{RpcParam, RpcReturn}; +use alloy_json_rpc::{RpcError, RpcParam, RpcReturn}; use alloy_network::{Network, TransactionBuilder}; use alloy_primitives::{ hex, Address, BlockHash, BlockNumber, Bytes, StorageKey, StorageValue, TxHash, B256, U256, U64, @@ -762,41 +762,23 @@ pub trait Provider: Send + Sync &self, estimator: Option, ) -> TransportResult { - let base_fee_per_gas = match self.get_block_by_number(BlockNumberOrTag::Latest, false).await - { - Ok(Some(block)) => match block.header.base_fee_per_gas { - Some(base_fee_per_gas) => base_fee_per_gas, - None => return Err(TransportErrorKind::custom_str("EIP-1559 not activated")), - }, - - Ok(None) => return Err(TransportErrorKind::custom_str("Latest block not found")), - - Err(err) => return Err(err), - }; - - let fee_history = match self - .get_fee_history( + let (bf, fh) = futures::join!( + self.get_block_by_number(BlockNumberOrTag::Latest, false), + self.get_fee_history( U256::from(utils::EIP1559_FEE_ESTIMATION_PAST_BLOCKS), BlockNumberOrTag::Latest, &[utils::EIP1559_FEE_ESTIMATION_REWARD_PERCENTILE], ) - .await - { - Ok(fee_history) => fee_history, - Err(err) => return Err(err), - }; + ); - // use the provided fee estimator function, or fallback to the default implementation. - let (max_fee_per_gas, max_priority_fee_per_gas) = if let Some(es) = estimator { - es(base_fee_per_gas, &fee_history.reward.unwrap_or_default()) - } else { - utils::eip1559_default_estimator( - base_fee_per_gas, - &fee_history.reward.unwrap_or_default(), - ) - }; + let base_fee_per_gas = + bf?.ok_or(RpcError::NullResp)?.header.base_fee_per_gas.ok_or(RpcError::NullResp)?; + let fee_history = fh?; - Ok(Eip1559Estimation { max_fee_per_gas, max_priority_fee_per_gas }) + Ok(estimator.unwrap_or(utils::eip1559_default_estimator)( + base_fee_per_gas, + &fee_history.reward.unwrap_or_default(), + )) } /// Get the account and storage values of the specified account including the merkle proofs. diff --git a/crates/provider/src/utils.rs b/crates/provider/src/utils.rs index 4e0f4b357aa..d14e2ecb9fb 100644 --- a/crates/provider/src/utils.rs +++ b/crates/provider/src/utils.rs @@ -10,10 +10,10 @@ pub const EIP1559_BASE_FEE_MULTIPLIER: f64 = 2.0; pub const EIP1559_FEE_ESTIMATION_REWARD_PERCENTILE: f64 = 20.0; /// An estimator function for EIP1559 fees. -pub type EstimatorFunction = fn(U256, &[Vec]) -> (U256, U256); +pub type EstimatorFunction = fn(U256, &[Vec]) -> Eip1559Estimation; /// Return type of EIP1155 gas fee estimator. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct Eip1559Estimation { /// The base fee per gas. pub max_fee_per_gas: U256, @@ -42,10 +42,14 @@ fn estimate_priority_fee(rewards: &[Vec]) -> U256 { /// The default EIP-1559 fee estimator which is based on the work by [MetaMask](https://github.com/MetaMask/core/blob/main/packages/gas-fee-controller/src/fetchGasEstimatesViaEthFeeHistory/calculateGasFeeEstimatesForPriorityLevels.ts#L56) /// (constants for "medium" priority level are used) -pub fn eip1559_default_estimator(base_fee_per_gas: U256, rewards: &[Vec]) -> (U256, U256) { +pub fn eip1559_default_estimator( + base_fee_per_gas: U256, + rewards: &[Vec], +) -> Eip1559Estimation { let max_priority_fee_per_gas = estimate_priority_fee(rewards); let potential_max_fee = base_fee_per_gas * U256::from(EIP1559_BASE_FEE_MULTIPLIER); - (potential_max_fee, max_priority_fee_per_gas) + + Eip1559Estimation { max_fee_per_gas: potential_max_fee, max_priority_fee_per_gas } } #[cfg(test)] @@ -88,7 +92,10 @@ mod tests { ]; assert_eq!( super::eip1559_default_estimator(base_fee_per_gas, &rewards), - (U256::from(2_000_000_000_u64), U256::from(200_000_000_000_u64)) + Eip1559Estimation { + max_fee_per_gas: U256::from(2_000_000_000_u64), + max_priority_fee_per_gas: U256::from(200_000_000_000_u64) + } ); } } diff --git a/crates/transport/src/error.rs b/crates/transport/src/error.rs index fe88dc5d175..42f0b03dfc0 100644 --- a/crates/transport/src/error.rs +++ b/crates/transport/src/error.rs @@ -31,10 +31,6 @@ pub enum TransportErrorKind { #[error("subscriptions are not available on this provider")] PubsubUnavailable, - /// Transaction confirmed but `get_transaction_receipt` returned `None`. - #[error("transaction confirmed but receipt returned was null")] - MissingReceipt, - /// Custom error. #[error("{0}")] Custom(#[source] Box), @@ -71,9 +67,4 @@ impl TransportErrorKind { pub const fn pubsub_unavailable() -> TransportError { RpcError::Transport(Self::PubsubUnavailable) } - - /// Instantiate a new `TransportError::MissingReceipt`. - pub const fn missing_receipt() -> TransportError { - RpcError::Transport(Self::MissingReceipt) - } } From ff7591149ec23fd58b07a45a6feb8632fabbbd25 Mon Sep 17 00:00:00 2001 From: James Date: Thu, 21 Mar 2024 19:17:41 -0700 Subject: [PATCH 2/4] fix: error handling in gas estimation --- crates/provider/src/layers/gas.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/crates/provider/src/layers/gas.rs b/crates/provider/src/layers/gas.rs index c3b4d9dca12..8a6fc672e91 100644 --- a/crates/provider/src/layers/gas.rs +++ b/crates/provider/src/layers/gas.rs @@ -1,6 +1,7 @@ use crate::{ utils::Eip1559Estimation, PendingTransactionBuilder, Provider, ProviderLayer, RootProvider, }; +use alloy_json_rpc::RpcError; use alloy_network::{Network, TransactionBuilder}; use alloy_primitives::U256; use alloy_transport::{Transport, TransportError, TransportResult}; @@ -122,16 +123,8 @@ where tx.set_max_priority_fee_per_gas(eip1559_fees.max_priority_fee_per_gas); Ok(()) } - Err(err) => { - if err.is_transport_error() - && err.to_string() == *"EIP-1559 not activated".to_string() - { - // If EIP-1559 is not activated, it will process as a legacy tx. - self.handle_legacy_tx(tx).await - } else { - Err(err) - } - } + Err(RpcError::NullResp) => self.handle_legacy_tx(tx).await, + Err(e) => Err(e), } } From 97d8866017a9908c7f5a1cd70b8ba3c6691c5aa6 Mon Sep 17 00:00:00 2001 From: James Date: Thu, 21 Mar 2024 21:42:49 -0700 Subject: [PATCH 3/4] refactor: try_join --- crates/provider/src/provider.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/provider/src/provider.rs b/crates/provider/src/provider.rs index e96fd7e949a..8486529c857 100644 --- a/crates/provider/src/provider.rs +++ b/crates/provider/src/provider.rs @@ -762,18 +762,17 @@ pub trait Provider: Send + Sync &self, estimator: Option, ) -> TransportResult { - let (bf, fh) = futures::join!( + let (bf, fee_history) = futures::try_join!( self.get_block_by_number(BlockNumberOrTag::Latest, false), self.get_fee_history( U256::from(utils::EIP1559_FEE_ESTIMATION_PAST_BLOCKS), BlockNumberOrTag::Latest, &[utils::EIP1559_FEE_ESTIMATION_REWARD_PERCENTILE], ) - ); + )?; let base_fee_per_gas = - bf?.ok_or(RpcError::NullResp)?.header.base_fee_per_gas.ok_or(RpcError::NullResp)?; - let fee_history = fh?; + bf.ok_or(RpcError::NullResp)?.header.base_fee_per_gas.ok_or(RpcError::NullResp)?; Ok(estimator.unwrap_or(utils::eip1559_default_estimator)( base_fee_per_gas, From 49268523406a10dbc700e0de8e33f583e3739a2c Mon Sep 17 00:00:00 2001 From: James Date: Thu, 21 Mar 2024 22:33:29 -0700 Subject: [PATCH 4/4] feat: unsupported --- crates/json-rpc/src/error.rs | 4 ++++ crates/provider/src/layers/gas.rs | 2 +- crates/provider/src/provider.rs | 7 +++++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/json-rpc/src/error.rs b/crates/json-rpc/src/error.rs index 075c3245366..238d6bf5184 100644 --- a/crates/json-rpc/src/error.rs +++ b/crates/json-rpc/src/error.rs @@ -12,6 +12,10 @@ pub enum RpcError> { #[error("server returned a null response when a non-null response was expected")] NullResp, + /// Rpc server returned an unsupported feature. + #[error("unsupported feature: {0}")] + UnsupportedFeature(&'static str), + /// JSON serialization error. #[error("serialization error: {0}")] SerError( diff --git a/crates/provider/src/layers/gas.rs b/crates/provider/src/layers/gas.rs index 8a6fc672e91..cfcd3bede01 100644 --- a/crates/provider/src/layers/gas.rs +++ b/crates/provider/src/layers/gas.rs @@ -123,7 +123,7 @@ where tx.set_max_priority_fee_per_gas(eip1559_fees.max_priority_fee_per_gas); Ok(()) } - Err(RpcError::NullResp) => self.handle_legacy_tx(tx).await, + Err(RpcError::UnsupportedFeature("eip1559")) => self.handle_legacy_tx(tx).await, Err(e) => Err(e), } } diff --git a/crates/provider/src/provider.rs b/crates/provider/src/provider.rs index 8486529c857..d23a5047458 100644 --- a/crates/provider/src/provider.rs +++ b/crates/provider/src/provider.rs @@ -771,8 +771,11 @@ pub trait Provider: Send + Sync ) )?; - let base_fee_per_gas = - bf.ok_or(RpcError::NullResp)?.header.base_fee_per_gas.ok_or(RpcError::NullResp)?; + let base_fee_per_gas = bf + .ok_or(RpcError::NullResp)? + .header + .base_fee_per_gas + .ok_or(RpcError::UnsupportedFeature("eip1559"))?; Ok(estimator.unwrap_or(utils::eip1559_default_estimator)( base_fee_per_gas,