fix: legacy signed rlp decoding (#1733)

ethers-core/src/types/transaction/eip2718.rs

@@ -320,33 +320,31 @@ impl TypedTransaction {
 
     /// Decodes a signed TypedTransaction from a rlp encoded byte stream
     pub fn decode_signed(rlp: &rlp::Rlp) -> Result<(Self, Signature), TypedTransactionError> {
-        let tx_type: Option<U64> = match rlp.is_data() {
+        let data = rlp.data()?;
-            true => Ok(Some(rlp.data()?.into())),
+        let first = *data.first().ok_or(rlp::DecoderError::Custom("empty slice"))?;
-            false => Err(TypedTransactionError::MissingTransactionType),
+        if rlp.is_list() {
-        }?;
+            // Legacy (0x00)
+            // use the original rlp
+            let decoded_request = TransactionRequest::decode_signed_rlp(rlp)?;
+            return Ok((Self::Legacy(decoded_request.0), decoded_request.1))
+        }
 
         let rest = rlp::Rlp::new(
             rlp.as_raw().get(1..).ok_or(TypedTransactionError::MissingTransactionPayload)?,
         );
 
-        match tx_type {
+        if first == 0x01 {
-            Some(x) if x == U64::from(1u64) => {
             // EIP-2930 (0x01)
             let decoded_request = Eip2930TransactionRequest::decode_signed_rlp(&rest)?;
-                Ok((Self::Eip2930(decoded_request.0), decoded_request.1))
+            return Ok((Self::Eip2930(decoded_request.0), decoded_request.1))
         }
-            Some(x) if x == U64::from(2u64) => {
+        if first == 0x02 {
             // EIP-1559 (0x02)
             let decoded_request = Eip1559TransactionRequest::decode_signed_rlp(&rest)?;
-                Ok((Self::Eip1559(decoded_request.0), decoded_request.1))
+            return Ok((Self::Eip1559(decoded_request.0), decoded_request.1))
-            }
-            _ => {
-                // Legacy (0x00)
-                // use the original rlp
-                let decoded_request = TransactionRequest::decode_signed_rlp(&rest)?;
-                Ok((Self::Legacy(decoded_request.0), decoded_request.1))
-            }
         }
+
+        Err(rlp::DecoderError::Custom("invalid tx type").into())
     }
 }
 

ethers-core/src/types/transaction/request.rs

@@ -7,6 +7,7 @@ use crate::{
     utils::keccak256,
 };
 
+use crate::types::transaction::BASE_NUM_TX_FIELDS;
 use rlp::{Decodable, RlpStream};
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
@@ -161,14 +162,16 @@ impl TransactionRequest {
     /// signing. Assumes the chainid exists.
     pub fn rlp(&self) -> Bytes {
         let mut rlp = RlpStream::new();
-        rlp.begin_list(NUM_TX_FIELDS);
+        if let Some(chain_id) = self.chain_id {
+            rlp.begin_list(BASE_NUM_TX_FIELDS);
             self.rlp_base(&mut rlp);
-
+            rlp.append(&chain_id);
-        // Only hash the 3 extra fields when preparing the
-        // data to sign if chain_id is present
-        rlp_opt(&mut rlp, &self.chain_id);
             rlp.append(&0u8);
             rlp.append(&0u8);
+        } else {
+            rlp.begin_list(BASE_NUM_TX_FIELDS - 3);
+            self.rlp_base(&mut rlp);
+        }
         rlp.out().freeze().into()
     }
 
@@ -350,10 +353,9 @@ impl TransactionRequest {
 #[cfg(test)]
 #[cfg(not(feature = "celo"))]
 mod tests {
-    use crate::types::{Bytes, NameOrAddress, Signature};
+    use super::*;
+    use crate::types::{transaction::eip2718::TypedTransaction, Bytes, NameOrAddress, Signature};
     use rlp::{Decodable, Rlp};
-
-    use super::{Address, TransactionRequest, U256, U64};
     use std::str::FromStr;
 
     #[test]
@@ -558,4 +560,17 @@ mod tests {
             assert_eq!(from_tx, from_addr);
         }
     }
+
+    #[test]
+    fn test_recover_legacy_tx() {
+        let raw_tx = "f9015482078b8505d21dba0083022ef1947a250d5630b4cf539739df2c5dacb4c659f2488d880c46549a521b13d8b8e47ff36ab50000000000000000000000000000000000000000000066ab5a608bd00a23f2fe000000000000000000000000000000000000000000000000000000000000008000000000000000000000000048c04ed5691981c42154c6167398f95e8f38a7ff00000000000000000000000000000000000000000000000000000000632ceac70000000000000000000000000000000000000000000000000000000000000002000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc20000000000000000000000006c6ee5e31d828de241282b9606c8e98ea48526e225a0c9077369501641a92ef7399ff81c21639ed4fd8fc69cb793cfa1dbfab342e10aa0615facb2f1bcf3274a354cfe384a38d0cc008a11c2dd23a69111bc6930ba27a8";
+
+        let data = hex::decode(raw_tx).unwrap();
+        let rlp = Rlp::new(&data);
+        let (tx, sig) = TypedTransaction::decode_signed(&rlp).unwrap();
+        let recovered = sig.recover(tx.sighash()).unwrap();
+
+        let expected: Address = "0xa12e1462d0ced572f396f58b6e2d03894cd7c8a4".parse().unwrap();
+        assert_eq!(expected, recovered);
+    }
 }