[Bug] TxEnvelope::Legacy::decode incorrect chain_id

[kpp]

Component

consensus, eips, genesis

What version of Alloy are you on?

commit 4157e2664fab0

Operating System

None

Describe the bug

Fails to encode/decode a Legacy TxEnvelope:

---- transaction::envelope::tests::test_encode_decode_legacy_parity_eip155 stdout ----
thread 'transaction::envelope::tests::test_encode_decode_legacy_parity_eip155' panicked at crates/consensus/src/transaction/envelope.rs:478:9:
assertion `left == right` failed
  left: Legacy(Signed { tx: TxLegacy { chain_id: Some(3), nonce: 100, gas_price: 3000000000, gas_limit: 50000, to: Call(0x0000000000000000000000000000000000000000), value: 10000000000000000000, input: 0x }, signature: Signature { inner: ecdsa::Signature<Secp256k1>(840CFC572845F5786E702984C2A582528CAD4B49B2A10B9DB1BE7FCA9005856525E7109CEB98168D95B09B18BBF6B685130E0562F233877D492B94EEE0C5B6D1), v: Eip155(42), r: 59728239767604526217550949535444980007647751110991992555989432467883920033125, s: 17143831728048845831134990513685258884275296176139135115431336905099564136145 }, hash: 0x7723239ba787d6d5c582b1cdfb2a931573ae84bacf31e586dea0ded664652f9f })
 right: Legacy(Signed { tx: TxLegacy { chain_id: Some(1), nonce: 100, gas_price: 3000000000, gas_limit: 50000, to: Call(0x0000000000000000000000000000000000000000), value: 10000000000000000000, input: 0x }, signature: Signature { inner: ecdsa::Signature<Secp256k1>(840CFC572845F5786E702984C2A582528CAD4B49B2A10B9DB1BE7FCA9005856525E7109CEB98168D95B09B18BBF6B685130E0562F233877D492B94EEE0C5B6D1), v: Eip155(42), r: 59728239767604526217550949535444980007647751110991992555989432467883920033125, s: 17143831728048845831134990513685258884275296176139135115431336905099564136145 }, hash: 0x7723239ba787d6d5c582b1cdfb2a931573ae84bacf31e586dea0ded664652f9f })

Test:

diff --git a/crates/consensus/src/transaction/envelope.rs b/crates/consensus/src/transaction/envelope.rs
index 93164122..7bc4329d 100644
--- a/crates/consensus/src/transaction/envelope.rs
+++ b/crates/consensus/src/transaction/envelope.rs
@@ -474,7 +474,7 @@ mod tests {
         let tx_envelope: TxEnvelope = tx_signed.into();
         let encoded = tx_envelope.encoded_2718();
         let decoded = TxEnvelope::decode_2718(&mut encoded.as_ref()).unwrap();
-        assert_eq!(encoded.len(), tx_envelope.encode_2718_len());
+        // assert_eq!(encoded.len(), tx_envelope.encode_2718_len());
         assert_eq!(decoded, tx_envelope);
     }
 
@@ -494,6 +494,21 @@ mod tests {
         test_encode_decode_roundtrip(tx, None);
     }
 
+    #[test]
+    fn test_encode_decode_legacy_parity_eip155() {
+        let tx = TxLegacy {
+            chain_id: Some(1),
+            nonce: 100,
+            gas_price: 3_000_000_000,
+            gas_limit: 50_000,
+            to: Address::default().into(),
+            value: U256::from(10e18),
+            input: Bytes::new(),
+        };
+        let signature = Signature::test_signature().with_parity(Parity::Eip155(42));
+        test_encode_decode_roundtrip(tx, Some(signature));
+    }
+
     #[test]
     fn test_encode_decode_eip1559_parity_eip155() {
         let tx = TxEip1559 {

Linked issue: #893
Linked issue: #539

[prestwich]

This is a bit of a GIGO situation, no? Passing a signature you know to be invalid results in invalid behavior. when the chain id of the signature and the legacy tx disagree, only the signature

we can protect against this better by storing parity only as a boolean instead of trying to capture eip155 status. but for now, I think the best solution is a debug_assert_eq until we prep that larger refactor

    fn into_signed(self, signature: Signature) -> Signed<Self> {
        debug_assert_eq!(signature.v().chain_id(), self.chain_id);