Skip to content

fix: restrict txpool fallback to dev mode only #155

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
tac0turtle merged 6 commits into from
Mar 4, 2026
Merged

fix: restrict txpool fallback to dev mode only #155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
18bda4c
fix: restrict txpool fallback to dev mode only
randygrok Mar 4, 2026
75746fa
fix: restrict txpool fallback to dev mode only
randygrok Mar 4, 2026
f22942a
test: add e2e test for dev mode txpool fallback
randygrok Mar 4, 2026
263e476
fix: resolve CI lint failures (fmt and clippy)
randygrok Mar 4, 2026
b89ab07
refactor: always use txpool in dev mode without empty check
randygrok Mar 4, 2026
52e3553
test: disable dev mode in e2e tests that use Engine API transactions
randygrok Mar 4, 2026
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
20 changes: 4 additions & 16 deletions crates/node/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ impl EvolvePayloadBuilderConfig {
}
}
}

Ok(config)
}

Expand Down Expand Up @@ -400,10 +401,7 @@ mod tests {
mint_admin: Some(address!("00000000000000000000000000000000000000aa")),
base_fee_redirect_activation_height: Some(0),
mint_precompile_activation_height: Some(0),
contract_size_limit: None,
contract_size_limit_activation_height: None,
deploy_allowlist: Vec::new(),
deploy_allowlist_activation_height: None,
..Default::default()
};
assert!(config_with_sink.validate().is_ok());
}
Expand Down Expand Up @@ -468,14 +466,9 @@ mod tests {
allowlist.push(addr);
}
let config = EvolvePayloadBuilderConfig {
base_fee_sink: None,
mint_admin: None,
base_fee_redirect_activation_height: None,
mint_precompile_activation_height: None,
contract_size_limit: None,
contract_size_limit_activation_height: None,
deploy_allowlist: allowlist,
deploy_allowlist_activation_height: Some(0),
..Default::default()
};

assert!(matches!(
Expand All @@ -489,13 +482,8 @@ mod tests {
let sink = address!("0000000000000000000000000000000000000003");
let mut config = EvolvePayloadBuilderConfig {
base_fee_sink: Some(sink),
mint_admin: None,
base_fee_redirect_activation_height: Some(5),
mint_precompile_activation_height: None,
contract_size_limit: None,
contract_size_limit_activation_height: None,
deploy_allowlist: Vec::new(),
deploy_allowlist_activation_height: None,
..Default::default()
};

assert_eq!(config.base_fee_sink_for_block(4), None);
Expand Down
12 changes: 8 additions & 4 deletions crates/node/src/payload_service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ where
pub(crate) evolve_builder: Arc<EvolvePayloadBuilder<Client>>,
pub(crate) config: EvolvePayloadBuilderConfig,
pub(crate) pool: Pool,
pub(crate) dev_mode: bool,
}

impl<Node, Pool> PayloadBuilderBuilder<Node, Pool, EvolveEvmConfig> for EvolvePayloadBuilderBuilder
Expand Down Expand Up @@ -117,6 +118,7 @@ where
evolve_builder,
config,
pool,
dev_mode: ctx.is_dev(),
})
}
}
Expand Down Expand Up @@ -180,9 +182,9 @@ where
}
}

// Use transactions from Engine API attributes if provided, otherwise pull from the pool
// (e.g. in --dev mode where LocalMiner sends empty attributes).
let transactions = if attributes.transactions.is_empty() {
// In dev mode, pull pending transactions from the txpool.
// In production, transactions come exclusively from Engine API attributes.
let transactions = if self.dev_mode {
let pool_txs: Vec<TransactionSigned> = self
.pool
.pending_transactions()
Expand All @@ -192,7 +194,7 @@ where
if !pool_txs.is_empty() {
info!(
pool_tx_count = pool_txs.len(),
"pulling transactions from pool"
"pulling transactions from pool (dev mode)"
);
}
pool_txs
Expand Down Expand Up @@ -382,6 +384,7 @@ mod tests {
evolve_builder,
config,
pool: NoopTransactionPool::<EvPooledTransaction>::new(),
dev_mode: false,
};
Comment on lines +387 to 388
Copy link

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add a unit test that exercises dev_mode: true transaction sourcing.

Both updated unit tests pin dev_mode: false; the new branch in try_build (Line 187) is not unit-covered here. Please add a focused unit test asserting dev mode pulls from pool and production mode uses Engine API attributes.

As per coding guidelines: "Write unit tests for individual components in Rust workspace."

Also applies to: 478-479

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@crates/node/src/payload_service.rs` around lines 387 - 388, Add a unit test
in crates/node/src/payload_service.rs that exercises the new branch in try_build
by constructing the relevant struct with dev_mode: true and injecting a mocked
pool that returns a known transaction source, then call try_build and assert the
transaction sourcing came from the pool; also add a complementary test with
dev_mode: false that injects mocked Engine API attributes and assert try_build
used those attributes. Target the symbols try_build and the struct/constructor
that accepts dev_mode so the test explicitly toggles dev_mode, injects test
doubles for the pool and Engine API, and asserts the correct branch behavior
(pool when dev_mode is true, Engine API attributes when false).


let rpc_attrs = RpcPayloadAttributes {
Expand Down Expand Up @@ -472,6 +475,7 @@ mod tests {
evolve_builder,
config,
pool: NoopTransactionPool::<EvPooledTransaction>::new(),
dev_mode: false,
};

let rpc_attrs = RpcPayloadAttributes {
Expand Down
85 changes: 76 additions & 9 deletions crates/tests/src/e2e_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ async fn test_e2e_base_fee_sink_receives_base_fee() -> Result<()> {
let mut setup = Setup::<EvolveEngineTypes>::default()
.with_chain_spec(chain_spec)
.with_network(NetworkSetup::single_node())
.with_dev_mode(true)
.with_dev_mode(false)
.with_tree_config(e2e_test_tree_config());

let mut env = Environment::<EvolveEngineTypes>::default();
Expand Down Expand Up @@ -408,7 +408,7 @@ async fn test_e2e_sponsored_evnode_transaction() -> Result<()> {
let mut setup = Setup::<EvolveEngineTypes>::default()
.with_chain_spec(chain_spec)
.with_network(NetworkSetup::single_node())
.with_dev_mode(true)
.with_dev_mode(false)
.with_tree_config(e2e_test_tree_config());

let mut env = Environment::<EvolveEngineTypes>::default();
Expand Down Expand Up @@ -614,7 +614,7 @@ async fn test_e2e_invalid_sponsor_signature_skipped() -> Result<()> {
let mut setup = Setup::<EvolveEngineTypes>::default()
.with_chain_spec(chain_spec)
.with_network(NetworkSetup::single_node())
.with_dev_mode(true)
.with_dev_mode(false)
.with_tree_config(e2e_test_tree_config());

let mut env = Environment::<EvolveEngineTypes>::default();
Expand Down Expand Up @@ -739,7 +739,7 @@ async fn test_e2e_empty_calls_skipped() -> Result<()> {
let mut setup = Setup::<EvolveEngineTypes>::default()
.with_chain_spec(chain_spec)
.with_network(NetworkSetup::single_node())
.with_dev_mode(true)
.with_dev_mode(false)
.with_tree_config(e2e_test_tree_config());

let mut env = Environment::<EvolveEngineTypes>::default();
Expand Down Expand Up @@ -852,7 +852,7 @@ async fn test_e2e_sponsor_insufficient_max_fee_skipped() -> Result<()> {
let mut setup = Setup::<EvolveEngineTypes>::default()
.with_chain_spec(chain_spec)
.with_network(NetworkSetup::single_node())
.with_dev_mode(true)
.with_dev_mode(false)
.with_tree_config(e2e_test_tree_config());

let mut env = Environment::<EvolveEngineTypes>::default();
Expand Down Expand Up @@ -1000,7 +1000,7 @@ async fn test_e2e_nonce_bumped_on_create_batch_failure() -> Result<()> {
let mut setup = Setup::<EvolveEngineTypes>::default()
.with_chain_spec(chain_spec)
.with_network(NetworkSetup::single_node())
.with_dev_mode(true)
.with_dev_mode(false)
.with_tree_config(e2e_test_tree_config());

let mut env = Environment::<EvolveEngineTypes>::default();
Expand Down Expand Up @@ -1239,7 +1239,7 @@ async fn test_e2e_mint_and_burn_to_new_wallet() -> Result<()> {
let mut setup = Setup::<EvolveEngineTypes>::default()
.with_chain_spec(chain_spec.clone())
.with_network(NetworkSetup::single_node())
.with_dev_mode(true)
.with_dev_mode(false)
.with_tree_config(e2e_test_tree_config());

let mut env = Environment::<EvolveEngineTypes>::default();
Expand Down Expand Up @@ -1661,7 +1661,7 @@ async fn test_e2e_mint_precompile_via_contract() -> Result<()> {
let mut setup = Setup::<EvolveEngineTypes>::default()
.with_chain_spec(chain_spec.clone())
.with_network(NetworkSetup::single_node())
.with_dev_mode(true)
.with_dev_mode(false)
.with_tree_config(e2e_test_tree_config());

let mut env = Environment::<EvolveEngineTypes>::default();
Expand Down Expand Up @@ -1892,7 +1892,7 @@ async fn test_e2e_deploy_allowlist_blocks_unauthorized_deploys() -> Result<()> {
let mut setup = Setup::<EvolveEngineTypes>::default()
.with_chain_spec(chain_spec)
.with_network(NetworkSetup::single_node())
.with_dev_mode(true)
.with_dev_mode(false)
.with_tree_config(e2e_test_tree_config());

let mut env = Environment::<EvolveEngineTypes>::default();
Expand Down Expand Up @@ -2023,3 +2023,70 @@ async fn test_e2e_deploy_allowlist_blocks_unauthorized_deploys() -> Result<()> {

Ok(())
}

/// Tests that dev mode correctly enables the txpool fallback.
///
/// When running with `--dev` flag, the payload builder pulls pending
/// transactions from the txpool instead of relying on Engine API attributes.
/// This validates the full flow:
/// `--dev` flag → `ctx.is_dev()` → `dev_mode` on payload builder → txpool
#[tokio::test(flavor = "multi_thread")]
async fn test_e2e_dev_mode_txpool_fallback() -> Result<()> {
reth_tracing::init_test_tracing();

let chain_spec = create_test_chain_spec();
let chain_id = chain_spec.chain().id();

let mut setup = Setup::<EvolveEngineTypes>::default()
.with_chain_spec(chain_spec)
.with_network(NetworkSetup::single_node())
.with_dev_mode(true)
.with_tree_config(e2e_test_tree_config());

let mut env = Environment::<EvolveEngineTypes>::default();
setup.apply::<EvolveNode>(&mut env).await?;

let parent_block = env.node_clients[0]
.get_block_by_number(BlockNumberOrTag::Latest)
.await?
.expect("parent block should exist");
let mut parent_hash = parent_block.header.hash;
let mut parent_timestamp = parent_block.header.inner.timestamp;
let mut parent_number = parent_block.header.inner.number;

// Create a signed transaction and send it to the txpool
let wallets = Wallet::new(1).with_chain_id(chain_id).wallet_gen();
let sender = wallets.into_iter().next().unwrap();
let raw_tx = TransactionTestContext::transfer_tx_bytes(chain_id, sender).await;

EthApiClient::<TransactionRequest, Transaction, Block, Receipt, Header, Bytes>::send_raw_transaction(
&env.node_clients[0].rpc,
raw_tx,
)
.await?;

// Build a block with empty transactions via Engine API.
// In dev mode, the payload builder pulls from the txpool.
let payload_envelope = build_block_with_transactions(
&mut env,
&mut parent_hash,
&mut parent_number,
&mut parent_timestamp,
None,
vec![],
Address::random(),
)
.await?;

let block_txs = &payload_envelope
.execution_payload
.payload_inner
.payload_inner
.transactions;
assert!(
!block_txs.is_empty(),
"dev mode should pull transaction from txpool when attributes are empty"
);

Ok(())
}
Comment on lines +2034 to +2092
Copy link

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add a non-dev regression test for the safety-critical branch.

This test validates the dev-mode path, but it does not verify the production behavior (with_dev_mode(false)) where txpool fallback must stay disabled. Without that companion assertion, the core safety guarantee from this PR can regress silently. See Line 2043 and Line 2068-Line 2089.

Proposed test addition 
+#[tokio::test(flavor = "multi_thread")]
+async fn test_e2e_non_dev_mode_does_not_txpool_fallback() -> Result<()> {
+    reth_tracing::init_test_tracing();
+
+    let chain_spec = create_test_chain_spec();
+    let chain_id = chain_spec.chain().id();
+
+    let mut setup = Setup::<EvolveEngineTypes>::default()
+        .with_chain_spec(chain_spec)
+        .with_network(NetworkSetup::single_node())
+        .with_dev_mode(false)
+        .with_tree_config(e2e_test_tree_config());
+
+    let mut env = Environment::<EvolveEngineTypes>::default();
+    setup.apply::<EvolveNode>(&mut env).await?;
+
+    let parent_block = env.node_clients[0]
+        .get_block_by_number(BlockNumberOrTag::Latest)
+        .await?
+        .expect("parent block should exist");
+    let mut parent_hash = parent_block.header.hash;
+    let mut parent_timestamp = parent_block.header.inner.timestamp;
+    let mut parent_number = parent_block.header.inner.number;
+
+    let wallets = Wallet::new(1).with_chain_id(chain_id).wallet_gen();
+    let sender = wallets.into_iter().next().expect("wallet exists");
+    let raw_tx = TransactionTestContext::transfer_tx_bytes(chain_id, sender).await;
+    EthApiClient::<TransactionRequest, Transaction, Block, Receipt, Header, Bytes>::send_raw_transaction(
+        &env.node_clients[0].rpc,
+        raw_tx,
+    )
+    .await?;
+
+    let payload_envelope = build_block_with_transactions(
+        &mut env,
+        &mut parent_hash,
+        &mut parent_number,
+        &mut parent_timestamp,
+        None,
+        vec![],
+        Address::ZERO,
+    )
+    .await?;
+
+    let block_txs = &payload_envelope
+        .execution_payload
+        .payload_inner
+        .payload_inner
+        .transactions;
+    assert!(
+        block_txs.is_empty(),
+        "non-dev mode must not pull from txpool when attributes are empty"
+    );
+
+    Ok(())
+}

Based on learnings: Transactions should bypass the mempool and be submitted directly via Engine API payload attributes.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async fn test_e2e_dev_mode_txpool_fallback() -> Result<()> {
reth_tracing::init_test_tracing();
let chain_spec = create_test_chain_spec();
let chain_id = chain_spec.chain().id();
let mut setup = Setup::<EvolveEngineTypes>::default()
.with_chain_spec(chain_spec)
.with_network(NetworkSetup::single_node())
.with_dev_mode(true)
.with_tree_config(e2e_test_tree_config());
let mut env = Environment::<EvolveEngineTypes>::default();
setup.apply::<EvolveNode>(&mut env).await?;
let parent_block = env.node_clients[0]
.get_block_by_number(BlockNumberOrTag::Latest)
.await?
.expect("parent block should exist");
let mut parent_hash = parent_block.header.hash;
let mut parent_timestamp = parent_block.header.inner.timestamp;
let mut parent_number = parent_block.header.inner.number;
// Create a signed transaction and send it to the txpool
let wallets = Wallet::new(1).with_chain_id(chain_id).wallet_gen();
let sender = wallets.into_iter().next().unwrap();
let raw_tx = TransactionTestContext::transfer_tx_bytes(chain_id, sender).await;
EthApiClient::<TransactionRequest, Transaction, Block, Receipt, Header, Bytes>::send_raw_transaction(
&env.node_clients[0].rpc,
raw_tx,
)
.await?;
// Build a block with empty transactions via Engine API.
// In dev mode, the payload builder pulls from the txpool.
let payload_envelope = build_block_with_transactions(
&mut env,
&mut parent_hash,
&mut parent_number,
&mut parent_timestamp,
None,
vec![],
Address::random(),
)
.await?;
let block_txs = &payload_envelope
.execution_payload
.payload_inner
.payload_inner
.transactions;
assert!(
!block_txs.is_empty(),
"dev mode should pull transaction from txpool when attributes are empty"
);
Ok(())
}
async fn test_e2e_non_dev_mode_does_not_txpool_fallback() -> Result<()> {
reth_tracing::init_test_tracing();
let chain_spec = create_test_chain_spec();
let chain_id = chain_spec.chain().id();
let mut setup = Setup::<EvolveEngineTypes>::default()
.with_chain_spec(chain_spec)
.with_network(NetworkSetup::single_node())
.with_dev_mode(false)
.with_tree_config(e2e_test_tree_config());
let mut env = Environment::<EvolveEngineTypes>::default();
setup.apply::<EvolveNode>(&mut env).await?;
let parent_block = env.node_clients[0]
.get_block_by_number(BlockNumberOrTag::Latest)
.await?
.expect("parent block should exist");
let mut parent_hash = parent_block.header.hash;
let mut parent_timestamp = parent_block.header.inner.timestamp;
let mut parent_number = parent_block.header.inner.number;
let wallets = Wallet::new(1).with_chain_id(chain_id).wallet_gen();
let sender = wallets.into_iter().next().expect("wallet exists");
let raw_tx = TransactionTestContext::transfer_tx_bytes(chain_id, sender).await;
EthApiClient::<TransactionRequest, Transaction, Block, Receipt, Header, Bytes>::send_raw_transaction(
&env.node_clients[0].rpc,
raw_tx,
)
.await?;
let payload_envelope = build_block_with_transactions(
&mut env,
&mut parent_hash,
&mut parent_number,
&mut parent_timestamp,
None,
vec![],
Address::ZERO,
)
.await?;
let block_txs = &payload_envelope
.execution_payload
.payload_inner
.payload_inner
.transactions;
assert!(
block_txs.is_empty(),
"non-dev mode must not pull from txpool when attributes are empty"
);
Ok(())
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@crates/tests/src/e2e_tests.rs` around lines 2034 - 2092, Add a non-dev
counterpart of test_e2e_dev_mode_txpool_fallback that uses the same setup but
sets with_dev_mode(false) (e.g., name it test_e2e_non_dev_txpool_no_fallback),
send a signed transaction into the txpool exactly as in
test_e2e_dev_mode_txpool_fallback, call build_block_with_transactions the same
way, then assert that the resulting payload's transactions
(payload_envelope.execution_payload.payload_inner.payload_inner.transactions)
are empty to verify txpool fallback is disabled in production mode; reuse
Setup::<EvolveEngineTypes>, Wallet::new(...).with_chain_id(...).wallet_gen(),
EthApiClient::send_raw_transaction and build_block_with_transactions to mirror
the dev test flow.

2 changes: 1 addition & 1 deletion crates/tests/src/test_deploy_allowlist.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ async fn test_e2e_deploy_allowlist_permits_create2_via_factory() -> Result<()> {
let mut setup = Setup::<EvolveEngineTypes>::default()
.with_chain_spec(chain_spec)
.with_network(NetworkSetup::single_node())
.with_dev_mode(true)
.with_dev_mode(false)
.with_tree_config(e2e_test_tree_config());

let mut env = Environment::<EvolveEngineTypes>::default();
Expand Down
4 changes: 2 additions & 2 deletions crates/tests/src/test_evolve_engine_api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ async fn test_e2e_engine_api_fork_choice_with_transactions() -> Result<()> {
let mut setup = Setup::<EvolveEngineTypes>::default()
.with_chain_spec(chain_spec)
.with_network(NetworkSetup::single_node())
.with_dev_mode(true)
.with_dev_mode(false)
.with_tree_config(e2e_test_tree_config());

let mut env = Environment::<EvolveEngineTypes>::default();
Expand Down Expand Up @@ -250,7 +250,7 @@ async fn test_e2e_engine_api_gas_limit_handling() -> Result<()> {
let mut setup = Setup::<EvolveEngineTypes>::default()
.with_chain_spec(chain_spec)
.with_network(NetworkSetup::single_node())
.with_dev_mode(true)
.with_dev_mode(false)
.with_tree_config(e2e_test_tree_config());

let mut env = Environment::<EvolveEngineTypes>::default();
Expand Down