diff --git a/crates/e2e/src/tests/payload_builder.rs b/crates/e2e/src/tests/payload_builder.rs index dd0d3bbfb3..1a07107481 100644 --- a/crates/e2e/src/tests/payload_builder.rs +++ b/crates/e2e/src/tests/payload_builder.rs @@ -1,4 +1,7 @@ -use std::time::Duration; +use std::{ + sync::{Mutex, MutexGuard}, + time::Duration, +}; use commonware_macros::test_traced; use commonware_runtime::{ @@ -14,16 +17,30 @@ const PAYLOAD_FINALIZATION_COUNT_METRIC: &str = "reth_tempo_payload_builder_payload_finalization_duration_seconds_count"; const STATE_ROOT_WITH_UPDATES_COUNT_METRIC: &str = "reth_tempo_payload_builder_state_root_with_updates_duration_seconds_count"; +const POOL_TRANSACTIONS_YIELDED_COUNT_METRIC: &str = + "reth_tempo_payload_builder_pool_transactions_yielded_count"; +const POOL_TRANSACTIONS_INCLUDED_COUNT_METRIC: &str = + "reth_tempo_payload_builder_pool_transactions_included_count"; +const POOL_TRANSACTIONS_INCLUSION_RATIO_COUNT_METRIC: &str = + "reth_tempo_payload_builder_pool_transactions_inclusion_ratio_count"; +const POOL_TRANSACTIONS_INCLUSION_RATIO_LAST_METRIC: &str = + "reth_tempo_payload_builder_pool_transactions_inclusion_ratio_last"; const NULLIFICATIONS_PER_LEADER_METRIC_SUFFIX: &str = "_nullifications_per_leader"; +// These tests compute deltas from the process-global Prometheus recorder, so +// running them concurrently lets one test observe the other's payload-builder metrics. +static PAYLOAD_BUILDER_TEST_LOCK: Mutex<()> = Mutex::new(()); + #[test_traced] fn shared_sparse_trie_single_validator_bypasses_sync_state_root() { + let _guard = payload_builder_test_lock(); let deltas = run_payload_builder_test(&[true], 10); assert!( deltas.finalization_count > 0, "expected payload builder finalization metrics to increase" ); + assert_pool_inclusion_metrics(&deltas); assert_eq!( deltas.state_root_count, 0, "expected shared sparse trie to bypass sync state-root work" @@ -32,14 +49,22 @@ fn shared_sparse_trie_single_validator_bypasses_sync_state_root() { #[test_traced] fn mixed_validators_build_blocks_with_and_without_shared_sparse_trie_payload_builder() { + let _guard = payload_builder_test_lock(); let deltas = run_payload_builder_test(&[true, false], 10); + assert_pool_inclusion_metrics(&deltas); assert_eq!( deltas.nullification_count, 0, "expected mixed sparse trie configuration to build without consensus nullifications" ); } +fn payload_builder_test_lock() -> MutexGuard<'static, ()> { + PAYLOAD_BUILDER_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) +} + fn run_payload_builder_test( share_sparse_trie_with_payload_builder: &[bool], target_height: u64, @@ -50,6 +75,14 @@ fn run_payload_builder_test( prometheus_histogram_count(metrics_recorder, PAYLOAD_FINALIZATION_COUNT_METRIC); let initial_state_root_count = prometheus_histogram_count(metrics_recorder, STATE_ROOT_WITH_UPDATES_COUNT_METRIC); + let initial_pool_transactions_yielded_count = + prometheus_histogram_count(metrics_recorder, POOL_TRANSACTIONS_YIELDED_COUNT_METRIC); + let initial_pool_transactions_included_count = + prometheus_histogram_count(metrics_recorder, POOL_TRANSACTIONS_INCLUDED_COUNT_METRIC); + let initial_pool_transactions_inclusion_ratio_count = prometheus_histogram_count( + metrics_recorder, + POOL_TRANSACTIONS_INCLUSION_RATIO_COUNT_METRIC, + ); let nullification_count = Runner::from(Config::default().with_seed(0)).start(|mut context| async move { @@ -84,10 +117,29 @@ fn run_payload_builder_test( prometheus_histogram_count(metrics_recorder, PAYLOAD_FINALIZATION_COUNT_METRIC); let final_state_root_count = prometheus_histogram_count(metrics_recorder, STATE_ROOT_WITH_UPDATES_COUNT_METRIC); + let final_pool_transactions_yielded_count = + prometheus_histogram_count(metrics_recorder, POOL_TRANSACTIONS_YIELDED_COUNT_METRIC); + let final_pool_transactions_included_count = + prometheus_histogram_count(metrics_recorder, POOL_TRANSACTIONS_INCLUDED_COUNT_METRIC); + let final_pool_transactions_inclusion_ratio_count = prometheus_histogram_count( + metrics_recorder, + POOL_TRANSACTIONS_INCLUSION_RATIO_COUNT_METRIC, + ); + let pool_transactions_inclusion_ratio_last = prometheus_metric_value( + metrics_recorder, + POOL_TRANSACTIONS_INCLUSION_RATIO_LAST_METRIC, + ); MetricDelta { finalization_count: final_finalization_count - initial_finalization_count, state_root_count: final_state_root_count - initial_state_root_count, + pool_transactions_yielded_count: final_pool_transactions_yielded_count + - initial_pool_transactions_yielded_count, + pool_transactions_included_count: final_pool_transactions_included_count + - initial_pool_transactions_included_count, + pool_transactions_inclusion_ratio_count: final_pool_transactions_inclusion_ratio_count + - initial_pool_transactions_inclusion_ratio_count, + pool_transactions_inclusion_ratio_last, nullification_count, } } @@ -95,9 +147,36 @@ fn run_payload_builder_test( struct MetricDelta { finalization_count: u64, state_root_count: u64, + pool_transactions_yielded_count: u64, + pool_transactions_included_count: u64, + pool_transactions_inclusion_ratio_count: u64, + pool_transactions_inclusion_ratio_last: Option, nullification_count: u64, } +fn assert_pool_inclusion_metrics(deltas: &MetricDelta) { + assert!( + deltas.pool_transactions_yielded_count > 0, + "expected pool transactions yielded metric to be recorded" + ); + assert!( + deltas.pool_transactions_included_count > 0, + "expected pool transactions included metric to be recorded" + ); + assert!( + deltas.pool_transactions_inclusion_ratio_count > 0, + "expected pool transactions inclusion ratio metric to be recorded" + ); + + let ratio = deltas + .pool_transactions_inclusion_ratio_last + .expect("expected pool transactions inclusion ratio last metric to be exported"); + assert!( + (0.0..=1.0).contains(&ratio), + "expected pool transactions inclusion ratio last to be within 0.0..=1.0, got {ratio}" + ); +} + async fn wait_for_height(context: &Context, expected_validators: u32, target_height: u64) { loop { let validators_at_height = context @@ -151,3 +230,11 @@ fn prometheus_histogram_count(recorder: &PrometheusRecorder, metric: &str) -> u6 }) .unwrap_or(0) } + +fn prometheus_metric_value(recorder: &PrometheusRecorder, metric: &str) -> Option { + recorder.handle().run_upkeep(); + recorder.handle().render().lines().find_map(|line| { + let mut parts = line.split_whitespace(); + (parts.next()? == metric).then(|| parts.next()?.parse().ok())? + }) +} diff --git a/crates/payload/builder/src/lib.rs b/crates/payload/builder/src/lib.rs index 7763b3574f..6d6196fd59 100644 --- a/crates/payload/builder/src/lib.rs +++ b/crates/payload/builder/src/lib.rs @@ -320,6 +320,8 @@ where .unwrap_or(0) + 1024; let mut payment_transactions = 0u64; + let mut pool_transactions_yielded = 0u64; + let mut pool_transactions_included = 0u64; let mut total_fees = U256::ZERO; // If building an empty payload, don't include any subblocks @@ -451,6 +453,7 @@ where } break; }; + pool_transactions_yielded += 1; let max_regular_gas_used = core::cmp::min( pool_tx.gas_limit(), @@ -595,6 +598,7 @@ where .record(elapsed); trace!(?elapsed, "Transaction executed"); + pool_transactions_included += 1; block_size_used += tx_rlp_length; } drop(_block_fill_span); @@ -800,6 +804,30 @@ where })); } + let pool_transactions_inclusion_ratio = if pool_transactions_yielded == 0 { + 0.0 + } else { + pool_transactions_included as f64 / pool_transactions_yielded as f64 + }; + self.metrics + .pool_transactions_yielded + .record(pool_transactions_yielded as f64); + self.metrics + .pool_transactions_yielded_last + .set(pool_transactions_yielded as f64); + self.metrics + .pool_transactions_included + .record(pool_transactions_included as f64); + self.metrics + .pool_transactions_included_last + .set(pool_transactions_included as f64); + self.metrics + .pool_transactions_inclusion_ratio + .record(pool_transactions_inclusion_ratio); + self.metrics + .pool_transactions_inclusion_ratio_last + .set(pool_transactions_inclusion_ratio); + let elapsed = start.elapsed(); self.metrics.payload_build_duration_seconds.record(elapsed); let gas_per_second = sealed_block.gas_used() as f64 / elapsed.as_secs_f64(); @@ -821,6 +849,9 @@ where extra_data = %sealed_block.extra_data(), subblocks_count, payment_transactions, + pool_transactions_yielded, + pool_transactions_included, + pool_transactions_inclusion_ratio, subblock_transactions, total_transactions, ?elapsed, diff --git a/crates/payload/builder/src/metrics.rs b/crates/payload/builder/src/metrics.rs index fa4ec1e743..7feb4935e2 100644 --- a/crates/payload/builder/src/metrics.rs +++ b/crates/payload/builder/src/metrics.rs @@ -29,6 +29,18 @@ pub(crate) struct TempoPayloadBuilderMetrics { pub(crate) payment_transactions: Histogram, /// Number of payment transactions in the payload. pub(crate) payment_transactions_last: Gauge, + /// Number of pool transactions yielded by the best transactions iterator. + pub(crate) pool_transactions_yielded: Histogram, + /// Number of pool transactions yielded by the best transactions iterator for the last payload. + pub(crate) pool_transactions_yielded_last: Gauge, + /// Number of yielded pool transactions included in the payload. + pub(crate) pool_transactions_included: Histogram, + /// Number of yielded pool transactions included in the last payload. + pub(crate) pool_transactions_included_last: Gauge, + /// Ratio of yielded pool transactions that were included in the payload. + pub(crate) pool_transactions_inclusion_ratio: Histogram, + /// Ratio of yielded pool transactions that were included in the last payload. + pub(crate) pool_transactions_inclusion_ratio_last: Gauge, /// Number of subblocks in the payload. pub(crate) subblocks: Histogram, /// Number of subblocks in the payload.