Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
4 changes: 4 additions & 0 deletions crates/op-rbuilder/src/args/op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ pub struct OpRbuilderArgs {
)]
pub chain_block_time: u64,

/// max gas a transaction can use
#[arg(long = "builder.max_gas_per_txn", default_value = "25000")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the default value

pub max_gas_per_txn: Option<u64>,

/// Signals whether to log pool transaction events
#[arg(long = "builder.log-pool-transactions", default_value = "false")]
pub log_pool_transactions: bool,
Expand Down
10 changes: 10 additions & 0 deletions crates/op-rbuilder/src/builders/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ pub struct OpPayloadBuilderCtx<ExtraCtx: Debug + Default = ()> {
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>,
}

impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
Expand Down Expand Up @@ -491,6 +493,14 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
// add gas used by the transaction to cumulative gas used, before creating the
// receipt
let gas_used = result.gas_used();
if let Some(max_gas_per_txn) = self.max_gas_per_txn {
if gas_used > max_gas_per_txn {
log_txn(TxnExecutionResult::MaxGasUsageExceeded);
best_txs.mark_invalid(tx.signer(), tx.nonce());
continue;
}
}

info.cumulative_gas_used += gas_used;
// record tx da size
info.cumulative_da_bytes_used += tx_da_size;
Expand Down
1 change: 1 addition & 0 deletions crates/op-rbuilder/src/builders/flashblocks/payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ where
flashblock_index: 0,
target_flashblock_count: self.config.flashblocks_per_block(),
},
max_gas_per_txn: self.config.max_gas_per_txn,
};

let state_provider = self.client.state_by_block_hash(ctx.parent().hash())?;
Expand Down
4 changes: 4 additions & 0 deletions crates/op-rbuilder/src/builders/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ pub struct BuilderConfig<Specific: Clone> {

/// Configuration values that are specific to the block builder implementation used.
pub specific: Specific,
/// Maximum gas a transaction can use before being excluded.
pub max_gas_per_txn: Option<u64>,
}

impl<S: Debug + Clone> core::fmt::Debug for BuilderConfig<S> {
Expand Down Expand Up @@ -145,6 +147,7 @@ impl<S: Default + Clone> Default for BuilderConfig<S> {
da_config: OpDAConfig::default(),
specific: S::default(),
sampling_ratio: 100,
max_gas_per_txn: Some(25_000),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default should be None

}
}
}
Expand All @@ -164,6 +167,7 @@ where
block_time_leeway: Duration::from_secs(args.extra_block_deadline_secs),
da_config: Default::default(),
sampling_ratio: args.telemetry.sampling_ratio,
max_gas_per_txn: args.max_gas_per_txn,
specific: S::try_from(args)?,
})
}
Expand Down
1 change: 1 addition & 0 deletions crates/op-rbuilder/src/builders/standard/payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ where
builder_signer: self.config.builder_signer,
metrics: self.metrics.clone(),
extra_ctx: Default::default(),
max_gas_per_txn: self.config.max_gas_per_txn,
};

let builder = OpBuilder::new(best);
Expand Down
1 change: 1 addition & 0 deletions crates/op-rbuilder/src/primitives/reth/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub enum TxnExecutionResult {
Success,
Reverted,
RevertedAndExcluded,
MaxGasUsageExceeded,
}

#[derive(Default, Debug)]
Expand Down
5 changes: 5 additions & 0 deletions crates/op-rbuilder/src/tests/framework/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use super::{TransactionBuilder, FUNDED_PRIVATE_KEYS};
pub trait TransactionBuilderExt {
fn random_valid_transfer(self) -> Self;
fn random_reverting_transaction(self) -> Self;
fn random_big_transaction(self) -> Self;
}

impl TransactionBuilderExt for TransactionBuilder {
Expand All @@ -33,6 +34,10 @@ impl TransactionBuilderExt for TransactionBuilder {
fn random_reverting_transaction(self) -> Self {
self.with_create().with_input(hex!("60006000fd").into()) // PUSH1 0x00 PUSH1 0x00 REVERT
}

fn random_big_transaction(self) -> Self {
self.with_create().with_input(hex!("3593564c000000000000000000000000000000000000000000000000000000000000006000000000000000000000000000000000000000000000000000000000000000a0000000000000000000000000000000000000000000000000000000006883d15f0000000000000000000000000000000000000000000000000000000000000004100504040000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000046000000000000000000000000000000000000000000000000000000000000004e0000000000000000000000000000000000000000000000000000000000000056000000000000000000000000000000000000000000000000000000000000003c0000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000003090b0e000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000030000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000022000000000000000000000000000000000000000000000000000000000000002a000000000000000000000000000000000000000000000000000000000000001a000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000418fa3488d7e13f0c06ac5f8485d306b5748f4f00000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000e9bacdc46b210000000000000000000000000000000000000000000000000000000da673855a3f900000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000640000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000006000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000600000000000000000000000000418fa3488d7e13f0c06ac5f8485d306b5748f4f0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000600000000000000000000000000418fa3488d7e13f0c06ac5f8485d306b5748f4f000000000000000000000000000000fee13a103a10d593b9ae06b3e05f2e7e1c0000000000000000000000000000000000000000000000000009536c7089100000000000000000000000000000000000000000000000000000000000000000600000000000000000000000000418fa3488d7e13f0c06ac5f8485d306b5748f4f000000000000000000000000090ec5e568dfab3242ac3f11cd497ec731e4a5cc0000000000000000000000000000000000000000000000000e92596fd629000000000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000090ec5e568dfab3242ac3f11cd497ec731e4a5cc00000000000000000000000000000000000000000000000000000000000000000c").into())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please explain what this transaction is in code comments

}
}

pub trait ChainDriverExt {
Expand Down
50 changes: 49 additions & 1 deletion crates/op-rbuilder/src/tests/smoke.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
use crate::tests::{LocalInstance, TransactionBuilderExt};
use crate::{
args::OpRbuilderArgs,
tests::{LocalInstance, TransactionBuilderExt},
};
use alloy_primitives::TxHash;

use core::{
sync::atomic::{AtomicBool, Ordering},
time::Duration,
Expand Down Expand Up @@ -189,3 +193,47 @@ async fn test_no_tx_pool(rbuilder: LocalInstance) -> eyre::Result<()> {

Ok(())
}

#[rb_test(args = OpRbuilderArgs {
max_gas_per_txn: Some(21000),
..Default::default()
})]
async fn chain_produces_big_txs(rbuilder: LocalInstance) -> eyre::Result<()> {
let driver = rbuilder.driver().await?;

#[cfg(target_os = "linux")]
let driver = driver
.with_validation_node(crate::tests::ExternalNode::reth().await?)
.await?;

let count = rand::random_range(1..8);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you creating a random number of transactions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I just used this and removed the outer loop...I can add that back again

let count = rand::random_range(1..8);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a test that sends a tx using an amount of gas below the threshold and one above and then checking that the tx that uses too much gas is excluded should be sufficient

let mut tx_hashes = HashSet::<TxHash>::default();

for _ in 0..count {
// insert txns with gas = 210_000
let tx = driver
.create_transaction()
.random_big_transaction()
.send()
.await
.expect("Failed to send transaction");

tx_hashes.insert(*tx.tx_hash());
}

// insert 1 valid txn with gas=21_000
let _ = driver
.create_transaction()
.random_valid_transfer()
.send()
.await
.expect("Failed to send transaction");

let block = driver.build_new_block_with_current_timestamp(None).await?;

let txs = block.transactions;

assert!(txs.len() == 4);

Ok(())
}
Loading