fix(core): decode from for EIP2930 transactions

 - add test which checks signed decoding for an EIP2930 transaction

Author: Rjected

Diff for: ethers-core/src/types/transaction/eip1559.rs

@@ -160,7 +160,7 @@ impl Eip1559TransactionRequest {
         self
     }
 
-    /// Hashes the transaction's data with the provided chain id
+    /// Hashes the transaction's data for signing
     pub fn sighash(&self) -> H256 {
         let mut encoded = vec![];
         encoded.extend_from_slice(&[Self::TX_TYPE]);

Diff for: ethers-core/src/types/transaction/eip2718.rs

@@ -1,6 +1,6 @@
 use super::{
     eip1559::{Eip1559RequestError, Eip1559TransactionRequest},
-    eip2930::{AccessList, Eip2930TransactionRequest},
+    eip2930::{AccessList, Eip2930RequestError, Eip2930TransactionRequest},
     request::RequestError,
 };
 use crate::{
@@ -48,6 +48,9 @@ pub enum TypedTransactionError {
     /// When decoding a signed Eip1559 transaction
     #[error(transparent)]
     Eip1559Error(#[from] Eip1559RequestError),
+    /// When decoding a signed Eip2930 transaction
+    #[error(transparent)]
+    Eip2930Error(#[from] Eip2930RequestError),
     /// Error decoding the transaction type from the transaction's RLP encoding
     #[error(transparent)]
     TypeDecodingError(#[from] rlp::DecoderError),
@@ -596,8 +599,12 @@ mod tests {
         let tx_real_rlp: String = tx_real_rlp_vec.encode_hex();
         assert_eq!(tx_expected_rlp, tx_real_rlp);
 
-        let r = U256::from_str("0x8085850e935fd6af9ace1b0343b9e21d2dcc7e914c36cce61a4e32756c785980").unwrap();
-        let s = U256::from_str("0x4c57c184d5096263df981cb8a2f2c7f81640792856909dbf3295a2b7a1dc4a55").unwrap();
+        let r =
+            U256::from_str("0x8085850e935fd6af9ace1b0343b9e21d2dcc7e914c36cce61a4e32756c785980")
+                .unwrap();
+        let s =
+            U256::from_str("0x4c57c184d5096263df981cb8a2f2c7f81640792856909dbf3295a2b7a1dc4a55")
+                .unwrap();
         let v = 0;
         assert_eq!(r, sig.r);
         assert_eq!(s, sig.s);

Diff for: ethers-core/src/types/transaction/eip2930.rs

@@ -1,8 +1,14 @@
-use super::{extract_chain_id, normalize_v};
-use crate::types::{Address, Bytes, Signature, Transaction, TransactionRequest, H256, U256, U64};
-use rlp::{Decodable, DecoderError, RlpStream};
+use super::normalize_v;
+use crate::{
+    types::{
+        Address, Bytes, Signature, SignatureError, Transaction, TransactionRequest, H256, U256, U64,
+    },
+    utils::keccak256,
+};
+use rlp::{Decodable, RlpStream};
 use rlp_derive::{RlpDecodable, RlpDecodableWrapper, RlpEncodable, RlpEncodableWrapper};
 use serde::{Deserialize, Serialize};
+use thiserror::Error;
 
 const NUM_EIP2930_FIELDS: usize = 8;
 
@@ -58,6 +64,17 @@ pub struct AccessListItem {
     pub storage_keys: Vec<H256>,
 }
 
+/// An error involving an EIP2930 transaction request.
+#[derive(Debug, Error)]
+pub enum Eip2930RequestError {
+    /// When decoding a transaction request from RLP
+    #[error(transparent)]
+    DecodingError(#[from] rlp::DecoderError),
+    /// When recovering the address from a signature
+    #[error(transparent)]
+    RecoveryError(#[from] SignatureError),
+}
+
 /// An EIP-2930 transaction is a legacy transaction including an [`AccessList`].
 #[derive(Clone, Serialize, Deserialize, PartialEq, Debug)]
 pub struct Eip2930TransactionRequest {
@@ -67,6 +84,9 @@ pub struct Eip2930TransactionRequest {
 }
 
 impl Eip2930TransactionRequest {
+    /// EIP-2718 transaction type
+    const TX_TYPE: u8 = 0x01;
+
     pub fn new(tx: TransactionRequest, access_list: AccessList) -> Self {
         Self { tx, access_list }
     }
@@ -104,30 +124,46 @@ impl Eip2930TransactionRequest {
     }
 
     /// Decodes fields based on the RLP offset passed.
-    fn decode_base_rlp(rlp: &rlp::Rlp, offset: &mut usize) -> Result<Self, DecoderError> {
-        let request = TransactionRequest::decode_unsigned_rlp_base(rlp, offset)?;
-        let access_list = rlp.val_at(*offset)?;
+    fn decode_base_rlp(rlp: &rlp::Rlp, offset: &mut usize) -> Result<Self, rlp::DecoderError> {
+        let chain_id: u64 = rlp.val_at(*offset)?;
+        *offset += 1;
+
+        let mut request = TransactionRequest::decode_unsigned_rlp_base(rlp, offset)?;
+        request.chain_id = Some(U64::from(chain_id));
+
+        let al = rlp::Rlp::new(rlp.at(*offset)?.as_raw()).data()?;
+        let access_list = match al.len() {
+            0 => AccessList(vec![]),
+            _ => rlp.val_at(*offset)?,
+        };
         *offset += 1;
 
         Ok(Self { tx: request, access_list })
     }
 
     /// Decodes the given RLP into a transaction, attempting to decode its signature as well.
-    pub fn decode_signed_rlp(rlp: &rlp::Rlp) -> Result<(Self, Signature), rlp::DecoderError> {
+    pub fn decode_signed_rlp(rlp: &rlp::Rlp) -> Result<(Self, Signature), Eip2930RequestError> {
         let mut offset = 0;
         let mut txn = Self::decode_base_rlp(rlp, &mut offset)?;
 
         let v = rlp.val_at(offset)?;
-        // populate chainid from v
-        txn.tx.chain_id = extract_chain_id(v);
         offset += 1;
         let r = rlp.val_at(offset)?;
         offset += 1;
         let s = rlp.val_at(offset)?;
 
         let sig = Signature { r, s, v };
+        txn.tx.from = Some(sig.recover(txn.sighash())?);
         Ok((txn, sig))
     }
+
+    /// Hashes the transaction's data for signing
+    pub fn sighash(&self) -> H256 {
+        let mut encoded = vec![];
+        encoded.extend_from_slice(&[Self::TX_TYPE]);
+        encoded.extend_from_slice(self.rlp().as_ref());
+        keccak256(encoded).into()
+    }
 }
 
 /// Get a Eip2930TransactionRequest from a rlp encoded byte stream
@@ -151,6 +187,7 @@ mod tests {
 
     use super::*;
     use crate::types::{transaction::eip2718::TypedTransaction, U256};
+    use std::str::FromStr;
 
     #[test]
     #[cfg_attr(feature = "celo", ignore)]
@@ -198,4 +235,82 @@ mod tests {
         let de: Eip2930TransactionRequest = serde_json::from_str(&serialized).unwrap();
         assert_eq!(tx, TypedTransaction::Eip2930(de));
     }
+
+    #[test]
+    fn decoding_eip2930_signed() {
+        let raw_tx = hex::decode("01f901ef018209068508d8f9fc0083124f8094f5b4f13bdbe12709bd3ea280ebf4b936e99b20f280b90184c5d404940000000000000000000000000000000000000000000000000c4d67a76e15d8190000000000000000000000000000000000000000000000000029d9d8fb7440000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001200000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000020000000000000000000000007b73644935b8e68019ac6356c40661e1bc315860000000000000000000000000761d38e5ddf6ccf6cf7c55759d5210750b5d60f30000000000000000000000000000000000000000000000000000000000000000000000000000000000000000381fe4eb128db1621647ca00965da3f9e09f4fac000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2000000000000000000000000000000000000000000000000000000000000000ac001a0881e7f5298290794bcaa0294986db5c375cbf135dd3c21456b159c470568b687a061fc5f52abab723053fbedf29e1c60b89006416d6c86e1c54ef85a3e84f2dc6e").unwrap();
+        let expected_tx = TransactionRequest::new()
+            .chain_id(1u64)
+            .nonce(2310u64)
+            .gas_price(38_000_000_000u64)
+            .gas(1_200_000u64)
+            .to(Address::from_str("0xf5b4f13bdbe12709bd3ea280ebf4b936e99b20f2").unwrap())
+            .value(0u64)
+            .data(hex::decode("c5d404940000000000000000000000000000000000000000000000000c4d67a76e15d8190000000000000000000000000000000000000000000000000029d9d8fb7440000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001200000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000020000000000000000000000007b73644935b8e68019ac6356c40661e1bc315860000000000000000000000000761d38e5ddf6ccf6cf7c55759d5210750b5d60f30000000000000000000000000000000000000000000000000000000000000000000000000000000000000000381fe4eb128db1621647ca00965da3f9e09f4fac000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2000000000000000000000000000000000000000000000000000000000000000a").unwrap())
+            .from(Address::from_str("0x82a33964706683db62b85a59128ce2fc07c91658").unwrap())
+            .with_access_list(AccessList(vec![]));
+        let r =
+            U256::from_str("0x881e7f5298290794bcaa0294986db5c375cbf135dd3c21456b159c470568b687")
+                .unwrap();
+        let s =
+            U256::from_str("0x61fc5f52abab723053fbedf29e1c60b89006416d6c86e1c54ef85a3e84f2dc6e")
+                .unwrap();
+        let v = 1;
+        let expected_sig = Signature { r, s, v };
+
+        let raw_tx_rlp = rlp::Rlp::new(&raw_tx[..]);
+
+        let (real_tx, real_sig) = TypedTransaction::decode_signed(&raw_tx_rlp).unwrap();
+        let real_tx = match real_tx {
+            TypedTransaction::Eip2930(tx) => tx,
+            _ => panic!("The raw bytes should decode to an EIP2930 tranaction"),
+        };
+
+        assert_eq!(expected_tx, real_tx);
+        assert_eq!(expected_sig, real_sig);
+    }
+
+    #[test]
+    fn decoding_eip2930_with_access_list() {
+        let raw_tx = hex::decode("01f90126018223ff850a02ffee00830f4240940000000000a8fb09af944ab3baf7a9b3e1ab29d880b876200200001525000000000b69ffb300000000557b933a7c2c45672b610f8954a3deb39a51a8cae53ec727dbdeb9e2d5456c3be40cff031ab40a55724d5c9c618a2152e99a45649a3b8cf198321f46720b722f4ec38f99ba3bb1303258d2e816e6a95b25647e01bd0967c1b9599fa3521939871d1d0888f845d694724d5c9c618a2152e99a45649a3b8cf198321f46c0d694720b722f4ec38f99ba3bb1303258d2e816e6a95bc0d69425647e01bd0967c1b9599fa3521939871d1d0888c001a08323efae7b9993bd31a58da7924359d24b5504aa2b33194fcc5ae206e65d2e62a054ce201e3b4b5cd38eb17c56ee2f9111b2e164efcd57b3e70fa308a0a51f7014").unwrap();
+        let expected_tx = TransactionRequest::new()
+            .chain_id(1u64)
+            .nonce(9215u64)
+            .gas_price(43_000_000_000u64)
+            .gas(1_000_000)
+            .to(Address::from_str("0x0000000000a8fb09af944ab3baf7a9b3e1ab29d8").unwrap())
+            .value(0)
+            .data(Bytes::from_str("0x200200001525000000000b69ffb300000000557b933a7c2c45672b610f8954a3deb39a51a8cae53ec727dbdeb9e2d5456c3be40cff031ab40a55724d5c9c618a2152e99a45649a3b8cf198321f46720b722f4ec38f99ba3bb1303258d2e816e6a95b25647e01bd0967c1b9599fa3521939871d1d0888").unwrap())
+            .from(Address::from_str("0xe9c790e8fde820ded558a4771b72eec916c04763").unwrap())
+            .with_access_list(AccessList(vec![
+                AccessListItem {
+                    address: Address::from_str("0x724d5c9c618a2152e99a45649a3b8cf198321f46").unwrap(),
+                    storage_keys: vec![],
+                },
+                AccessListItem {
+                    address: Address::from_str("0x720b722f4ec38f99ba3bb1303258d2e816e6a95b").unwrap(),
+                    storage_keys: vec![],
+                },
+                AccessListItem {
+                    address: Address::from_str("0x25647e01bd0967c1b9599fa3521939871d1d0888").unwrap(),
+                    storage_keys: vec![],
+                },
+            ]));
+        let expected_sig = Signature {
+            r: "0x8323efae7b9993bd31a58da7924359d24b5504aa2b33194fcc5ae206e65d2e62".into(),
+            s: "0x54ce201e3b4b5cd38eb17c56ee2f9111b2e164efcd57b3e70fa308a0a51f7014".into(),
+            v: 1u64,
+        };
+
+        let raw_tx_rlp = rlp::Rlp::new(&raw_tx[..]);
+
+        let (real_tx, real_sig) = TypedTransaction::decode_signed(&raw_tx_rlp).unwrap();
+        let real_tx = match real_tx {
+            TypedTransaction::Eip2930(tx) => tx,
+            _ => panic!("The raw bytes should decode to an EIP2930 tranaction"),
+        };
+
+        assert_eq!(expected_tx, real_tx);
+        assert_eq!(expected_sig, real_sig);
+    }
 }

Diff for: ethers-core/src/types/transaction/request.rs

@@ -250,15 +250,14 @@ impl TransactionRequest {
         let mut offset = 0;
         let mut txn = Self::decode_unsigned_rlp_base(rlp, &mut offset)?;
 
-        // If a signed transaction is passed to this method, the chainid would be set to the v value
-        // of the signature.
-        if let Ok(chainid) = rlp.at(offset)?.as_val() {
+        // If the transaction includes more info, like the chainid, as we serialize in `rlp`, this
+        // will decode that value.
+        if let Ok(chainid) = rlp.val_at(offset) {
+            // If a signed transaction is passed to this method, the chainid would be set to the v
+            // value of the signature.
             txn.chain_id = Some(chainid);
         }
 
-        // parse the last two elements so we return an error if a signed transaction is passed
-        let _first_zero: u8 = rlp.at(offset + 1)?.as_val()?;
-        let _second_zero: u8 = rlp.at(offset + 2)?.as_val()?;
         Ok(txn)
     }
 
@@ -351,7 +350,7 @@ impl TransactionRequest {
 #[cfg(test)]
 #[cfg(not(feature = "celo"))]
 mod tests {
-    use crate::types::{NameOrAddress, Signature};
+    use crate::types::{Bytes, NameOrAddress, Signature};
     use rlp::{Decodable, Rlp};
 
     use super::{Address, TransactionRequest, U256, U64};
@@ -429,6 +428,31 @@ mod tests {
         assert_eq!(got_tx.sighash(), tx.sighash());
     }
 
+    #[test]
+    fn decode_unsigned_rlp_no_chainid() {
+        // unlike the corresponding transaction
+        // 0x02c563d96acaf8c157d08db2228c84836faaf3dd513fc959a54ed4ca6c72573e, this doesn't have a
+        // `from` field because the `from` field is only obtained via signature recovery
+        let expected_tx = TransactionRequest::new()
+            .to(Address::from_str("0xc7696b27830dd8aa4823a1cba8440c27c36adec4").unwrap())
+            .gas(3_000_000)
+            .gas_price(20_000_000_000u64)
+            .value(0)
+            .nonce(6306u64)
+            .data(
+                Bytes::from_str(
+                    "0x91b7f5ed0000000000000000000000000000000000000000000000000000000000000372",
+                )
+                .unwrap(),
+            );
+
+        // manually stripped the signature off the end and modified length
+        let expected_rlp = hex::decode("f8488218a28504a817c800832dc6c094c7696b27830dd8aa4823a1cba8440c27c36adec480a491b7f5ed0000000000000000000000000000000000000000000000000000000000000372").unwrap();
+        let real_tx = TransactionRequest::decode(&Rlp::new(&expected_rlp)).unwrap();
+
+        assert_eq!(real_tx, expected_tx);
+    }
+
     #[test]
     fn test_eip155_encode() {
         let tx = TransactionRequest::new()