From 18d1aa938d5bad881a3ce5eb91435fb85d610a09 Mon Sep 17 00:00:00 2001 From: Jorge Martinez Date: Thu, 11 Jul 2024 18:13:26 -0700 Subject: [PATCH 1/6] Added OptimismPayloadBuilderAttributesHelper type, added encoded-transactions to OptimismPayloadBuilderAttributes and updated try_new to store the encoded transactions --- crates/optimism/node/tests/e2e/utils.rs | 1 + crates/optimism/payload/src/payload.rs | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/crates/optimism/node/tests/e2e/utils.rs b/crates/optimism/node/tests/e2e/utils.rs index feacabfb2c29..5104fd3be8e2 100644 --- a/crates/optimism/node/tests/e2e/utils.rs +++ b/crates/optimism/node/tests/e2e/utils.rs @@ -66,6 +66,7 @@ pub(crate) fn optimism_payload_attributes(timestamp: u64) -> OptimismPayloadBuil OptimismPayloadBuilderAttributes { payload_attributes: EthPayloadBuilderAttributes::new(B256::ZERO, attributes), transactions: vec![], + encoded_transactions: vec![], no_tx_pool: false, gas_limit: Some(30_000_000), } diff --git a/crates/optimism/payload/src/payload.rs b/crates/optimism/payload/src/payload.rs index b33c4db371ee..75146cd899a8 100644 --- a/crates/optimism/payload/src/payload.rs +++ b/crates/optimism/payload/src/payload.rs @@ -2,6 +2,7 @@ //! Optimism builder support +use reth_primitives::Bytes; use alloy_rlp::Encodable; use reth_chainspec::{ChainSpec, EthereumHardforks}; use reth_evm_optimism::revm_spec_by_timestamp_after_bedrock; @@ -33,12 +34,23 @@ pub struct OptimismPayloadBuilderAttributes { pub payload_attributes: EthPayloadBuilderAttributes, /// `NoTxPool` option for the generated payload pub no_tx_pool: bool, - /// Transactions for the generated payload + /// Decoded Transactions for the generated payload pub transactions: Vec, + /// Encoded Transactions for the generated payload + pub encoded_transactions: Vec, /// The gas limit for the generated payload pub gas_limit: Option, } +/// Optimism Payload Builder Attributes Helper +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct OptimismPayloadBuilderAttributesHelper { + /// An decoded signed transaction + pub decoded_transaction: TransactionSigned, + /// An encoded signed transaction + pub encoded_transaction: Bytes, +} + impl PayloadBuilderAttributes for OptimismPayloadBuilderAttributes { type RpcPayloadAttributes = OptimismPayloadAttributes; type Error = alloy_rlp::Error; @@ -58,6 +70,9 @@ impl PayloadBuilderAttributes for OptimismPayloadBuilderAttributes { (payload_id_optimism(&parent, &attributes, &transactions), transactions) }; + // save the encoded transactions + let encoded_transactions = attributes.transactions.unwrap_or_default(); + let payload_attributes = EthPayloadBuilderAttributes { id, parent, @@ -72,6 +87,7 @@ impl PayloadBuilderAttributes for OptimismPayloadBuilderAttributes { payload_attributes, no_tx_pool: attributes.no_tx_pool.unwrap_or_default(), transactions, + encoded_transactions, gas_limit: attributes.gas_limit, }) } From 6e7fa3ce9d2113f454ddbdbeb428d199c307e28a Mon Sep 17 00:00:00 2001 From: Jorge Martinez Date: Fri, 12 Jul 2024 14:34:44 -0700 Subject: [PATCH 2/6] Added a generic wrapper with encoded bytes --- crates/primitives/src/transaction/mod.rs | 54 ++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index b5f1f1f7c472..dfcfc492fd21 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/crates/primitives/src/transaction/mod.rs @@ -1693,6 +1693,60 @@ impl IntoRecoveredTransaction for TransactionSignedEcRecovered { } } +/// Generic wrapper with encoded Bytes +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct WithEncoded(Bytes, pub T); + +impl From<(Bytes, T)> for WithEncoded { + fn from(value: (Bytes, T)) -> Self { + Self(value.0, value.1) + } +} + +impl WithEncoded { + /// Wraps the value with the bytes. + pub const fn new(bytes: Bytes, value: T) -> Self { + Self(bytes, value) + } + + /// Get the encoded bytes + pub fn bytes(&self) -> Bytes { + self.0.clone() + } + + /// Get the underlying data + pub const fn data(&self) -> &T { + &self.1 + } + + /// Returns ownership of the underlying data. + pub fn into_data(self) -> T { + self.1 + } + + /// Transform the data + pub fn transform>(self) -> WithEncoded { + WithEncoded(self.0, self.1.into()) + } + + /// Split the wrapper into [`Bytes`] and data tuple + pub fn split(self) -> (Bytes, T) { + (self.0, self.1) + } + + /// Maps the inner value to a new value using the given function. + pub fn map U>(self, op: F) -> WithEncoded { + WithEncoded(self.0, op(self.1)) + } +} + +impl WithEncoded> { + /// returns `None` if the inner value is `None`, otherwise returns `Some(WithPeerId)`. + pub fn transpose(self) -> Option> { + self.1.map(|v| WithEncoded(self.0, v)) + } +} + #[cfg(test)] mod tests { use crate::{ From fc89238bd3778f1c192a4ef3c65afd8b98ca677e Mon Sep 17 00:00:00 2001 From: Jorge Martinez Date: Fri, 12 Jul 2024 14:36:54 -0700 Subject: [PATCH 3/6] used the WithEncoded wrapper on TransactionSigned for payloads --- crates/optimism/payload/src/payload.rs | 29 +++++++------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/crates/optimism/payload/src/payload.rs b/crates/optimism/payload/src/payload.rs index 75146cd899a8..2337f9f13e63 100644 --- a/crates/optimism/payload/src/payload.rs +++ b/crates/optimism/payload/src/payload.rs @@ -2,7 +2,7 @@ //! Optimism builder support -use reth_primitives::Bytes; +use reth_primitives::transaction::WithEncoded; use alloy_rlp::Encodable; use reth_chainspec::{ChainSpec, EthereumHardforks}; use reth_evm_optimism::revm_spec_by_timestamp_after_bedrock; @@ -34,23 +34,12 @@ pub struct OptimismPayloadBuilderAttributes { pub payload_attributes: EthPayloadBuilderAttributes, /// `NoTxPool` option for the generated payload pub no_tx_pool: bool, - /// Decoded Transactions for the generated payload - pub transactions: Vec, - /// Encoded Transactions for the generated payload - pub encoded_transactions: Vec, + /// Transactions for the generated payload + pub transactions: Vec>, /// The gas limit for the generated payload pub gas_limit: Option, } -/// Optimism Payload Builder Attributes Helper -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct OptimismPayloadBuilderAttributesHelper { - /// An decoded signed transaction - pub decoded_transaction: TransactionSigned, - /// An encoded signed transaction - pub encoded_transaction: Bytes, -} - impl PayloadBuilderAttributes for OptimismPayloadBuilderAttributes { type RpcPayloadAttributes = OptimismPayloadAttributes; type Error = alloy_rlp::Error; @@ -65,14 +54,11 @@ impl PayloadBuilderAttributes for OptimismPayloadBuilderAttributes { .as_deref() .unwrap_or(&[]) .iter() - .map(|tx| TransactionSigned::decode_enveloped(&mut tx.as_ref())) - .collect::>()?; + .map(|tx| WithEncoded::new(tx.clone(), TransactionSigned::decode_enveloped(&mut tx.as_ref()).unwrap())) + .collect(); (payload_id_optimism(&parent, &attributes, &transactions), transactions) }; - // save the encoded transactions - let encoded_transactions = attributes.transactions.unwrap_or_default(); - let payload_attributes = EthPayloadBuilderAttributes { id, parent, @@ -87,7 +73,6 @@ impl PayloadBuilderAttributes for OptimismPayloadBuilderAttributes { payload_attributes, no_tx_pool: attributes.no_tx_pool.unwrap_or_default(), transactions, - encoded_transactions, gas_limit: attributes.gas_limit, }) } @@ -324,7 +309,7 @@ impl From for OptimismExecutionPayloadEnvelopeV4 { pub(crate) fn payload_id_optimism( parent: &B256, attributes: &OptimismPayloadAttributes, - txs: &[TransactionSigned], + txs: &[WithEncoded], ) -> PayloadId { use sha2::Digest; let mut hasher = sha2::Sha256::new(); @@ -346,7 +331,7 @@ pub(crate) fn payload_id_optimism( if no_tx_pool || !txs.is_empty() { hasher.update([no_tx_pool as u8]); hasher.update(txs.len().to_be_bytes()); - txs.iter().for_each(|tx| hasher.update(tx.hash())); + txs.iter().for_each(|tx| hasher.update(tx.data().hash())); } if let Some(gas_limit) = attributes.gas_limit { From b50ad2a71edf19d352bebff61eebe3cc49434252 Mon Sep 17 00:00:00 2001 From: Jorge Martinez Date: Fri, 12 Jul 2024 14:37:36 -0700 Subject: [PATCH 4/6] updated functions affected by WithEncoded wrapper change --- crates/optimism/node/tests/e2e/utils.rs | 1 - crates/optimism/payload/src/builder.rs | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/optimism/node/tests/e2e/utils.rs b/crates/optimism/node/tests/e2e/utils.rs index 5104fd3be8e2..feacabfb2c29 100644 --- a/crates/optimism/node/tests/e2e/utils.rs +++ b/crates/optimism/node/tests/e2e/utils.rs @@ -66,7 +66,6 @@ pub(crate) fn optimism_payload_attributes(timestamp: u64) -> OptimismPayloadBuil OptimismPayloadBuilderAttributes { payload_attributes: EthPayloadBuilderAttributes::new(B256::ZERO, attributes), transactions: vec![], - encoded_transactions: vec![], no_tx_pool: false, gas_limit: Some(30_000_000), } diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index 93b8c9606132..e2775cac099e 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -318,7 +318,7 @@ where } // A sequencer's block should never contain blob transactions. - if sequencer_tx.is_eip4844() { + if sequencer_tx.data().is_eip4844() { return Err(PayloadBuilderError::other( OptimismPayloadBuilderError::BlobTransactionRejected, )) @@ -328,7 +328,7 @@ where // purely for the purposes of utilizing the `evm_config.tx_env`` function. // Deposit transactions do not have signatures, so if the tx is a deposit, this // will just pull in its `from` address. - let sequencer_tx = sequencer_tx.clone().try_into_ecrecovered().map_err(|_| { + let sequencer_tx = sequencer_tx.data().clone().try_into_ecrecovered().map_err(|_| { PayloadBuilderError::other(OptimismPayloadBuilderError::TransactionEcRecoverFailed) })?; From b9e3bbc91d3fdab684681edfb962612db6599cec Mon Sep 17 00:00:00 2001 From: Jorge Martinez Date: Fri, 12 Jul 2024 16:23:15 -0700 Subject: [PATCH 5/6] fixing comment typo and changed WithEncoded::bytes() to WithEncoded::encoded_bytes() --- crates/primitives/src/transaction/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index dfcfc492fd21..293c82df412c 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/crates/primitives/src/transaction/mod.rs @@ -1710,7 +1710,7 @@ impl WithEncoded { } /// Get the encoded bytes - pub fn bytes(&self) -> Bytes { + pub fn encoded_bytes(&self) -> Bytes { self.0.clone() } @@ -1741,7 +1741,7 @@ impl WithEncoded { } impl WithEncoded> { - /// returns `None` if the inner value is `None`, otherwise returns `Some(WithPeerId)`. + /// returns `None` if the inner value is `None`, otherwise returns `Some(WithEncoded)`. pub fn transpose(self) -> Option> { self.1.map(|v| WithEncoded(self.0, v)) } From 374b7fa2cac6058d3702c203f2048fe1add36d90 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Sat, 13 Jul 2024 07:03:25 +0200 Subject: [PATCH 6/6] touchups --- crates/optimism/payload/src/builder.rs | 4 +-- crates/optimism/payload/src/payload.rs | 34 ++++++++++++------------ crates/primitives/src/transaction/mod.rs | 14 +++++----- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index e2775cac099e..6801560deb91 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -318,7 +318,7 @@ where } // A sequencer's block should never contain blob transactions. - if sequencer_tx.data().is_eip4844() { + if sequencer_tx.value().is_eip4844() { return Err(PayloadBuilderError::other( OptimismPayloadBuilderError::BlobTransactionRejected, )) @@ -328,7 +328,7 @@ where // purely for the purposes of utilizing the `evm_config.tx_env`` function. // Deposit transactions do not have signatures, so if the tx is a deposit, this // will just pull in its `from` address. - let sequencer_tx = sequencer_tx.data().clone().try_into_ecrecovered().map_err(|_| { + let sequencer_tx = sequencer_tx.value().clone().try_into_ecrecovered().map_err(|_| { PayloadBuilderError::other(OptimismPayloadBuilderError::TransactionEcRecoverFailed) })?; diff --git a/crates/optimism/payload/src/payload.rs b/crates/optimism/payload/src/payload.rs index 2337f9f13e63..f2b871e348d7 100644 --- a/crates/optimism/payload/src/payload.rs +++ b/crates/optimism/payload/src/payload.rs @@ -2,7 +2,6 @@ //! Optimism builder support -use reth_primitives::transaction::WithEncoded; use alloy_rlp::Encodable; use reth_chainspec::{ChainSpec, EthereumHardforks}; use reth_evm_optimism::revm_spec_by_timestamp_after_bedrock; @@ -10,6 +9,7 @@ use reth_payload_builder::EthPayloadBuilderAttributes; use reth_payload_primitives::{BuiltPayload, PayloadBuilderAttributes}; use reth_primitives::{ revm_primitives::{BlobExcessGasAndPrice, BlockEnv, CfgEnv, CfgEnvWithHandlerCfg, SpecId}, + transaction::WithEncoded, Address, BlobTransactionSidecar, Header, SealedBlock, TransactionSigned, Withdrawals, B256, U256, }; @@ -34,7 +34,8 @@ pub struct OptimismPayloadBuilderAttributes { pub payload_attributes: EthPayloadBuilderAttributes, /// `NoTxPool` option for the generated payload pub no_tx_pool: bool, - /// Transactions for the generated payload + /// Decoded transactions and the original EIP-2718 encoded bytes as received in the payload + /// attributes. pub transactions: Vec>, /// The gas limit for the generated payload pub gas_limit: Option, @@ -48,16 +49,17 @@ impl PayloadBuilderAttributes for OptimismPayloadBuilderAttributes { /// /// Derives the unique [`PayloadId`] for the given parent and attributes fn try_new(parent: B256, attributes: OptimismPayloadAttributes) -> Result { - let (id, transactions) = { - let transactions: Vec<_> = attributes - .transactions - .as_deref() - .unwrap_or(&[]) - .iter() - .map(|tx| WithEncoded::new(tx.clone(), TransactionSigned::decode_enveloped(&mut tx.as_ref()).unwrap())) - .collect(); - (payload_id_optimism(&parent, &attributes, &transactions), transactions) - }; + let id = payload_id_optimism(&parent, &attributes); + + let transactions = attributes + .transactions + .unwrap_or_default() + .into_iter() + .map(|data| { + TransactionSigned::decode_enveloped(&mut data.as_ref()) + .map(|tx| WithEncoded::new(data, tx)) + }) + .collect::>()?; let payload_attributes = EthPayloadBuilderAttributes { id, @@ -309,7 +311,6 @@ impl From for OptimismExecutionPayloadEnvelopeV4 { pub(crate) fn payload_id_optimism( parent: &B256, attributes: &OptimismPayloadAttributes, - txs: &[WithEncoded], ) -> PayloadId { use sha2::Digest; let mut hasher = sha2::Sha256::new(); @@ -328,10 +329,9 @@ pub(crate) fn payload_id_optimism( } let no_tx_pool = attributes.no_tx_pool.unwrap_or_default(); - if no_tx_pool || !txs.is_empty() { - hasher.update([no_tx_pool as u8]); - hasher.update(txs.len().to_be_bytes()); - txs.iter().for_each(|tx| hasher.update(tx.data().hash())); + hasher.update([no_tx_pool as u8]); + if let Some(txs) = &attributes.transactions { + txs.iter().for_each(|tx| hasher.update(tx)); } if let Some(gas_limit) = attributes.gas_limit { diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index 293c82df412c..b5af542901b6 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/crates/primitives/src/transaction/mod.rs @@ -1693,7 +1693,7 @@ impl IntoRecoveredTransaction for TransactionSignedEcRecovered { } } -/// Generic wrapper with encoded Bytes +/// Generic wrapper with encoded Bytes, such as transaction data. #[derive(Debug, Clone, PartialEq, Eq)] pub struct WithEncoded(Bytes, pub T); @@ -1714,22 +1714,22 @@ impl WithEncoded { self.0.clone() } - /// Get the underlying data - pub const fn data(&self) -> &T { + /// Get the underlying value + pub const fn value(&self) -> &T { &self.1 } - /// Returns ownership of the underlying data. - pub fn into_data(self) -> T { + /// Returns ownership of the underlying value. + pub fn into_value(self) -> T { self.1 } - /// Transform the data + /// Transform the value pub fn transform>(self) -> WithEncoded { WithEncoded(self.0, self.1.into()) } - /// Split the wrapper into [`Bytes`] and data tuple + /// Split the wrapper into [`Bytes`] and value tuple pub fn split(self) -> (Bytes, T) { (self.0, self.1) }