Merge pull request #128 from anton-rs/refcell/outline-optimism-payloa…

…d-attributes

bin/reth/src/debug_cmd/build_block.rs

@@ -246,11 +246,11 @@ impl Command {
             // TODO: add support for withdrawals
             withdrawals: None,
             #[cfg(feature = "optimism")]
-            transactions: None,
-            #[cfg(feature = "optimism")]
-            no_tx_pool: None,
-            #[cfg(feature = "optimism")]
-            gas_limit: None,
+            optimism_payload_attributes: reth_rpc_types::engine::OptimismPayloadAttributes {
+                transactions: None,
+                no_tx_pool: None,
+                gas_limit: None,
+            },
         };
         let payload_config = PayloadConfig::new(
             Arc::clone(&best_block),

crates/payload/basic/src/lib.rs

@@ -329,7 +329,7 @@ where
         // check if the deadline is reached
         if this.deadline.as_mut().poll(cx).is_ready() {
             trace!(target: "payload_builder", "payload building deadline reached");
-            return Poll::Ready(Ok(()))
+            return Poll::Ready(Ok(()));
         }
 
         // check if the interval is reached
@@ -414,7 +414,7 @@ where
 
     fn best_payload(&self) -> Result<Arc<BuiltPayload>, PayloadBuilderError> {
         if let Some(ref payload) = self.best_payload {
-            return Ok(payload.clone())
+            return Ok(payload.clone());
         }
         // No payload has been built yet, but we need to return something that the CL then can
         // deliver, so we need to return an empty payload.
@@ -452,7 +452,9 @@ where
             // upfront with the list of transactions sent in the attributes without caring about
             // the results of the polling job, if a best payload has not already been built.
             #[cfg(feature = "optimism")]
-            if self.config.chain_spec.optimism && self.config.attributes.no_tx_pool {
+            if self.config.chain_spec.optimism &&
+                self.config.attributes.optimism_payload_attributes.no_tx_pool
+            {
                 let args = BuildArguments {
                     client: self.client.clone(),
                     pool: self.pool.clone(),
@@ -474,7 +476,7 @@ where
                             empty_payload,
                         },
                         KeepPayloadJobAlive::Yes,
-                    )
+                    );
                 }
             }
 
@@ -517,13 +519,13 @@ impl Future for ResolveBestPayload {
             if let Poll::Ready(res) = fut.poll(cx) {
                 this.maybe_better = None;
                 if let Ok(BuildOutcome::Better { payload, .. }) = res {
-                    return Poll::Ready(Ok(Arc::new(payload)))
+                    return Poll::Ready(Ok(Arc::new(payload)));
                 }
             }
         }
 
         if let Some(best) = this.best_payload.take() {
-            return Poll::Ready(Ok(best))
+            return Poll::Ready(Ok(best));
         }
 
         let mut empty_payload = this.empty_payload.take().expect("polled after completion");
@@ -604,7 +606,7 @@ impl PayloadConfig {
     pub(crate) fn extra_data(&self) -> reth_primitives::Bytes {
         #[cfg(feature = "optimism")]
         if self.chain_spec.optimism {
-            return Default::default()
+            return Default::default();
         }
         self.extra_data.clone()
     }
@@ -787,12 +789,12 @@ where
             // which also removes all dependent transaction from the iterator before we can
             // continue
             best_txs.mark_invalid(&pool_tx);
-            continue
+            continue;
         }
 
         // check if the job was cancelled, if so we can exit early
         if cancel.is_cancelled() {
-            return Ok(BuildOutcome::Cancelled)
+            return Ok(BuildOutcome::Cancelled);
         }
 
         // convert tx to a signed transaction
@@ -808,7 +810,7 @@ where
                 // the gas limit condition for regular transactions above.
                 trace!(target: "payload_builder", tx=?tx.hash, ?sum_blob_gas_used, ?tx_blob_gas, "skipping blob transaction because it would exceed the max data gas per block");
                 best_txs.mark_invalid(&pool_tx);
-                continue
+                continue;
             }
         }
 
@@ -837,11 +839,11 @@ where
                             best_txs.mark_invalid(&pool_tx);
                         }
 
-                        continue
+                        continue;
                     }
                     err => {
                         // this is an error that we should treat as fatal for this attempt
-                        return Err(PayloadBuilderError::EvmExecutionError(err))
+                        return Err(PayloadBuilderError::EvmExecutionError(err));
                     }
                 }
             }
@@ -888,7 +890,7 @@ where
     // check if we have a better block
     if !is_better_payload(best_payload.as_deref(), total_fees) {
         // can skip building the block
-        return Ok(BuildOutcome::Aborted { fees: total_fees, cached_reads })
+        return Ok(BuildOutcome::Aborted { fees: total_fees, cached_reads });
     }
 
     let WithdrawalsOutcome { withdrawals_root, withdrawals } =
@@ -1086,11 +1088,11 @@ fn commit_withdrawals<DB: Database<Error = RethError>>(
     withdrawals: Vec<Withdrawal>,
 ) -> RethResult<WithdrawalsOutcome> {
     if !chain_spec.is_shanghai_active_at_timestamp(timestamp) {
-        return Ok(WithdrawalsOutcome::pre_shanghai())
+        return Ok(WithdrawalsOutcome::pre_shanghai());
     }
 
     if withdrawals.is_empty() {
-        return Ok(WithdrawalsOutcome::empty())
+        return Ok(WithdrawalsOutcome::empty());
     }
 
     let balance_increments =

crates/payload/basic/src/optimism.rs

@@ -39,6 +39,7 @@ where
     debug!(target: "payload_builder", parent_hash = ?parent_block.hash, parent_number = parent_block.number, "building new payload");
     let mut cumulative_gas_used = 0;
     let block_gas_limit: u64 = attributes
+        .optimism_payload_attributes
         .gas_limit
         .unwrap_or(initialized_block_env.gas_limit.try_into().unwrap_or(u64::MAX));
     let base_fee = initialized_block_env.basefee.to::<u64>();
@@ -54,7 +55,7 @@ where
         chain_spec.is_fork_active_at_timestamp(Hardfork::Regolith, attributes.timestamp);
 
     let mut receipts = Vec::new();
-    for sequencer_tx in attributes.transactions {
+    for sequencer_tx in attributes.optimism_payload_attributes.transactions {
         // Check if the job was cancelled, if so we can exit early.
         if cancel.is_cancelled() {
             return Ok(BuildOutcome::Cancelled);
@@ -133,7 +134,7 @@ where
         executed_txs.push(sequencer_tx.into_signed());
     }
 
-    if !attributes.no_tx_pool {
+    if !attributes.optimism_payload_attributes.no_tx_pool {
         while let Some(pool_tx) = best_txs.next() {
             // ensure we still have capacity for this transaction
             if cumulative_gas_used + pool_tx.gas_limit() > block_gas_limit {

crates/payload/builder/src/payload.rs

@@ -142,6 +142,14 @@ pub struct PayloadBuilderAttributes {
     pub withdrawals: Vec<Withdrawal>,
     /// Root of the parent beacon block
     pub parent_beacon_block_root: Option<B256>,
+    /// Optimism Payload Builder Attributes
+    #[cfg(feature = "optimism")]
+    pub optimism_payload_attributes: OptimismPayloadBuilderAttributes,
+}
+
+/// Optimism Payload Builder Attributes
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct OptimismPayloadBuilderAttributes {
     /// NoTxPool option for the generated payload
     #[cfg(feature = "optimism")]
     pub no_tx_pool: bool,
@@ -166,9 +174,10 @@ impl PayloadBuilderAttributes {
         #[cfg(feature = "optimism")]
         let (id, transactions) = {
             let transactions = attributes
+                .optimism_payload_attributes
                 .transactions
-                .as_ref()
-                .unwrap_or(&Vec::default())
+                .as_deref()
+                .unwrap_or(&[])
                 .iter()
                 .map(|tx| TransactionSigned::decode_enveloped(tx.clone()))
                 .collect::<Result<_, _>>()?;
@@ -193,11 +202,11 @@ impl PayloadBuilderAttributes {
             withdrawals: withdraw.unwrap_or_default(),
             parent_beacon_block_root: attributes.parent_beacon_block_root,
             #[cfg(feature = "optimism")]
-            no_tx_pool: attributes.no_tx_pool.unwrap_or_default(),
-            #[cfg(feature = "optimism")]
-            transactions,
-            #[cfg(feature = "optimism")]
-            gas_limit: attributes.gas_limit,
+            optimism_payload_attributes: OptimismPayloadBuilderAttributes {
+                no_tx_pool: attributes.optimism_payload_attributes.no_tx_pool.unwrap_or_default(),
+                transactions,
+                gas_limit: attributes.optimism_payload_attributes.gas_limit,
+            },
         })
     }
     /// Returns the configured [CfgEnv] and [BlockEnv] for the targeted payload (that has the
@@ -289,14 +298,14 @@ pub(crate) fn payload_id(
 
     #[cfg(feature = "optimism")]
     {
-        let no_tx_pool = attributes.no_tx_pool.unwrap_or_default();
+        let no_tx_pool = attributes.optimism_payload_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.hash()));
         }
 
-        if let Some(gas_limit) = attributes.gas_limit {
+        if let Some(gas_limit) = attributes.optimism_payload_attributes.gas_limit {
             hasher.update(gas_limit.to_be_bytes());
         }
     }

crates/rpc/rpc-engine-api/src/engine_api.rs

@@ -262,12 +262,12 @@ where
         self.inner.task_spawner.spawn_blocking(Box::pin(async move {
             if count > MAX_PAYLOAD_BODIES_LIMIT {
                 tx.send(Err(EngineApiError::PayloadRequestTooLarge { len: count })).ok();
-                return
+                return;
             }
 
             if start == 0 || count == 0 {
                 tx.send(Err(EngineApiError::InvalidBodiesRange { start, count })).ok();
-                return
+                return;
             }
 
             let mut result = Vec::with_capacity(count as usize);
@@ -291,7 +291,7 @@ where
                     }
                     Err(err) => {
                         tx.send(Err(EngineApiError::Internal(Box::new(err)))).ok();
-                        return
+                        return;
                     }
                 };
             }
@@ -308,7 +308,7 @@ where
     ) -> EngineApiResult<ExecutionPayloadBodiesV1> {
         let len = hashes.len() as u64;
         if len > MAX_PAYLOAD_BODIES_LIMIT {
-            return Err(EngineApiError::PayloadRequestTooLarge { len })
+            return Err(EngineApiError::PayloadRequestTooLarge { len });
         }
 
         let mut result = Vec::with_capacity(hashes.len());
@@ -348,7 +348,7 @@ where
             return Err(EngineApiError::TerminalTD {
                 execution: merge_terminal_td,
                 consensus: terminal_total_difficulty,
-            })
+            });
         }
 
         self.inner.beacon_consensus.transition_configuration_exchanged().await;
@@ -358,7 +358,7 @@ where
             return Ok(TransitionConfiguration {
                 terminal_total_difficulty: merge_terminal_td,
                 ..Default::default()
-            })
+            });
         }
 
         // Attempt to look up terminal block hash
@@ -413,7 +413,7 @@ where
             // 1. Client software **MUST** return `-38005: Unsupported fork` error if the
             //    `timestamp` of payload or payloadAttributes greater or equal to the Cancun
             //    activation timestamp.
-            return Err(EngineApiError::UnsupportedFork)
+            return Err(EngineApiError::UnsupportedFork);
         }
 
         if version == EngineApiMessageVersion::V3 && !is_cancun {
@@ -423,7 +423,7 @@ where
             // 1. Client software **MUST** return `-38005: Unsupported fork` error if the
             //    `timestamp` of the built payload does not fall within the time frame of the Cancun
             //    fork.
-            return Err(EngineApiError::UnsupportedFork)
+            return Err(EngineApiError::UnsupportedFork);
         }
         Ok(())
     }
@@ -443,18 +443,18 @@ where
         match version {
             EngineApiMessageVersion::V1 => {
                 if has_withdrawals {
-                    return Err(EngineApiError::WithdrawalsNotSupportedInV1)
+                    return Err(EngineApiError::WithdrawalsNotSupportedInV1);
                 }
                 if is_shanghai {
-                    return Err(EngineApiError::NoWithdrawalsPostShanghai)
+                    return Err(EngineApiError::NoWithdrawalsPostShanghai);
                 }
             }
             EngineApiMessageVersion::V2 | EngineApiMessageVersion::V3 => {
                 if is_shanghai && !has_withdrawals {
-                    return Err(EngineApiError::NoWithdrawalsPostShanghai)
+                    return Err(EngineApiError::NoWithdrawalsPostShanghai);
                 }
                 if !is_shanghai && has_withdrawals {
-                    return Err(EngineApiError::HasWithdrawalsPreShanghai)
+                    return Err(EngineApiError::HasWithdrawalsPreShanghai);
                 }
             }
         };
@@ -498,12 +498,12 @@ where
         match version {
             EngineApiMessageVersion::V1 | EngineApiMessageVersion::V2 => {
                 if has_parent_beacon_block_root {
-                    return Err(EngineApiError::ParentBeaconBlockRootNotSupportedBeforeV3)
+                    return Err(EngineApiError::ParentBeaconBlockRootNotSupportedBeforeV3);
                 }
             }
             EngineApiMessageVersion::V3 => {
                 if !has_parent_beacon_block_root {
-                    return Err(EngineApiError::NoParentBeaconBlockRootPostCancun)
+                    return Err(EngineApiError::NoParentBeaconBlockRootPostCancun);
                 }
             }
         };
@@ -558,8 +558,10 @@ where
             let attr_validation_res = self.validate_version_specific_fields(version, &attrs.into());
 
             #[cfg(feature = "optimism")]
-            if attrs.gas_limit.is_none() && self.inner.chain_spec.optimism {
-                return Err(EngineApiError::MissingGasLimitInPayloadAttributes)
+            if attrs.optimism_payload_attributes.gas_limit.is_none() &&
+                self.inner.chain_spec.optimism
+            {
+                return Err(EngineApiError::MissingGasLimitInPayloadAttributes);
             }
 
             // From the engine API spec:
@@ -580,9 +582,9 @@ where
                 // TODO: decide if we want this branch - the FCU INVALID response might be more
                 // useful than the payload attributes INVALID response
                 if fcu_res.is_invalid() {
-                    return Ok(fcu_res)
+                    return Ok(fcu_res);
                 }
-                return Err(err)
+                return Err(err);
             }
         }