From 123f7f3a485626b67d0dcb95d5a86fc0dd31fe97 Mon Sep 17 00:00:00 2001 From: wolflo <33909953+wolflo@users.noreply.github.com> Date: Mon, 26 Jul 2021 10:50:52 -0600 Subject: [PATCH] fix: preserve from field in SignerMiddleware Fixes #349 --- ethers-middleware/src/signer.rs | 55 ++++++++++++++++++++++++ ethers-middleware/tests/signer.rs | 70 +++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+) diff --git a/ethers-middleware/src/signer.rs b/ethers-middleware/src/signer.rs index ae649a4c6..acda54271 100644 --- a/ethers-middleware/src/signer.rs +++ b/ethers-middleware/src/signer.rs @@ -99,6 +99,9 @@ pub enum SignerMiddlewareError { /// Thrown if the `gas` field is missing #[error("no gas was specified")] GasMissing, + /// Thrown if a signature is requested from a different address + #[error("specified from address is not signer")] + WrongSigner, } // Helper functions for locally signing transactions @@ -126,6 +129,11 @@ where let gas_price = tx.gas_price.ok_or(SignerMiddlewareError::GasPriceMissing)?; let gas = tx.gas.ok_or(SignerMiddlewareError::GasMissing)?; + // Can't sign a transaction from a different address + if tx.from.is_some() && tx.from != Some(self.address()) { + return Err(SignerMiddlewareError::WrongSigner); + } + let signature = self .signer .sign_transaction(&tx) @@ -247,6 +255,15 @@ where mut tx: TransactionRequest, block: Option, ) -> Result, Self::Error> { + // If the from address is set and is not our signer, delegate to inner + if tx.from.is_some() && tx.from != Some(self.address()) { + return self + .inner + .send_transaction(tx, block) + .await + .map_err(SignerMiddlewareError::MiddlewareError); + } + if let Some(NameOrAddress::Name(ens_name)) = tx.to { let addr = self .inner @@ -341,4 +358,42 @@ mod tests { let expected_rlp = Bytes::from(hex::decode("f869808504e3b29200831e848094f0109fc8df283027b6285cc889f5aa624eac1f55843b9aca008025a0c9cf86333bcb065d140032ecaab5d9281bde80f21b9687b3e94161de42d51895a0727a108a0b8d101465414033c3f705a9c7b826e596766046ee1183dbc8aeaa68").unwrap()); assert_eq!(tx.rlp(), expected_rlp); } + + #[tokio::test] + async fn handles_tx_from_field() { + use ethers_core::types::Address; + + // new SignerMiddleware + let provider = Provider::try_from("http://localhost:8545").unwrap(); + let key = LocalWallet::new(&mut rand::thread_rng()); + let client = SignerMiddleware::new(provider, key); + + // an address that is not the signer address + let other = "0x863DF6BFa4469f3ead0bE8f9F2AAE51c91A907b4" + .parse::
() + .unwrap(); + + let request = TransactionRequest::new().nonce(0).gas_price(0).gas(0); + + // signing a TransactionRequest with a from field of None should yield + // a signed transaction from the signer address + let request_from_none = request.clone(); + let signing_result = client.sign_transaction(request_from_none).await; + + assert_eq!(signing_result.unwrap().from, client.address()); + + // signing a TransactionRequest with the signer as the from address + // should yield a signed transaction from the signer + let request_from_signer = request.clone().from(client.address()); + let signing_result = client.sign_transaction(request_from_signer.clone()).await; + + assert_eq!(signing_result.unwrap().from, client.address()); + + // signing a TransactionRequest with a from address that is not the + // signer should result in a WrongSigner error + let request_from_other = request.from(other); + let signing_result = client.sign_transaction(request_from_other.clone()).await; + + assert!(signing_result.is_err()); + } } diff --git a/ethers-middleware/tests/signer.rs b/ethers-middleware/tests/signer.rs index c1c13ba55..0c7419f7c 100644 --- a/ethers-middleware/tests/signer.rs +++ b/ethers-middleware/tests/signer.rs @@ -69,3 +69,73 @@ async fn test_send_transaction() { let balance_after = client.get_balance(client.address(), None).await.unwrap(); assert!(balance_before > balance_after); } + +#[tokio::test] +async fn send_transaction_handles_tx_from_field() { + use ethers_core::utils::Ganache; + + // launch ganache + let ganache = Ganache::new().spawn(); + + // grab 2 wallets + let signer: LocalWallet = ganache.keys()[0].clone().into(); + let other: LocalWallet = ganache.keys()[1].clone().into(); + + // connect to the network + let provider = Provider::try_from(ganache.endpoint()).unwrap(); + let provider = SignerMiddleware::new(provider, signer.clone()); + + // sending a TransactionRequest with a from field of None should result + // in a transaction from the signer address + let request_from_none = TransactionRequest::new(); + let receipt = provider + .send_transaction(request_from_none, None) + .await + .unwrap() + .await + .unwrap() + .unwrap(); + let sent_tx = provider + .get_transaction(receipt.transaction_hash) + .await + .unwrap() + .unwrap(); + + assert_eq!(sent_tx.from, signer.address()); + + // sending a TransactionRequest with the signer as the from address should + // result in a transaction from the signer address + let request_from_signer = TransactionRequest::new().from(signer.address()); + let receipt = provider + .send_transaction(request_from_signer, None) + .await + .unwrap() + .await + .unwrap() + .unwrap(); + let sent_tx = provider + .get_transaction(receipt.transaction_hash) + .await + .unwrap() + .unwrap(); + + assert_eq!(sent_tx.from, signer.address()); + + // sending a TransactionRequest with a from address that is not the signer + // should result in a transaction from the specified address + let request_from_other = TransactionRequest::new().from(other.address()); + let receipt = provider + .send_transaction(request_from_other, None) + .await + .unwrap() + .await + .unwrap() + .unwrap(); + let sent_tx = provider + .get_transaction(receipt.transaction_hash) + .await + .unwrap() + .unwrap(); + + assert_eq!(sent_tx.from, other.address()); +}