Skip to content
Closed
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
10 changes: 10 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions crates/op-rbuilder/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ reth-optimism-rpc.workspace = true
reth-tasks.workspace = true
reth-tracing-otlp = { workspace = true, optional = true }

# reth-monitor for transaction and block monitoring
# Note: Initialization is done in xlayer-reth, op-rbuilder only uses get_global_tracer()
reth-monitor = { git = "https://github.com/okx/reth", branch = "leo/support-full-trace" }

Choose a reason for hiding this comment

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

You should specify the patch version here?


alloy-primitives.workspace = true
alloy-consensus.workspace = true
alloy-contract.workspace = true
Expand Down
93 changes: 92 additions & 1 deletion crates/op-rbuilder/src/builders/context.rs

Choose a reason for hiding this comment

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

Standard payload builders are not used and can completely be removed

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use alloy_consensus::{Eip658Value, Transaction, conditional::BlockConditionalAtt
use alloy_eips::{Encodable2718, Typed2718};
use alloy_evm::Database;
use alloy_op_evm::block::receipt_builder::OpReceiptBuilder;
use alloy_primitives::{BlockHash, Bytes, U256};
use alloy_primitives::{BlockHash, Bytes, U256, B256};
use alloy_rpc_types_eth::Withdrawals;
use core::fmt::Debug;
use op_alloy_consensus::OpDepositReceipt;
Expand Down Expand Up @@ -46,6 +46,7 @@ use crate::{
traits::PayloadTxsBounds,
tx_signer::Signer,
};
use reth_monitor::{get_global_tracer, TransactionProcessId};

/// Container type that holds all necessities to build a new payload.
#[derive(Debug)]
Expand Down Expand Up @@ -326,9 +327,21 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
))
})?;

let tx_hash = sequencer_tx.hash();
let block_number = self.block_number();

let ResultAndState { result, state } = match evm.transact(&sequencer_tx) {
Ok(res) => res,
Err(err) => {
// Log transaction execution end even for failed transactions
if let Some(tracer) = get_global_tracer() {
tracer.log_transaction(
B256::from(*tx_hash),
TransactionProcessId::SeqTxExecutionEnd,
Some(block_number),
);
}

if err.is_invalid_tx_err() {
trace!(target: "payload_builder", %err, ?sequencer_tx, "Error in sequencer transaction, skipping.");
continue;
Expand Down Expand Up @@ -361,6 +374,15 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
// commit changes
evm.db_mut().commit(state);

// Log transaction execution end
if let Some(tracer) = get_global_tracer() {
tracer.log_transaction(
B256::from(*tx_hash),
TransactionProcessId::SeqTxExecutionEnd,
Some(block_number),
);
}

// append sender and transaction to the respective lists
info.executed_senders.push(sequencer_tx.signer());
info.executed_transactions.push(sequencer_tx.into_inner());
Expand Down Expand Up @@ -434,10 +456,20 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {

num_txs_considered += 1;

let block_number = self.block_number();

// TODO: ideally we should get this from the txpool stream
if let Some(conditional) = conditional
&& !conditional.matches_block_attributes(&block_attr)
{
// Log transaction execution end for rejected transactions
if let Some(tracer) = get_global_tracer() {
tracer.log_transaction(
B256::from(*tx_hash),
TransactionProcessId::SeqTxExecutionEnd,
Some(block_number),
);
}
best_txs.mark_invalid(tx.signer(), tx.nonce());
continue;
}
Expand All @@ -450,6 +482,14 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
&& !is_valid_interop(interop, self.config.attributes.timestamp())
{
log_txn(TxnExecutionResult::InteropFailed);
// Log transaction execution end for rejected transactions
if let Some(tracer) = get_global_tracer() {
tracer.log_transaction(
B256::from(*tx_hash),
TransactionProcessId::SeqTxExecutionEnd,
Some(block_number),
);
}
best_txs.mark_invalid(tx.signer(), tx.nonce());
continue;
}
Expand All @@ -469,13 +509,29 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
// invalid which also removes all dependent transaction from
// the iterator before we can continue
log_txn(result);
// Log transaction execution end for rejected transactions
if let Some(tracer) = get_global_tracer() {
tracer.log_transaction(
B256::from(*tx_hash),
TransactionProcessId::SeqTxExecutionEnd,
Some(block_number),
);
}
best_txs.mark_invalid(tx.signer(), tx.nonce());
continue;
}

// A sequencer's block should never contain blob or deposit transactions from the pool.
if tx.is_eip4844() || tx.is_deposit() {
log_txn(TxnExecutionResult::SequencerTransaction);
// Log transaction execution end for rejected transactions
if let Some(tracer) = get_global_tracer() {
tracer.log_transaction(
B256::from(*tx_hash),
TransactionProcessId::SeqTxExecutionEnd,
Some(block_number),
);
}
best_txs.mark_invalid(tx.signer(), tx.nonce());
continue;
}
Expand All @@ -489,6 +545,15 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
let ResultAndState { result, state } = match evm.transact(&tx) {
Ok(res) => res,
Err(err) => {
// Log transaction execution end for failed transactions
if let Some(tracer) = get_global_tracer() {
tracer.log_transaction(
B256::from(*tx_hash),
TransactionProcessId::SeqTxExecutionEnd,
Some(block_number),
);
}

if let Some(err) = err.as_invalid_tx_err() {
if err.is_nonce_too_low() {
// if the nonce is too low, we can skip this transaction
Expand Down Expand Up @@ -526,6 +591,14 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
.is_err()
{
log_txn(TxnExecutionResult::MaxGasUsageExceeded);
// Log transaction execution end for rejected transactions
if let Some(tracer) = get_global_tracer() {
tracer.log_transaction(
B256::from(*tx_hash),
TransactionProcessId::SeqTxExecutionEnd,
Some(block_number),
);
}
best_txs.mark_invalid(tx.signer(), tx.nonce());
continue;
}
Expand All @@ -547,6 +620,14 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
&& gas_used > max_gas_per_txn
{
log_txn(TxnExecutionResult::MaxGasUsageExceeded);
// Log transaction execution end for rejected transactions
if let Some(tracer) = get_global_tracer() {
tracer.log_transaction(
B256::from(*tx_hash),
TransactionProcessId::SeqTxExecutionEnd,
Some(block_number),
);
}
best_txs.mark_invalid(tx.signer(), tx.nonce());
continue;
}
Expand All @@ -568,6 +649,16 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
// commit changes
evm.db_mut().commit(state);

// Log transaction execution end (SeqTxExecutionEnd - ID: 15034)
let block_number = self.block_number();
if let Some(tracer) = get_global_tracer() {
tracer.log_transaction(
B256::from(*tx_hash),
TransactionProcessId::SeqTxExecutionEnd,
Some(block_number),
);
}

// update add to total fees
let miner_fee = tx
.effective_tip_per_gas(base_fee)
Expand Down
28 changes: 28 additions & 0 deletions crates/op-rbuilder/src/builders/flashblocks/payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,19 @@ where
cancel: block_cancel,
} = args;

// Log block build start
{
use reth_monitor::{get_global_tracer, TransactionProcessId};
let block_number = config.parent_header.number + 1;
if let Some(tracer) = get_global_tracer() {
tracer.log_block(
config.parent_header.hash(),
block_number,
TransactionProcessId::SeqBlockBuildStart,
);
}
}

// We log only every 100th block to reduce usage
let span = if cfg!(feature = "telemetry")
&& config
Expand Down Expand Up @@ -571,6 +584,21 @@ where
&resolve_payload,
)
.await;

// Log block build end
if let Some(payload) = resolve_payload.get() {
use reth_monitor::{get_global_tracer, TransactionProcessId};
let block_hash = payload.block().hash();
let block_number = payload.block().header().number;
if let Some(tracer) = get_global_tracer() {
tracer.log_block(
B256::from(*block_hash),
block_number,
TransactionProcessId::SeqBlockBuildEnd,
);
}
}
Comment on lines +589 to +600

Choose a reason for hiding this comment

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

This is incorrect. We should add this inside the resolve_best_payload function, after resolve_payload.set(payload) is set


self.record_flashblocks_metrics(
&ctx,
&info,
Expand Down
15 changes: 15 additions & 0 deletions crates/op-rbuilder/src/builders/flashblocks/payload_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,21 @@ where
let _ = p2p_tx.send(Message::from_flashblock_payload(payload)).await;
}
Some(payload) = built_payload_rx.recv() => {
// Log block send start
{
use reth_monitor::{get_global_tracer, TransactionProcessId};
use alloy_primitives::B256;
let block_hash = payload.block().hash();
let block_number = payload.block().header().number;
if let Some(tracer) = get_global_tracer() {
tracer.log_block(
B256::from(*block_hash),
block_number,
TransactionProcessId::SeqBlockSendStart,
);
}
}
Comment on lines +115 to +128

Choose a reason for hiding this comment

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

Remove this. This is unnecessary because you can add this log directly inside the reth repo instead, since this message will be handled on the engine state tree handler


// Update engine tree state with locally built block payloads
if let Err(e) = payload_events_handle.send(Events::BuiltPayload(payload.clone())) {
warn!(e = ?e, "failed to send BuiltPayload event");
Expand Down
22 changes: 22 additions & 0 deletions crates/op-rbuilder/src/builders/standard/payload.rs

Choose a reason for hiding this comment

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

Standard payload builders are not used and can completely be removed

Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ use reth_transaction_pool::{
use std::{sync::Arc, time::Instant};
use tokio_util::sync::CancellationToken;
use tracing::{error, info, warn};
use reth_monitor::{get_global_tracer, TransactionProcessId};
use alloy_primitives::B256;

/// Optimism's payload builder
#[derive(Debug, Clone)]
Expand Down Expand Up @@ -200,6 +202,16 @@ where
cancel,
} = args;

// Log block build start
let block_number = config.parent_header.number + 1;
if let Some(tracer) = get_global_tracer() {
tracer.log_block(
config.parent_header.hash(),
block_number,
TransactionProcessId::SeqBlockBuildStart,
);
}

let chain_spec = self.client.chain_spec();
let timestamp = config.attributes.timestamp();

Expand Down Expand Up @@ -584,6 +596,16 @@ impl<Txs: PayloadTxsBounds> OpBuilder<'_, Txs> {
let sealed_block = Arc::new(block.seal_slow());
info!(target: "payload_builder", id=%ctx.attributes().payload_id(), "sealed built block");

// Log block build end
let block_hash = sealed_block.hash();
if let Some(tracer) = get_global_tracer() {
tracer.log_block(
B256::from(*block_hash),
block_number,
TransactionProcessId::SeqBlockBuildEnd,
);
}

// create the executed block data
let executed = ExecutedBlock {
recovered_block: Arc::new(
Expand Down