Skip to content

refactor: remove ExtraCtx generic param #404

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
akundaz merged 1 commit into from
Feb 25, 2026
Merged

refactor: remove ExtraCtx generic param #404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
186 changes: 95 additions & 91 deletions crates/op-rbuilder/src/builders/builder_tx.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,25 +138,31 @@ impl BuilderTransactionError {
}
}

pub trait BuilderTransactions<ExtraCtx: Debug + Default = (), Extra: Debug + Default = ()> {
pub trait BuilderTransactions {
// Simulates and returns the signed builder transactions. The simulation modifies and commit
// changes to the db so call new_simulation_state to simulate on a new copy of the state
#[expect(clippy::too_many_arguments)]
fn simulate_builder_txs(
&self,
state_provider: impl StateProvider + Clone,
info: &mut ExecutionInfo<Extra>,
ctx: &OpPayloadBuilderCtx<ExtraCtx>,
info: &mut ExecutionInfo,
ctx: &OpPayloadBuilderCtx,
db: &mut State<impl Database + DatabaseRef>,
top_of_block: bool,
is_first_flashblock: bool,
is_last_flashblock: bool,
) -> Result<Vec<BuilderTransactionCtx>, BuilderTransactionError>;

#[expect(clippy::too_many_arguments)]
fn simulate_builder_txs_with_state_copy(
&self,
state_provider: impl StateProvider + Clone,
info: &mut ExecutionInfo<Extra>,
ctx: &OpPayloadBuilderCtx<ExtraCtx>,
info: &mut ExecutionInfo,
ctx: &OpPayloadBuilderCtx,
db: &State<impl Database>,
top_of_block: bool,
is_first_flashblock: bool,
is_last_flashblock: bool,
) -> Result<Vec<BuilderTransactionCtx>, BuilderTransactionError> {
let mut simulation_state = self.new_simulation_state(state_provider.clone(), db);
self.simulate_builder_txs(
Expand All @@ -165,98 +171,100 @@ pub trait BuilderTransactions<ExtraCtx: Debug + Default = (), Extra: Debug + Def
ctx,
&mut simulation_state,
top_of_block,
is_first_flashblock,
is_last_flashblock,
)
}

#[expect(clippy::too_many_arguments)]
fn add_builder_txs(
&self,
state_provider: impl StateProvider + Clone,
info: &mut ExecutionInfo<Extra>,
builder_ctx: &OpPayloadBuilderCtx<ExtraCtx>,
info: &mut ExecutionInfo,
builder_ctx: &OpPayloadBuilderCtx,
db: &mut State<impl Database>,
top_of_block: bool,
is_first_flashblock: bool,
is_last_flashblock: bool,
) -> Result<Vec<BuilderTransactionCtx>, BuilderTransactionError> {
{
let builder_txs = self.simulate_builder_txs_with_state_copy(
state_provider,
info,
builder_ctx,
db,
top_of_block,
)?;

let mut evm = builder_ctx
.evm_config
.evm_with_env(&mut *db, builder_ctx.evm_env.clone());

let mut invalid = HashSet::new();

for builder_tx in builder_txs.iter() {
if builder_tx.is_top_of_block != top_of_block {
// don't commit tx if the buidler tx is not being added in the intended
// position in the block
continue;
}
if invalid.contains(&builder_tx.signed_tx.signer()) {
warn!(target: "payload_builder", tx_hash = ?builder_tx.signed_tx.tx_hash(), "builder signer invalid as previous builder tx reverted");
continue;
}
let builder_txs = self.simulate_builder_txs_with_state_copy(
state_provider,
info,
builder_ctx,
db,
top_of_block,
is_first_flashblock,
is_last_flashblock,
)?;

let mut evm = builder_ctx
.evm_config
.evm_with_env(&mut *db, builder_ctx.evm_env.clone());

let mut invalid = HashSet::new();

for builder_tx in builder_txs.iter() {
if builder_tx.is_top_of_block != top_of_block {
// don't commit tx if the buidler tx is not being added in the intended
// position in the block
continue;
}
if invalid.contains(&builder_tx.signed_tx.signer()) {
warn!(target: "payload_builder", tx_hash = ?builder_tx.signed_tx.tx_hash(), "builder signer invalid as previous builder tx reverted");
continue;
}

let ResultAndState { result, state } = match evm.transact(&builder_tx.signed_tx) {
Ok(res) => res,
Err(err) => {
if let Some(err) = err.as_invalid_tx_err() {
if err.is_nonce_too_low() {
// if the nonce is too low, we can skip this transaction
trace!(target: "payload_builder", %err, ?builder_tx.signed_tx, "skipping nonce too low builder transaction");
} else {
// if the transaction is invalid, we can skip it and all of its
// descendants
trace!(target: "payload_builder", %err, ?builder_tx.signed_tx, "skipping invalid builder transaction and its descendants");
invalid.insert(builder_tx.signed_tx.signer());
}

continue;
let ResultAndState { result, state } = match evm.transact(&builder_tx.signed_tx) {
Ok(res) => res,
Err(err) => {
if let Some(err) = err.as_invalid_tx_err() {
if err.is_nonce_too_low() {
// if the nonce is too low, we can skip this transaction
trace!(target: "payload_builder", %err, ?builder_tx.signed_tx, "skipping nonce too low builder transaction");
} else {
// if the transaction is invalid, we can skip it and all of its
// descendants
trace!(target: "payload_builder", %err, ?builder_tx.signed_tx, "skipping invalid builder transaction and its descendants");
invalid.insert(builder_tx.signed_tx.signer());
}
// this is an error that we should treat as fatal for this attempt
return Err(BuilderTransactionError::EvmExecutionError(Box::new(err)));
}
};

if !result.is_success() {
warn!(target: "payload_builder", tx_hash = ?builder_tx.signed_tx.tx_hash(), result = ?result, "builder tx reverted");
invalid.insert(builder_tx.signed_tx.signer());
continue;
continue;
}
// this is an error that we should treat as fatal for this attempt
return Err(BuilderTransactionError::EvmExecutionError(Box::new(err)));
}
};

// Add gas used by the transaction to cumulative gas used, before creating the receipt
let gas_used = result.gas_used();
info.cumulative_gas_used += gas_used;
info.cumulative_da_bytes_used += builder_tx.da_size;

let ctx = ReceiptBuilderCtx {
tx: builder_tx.signed_tx.inner(),
evm: &evm,
result,
state: &state,
cumulative_gas_used: info.cumulative_gas_used,
};
info.receipts.push(builder_ctx.build_receipt(ctx, None));

// Commit changes
evm.db_mut().commit(state);

// Append sender and transaction to the respective lists
info.executed_senders.push(builder_tx.signed_tx.signer());
info.executed_transactions
.push(builder_tx.signed_tx.clone().into_inner());
if !result.is_success() {
warn!(target: "payload_builder", tx_hash = ?builder_tx.signed_tx.tx_hash(), result = ?result, "builder tx reverted");
invalid.insert(builder_tx.signed_tx.signer());
continue;
}

// Release the db reference by dropping evm
drop(evm);

Ok(builder_txs)
// Add gas used by the transaction to cumulative gas used, before creating the receipt
let gas_used = result.gas_used();
info.cumulative_gas_used += gas_used;
info.cumulative_da_bytes_used += builder_tx.da_size;

let ctx = ReceiptBuilderCtx {
tx: builder_tx.signed_tx.inner(),
evm: &evm,
result,
state: &state,
cumulative_gas_used: info.cumulative_gas_used,
};
info.receipts.push(builder_ctx.build_receipt(ctx, None));

// Commit changes
evm.db_mut().commit(state);

// Append sender and transaction to the respective lists
info.executed_senders.push(builder_tx.signed_tx.signer());
info.executed_transactions
.push(builder_tx.signed_tx.clone().into_inner());
}

Ok(builder_txs)
}

// Creates a copy of the state to simulate against
Expand All @@ -280,7 +288,7 @@ pub trait BuilderTransactions<ExtraCtx: Debug + Default = (), Extra: Debug + Def
from: Signer,
gas_used: u64,
calldata: Bytes,
ctx: &OpPayloadBuilderCtx<ExtraCtx>,
ctx: &OpPayloadBuilderCtx,
db: impl DatabaseRef,
) -> Result<Recovered<OpTransactionSigned>, BuilderTransactionError> {
let nonce = get_nonce(db, from.address)?;
Expand All @@ -301,7 +309,7 @@ pub trait BuilderTransactions<ExtraCtx: Debug + Default = (), Extra: Debug + Def
fn commit_txs(
&self,
signed_txs: Vec<Recovered<OpTransactionSigned>>,
ctx: &OpPayloadBuilderCtx<ExtraCtx>,
ctx: &OpPayloadBuilderCtx,
db: &mut State<impl Database>,
) -> Result<(), BuilderTransactionError> {
let mut evm = ctx.evm_config.evm_with_env(&mut *db, ctx.evm_env.clone());
Expand Down Expand Up @@ -389,22 +397,18 @@ pub trait BuilderTransactions<ExtraCtx: Debug + Default = (), Extra: Debug + Def
}

#[derive(Debug, Clone)]
pub(super) struct BuilderTxBase<ExtraCtx = ()> {
pub(super) struct BuilderTxBase {
pub signer: Option<Signer>,
_marker: std::marker::PhantomData<ExtraCtx>,
}

impl<ExtraCtx: Debug + Default> BuilderTxBase<ExtraCtx> {
impl BuilderTxBase {
pub(super) fn new(signer: Option<Signer>) -> Self {
Self {
signer,
_marker: std::marker::PhantomData,
}
Self { signer }
}

pub(super) fn simulate_builder_tx(
&self,
ctx: &OpPayloadBuilderCtx<ExtraCtx>,
ctx: &OpPayloadBuilderCtx,
db: impl DatabaseRef,
) -> Result<Option<BuilderTransactionCtx>, BuilderTransactionError> {
match self.signer {
Expand Down Expand Up @@ -449,7 +453,7 @@ impl<ExtraCtx: Debug + Default> BuilderTxBase<ExtraCtx> {

fn signed_builder_tx(
&self,
ctx: &OpPayloadBuilderCtx<ExtraCtx>,
ctx: &OpPayloadBuilderCtx,
db: impl DatabaseRef,
signer: Signer,
gas_used: u64,
Expand Down
41 changes: 12 additions & 29 deletions crates/op-rbuilder/src/builders/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,9 @@ use crate::{
tx_signer::Signer,
};

/// Extra context trait that optionally provides the current flashblock index within a block.
pub trait MaybeFlashblockIndex {
fn flashblock_index(&self) -> Option<u64> {
None
}
}

/// Standard (non-flashblock) builds have no flashblock index.
impl MaybeFlashblockIndex for () {}

/// Container type that holds all necessities to build a new payload.
#[derive(Debug)]
pub struct OpPayloadBuilderCtx<ExtraCtx: Debug + Default = ()> {
pub struct OpPayloadBuilderCtx {
/// The type that knows how to perform system calls and configure the evm.
pub evm_config: OpEvmConfig,
/// The DA config for the payload builder
Expand All @@ -86,8 +76,6 @@ pub struct OpPayloadBuilderCtx<ExtraCtx: Debug + Default = ()> {
pub builder_signer: Option<Signer>,
/// The metrics for the builder
pub metrics: Arc<OpRBuilderMetrics>,
/// Extra context for the payload builder
pub extra_ctx: ExtraCtx,
/// Max gas that can be used by a transaction.
pub max_gas_per_txn: Option<u64>,
/// Rate limiting based on gas. This is an optional feature.
Expand All @@ -96,15 +84,11 @@ pub struct OpPayloadBuilderCtx<ExtraCtx: Debug + Default = ()> {
pub backrun_ctx: BackrunBundlesPayloadCtx,
}

impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
impl OpPayloadBuilderCtx {
pub(super) fn with_cancel(self, cancel: CancellationToken) -> Self {
Self { cancel, ..self }
}

pub(super) fn with_extra_ctx(self, extra_ctx: ExtraCtx) -> Self {
Self { extra_ctx, ..self }
}

/// Returns the parent block the payload will be build on.
pub fn parent(&self) -> &SealedHeader {
&self.config.parent_header
Expand Down Expand Up @@ -166,10 +150,7 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
/// This will return the culmative DA bytes * scalar after Jovian
/// after Ecotone, this will always return Some(0) as blobs aren't supported
/// pre Ecotone, these fields aren't used.
pub fn blob_fields<Extra: Debug + Default>(
&self,
info: &ExecutionInfo<Extra>,
) -> (Option<u64>, Option<u64>) {
pub fn blob_fields(&self, info: &ExecutionInfo) -> (Option<u64>, Option<u64>) {
// For payload validation
if let Some(blob_fields) = info.optional_blob_fields {
return blob_fields;
Expand Down Expand Up @@ -265,7 +246,7 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
}
}

impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
impl OpPayloadBuilderCtx {
/// Constructs a receipt for the given transaction.
pub fn build_receipt<E: Evm>(
&self,
Expand Down Expand Up @@ -299,10 +280,10 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
}

/// Executes all sequencer transactions that are included in the payload attributes.
pub(super) fn execute_sequencer_transactions<E: Debug + Default>(
pub(super) fn execute_sequencer_transactions(
&self,
db: &mut State<impl Database>,
) -> Result<ExecutionInfo<E>, PayloadBuilderError> {
) -> Result<ExecutionInfo, PayloadBuilderError> {
let mut info = ExecutionInfo::with_capacity(self.attributes().transactions.len());

let mut evm = self.evm_config.evm_with_env(&mut *db, self.evm_env.clone());
Expand Down Expand Up @@ -398,18 +379,20 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
}
}

impl<ExtraCtx: Debug + Default + MaybeFlashblockIndex> OpPayloadBuilderCtx<ExtraCtx> {
impl OpPayloadBuilderCtx {
/// Executes the given best transactions and updates the execution info.
///
/// Returns `Ok(Some(())` if the job was cancelled.
pub(super) fn execute_best_transactions<E: Debug + Default>(
#[expect(clippy::too_many_arguments)]
pub(super) fn execute_best_transactions(
&self,
info: &mut ExecutionInfo<E>,
info: &mut ExecutionInfo,
db: &mut State<impl Database>,
best_txs: &mut impl PayloadTxsBounds,
block_gas_limit: u64,
block_da_limit: Option<u64>,
block_da_footprint_limit: Option<u64>,
flashblock_index: Option<u64>,
) -> Result<Option<()>, PayloadBuilderError> {
let execute_txs_start_time = Instant::now();
let mut num_txs_considered = 0;
Expand Down Expand Up @@ -704,7 +687,7 @@ impl<ExtraCtx: Debug + Default + MaybeFlashblockIndex> OpPayloadBuilderCtx<Extra
num_txs_considered += 1;
num_backruns_considered += 1;

if !bundle.is_valid(block_attr.number, self.extra_ctx.flashblock_index()) {
if !bundle.is_valid(block_attr.number, flashblock_index) {
log_br_txn(TxnExecutionResult::ConditionalCheckFailed);
continue;
}
Expand Down
Loading
Loading