Repository: gakonst/ethers-rs
Pull request #1619 (2022-08-19 10:40:29)

Don't auto-generate an AccessList when sending/filling a tx
## Motivation

Cf. discussion in https://github.com/foundry-rs/foundry/pull/2839 

I think that the automatic generation of access-list isn't a wanted feature, for a couple of reasons:
1. There is no way to disable it when using a compatible tx type (if it's set to empty it's always queried)
2. There is no good solution to compare the gas usages. As per other implementations (Geth, Nevermind), the returned GasUsed is the actual transaction gas usage, and not the gas estimation (which wouldn't include saved gas from using AL).

Thus I think it should be a manual step.

## Solution

Disable the automatic fetching of AL when filling a transaction.

I also added an optional block parameter to the gas estimation methods, since it's available on every node implementation. It also rightly use it when a block is passed to `send_transaction` or `fill_transaction`.

## PR Checklist

- [ ] Added Tests
- [ ] Added Documentation
- [ ] Updated the changelog



Patch:
diff --git a/ethers-contract/src/call.rs b/ethers-contract/src/call.rs
index 8dcc4fba..1f753414 100644
--- a/ethers-contract/src/call.rs
+++ b/ethers-contract/src/call.rs
@@ -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.
diff --git a/ethers-core/Cargo.toml b/ethers-core/Cargo.toml
index 6358c3ae..0a51182c 100644
--- a/ethers-core/Cargo.toml
+++ b/ethers-core/Cargo.toml
@@ -41,7 +41,7 @@ cargo_metadata = { version = "0.15.0", optional = true }
 convert_case = { version = "0.5.0", optional = true }
 syn = { version = "1.0.99", optional = true }
 proc-macro2 = { version = "1.0.43", optional = true }
-rust_decimal = "1.26.1"
+rust_decimal = { version = "1.26.1", features = ["maths"] }
 
 [dev-dependencies]
 serde_json = { version = "1.0.64", default-features = false }
diff --git a/ethers-core/src/types/filter.rs b/ethers-core/src/types/filter.rs
index 077b6045..04f750da 100644
--- a/ethers-core/src/types/filter.rs
+++ b/ethers-core/src/types/filter.rs
@@ -254,13 +254,21 @@ impl Filter {
         self
     }
 
-    /// given the event in string form, it hashes it and adds it to the topics to monitor
+    /// Given the event signature in string form, it hashes it and adds it to the topics to monitor
     #[must_use]
     pub fn event(self, event_name: &str) -> Self {
         let hash = H256::from(keccak256(event_name.as_bytes()));
         self.topic0(hash)
     }
 
+    /// Hashes all event signatures and sets them as array to topic0
+    #[must_use]
+    pub fn events(self, events: impl IntoIterator<Item = impl AsRef<[u8]>>) -> Self {
+        let events =
+            events.into_iter().map(|e| H256::from(keccak256(e.as_ref()))).collect::<Vec<_>>();
+        self.topic0(events)
+    }
+
     /// Sets topic0 (the event name for non-anonymous events)
     #[must_use]
     pub fn topic0<T: Into<Topic>>(mut self, topic: T) -> Self {
@@ -544,6 +552,12 @@ impl From<H256> for Topic {
     }
 }
 
+impl From<Vec<H256>> for ValueOrArray<H256> {
+    fn from(src: Vec<H256>) -> Self {
+        ValueOrArray::Array(src)
+    }
+}
+
 impl From<ValueOrArray<H256>> for Topic {
     fn from(src: ValueOrArray<H256>) -> Self {
         match src {
diff --git a/ethers-core/src/types/proof.rs b/ethers-core/src/types/proof.rs
index 011c3576..a1d9cc17 100644
--- a/ethers-core/src/types/proof.rs
+++ b/ethers-core/src/types/proof.rs
@@ -1,4 +1,4 @@
-use crate::types::{Address, Bytes, H256, U256};
+use crate::types::{Address, Bytes, H256, U256, U64};
 use serde::{Deserialize, Serialize};
 
 #[derive(Debug, Default, Clone, PartialEq, Eq, Deserialize, Serialize)]
@@ -11,13 +11,13 @@ pub struct StorageProof {
 #[derive(Debug, Default, Clone, PartialEq, Eq, Deserialize, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub struct EIP1186ProofResponse {
-    address: Address,
-    balance: U256,
-    code_hash: H256,
-    nonce: U256,
-    storage_hash: H256,
-    account_proof: Vec<Bytes>,
-    storage_proof: Vec<StorageProof>,
+    pub address: Address,
+    pub balance: U256,
+    pub code_hash: H256,
+    pub nonce: U64,
+    pub storage_hash: H256,
+    pub account_proof: Vec<Bytes>,
+    pub storage_proof: Vec<StorageProof>,
 }
 
 #[cfg(test)]
diff --git a/ethers-core/src/utils/mod.rs b/ethers-core/src/utils/mod.rs
index b1cf9ca2..5e3b1c2d 100644
--- a/ethers-core/src/utils/mod.rs
+++ b/ethers-core/src/utils/mod.rs
@@ -155,9 +155,15 @@ where
     S: ToString,
     K: TryInto<Units, Error = ConversionError> + Copy,
 {
-    use rust_decimal::Decimal;
+    use rust_decimal::{Decimal, MathematicalOps};
+
     let num: Decimal = amount.to_string().parse()?;
-    let multiplier: Decimal = 10u64.pow(units.try_into()?.as_num()).into();
+    let exponent = units.try_into()?.as_num();
+
+    let multiplier = Decimal::TEN
+        .checked_powu(exponent.into())
+        .ok_or(rust_decimal::Error::ExceedsMaximumPossibleValue)?;
+
     let val =
         num.checked_mul(multiplier).ok_or(rust_decimal::Error::ExceedsMaximumPossibleValue)?;
     let u256_n: U256 = U256::from_dec_str(&val.round().to_string())?;
@@ -441,6 +447,14 @@ mod tests {
         assert_eq!(eth, "1.005633240123456789");
     }
 
+    #[test]
+    fn parse_large_units() {
+        let decimals = 27u32;
+        let val = "10.55";
+        let unit = parse_units(val, decimals).unwrap();
+        assert_eq!(unit.to_string(), "10550000000000000000000000000");
+    }
+
     #[test]
     fn test_parse_units() {
         let gwei = parse_units(1.5, 9).unwrap();
diff --git a/ethers-middleware/src/signer.rs b/ethers-middleware/src/signer.rs
index e38ed7e1..b911a660 100644
--- a/ethers-middleware/src/signer.rs
+++ b/ethers-middleware/src/signer.rs
@@ -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(
diff --git a/ethers-providers/src/lib.rs b/ethers-providers/src/lib.rs
index 9e7ee807..f5f58fa3 100644
--- a/ethers-providers/src/lib.rs
+++ b/ethers-providers/src/lib.rs
@@ -20,7 +20,7 @@ mod pending_escalator;
 pub use pending_escalator::EscalatingPending;
 
 mod log_query;
-pub use log_query::LogQuery;
+pub use log_query::{LogQuery, LogQueryError};
 
 mod stream;
 pub use futures_util::StreamExt;
@@ -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(
diff --git a/ethers-providers/src/provider.rs b/ethers-providers/src/provider.rs
index d4d0a0eb..ed57facd 100644
--- a/ethers-providers/src/provider.rs
+++ b/ethers-providers/src/provider.rs
@@ -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();
diff --git a/ethers-solc/src/error.rs b/ethers-solc/src/error.rs
index fc00601c..72dbc95e 100644
--- a/ethers-solc/src/error.rs
+++ b/ethers-solc/src/error.rs
@@ -32,6 +32,13 @@ pub enum SolcError {
     Resolve(SolcIoError),
     #[error("File could not be resolved due to broken symlink: {0}.")]
     ResolveBadSymlink(SolcIoError),
+    #[error(
+        r#"Failed to resolve file: {0}.
+    --> {1:?}
+        {2:?}
+    Check configured remappings."#
+    )]
+    FailedResolveImport(SolcIoError, PathBuf, PathBuf),
     #[cfg(feature = "svm-solc")]
     #[error(transparent)]
     SvmError(#[from] svm::SolcVmError),
diff --git a/ethers-solc/src/resolver/mod.rs b/ethers-solc/src/resolver/mod.rs
index 945e30e8..252af2ca 100644
--- a/ethers-solc/src/resolver/mod.rs
+++ b/ethers-solc/src/resolver/mod.rs
@@ -368,7 +368,20 @@ impl Graph {
                     &mut resolved_solc_include_paths,
                 ) {
                     Ok(import) => {
-                        add_node(&mut unresolved, &mut index, &mut resolved_imports, import)?;
+                        add_node(&mut unresolved, &mut index, &mut resolved_imports, import)
+                            .map_err(|err| {
+                                match err {
+                                    SolcError::Resolve(err) => {
+                                        // make the error more verbose
+                                        SolcError::FailedResolveImport(
+                                            err,
+                                            node.path.clone(),
+                                            import_path.clone(),
+                                        )
+                                    }
+                                    _ => err,
+                                }
+                            })?
                     }
                     Err(err) => {
                         unresolved_imports.insert((import_path.to_path_buf(), node.path.clone()));


Test patch:
diff --git a/ethers-contract/tests/it/abigen.rs b/ethers-contract/tests/it/abigen.rs
index 5dd25efc..e8d2c441 100644
--- a/ethers-contract/tests/it/abigen.rs
+++ b/ethers-contract/tests/it/abigen.rs
@@ -557,17 +557,18 @@ async fn can_abiencoderv2_output() {
     assert_eq!(res, person);
 }
 
-#[test]
-fn can_gen_multi_etherscan() {
-    abigen!(
-        MyContract, "etherscan:0xdAC17F958D2ee523a2206206994597C13D831ec7";
-        MyContract2, "etherscan:0x8418bb725b3ac45ec8fff3791dd8b4e0480cc2a2";
-    );
-
-    let provider = Arc::new(Provider::new(MockProvider::new()));
-    let _contract = MyContract::new(Address::default(), Arc::clone(&provider));
-    let _contract = MyContract2::new(Address::default(), provider);
-}
+// NOTE: this is commented out because this would result in compiler errors if key not set or
+// etherscan API not working #[test]
+// fn can_gen_multi_etherscan() {
+//     abigen!(
+//         MyContract, "etherscan:0xdAC17F958D2ee523a2206206994597C13D831ec7";
+//         MyContract2, "etherscan:0x8418bb725b3ac45ec8fff3791dd8b4e0480cc2a2";
+//     );
+//
+//     let provider = Arc::new(Provider::new(MockProvider::new()));
+//     let _contract = MyContract::new(Address::default(), Arc::clone(&provider));
+//     let _contract = MyContract2::new(Address::default(), provider);
+// }
 
 #[test]
 fn can_gen_reserved_word_field_names() {