Skip to content

Commit 5697532

Browse files
authored
feat(op-reth): gate payload builder on interop failsafe (#21448)
* feat(op-reth): gate payload builder on interop failsafe The payload builder never read the supervisor interop failsafe, so it kept sealing interop txs into unsafe blocks during a failsafe window until async pool eviction caught up, and it never excluded interop txs that bypassed the filter (e.g. private or locally-submitted txs). Thread a shared InteropFailsafe handle (Arc<AtomicBool>, mirroring SdmPostExecOptIn) from the txpool interop filter client (writer) through node setup into OpBuilderConfig (reader). When the failsafe is active, the build loop now excludes every interop tx, detected intrinsically via its CrossL2Inbox access list rather than the pool's interop-deadline marker, so unfiltered interop txs are caught too. * docs(op-reth): tighten interop failsafe comments * test(op-reth): assert interop tx re-includes after failsafe clears Thread one shared InteropFailsafe handle through three builds (off, on, off) so the test proves the gate is re-evaluated per build and the mark_invalid an excluded interop tx triggers does not persist once the failsafe clears.
1 parent 9c0f351 commit 5697532

6 files changed

Lines changed: 199 additions & 20 deletions

File tree

rust/op-reth/crates/node/src/node.rs

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@ use reth_optimism_rpc::{
5252
witness::{DebugExecutionWitnessApiServer, OpDebugPostExecApiServer, OpDebugWitnessApi},
5353
};
5454
use reth_optimism_storage::OpStorage;
55-
use reth_optimism_txpool::{OpPool, OpPooledTx, interop_filter::InteropFilterClient};
55+
use reth_optimism_txpool::{
56+
OpPool, OpPooledTx, interop::InteropFailsafe, interop_filter::InteropFilterClient,
57+
};
5658
use reth_primitives_traits::header::HeaderMut;
5759
use reth_provider::{CanonStateSubscriptions, providers::ProviderFactoryBuilder};
5860
use reth_rpc_api::{
@@ -207,6 +209,9 @@ pub struct OpNode {
207209
/// Local operator opt-in for SDM `PostExec` production. Shared (via Arc clones) between the
208210
/// payload builder and the `admin_setSdmPostExecOptIn` RPC handler.
209211
pub sdm_post_exec_opt_in: SdmPostExecOptIn,
212+
/// Interop failsafe gate, shared between the txpool's interop filter client (writer) and the
213+
/// payload builder (reader, to exclude interop txs while it is active).
214+
pub interop_failsafe: InteropFailsafe,
210215
}
211216

212217
/// A [`ComponentsBuilder`] with its generic arguments set to a stack of Optimism specific builders.
@@ -227,6 +232,7 @@ impl OpNode {
227232
da_config: OpDAConfig::default(),
228233
gas_limit_config: OpGasLimitConfig::default(),
229234
sdm_post_exec_opt_in: SdmPostExecOptIn::default(),
235+
interop_failsafe: InteropFailsafe::default(),
230236
}
231237
}
232238

@@ -259,13 +265,15 @@ impl OpNode {
259265
self.args.interop_http.clone(),
260266
self.args.interop_min_responses,
261267
self.args.interop_safety_level,
262-
),
268+
)
269+
.with_interop_failsafe(self.interop_failsafe.clone()),
263270
)
264271
.payload(BasicPayloadServiceBuilder::new(
265272
OpPayloadBuilder::new(compute_pending_block)
266273
.with_da_config(self.da_config.clone())
267274
.with_gas_limit_config(self.gas_limit_config.clone())
268275
.with_sdm_post_exec_opt_in(self.sdm_post_exec_opt_in.clone())
276+
.with_interop_failsafe(self.interop_failsafe.clone())
269277
.with_max_uncompressed_block_size(self.args.max_uncompressed_block_size),
270278
))
271279
.network(OpNetworkBuilder::new(disable_txpool_gossip, !discovery_v4))
@@ -1083,6 +1091,8 @@ pub struct OpPoolBuilder<T = crate::txpool::OpPooledTransaction> {
10831091
pub interop_min_responses: Option<usize>,
10841092
/// Safety level for interop filter validation.
10851093
pub interop_safety_level: SafetyLevel,
1094+
/// Shared interop failsafe gate, passed to the interop filter client this builder constructs.
1095+
pub interop_failsafe: InteropFailsafe,
10861096
/// Marker for the pooled transaction type.
10871097
_pd: core::marker::PhantomData<T>,
10881098
}
@@ -1095,6 +1105,7 @@ impl<T> Default for OpPoolBuilder<T> {
10951105
interop_endpoints: Vec::new(),
10961106
interop_min_responses: None,
10971107
interop_safety_level: SafetyLevel::CrossUnsafe,
1108+
interop_failsafe: InteropFailsafe::default(),
10981109
_pd: Default::default(),
10991110
}
11001111
}
@@ -1108,6 +1119,7 @@ impl<T> Clone for OpPoolBuilder<T> {
11081119
interop_endpoints: self.interop_endpoints.clone(),
11091120
interop_min_responses: self.interop_min_responses,
11101121
interop_safety_level: self.interop_safety_level,
1122+
interop_failsafe: self.interop_failsafe.clone(),
11111123
_pd: core::marker::PhantomData,
11121124
}
11131125
}
@@ -1143,6 +1155,12 @@ impl<T> OpPoolBuilder<T> {
11431155
self.interop_safety_level = interop_safety_level;
11441156
self
11451157
}
1158+
1159+
/// Shares the interop failsafe gate, written by the interop filter client this builder builds.
1160+
pub fn with_interop_failsafe(mut self, interop_failsafe: InteropFailsafe) -> Self {
1161+
self.interop_failsafe = interop_failsafe;
1162+
self
1163+
}
11461164
}
11471165

11481166
impl<Node, T, Evm> PoolBuilder<Node, Evm> for OpPoolBuilder<T>
@@ -1180,7 +1198,8 @@ where
11801198
self.interop_endpoints.clone(),
11811199
ctx.chain_spec().chain_id(),
11821200
)
1183-
.minimum_safety(self.interop_safety_level);
1201+
.minimum_safety(self.interop_safety_level)
1202+
.failsafe(self.interop_failsafe.clone());
11841203
if let Some(min) = self.interop_min_responses {
11851204
builder = builder.min_responses(min);
11861205
}
@@ -1302,6 +1321,8 @@ pub struct OpPayloadBuilder<Txs = ()> {
13021321
pub gas_limit_config: OpGasLimitConfig,
13031322
/// Operator opt-in flag for SDM `PostExec` production. Shared with the admin RPC.
13041323
pub sdm_post_exec_opt_in: SdmPostExecOptIn,
1324+
/// Interop failsafe gate, read by the builder to exclude interop txs while it is active.
1325+
pub interop_failsafe: InteropFailsafe,
13051326
/// Maximum cumulative uncompressed (EIP-2718 encoded) block size in bytes.
13061327
///
13071328
/// `None` disables the limit. See
@@ -1319,6 +1340,7 @@ impl OpPayloadBuilder {
13191340
da_config: OpDAConfig::default(),
13201341
gas_limit_config: OpGasLimitConfig::default(),
13211342
sdm_post_exec_opt_in: SdmPostExecOptIn::default(),
1343+
interop_failsafe: InteropFailsafe::default(),
13221344
max_uncompressed_block_size: None,
13231345
}
13241346
}
@@ -1350,6 +1372,13 @@ impl OpPayloadBuilder {
13501372
self.sdm_post_exec_opt_in = sdm_post_exec_opt_in;
13511373
self
13521374
}
1375+
1376+
/// Provide the shared interop failsafe gate read by the builder.
1377+
#[must_use]
1378+
pub fn with_interop_failsafe(mut self, interop_failsafe: InteropFailsafe) -> Self {
1379+
self.interop_failsafe = interop_failsafe;
1380+
self
1381+
}
13531382
}
13541383

13551384
impl<Txs> OpPayloadBuilder<Txs> {
@@ -1361,6 +1390,7 @@ impl<Txs> OpPayloadBuilder<Txs> {
13611390
da_config,
13621391
gas_limit_config,
13631392
sdm_post_exec_opt_in,
1393+
interop_failsafe,
13641394
max_uncompressed_block_size,
13651395
..
13661396
} = self;
@@ -1370,6 +1400,7 @@ impl<Txs> OpPayloadBuilder<Txs> {
13701400
da_config,
13711401
gas_limit_config,
13721402
sdm_post_exec_opt_in,
1403+
interop_failsafe,
13731404
max_uncompressed_block_size,
13741405
}
13751406
}
@@ -1421,6 +1452,7 @@ where
14211452
da_config: self.da_config.clone(),
14221453
gas_limit_config: self.gas_limit_config.clone(),
14231454
sdm_post_exec_opt_in: self.sdm_post_exec_opt_in.clone(),
1455+
interop_failsafe: self.interop_failsafe.clone(),
14241456
max_uncompressed_block_size: self.max_uncompressed_block_size,
14251457
},
14261458
)

rust/op-reth/crates/payload/src/builder.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ use reth_optimism_primitives::{L2_TO_L1_MESSAGE_PASSER_ADDRESS, OpTransaction};
3535
use reth_optimism_txpool::{
3636
OpPooledTx,
3737
estimated_da_size::DataAvailabilitySized,
38-
interop::{MaybeInteropTransaction, is_valid_interop},
38+
interop::{MaybeInteropTransaction, is_interop_tx, is_valid_interop},
3939
};
4040
use reth_payload_builder_primitives::PayloadBuilderError;
4141
use reth_payload_primitives::{BuildNextEnv, BuiltPayloadExecutedBlock};
@@ -978,6 +978,10 @@ where
978978
let tx_da_limit = self.builder_config.da_config.max_da_tx_size();
979979
let max_uncompressed_block_size = self.builder_config.max_uncompressed_block_size;
980980
let base_fee = builder.evm_mut().block().basefee();
981+
// Snapshot the interop failsafe once for this build. Gating here, rather than relying on
982+
// async pool eviction, excludes interop txs that bypassed the filter (e.g. private or
983+
// local txs) and avoids racing eviction to drain the pool.
984+
let interop_failsafe_active = self.builder_config.interop_failsafe.enabled();
981985

982986
while let Some(tx) = best_txs.next(()) {
983987
let interop = tx.interop_deadline();
@@ -1023,6 +1027,12 @@ where
10231027
continue;
10241028
}
10251029

1030+
// While the failsafe is active, exclude every interop tx regardless of its deadline.
1031+
if interop_failsafe_active && is_interop_tx(&*tx) {
1032+
best_txs.mark_invalid(tx.signer(), tx.nonce());
1033+
continue;
1034+
}
1035+
10261036
// We skip invalid cross chain txs, they would be removed on the next block update in
10271037
// the maintenance job
10281038
if let Some(interop) = interop &&

rust/op-reth/crates/payload/src/builder/tests.rs

Lines changed: 93 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use alloy_consensus::{
88
};
99
use alloy_eips::{
1010
eip2718::{Encodable2718, WithEncoded},
11-
eip2930::AccessList,
11+
eip2930::{AccessList, AccessListItem},
1212
eip7702::SignedAuthorization,
1313
};
1414
use alloy_evm::RecoveredTx;
@@ -22,8 +22,11 @@ use reth_optimism_chainspec::{OpChainSpec, OpChainSpecBuilder};
2222
use reth_optimism_evm::{OpEvmConfig, PostExecMode};
2323
use reth_optimism_primitives::{OpPrimitives, OpTransactionSigned};
2424
use reth_optimism_txpool::{
25-
OpPooledTransaction, OpPooledTx, conditional::MaybeConditionalTransaction,
26-
estimated_da_size::DataAvailabilitySized, interop::MaybeInteropTransaction,
25+
OpPooledTransaction, OpPooledTx,
26+
conditional::MaybeConditionalTransaction,
27+
estimated_da_size::DataAvailabilitySized,
28+
interop::{InteropFailsafe, MaybeInteropTransaction},
29+
interop_filter::CROSS_L2_INBOX_ADDRESS,
2730
};
2831
use reth_payload_builder_primitives::PayloadBuilderError;
2932
use reth_payload_util::PayloadTransactionsFixed;
@@ -165,6 +168,31 @@ fn op_pooled_tx(nonce: u64, signer: Address, recipient: Address) -> OpPooledTran
165168
OpPooledTransaction::new(Recovered::new_unchecked(tx, signer), encoded_len)
166169
}
167170

171+
/// Builds an interop pool tx: a `CROSS_L2_INBOX_ADDRESS` access-list entry makes `is_interop_tx`
172+
/// match it; the gas limit covers the access-list intrinsic cost so it executes when failsafe is
173+
/// off.
174+
fn op_interop_pooled_tx(nonce: u64, signer: Address, recipient: Address) -> OpPooledTransaction {
175+
let tx: OpTransactionSigned = TxEip1559 {
176+
chain_id: 8453,
177+
nonce,
178+
gas_limit: 100_000,
179+
max_fee_per_gas: 1,
180+
max_priority_fee_per_gas: 1,
181+
to: TxKind::Call(recipient),
182+
value: U256::ZERO,
183+
access_list: AccessList(vec![AccessListItem {
184+
address: CROSS_L2_INBOX_ADDRESS,
185+
storage_keys: vec![B256::ZERO],
186+
}]),
187+
..Default::default()
188+
}
189+
.into_signed(Signature::test_signature())
190+
.into();
191+
let encoded_len = tx.encode_2718_len();
192+
193+
OpPooledTransaction::new(Recovered::new_unchecked(tx, signer), encoded_len)
194+
}
195+
168196
fn tx_hashes<'a>(txs: impl IntoIterator<Item = &'a Recovered<OpTransactionSigned>>) -> Vec<TxHash> {
169197
txs.into_iter().map(|tx| *TxHashRef::tx_hash(tx)).collect()
170198
}
@@ -181,7 +209,23 @@ where
181209
let gas_limit = 1_000_000;
182210
let chain_spec = Arc::new(OpChainSpecBuilder::base_mainnet().regolith_activated().build());
183211
let ctx = payload_builder_ctx(chain_spec, gas_limit);
212+
run_execute_best_transactions_with_ctx(ctx, signer, txs, gas_limit_cap, committed_txs)
213+
}
184214

215+
fn run_execute_best_transactions_with_ctx<T>(
216+
ctx: OpPayloadBuilderCtx<
217+
OpEvmConfig<OpChainSpec, OpPrimitives>,
218+
OpChainSpec,
219+
OpPayloadBuilderAttributes<OpTransactionSigned>,
220+
>,
221+
signer: Address,
222+
txs: Vec<T>,
223+
gas_limit_cap: Option<u64>,
224+
committed_txs: Option<&mut Vec<Recovered<OpTransactionSigned>>>,
225+
) -> (ExecutionInfo, Vec<TxHash>)
226+
where
227+
T: PoolTransaction<Consensus = OpTransactionSigned> + OpPooledTx,
228+
{
185229
let mut state_provider = StateProviderTest::default();
186230
state_provider.insert_account(
187231
signer,
@@ -600,3 +644,49 @@ fn miner_fee_uses_pool_wrapper_tip() {
600644
assert_eq!(info.total_fees, expected_fees);
601645
assert_ne!(info.total_fees, natural_fees);
602646
}
647+
648+
/// With the failsafe active the builder excludes interop txs but keeps normal txs; with it off the
649+
/// same interop tx is included — proving the gate is flag-driven, not a blanket exclusion, and does
650+
/// not depend on the pool's interop-deadline marker. Clearing the flag and rebuilding includes the
651+
/// interop tx again, proving the exclusion is a per-build decision and the `mark_invalid` it
652+
/// triggers does not stick across builds.
653+
#[test]
654+
fn execute_best_transactions_excludes_interop_txs_when_failsafe_active() {
655+
let signer = Address::repeat_byte(0x11);
656+
let normal = op_pooled_tx(0, signer, Address::repeat_byte(0x22));
657+
let interop = op_interop_pooled_tx(1, signer, Address::repeat_byte(0x33));
658+
let normal_hash = *normal.hash();
659+
let interop_hash = *interop.hash();
660+
661+
let gas_limit = 1_000_000;
662+
let chain_spec = Arc::new(OpChainSpecBuilder::base_mainnet().regolith_activated().build());
663+
664+
// One shared handle drives every build, mirroring the single failsafe threaded through node
665+
// setup; toggling it is what flips the gate, not building a fresh config each time.
666+
let failsafe = InteropFailsafe::default();
667+
let build = |failsafe: &InteropFailsafe| {
668+
let mut ctx = payload_builder_ctx(chain_spec.clone(), gas_limit);
669+
ctx.builder_config.interop_failsafe = failsafe.clone();
670+
let (_info, included) = run_execute_best_transactions_with_ctx(
671+
ctx,
672+
signer,
673+
vec![normal.clone(), interop.clone()],
674+
None,
675+
None,
676+
);
677+
included
678+
};
679+
680+
// Failsafe off: both the normal and the interop tx are included.
681+
failsafe.set(false);
682+
assert_eq!(build(&failsafe), vec![normal_hash, interop_hash]);
683+
684+
// Failsafe on: the interop tx is excluded (marked invalid), the normal tx is still included.
685+
failsafe.set(true);
686+
assert_eq!(build(&failsafe), vec![normal_hash]);
687+
688+
// Failsafe cleared again: the same interop tx is included once more, confirming the previous
689+
// build's `mark_invalid` did not permanently exclude it.
690+
failsafe.set(false);
691+
assert_eq!(build(&failsafe), vec![normal_hash, interop_hash]);
692+
}

rust/op-reth/crates/payload/src/config.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//! Additional configuration for the OP builder
22
3+
use reth_optimism_txpool::interop::InteropFailsafe;
34
use std::sync::{
45
Arc,
56
atomic::{AtomicBool, AtomicU64, Ordering},
@@ -14,6 +15,9 @@ pub struct OpBuilderConfig {
1415
pub gas_limit_config: OpGasLimitConfig,
1516
/// Local SDM `PostExec` production opt-in. Shared with the admin RPC.
1617
pub sdm_post_exec_opt_in: SdmPostExecOptIn,
18+
/// Interop failsafe gate. Set by the interop filter client; read by the builder to exclude
19+
/// interop txs from blocks while it is enabled.
20+
pub interop_failsafe: InteropFailsafe,
1721
/// Maximum cumulative uncompressed (EIP-2718 encoded) block size in bytes.
1822
///
1923
/// `None` disables the limit (the historical behavior). When set, the payload builder stops
@@ -32,6 +36,7 @@ impl OpBuilderConfig {
3236
da_config,
3337
gas_limit_config,
3438
sdm_post_exec_opt_in: SdmPostExecOptIn::default(),
39+
interop_failsafe: InteropFailsafe::default(),
3540
max_uncompressed_block_size: None,
3641
}
3742
}

rust/op-reth/crates/txpool/src/interop.rs

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
11
//! Additional support for pooled interop transactions.
22
33
use alloy_consensus::Transaction;
4-
use reth_transaction_pool::PoolTransaction;
4+
use std::sync::{
5+
Arc,
6+
atomic::{AtomicBool, Ordering},
7+
};
58

69
use crate::interop_filter::CROSS_L2_INBOX_ADDRESS;
710

8-
/// Returns true if the transaction's access list targets `CROSS_L2_INBOX_ADDRESS`
9-
/// with at least one storage key.
10-
pub(crate) fn is_interop_tx<T>(tx: &T) -> bool
11+
/// Returns true if the transaction's access list targets `CROSS_L2_INBOX_ADDRESS` with at least
12+
/// one storage key. Detection is intrinsic to the tx, so it also catches interop txs that never
13+
/// went through interop validation (e.g. private or locally-submitted txs).
14+
pub fn is_interop_tx<T>(tx: &T) -> bool
1115
where
12-
T: PoolTransaction + Transaction,
16+
T: Transaction,
1317
{
1418
tx.access_list()
1519
.map(|al| {
@@ -19,6 +23,26 @@ where
1923
.unwrap_or(false)
2024
}
2125

26+
/// Shareable interop failsafe gate. The interop filter client writes it; the payload builder reads
27+
/// it to exclude interop txs while it is enabled. Cloning shares the flag, so one handle keeps the
28+
/// writer and reader in sync.
29+
#[derive(Debug, Clone, Default)]
30+
pub struct InteropFailsafe {
31+
inner: Arc<AtomicBool>,
32+
}
33+
34+
impl InteropFailsafe {
35+
/// Returns the current failsafe state.
36+
pub fn enabled(&self) -> bool {
37+
self.inner.load(Ordering::Acquire)
38+
}
39+
40+
/// Sets the failsafe state.
41+
pub fn set(&self, enabled: bool) {
42+
self.inner.store(enabled, Ordering::Release);
43+
}
44+
}
45+
2246
/// Helper trait that allows attaching an interop deadline.
2347
pub trait MaybeInteropTransaction {
2448
/// Attach an interop deadline

0 commit comments

Comments
 (0)