Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions crates/op-rbuilder/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,7 @@ pub struct OpRbuilderArgs {
/// How much time extra to wait for the block building job to complete and not get garbage collected
#[arg(long = "builder.extra-block-deadline-secs", default_value = "20")]
pub extra_block_deadline_secs: u64,
/// Whether to enable revert protection by default
#[arg(long = "builder.enable-revert-protection", default_value = "false")]
pub enable_revert_protection: bool,
}
12 changes: 12 additions & 0 deletions crates/op-rbuilder/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ pub struct BlockPayloadJobGenerator<Client, Tasks, Builder> {
last_payload: Arc<Mutex<CancellationToken>>,
/// The extra block deadline in seconds
extra_block_deadline: std::time::Duration,
/// Whether to enable revert protection
enable_revert_protection: bool,
}

// === impl EmptyBlockPayloadJobGenerator ===
Expand All @@ -88,6 +90,7 @@ impl<Client, Tasks, Builder> BlockPayloadJobGenerator<Client, Tasks, Builder> {
builder: Builder,
ensure_only_one_payload: bool,
extra_block_deadline: std::time::Duration,
enable_revert_protection: bool,
) -> Self {
Self {
client,
Expand All @@ -97,6 +100,7 @@ impl<Client, Tasks, Builder> BlockPayloadJobGenerator<Client, Tasks, Builder> {
ensure_only_one_payload,
last_payload: Arc::new(Mutex::new(CancellationToken::new())),
extra_block_deadline,
enable_revert_protection,
}
}
}
Expand Down Expand Up @@ -182,6 +186,7 @@ where
cancel: cancel_token,
deadline,
build_complete: None,
enable_revert_protection: self.enable_revert_protection,
};

job.spawn_build_job();
Expand Down Expand Up @@ -214,6 +219,8 @@ where
pub(crate) cancel: CancellationToken,
pub(crate) deadline: Pin<Box<Sleep>>, // Add deadline
pub(crate) build_complete: Option<oneshot::Receiver<Result<(), PayloadBuilderError>>>,
/// Block building options
pub(crate) enable_revert_protection: bool,
}

impl<Tasks, Builder> PayloadJob for BlockPayloadJob<Tasks, Builder>
Expand Down Expand Up @@ -256,6 +263,8 @@ pub struct BuildArguments<Attributes, Payload: BuiltPayload> {
pub config: PayloadConfig<Attributes, HeaderTy<Payload::Primitives>>,
/// A marker that can be used to cancel the job.
pub cancel: CancellationToken,
/// Whether to enable revert protection
pub enable_revert_protection: bool,
}

/// A [PayloadJob] is a future that's being polled by the `PayloadBuilderService`
Expand All @@ -271,6 +280,7 @@ where
let payload_config = self.config.clone();
let cell = self.cell.clone();
let cancel = self.cancel.clone();
let enable_revert_protection = self.enable_revert_protection;

let (tx, rx) = oneshot::channel();
self.build_complete = Some(rx);
Expand All @@ -280,6 +290,7 @@ where
cached_reads: Default::default(),
config: payload_config,
cancel,
enable_revert_protection,
};

let result = builder.try_build(args, cell);
Expand Down Expand Up @@ -639,6 +650,7 @@ mod tests {
builder.clone(),
false,
std::time::Duration::from_secs(1),
false,
);

// this is not nice but necessary
Expand Down
39 changes: 38 additions & 1 deletion crates/op-rbuilder/src/integration/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,42 @@ mod tests {
Ok(())
}

#[tokio::test]
#[cfg(not(feature = "flashblocks"))]
async fn integration_test_revert_protection_disabled() -> eyre::Result<()> {
let harness = TestHarness::new("integration_test_revert_protection_disabled").await;
let mut generator = harness.block_generator().await?;

let txn1 = harness.send_valid_transaction().await?;
let txn2 = harness.send_revert_transaction().await?;
let pending_txn = vec![txn1, txn2];

let block_hash = generator.generate_block().await?;

// the transactions should be included in the block now
let pending_txn = {
let mut transaction_hashes = Vec::new();
for txn in pending_txn {
let txn_hash = txn.with_timeout(None).watch().await?;
transaction_hashes.push(txn_hash);
}
transaction_hashes
};

// validate that all the transaction hashes are included in the block
let provider = harness.provider()?;
let block = provider
.get_block_by_hash(block_hash)
.await?
.expect("block");

for txn in pending_txn {
assert!(block.transactions.hashes().any(|hash| hash == txn));
}

Ok(())
}

#[tokio::test]
#[cfg(not(feature = "flashblocks"))]
async fn integration_test_revert_protection() -> eyre::Result<()> {
Expand All @@ -115,7 +151,8 @@ mod tests {
.auth_rpc_port(1244)
.network_port(1245)
.http_port(1248)
.with_builder_private_key(BUILDER_PRIVATE_KEY);
.with_builder_private_key(BUILDER_PRIVATE_KEY)
.with_revert_protection(true);

// create the validation reth node
let reth_data_dir = std::env::temp_dir().join(Uuid::new_v4().to_string());
Expand Down
66 changes: 62 additions & 4 deletions crates/op-rbuilder/src/integration/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use alloy_consensus::TxEip1559;
use alloy_eips::{eip1559::MIN_PROTOCOL_BASE_FEE, eip2718::Encodable2718, BlockNumberOrTag};
use alloy_provider::{Identity, Provider, ProviderBuilder};
use alloy_primitives::hex;
use alloy_provider::{
Identity, PendingTransactionBuilder, Provider, ProviderBuilder, RootProvider,
};
use op_alloy_consensus::OpTypedTransaction;
use op_alloy_network::Optimism;
use op_rbuilder::OpRbuilderConfig;
Expand Down Expand Up @@ -269,7 +272,9 @@ impl TestHarness {
}
}

pub async fn send_valid_transaction(&self) -> eyre::Result<()> {
pub async fn send_valid_transaction(
&self,
) -> eyre::Result<PendingTransactionBuilder<Optimism>> {
// Get builder's address
let known_wallet = Signer::try_from_secret(BUILDER_PRIVATE_KEY.parse()?)?;
let builder_address = known_wallet.address;
Expand Down Expand Up @@ -303,11 +308,64 @@ impl TestHarness {
..Default::default()
});
let signed_tx = known_wallet.sign_tx(tx_request)?;
let _ = provider
let pending_tx = provider
.send_raw_transaction(signed_tx.encoded_2718().as_slice())
.await?;

Ok(())
Ok(pending_tx)
}

pub async fn send_revert_transaction(
&self,
) -> eyre::Result<PendingTransactionBuilder<Optimism>> {
// TODO: Merge this with send_valid_transaction
// Get builder's address
let known_wallet = Signer::try_from_secret(BUILDER_PRIVATE_KEY.parse()?)?;
let builder_address = known_wallet.address;

let url = format!("http://localhost:{}", self.builder_http_port);
let provider =
ProviderBuilder::<Identity, Identity, Optimism>::default().on_http(url.parse()?);

// Get current nonce includeing the ones from the txpool
let nonce = provider
.get_transaction_count(builder_address)
.pending()
.await?;

let latest_block = provider
.get_block_by_number(BlockNumberOrTag::Latest)
.await?
.unwrap();

let base_fee = max(
latest_block.header.base_fee_per_gas.unwrap(),
MIN_PROTOCOL_BASE_FEE,
);

// Transaction from builder should succeed
let tx_request = OpTypedTransaction::Eip1559(TxEip1559 {
chain_id: 901,
nonce,
gas_limit: 210000,
max_fee_per_gas: base_fee.into(),
input: hex!("60006000fd").into(), // PUSH1 0x00 PUSH1 0x00 REVERT
..Default::default()
});
let signed_tx = known_wallet.sign_tx(tx_request)?;
let pending_tx = provider
.send_raw_transaction(signed_tx.encoded_2718().as_slice())
.await?;

Ok(pending_tx)
}

pub fn provider(&self) -> eyre::Result<RootProvider<Optimism>> {
let url = format!("http://localhost:{}", self.builder_http_port);
let provider =
ProviderBuilder::<Identity, Identity, Optimism>::default().on_http(url.parse()?);

Ok(provider)
}

pub async fn block_generator(&self) -> eyre::Result<BlockGenerator> {
Expand Down
12 changes: 12 additions & 0 deletions crates/op-rbuilder/src/integration/op_rbuilder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub struct OpRbuilderConfig {
flashblocks_ws_url: Option<String>,
chain_block_time: Option<u64>,
flashbots_block_time: Option<u64>,
with_revert_protection: Option<bool>,
}

impl OpRbuilderConfig {
Expand Down Expand Up @@ -64,6 +65,11 @@ impl OpRbuilderConfig {
self
}

pub fn with_revert_protection(mut self, revert_protection: bool) -> Self {
self.with_revert_protection = Some(revert_protection);
self
}

pub fn with_flashblocks_ws_url(mut self, url: &str) -> Self {
self.flashblocks_ws_url = Some(url.to_string());
self
Expand Down Expand Up @@ -114,6 +120,12 @@ impl Service for OpRbuilderConfig {
.arg(self.network_port.expect("network_port not set").to_string())
.arg("--ipcdisable");

if let Some(revert_protection) = self.with_revert_protection {
if revert_protection {
cmd.arg("--builder.enable-revert-protection");
}
}

if let Some(builder_private_key) = &self.builder_private_key {
cmd.arg("--rollup.builder-secret-key")
.arg(builder_private_key);
Expand Down
1 change: 1 addition & 0 deletions crates/op-rbuilder/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ fn main() {
.with_components(op_node.components().payload(CustomOpPayloadBuilder::new(
builder_args.builder_signer,
std::time::Duration::from_secs(builder_args.extra_block_deadline_secs),
builder_args.enable_revert_protection,
builder_args.flashblocks_ws_url,
builder_args.chain_block_time,
builder_args.flashblock_block_time,
Expand Down
5 changes: 5 additions & 0 deletions crates/op-rbuilder/src/payload_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,14 @@ pub struct CustomOpPayloadBuilder {
chain_block_time: u64,
flashblock_block_time: u64,
extra_block_deadline: std::time::Duration,
enable_revert_protection: bool,
}

impl CustomOpPayloadBuilder {
pub fn new(
builder_signer: Option<Signer>,
extra_block_deadline: std::time::Duration,
enable_revert_protection: bool,
flashblocks_ws_url: String,
chain_block_time: u64,
flashblock_block_time: u64,
Expand All @@ -116,6 +118,7 @@ impl CustomOpPayloadBuilder {
chain_block_time,
flashblock_block_time,
extra_block_deadline,
enable_revert_protection,
}
}
}
Expand Down Expand Up @@ -172,6 +175,7 @@ where
) -> eyre::Result<PayloadBuilderHandle<<Node::Types as NodeTypes>::Payload>> {
tracing::info!("Spawning a custom payload builder");
let extra_block_deadline = self.extra_block_deadline;
let enable_revert_protection = self.enable_revert_protection;
let payload_builder = self.build_payload_builder(ctx, pool).await?;
let payload_job_config = BasicPayloadJobGeneratorConfig::default();

Expand All @@ -182,6 +186,7 @@ where
payload_builder,
true,
extra_block_deadline,
enable_revert_protection,
);

let (payload_service, payload_builder) =
Expand Down
25 changes: 21 additions & 4 deletions crates/op-rbuilder/src/payload_builder_vanilla.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ const TOTAL_COST_FLOOR_PER_TOKEN: u64 = 10;
pub struct CustomOpPayloadBuilder {
builder_signer: Option<Signer>,
extra_block_deadline: std::time::Duration,
enable_revert_protection: bool,
#[cfg(feature = "flashblocks")]
flashblocks_ws_url: String,
#[cfg(feature = "flashblocks")]
Expand Down Expand Up @@ -105,13 +106,15 @@ impl CustomOpPayloadBuilder {
pub fn new(
builder_signer: Option<Signer>,
extra_block_deadline: std::time::Duration,
enable_revert_protection: bool,
_flashblocks_ws_url: String,
_chain_block_time: u64,
_flashblock_block_time: u64,
) -> Self {
Self {
builder_signer,
extra_block_deadline,
enable_revert_protection,
}
}
}
Expand Down Expand Up @@ -167,6 +170,7 @@ where
) -> eyre::Result<PayloadBuilderHandle<<Node::Types as NodeTypes>::Payload>> {
tracing::info!("Spawning a custom payload builder");
let extra_block_deadline = self.extra_block_deadline;
let enable_revert_protection = self.enable_revert_protection;
let payload_builder = self.build_payload_builder(ctx, pool).await?;
let payload_job_config = BasicPayloadJobGeneratorConfig::default();

Expand All @@ -177,6 +181,7 @@ where
payload_builder,
false,
extra_block_deadline,
enable_revert_protection,
);

let (payload_service, payload_builder) =
Expand Down Expand Up @@ -351,6 +356,7 @@ where
mut cached_reads,
config,
cancel,
enable_revert_protection,
} = args;

let chain_spec = self.client.chain_spec();
Expand Down Expand Up @@ -392,6 +398,7 @@ where
cancel,
builder_signer: self.builder_signer,
metrics: Default::default(),
enable_revert_protection,
};

let builder = OpBuilder::new(best, remove_reverted);
Expand Down Expand Up @@ -771,6 +778,8 @@ pub struct OpPayloadBuilderCtx<ChainSpec, N: NodePrimitives> {
pub builder_signer: Option<Signer>,
/// The metrics for the builder
pub metrics: OpRBuilderMetrics,
/// Whether we enabled revert protection
pub enable_revert_protection: bool,
}

impl<ChainSpec, N> OpPayloadBuilderCtx<ChainSpec, N>
Expand Down Expand Up @@ -1094,6 +1103,12 @@ where
}
};

println!("include reverted txn: {}", result.is_success());
println!(
"self.enable_revert_protection: {}",
self.enable_revert_protection
);

self.metrics
.tx_simulation_duration
.record(tx_simulation_start_time.elapsed());
Expand All @@ -1103,10 +1118,12 @@ where
num_txs_simulated_success += 1;
} else {
num_txs_simulated_fail += 1;
info!(target: "payload_builder", tx_hash = ?tx.tx_hash(), "skipping reverted transaction");
best_txs.mark_invalid(tx.signer(), tx.nonce());
info.invalid_tx_hashes.insert(tx.tx_hash());
continue;
if self.enable_revert_protection {
info!(target: "payload_builder", tx_hash = ?tx.tx_hash(), "skipping reverted transaction");
best_txs.mark_invalid(tx.signer(), tx.nonce());
info.invalid_tx_hashes.insert(tx.tx_hash());
continue;
}
}

// add gas used by the transaction to cumulative gas used, before creating the
Expand Down
Loading