Repository: gakonst/ethers-rs
Pull request #362 (2021-08-02 10:49:53)

feat(ethers-contract): typed txs (part 4)
* Adds `TypedTransaction` inside the `ContractCall` object
* Uses EIP1559 Transactions by default (Legacy if `legacy` feature flag is on), allows falling back to legacy ones via the `legacy` helper function
* Most tests need to use `legacy()` because ganache does not support Berlin or London yet (?!?)

aidenbenner: Thanks for the amazing work on this library!
Is this ready to merge? This would be very useful now that the London fork has landed.
gakonst: I'll be working on the final bits to merge these PRs -today-, there's currently a serialization bug (along with some extra tests against testnets)

Patch:
diff --git a/ethers-contract/Cargo.toml b/ethers-contract/Cargo.toml
index 05f4fe78..e72a81ad 100644
--- a/ethers-contract/Cargo.toml
+++ b/ethers-contract/Cargo.toml
@@ -34,7 +34,8 @@ ethers-middleware = { version = "0.4.0", path = "../ethers-middleware" }
 
 [features]
 abigen = ["ethers-contract-abigen", "ethers-contract-derive"]
-celo = ["ethers-core/celo", "ethers-core/celo", "ethers-providers/celo"]
+celo = ["legacy", "ethers-core/celo", "ethers-core/celo", "ethers-providers/celo"]
+legacy = []
 
 [package.metadata.docs.rs]
 all-features = true
diff --git a/ethers-contract/src/call.rs b/ethers-contract/src/call.rs
index d9f18f30..b6f7edda 100644
--- a/ethers-contract/src/call.rs
+++ b/ethers-contract/src/call.rs
@@ -1,7 +1,9 @@
 use super::base::{decode_function_data, AbiError};
 use ethers_core::{
     abi::{Detokenize, Function, InvalidOutputType},
-    types::{Address, BlockId, Bytes, TransactionRequest, U256},
+    types::{
+        transaction::eip2718::TypedTransaction, Address, BlockId, Bytes, TransactionRequest, U256,
+    },
 };
 use ethers_providers::{Middleware, PendingTransaction, ProviderError};
 
@@ -48,7 +50,7 @@ pub enum ContractError<M: Middleware> {
 /// Helper for managing a transaction before submitting it to a node
 pub struct ContractCall<M, D> {
     /// The raw transaction object
-    pub tx: TransactionRequest,
+    pub tx: TypedTransaction,
     /// The ABI of the function being called
     pub function: Function,
     /// Optional block number to be used when calculating the transaction's gas and nonce
@@ -60,25 +62,39 @@ pub struct ContractCall<M, D> {
 impl<M, D: Detokenize> ContractCall<M, D> {
     /// Sets the `from` field in the transaction to the provided value
     pub fn from<T: Into<Address>>(mut self, from: T) -> Self {
-        self.tx.from = Some(from.into());
+        self.tx.set_from(from.into());
+        self
+    }
+
+    /// Uses a Legacy transaction instead of an EIP-1559 one to execute the call
+    pub fn legacy(mut self) -> Self {
+        self.tx = match self.tx {
+            TypedTransaction::Eip1559(inner) => {
+                let tx: TransactionRequest = inner.into();
+                TypedTransaction::Legacy(tx)
+            }
+            other => other,
+        };
         self
     }
 
     /// Sets the `gas` field in the transaction to the provided value
     pub fn gas<T: Into<U256>>(mut self, gas: T) -> Self {
-        self.tx.gas = Some(gas.into());
+        self.tx.set_gas(gas);
         self
     }
 
     /// Sets the `gas_price` field in the transaction to the provided value
+    /// If the internal transaction is an EIP-1559 one, then it sets both
+    /// `max_fee_per_gas` and `max_priority_fee_per_gas` to the same value
     pub fn gas_price<T: Into<U256>>(mut self, gas_price: T) -> Self {
-        self.tx.gas_price = Some(gas_price.into());
+        self.tx.set_gas_price(gas_price);
         self
     }
 
     /// Sets the `value` field in the transaction to the provided value
     pub fn value<T: Into<U256>>(mut self, value: T) -> Self {
-        self.tx.value = Some(value.into());
+        self.tx.set_value(value);
         self
     }
 
@@ -96,13 +112,13 @@ where
 {
     /// Returns the underlying transaction's ABI encoded data
     pub fn calldata(&self) -> Option<Bytes> {
-        self.tx.data.clone()
+        self.tx.data().cloned()
     }
 
     /// 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.clone().into())
+            .estimate_gas(&self.tx)
             .await
             .map_err(ContractError::MiddlewareError)
     }
@@ -119,7 +135,7 @@ where
     pub async fn call(&self) -> Result<D, ContractError<M>> {
         let bytes = self
             .client
-            .call(&self.tx.clone().into(), self.block)
+            .call(&self.tx.clone(), self.block)
             .await
             .map_err(ContractError::MiddlewareError)?;
 
diff --git a/ethers-contract/src/contract.rs b/ethers-contract/src/contract.rs
index c4d4e8c0..6ede8aa2 100644
--- a/ethers-contract/src/contract.rs
+++ b/ethers-contract/src/contract.rs
@@ -7,8 +7,14 @@ use crate::{
 
 use ethers_core::{
     abi::{Abi, Detokenize, Error, EventExt, Function, Tokenize},
-    types::{Address, Filter, NameOrAddress, Selector, TransactionRequest},
+    types::{Address, Filter, NameOrAddress, Selector},
 };
+
+#[cfg(not(feature = "legacy"))]
+use ethers_core::types::Eip1559TransactionRequest;
+#[cfg(feature = "legacy")]
+use ethers_core::types::TransactionRequest;
+
 use ethers_providers::Middleware;
 
 use std::{fmt::Debug, marker::PhantomData, sync::Arc};
@@ -220,12 +226,20 @@ impl<M: Middleware> Contract<M> {
     ) -> Result<ContractCall<M, D>, AbiError> {
         let data = encode_function_data(function, args)?;
 
-        // create the tx object
+        #[cfg(feature = "legacy")]
         let tx = TransactionRequest {
             to: Some(NameOrAddress::Address(self.address)),
             data: Some(data),
             ..Default::default()
         };
+        #[cfg(not(feature = "legacy"))]
+        let tx = Eip1559TransactionRequest {
+            to: Some(NameOrAddress::Address(self.address)),
+            data: Some(data),
+            ..Default::default()
+        };
+
+        let tx = tx.into();
 
         Ok(ContractCall {
             tx,
diff --git a/ethers-contract/src/factory.rs b/ethers-contract/src/factory.rs
index d0405ec8..ae818f06 100644
--- a/ethers-contract/src/factory.rs
+++ b/ethers-contract/src/factory.rs
@@ -2,17 +2,20 @@ use crate::{Contract, ContractError};
 
 use ethers_core::{
     abi::{Abi, Tokenize},
-    types::{BlockNumber, Bytes, TransactionRequest},
+    types::{transaction::eip2718::TypedTransaction, BlockNumber, Bytes, TransactionRequest},
 };
 use ethers_providers::Middleware;
 
+#[cfg(not(feature = "legacy"))]
+use ethers_core::types::Eip1559TransactionRequest;
+
 use std::sync::Arc;
 
 #[derive(Debug, Clone)]
 /// Helper which manages the deployment transaction of a smart contract
 pub struct Deployer<M> {
     /// The deployer's transaction, exposed for overriding the defaults
-    pub tx: TransactionRequest,
+    pub tx: TypedTransaction,
     abi: Abi,
     client: Arc<M>,
     confs: usize,
@@ -31,6 +34,18 @@ impl<M: Middleware> Deployer<M> {
         self
     }
 
+    /// Uses a Legacy transaction instead of an EIP-1559 one to do the deployment
+    pub fn legacy(mut self) -> Self {
+        self.tx = match self.tx {
+            TypedTransaction::Eip1559(inner) => {
+                let tx: TransactionRequest = inner.into();
+                TypedTransaction::Legacy(tx)
+            }
+            other => other,
+        };
+        self
+    }
+
     /// Broadcasts the contract deployment transaction and after waiting for it to
     /// be sufficiently confirmed (default: 1), it returns a [`Contract`](crate::Contract)
     /// struct at the deployed contract's address.
@@ -150,11 +165,21 @@ impl<M: Middleware> ContractFactory<M> {
         };
 
         // create the tx object. Since we're deploying a contract, `to` is `None`
+        // We default to EIP-1559 transactions, but the sender can convert it back
+        // to a legacy one
+        #[cfg(feature = "legacy")]
         let tx = TransactionRequest {
             to: None,
             data: Some(data),
             ..Default::default()
         };
+        #[cfg(not(feature = "legacy"))]
+        let tx = Eip1559TransactionRequest {
+            to: None,
+            data: Some(data),
+            ..Default::default()
+        };
+        let tx = tx.into();
 
         Ok(Deployer {
             client: Arc::clone(&self.client), // cheap clone behind the arc
diff --git a/ethers-contract/src/multicall/mod.rs b/ethers-contract/src/multicall/mod.rs
index 1b644327..11d4a108 100644
--- a/ethers-contract/src/multicall/mod.rs
+++ b/ethers-contract/src/multicall/mod.rs
@@ -137,6 +137,7 @@ pub struct Multicall<M> {
     calls: Vec<Call>,
     block: Option<BlockNumber>,
     contract: MulticallContract<M>,
+    legacy: bool,
 }
 
 #[derive(Clone)]
@@ -188,9 +189,16 @@ impl<M: Middleware> Multicall<M> {
             calls: vec![],
             block: None,
             contract,
+            legacy: false,
         })
     }
 
+    /// Makes a legacy transaction instead of an EIP-1559 one
+    pub fn legacy(mut self) -> Self {
+        self.legacy = true;
+        self
+    }
+
     /// Sets the `block` field for the multicall aggregate call
     pub fn block<T: Into<BlockNumber>>(mut self, block: T) -> Self {
         self.block = Some(block.into());
@@ -208,11 +216,11 @@ impl<M: Middleware> Multicall<M> {
             panic!("Cannot support more than {} calls", 16);
         }
 
-        match (call.tx.to, call.tx.data) {
+        match (call.tx.to(), call.tx.data()) {
             (Some(NameOrAddress::Address(target)), Some(data)) => {
                 let call = Call {
-                    target,
-                    data,
+                    target: *target,
+                    data: data.clone(),
                     function: call.function,
                 };
                 self.calls.push(call);
@@ -366,11 +374,15 @@ impl<M: Middleware> Multicall<M> {
             .collect();
 
         // Construct the ContractCall for `aggregate` function to broadcast the transaction
-        let contract_call = self.contract.aggregate(calls);
+        let mut contract_call = self.contract.aggregate(calls);
         if let Some(block) = self.block {
-            contract_call.block(block)
-        } else {
-            contract_call
-        }
+            contract_call = contract_call.block(block)
+        };
+
+        if self.legacy {
+            contract_call = contract_call.legacy();
+        };
+
+        contract_call
     }
 }
diff --git a/ethers-core/src/types/transaction/eip1559.rs b/ethers-core/src/types/transaction/eip1559.rs
index d8c2adda..0c80fe8a 100644
--- a/ethers-core/src/types/transaction/eip1559.rs
+++ b/ethers-core/src/types/transaction/eip1559.rs
@@ -168,3 +168,23 @@ impl Eip1559TransactionRequest {
         rlp.append(&self.access_list);
     }
 }
+
+impl From<Eip1559TransactionRequest> for super::request::TransactionRequest {
+    fn from(tx: Eip1559TransactionRequest) -> Self {
+        Self {
+            from: tx.from,
+            to: tx.to,
+            gas: tx.gas,
+            gas_price: tx.max_fee_per_gas,
+            value: tx.value,
+            data: tx.data,
+            nonce: tx.nonce,
+            #[cfg(feature = "celo")]
+            fee_currency: None,
+            #[cfg(feature = "celo")]
+            gateway_fee_recipient: None,
+            #[cfg(feature = "celo")]
+            gateway_fee: None,
+        }
+    }
+}
diff --git a/ethers-middleware/src/transformer/ds_proxy/mod.rs b/ethers-middleware/src/transformer/ds_proxy/mod.rs
index e55e4535..3e1dfa10 100644
--- a/ethers-middleware/src/transformer/ds_proxy/mod.rs
+++ b/ethers-middleware/src/transformer/ds_proxy/mod.rs
@@ -109,6 +109,7 @@ impl DsProxy {
         let ds_proxy_factory = DsProxyFactory::new(factory, client);
         let tx_receipt = ds_proxy_factory
             .build(owner)
+            .legacy()
             .send()
             .await?
             .await
diff --git a/ethers-providers/src/lib.rs b/ethers-providers/src/lib.rs
index 2b115c9b..c09d494f 100644
--- a/ethers-providers/src/lib.rs
+++ b/ethers-providers/src/lib.rs
@@ -240,6 +240,10 @@ pub trait Middleware: Sync + Send + Debug {
                     inner.tx.to = Some(addr.into());
                 };
 
+                if inner.tx.from.is_none() {
+                    inner.tx.from = self.default_sender();
+                }
+
                 let (gas_price, gas) = futures_util::try_join!(
                     maybe(inner.tx.gas_price, self.get_gas_price()),
                     maybe(inner.tx.gas, self.estimate_gas(&tx_clone)),
@@ -257,6 +261,10 @@ pub trait Middleware: Sync + Send + Debug {
                     inner.to = Some(addr.into());
                 };
 
+                if inner.from.is_none() {
+                    inner.from = self.default_sender();
+                }
+
                 let (max_priority_fee_per_gas, max_fee_per_gas, gas) = futures_util::try_join!(
                     // TODO: Replace with algorithms using eth_feeHistory
                     maybe(inner.max_priority_fee_per_gas, self.get_gas_price()),
diff --git a/ethers-signers/src/aws/mod.rs b/ethers-signers/src/aws/mod.rs
index e99ee1b5..8cc668d7 100644
--- a/ethers-signers/src/aws/mod.rs
+++ b/ethers-signers/src/aws/mod.rs
@@ -2,7 +2,7 @@
 
 use ethers_core::{
     k256::ecdsa::{Error as K256Error, Signature as KSig, VerifyingKey},
-    types::{Address, Signature as EthSig, H256, transaction::eip2718::TypedTransaction},
+    types::{transaction::eip2718::TypedTransaction, Address, Signature as EthSig, H256},
     utils::hash_message,
 };
 use rusoto_core::RusotoError;


Test patch:
diff --git a/ethers-contract/tests/common/mod.rs b/ethers-contract/tests/common/mod.rs
index 8404d038..83386852 100644
--- a/ethers-contract/tests/common/mod.rs
+++ b/ethers-contract/tests/common/mod.rs
@@ -50,6 +50,7 @@ pub async fn deploy<M: Middleware>(client: Arc<M>, abi: Abi, bytecode: Bytes) ->
     factory
         .deploy("initial value".to_string())
         .unwrap()
+        .legacy()
         .send()
         .await
         .unwrap()
diff --git a/ethers-contract/tests/contract.rs b/ethers-contract/tests/contract.rs
index 773bb868..67f04835 100644
--- a/ethers-contract/tests/contract.rs
+++ b/ethers-contract/tests/contract.rs
@@ -32,7 +32,10 @@ mod eth_tests {
         // `send` consumes the deployer so it must be cloned for later re-use
         // (practically it's not expected that you'll need to deploy multiple instances of
         // the _same_ deployer, so it's fine to clone here from a dev UX vs perf tradeoff)
-        let deployer = factory.deploy("initial value".to_string()).unwrap();
+        let deployer = factory
+            .deploy("initial value".to_string())
+            .unwrap()
+            .legacy();
         let contract = deployer.clone().send().await.unwrap();
 
         let get_value = contract.method::<_, String>("getValue", ()).unwrap();
@@ -51,6 +54,7 @@ mod eth_tests {
             .unwrap();
         let calldata = contract_call.calldata().unwrap();
         let gas_estimate = contract_call.estimate_gas().await.unwrap();
+        let contract_call = contract_call.legacy();
         let pending_tx = contract_call.send().await.unwrap();
         let tx = client.get_transaction(*pending_tx).await.unwrap().unwrap();
         let tx_receipt = pending_tx.await.unwrap().unwrap();
@@ -82,6 +86,7 @@ mod eth_tests {
         let _tx_hash = contract
             .method::<_, H256>("setValues", ("hi".to_owned(), "bye".to_owned()))
             .unwrap()
+            .legacy()
             .send()
             .await
             .unwrap()
@@ -99,7 +104,8 @@ mod eth_tests {
         // make a call with `client`
         let func = contract
             .method::<_, H256>("setValue", "hi".to_owned())
-            .unwrap();
+            .unwrap()
+            .legacy();
         let tx = func.send().await.unwrap();
         let _receipt = tx.await.unwrap();
 
@@ -171,6 +177,7 @@ mod eth_tests {
         let value = contract
             .method::<_, String>("getValue", ())
             .unwrap()
+            .legacy()
             .call()
             .await
             .unwrap();
@@ -180,6 +187,7 @@ mod eth_tests {
         let _tx_hash = *contract
             .method::<_, H256>("setValue", "hi".to_owned())
             .unwrap()
+            .legacy()
             .send()
             .await
             .unwrap();
@@ -188,6 +196,7 @@ mod eth_tests {
         let value = contract
             .method::<_, String>("getValue", ())
             .unwrap()
+            .legacy()
             .call()
             .await
             .unwrap();
@@ -197,6 +206,7 @@ mod eth_tests {
         let value = contract
             .method::<_, String>("getValue", ())
             .unwrap()
+            .legacy()
             .block(BlockId::Number(deployed_block.into()))
             .call()
             .await
@@ -302,7 +312,8 @@ mod eth_tests {
         for i in 0..num_calls {
             let call = contract
                 .method::<_, H256>("setValue", i.to_string())
-                .unwrap();
+                .unwrap()
+                .legacy();
             let pending_tx = call.send().await.unwrap();
             let _receipt = pending_tx.await.unwrap();
         }
@@ -341,7 +352,6 @@ mod eth_tests {
         // get the first account
         let deployer = provider.get_accounts().await.unwrap()[0];
         let client = Arc::new(provider.with_sender(deployer));
-        dbg!(deployer);
 
         let contract = deploy(client, abi, bytecode).await;
 
@@ -396,18 +406,26 @@ mod eth_tests {
         let not_so_simple_factory =
             ContractFactory::new(not_so_simple_abi, not_so_simple_bytecode, client3.clone());
 
-        let multicall_contract = multicall_factory.deploy(()).unwrap().send().await.unwrap();
+        let multicall_contract = multicall_factory
+            .deploy(())
+            .unwrap()
+            .legacy()
+            .send()
+            .await
+            .unwrap();
         let addr = multicall_contract.address();
 
         let simple_contract = simple_factory
             .deploy("the first one".to_string())
             .unwrap()
+            .legacy()
             .send()
             .await
             .unwrap();
         let not_so_simple_contract = not_so_simple_factory
             .deploy("the second one".to_string())
             .unwrap()
+            .legacy()
             .send()
             .await
             .unwrap();
@@ -417,6 +435,7 @@ mod eth_tests {
             .connect(client2.clone())
             .method::<_, H256>("setValue", "reset first".to_owned())
             .unwrap()
+            .legacy()
             .send()
             .await
             .unwrap();
@@ -424,6 +443,7 @@ mod eth_tests {
             .connect(client3.clone())
             .method::<_, H256>("setValue", "reset second".to_owned())
             .unwrap()
+            .legacy()
             .send()
             .await
             .unwrap();
@@ -479,7 +499,7 @@ mod eth_tests {
             .add_call(broadcast2);
 
         // broadcast the transaction and wait for it to be mined
-        let tx_hash = multicall_send.send().await.unwrap();
+        let tx_hash = multicall_send.legacy().send().await.unwrap();
         let _tx_receipt = PendingTransaction::new(tx_hash, client.provider())
             .await
             .unwrap();
@@ -544,7 +564,10 @@ mod celo_tests {
         let client = Arc::new(client);
 
         let factory = ContractFactory::new(abi, bytecode, client);
-        let deployer = factory.deploy("initial value".to_string()).unwrap();
+        let deployer = factory
+            .deploy("initial value".to_string())
+            .unwrap()
+            .legacy();
         let contract = deployer.block(BlockNumber::Pending).send().await.unwrap();
 
         let value: String = contract
diff --git a/ethers-middleware/tests/transformer.rs b/ethers-middleware/tests/transformer.rs
index 630447e5..1a7addf2 100644
--- a/ethers-middleware/tests/transformer.rs
+++ b/ethers-middleware/tests/transformer.rs
@@ -44,7 +44,8 @@ async fn ds_proxy_transformer() {
         contract.bytecode.clone(),
         Arc::clone(&provider),
     );
-    let ds_proxy_factory = factory.deploy(()).unwrap().send().await.unwrap();
+    let ds_proxy_factory = factory.deploy(()).unwrap().legacy();
+    let ds_proxy_factory = ds_proxy_factory.send().await.unwrap();
 
     // deploy a new DsProxy contract.
     let ds_proxy = DsProxy::build::<HttpWallet, Arc<HttpWallet>>(
@@ -68,7 +69,8 @@ async fn ds_proxy_transformer() {
         contract.bytecode.clone(),
         Arc::clone(&provider),
     );
-    let simple_storage = factory.deploy(()).unwrap().send().await.unwrap();
+    let deployer = factory.deploy(()).unwrap().legacy();
+    let simple_storage = deployer.send().await.unwrap();
 
     // instantiate a new transformer middleware.
     let provider = TransformerMiddleware::new(signer_middleware, ds_proxy.clone());
@@ -131,7 +133,8 @@ async fn ds_proxy_code() {
         contract.bytecode.clone(),
         Arc::clone(&provider),
     );
-    let ds_proxy_factory = factory.deploy(()).unwrap().send().await.unwrap();
+    let ds_proxy_factory = factory.deploy(()).unwrap().legacy();
+    let ds_proxy_factory = ds_proxy_factory.send().await.unwrap();
 
     // deploy a new DsProxy contract.
     let ds_proxy = DsProxy::build::<HttpWallet, Arc<HttpWallet>>(
@@ -164,6 +167,7 @@ async fn ds_proxy_code() {
             calldata,
         )
         .expect("could not construct DSProxy contract call")
+        .legacy()
         .send()
         .await
         .unwrap();