fix: use custom address recovery for fake signature (#4890)

NomicFoundation · Feb 22, 2024 · 5aab752 · 5aab752
1 parent 4298679
commit 5aab752
Show file tree

Hide file tree

Showing 16 changed files with 183 additions and 18 deletions.
diff --git a/crates/edr_eth/src/transaction.rs b/crates/edr_eth/src/transaction.rs
@@ -4,6 +4,7 @@
 
 //! transaction related data
 
+mod fake_signature;
 mod kind;
 mod request;
 mod signed;

diff --git a/...src/transaction/request/fake_signature.rs → ...edr_eth/src/transaction/fake_signature.rs b/...src/transaction/request/fake_signature.rs → ...edr_eth/src/transaction/fake_signature.rs
@@ -26,6 +26,11 @@ pub(super) fn make_fake_signature<const V: usize>(sender: &Address) -> Signature
     Signature { r, s, v }
 }
 
+pub(super) fn recover_fake_signature(signature: &Signature) -> Address {
+    let address: [u8; 32] = signature.r.to_be_bytes();
+    Address::from_slice(&address.as_slice()[12..])
+}
+
 #[cfg(test)]
 pub(super) mod tests {
     macro_rules! test_fake_sign_properties {
@@ -60,6 +65,22 @@ pub(super) mod tests {
 
                 assert_ne!(hash_one, hash_two);
             }
+
+            #[test]
+            fn recovers_fake_sender() {
+                let transaction_request = dummy_request();
+
+                // Fails to recover with signature error if tried to ecrocver a fake signature
+                let sender: Address = "0x67091a7dd65bf4f1e95af0a479fbc782b61c129a"
+                    .parse()
+                    .expect("valid address");
+
+                let signed_transaction = transaction_request.fake_sign(&sender);
+
+                let recovered = signed_transaction.recover().expect("valid signature");
+
+                assert_eq!(recovered, sender);
+            }
         };
     }
 

diff --git a/crates/edr_eth/src/transaction/request.rs b/crates/edr_eth/src/transaction/request.rs
@@ -2,7 +2,6 @@ mod eip155;
 mod eip1559;
 mod eip2930;
 mod eip4844;
-mod fake_signature;
 mod legacy;
 
 use k256::SecretKey;

diff --git a/crates/edr_eth/src/transaction/request/eip155.rs b/crates/edr_eth/src/transaction/request/eip155.rs
@@ -7,8 +7,7 @@ use revm_primitives::keccak256;
 use crate::{
     signature::{Signature, SignatureError},
     transaction::{
-        kind::TransactionKind, request::fake_signature::make_fake_signature,
-        signed::Eip155SignedTransaction,
+        fake_signature::make_fake_signature, kind::TransactionKind, signed::Eip155SignedTransaction,
     },
     Address, Bytes, B256, U256,
 };
@@ -47,6 +46,7 @@ impl Eip155TransactionRequest {
             input: self.input,
             signature,
             hash: OnceLock::new(),
+            is_fake: false,
         })
     }
 
@@ -64,6 +64,7 @@ impl Eip155TransactionRequest {
             input: self.input,
             signature,
             hash: OnceLock::new(),
+            is_fake: true,
         }
     }
 
@@ -132,7 +133,7 @@ mod tests {
     use std::str::FromStr;
 
     use super::*;
-    use crate::transaction::request::fake_signature::tests::test_fake_sign_properties;
+    use crate::transaction::fake_signature::tests::test_fake_sign_properties;
 
     fn dummy_request() -> Eip155TransactionRequest {
         let to = Address::from_str("0xc014ba5ec014ba5ec014ba5ec014ba5ec014ba5e").unwrap();

diff --git a/crates/edr_eth/src/transaction/request/eip1559.rs b/crates/edr_eth/src/transaction/request/eip1559.rs
@@ -8,7 +8,7 @@ use crate::{
     access_list::AccessListItem,
     signature::{Signature, SignatureError},
     transaction::{
-        kind::TransactionKind, request::fake_signature::make_fake_signature,
+        fake_signature::make_fake_signature, kind::TransactionKind,
         signed::Eip1559SignedTransaction,
     },
     utils::envelop_bytes,
@@ -56,6 +56,7 @@ impl Eip1559TransactionRequest {
             r: signature.r,
             s: signature.s,
             hash: OnceLock::new(),
+            is_fake: false,
         })
     }
 
@@ -76,6 +77,7 @@ impl Eip1559TransactionRequest {
             r: signature.r,
             s: signature.s,
             hash: OnceLock::new(),
+            is_fake: true,
         }
     }
 }
@@ -101,7 +103,7 @@ pub(crate) mod tests {
     use std::str::FromStr;
 
     use super::*;
-    use crate::transaction::request::fake_signature::tests::test_fake_sign_properties;
+    use crate::transaction::fake_signature::tests::test_fake_sign_properties;
 
     fn dummy_request() -> Eip1559TransactionRequest {
         let to = Address::from_str("0xc014ba5ec014ba5ec014ba5ec014ba5ec014ba5e").unwrap();

diff --git a/crates/edr_eth/src/transaction/request/eip2930.rs b/crates/edr_eth/src/transaction/request/eip2930.rs
@@ -8,7 +8,7 @@ use crate::{
     access_list::AccessListItem,
     signature::{Signature, SignatureError},
     transaction::{
-        kind::TransactionKind, request::fake_signature::make_fake_signature,
+        fake_signature::make_fake_signature, kind::TransactionKind,
         signed::Eip2930SignedTransaction,
     },
     utils::envelop_bytes,
@@ -55,6 +55,7 @@ impl Eip2930TransactionRequest {
             r: signature.r,
             s: signature.s,
             hash: OnceLock::new(),
+            is_fake: false,
         })
     }
 
@@ -75,6 +76,7 @@ impl Eip2930TransactionRequest {
             r: signature.r,
             s: signature.s,
             hash: OnceLock::new(),
+            is_fake: true,
         }
     }
 }
@@ -99,7 +101,7 @@ mod tests {
     use std::str::FromStr;
 
     use super::*;
-    use crate::transaction::request::fake_signature::tests::test_fake_sign_properties;
+    use crate::transaction::fake_signature::tests::test_fake_sign_properties;
 
     fn dummy_request() -> Eip2930TransactionRequest {
         let to = Address::from_str("0xc014ba5ec014ba5ec014ba5ec014ba5ec014ba5e").unwrap();

diff --git a/crates/edr_eth/src/transaction/request/eip4844.rs b/crates/edr_eth/src/transaction/request/eip4844.rs
@@ -7,7 +7,7 @@ use k256::SecretKey;
 use crate::{
     access_list::AccessListItem,
     signature::{Signature, SignatureError},
-    transaction::{request::fake_signature::make_fake_signature, Eip4844SignedTransaction},
+    transaction::{fake_signature::make_fake_signature, Eip4844SignedTransaction},
     utils::envelop_bytes,
     Address, Bytes, B256, U256,
 };
@@ -57,6 +57,7 @@ impl Eip4844TransactionRequest {
             r: signature.r,
             s: signature.s,
             hash: OnceLock::new(),
+            is_fake: false,
         })
     }
 
@@ -79,6 +80,7 @@ impl Eip4844TransactionRequest {
             r: signature.r,
             s: signature.s,
             hash: OnceLock::new(),
+            is_fake: true,
         }
     }
 }
@@ -106,7 +108,7 @@ pub(crate) mod tests {
     use std::str::FromStr;
 
     use super::*;
-    use crate::transaction::request::fake_signature::tests::test_fake_sign_properties;
+    use crate::transaction::fake_signature::tests::test_fake_sign_properties;
 
     fn dummy_request() -> Eip4844TransactionRequest {
         // From https://github.com/ethereumjs/ethereumjs-monorepo/blob/master/packages/tx/test/eip4844.spec.ts#L68

diff --git a/crates/edr_eth/src/transaction/request/legacy.rs b/crates/edr_eth/src/transaction/request/legacy.rs
@@ -7,8 +7,7 @@ use k256::SecretKey;
 use crate::{
     signature::{Signature, SignatureError},
     transaction::{
-        kind::TransactionKind, request::fake_signature::make_fake_signature,
-        signed::LegacySignedTransaction,
+        fake_signature::make_fake_signature, kind::TransactionKind, signed::LegacySignedTransaction,
     },
     Address, Bytes, B256, U256,
 };
@@ -45,6 +44,7 @@ impl LegacyTransactionRequest {
             input: self.input,
             signature,
             hash: OnceLock::new(),
+            is_fake: false,
         })
     }
 
@@ -61,6 +61,7 @@ impl LegacyTransactionRequest {
             input: self.input,
             signature,
             hash: OnceLock::new(),
+            is_fake: true,
         }
     }
 }
@@ -83,7 +84,7 @@ mod tests {
     use std::str::FromStr;
 
     use super::*;
-    use crate::transaction::request::fake_signature::tests::test_fake_sign_properties;
+    use crate::transaction::fake_signature::tests::test_fake_sign_properties;
 
     fn dummy_request() -> LegacyTransactionRequest {
         let to = Address::from_str("0xc014ba5ec014ba5ec014ba5ec014ba5ec014ba5e").unwrap();

diff --git a/crates/edr_eth/src/transaction/signed.rs b/crates/edr_eth/src/transaction/signed.rs
@@ -438,6 +438,7 @@ mod tests {
                     v: 1,
                 },
                 hash: OnceLock::new(),
+                is_fake: false
             }),
             post_eip155 => SignedTransaction::PostEip155Legacy(Eip155SignedTransaction {
                 nonce: 0,
@@ -452,6 +453,7 @@ mod tests {
                     v: 37,
                 },
                 hash: OnceLock::new(),
+                is_fake: false
             }),
             eip2930 => SignedTransaction::Eip2930(Eip2930SignedTransaction {
                 chain_id: 1,
@@ -466,6 +468,7 @@ mod tests {
                 s: U256::default(),
                 access_list: vec![].into(),
                 hash: OnceLock::new(),
+                is_fake: false
             }),
             eip1559 => SignedTransaction::Eip1559(Eip1559SignedTransaction {
                 chain_id: 1,
@@ -481,6 +484,7 @@ mod tests {
                 r: U256::default(),
                 s: U256::default(),
                 hash: OnceLock::new(),
+                is_fake: false
             }),
             eip4844 => SignedTransaction::Eip4844(Eip4844SignedTransaction {
                 chain_id: 1,
@@ -498,6 +502,7 @@ mod tests {
                 r: U256::default(),
                 s: U256::default(),
                 hash: OnceLock::new(),
+                is_fake: false
             }),
     }
 
@@ -527,6 +532,7 @@ mod tests {
                 .unwrap(),
             },
             hash: OnceLock::new(),
+            is_fake: false,
         });
         assert_eq!(
             expected,
@@ -555,6 +561,7 @@ mod tests {
                 .unwrap(),
             },
             hash: OnceLock::new(),
+            is_fake: false,
         });
         assert_eq!(
             expected,
@@ -583,6 +590,7 @@ mod tests {
                 .unwrap(),
             },
             hash: OnceLock::new(),
+            is_fake: false,
         });
         assert_eq!(
             expected,
@@ -608,6 +616,7 @@ mod tests {
             s: U256::from_str("0x016b83f4f980694ed2eee4d10667242b1f40dc406901b34125b008d334d47469")
                 .unwrap(),
             hash: OnceLock::new(),
+            is_fake: false,
         });
         assert_eq!(
             expected,
@@ -636,6 +645,7 @@ mod tests {
                 .unwrap(),
             },
             hash: OnceLock::new(),
+            is_fake: false,
         });
         assert_eq!(
             expected,

diff --git a/crates/edr_eth/src/transaction/signed/eip155.rs b/crates/edr_eth/src/transaction/signed/eip155.rs
@@ -6,7 +6,10 @@ use alloy_rlp::{RlpDecodable, RlpEncodable};
 use super::LegacySignedTransaction;
 use crate::{
     signature::{Signature, SignatureError},
-    transaction::{kind::TransactionKind, request::Eip155TransactionRequest},
+    transaction::{
+        fake_signature::recover_fake_signature, kind::TransactionKind,
+        request::Eip155TransactionRequest,
+    },
     Address, Bytes, B256, U256,
 };
 
@@ -28,6 +31,11 @@ pub struct Eip155SignedTransaction {
     #[rlp(skip)]
     #[cfg_attr(feature = "serde", serde(skip))]
     pub hash: OnceLock<B256>,
+    /// Whether the signed transaction is from an impersonated account.
+    #[rlp(default)]
+    #[rlp(skip)]
+    #[cfg_attr(feature = "serde", serde(skip))]
+    pub is_fake: bool,
 }
 
 impl Eip155SignedTransaction {
@@ -37,6 +45,9 @@ impl Eip155SignedTransaction {
 
     /// Recovers the Ethereum address which was used to sign the transaction.
     pub fn recover(&self) -> Result<Address, SignatureError> {
+        if self.is_fake {
+            return Ok(recover_fake_signature(&self.signature));
+        }
         self.signature
             .recover(Eip155TransactionRequest::from(self).hash())
     }
@@ -57,6 +68,7 @@ impl From<LegacySignedTransaction> for Eip155SignedTransaction {
             input: tx.input,
             signature: tx.signature,
             hash: tx.hash,
+            is_fake: tx.is_fake,
         }
     }
 }

diff --git a/crates/edr_eth/src/transaction/signed/eip1559.rs b/crates/edr_eth/src/transaction/signed/eip1559.rs
@@ -6,7 +6,10 @@ use alloy_rlp::{RlpDecodable, RlpEncodable};
 use crate::{
     access_list::AccessList,
     signature::{Signature, SignatureError},
-    transaction::{kind::TransactionKind, request::Eip1559TransactionRequest},
+    transaction::{
+        fake_signature::recover_fake_signature, kind::TransactionKind,
+        request::Eip1559TransactionRequest,
+    },
     utils::envelop_bytes,
     Address, Bytes, B256, U256,
 };
@@ -35,6 +38,11 @@ pub struct Eip1559SignedTransaction {
     #[rlp(skip)]
     #[cfg_attr(feature = "serde", serde(skip))]
     pub hash: OnceLock<B256>,
+    /// Whether the signature is from an impersonated account.
+    #[rlp(default)]
+    #[rlp(skip)]
+    #[cfg_attr(feature = "serde", serde(skip))]
+    pub is_fake: bool,
 }
 
 impl Eip1559SignedTransaction {
@@ -55,6 +63,10 @@ impl Eip1559SignedTransaction {
             v: u64::from(self.odd_y_parity),
         };
 
+        if self.is_fake {
+            return Ok(recover_fake_signature(&signature));
+        }
+
         signature.recover(Eip1559TransactionRequest::from(self).hash())
     }
 }