From 79916592bd0ac1873f680c9163452719fad368f1 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Wed, 28 Feb 2024 18:11:55 -0500 Subject: [PATCH] fix: use enveloped encoding for typed transactions remove redundant method fix: remove encode_enveloped * improve docs for `encode_signed` * fix vec with_capacity and `into_signed` for 4844 with sidecar fix more raw tx encodings, need to fix length hmm this is all network encoding? still TODO: clear up api boundaries remove encode_signed, decode_signed large refactors to encoding logic todo: analogous methods on Signed, fix tests fix tests fix more tests and lints remove incorrect doc remove unnecessary printlns remove deref, remove tagged legacy fix tests fix Encodable length and add test --- crates/consensus/src/receipt/envelope.rs | 37 +- crates/consensus/src/transaction/eip1559.rs | 175 ++++++--- crates/consensus/src/transaction/eip2930.rs | 134 ++++--- crates/consensus/src/transaction/eip4844.rs | 379 +++++++++++++------ crates/consensus/src/transaction/envelope.rs | 122 ++++-- crates/consensus/src/transaction/legacy.rs | 103 ++--- crates/eips/src/eip2718.rs | 12 +- crates/network/src/transaction/mod.rs | 13 - crates/network/src/transaction/signed.rs | 40 +- crates/providers/src/new.rs | 15 - 10 files changed, 639 insertions(+), 391 deletions(-) diff --git a/crates/consensus/src/receipt/envelope.rs b/crates/consensus/src/receipt/envelope.rs index 134a2912ca3d..305024c56258 100644 --- a/crates/consensus/src/receipt/envelope.rs +++ b/crates/consensus/src/receipt/envelope.rs @@ -1,5 +1,5 @@ use crate::{Receipt, ReceiptWithBloom, TxType}; -use alloy_eips::eip2718::{Decodable2718, Eip2718Error, Encodable2718}; +use alloy_eips::eip2718::{Decodable2718, Encodable2718}; use alloy_rlp::{length_of_length, BufMut, Decodable, Encodable}; /// Receipt envelope, as defined in [EIP-2718]. @@ -16,8 +16,6 @@ use alloy_rlp::{length_of_length, BufMut, Decodable, Encodable}; pub enum ReceiptEnvelope { /// Receipt envelope with no type flag. Legacy(ReceiptWithBloom), - /// Receipt envelope with type flag 0. - TaggedLegacy(ReceiptWithBloom), /// Receipt envelope with type flag 1, containing a [EIP-2930] receipt. /// /// [EIP-2930]: https://eips.ethereum.org/EIPS/eip-2930 @@ -36,7 +34,7 @@ impl ReceiptEnvelope { /// Return the [`TxType`] of the inner receipt. pub const fn tx_type(&self) -> TxType { match self { - Self::Legacy(_) | Self::TaggedLegacy(_) => TxType::Legacy, + Self::Legacy(_) => TxType::Legacy, Self::Eip2930(_) => TxType::Eip2930, Self::Eip1559(_) => TxType::Eip1559, Self::Eip4844(_) => TxType::Eip4844, @@ -47,11 +45,7 @@ impl ReceiptEnvelope { /// however, future receipt types may be added. pub const fn as_receipt_with_bloom(&self) -> Option<&ReceiptWithBloom> { match self { - Self::Legacy(t) - | Self::TaggedLegacy(t) - | Self::Eip2930(t) - | Self::Eip1559(t) - | Self::Eip4844(t) => Some(t), + Self::Legacy(t) | Self::Eip2930(t) | Self::Eip1559(t) | Self::Eip4844(t) => Some(t), } } @@ -59,11 +53,9 @@ impl ReceiptEnvelope { /// receipt types may be added. pub const fn as_receipt(&self) -> Option<&Receipt> { match self { - Self::Legacy(t) - | Self::TaggedLegacy(t) - | Self::Eip2930(t) - | Self::Eip1559(t) - | Self::Eip4844(t) => Some(&t.receipt), + Self::Legacy(t) | Self::Eip2930(t) | Self::Eip1559(t) | Self::Eip4844(t) => { + Some(&t.receipt) + } } } @@ -100,7 +92,6 @@ impl Decodable for ReceiptEnvelope { fn decode(buf: &mut &[u8]) -> alloy_rlp::Result { match Self::network_decode(buf) { Ok(t) => Ok(t), - Err(Eip2718Error::RlpError(e)) => Err(e), Err(_) => Err(alloy_rlp::Error::Custom("Unexpected type")), } } @@ -110,7 +101,6 @@ impl Encodable2718 for ReceiptEnvelope { fn type_flag(&self) -> Option { match self { Self::Legacy(_) => None, - Self::TaggedLegacy(_) => Some(TxType::Legacy as u8), Self::Eip2930(_) => Some(TxType::Eip2930 as u8), Self::Eip1559(_) => Some(TxType::Eip1559 as u8), Self::Eip4844(_) => Some(TxType::Eip4844 as u8), @@ -131,17 +121,19 @@ impl Encodable2718 for ReceiptEnvelope { } impl Decodable2718 for ReceiptEnvelope { - fn typed_decode(ty: u8, buf: &mut &[u8]) -> Result { + fn typed_decode(ty: u8, buf: &mut &[u8]) -> alloy_rlp::Result { let receipt = Decodable::decode(buf)?; - match ty.try_into()? { - TxType::Legacy => Ok(Self::TaggedLegacy(receipt)), + match ty.try_into().map_err(|_| alloy_rlp::Error::Custom("Unexpected type"))? { TxType::Eip2930 => Ok(Self::Eip2930(receipt)), TxType::Eip1559 => Ok(Self::Eip1559(receipt)), TxType::Eip4844 => Ok(Self::Eip4844(receipt)), + TxType::Legacy => { + Err(alloy_rlp::Error::Custom("type-0 eip2718 transactions are not supported")) + } } } - fn fallback_decode(buf: &mut &[u8]) -> Result { + fn fallback_decode(buf: &mut &[u8]) -> alloy_rlp::Result { Ok(Self::Legacy(Decodable::decode(buf)?)) } } @@ -149,12 +141,11 @@ impl Decodable2718 for ReceiptEnvelope { #[cfg(any(test, feature = "arbitrary"))] impl<'a> arbitrary::Arbitrary<'a> for ReceiptEnvelope { fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result { - let tx_type = u.int_in_range(-1..=2)?; + let tx_type = u.int_in_range(0..=2)?; let receipt = Receipt::arbitrary(u)?.with_bloom(); match tx_type { - -1 => Ok(Self::Legacy(receipt)), - 0 => Ok(Self::TaggedLegacy(receipt)), + 0 => Ok(Self::Legacy(receipt)), 1 => Ok(Self::Eip2930(receipt)), 2 => Ok(Self::Eip1559(receipt)), _ => unreachable!(), diff --git a/crates/consensus/src/transaction/eip1559.rs b/crates/consensus/src/transaction/eip1559.rs index ea2ae383ac20..87a9d7c74acc 100644 --- a/crates/consensus/src/transaction/eip1559.rs +++ b/crates/consensus/src/transaction/eip1559.rs @@ -2,7 +2,7 @@ use crate::{TxKind, TxType}; use alloy_eips::eip2930::AccessList; use alloy_network::{Signed, Transaction}; use alloy_primitives::{keccak256, Bytes, ChainId, Signature, U256}; -use alloy_rlp::{length_of_length, BufMut, Decodable, Encodable, Header}; +use alloy_rlp::{BufMut, Decodable, Encodable, Header}; use std::mem; /// A transaction with a priority fee ([EIP-1559](https://eips.ethereum.org/EIPS/eip-1559)). @@ -93,7 +93,7 @@ impl TxEip1559 { /// - `value` /// - `data` (`input`) /// - `access_list` - pub(crate) fn decode_inner(buf: &mut &[u8]) -> alloy_rlp::Result { + pub(crate) fn decode_fields(buf: &mut &[u8]) -> alloy_rlp::Result { Ok(Self { chain_id: Decodable::decode(buf)?, nonce: Decodable::decode(buf)?, @@ -135,31 +135,96 @@ impl TxEip1559 { self.access_list.encode(out); } + /// Returns what the encoded length should be, if the transaction were RLP encoded with the + /// given signature, depending on the value of `with_header`. + /// + /// If `with_header` is `true`, the payload length will include the RLP header length. + /// If `with_header` is `false`, the payload length will not include the RLP header length. + pub(crate) fn encoded_len_with_signature( + &self, + signature: &Signature, + with_header: bool, + ) -> usize { + // this counts the tx fields and signature fields + let payload_length = self.fields_len() + signature.rlp_vrs_len(); + + // this counts: + // * tx type byte + // * inner header length + // * inner payload length + let inner_payload_length = + 1 + Header { list: true, payload_length }.length() + payload_length; + + if with_header { + // header length plus length of the above, wrapped with a string header + Header { list: false, payload_length: inner_payload_length }.length() + + inner_payload_length + } else { + inner_payload_length + } + } + /// Inner encoding function that is used for both rlp [`Encodable`] trait and for calculating - /// hash that for eip2718 does not require rlp header + /// hash that for eip2718 does not require a rlp header pub(crate) fn encode_with_signature( &self, signature: &Signature, - out: &mut dyn alloy_rlp::BufMut, + out: &mut dyn BufMut, + with_header: bool, ) { let payload_length = self.fields_len() + signature.rlp_vrs_len(); - let header = Header { list: true, payload_length }; - header.encode(out); - self.encode_fields(out); - signature.write_rlp_vrs(out); + if with_header { + Header { + list: false, + payload_length: 1 + Header { list: true, payload_length }.length() + payload_length, + } + .encode(out); + } + out.put_u8(self.tx_type() as u8); + self.encode_with_signature_fields(signature, out); } - /// Output the length of the RLP signed transaction encoding, _without_ a RLP string header. - pub fn payload_len_with_signature_without_header(&self, signature: &Signature) -> usize { - let payload_length = self.fields_len() + signature.rlp_vrs_len(); - // 'transaction type byte length' + 'header length' + 'payload length' - 1 + length_of_length(payload_length) + payload_length + /// Decodes the transaction from RLP bytes, including the signature. + /// + /// This __does not__ expect the bytes to start with a transaction type byte or string + /// header. + /// + /// This __does__ expect the bytes to start with a list header and include a signature. + pub(crate) fn decode_signed_fields( + buf: &mut &[u8], + ) -> alloy_rlp::Result> { + let header = Header::decode(buf)?; + if !header.list { + return Err(alloy_rlp::Error::UnexpectedString); + } + + // record original length so we can check encoding + let original_len = buf.len(); + + let tx = Self::decode_fields(buf)?; + let signature = Signature::decode_rlp_vrs(buf)?; + + let signed = tx.into_signed(signature); + if buf.len() + header.payload_length != original_len { + return Err(alloy_rlp::Error::ListLengthMismatch { + expected: header.payload_length, + got: original_len - buf.len(), + }); + } + + Ok(signed) } - /// Output the length of the RLP signed transaction encoding. This encodes with a RLP header. - pub fn payload_len_with_signature(&self, signature: &Signature) -> usize { - let len = self.payload_len_with_signature_without_header(signature); - length_of_length(len) + len + /// Encodes the transaction from RLP bytes, including the signature. This __does not__ encode a + /// tx type byte or string header. + /// + /// This __does__ encode a list header and include a signature. + pub(crate) fn encode_with_signature_fields(&self, signature: &Signature, out: &mut dyn BufMut) { + let payload_length = self.fields_len() + signature.rlp_vrs_len(); + let header = Header { list: true, payload_length }; + header.encode(out); + self.encode_fields(out); + signature.write_rlp_vrs(out); } /// Get transaction type @@ -183,6 +248,8 @@ impl TxEip1559 { } impl Encodable for TxEip1559 { + /// This implementation of [Encodable] encodes _just_ the transaction fields, preceded by a + /// header, without a signature. fn encode(&self, out: &mut dyn BufMut) { Header { list: true, payload_length: self.fields_len() }.encode(out); self.encode_fields(out); @@ -190,11 +257,13 @@ impl Encodable for TxEip1559 { fn length(&self) -> usize { let payload_length = self.fields_len(); - length_of_length(payload_length) + payload_length + Header { list: true, payload_length }.length() + payload_length } } impl Decodable for TxEip1559 { + /// This implementation of [Decodable] decodes _just_ the transaction fields, preceded by a + /// header, without a signature. fn decode(data: &mut &[u8]) -> alloy_rlp::Result { let header = Header::decode(data)?; let remaining_len = data.len(); @@ -203,7 +272,7 @@ impl Decodable for TxEip1559 { return Err(alloy_rlp::Error::InputTooShort); } - Self::decode_inner(data) + Self::decode_fields(data) } } @@ -212,21 +281,16 @@ impl Transaction for TxEip1559 { fn encode_for_signing(&self, out: &mut dyn alloy_rlp::BufMut) { out.put_u8(self.tx_type() as u8); - Header { list: true, payload_length: self.fields_len() }.encode(out); - self.encode_fields(out); + self.encode(out) } fn payload_len_for_signature(&self) -> usize { - let payload_length = self.fields_len(); - // 'transaction type byte length' + 'header length' + 'payload length' - 1 + length_of_length(payload_length) + payload_length + self.length() + 1 } fn into_signed(self, signature: Signature) -> Signed { - let payload_length = 1 + self.fields_len() + signature.rlp_vrs_len(); - let mut buf = Vec::with_capacity(payload_length); - buf.put_u8(TxType::Eip1559 as u8); - self.encode_signed(&signature, &mut buf); + let mut buf = Vec::with_capacity(self.encoded_len_with_signature(&signature, false)); + self.encode_with_signature(&signature, &mut buf, false); let hash = keccak256(&buf); // Drop any v chain id value to ensure the signature format is correct at the time of @@ -235,22 +299,6 @@ impl Transaction for TxEip1559 { Signed::new_unchecked(self, signature.with_parity_bool(), hash) } - fn encode_signed(&self, signature: &Signature, out: &mut dyn BufMut) { - TxEip1559::encode_with_signature(self, signature, out) - } - - fn decode_signed(buf: &mut &[u8]) -> alloy_rlp::Result> { - let header = Header::decode(buf)?; - if !header.list { - return Err(alloy_rlp::Error::UnexpectedString); - } - - let tx = Self::decode_inner(buf)?; - let signature = Signature::decode_rlp_vrs(buf)?; - - Ok(tx.into_signed(signature)) - } - fn input(&self) -> &[u8] { &self.input } @@ -319,7 +367,6 @@ mod tests { use alloy_eips::eip2930::AccessList; use alloy_network::Transaction; use alloy_primitives::{address, b256, hex, Address, Signature, B256, U256}; - use alloy_rlp::Encodable; #[test] fn recover_signer_eip1559() { @@ -350,16 +397,38 @@ mod tests { hex!("0d5688ac3897124635b6cf1bc0e29d6dfebceebdc10a54d74f2ef8b56535b682") ); - dbg!({ - let mut buf = vec![]; - tx.encode(&mut buf); - alloy_primitives::hex::encode(&buf) - }); - - dbg!(alloy_primitives::hex::encode(tx.signature_hash())); - let signed_tx = tx.into_signed(sig); assert_eq!(*signed_tx.hash(), hash, "Expected same hash"); assert_eq!(signed_tx.recover_signer().unwrap(), signer, "Recovering signer should pass."); } + + #[test] + fn encode_decode_eip1559() { + let hash: B256 = b256!("0ec0b6a2df4d87424e5f6ad2a654e27aaeb7dac20ae9e8385cc09087ad532ee0"); + + let tx = TxEip1559 { + chain_id: 1, + nonce: 0x42, + gas_limit: 44386, + to: TxKind::Call( address!("6069a6c32cf691f5982febae4faf8a6f3ab2f0f6")), + value: U256::from(0_u64), + input: hex!("a22cb4650000000000000000000000005eee75727d804a2b13038928d36f8b188945a57a0000000000000000000000000000000000000000000000000000000000000000").into(), + max_fee_per_gas: 0x4a817c800, + max_priority_fee_per_gas: 0x3b9aca00, + access_list: AccessList::default(), + }; + + let sig = Signature::from_scalars_and_parity( + b256!("840cfc572845f5786e702984c2a582528cad4b49b2a10b9db1be7fca90058565"), + b256!("25e7109ceb98168d95b09b18bbf6b685130e0562f233877d492b94eee0c5b6d1"), + false, + ) + .unwrap(); + + let mut buf = vec![]; + tx.encode_with_signature_fields(&sig, &mut buf); + let decoded = TxEip1559::decode_signed_fields(&mut &buf[..]).unwrap(); + assert_eq!(decoded, tx.into_signed(sig)); + assert_eq!(*decoded.hash(), hash); + } } diff --git a/crates/consensus/src/transaction/eip2930.rs b/crates/consensus/src/transaction/eip2930.rs index 410b187af5ad..1fac9b9787c0 100644 --- a/crates/consensus/src/transaction/eip2930.rs +++ b/crates/consensus/src/transaction/eip2930.rs @@ -75,7 +75,7 @@ impl TxEip2930 { /// - `value` /// - `data` (`input`) /// - `access_list` - pub(crate) fn decode_inner(buf: &mut &[u8]) -> alloy_rlp::Result { + pub(crate) fn decode_fields(buf: &mut &[u8]) -> alloy_rlp::Result { Ok(Self { chain_id: Decodable::decode(buf)?, nonce: Decodable::decode(buf)?, @@ -114,9 +114,60 @@ impl TxEip2930 { self.access_list.encode(out); } + /// Returns what the encoded length should be, if the transaction were RLP encoded with the + /// given signature, depending on the value of `with_header`. + /// + /// If `with_header` is `true`, the payload length will include the RLP header length. + /// If `with_header` is `false`, the payload length will not include the RLP header length. + pub(crate) fn encoded_len_with_signature( + &self, + signature: &Signature, + with_header: bool, + ) -> usize { + // this counts the tx fields and signature fields + let payload_length = self.fields_len() + signature.rlp_vrs_len(); + + // this counts: + // * tx type byte + // * inner header length + // * inner payload length + let inner_payload_length = + 1 + Header { list: true, payload_length }.length() + payload_length; + + if with_header { + // header length plus length of the above, wrapped with a string header + Header { list: false, payload_length: inner_payload_length }.length() + + inner_payload_length + } else { + inner_payload_length + } + } + /// Inner encoding function that is used for both rlp [`Encodable`] trait and for calculating - /// hash that for eip2718 does not require rlp header - pub(crate) fn encode_with_signature(&self, signature: &Signature, out: &mut dyn BufMut) { + /// hash that for eip2718 does not require a rlp header + pub(crate) fn encode_with_signature( + &self, + signature: &Signature, + out: &mut dyn BufMut, + with_header: bool, + ) { + let payload_length = self.fields_len() + signature.rlp_vrs_len(); + if with_header { + Header { + list: false, + payload_length: 1 + Header { list: true, payload_length }.length() + payload_length, + } + .encode(out); + } + out.put_u8(self.tx_type() as u8); + self.encode_with_signature_fields(signature, out); + } + + /// Encodes the transaction from RLP bytes, including the signature. This __does not__ encode a + /// tx type byte or string header. + /// + /// This __does__ encode a list header and include a signature. + pub(crate) fn encode_with_signature_fields(&self, signature: &Signature, out: &mut dyn BufMut) { let payload_length = self.fields_len() + signature.rlp_vrs_len(); let header = Header { list: true, payload_length }; header.encode(out); @@ -124,17 +175,35 @@ impl TxEip2930 { signature.write_rlp_vrs(out); } - /// Output the length of the RLP signed transaction encoding, _without_ a RLP string header. - pub fn payload_len_with_signature_without_header(&self, signature: &Signature) -> usize { - let payload_length = self.fields_len() + signature.rlp_vrs_len(); - // 'transaction type byte length' + 'header length' + 'payload length' - 1 + length_of_length(payload_length) + payload_length - } + /// Decodes the transaction from RLP bytes, including the signature. + /// + /// This __does not__ expect the bytes to start with a transaction type byte or string + /// header. + /// + /// This __does__ expect the bytes to start with a list header and include a signature. + pub(crate) fn decode_signed_fields( + buf: &mut &[u8], + ) -> alloy_rlp::Result> { + let header = Header::decode(buf)?; + if !header.list { + return Err(alloy_rlp::Error::UnexpectedString); + } + + // record original length so we can check encoding + let original_len = buf.len(); + + let tx = Self::decode_fields(buf)?; + let signature = Signature::decode_rlp_vrs(buf)?; + + let signed = tx.into_signed(signature); + if buf.len() + header.payload_length != original_len { + return Err(alloy_rlp::Error::ListLengthMismatch { + expected: header.payload_length, + got: original_len - buf.len(), + }); + } - /// Output the length of the RLP signed transaction encoding. This encodes with a RLP header. - pub fn payload_len_with_signature(&self, signature: &Signature) -> usize { - let len = self.payload_len_with_signature_without_header(signature); - length_of_length(len) + len + Ok(signed) } /// Get transaction type. @@ -164,7 +233,7 @@ impl Decodable for TxEip2930 { return Err(alloy_rlp::Error::InputTooShort); } - Self::decode_inner(data) + Self::decode_fields(data) } } @@ -181,14 +250,12 @@ impl Transaction for TxEip2930 { fn payload_len_for_signature(&self) -> usize { let payload_length = self.fields_len(); // 'transaction type byte length' + 'header length' + 'payload length' - 1 + length_of_length(payload_length) + payload_length + 1 + Header { list: true, payload_length }.length() + payload_length } fn into_signed(self, signature: Signature) -> Signed { - let payload_length = 1 + self.fields_len() + signature.rlp_vrs_len(); - let mut buf = Vec::with_capacity(payload_length); - buf.put_u8(TxType::Eip2930 as u8); - self.encode_signed(&signature, &mut buf); + let mut buf = Vec::with_capacity(self.encoded_len_with_signature(&signature, false)); + self.encode_with_signature(&signature, &mut buf, false); let hash = keccak256(&buf); // Drop any v chain id value to ensure the signature format is correct at the time of @@ -197,22 +264,6 @@ impl Transaction for TxEip2930 { Signed::new_unchecked(self, signature.with_parity_bool(), hash) } - fn encode_signed(&self, signature: &Signature, out: &mut dyn BufMut) { - self.encode_with_signature(signature, out) - } - - fn decode_signed(buf: &mut &[u8]) -> alloy_rlp::Result> { - let header = Header::decode(buf)?; - if !header.list { - return Err(alloy_rlp::Error::UnexpectedString); - } - - let tx = Self::decode_inner(buf)?; - let signature = Signature::decode_rlp_vrs(buf)?; - - Ok(tx.into_signed(signature)) - } - fn input(&self) -> &[u8] { &self.input } @@ -280,14 +331,14 @@ impl Transaction for TxEip2930 { mod tests { use super::TxEip2930; use crate::{TxEnvelope, TxKind}; - use alloy_network::{Signed, Transaction}; + use alloy_network::Transaction; use alloy_primitives::{Address, Bytes, Signature, U256}; use alloy_rlp::{Decodable, Encodable}; #[test] fn test_decode_create() { // tests that a contract creation tx encodes and decodes properly - let request = TxEip2930 { + let tx = TxEip2930 { chain_id: 1u64, nonce: 0, gas_price: 1, @@ -299,14 +350,11 @@ mod tests { }; let signature = Signature::test_signature(); - let tx = request.into_signed(signature); - let mut encoded = Vec::new(); - tx.encode(&mut encoded); - assert_eq!(encoded.len(), tx.length()); + tx.encode_with_signature_fields(&signature, &mut encoded); - let decoded = Signed::decode(&mut &*encoded).unwrap(); - assert_eq!(decoded, tx); + let decoded = TxEip2930::decode_signed_fields(&mut &*encoded).unwrap(); + assert_eq!(decoded, tx.into_signed(signature)); } #[test] diff --git a/crates/consensus/src/transaction/eip4844.rs b/crates/consensus/src/transaction/eip4844.rs index 93ffef3c118f..b53a5acd3566 100644 --- a/crates/consensus/src/transaction/eip4844.rs +++ b/crates/consensus/src/transaction/eip4844.rs @@ -58,6 +58,12 @@ pub enum TxEip4844Variant { TxEip4844WithSidecar(TxEip4844WithSidecar), } +impl From for TxEip4844Variant { + fn from(tx: TxEip4844WithSidecar) -> Self { + TxEip4844Variant::TxEip4844WithSidecar(tx) + } +} + impl From for TxEip4844Variant { fn from(tx: TxEip4844) -> Self { TxEip4844Variant::TxEip4844(tx) @@ -122,34 +128,64 @@ impl TxEip4844Variant { let payload_length = match self { TxEip4844Variant::TxEip4844(tx) => tx.fields_len() + signature.rlp_vrs_len(), TxEip4844Variant::TxEip4844WithSidecar(tx) => { - tx.tx().fields_len() + signature.rlp_vrs_len() + let payload_length = tx.tx().fields_len() + signature.rlp_vrs_len(); + let inner_header = Header { list: true, payload_length }; + inner_header.length() + payload_length + tx.sidecar().fields_len() } }; if with_header { Header { list: false, - payload_length: 1 + length_of_length(payload_length) + payload_length, + payload_length: 1 + + Header { list: false, payload_length }.length() + + payload_length, } .encode(out); } out.put_u8(self.tx_type() as u8); - let header = Header { list: true, payload_length }; - header.encode(out); - match self { TxEip4844Variant::TxEip4844(tx) => { - tx.encode_fields(out); - signature.encode(out); + tx.encode_with_signature_fields(signature, out); } TxEip4844Variant::TxEip4844WithSidecar(tx) => { - tx.tx().encode_fields(out); - signature.encode(out); - tx.sidecar().encode_inner(out); + tx.encode_with_signature_fields(signature, out); } } } + + pub(crate) fn decode_signed_fields( + buf: &mut &[u8], + ) -> alloy_rlp::Result> { + let mut current_buf = *buf; + let _header = Header::decode(&mut current_buf)?; + + // There are two possibilities when decoding a signed EIP-4844 transaction: + // If it's a historical transaction, it will only have the transaction fields, and no + // sidecar. If it's a transaction received during the gossip stage or sent through + // eth_sendRawTransaction, it will have the transaction fields and a sidecar. + // + // To disambiguate, we try to decode two list headers. If there is only one list header, we + // assume it's a historical transaction. If there are two, we know the transaction contains + // a sidecar. + let header = Header::decode(&mut current_buf)?; + if header.list { + let tx = TxEip4844WithSidecar::decode_signed_fields(buf)?; + let (tx, signature, hash) = tx.into_parts(); + return Ok(Signed::new_unchecked( + TxEip4844Variant::TxEip4844WithSidecar(tx), + signature, + hash, + )); + } + + // Since there is not a second list header, this is a historical 4844 transaction without a + // sidecar. + let tx = TxEip4844::decode_signed_fields(buf)?; + let (tx, signature, hash) = tx.into_parts(); + Ok(Signed::new_unchecked(TxEip4844Variant::TxEip4844(tx), signature, hash)) + } } impl Transaction for TxEip4844Variant { @@ -165,7 +201,8 @@ impl Transaction for TxEip4844Variant { let payload_length = 1 + self.fields_len() + signature.rlp_vrs_len(); let mut buf = Vec::with_capacity(payload_length); buf.put_u8(TxType::Eip4844 as u8); - self.encode_signed(&signature, &mut buf); + // we use the inner tx to encode the fields + self.tx().encode_with_signature(&signature, &mut buf, false); let hash = keccak256(&buf); // Drop any v chain id value to ensure the signature format is correct at the time of @@ -174,33 +211,6 @@ impl Transaction for TxEip4844Variant { Signed::new_unchecked(self, signature.with_parity_bool(), hash) } - fn decode_signed(buf: &mut &[u8]) -> alloy_rlp::Result> { - let header = Header::decode(buf)?; - if !header.list { - return Err(alloy_rlp::Error::UnexpectedString); - } - - let tx = TxEip4844::decode_inner(buf)?; - let signature = Signature::decode_rlp_vrs(buf)?; - - // There are two possibilities when decoding a signed EIP-4844 transaction: - // If it's a historical transaction, it will only have the transaction fields, and no - // sidecar. If it's a transaction received during the gossip stage or sent through - // eth_sendRawTransaction, it will have the transaction fields and a sidecar. - // To disambiguate, we try to decode the sidecar in all instances, and if it fails, we - // assume it's a historical transaction. - let sidecar = BlobTransactionSidecar::decode_inner(buf); - - if let Ok(sidecar) = sidecar { - Ok(TxEip4844Variant::TxEip4844WithSidecar(TxEip4844WithSidecar::from_tx_and_sidecar( - tx, sidecar, - )) - .into_signed(signature)) - } else { - Ok(TxEip4844Variant::TxEip4844(tx).into_signed(signature)) - } - } - fn encode_for_signing(&self, out: &mut dyn alloy_rlp::BufMut) { // A signature for a [TxEip4844WithSidecar] is a signature over the [TxEip4844Variant] // EIP-2718 payload fields: @@ -210,10 +220,6 @@ impl Transaction for TxEip4844Variant { self.tx().encode_for_signing(out); } - fn encode_signed(&self, signature: &Signature, out: &mut dyn BufMut) { - Self::encode_with_signature(self, signature, out, true); - } - fn chain_id(&self) -> Option { match self { TxEip4844Variant::TxEip4844(tx) => Some(tx.chain_id), @@ -482,7 +488,7 @@ impl TxEip4844 { /// - `access_list` /// - `max_fee_per_blob_gas` /// - `blob_versioned_hashes` - pub fn decode_inner(buf: &mut &[u8]) -> alloy_rlp::Result { + pub fn decode_fields(buf: &mut &[u8]) -> alloy_rlp::Result { Ok(Self { chain_id: Decodable::decode(buf)?, nonce: Decodable::decode(buf)?, @@ -546,8 +552,37 @@ impl TxEip4844 { mem::size_of::() // max_fee_per_data_gas } + /// Returns what the encoded length should be, if the transaction were RLP encoded with the + /// given signature, depending on the value of `with_header`. + /// + /// If `with_header` is `true`, the payload length will include the RLP header length. + /// If `with_header` is `false`, the payload length will not include the RLP header length. + pub(crate) fn encoded_len_with_signature( + &self, + signature: &Signature, + with_header: bool, + ) -> usize { + // this counts the tx fields and signature fields + let payload_length = self.fields_len() + signature.rlp_vrs_len(); + + // this counts: + // * tx type byte + // * inner header length + // * inner payload length + let inner_payload_length = + 1 + Header { list: true, payload_length }.length() + payload_length; + + if with_header { + // header length plus length of the above, wrapped with a string header + Header { list: false, payload_length: inner_payload_length }.length() + + inner_payload_length + } else { + inner_payload_length + } + } + /// Inner encoding function that is used for both rlp [`Encodable`] trait and for calculating - /// hash that for eip2718 does not require rlp header + /// hash that for eip2718 does not require a rlp header pub(crate) fn encode_with_signature( &self, signature: &Signature, @@ -558,28 +593,55 @@ impl TxEip4844 { if with_header { Header { list: false, - payload_length: 1 + length_of_length(payload_length) + payload_length, + payload_length: 1 + Header { list: true, payload_length }.length() + payload_length, } .encode(out); } out.put_u8(self.tx_type() as u8); + self.encode_with_signature_fields(signature, out); + } + + /// Encodes the transaction from RLP bytes, including the signature. This __does not__ encode a + /// tx type byte or string header. + /// + /// This __does__ encode a list header and include a signature. + pub(crate) fn encode_with_signature_fields(&self, signature: &Signature, out: &mut dyn BufMut) { + let payload_length = self.fields_len() + signature.rlp_vrs_len(); let header = Header { list: true, payload_length }; header.encode(out); self.encode_fields(out); - signature.encode(out); + signature.write_rlp_vrs(out); } - /// Output the length of the RLP signed transaction encoding. This encodes with a RLP header. - pub fn payload_len_with_signature(&self, signature: &Signature) -> usize { - let len = self.payload_len_with_signature_without_header(signature); - length_of_length(len) + len - } + /// Decodes the transaction from RLP bytes, including the signature. + /// + /// This __does not__ expect the bytes to start with a transaction type byte or string + /// header. + /// + /// This __does__ expect the bytes to start with a list header and include a signature. + pub(crate) fn decode_signed_fields( + buf: &mut &[u8], + ) -> alloy_rlp::Result> { + let header = Header::decode(buf)?; + if !header.list { + return Err(alloy_rlp::Error::UnexpectedString); + } - /// Output the length of the RLP signed transaction encoding, _without_ a RLP header. - pub fn payload_len_with_signature_without_header(&self, signature: &Signature) -> usize { - let payload_length = self.fields_len() + signature.rlp_vrs_len(); - // 'transaction type byte length' + 'header length' + 'payload length' - 1 + length_of_length(payload_length) + payload_length + // record original length so we can check encoding + let original_len = buf.len(); + + let tx = Self::decode_fields(buf)?; + let signature = Signature::decode_rlp_vrs(buf)?; + + let signed = tx.into_signed(signature); + if buf.len() + header.payload_length != original_len { + return Err(alloy_rlp::Error::ListLengthMismatch { + expected: header.payload_length, + got: original_len - buf.len(), + }); + } + + Ok(signed) } /// Get transaction type @@ -604,7 +666,7 @@ impl TxEip4844 { pub fn payload_len_for_signature(&self) -> usize { let payload_length = self.fields_len(); // 'transaction type byte length' + 'header length' + 'payload length' - 1 + length_of_length(payload_length) + payload_length + 1 + Header { list: true, payload_length }.length() + payload_length } } @@ -616,16 +678,12 @@ impl Transaction for TxEip4844 { } fn payload_len_for_signature(&self) -> usize { - let payload_length = self.fields_len(); - // 'transaction type byte length' + 'header length' + 'payload length' - 1 + length_of_length(payload_length) + payload_length + self.payload_len_for_signature() } fn into_signed(self, signature: Signature) -> Signed { - let payload_length = 1 + self.fields_len() + signature.rlp_vrs_len(); - let mut buf = Vec::with_capacity(payload_length); - buf.put_u8(TxType::Eip4844 as u8); - self.encode_signed(&signature, &mut buf); + let mut buf = Vec::with_capacity(self.encoded_len_with_signature(&signature, false)); + self.encode_with_signature(&signature, &mut buf, false); let hash = keccak256(&buf); // Drop any v chain id value to ensure the signature format is correct at the time of @@ -634,26 +692,10 @@ impl Transaction for TxEip4844 { Signed::new_unchecked(self, signature.with_parity_bool(), hash) } - fn decode_signed(buf: &mut &[u8]) -> alloy_rlp::Result> { - let header = Header::decode(buf)?; - if !header.list { - return Err(alloy_rlp::Error::UnexpectedString); - } - - let tx = Self::decode_inner(buf)?; - let signature = Signature::decode_rlp_vrs(buf)?; - - Ok(tx.into_signed(signature)) - } - fn encode_for_signing(&self, out: &mut dyn alloy_rlp::BufMut) { self.encode_for_signing(out); } - fn encode_signed(&self, signature: &Signature, out: &mut dyn BufMut) { - TxEip4844::encode_with_signature(self, signature, out, true); - } - fn input(&self) -> &[u8] { &self.input } @@ -776,47 +818,80 @@ impl TxEip4844WithSidecar { (self.tx, self.sidecar) } - /// Inner encoding function that is used for both rlp [`Encodable`] trait and for calculating - /// hash that for eip2718 does not require rlp header - pub(crate) fn encode_with_signature( - &self, - signature: &Signature, - out: &mut dyn BufMut, - with_header: bool, - ) { - let payload_length = self.tx.fields_len() + signature.rlp_vrs_len(); - if with_header { - Header { - list: false, - payload_length: 1 + length_of_length(payload_length) + payload_length, - } - .encode(out); - } - out.put_u8(self.tx.tx_type() as u8); - let header = Header { list: true, payload_length }; - header.encode(out); + /// Encodes the transaction from RLP bytes, including the signature. This __does not__ encode a + /// tx type byte or string header. + /// + /// This __does__ encode a list header and include a signature. + /// + /// This encodes the following: + /// `rlp([tx_payload, blobs, commitments, proofs])` + /// + /// where `tx_payload` is the RLP encoding of the [TxEip4844] transaction fields: + /// `rlp([chain_id, nonce, max_priority_fee_per_gas, ..., v, r, s])` + pub(crate) fn encode_with_signature_fields(&self, signature: &Signature, out: &mut dyn BufMut) { + let inner_payload_length = self.tx.fields_len() + signature.rlp_vrs_len(); + let inner_header = Header { list: true, payload_length: inner_payload_length }; + + let outer_payload_length = + inner_header.length() + inner_payload_length + self.sidecar.fields_len(); + let outer_header = Header { list: true, payload_length: outer_payload_length }; + + // write the two headers + outer_header.encode(out); + inner_header.encode(out); + + // now write the fields self.tx.encode_fields(out); - signature.encode(out); + signature.write_rlp_vrs(out); self.sidecar.encode_inner(out); } -} -impl Transaction for TxEip4844WithSidecar { - type Signature = Signature; - - fn decode_signed(buf: &mut &[u8]) -> alloy_rlp::Result> { + /// Decodes the transaction from RLP bytes, including the signature. + /// + /// This __does not__ expect the bytes to start with a transaction type byte or string + /// header. + /// + /// This __does__ expect the bytes to start with a list header and include a signature. + /// + /// This is the inverse of [TxEip4844WithSidecar::encode_with_signature_fields]. + pub(crate) fn decode_signed_fields( + buf: &mut &[u8], + ) -> alloy_rlp::Result> { let header = Header::decode(buf)?; if !header.list { return Err(alloy_rlp::Error::UnexpectedString); } - let tx = TxEip4844::decode_inner(buf)?; - let signature = Signature::decode_rlp_vrs(buf)?; - let sidecar = BlobTransactionSidecar::decode_inner(buf).unwrap_or_default(); + // record original length so we can check encoding + let original_len = buf.len(); + + // decode the inner tx + let inner_tx = TxEip4844::decode_signed_fields(buf)?; + + // decode the sidecar + let sidecar = BlobTransactionSidecar::decode_inner(buf)?; - Ok(Self::from_tx_and_sidecar(tx, sidecar).into_signed(signature)) + if buf.len() + header.payload_length != original_len { + return Err(alloy_rlp::Error::ListLengthMismatch { + expected: header.payload_length, + got: original_len - buf.len(), + }); + } + + let (tx, signature, hash) = inner_tx.into_parts(); + + // create unchecked signed tx because these checks should have happened during construction + // of `Signed` in `TxEip4844::decode_signed_fields` + Ok(Signed::new_unchecked( + TxEip4844WithSidecar::from_tx_and_sidecar(tx, sidecar), + signature, + hash, + )) } +} +impl Transaction for TxEip4844WithSidecar { + type Signature = Signature; fn encode_for_signing(&self, out: &mut dyn alloy_rlp::BufMut) { // A signature for a [TxEip4844WithSidecar] is a signature over the [TxEip4844] EIP-2718 // payload fields: @@ -826,18 +901,14 @@ impl Transaction for TxEip4844WithSidecar { self.tx.encode_for_signing(out); } - fn encode_signed(&self, signature: &Signature, out: &mut dyn BufMut) { - self.encode_with_signature(signature, out, true) - } - fn into_signed(self, signature: Signature) -> Signed { - let payload_length = 1 + self.tx.fields_len() + signature.rlp_vrs_len(); - let mut buf = Vec::with_capacity(payload_length); + let mut buf = Vec::with_capacity(self.tx.encoded_len_with_signature(&signature, false)); // The sidecar is NOT included in the signed payload, only the transaction fields and the // type byte. Include the type byte. - buf.put_u8(TxType::Eip4844 as u8); - // Include the transaction fields. - self.tx.encode_signed(&signature, &mut buf); + // + // Include the transaction fields, making sure to __not__ use the sidecar, and __not__ + // encode a header. + self.tx.encode_with_signature(&signature, &mut buf, false); let hash = keccak256(&buf); // Drop any v chain id value to ensure the signature format is correct at the time of @@ -1039,3 +1110,75 @@ pub(crate) fn kzg_to_versioned_hash(commitment: KzgCommitment) -> B256 { res[0] = alloy_eips::eip4844::VERSIONED_HASH_VERSION_KZG; B256::new(res.into()) } + +#[cfg(test)] +mod tests { + use super::{BlobTransactionSidecar, TxEip4844, TxEip4844WithSidecar}; + use crate::{TxEnvelope, TxKind}; + #[cfg(not(feature = "kzg"))] + use alloy_eips::eip4844::{Blob, Bytes48}; + use alloy_network::Transaction; + use alloy_primitives::{Signature, U256}; + use alloy_rlp::{Decodable, Encodable}; + #[cfg(feature = "kzg")] + use c_kzg::{Blob, Bytes48}; + + #[test] + fn different_sidecar_same_hash() { + // this should make sure that the hash calculated for the `into_signed` conversion does not + // change if the sidecar is different + let tx = TxEip4844 { + chain_id: 1, + nonce: 1, + max_priority_fee_per_gas: 1, + max_fee_per_gas: 1, + gas_limit: 1, + to: TxKind::Call(Default::default()), + value: U256::from(1), + access_list: Default::default(), + blob_versioned_hashes: vec![Default::default()], + max_fee_per_blob_gas: 1, + input: Default::default(), + }; + let sidecar = BlobTransactionSidecar { + blobs: vec![Blob::from([2; 131072])], + commitments: vec![Bytes48::from([3; 48])], + proofs: vec![Bytes48::from([4; 48])], + }; + let mut tx = TxEip4844WithSidecar { tx, sidecar }; + let signature = Signature::test_signature(); + + // turn this transaction into_signed + let expected_signed = tx.clone().into_signed(signature); + + // change the sidecar, adding a single (blob, commitment, proof) pair + tx.sidecar = BlobTransactionSidecar { + blobs: vec![Blob::from([1; 131072])], + commitments: vec![Bytes48::from([1; 48])], + proofs: vec![Bytes48::from([1; 48])], + }; + + // turn this transaction into_signed + let actual_signed = tx.into_signed(signature); + + // the hashes should be the same + assert_eq!(expected_signed.hash(), actual_signed.hash()); + + // convert to envelopes + let expected_envelope: TxEnvelope = expected_signed.into(); + let actual_envelope: TxEnvelope = actual_signed.into(); + + // now encode the transaction and check the length + let mut buf = Vec::new(); + expected_envelope.encode(&mut buf); + assert_eq!(buf.len(), expected_envelope.length()); + + // ensure it's also the same size that `actual` claims to be, since we just changed the + // sidecar values. + assert_eq!(buf.len(), actual_envelope.length()); + + // now decode the transaction and check the values + let decoded = TxEnvelope::decode(&mut &buf[..]).unwrap(); + assert_eq!(decoded, expected_envelope); + } +} diff --git a/crates/consensus/src/transaction/envelope.rs b/crates/consensus/src/transaction/envelope.rs index 97bf3efdae76..9e11bf50a36e 100644 --- a/crates/consensus/src/transaction/envelope.rs +++ b/crates/consensus/src/transaction/envelope.rs @@ -1,7 +1,7 @@ -use crate::{TxEip1559, TxEip2930, TxEip4844Variant, TxLegacy}; +use crate::{TxEip1559, TxEip2930, TxEip4844, TxEip4844Variant, TxEip4844WithSidecar, TxLegacy}; use alloy_eips::eip2718::{Decodable2718, Eip2718Error, Encodable2718}; use alloy_network::Signed; -use alloy_rlp::{length_of_length, Decodable, Encodable}; +use alloy_rlp::{Decodable, Encodable, Header}; /// Ethereum `TransactionType` flags as specified in EIPs [2718], [1559], and /// [2930]. @@ -13,7 +13,7 @@ use alloy_rlp::{length_of_length, Decodable, Encodable}; #[repr(u8)] #[derive(Debug, Copy, Clone, Eq, PartialEq, PartialOrd, Ord)] pub enum TxType { - /// Wrapped legacy transaction type. + /// Legacy transaction type. Legacy = 0, /// EIP-2930 transaction type. Eip2930 = 1, @@ -27,7 +27,6 @@ pub enum TxType { impl<'a> arbitrary::Arbitrary<'a> for TxType { fn arbitrary(u: &mut arbitrary::Unstructured<'_>) -> arbitrary::Result { Ok(match u.int_in_range(0..=2)? { - 0 => TxType::Legacy, 1 => TxType::Eip2930, 2 => TxType::Eip1559, 3 => TxType::Eip4844, @@ -41,6 +40,7 @@ impl TryFrom for TxType { fn try_from(value: u8) -> Result { match value { + 0 => Err(Eip2718Error::UnexpectedType(value)), // SAFETY: repr(u8) with explicit discriminant ..=3 => Ok(unsafe { std::mem::transmute(value) }), _ => Err(Eip2718Error::UnexpectedType(value)), @@ -63,8 +63,6 @@ impl TryFrom for TxType { pub enum TxEnvelope { /// An untagged [`TxLegacy`]. Legacy(Signed), - /// A [`TxLegacy`] tagged with type 0. - TaggedLegacy(Signed), /// A [`TxEip2930`] tagged with type 1. Eip2930(Signed), /// A [`TxEip1559`] tagged with type 2. @@ -72,7 +70,9 @@ pub enum TxEnvelope { /// A TxEip4844 tagged with type 3. /// An EIP-4844 transaction has two network representations: /// 1 - The transaction itself, which is a regular RLP-encoded transaction and used to retrieve - /// historical transactions.. 2 - The transaction with a sidecar, which is the form used to + /// historical transactions.. + /// + /// 2 - The transaction with a sidecar, which is the form used to /// send transactions to the network. Eip4844(Signed), } @@ -89,31 +89,77 @@ impl From> for TxEnvelope { } } +impl From> for TxEnvelope { + fn from(v: Signed) -> Self { + let (tx, signature, hash) = v.into_parts(); + Self::Eip4844(Signed::new_unchecked(TxEip4844Variant::TxEip4844(tx), signature, hash)) + } +} + +impl From> for TxEnvelope { + fn from(v: Signed) -> Self { + let (tx, signature, hash) = v.into_parts(); + Self::Eip4844(Signed::new_unchecked( + TxEip4844Variant::TxEip4844WithSidecar(tx), + signature, + hash, + )) + } +} + +impl From> for TxEnvelope { + fn from(v: Signed) -> Self { + Self::Legacy(v) + } +} + impl TxEnvelope { /// Return the [`TxType`] of the inner txn. pub const fn tx_type(&self) -> TxType { match self { - Self::Legacy(_) | Self::TaggedLegacy(_) => TxType::Legacy, + Self::Legacy(_) => TxType::Legacy, Self::Eip2930(_) => TxType::Eip2930, Self::Eip1559(_) => TxType::Eip1559, Self::Eip4844(_) => TxType::Eip4844, } } - /// Return the length of the inner txn. + /// Return the length of the inner txn, __without a type byte__. pub fn inner_length(&self) -> usize { match self { - Self::Legacy(t) | Self::TaggedLegacy(t) => t.length(), - Self::Eip2930(t) => t.length(), - Self::Eip1559(t) => t.length(), - Self::Eip4844(t) => t.length(), + Self::Legacy(t) => t.tx().fields_len() + t.signature().rlp_vrs_len(), + Self::Eip2930(t) => { + let payload_length = t.tx().fields_len() + t.signature().rlp_vrs_len(); + Header { list: true, payload_length }.length() + payload_length + } + Self::Eip1559(t) => { + let payload_length = t.tx().fields_len() + t.signature().rlp_vrs_len(); + Header { list: true, payload_length }.length() + payload_length + } + Self::Eip4844(t) => match t.tx() { + TxEip4844Variant::TxEip4844(tx) => { + let payload_length = tx.fields_len() + t.signature().rlp_vrs_len(); + Header { list: true, payload_length }.length() + payload_length + } + TxEip4844Variant::TxEip4844WithSidecar(tx) => { + let inner_payload_length = tx.tx().fields_len() + t.signature().rlp_vrs_len(); + let inner_header = Header { list: true, payload_length: inner_payload_length }; + + let outer_payload_length = + inner_header.length() + inner_payload_length + tx.sidecar.fields_len(); + let outer_header = Header { list: true, payload_length: outer_payload_length }; + + outer_header.length() + outer_payload_length + } + }, } } /// Return the RLP payload length of the network-serialized wrapper fn rlp_payload_length(&self) -> usize { if let Self::Legacy(t) = self { - return t.length(); + let payload_length = t.tx().fields_len() + t.signature().rlp_vrs_len(); + return Header { list: true, payload_length }.length() + payload_length; } // length of inner tx body let inner_length = self.inner_length(); @@ -130,34 +176,33 @@ impl Encodable for TxEnvelope { fn length(&self) -> usize { let mut payload_length = self.rlp_payload_length(); if !self.is_legacy() { - payload_length += length_of_length(payload_length); + payload_length += Header { list: false, payload_length }.length(); } + payload_length } } impl Decodable for TxEnvelope { fn decode(buf: &mut &[u8]) -> alloy_rlp::Result { - match Self::network_decode(buf) { - Ok(t) => Ok(t), - Err(Eip2718Error::RlpError(e)) => Err(e), - Err(_) => Err(alloy_rlp::Error::Custom("Unexpected type")), - } + Self::network_decode(buf) } } impl Decodable2718 for TxEnvelope { - fn typed_decode(ty: u8, buf: &mut &[u8]) -> Result { - match ty.try_into()? { - TxType::Legacy => Ok(Self::TaggedLegacy(Decodable::decode(buf)?)), - TxType::Eip2930 => Ok(Self::Eip2930(Decodable::decode(buf)?)), - TxType::Eip1559 => Ok(Self::Eip1559(Decodable::decode(buf)?)), - TxType::Eip4844 => Ok(Self::Eip4844(Decodable::decode(buf)?)), + fn typed_decode(ty: u8, buf: &mut &[u8]) -> alloy_rlp::Result { + match ty.try_into().map_err(|_| alloy_rlp::Error::Custom("unexpected tx type"))? { + TxType::Eip2930 => Ok(Self::Eip2930(TxEip2930::decode_signed_fields(buf)?)), + TxType::Eip1559 => Ok(Self::Eip1559(TxEip1559::decode_signed_fields(buf)?)), + TxType::Eip4844 => Ok(Self::Eip4844(TxEip4844Variant::decode_signed_fields(buf)?)), + TxType::Legacy => { + Err(alloy_rlp::Error::Custom("type-0 eip2718 transactions are not supported")) + } } } - fn fallback_decode(buf: &mut &[u8]) -> Result { - Ok(TxEnvelope::Legacy(Decodable::decode(buf)?)) + fn fallback_decode(buf: &mut &[u8]) -> alloy_rlp::Result { + Ok(TxEnvelope::Legacy(TxLegacy::decode_signed_fields(buf)?)) } } @@ -165,7 +210,6 @@ impl Encodable2718 for TxEnvelope { fn type_flag(&self) -> Option { match self { Self::Legacy(_) => None, - Self::TaggedLegacy(_) => Some(TxType::Legacy as u8), Self::Eip2930(_) => Some(TxType::Eip2930 as u8), Self::Eip1559(_) => Some(TxType::Eip1559 as u8), Self::Eip4844(_) => Some(TxType::Eip4844 as u8), @@ -178,22 +222,16 @@ impl Encodable2718 for TxEnvelope { fn encode_2718(&self, out: &mut dyn alloy_rlp::BufMut) { match self { - TxEnvelope::Legacy(tx) => tx.encode(out), - TxEnvelope::TaggedLegacy(tx) => { - out.put_u8(TxType::Legacy as u8); - tx.encode(out); - } + // Legacy transactions have no difference between network and 2718 + TxEnvelope::Legacy(_) => self.network_encode(out), TxEnvelope::Eip2930(tx) => { - out.put_u8(TxType::Eip2930 as u8); - tx.encode(out); + tx.tx().encode_with_signature(tx.signature(), out, false); } TxEnvelope::Eip1559(tx) => { - out.put_u8(TxType::Eip1559 as u8); - tx.encode(out); + tx.tx().encode_with_signature(tx.signature(), out, false); } TxEnvelope::Eip4844(tx) => { - out.put_u8(TxType::Eip4844 as u8); - tx.encode(out); + tx.tx().encode_with_signature(tx.signature(), out, false); } } } @@ -243,6 +281,10 @@ mod tests { }; assert_eq!(tx.tx().to, TxKind::Call(address!("7a250d5630B4cF539739dF2C5dAcb4c659F2488D"))); + assert_eq!( + tx.hash().to_string(), + "0x280cde7cdefe4b188750e76c888f13bd05ce9a4d7767730feefe8a0e50ca6fc4" + ); let from = tx.recover_signer().unwrap(); assert_eq!(from, address!("a12e1462d0ceD572f396F58B6E2D03894cD7C8a4")); } diff --git a/crates/consensus/src/transaction/legacy.rs b/crates/consensus/src/transaction/legacy.rs index 01f143860ce0..60ab1f84915a 100644 --- a/crates/consensus/src/transaction/legacy.rs +++ b/crates/consensus/src/transaction/legacy.rs @@ -81,9 +81,15 @@ impl TxLegacy { self.input.0.encode(out); } - /// Inner encoding function that is used for both rlp [`Encodable`] trait and for calculating - /// hash. - pub fn encode_with_signature(&self, signature: &Signature, out: &mut dyn alloy_rlp::BufMut) { + /// Encodes the transaction from RLP bytes, including the signature. This __does not__ encode a + /// tx type byte or string header. + /// + /// This __does__ encode a list header and include a signature. + pub fn encode_with_signature_fields( + &self, + signature: &Signature, + out: &mut dyn alloy_rlp::BufMut, + ) { let payload_length = self.fields_len() + signature.rlp_vrs_len(); let header = Header { list: true, payload_length }; header.encode(out); @@ -91,11 +97,11 @@ impl TxLegacy { signature.write_rlp_vrs(out); } - /// Output the length of the RLP signed transaction encoding. - pub fn payload_len_with_signature(&self, signature: &Signature) -> usize { + /// Returns what the encoded length should be, if the transaction were RLP encoded with the + /// given signature. + pub(crate) fn encoded_len_with_signature(&self, signature: &Signature) -> usize { let payload_length = self.fields_len() + signature.rlp_vrs_len(); - // 'header length' + 'payload length' - length_of_length(payload_length) + payload_length + Header { list: true, payload_length }.length() + payload_length } /// Encodes EIP-155 arguments into the desired buffer. Only encodes values @@ -125,6 +131,41 @@ impl TxLegacy { } } + /// Decodes the transaction from RLP bytes, including the signature. + /// + /// This __does not__ expect the bytes to start with a transaction type byte or string + /// header. + /// + /// This __does__ expect the bytes to start with a list header and include a signature. + pub(crate) fn decode_signed_fields( + buf: &mut &[u8], + ) -> alloy_rlp::Result> { + let header = Header::decode(buf)?; + if !header.list { + return Err(alloy_rlp::Error::UnexpectedString); + } + + // record original length so we can check encoding + let original_len = buf.len(); + + let mut tx = Self::decode_fields(buf)?; + let signature = Signature::decode_rlp_vrs(buf)?; + + // extract chain id from signature + let v = signature.v(); + tx.chain_id = v.chain_id(); + + let signed = tx.into_signed(signature); + if buf.len() + header.payload_length != original_len { + return Err(alloy_rlp::Error::ListLengthMismatch { + expected: header.payload_length, + got: original_len - buf.len(), + }); + } + + Ok(signed) + } + /// Decode the RLP fields of the transaction, without decoding an RLP /// header. pub(crate) fn decode_fields(data: &mut &[u8]) -> Result { @@ -195,37 +236,16 @@ impl Transaction for TxLegacy { fn payload_len_for_signature(&self) -> usize { let payload_length = self.fields_len() + self.eip155_fields_len(); // 'header length' + 'payload length' - length_of_length(payload_length) + payload_length + Header { list: true, payload_length }.length() + payload_length } fn into_signed(self, signature: Signature) -> Signed { - let payload_length = self.fields_len() + signature.rlp_vrs_len(); - let mut buf = Vec::with_capacity(payload_length); - self.encode_with_signature(&signature, &mut buf); + let mut buf = Vec::with_capacity(self.encoded_len_with_signature(&signature)); + self.encode_with_signature_fields(&signature, &mut buf); let hash = keccak256(&buf); Signed::new_unchecked(self, signature, hash) } - fn encode_signed(&self, signature: &Signature, out: &mut dyn BufMut) { - self.encode_with_signature(signature, out); - } - - fn decode_signed(buf: &mut &[u8]) -> alloy_rlp::Result> { - let header = Header::decode(buf)?; - if !header.list { - return Err(alloy_rlp::Error::UnexpectedString); - } - let mut tx = Self::decode_fields(buf)?; - - let signature = Signature::decode_rlp_vrs(buf)?; - - let v = signature.v(); - - tx.chain_id = v.chain_id(); - - Ok(tx.into_signed(signature)) - } - fn input(&self) -> &[u8] { &self.input } @@ -289,15 +309,14 @@ impl Transaction for TxLegacy { } } -#[cfg(test)] +#[cfg(all(test, feature = "k256"))] mod tests { + use crate::{TxKind, TxLegacy}; + use alloy_network::Transaction; + use alloy_primitives::{address, b256, hex, Address, Signature, B256, U256}; + #[test] - #[cfg(feature = "k256")] fn recover_signer_legacy() { - use crate::{TxKind, TxLegacy}; - use alloy_network::Transaction; - use alloy_primitives::{b256, hex, Address, Signature, B256, U256}; - let signer: Address = hex!("398137383b3d25c92898c656696e41950e47316b").into(); let hash: B256 = hex!("bb3a336e3f823ec18197f1e13ee875700f08f03e2cab75f0d0b118dabb44cba0").into(); @@ -326,17 +345,11 @@ mod tests { } #[test] - #[cfg(feature = "k256")] // Test vector from https://github.com/alloy-rs/alloy/issues/125 fn decode_legacy_and_recover_signer() { - use crate::TxLegacy; - use alloy_network::Signed; - use alloy_primitives::address; - use alloy_rlp::Decodable; - let raw_tx = "f9015482078b8505d21dba0083022ef1947a250d5630b4cf539739df2c5dacb4c659f2488d880c46549a521b13d8b8e47ff36ab50000000000000000000000000000000000000000000066ab5a608bd00a23f2fe000000000000000000000000000000000000000000000000000000000000008000000000000000000000000048c04ed5691981c42154c6167398f95e8f38a7ff00000000000000000000000000000000000000000000000000000000632ceac70000000000000000000000000000000000000000000000000000000000000002000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc20000000000000000000000006c6ee5e31d828de241282b9606c8e98ea48526e225a0c9077369501641a92ef7399ff81c21639ed4fd8fc69cb793cfa1dbfab342e10aa0615facb2f1bcf3274a354cfe384a38d0cc008a11c2dd23a69111bc6930ba27a8"; - let tx = as Decodable>::decode( + let tx = TxLegacy::decode_signed_fields( &mut alloy_primitives::hex::decode(raw_tx).unwrap().as_slice(), ) .unwrap(); @@ -344,7 +357,7 @@ mod tests { let recovered = tx.recover_signer().unwrap(); let expected = address!("a12e1462d0ceD572f396F58B6E2D03894cD7C8a4"); - assert_eq!(tx.chain_id, Some(1), "Expected same chain id"); + assert_eq!(tx.tx().chain_id, Some(1), "Expected same chain id"); assert_eq!(expected, recovered, "Expected same signer"); } } diff --git a/crates/eips/src/eip2718.rs b/crates/eips/src/eip2718.rs index 5e485c4efac5..79b260029504 100644 --- a/crates/eips/src/eip2718.rs +++ b/crates/eips/src/eip2718.rs @@ -43,15 +43,15 @@ pub trait Decodable2718: Sized { /// ## Note /// /// This should be a simple match block that invokes an inner type's RLP decoder. - fn typed_decode(ty: u8, buf: &mut &[u8]) -> Result; + fn typed_decode(ty: u8, buf: &mut &[u8]) -> alloy_rlp::Result; /// Decode the default variant. /// /// This function is invoked by [`Self::decode_2718`] when no type byte can be extracted. - fn fallback_decode(buf: &mut &[u8]) -> Result; + fn fallback_decode(buf: &mut &[u8]) -> alloy_rlp::Result; /// Decode an EIP-2718 transaction into a concrete instance - fn decode_2718(buf: &mut &[u8]) -> Result { + fn decode_2718(buf: &mut &[u8]) -> alloy_rlp::Result { Self::extract_type_byte(buf) .map(|ty| Self::typed_decode(ty, &mut &buf[1..])) .unwrap_or_else(|| Self::fallback_decode(buf)) @@ -63,7 +63,7 @@ pub trait Decodable2718: Sized { /// type-flag prepended to an opaque inner encoding. The inner encoding is /// RLP for all current Ethereum transaction types, but may not be in future /// versions of the protocol. - fn network_decode(buf: &mut &[u8]) -> Result { + fn network_decode(buf: &mut &[u8]) -> alloy_rlp::Result { // Keep the original buffer around by copying it. let mut h_decode = *buf; let h = Header::decode(&mut h_decode)?; @@ -78,7 +78,7 @@ pub trait Decodable2718: Sized { let remaining_len = buf.len(); if remaining_len == 0 || remaining_len < h.payload_length { - return Err(alloy_rlp::Error::InputTooShort.into()); + return Err(alloy_rlp::Error::InputTooShort); } let ty = buf[0]; @@ -90,7 +90,7 @@ pub trait Decodable2718: Sized { // string Header with payload_length of 1, we need to make sure this check is only // performed for transactions with a string header if bytes_consumed != h.payload_length && h_decode[0] > EMPTY_STRING_CODE { - return Err(alloy_rlp::Error::UnexpectedLength.into()); + return Err(alloy_rlp::Error::UnexpectedLength); } Ok(tx) diff --git a/crates/network/src/transaction/mod.rs b/crates/network/src/transaction/mod.rs index 109b238cba53..7f5bbab5cb86 100644 --- a/crates/network/src/transaction/mod.rs +++ b/crates/network/src/transaction/mod.rs @@ -1,5 +1,4 @@ use alloy_primitives::{keccak256, Bytes, ChainId, Signature, B256, U256}; -use alloy_rlp::BufMut; mod common; pub use common::TxKind; @@ -42,18 +41,6 @@ pub trait Transaction: std::any::Any + Send + Sync + 'static { where Self: Sized; - /// Encode with a signature. This encoding is usually RLP, but may be - /// different for future EIP-2718 transaction types. - fn encode_signed(&self, signature: &Signature, out: &mut dyn BufMut); - - /// Decode a signed transaction. This decoding is usually RLP, but may be - /// different for future EIP-2718 transaction types. - /// - /// This MUST be the inverse of [`Transaction::encode_signed`]. - fn decode_signed(buf: &mut &[u8]) -> alloy_rlp::Result> - where - Self: Sized; - /// Get `data`. fn input(&self) -> &[u8]; /// Get `data`. diff --git a/crates/network/src/transaction/signed.rs b/crates/network/src/transaction/signed.rs index 92cecd431506..88f64e24edf3 100644 --- a/crates/network/src/transaction/signed.rs +++ b/crates/network/src/transaction/signed.rs @@ -1,6 +1,5 @@ use crate::Transaction; use alloy_primitives::{Signature, B256}; -use alloy_rlp::BufMut; /// A transaction with a signature and hash seal. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -10,14 +9,6 @@ pub struct Signed { hash: B256, } -impl std::ops::Deref for Signed { - type Target = T; - - fn deref(&self) -> &Self::Target { - &self.tx - } -} - impl Signed { /// Returns a reference to the transaction. pub const fn tx(&self) -> &T { @@ -33,6 +24,11 @@ impl Signed { pub const fn hash(&self) -> &B256 { &self.hash } + + /// Splits the transaction into parts. + pub fn into_parts(self) -> (T, Sig, B256) { + (self.tx, self.signature, self.hash) + } } impl Signed { @@ -45,32 +41,6 @@ impl Signed { pub fn signature_hash(&self) -> B256 { self.tx.signature_hash() } - - /// Output the signed RLP for the transaction. - pub fn encode_signed(&self, out: &mut dyn BufMut) { - self.tx.encode_signed(&self.signature, out); - } - - /// Produce the RLP encoded signed transaction. - pub fn rlp_signed(&self) -> Vec { - let mut buf = vec![]; - self.encode_signed(&mut buf); - buf - } -} - -impl alloy_rlp::Encodable for Signed { - fn encode(&self, out: &mut dyn BufMut) { - self.tx.encode_signed(&self.signature, out) - } - - // TODO: impl length -} - -impl alloy_rlp::Decodable for Signed { - fn decode(buf: &mut &[u8]) -> alloy_rlp::Result { - T::decode_signed(buf) - } } #[cfg(feature = "k256")] diff --git a/crates/providers/src/new.rs b/crates/providers/src/new.rs index 71b37233e6d4..cc0b006f8f57 100644 --- a/crates/providers/src/new.rs +++ b/crates/providers/src/new.rs @@ -248,21 +248,6 @@ mod tests { todo!() } - fn encode_signed( - &self, - signature: &alloy_primitives::Signature, - out: &mut dyn alloy_primitives::bytes::BufMut, - ) { - todo!() - } - - fn decode_signed(buf: &mut &[u8]) -> alloy_rlp::Result> - where - Self: Sized, - { - todo!() - } - fn input(&self) -> &[u8] { todo!() }