From 25c2e0e1992e2b509093b7ba873b0259bcae6fa1 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Mon, 9 Aug 2021 03:50:38 +0300 Subject: [PATCH] feat(ethers-contract): typed txs (part 4) (#362) * feat(contract): use eip-1559 txs by default This may break any tests which use Ganache, since it only supports legacy transactions. We should replace Ganache with hardhat where possible * fix(providers): always try setting default_sender * chore: make all test transactions legacy * feat(multicall): allow submitting legacy txs * chore: default to legacy txs via feature flag This is useful for chains like Celo that do not support the Typed Envelope * fix: use legacy txs in ds proxy deployment / tests * chore: fix review comments --- ethers-contract/Cargo.toml | 3 +- ethers-contract/src/call.rs | 34 ++++++++++++----- ethers-contract/src/contract.rs | 18 ++++++++- ethers-contract/src/factory.rs | 29 ++++++++++++++- ethers-contract/src/multicall/mod.rs | 28 ++++++++++---- ethers-contract/tests/common/mod.rs | 1 + ethers-contract/tests/contract.rs | 37 +++++++++++++++---- ethers-core/src/types/transaction/eip1559.rs | 20 ++++++++++ .../src/transformer/ds_proxy/mod.rs | 1 + ethers-middleware/tests/transformer.rs | 10 +++-- ethers-providers/src/lib.rs | 8 ++++ ethers-signers/src/aws/mod.rs | 2 +- 12 files changed, 158 insertions(+), 33 deletions(-) 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 { /// Helper for managing a transaction before submitting it to a node pub struct ContractCall { /// 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 { impl ContractCall { /// Sets the `from` field in the transaction to the provided value pub fn from>(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>(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>(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>(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 { - 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> { 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> { 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 Contract { ) -> Result, 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 { /// The deployer's transaction, exposed for overriding the defaults - pub tx: TransactionRequest, + pub tx: TypedTransaction, abi: Abi, client: Arc, confs: usize, @@ -31,6 +34,18 @@ impl Deployer { 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 ContractFactory { }; // 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 { calls: Vec, block: Option, contract: MulticallContract, + legacy: bool, } #[derive(Clone)] @@ -188,9 +189,16 @@ impl Multicall { 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>(mut self, block: T) -> Self { self.block = Some(block.into()); @@ -208,11 +216,11 @@ impl Multicall { 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 Multicall { .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-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(client: Arc, 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-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 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-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::>( @@ -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::>( @@ -164,6 +167,7 @@ async fn ds_proxy_code() { calldata, ) .expect("could not construct DSProxy contract call") + .legacy() .send() .await .unwrap(); 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;