Add permit flashtestations tx calls from builder

crates/op-rbuilder/src/builders/flashblocks/builder_tx.rs

@@ -2,7 +2,7 @@ use alloy_consensus::TxEip1559;
 use alloy_eips::Encodable2718;
 use alloy_evm::{Database, Evm};
 use alloy_op_evm::OpEvm;
-use alloy_primitives::{Address, B256, TxKind};
+use alloy_primitives::{Address, TxKind};
 use alloy_sol_types::{Error, SolCall, SolEvent, SolInterface, sol};
 use core::fmt::Debug;
 use op_alloy_consensus::OpTypedTransaction;
@@ -11,8 +11,9 @@ use reth_evm::{ConfigureEvm, precompiles::PrecompilesMap};
 use reth_optimism_primitives::OpTransactionSigned;
 use reth_primitives::Recovered;
 use reth_provider::StateProvider;
-use reth_revm::{State, database::StateProviderDatabase};
+use reth_revm::State;
 use revm::{
+    DatabaseRef,
     context::result::{ExecutionResult, ResultAndState},
     inspector::NoOpInspector,
 };
@@ -21,9 +22,10 @@ use tracing::warn;
 use crate::{
     builders::{
         BuilderTransactionCtx, BuilderTransactionError, BuilderTransactions,
-        builder_tx::{BuilderTxBase, get_nonce, log_exists},
+        InvalidContractDataError,
+        builder_tx::{BuilderTxBase, get_nonce},
         context::OpPayloadBuilderCtx,
-        flashblocks::payload::FlashblocksExtraCtx,
+        flashblocks::payload::{FlashblocksExecutionInfo, FlashblocksExtraCtx},
     },
     flashtestations::builder_tx::FlashtestationsBuilderTx,
     primitives::reth::ExecutionInfo,
@@ -53,8 +55,6 @@ sol!(
 pub(super) enum FlashblockNumberError {
     #[error("flashblocks number contract tx reverted: {0:?}")]
     Revert(IFlashblockNumber::IFlashblockNumberErrors),
-    #[error("contract may be invalid, mismatch in log emitted: expected {0:?}")]
-    LogMismatch(B256),
     #[error("unknown revert: {0} err: {1}")]
     Unknown(String, Error),
     #[error("halt: {0:?}")]
@@ -64,14 +64,17 @@ pub(super) enum FlashblockNumberError {
 // This will be the end of block transaction of a regular block
 #[derive(Debug, Clone)]
 pub(super) struct FlashblocksBuilderTx {
-    pub base_builder_tx: BuilderTxBase,
-    pub flashtestations_builder_tx: Option<FlashtestationsBuilderTx>,
+    pub base_builder_tx: BuilderTxBase<FlashblocksExtraCtx>,
+    pub flashtestations_builder_tx:
+        Option<FlashtestationsBuilderTx<FlashblocksExtraCtx, FlashblocksExecutionInfo>>,
 }
 
 impl FlashblocksBuilderTx {
     pub(super) fn new(
         signer: Option<Signer>,
-        flashtestations_builder_tx: Option<FlashtestationsBuilderTx>,
+        flashtestations_builder_tx: Option<
+            FlashtestationsBuilderTx<FlashblocksExtraCtx, FlashblocksExecutionInfo>,
+        >,
     ) -> Self {
         let base_builder_tx = BuilderTxBase::new(signer);
         Self {
@@ -81,40 +84,46 @@ impl FlashblocksBuilderTx {
     }
 }
 
-impl BuilderTransactions<FlashblocksExtraCtx> for FlashblocksBuilderTx {
-    fn simulate_builder_txs<Extra: Debug + Default>(
+impl BuilderTransactions<FlashblocksExtraCtx, FlashblocksExecutionInfo> for FlashblocksBuilderTx {
+    fn simulate_builder_txs(
         &self,
         state_provider: impl StateProvider + Clone,
-        info: &mut ExecutionInfo<Extra>,
+        info: &mut ExecutionInfo<FlashblocksExecutionInfo>,
         ctx: &OpPayloadBuilderCtx<FlashblocksExtraCtx>,
-        db: &mut State<impl Database>,
+        db: &mut State<impl Database + DatabaseRef>,
+        top_of_block: bool,
     ) -> Result<Vec<BuilderTransactionCtx>, BuilderTransactionError> {
         let mut builder_txs = Vec::<BuilderTransactionCtx>::new();
 
         if ctx.is_first_flashblock() {
-            let flashblocks_builder_tx = self.base_builder_tx.simulate_builder_tx(ctx, db)?;
+            let flashblocks_builder_tx = self.base_builder_tx.simulate_builder_tx(ctx, &mut *db)?;
             builder_txs.extend(flashblocks_builder_tx.clone());
         }
 
         if ctx.is_last_flashblock() {
-            let base_tx = self.base_builder_tx.simulate_builder_tx(ctx, db)?;
+            let base_tx = self.base_builder_tx.simulate_builder_tx(ctx, &mut *db)?;
             builder_txs.extend(base_tx.clone());
 
             if let Some(flashtestations_builder_tx) = &self.flashtestations_builder_tx {
+                // Commit state that is included to get the correct nonce
+                if let Some(builder_tx) = base_tx {
+                    self.commit_txs(vec![builder_tx.signed_tx], ctx, db)?;
+                }
                 // We only include flashtestations txs in the last flashblock
-                let mut simulation_state = self.simulate_builder_txs_state::<FlashblocksExtraCtx>(
-                    state_provider.clone(),
-                    base_tx.iter().collect(),
-                    ctx,
-                    db,
-                )?;
-                let flashtestations_builder_txs = flashtestations_builder_tx.simulate_builder_txs(
+                match flashtestations_builder_tx.simulate_builder_txs(
                     state_provider,
                     info,
                     ctx,
-                    &mut simulation_state,
-                )?;
-                builder_txs.extend(flashtestations_builder_txs);
+                    db,
+                    top_of_block,
+                ) {
+                    Ok(flashtestations_builder_txs) => {
+                        builder_txs.extend(flashtestations_builder_txs)
+                    }
+                    Err(e) => {
+                        warn!(target: "flashtestations", error = ?e, "failed to add flashtestations builder tx")
+                    }
+                }
             }
         }
         Ok(builder_txs)
@@ -126,15 +135,18 @@ impl BuilderTransactions<FlashblocksExtraCtx> for FlashblocksBuilderTx {
 pub(super) struct FlashblocksNumberBuilderTx {
     pub signer: Option<Signer>,
     pub flashblock_number_address: Address,
-    pub base_builder_tx: BuilderTxBase,
-    pub flashtestations_builder_tx: Option<FlashtestationsBuilderTx>,
+    pub base_builder_tx: BuilderTxBase<FlashblocksExtraCtx>,
+    pub flashtestations_builder_tx:
+        Option<FlashtestationsBuilderTx<FlashblocksExtraCtx, FlashblocksExecutionInfo>>,
 }
 
 impl FlashblocksNumberBuilderTx {
     pub(super) fn new(
         signer: Option<Signer>,
         flashblock_number_address: Address,
-        flashtestations_builder_tx: Option<FlashtestationsBuilderTx>,
+        flashtestations_builder_tx: Option<
+            FlashtestationsBuilderTx<FlashblocksExtraCtx, FlashblocksExecutionInfo>,
+        >,
     ) -> Self {
         let base_builder_tx = BuilderTxBase::new(signer);
         Self {
@@ -145,14 +157,11 @@ impl FlashblocksNumberBuilderTx {
         }
     }
 
+    // TODO: remove and clean up in favour of simulate_call()
     fn estimate_flashblock_number_tx_gas(
         &self,
         ctx: &OpPayloadBuilderCtx<FlashblocksExtraCtx>,
-        evm: &mut OpEvm<
-            State<StateProviderDatabase<impl StateProvider + Clone>>,
-            NoOpInspector,
-            PrecompilesMap,
-        >,
+        evm: &mut OpEvm<impl Database, NoOpInspector, PrecompilesMap>,
         signer: &Signer,
         nonce: u64,
     ) -> Result<u64, BuilderTransactionError> {
@@ -166,15 +175,17 @@ impl FlashblocksNumberBuilderTx {
 
         match result {
             ExecutionResult::Success { gas_used, logs, .. } => {
-                if log_exists(
-                    &logs,
-                    &IFlashblockNumber::FlashblockIncremented::SIGNATURE_HASH,
-                ) {
+                if logs.iter().any(|log| {
+                    log.topics().first()
+                        == Some(&IFlashblockNumber::FlashblockIncremented::SIGNATURE_HASH)
+                }) {
                     Ok(gas_used)
                 } else {
-                    Err(BuilderTransactionError::other(
-                        FlashblockNumberError::LogMismatch(
-                            IFlashblockNumber::FlashblockIncremented::SIGNATURE_HASH,
+                    Err(BuilderTransactionError::InvalidContract(
+                        self.flashblock_number_address,
+                        InvalidContractDataError::InvalidLogs(
+                            vec![IFlashblockNumber::FlashblockIncremented::SIGNATURE_HASH],
+                            vec![],
                         ),
                     ))
                 }
@@ -213,33 +224,29 @@ impl FlashblocksNumberBuilderTx {
     }
 }
 
-impl BuilderTransactions<FlashblocksExtraCtx> for FlashblocksNumberBuilderTx {
-    fn simulate_builder_txs<Extra: Debug + Default>(
+impl BuilderTransactions<FlashblocksExtraCtx, FlashblocksExecutionInfo>
+    for FlashblocksNumberBuilderTx
+{
+    fn simulate_builder_txs(
         &self,
         state_provider: impl StateProvider + Clone,
-        info: &mut ExecutionInfo<Extra>,
+        info: &mut ExecutionInfo<FlashblocksExecutionInfo>,
         ctx: &OpPayloadBuilderCtx<FlashblocksExtraCtx>,
-        db: &mut State<impl Database>,
+        db: &mut State<impl Database + DatabaseRef>,
+        top_of_block: bool,
     ) -> Result<Vec<BuilderTransactionCtx>, BuilderTransactionError> {
         let mut builder_txs = Vec::<BuilderTransactionCtx>::new();
-        let state = StateProviderDatabase::new(state_provider.clone());
-        let simulation_state = State::builder()
-            .with_database(state)
-            .with_cached_prestate(db.cache.clone())
-            .with_bundle_update()
-            .build();
 
         if ctx.is_first_flashblock() {
             // fallback block builder tx
-            builder_txs.extend(self.base_builder_tx.simulate_builder_tx(ctx, db)?);
+            builder_txs.extend(self.base_builder_tx.simulate_builder_tx(ctx, &mut *db)?);
         } else {
             // we increment the flashblock number for the next flashblock so we don't increment in the last flashblock
             if let Some(signer) = &self.signer {
-                let mut evm = ctx
-                    .evm_config
-                    .evm_with_env(simulation_state, ctx.evm_env.clone());
+                let mut evm = ctx.evm_config.evm_with_env(&mut *db, ctx.evm_env.clone());
                 evm.modify_cfg(|cfg| {
                     cfg.disable_balance_check = true;
+                    cfg.disable_block_gas_limit = true;
                 });
 
                 let nonce = get_nonce(evm.db_mut(), signer.address)?;
@@ -268,7 +275,7 @@ impl BuilderTransactions<FlashblocksExtraCtx> for FlashblocksNumberBuilderTx {
                     Err(e) => {
                         warn!(target: "builder_tx", error = ?e, "Flashblocks number contract tx simulation failed, defaulting to fallback builder tx");
                         self.base_builder_tx
-                            .simulate_builder_tx(ctx, db)?
+                            .simulate_builder_tx(ctx, &mut *db)?
                             .map(|tx| tx.set_top_of_block())
                     }
                 };
@@ -279,21 +286,29 @@ impl BuilderTransactions<FlashblocksExtraCtx> for FlashblocksNumberBuilderTx {
 
         if ctx.is_last_flashblock() {
             if let Some(flashtestations_builder_tx) = &self.flashtestations_builder_tx {
-                let flashblocks_builder_txs = builder_txs.clone();
-                let mut simulation_state = self.simulate_builder_txs_state::<FlashblocksExtraCtx>(
-                    state_provider.clone(),
-                    flashblocks_builder_txs.iter().collect(),
-                    ctx,
-                    db,
-                )?;
+                // Commit state that should be included to compute the correct nonce
+                let flashblocks_builder_txs = builder_txs
+                    .iter()
+                    .filter(|tx| tx.is_top_of_block == top_of_block)
+                    .map(|tx| tx.signed_tx.clone())
+                    .collect();
+                self.commit_txs(flashblocks_builder_txs, ctx, &mut *db)?;
+
                 // We only include flashtestations txs in the last flashblock
-                let flashtestations_builder_txs = flashtestations_builder_tx.simulate_builder_txs(
+                match flashtestations_builder_tx.simulate_builder_txs(
                     state_provider,
                     info,
                     ctx,
-                    &mut simulation_state,
-                )?;
-                builder_txs.extend(flashtestations_builder_txs);
+                    db,
+                    top_of_block,
+                ) {
+                    Ok(flashtestations_builder_txs) => {
+                        builder_txs.extend(flashtestations_builder_txs)
+                    }
+                    Err(e) => {
+                        warn!(target: "flashtestations", error = ?e, "failed to add flashtestations builder tx")
+                    }
+                }
             }
         }
 

crates/op-rbuilder/src/builders/flashblocks/payload.rs

@@ -67,13 +67,13 @@ type NextBestFlashblocksTxs<Pool> = BestFlashblocksTxs<
     >,
 >;
 
-#[derive(Debug, Default)]
-struct ExtraExecutionInfo {
+#[derive(Debug, Default, Clone)]
+pub(super) struct FlashblocksExecutionInfo {
     /// Index of the last consumed flashblock
     pub last_flashblock_index: usize,
 }
 
-#[derive(Debug, Default)]
+#[derive(Debug, Default, Clone)]
 pub struct FlashblocksExtraCtx {
     /// Current flashblock index
     flashblock_index: u64,
@@ -211,7 +211,7 @@ impl<Pool, Client, BuilderTx> OpPayloadBuilder<Pool, Client, BuilderTx>
 where
     Pool: PoolBounds,
     Client: ClientBounds,
-    BuilderTx: BuilderTransactions<FlashblocksExtraCtx> + Send + Sync,
+    BuilderTx: BuilderTransactions<FlashblocksExtraCtx, FlashblocksExecutionInfo> + Send + Sync,
 {
     /// Constructs an Optimism payload from the transactions sent via the
     /// Payload attributes by the sequencer. If the `no_tx_pool` argument is passed in
@@ -542,7 +542,7 @@ where
     >(
         &self,
         ctx: &mut OpPayloadBuilderCtx<FlashblocksExtraCtx>,
-        info: &mut ExecutionInfo<ExtraExecutionInfo>,
+        info: &mut ExecutionInfo<FlashblocksExecutionInfo>,
         state: &mut State<DB>,
         state_provider: impl reth::providers::StateProvider + Clone,
         best_txs: &mut NextBestFlashblocksTxs<Pool>,
@@ -756,7 +756,7 @@ where
     fn record_flashblocks_metrics(
         &self,
         ctx: &OpPayloadBuilderCtx<FlashblocksExtraCtx>,
-        info: &ExecutionInfo<ExtraExecutionInfo>,
+        info: &ExecutionInfo<FlashblocksExecutionInfo>,
         flashblocks_per_block: u64,
         span: &tracing::Span,
         message: &str,
@@ -872,7 +872,8 @@ impl<Pool, Client, BuilderTx> PayloadBuilder for OpPayloadBuilder<Pool, Client,
 where
     Pool: PoolBounds,
     Client: ClientBounds,
-    BuilderTx: BuilderTransactions<FlashblocksExtraCtx> + Clone + Send + Sync,
+    BuilderTx:
+        BuilderTransactions<FlashblocksExtraCtx, FlashblocksExecutionInfo> + Clone + Send + Sync,
 {
     type Attributes = OpPayloadBuilderAttributes<OpTransactionSigned>;
     type BuiltPayload = OpBuiltPayload;
@@ -896,7 +897,7 @@ struct FlashblocksMetadata {
 fn execute_pre_steps<DB, ExtraCtx>(
     state: &mut State<DB>,
     ctx: &OpPayloadBuilderCtx<ExtraCtx>,
-) -> Result<ExecutionInfo<ExtraExecutionInfo>, PayloadBuilderError>
+) -> Result<ExecutionInfo<FlashblocksExecutionInfo>, PayloadBuilderError>
 where
     DB: Database<Error = ProviderError> + std::fmt::Debug,
     ExtraCtx: std::fmt::Debug + Default,
@@ -916,7 +917,7 @@ where
 fn build_block<DB, P, ExtraCtx>(
     state: &mut State<DB>,
     ctx: &OpPayloadBuilderCtx<ExtraCtx>,
-    info: &mut ExecutionInfo<ExtraExecutionInfo>,
+    info: &mut ExecutionInfo<FlashblocksExecutionInfo>,
     calculate_state_root: bool,
 ) -> Result<(OpBuiltPayload, FlashblocksPayloadV1), PayloadBuilderError>
 where

crates/op-rbuilder/src/builders/flashblocks/service.rs

@@ -5,7 +5,7 @@ use crate::{
         builder_tx::BuilderTransactions,
         flashblocks::{
             builder_tx::{FlashblocksBuilderTx, FlashblocksNumberBuilderTx},
-            payload::FlashblocksExtraCtx,
+            payload::{FlashblocksExecutionInfo, FlashblocksExtraCtx},
         },
         generator::BlockPayloadJobGenerator,
     },
@@ -32,7 +32,12 @@ impl FlashblocksServiceBuilder {
     where
         Node: NodeBounds,
         Pool: PoolBounds,
-        BuilderTx: BuilderTransactions<FlashblocksExtraCtx> + Unpin + Clone + Send + Sync + 'static,
+        BuilderTx: BuilderTransactions<FlashblocksExtraCtx, FlashblocksExecutionInfo>
+            + Unpin
+            + Clone
+            + Send
+            + Sync
+            + 'static,
     {
         let once_lock = Arc::new(std::sync::OnceLock::new());
 
@@ -84,8 +89,12 @@ where
         _: OpEvmConfig,
     ) -> eyre::Result<PayloadBuilderHandle<<Node::Types as NodeTypes>::Payload>> {
         let signer = self.0.builder_signer;
-        let flashtestations_builder_tx = if self.0.flashtestations_config.flashtestations_enabled {
-            match bootstrap_flashtestations(self.0.flashtestations_config.clone(), ctx).await {
+        let flashtestations_builder_tx = if let Some(builder_key) = signer
+            && self.0.flashtestations_config.flashtestations_enabled
+        {
+            match bootstrap_flashtestations(self.0.flashtestations_config.clone(), builder_key, ctx)
+                .await
+            {
                 Ok(builder_tx) => Some(builder_tx),
                 Err(e) => {
                     tracing::warn!(error = %e, "Failed to bootstrap flashtestations, builder will not include flashtestations txs");