diff --git a/ethers-signers/src/aws/mod.rs b/ethers-signers/src/aws/mod.rs index 798cb2dbd..784f59d6f 100644 --- a/ethers-signers/src/aws/mod.rs +++ b/ethers-signers/src/aws/mod.rs @@ -203,15 +203,19 @@ impl<'a> AwsSigner<'a> { } /// Sign a digest with this signer's key and add the eip155 `v` value - /// corresponding to this signer's chain_id + /// corresponding to the input chain_id #[instrument(err, skip(digest), fields(digest = %hex::encode(&digest)))] - async fn sign_digest_with_eip155(&self, digest: H256) -> Result { + async fn sign_digest_with_eip155( + &self, + digest: H256, + chain_id: u64, + ) -> Result { let sig = self.sign_digest(digest.into()).await?; let sig = utils::rsig_from_digest_bytes_trial_recovery(&sig, digest.into(), &self.pubkey); let mut sig = rsig_to_ethsig(&sig); - apply_eip155(&mut sig, self.chain_id); + apply_eip155(&mut sig, chain_id); Ok(sig) } } @@ -230,13 +234,17 @@ impl<'a> super::Signer for AwsSigner<'a> { trace!("{:?}", message_hash); trace!("{:?}", message); - self.sign_digest_with_eip155(message_hash).await + self.sign_digest_with_eip155(message_hash, self.chain_id).await } #[instrument(err)] async fn sign_transaction(&self, tx: &TypedTransaction) -> Result { + let mut tx_with_chain = tx.clone(); + let chain_id = tx_with_chain.chain_id().map(|id| id.as_u64()).unwrap_or(self.chain_id); + tx_with_chain.set_chain_id(chain_id); + let sighash = tx.sighash(); - self.sign_digest_with_eip155(sighash).await + self.sign_digest_with_eip155(sighash, chain_id).await } async fn sign_typed_data( @@ -245,7 +253,7 @@ impl<'a> super::Signer for AwsSigner<'a> { ) -> Result { let hash = payload.encode_eip712().map_err(|e| Self::Error::Eip712Error(e.to_string()))?; - let digest = self.sign_digest_with_eip155(hash.into()).await?; + let digest = self.sign_digest_with_eip155(hash.into(), self.chain_id).await?; Ok(digest) } diff --git a/ethers-signers/src/ledger/app.rs b/ethers-signers/src/ledger/app.rs index 346231fc0..02fa0c872 100644 --- a/ethers-signers/src/ledger/app.rs +++ b/ethers-signers/src/ledger/app.rs @@ -117,8 +117,13 @@ impl LedgerEthereum { /// Signs an Ethereum transaction (requires confirmation on the ledger) pub async fn sign_tx(&self, tx: &TypedTransaction) -> Result { + let mut tx_with_chain = tx.clone(); + if tx_with_chain.chain_id().is_none() { + // in the case we don't have a chain_id, let's use the signer chain id instead + tx_with_chain.set_chain_id(self.chain_id); + } let mut payload = Self::path_to_bytes(&self.derivation); - payload.extend_from_slice(tx.rlp().as_ref()); + payload.extend_from_slice(tx_with_chain.rlp().as_ref()); self.sign_payload(INS::SIGN, payload).await } diff --git a/ethers-signers/src/ledger/mod.rs b/ethers-signers/src/ledger/mod.rs index 9f24e13b0..d233e8980 100644 --- a/ethers-signers/src/ledger/mod.rs +++ b/ethers-signers/src/ledger/mod.rs @@ -25,7 +25,12 @@ impl Signer for LedgerEthereum { /// Signs the transaction async fn sign_transaction(&self, message: &TypedTransaction) -> Result { - self.sign_tx(message).await + let mut tx_with_chain = message.clone(); + if tx_with_chain.chain_id().is_none() { + // in the case we don't have a chain_id, let's use the signer chain id instead + tx_with_chain.set_chain_id(self.chain_id); + } + self.sign_tx(tx_with_chain).await } /// Signs a EIP712 derived struct diff --git a/ethers-signers/src/trezor/app.rs b/ethers-signers/src/trezor/app.rs index d67d6b228..5994c79ad 100644 --- a/ethers-signers/src/trezor/app.rs +++ b/ethers-signers/src/trezor/app.rs @@ -160,6 +160,8 @@ impl TrezorEthereum { let transaction = TrezorTransaction::load(tx)?; + let chain_id = tx.chain_id().map(|id| id.as_u64()).unwrap_or(self.chain_id); + let signature = match tx { TypedTransaction::Eip2930(_) | TypedTransaction::Legacy(_) => client.ethereum_sign_tx( arr_path, @@ -169,7 +171,7 @@ impl TrezorEthereum { transaction.to, transaction.value, transaction.data, - self.chain_id, + chain_id, )?, TypedTransaction::Eip1559(eip1559_tx) => client.ethereum_sign_eip1559_tx( arr_path, @@ -178,7 +180,7 @@ impl TrezorEthereum { transaction.to, transaction.value, transaction.data, - self.chain_id, + chain_id, transaction.max_fee_per_gas, transaction.max_priority_fee_per_gas, transaction.access_list, diff --git a/ethers-signers/src/trezor/mod.rs b/ethers-signers/src/trezor/mod.rs index 060959b6a..4bf0e5b57 100644 --- a/ethers-signers/src/trezor/mod.rs +++ b/ethers-signers/src/trezor/mod.rs @@ -25,7 +25,12 @@ impl Signer for TrezorEthereum { /// Signs the transaction async fn sign_transaction(&self, message: &TypedTransaction) -> Result { - self.sign_tx(message).await + let mut tx_with_chain = message.clone(); + if tx_with_chain.chain_id().is_none() { + // in the case we don't have a chain_id, let's use the signer chain id instead + tx_with_chain.set_chain_id(self.chain_id); + } + self.sign_tx(&tx_with_chain).await } /// Signs a EIP712 derived struct diff --git a/ethers-signers/src/wallet/mod.rs b/ethers-signers/src/wallet/mod.rs index efde9cf2b..5f44ef6fa 100644 --- a/ethers-signers/src/wallet/mod.rs +++ b/ethers-signers/src/wallet/mod.rs @@ -18,7 +18,7 @@ use ethers_core::{ }, types::{ transaction::{eip2718::TypedTransaction, eip712::Eip712}, - Address, Signature, H256, U256, U64, + Address, Signature, H256, U256, }, utils::hash_message, }; @@ -79,27 +79,16 @@ impl> Signer fo let message = message.as_ref(); let message_hash = hash_message(message); - Ok(self.sign_hash(message_hash, false)) + Ok(self.sign_hash(message_hash)) } async fn sign_transaction(&self, tx: &TypedTransaction) -> Result { - let chain_id = tx.chain_id(); - match chain_id { - Some(id) => { - if U64::from(self.chain_id) != id { - return Err(WalletError::InvalidTransactionError( - "transaction chain_id does not match the signer".to_string(), - )) - } - Ok(self.sign_transaction_sync(tx)) - } - None => { - // in the case we don't have a chain_id, let's use the signer chain id instead - let mut tx_with_chain = tx.clone(); - tx_with_chain.set_chain_id(self.chain_id); - Ok(self.sign_transaction_sync(&tx_with_chain)) - } + let mut tx_with_chain = tx.clone(); + if tx_with_chain.chain_id() == None { + // in the case we don't have a chain_id, let's use the signer chain id instead + tx_with_chain.set_chain_id(self.chain_id); } + Ok(self.sign_transaction_sync(&tx_with_chain)) } async fn sign_typed_data( @@ -109,7 +98,7 @@ impl> Signer fo let encoded = payload.encode_eip712().map_err(|e| Self::Error::Eip712Error(e.to_string()))?; - Ok(self.sign_hash(H256::from(encoded), false)) + Ok(self.sign_hash(H256::from(encoded))) } fn address(&self) -> Address { @@ -129,23 +118,24 @@ impl> Signer fo } impl> Wallet { - /// Synchronously signs the provided transaction. + /// Synchronously signs the provided transaction, normalizing the signature `v` value with + /// EIP-155 using the transaction's `chain_id`. pub fn sign_transaction_sync(&self, tx: &TypedTransaction) -> Signature { let sighash = tx.sighash(); - self.sign_hash(sighash, true) + let chain_id = tx.chain_id().map(|id| id.as_u64()).unwrap_or(self.chain_id); + let mut sig = self.sign_hash(sighash); + + // sign_hash sets `v` to recid + 27, so we need to subtract 27 before normalizing + sig.v = to_eip155_v(sig.v as u8 - 27, chain_id); + sig } - /// Signs the provided hash and proceeds to normalize the `v` value of the - /// signature with EIP-155 if the flag is set to true. - pub fn sign_hash(&self, hash: H256, eip155: bool) -> Signature { + /// Signs the provided hash. + pub fn sign_hash(&self, hash: H256) -> Signature { let recoverable_sig: RecoverableSignature = self.signer.sign_digest(Sha256Proxy::from(hash)); - let v = if eip155 { - to_eip155_v(recoverable_sig.recovery_id(), self.chain_id) - } else { - u8::from(recoverable_sig.recovery_id()) as u64 + 27 - }; + let v = u8::from(recoverable_sig.recovery_id()) as u64 + 27; let r_bytes: FieldBytes = recoverable_sig.r().into(); let s_bytes: FieldBytes = recoverable_sig.s().into(); diff --git a/ethers-signers/src/wallet/private_key.rs b/ethers-signers/src/wallet/private_key.rs index eb3f2cfb0..d2bd75fa9 100644 --- a/ethers-signers/src/wallet/private_key.rs +++ b/ethers-signers/src/wallet/private_key.rs @@ -46,8 +46,6 @@ pub enum WalletError { /// Error type from Eip712Error message #[error("error encoding eip712 struct: {0:?}")] Eip712Error(String), - #[error("invalid transaction: {0:?}")] - InvalidTransactionError(String), } impl Clone for Wallet { @@ -221,6 +219,39 @@ mod tests { assert!(sig.verify(sighash, wallet.address).is_ok()); } + #[tokio::test] + #[cfg(not(feature = "celo"))] + async fn signs_tx_empty_chain_id() { + use crate::TypedTransaction; + use ethers_core::types::TransactionRequest; + // retrieved test vector from: + // https://web3js.readthedocs.io/en/v1.2.0/web3-eth-accounts.html#eth-accounts-signtransaction + let tx: TypedTransaction = TransactionRequest { + from: None, + to: Some("F0109fC8DF283027b6285cc889F5aA624EaC1F55".parse::
().unwrap().into()), + value: Some(1_000_000_000.into()), + gas: Some(2_000_000.into()), + nonce: Some(0.into()), + gas_price: Some(21_000_000_000u128.into()), + data: None, + chain_id: None, + } + .into(); + let wallet: Wallet = + "4c0883a69102937d6231471b5dbb6204fe5129617082792ae468d01a3f362318".parse().unwrap(); + let wallet = wallet.with_chain_id(1u64); + + // this should populate the tx chain_id as the signer's chain_id (1) before signing + let sig = wallet.sign_transaction(&tx).await.unwrap(); + + // since we initialize with None we need to re-set the chain_id for the sighash to be + // correct + let mut tx = tx; + tx.set_chain_id(1); + let sighash = tx.sighash(); + assert!(sig.verify(sighash, wallet.address).is_ok()); + } + #[test] fn key_to_address() { let wallet: Wallet =