Skip to content

Commit 4d87cf1

Browse files
authored
Fix integration tests (flashbots#536)
## πŸ“ Summary Fix deposit and flashblocks integration tests. Resolves flashbots#533 and flashbots#521 ## βœ… I have completed the following steps: * [x] Run `make lint` * [x] Run `make test` * [x] Added tests (if applicable)
1 parent e4611f9 commit 4d87cf1

File tree

4 files changed

+144
-72
lines changed

4 files changed

+144
-72
lines changed

β€ŽCargo.lock

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

β€Žcrates/op-rbuilder/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ derive_more.workspace = true
7979
metrics.workspace = true
8080
serde_json.workspace = true
8181
tokio-util.workspace = true
82+
thiserror.workspace = true
8283

8384
time = { version = "0.3.36", features = ["macros", "formatting", "parsing"] }
8485
chrono = "0.4"

β€Žcrates/op-rbuilder/src/payload_builder.rs

+140-70
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ use crate::{
33
primitives::reth::ExecutionInfo,
44
tx_signer::Signer,
55
};
6-
use alloy_consensus::{Eip658Value, Header, Transaction, Typed2718, EMPTY_OMMER_ROOT_HASH};
6+
use alloy_consensus::{
7+
constants::EMPTY_WITHDRAWALS, Eip658Value, Header, Transaction, Typed2718,
8+
EMPTY_OMMER_ROOT_HASH,
9+
};
710
use alloy_eips::{merge::BEACON_NONCE, Encodable2718};
811
use alloy_op_evm::block::receipt_builder::OpReceiptBuilder;
912
use alloy_primitives::{map::HashMap, Address, Bytes, B256, U256};
@@ -29,7 +32,7 @@ use reth_evm::{
2932
use reth_execution_types::ExecutionOutcome;
3033
use reth_node_api::{NodePrimitives, NodeTypesWithEngine, TxTy};
3134
use reth_optimism_chainspec::OpChainSpec;
32-
use reth_optimism_consensus::calculate_receipt_root_no_memo_optimism;
35+
use reth_optimism_consensus::{calculate_receipt_root_no_memo_optimism, isthmus};
3336
use reth_optimism_evm::{OpEvmConfig, OpNextBlockEnvAttributes};
3437
use reth_optimism_forks::OpHardforks;
3538
use reth_optimism_node::OpEngineTypes;
@@ -60,14 +63,25 @@ use rollup_boost::{
6063
ExecutionPayloadBaseV1, ExecutionPayloadFlashblockDeltaV1, FlashblocksPayloadV1,
6164
};
6265
use serde::{Deserialize, Serialize};
63-
use std::sync::{Arc, Mutex};
66+
use std::{
67+
sync::{Arc, Mutex},
68+
time::Duration,
69+
};
6470
use tokio::{
6571
net::{TcpListener, TcpStream},
6672
sync::mpsc,
6773
};
6874
use tokio_tungstenite::{accept_async, WebSocketStream};
6975
use tokio_util::sync::CancellationToken;
70-
use tracing::{debug, trace, warn};
76+
use tracing::{debug, error, trace, warn};
77+
78+
/// Flashblocks specific payload building errors.
79+
#[derive(Debug, thiserror::Error)]
80+
pub enum FlashblockPayloadBuilderError {
81+
/// Thrown when the job was cancelled.
82+
#[error("error sending build signal")]
83+
SendBuildSignalError,
84+
}
7185

7286
#[derive(Debug, Serialize, Deserialize)]
7387
struct FlashblocksMetadata<N: NodePrimitives> {
@@ -395,65 +409,115 @@ where
395409

396410
let mut flashblock_count = 0;
397411

398-
// 2. loop every n time and try to build an increasing block
399-
loop {
400-
if ctx.cancel.is_cancelled() {
401-
tracing::info!(
402-
target: "payload_builder",
403-
"Job cancelled, stopping payload building",
404-
);
405-
// if the job was cancelled, stop
406-
return Ok(());
407-
}
408-
409-
println!(
410-
"Building flashblock {} {}",
411-
ctx.payload_id(),
412-
flashblock_count,
413-
);
412+
// Create a channel to coordinate flashblock building
413+
let (build_tx, mut build_rx) = mpsc::channel(1);
414414

415-
tracing::info!(
416-
target: "payload_builder",
417-
"Building flashblock {}",
418-
flashblock_count,
419-
);
420-
421-
let state = StateProviderDatabase::new(&state_provider);
422-
423-
let mut db = State::builder()
424-
.with_database(state)
425-
.with_bundle_update()
426-
.with_bundle_prestate(bundle_state)
427-
.build();
415+
// Spawn the timer task that signals when to build a new flashblock
416+
let cancel_clone = ctx.cancel.clone();
417+
let flashblock_block_time = self.flashblock_block_time;
418+
tokio::spawn(async move {
419+
let mut interval = tokio::time::interval(Duration::from_millis(flashblock_block_time));
420+
loop {
421+
tokio::select! {
422+
// Add a cancellation check that only runs every 10ms to avoid tight polling
423+
_ = tokio::time::sleep(Duration::from_millis(10)) => {
424+
if cancel_clone.is_cancelled() {
425+
tracing::info!(target: "payload_builder", "Job cancelled during sleep, stopping payload building");
426+
drop(build_tx);
427+
break;
428+
}
429+
}
430+
_ = interval.tick() => {
431+
if let Err(err) = build_tx.send(()).await {
432+
error!(target: "payload_builder", "Error sending build signal: {}", err);
433+
break;
434+
}
435+
}
436+
}
437+
}
438+
});
428439

429-
let best_txs = BestPayloadTransactions::new(
430-
self.pool
431-
.best_transactions_with_attributes(ctx.best_transaction_attributes()),
432-
);
433-
ctx.execute_best_transactions(&mut info, &mut db, best_txs, total_gas_per_batch)?;
440+
// Process flashblocks in a blocking loop
441+
loop {
442+
// Block on receiving a message, break on cancellation or closed channel
443+
let received = tokio::task::block_in_place(|| {
444+
// Get runtime handle
445+
let rt = tokio::runtime::Handle::current();
446+
447+
// Run the async operation to completion, blocking the current thread
448+
rt.block_on(async {
449+
// Check for cancellation first
450+
if ctx.cancel.is_cancelled() {
451+
tracing::info!(
452+
target: "payload_builder",
453+
"Job cancelled, stopping payload building",
454+
);
455+
return None;
456+
}
434457

435-
if ctx.cancel.is_cancelled() {
436-
tracing::info!(
437-
target: "payload_builder",
438-
"Job cancelled, stopping payload building",
439-
);
440-
// if the job was cancelled, stop
441-
return Ok(());
442-
}
458+
// Wait for next message
459+
build_rx.recv().await
460+
})
461+
});
443462

444-
let (payload, mut fb_payload, new_bundle_state) = build_block(db, &ctx, &mut info)?;
463+
// Exit loop if channel closed or cancelled
464+
match received {
465+
Some(()) => {
466+
// Continue with flashblock building
467+
tracing::info!(
468+
target: "payload_builder",
469+
"Building flashblock {}",
470+
flashblock_count,
471+
);
472+
473+
let state = StateProviderDatabase::new(&state_provider);
474+
475+
let mut db = State::builder()
476+
.with_database(state)
477+
.with_bundle_update()
478+
.with_bundle_prestate(bundle_state)
479+
.build();
480+
481+
let best_txs = BestPayloadTransactions::new(
482+
self.pool
483+
.best_transactions_with_attributes(ctx.best_transaction_attributes()),
484+
);
485+
ctx.execute_best_transactions(
486+
&mut info,
487+
&mut db,
488+
best_txs,
489+
total_gas_per_batch,
490+
)?;
491+
492+
if ctx.cancel.is_cancelled() {
493+
tracing::info!(
494+
target: "payload_builder",
495+
"Job cancelled, stopping payload building",
496+
);
497+
// if the job was cancelled, stop
498+
return Ok(());
499+
}
445500

446-
best_payload.set(payload.clone());
501+
let (new_payload, mut fb_payload, new_bundle_state) =
502+
build_block(db, &ctx, &mut info)?;
447503

448-
fb_payload.index = flashblock_count + 1; // we do this because the fallback block is index 0
449-
fb_payload.base = None;
450-
let _ = self.send_message(serde_json::to_string(&fb_payload).unwrap_or_default());
504+
fb_payload.index = flashblock_count + 1; // we do this because the fallback block is index 0
505+
fb_payload.base = None;
506+
let _ =
507+
self.send_message(serde_json::to_string(&fb_payload).unwrap_or_default());
451508

452-
bundle_state = new_bundle_state;
453-
total_gas_per_batch += gas_per_batch;
454-
flashblock_count += 1;
509+
best_payload.set(new_payload.clone());
510+
bundle_state = new_bundle_state;
511+
total_gas_per_batch += gas_per_batch;
512+
flashblock_count += 1;
455513

456-
std::thread::sleep(std::time::Duration::from_millis(self.flashblock_block_time));
514+
tracing::info!(target: "payload_builder", "Flashblock {} built", flashblock_count);
515+
}
516+
None => {
517+
// Exit loop if channel closed or cancelled
518+
return Ok(());
519+
}
520+
}
457521
}
458522
}
459523
}
@@ -501,6 +565,7 @@ where
501565
block_number,
502566
vec![],
503567
);
568+
504569
let receipts_root = execution_outcome
505570
.generic_receipts_root_slow(block_number, |receipts| {
506571
calculate_receipt_root_no_memo_optimism(
@@ -531,6 +596,25 @@ where
531596
})?
532597
};
533598

599+
let withdrawals_root = if ctx
600+
.chain_spec
601+
.is_isthmus_active_at_timestamp(ctx.attributes().timestamp())
602+
{
603+
// withdrawals root field in block header is used for storage root of L2 predeploy
604+
// `l2tol1-message-passer`
605+
Some(
606+
isthmus::withdrawals_root(&execution_outcome.state(), state.database.as_ref())
607+
.map_err(PayloadBuilderError::other)?,
608+
)
609+
} else if ctx
610+
.chain_spec
611+
.is_canyon_active_at_timestamp(ctx.attributes().timestamp())
612+
{
613+
Some(EMPTY_WITHDRAWALS)
614+
} else {
615+
None
616+
};
617+
534618
// create the block header
535619
let transactions_root = proofs::calculate_transaction_root(&info.executed_transactions);
536620

@@ -547,7 +631,7 @@ where
547631
state_root,
548632
transactions_root,
549633
receipts_root,
550-
withdrawals_root: None,
634+
withdrawals_root,
551635
logs_bloom,
552636
timestamp: ctx.attributes().payload_attributes.timestamp,
553637
mix_hash: ctx.attributes().payload_attributes.prev_randao,
@@ -961,30 +1045,19 @@ where
9611045
where
9621046
DB: Database<Error = ProviderError>,
9631047
{
964-
println!("Executing best transactions");
9651048
let base_fee = self.base_fee();
9661049

9671050
let mut evm = self.evm_config.evm_with_env(&mut *db, self.evm_env.clone());
9681051

9691052
while let Some(tx) = best_txs.next(()) {
9701053
let tx = tx.into_consensus();
971-
println!("tx: {:?}", tx);
972-
println!(
973-
"gas limit: {:?}, batch gas limit: {:?} cummulative gas used: {:?}",
974-
tx.gas_limit(),
975-
batch_gas_limit,
976-
info.cumulative_gas_used
977-
);
978-
// gas limit: 100816112, batch gas limit: 2500000000 cummulative gas used: 100062216
979-
9801054
// check in info if the txn has been executed already
9811055
if info.executed_transactions.contains(&tx) {
9821056
continue;
9831057
}
9841058

9851059
// ensure we still have capacity for this transaction
9861060
if info.is_tx_over_limits(tx.inner(), batch_gas_limit, None, None) {
987-
println!("A");
9881061
// we can't fit this transaction into the block, so we need to mark it as
9891062
// invalid which also removes all dependent transaction from
9901063
// the iterator before we can continue
@@ -1001,11 +1074,9 @@ where
10011074

10021075
// check if the job was cancelled, if so we can exit early
10031076
if self.cancel.is_cancelled() {
1004-
println!("C");
10051077
return Ok(Some(()));
10061078
}
10071079

1008-
println!("Start transaction");
10091080
let ResultAndState { result, state } = match evm.transact(&tx) {
10101081
Ok(res) => res,
10111082
Err(err) => {
@@ -1026,7 +1097,6 @@ where
10261097
return Err(PayloadBuilderError::EvmExecutionError(Box::new(err)));
10271098
}
10281099
};
1029-
println!("Finish transaction");
10301100

10311101
// add gas used by the transaction to cumulative gas used, before creating the
10321102
// receipt

β€Žcrates/op-rbuilder/src/tester/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ impl<'a> BlockGenerator<'a> {
350350
mint: None,
351351
value: U256::default(),
352352
gas_limit: 210000,
353-
is_system_transaction: true,
353+
is_system_transaction: false,
354354
input: FJORD_DATA.into(),
355355
};
356356

@@ -471,7 +471,7 @@ impl<'a> BlockGenerator<'a> {
471471
mint: Some(value), // Amount to deposit
472472
value: U256::default(),
473473
gas_limit: 210000,
474-
is_system_transaction: true,
474+
is_system_transaction: false,
475475
input: Bytes::default(),
476476
};
477477

0 commit comments

Comments
Β (0)