Skip to content

Commit 8f15b79

Browse files
author
Solar Mithril
committed
Added drop impl to clean up after tests
Changed revert tests a bit fmt Add txpool test Fix DA config, set it via miner Extend tests Add op-alloy-flz dependency and update related configurations - Added `op-alloy-flz` as a dependency in `Cargo.toml` and `Cargo.lock`. - Configured `op-alloy-flz` to be part of the workspace in `op-rbuilder`'s `Cargo.toml`. - Updated the `payload_builder_vanilla.rs` to utilize `op-alloy-flz` for transaction size estimation. - Enhanced test framework to include new data availability tests ensuring block size limits are respected. Add max data availability transaction and block size configuration - Introduced `max_da_tx_size` and `max_da_block_size` fields in `TestHarnessBuilder` and `OpRbuilderConfig`. - Added builder methods `with_max_da_tx_size` and `with_max_da_block_size` for setting these values. - Implemented a new test to ensure transaction size limits are respected in data availability scenarios. - Updated test module to include the new data availability test. Add cumulative_da_bytes_used accur Add da config Use correct DA transaction compression
1 parent 92b8f6a commit 8f15b79

File tree

13 files changed

+262
-17
lines changed

13 files changed

+262
-17
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ op-alloy-rpc-types-engine = { version = "0.16.0", default-features = false }
132132
op-alloy-rpc-jsonrpsee = { version = "0.16.0", default-features = false }
133133
op-alloy-network = { version = "0.16.0", default-features = false }
134134
op-alloy-consensus = { version = "0.16.0", default-features = false }
135+
op-alloy-flz = { version = "0.13.0", default-features = false }
135136

136137
async-trait = { version = "0.1.83" }
137138
clap = { version = "4.4.3", features = ["derive", "env"] }

crates/op-rbuilder/Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ op-alloy-consensus.workspace = true
6464
op-alloy-rpc-types-engine.workspace = true
6565
op-alloy-rpc-types.workspace = true
6666
op-alloy-network.workspace = true
67+
op-alloy-flz.workspace = true
6768

6869
revm.workspace = true
6970
op-revm.workspace = true
@@ -110,6 +111,9 @@ tikv-jemallocator = { version = "0.6", optional = true }
110111
vergen = { workspace = true, features = ["build", "cargo", "emit_and_set"] }
111112
vergen-git2.workspace = true
112113

114+
[dev-dependencies]
115+
alloy-provider = {workspace = true, default-features = true, features = ["txpool-api"]}
116+
113117
[features]
114118
default = ["jemalloc"]
115119

crates/op-rbuilder/src/args/op.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,8 @@
55
//! clap [Args](clap::Args) for optimism rollup configuration
66
use std::path::PathBuf;
77

8-
use reth_optimism_node::args::RollupArgs;
9-
108
use crate::tx_signer::Signer;
9+
use reth_optimism_node::args::RollupArgs;
1110

1211
/// Parameters for rollup configuration
1312
#[derive(Debug, Clone, Default, PartialEq, Eq, clap::Args)]

crates/op-rbuilder/src/main.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use reth_optimism_node::{
55
node::{OpAddOnsBuilder, OpPoolBuilder},
66
OpNode,
77
};
8+
use reth_optimism_payload_builder::config::OpDAConfig;
89
use reth_transaction_pool::TransactionPool;
910

1011
/// CLI argument parsing.
@@ -68,7 +69,7 @@ where
6869
cli.run(|builder, builder_args| async move {
6970
let builder_config = BuilderConfig::<B::Config>::try_from(builder_args.clone())
7071
.expect("Failed to convert rollup args to builder config");
71-
72+
let da_config = OpDAConfig::default();
7273
let rollup_args = builder_args.rollup_args;
7374
let op_node = OpNode::new(rollup_args.clone());
7475
let handle = builder
@@ -95,6 +96,7 @@ where
9596
OpAddOnsBuilder::default()
9697
.with_sequencer(rollup_args.sequencer.clone())
9798
.with_enable_tx_conditional(rollup_args.enable_tx_conditional)
99+
.with_da_config(da_config)
98100
.build(),
99101
)
100102
.extend_rpc_modules(move |ctx| {

crates/op-rbuilder/src/primitives/reth/execution.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
//! Heavily influenced by [reth](https://github.com/paradigmxyz/reth/blob/1e965caf5fa176f244a31c0d2662ba1b590938db/crates/optimism/payload/src/builder.rs#L570)
2-
use alloy_consensus::Transaction;
3-
use alloy_primitives::{private::alloy_rlp::Encodable, Address, U256};
2+
use alloy_primitives::{Address, U256};
43
use core::fmt::Debug;
54
use reth_optimism_primitives::{OpReceipt, OpTransactionSigned};
65

@@ -44,21 +43,22 @@ impl<T: Debug + Default> ExecutionInfo<T> {
4443
/// maximum allowed DA limit per block.
4544
pub fn is_tx_over_limits(
4645
&self,
47-
tx: &OpTransactionSigned,
46+
tx_da_size: u64,
4847
block_gas_limit: u64,
4948
tx_data_limit: Option<u64>,
5049
block_data_limit: Option<u64>,
50+
tx_gas_limit: u64,
5151
) -> bool {
52-
if tx_data_limit.is_some_and(|da_limit| tx.length() as u64 > da_limit) {
52+
if tx_data_limit.is_some_and(|da_limit| tx_da_size > da_limit) {
5353
return true;
5454
}
5555

5656
if block_data_limit
57-
.is_some_and(|da_limit| self.cumulative_da_bytes_used + (tx.length() as u64) > da_limit)
57+
.is_some_and(|da_limit| self.cumulative_da_bytes_used + tx_da_size > da_limit)
5858
{
5959
return true;
6060
}
6161

62-
self.cumulative_gas_used + tx.gas_limit() > block_gas_limit
62+
self.cumulative_gas_used + tx_gas_limit > block_gas_limit
6363
}
6464
}

crates/op-rbuilder/src/tests/framework/blocks.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,4 +372,12 @@ impl BlockGenerated {
372372
pub fn includes(&self, tx_hash: B256) -> bool {
373373
self.block.transactions.hashes().any(|hash| hash == tx_hash)
374374
}
375+
376+
pub fn includes_vec(&self, tx_hashes: Vec<B256>) -> bool {
377+
tx_hashes.iter().all(|hash| self.includes(*hash))
378+
}
379+
380+
pub fn not_includes_vec(&self, tx_hashes: Vec<B256>) -> bool {
381+
tx_hashes.iter().all(|hash| self.not_includes(*hash))
382+
}
375383
}

crates/op-rbuilder/src/tests/framework/harness.rs

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ pub struct TestHarnessBuilder {
2525
flashblocks_ws_url: Option<String>,
2626
chain_block_time: Option<u64>,
2727
flashbots_block_time: Option<u64>,
28+
namespaces: Option<String>,
29+
extra_params: Option<String>,
2830
}
2931

3032
impl TestHarnessBuilder {
@@ -35,6 +37,8 @@ impl TestHarnessBuilder {
3537
flashblocks_ws_url: None,
3638
chain_block_time: None,
3739
flashbots_block_time: None,
40+
namespaces: None,
41+
extra_params: None,
3842
}
3943
}
4044

@@ -58,6 +62,16 @@ impl TestHarnessBuilder {
5862
self
5963
}
6064

65+
pub fn with_namespaces(mut self, namespaces: &str) -> Self {
66+
self.namespaces = Some(namespaces.to_string());
67+
self
68+
}
69+
70+
pub fn with_extra_params(mut self, extra_params: &str) -> Self {
71+
self.extra_params = Some(extra_params.to_string());
72+
self
73+
}
74+
6175
pub async fn build(self) -> eyre::Result<TestHarness> {
6276
let mut framework = IntegrationFramework::new(&self.name).unwrap();
6377

@@ -69,7 +83,7 @@ impl TestHarnessBuilder {
6983
std::fs::write(&genesis_path, genesis)?;
7084

7185
// create the builder
72-
let builder_data_dir = std::env::temp_dir().join(Uuid::new_v4().to_string());
86+
let builder_data_dir: PathBuf = std::env::temp_dir().join(Uuid::new_v4().to_string());
7387
let builder_auth_rpc_port = get_available_port();
7488
let builder_http_port = get_available_port();
7589
let mut op_rbuilder_config = OpRbuilderConfig::new()
@@ -79,8 +93,9 @@ impl TestHarnessBuilder {
7993
.network_port(get_available_port())
8094
.http_port(builder_http_port)
8195
.with_builder_private_key(BUILDER_PRIVATE_KEY)
82-
.with_revert_protection(self.use_revert_protection);
83-
96+
.with_revert_protection(self.use_revert_protection)
97+
.with_namespaces(self.namespaces)
98+
.with_extra_params(self.extra_params);
8499
if let Some(flashblocks_ws_url) = self.flashblocks_ws_url {
85100
op_rbuilder_config = op_rbuilder_config.with_flashblocks_ws_url(&flashblocks_ws_url);
86101
}
@@ -113,7 +128,7 @@ impl TestHarnessBuilder {
113128
let builder_log_path = builder.log_path.clone();
114129

115130
Ok(TestHarness {
116-
_framework: framework,
131+
framework: framework,
117132
builder_auth_rpc_port,
118133
builder_http_port,
119134
validator_auth_rpc_port,
@@ -123,7 +138,7 @@ impl TestHarnessBuilder {
123138
}
124139

125140
pub struct TestHarness {
126-
_framework: IntegrationFramework,
141+
framework: IntegrationFramework,
127142
builder_auth_rpc_port: u16,
128143
builder_http_port: u16,
129144
validator_auth_rpc_port: u16,
@@ -237,6 +252,16 @@ impl TestHarness {
237252
}
238253
}
239254

255+
impl Drop for TestHarness {
256+
fn drop(&mut self) {
257+
for service in &mut self.framework.services {
258+
let res = service.stop();
259+
if let Err(e) = res {
260+
println!("Failed to stop service: {}", e);
261+
}
262+
}
263+
}
264+
}
240265
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
241266
pub enum TransactionStatus {
242267
NotFound,

crates/op-rbuilder/src/tests/framework/op.rs

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ pub struct OpRbuilderConfig {
2727
chain_block_time: Option<u64>,
2828
flashbots_block_time: Option<u64>,
2929
with_revert_protection: Option<bool>,
30+
namespaces: Option<String>,
31+
extra_params: Option<String>,
3032
}
3133

3234
impl OpRbuilderConfig {
@@ -83,6 +85,16 @@ impl OpRbuilderConfig {
8385
self.flashbots_block_time = Some(time);
8486
self
8587
}
88+
89+
pub fn with_namespaces(mut self, namespaces: Option<String>) -> Self {
90+
self.namespaces = namespaces;
91+
self
92+
}
93+
94+
pub fn with_extra_params(mut self, extra_params: Option<String>) -> Self {
95+
self.extra_params = extra_params;
96+
self
97+
}
8698
}
8799

88100
impl Service for OpRbuilderConfig {
@@ -137,9 +149,7 @@ impl Service for OpRbuilderConfig {
137149
if let Some(http_port) = self.http_port {
138150
cmd.arg("--http")
139151
.arg("--http.port")
140-
.arg(http_port.to_string())
141-
.arg("--http.api")
142-
.arg("eth,web3,txpool");
152+
.arg(http_port.to_string());
143153
}
144154

145155
if let Some(flashblocks_ws_url) = &self.flashblocks_ws_url {
@@ -159,6 +169,14 @@ impl Service for OpRbuilderConfig {
159169
.arg(flashbots_block_time.to_string());
160170
}
161171

172+
if let Some(namespaces) = &self.namespaces {
173+
cmd.arg("--http.api").arg(namespaces);
174+
}
175+
176+
if let Some(extra_params) = &self.extra_params {
177+
cmd.args(extra_params.split_ascii_whitespace());
178+
}
179+
162180
cmd
163181
}
164182

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
use crate::tests::framework::TestHarnessBuilder;
2+
use alloy_provider::Provider;
3+
4+
/// This test ensures that the transaction size limit is respected.
5+
/// We will set limit to 1 byte and see that the builder will not include any transactions.
6+
#[tokio::test]
7+
async fn data_availability_tx_size_limit() -> eyre::Result<()> {
8+
let harness = TestHarnessBuilder::new("data_availability_tx_size_limit")
9+
.with_namespaces("admin,eth,miner")
10+
.build()
11+
.await?;
12+
13+
let mut generator = harness.block_generator().await?;
14+
15+
// Set (max_tx_da_size, max_block_da_size), with this case block won't fit any transaction
16+
let call = harness
17+
.provider()?
18+
.raw_request::<(i32, i32), bool>("miner_setMaxDASize".into(), (1, 0))
19+
.await?;
20+
assert!(call, "miner_setMaxDASize should be executed successfully");
21+
22+
let unfit_tx = harness
23+
.create_transaction()
24+
.with_max_priority_fee_per_gas(50)
25+
.send()
26+
.await?;
27+
let block = generator.generate_block().await?;
28+
// tx should not be included because we set the tx_size_limit to 1
29+
assert!(
30+
block.not_includes(*unfit_tx.tx_hash()),
31+
"transaction should not be included in the block"
32+
);
33+
34+
Ok(())
35+
}
36+
37+
/// This test ensures that the block size limit is respected.
38+
/// We will set limit to 1 byte and see that the builder will not include any transactions.
39+
#[tokio::test]
40+
async fn data_availability_block_size_limit() -> eyre::Result<()> {
41+
let harness = TestHarnessBuilder::new("data_availability_block_size_limit")
42+
.with_namespaces("admin,eth,miner")
43+
.build()
44+
.await?;
45+
46+
let mut generator = harness.block_generator().await?;
47+
48+
// Set block da size to be small, so it won't include tx
49+
let call = harness
50+
.provider()?
51+
.raw_request::<(i32, i32), bool>("miner_setMaxDASize".into(), (0, 1))
52+
.await?;
53+
assert!(call, "miner_setMaxDASize should be executed successfully");
54+
55+
let unfit_tx = harness.send_valid_transaction().await?;
56+
let block = generator.generate_block().await?;
57+
// tx should not be included because we set the tx_size_limit to 1
58+
assert!(
59+
block.not_includes(*unfit_tx.tx_hash()),
60+
"transaction should not be included in the block"
61+
);
62+
63+
Ok(())
64+
}
65+
66+
/// This test ensures that block will fill up to the limit.
67+
/// Size of each transaction is 100000000
68+
/// We will set limit to 3 txs and see that the builder will include 3 transactions.
69+
/// We should not forget about builder transaction so we will spawn only 2 regular txs.
70+
#[tokio::test]
71+
async fn data_availability_block_fill() -> eyre::Result<()> {
72+
let harness = TestHarnessBuilder::new("data_availability_block_fill")
73+
.with_namespaces("admin,eth,miner")
74+
.build()
75+
.await?;
76+
77+
let mut generator = harness.block_generator().await?;
78+
79+
// Set block big enough so it could fit 3 transactions without tx size limit
80+
let call = harness
81+
.provider()?
82+
.raw_request::<(i32, i32), bool>("miner_setMaxDASize".into(), (0, 100000000 * 3))
83+
.await?;
84+
assert!(call, "miner_setMaxDASize should be executed successfully");
85+
86+
// We already have 2 so we will spawn one more to check that it won't be included (it won't fit
87+
// because of builder tx)
88+
let fit_tx_1 = harness
89+
.create_transaction()
90+
.with_max_priority_fee_per_gas(50)
91+
.send()
92+
.await?;
93+
let fit_tx_2 = harness
94+
.create_transaction()
95+
.with_max_priority_fee_per_gas(50)
96+
.send()
97+
.await?;
98+
let unfit_tx_3 = harness.create_transaction().send().await?;
99+
100+
let block = generator.generate_block().await?;
101+
// Now the first 2 txs will fit into the block
102+
assert!(block.includes(*fit_tx_1.tx_hash()), "tx should be in block");
103+
assert!(block.includes(*fit_tx_2.tx_hash()), "tx should be in block");
104+
assert!(
105+
block.not_includes(*unfit_tx_3.tx_hash()),
106+
"unfit tx should not be in block"
107+
);
108+
assert!(
109+
harness.latest_block().await.transactions.len() == 4,
110+
"builder + deposit + 2 valid txs should be in the block"
111+
);
112+
113+
Ok(())
114+
}

0 commit comments

Comments
 (0)