diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d813f1e..8aa17a08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ [#668](https://github.com/gakonst/ethers-rs/pull/668) - Fix `fill_transaction` to set nonces in transactions, if the sender is known and no nonce is specified +- Move `fill_transaction` implementation to the provider, to allow middleware + to properly override its behavior. ## ethers-contract-abigen diff --git a/ethers-providers/src/lib.rs b/ethers-providers/src/lib.rs index 2c72a163..66547cec 100644 --- a/ethers-providers/src/lib.rs +++ b/ethers-providers/src/lib.rs @@ -159,66 +159,32 @@ pub trait Middleware: Sync + Send + Debug { self.inner().client_version().await.map_err(FromErr::from) } - /// Helper for filling a transaction + /// Fill necessary details of a transaction for dispatch + /// + /// This function is defined on providers to behave as follows: + /// 1. populate the `from` field with the default sender + /// 2. resolve any ENS names in the tx `to` field + /// 3. Estimate gas usage _without_ access lists + /// 4. Estimate gas usage _with_ access lists + /// 5. Enable access lists IFF they are cheaper + /// 6. Poll and set legacy or 1559 gas prices + /// + /// It does NOT set the nonce by default. + /// It MAY override the gas amount set by the user, if access lists are + /// cheaper. + /// + /// Middleware are encouraged to override any values _before_ delegating + /// to the inner implementation AND/OR modify the values provided by the + /// default implementation _after_ delegating. + /// + /// E.g. a middleware wanting to double gas prices should consider doing so + /// _after_ delegating and allowing the default implementation to poll gas. async fn fill_transaction( &self, tx: &mut TypedTransaction, block: Option, ) -> Result<(), Self::Error> { - if let Some(default_sender) = self.default_sender() { - if tx.from().is_none() { - tx.set_from(default_sender); - } - } - - // TODO: Can we poll the futures below at the same time? - // Access List + Name resolution and then Gas price + Gas - - // set the ENS name - if let Some(NameOrAddress::Name(ref ens_name)) = tx.to() { - let addr = self.resolve_name(ens_name).await?; - tx.set_to(addr); - } - - // estimate the gas without the access list - let gas = maybe(tx.gas().cloned(), self.estimate_gas(tx)).await?; - let mut al_used = false; - - // set the access lists - if let Some(access_list) = tx.access_list() { - if access_list.0.is_empty() { - if let Ok(al_with_gas) = self.create_access_list(tx, block).await { - // only set the access list if the used gas is less than the - // normally estimated gas - if al_with_gas.gas_used < gas { - tx.set_access_list(al_with_gas.access_list); - tx.set_gas(al_with_gas.gas_used); - al_used = true; - } - } - } - } - - if !al_used { - tx.set_gas(gas); - } - - match tx { - TypedTransaction::Eip2930(_) | TypedTransaction::Legacy(_) => { - let gas_price = maybe(tx.gas_price(), self.get_gas_price()).await?; - tx.set_gas_price(gas_price); - } - TypedTransaction::Eip1559(ref mut inner) => { - if inner.max_fee_per_gas.is_none() || inner.max_priority_fee_per_gas.is_none() { - let (max_fee_per_gas, max_priority_fee_per_gas) = - self.estimate_eip1559_fees(None).await?; - inner.max_fee_per_gas = Some(max_fee_per_gas); - inner.max_priority_fee_per_gas = Some(max_priority_fee_per_gas); - }; - } - } - - Ok(()) + self.inner().fill_transaction(tx, block).await.map_err(FromErr::from) } async fn get_block_number(&self) -> Result { diff --git a/ethers-providers/src/provider.rs b/ethers-providers/src/provider.rs index a20544b6..e0ebdeb4 100644 --- a/ethers-providers/src/provider.rs +++ b/ethers-providers/src/provider.rs @@ -1,5 +1,5 @@ use crate::{ - ens, + ens, maybe, pubsub::{PubsubClient, SubscriptionStream}, stream::{FilterWatcher, DEFAULT_POLL_INTERVAL}, FromErr, Http as HttpProvider, JsonRpcClient, JsonRpcClientWrapper, MockProvider, @@ -260,6 +260,67 @@ impl Middleware for Provider

{ self.request("web3_clientVersion", ()).await } + async fn fill_transaction( + &self, + tx: &mut TypedTransaction, + block: Option, + ) -> Result<(), Self::Error> { + if let Some(default_sender) = self.default_sender() { + if tx.from().is_none() { + tx.set_from(default_sender); + } + } + + // TODO: Can we poll the futures below at the same time? + // Access List + Name resolution and then Gas price + Gas + + // set the ENS name + if let Some(NameOrAddress::Name(ref ens_name)) = tx.to() { + let addr = self.resolve_name(ens_name).await?; + tx.set_to(addr); + } + + // estimate the gas without the access list + let gas = maybe(tx.gas().cloned(), self.estimate_gas(tx)).await?; + let mut al_used = false; + + // set the access lists + if let Some(access_list) = tx.access_list() { + if access_list.0.is_empty() { + if let Ok(al_with_gas) = self.create_access_list(tx, block).await { + // only set the access list if the used gas is less than the + // normally estimated gas + if al_with_gas.gas_used < gas { + tx.set_access_list(al_with_gas.access_list); + tx.set_gas(al_with_gas.gas_used); + al_used = true; + } + } + } + } + + if !al_used { + tx.set_gas(gas); + } + + match tx { + TypedTransaction::Eip2930(_) | TypedTransaction::Legacy(_) => { + let gas_price = maybe(tx.gas_price(), self.get_gas_price()).await?; + tx.set_gas_price(gas_price); + } + TypedTransaction::Eip1559(ref mut inner) => { + if inner.max_fee_per_gas.is_none() || inner.max_priority_fee_per_gas.is_none() { + let (max_fee_per_gas, max_priority_fee_per_gas) = + self.estimate_eip1559_fees(None).await?; + inner.max_fee_per_gas = Some(max_fee_per_gas); + inner.max_priority_fee_per_gas = Some(max_priority_fee_per_gas); + }; + } + } + + Ok(()) + } + /// Gets the latest block number via the `eth_BlockNumber` API async fn get_block_number(&self) -> Result { self.request("eth_blockNumber", ()).await