Skip to content

add metrics to track gas used by reverting txs #273

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
merged 2 commits into from
Sep 30, 2025
Merged

add metrics to track gas used by reverting txs #273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6bed94a
remove metric covered by reth
akundaz Sep 22, 2025
3593773
track gas used by reverting txs
akundaz Sep 22, 2025
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
16 changes: 6 additions & 10 deletions crates/op-rbuilder/src/builders/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
let mut num_txs_simulated_success = 0;
let mut num_txs_simulated_fail = 0;
let mut num_bundles_reverted = 0;
let mut reverted_gas_used = 0;
let base_fee = self.base_fee();
let tx_da_limit = self.da_config.max_da_tx_size();
let mut evm = self.evm_config.evm_with_env(&mut *db, self.evm_env.clone());
Expand All @@ -337,15 +338,6 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
block_gas_limit = ?block_gas_limit,
);

// Remove once we merge Reth 1.4.4
// Fixed in https://github.com/paradigmxyz/reth/pull/16514
self.metrics
.da_block_size_limit
.set(block_da_limit.map_or(-1.0, |v| v as f64));
self.metrics
.da_tx_size_limit
.set(tx_da_limit.map_or(-1.0, |v| v as f64));

let block_attr = BlockConditionalAttributes {
number: self.block_number(),
timestamp: self.attributes().timestamp(),
Expand Down Expand Up @@ -451,7 +443,7 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
}
// this is an error that we should treat as fatal for this attempt
log_txn(TxnExecutionResult::EvmError);
return Err(PayloadBuilderError::EvmExecutionError(Box::new(err)));
return Err(PayloadBuilderError::evm(err));
}
};

Expand All @@ -478,8 +470,11 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
if result.is_success() {
log_txn(TxnExecutionResult::Success);
num_txs_simulated_success += 1;
self.metrics.successful_tx_gas_used.record(gas_used as f64);
Copy link
Collaborator

Choose a reason for hiding this comment

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

does this not need reverted_gas_used += gas_used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no i'm only tracking reverted gas when the tx reverts

Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry I meant successful_tx_gas_used += gas_used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that would calculate the cumulative gas used by successful txs in a block. reth already tracks this in the reth_sync_execution_gas_processed_total metrics. successful_tx_gas_used is a histogram metric so it will tell us what the distribution of gas looks like used by each tx

} else {
num_txs_simulated_fail += 1;
reverted_gas_used += gas_used as i32;
self.metrics.reverted_tx_gas_used.record(gas_used as f64);
if is_bundle_tx {
num_bundles_reverted += 1;
}
Expand Down Expand Up @@ -539,6 +534,7 @@ impl<ExtraCtx: Debug + Default> OpPayloadBuilderCtx<ExtraCtx> {
num_txs_simulated_success,
num_txs_simulated_fail,
num_bundles_reverted,
reverted_gas_used,
);

debug!(
Expand Down
13 changes: 9 additions & 4 deletions crates/op-rbuilder/src/metrics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,14 +123,16 @@ pub struct OpRBuilderMetrics {
pub payload_num_tx_simulated_fail: Histogram,
/// Latest number of transactions in the payload that failed simulation
pub payload_num_tx_simulated_fail_gauge: Gauge,
/// Histogram of gas used by successful transactions
pub successful_tx_gas_used: Histogram,
/// Histogram of gas used by reverted transactions
pub reverted_tx_gas_used: Histogram,
/// Gas used by reverted transactions in the latest block
pub payload_reverted_tx_gas_used: Gauge,
/// Histogram of tx simulation duration
pub tx_simulation_duration: Histogram,
/// Byte size of transactions
pub tx_byte_size: Histogram,
/// Da block size limit
pub da_block_size_limit: Gauge,
/// Da tx size limit
pub da_tx_size_limit: Gauge,
/// How much less flashblocks we issue to be on time with block construction
pub reduced_flashblocks_number: Histogram,
/// How much less flashblocks we issued in reality, comparing to calculated number for block
Expand All @@ -152,6 +154,7 @@ pub struct OpRBuilderMetrics {
}

impl OpRBuilderMetrics {
#[expect(clippy::too_many_arguments)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

can group these metrics as a struct instead of adding more args

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you mean a type where each field is one of these args? i'm not sure that's a good idea it would introduce more complexity than just having the directive to ignore the lint here

pub fn set_payload_builder_metrics(
&self,
payload_tx_simulation_time: impl IntoF64 + Copy,
Expand All @@ -160,6 +163,7 @@ impl OpRBuilderMetrics {
num_txs_simulated_success: impl IntoF64 + Copy,
num_txs_simulated_fail: impl IntoF64 + Copy,
num_bundles_reverted: impl IntoF64,
reverted_gas_used: impl IntoF64,
) {
self.payload_tx_simulation_duration
.record(payload_tx_simulation_time);
Expand All @@ -178,6 +182,7 @@ impl OpRBuilderMetrics {
self.payload_num_tx_simulated_fail_gauge
.set(num_txs_simulated_fail);
self.bundles_reverted.record(num_bundles_reverted);
self.payload_reverted_tx_gas_used.set(reverted_gas_used);
}
}

Expand Down
Loading