fix: preserve from field in SignerMiddleware (#350)

Fixes #349

ethers-middleware/src/signer.rs

@@ -99,6 +99,9 @@ pub enum SignerMiddlewareError<M: Middleware, S: Signer> {
     /// 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<BlockId>,
     ) -> Result<PendingTransaction<'_, Self::Provider>, 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::<Address>()
+            .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());
+    }
 }

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());
+}