Don't auto-generate an AccessList when sending/filling a tx (#1619)

Add an optionnal `block` parameter to the gas estimation method
This commit is contained in:
Nicolas Gotchac 2022-08-19 23:33:12 +02:00 committed by GitHub
parent 6f1d47f3aa
commit 68fba606c2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 31 additions and 75 deletions

View File

@ -150,7 +150,7 @@ where
/// Returns the estimated gas cost for the underlying transaction to be executed
pub async fn estimate_gas(&self) -> Result<U256, ContractError<M>> {
self.client.estimate_gas(&self.tx).await.map_err(ContractError::MiddlewareError)
self.client.estimate_gas(&self.tx, self.block).await.map_err(ContractError::MiddlewareError)
}
/// Queries the blockchain via an `eth_call` for the provided transaction.

View File

@ -303,9 +303,13 @@ where
self.signer.sign_message(data.into()).await.map_err(SignerMiddlewareError::SignerError)
}
async fn estimate_gas(&self, tx: &TypedTransaction) -> Result<U256, Self::Error> {
async fn estimate_gas(
&self,
tx: &TypedTransaction,
block: Option<BlockId>,
) -> Result<U256, Self::Error> {
let tx = self.set_tx_from_if_none(tx);
self.inner.estimate_gas(&tx).await.map_err(SignerMiddlewareError::MiddlewareError)
self.inner.estimate_gas(&tx, block).await.map_err(SignerMiddlewareError::MiddlewareError)
}
async fn create_access_list(

View File

@ -106,7 +106,7 @@ pub enum SyncingStatus {
///
/// ```rust
/// use ethers_providers::{Middleware, FromErr};
/// use ethers_core::types::{U64, TransactionRequest, U256, transaction::eip2718::TypedTransaction};
/// use ethers_core::types::{U64, TransactionRequest, U256, transaction::eip2718::TypedTransaction, BlockId};
/// use thiserror::Error;
/// use async_trait::async_trait;
///
@ -147,9 +147,9 @@ pub enum SyncingStatus {
///
/// /// Overrides the default `estimate_gas` method to log that it was called,
/// /// before forwarding the call to the next layer.
/// async fn estimate_gas(&self, tx: &TypedTransaction) -> Result<U256, Self::Error> {
/// async fn estimate_gas(&self, tx: &TypedTransaction, block: Option<BlockId>) -> Result<U256, Self::Error> {
/// println!("Estimating gas...");
/// self.inner().estimate_gas(tx).await.map_err(FromErr::from)
/// self.inner().estimate_gas(tx, block).await.map_err(FromErr::from)
/// }
/// }
/// ```
@ -182,15 +182,11 @@ pub trait Middleware: Sync + Send + Debug {
/// 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
/// 7. Set the chain_id with the provider's, if not already set
/// 3. Estimate gas usage
/// 4. Poll and set legacy or 1559 gas prices
/// 5. Set the chain_id with the provider's, if not already set
///
/// 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
@ -321,8 +317,12 @@ pub trait Middleware: Sync + Send + Debug {
self.inner().get_transaction_count(from, block).await.map_err(FromErr::from)
}
async fn estimate_gas(&self, tx: &TypedTransaction) -> Result<U256, Self::Error> {
self.inner().estimate_gas(tx).await.map_err(FromErr::from)
async fn estimate_gas(
&self,
tx: &TypedTransaction,
block: Option<BlockId>,
) -> Result<U256, Self::Error> {
self.inner().estimate_gas(tx, block).await.map_err(FromErr::from)
}
async fn call(

View File

@ -352,34 +352,10 @@ impl<P: JsonRpcClient> Middleware for Provider<P> {
}
}
// If the tx has an access list but it is empty, it is an Eip1559 or Eip2930 tx,
// and we attempt to populate the acccess list. This may require `eth_estimateGas`,
// in which case we save the result in maybe_gas_res for later
let mut maybe_gas = None;
if let Some(starting_al) = tx.access_list() {
if starting_al.0.is_empty() {
let (gas_res, al_res) = futures_util::join!(
maybe(tx.gas().cloned(), self.estimate_gas(tx)),
self.create_access_list(tx, block)
);
let mut gas = gas_res?;
if let Ok(al_with_gas) = al_res {
// Set access list if it saves gas over the estimated (or previously set) value
if al_with_gas.gas_used < gas {
// Update the gas estimate with the lower amount
gas = al_with_gas.gas_used;
tx.set_access_list(al_with_gas.access_list);
}
}
maybe_gas = Some(gas);
}
}
// Set gas to estimated value only if it was not set by the caller,
// even if the access list has been populated and saves gas
if tx.gas().is_none() {
let gas_estimate = maybe(maybe_gas, self.estimate_gas(tx)).await?;
let gas_estimate = self.estimate_gas(tx, block).await?;
tx.set_gas(gas_estimate);
}
@ -611,8 +587,14 @@ impl<P: JsonRpcClient> Middleware for Provider<P> {
/// required (as a U256) to send it This is free, but only an estimate. Providing too little
/// gas will result in a transaction being rejected (while still consuming all provided
/// gas).
async fn estimate_gas(&self, tx: &TypedTransaction) -> Result<U256, ProviderError> {
self.request("eth_estimateGas", [tx]).await
async fn estimate_gas(
&self,
tx: &TypedTransaction,
block: Option<BlockId>,
) -> Result<U256, ProviderError> {
let tx = utils::serialize(tx);
let block = utils::serialize(&block.unwrap_or_else(|| BlockNumber::Latest.into()));
self.request("eth_estimateGas", [tx, block]).await
}
async fn create_access_list(
@ -2064,42 +2046,14 @@ mod tests {
assert_eq!(tx.gas_price(), Some(max_fee));
assert_eq!(tx.access_list(), Some(&access_list));
// --- fills a 1559 transaction, leaving the existing gas limit unchanged, but including
// access list if cheaper
let gas_with_al = gas - 1;
// --- fills a 1559 transaction, leaving the existing gas limit unchanged,
// without generating an access-list
let mut tx = Eip1559TransactionRequest::new()
.gas(gas)
.max_fee_per_gas(max_fee)
.max_priority_fee_per_gas(prio_fee)
.into();
mock.push(AccessListWithGasUsed {
access_list: access_list.clone(),
gas_used: gas_with_al,
})
.unwrap();
provider.fill_transaction(&mut tx, None).await.unwrap();
assert_eq!(tx.from(), provider.from.as_ref());
assert!(tx.to().is_none());
assert_eq!(tx.gas(), Some(&gas));
assert_eq!(tx.access_list(), Some(&access_list));
// --- fills a 1559 transaction, ignoring access list if more expensive
let gas_with_al = gas + 1;
let mut tx = Eip1559TransactionRequest::new()
.max_fee_per_gas(max_fee)
.max_priority_fee_per_gas(prio_fee)
.into();
mock.push(AccessListWithGasUsed {
access_list: access_list.clone(),
gas_used: gas_with_al,
})
.unwrap();
mock.push(gas).unwrap();
provider.fill_transaction(&mut tx, None).await.unwrap();
assert_eq!(tx.from(), provider.from.as_ref());
@ -2107,14 +2061,12 @@ mod tests {
assert_eq!(tx.gas(), Some(&gas));
assert_eq!(tx.access_list(), Some(&Default::default()));
// --- fills a 1559 transaction, using estimated gas if create_access_list() errors
// --- fills a 1559 transaction, using estimated gas
let mut tx = Eip1559TransactionRequest::new()
.max_fee_per_gas(max_fee)
.max_priority_fee_per_gas(prio_fee)
.into();
// bad mock value causes error response for eth_createAccessList
mock.push(b'b').unwrap();
mock.push(gas).unwrap();
provider.fill_transaction(&mut tx, None).await.unwrap();