Skip to content

Commit 477ee31

Browse files
committed
op-reth: meter PostExec structural validation failures
1 parent 9104d73 commit 477ee31

6 files changed

Lines changed: 195 additions & 3 deletions

File tree

rust/Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rust/op-reth/crates/evm/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,11 @@ revm.workspace = true
4343
op-revm.workspace = true
4444

4545
# misc
46+
metrics = { workspace = true, optional = true }
4647
thiserror.workspace = true
4748

4849
[dev-dependencies]
50+
metrics-util = { workspace = true, features = ["debugging"] }
4951
reth-evm = { workspace = true, features = ["test-utils"] }
5052
reth-revm = { workspace = true, features = ["test-utils"] }
5153
alloy-genesis.workspace = true
@@ -56,6 +58,7 @@ reth-optimism-chainspec = { workspace = true, features = ["superchain-configs"]
5658
[features]
5759
default = ["std"]
5860
std = [
61+
"dep:metrics",
5962
"reth-revm/std",
6063
"alloy-consensus/std",
6164
"alloy-eips/std",

rust/op-reth/crates/evm/src/lib.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ pub use build::OpBlockAssembler;
6363
mod error;
6464
pub use error::{L1BlockInfoError, OpBlockExecutionError};
6565

66+
pub mod metrics;
67+
6668
pub mod tx;
6769
pub use tx::OpTx;
6870

@@ -163,7 +165,13 @@ where
163165
T: OpConsensusTransaction + 'a,
164166
{
165167
parse_post_exec_payload_from_transactions(transactions, block_number, sdm_active)
166-
.map_err(|_| EIP1559ParamError::InvalidPostExecPayload)
168+
.map_err(|error| {
169+
#[cfg(feature = "std")]
170+
metrics::record_post_exec_validation_failure((&error).into());
171+
#[cfg(not(feature = "std"))]
172+
let _ = error;
173+
EIP1559ParamError::InvalidPostExecPayload
174+
})
167175
.map(|parsed| {
168176
parsed.map_or_else(PostExecMode::default, |parsed| PostExecMode::Verify(parsed.payload))
169177
})
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
//! Metrics for Optimism-specific EVM validation.
2+
3+
#[cfg(feature = "std")]
4+
use metrics::{counter, describe_counter};
5+
use op_alloy_consensus::PostExecPayloadValidationError;
6+
7+
/// Counter incremented when an execution-client path rejects `PostExec` block structure.
8+
pub const POST_EXEC_VALIDATION_FAILURES: &str = "optimism_post_exec.validation_failures";
9+
10+
/// Stable, bounded reason labels for `PostExec` structural validation failures.
11+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
12+
pub enum PostExecValidationFailureReason {
13+
/// The `0x7D` transaction or its schema could not be decoded.
14+
InvalidEncodingOrSchema,
15+
/// A block contained `0x7D` before SDM activation.
16+
SdmInactive,
17+
/// A block contained more than one `0x7D` transaction.
18+
MultipleTransactions,
19+
/// The `0x7D` transaction was not the final transaction.
20+
NotLast,
21+
/// The payload block number did not match the containing block.
22+
BlockNumberMismatch,
23+
}
24+
25+
impl PostExecValidationFailureReason {
26+
/// Returns the stable metric label for this failure reason.
27+
pub const fn as_str(self) -> &'static str {
28+
match self {
29+
Self::InvalidEncodingOrSchema => "invalid_encoding_or_schema",
30+
Self::SdmInactive => "sdm_inactive",
31+
Self::MultipleTransactions => "multiple_transactions",
32+
Self::NotLast => "not_last",
33+
Self::BlockNumberMismatch => "block_number_mismatch",
34+
}
35+
}
36+
}
37+
38+
impl From<&PostExecPayloadValidationError> for PostExecValidationFailureReason {
39+
fn from(error: &PostExecPayloadValidationError) -> Self {
40+
match error {
41+
PostExecPayloadValidationError::UnexpectedPostExecTx { .. } => Self::SdmInactive,
42+
PostExecPayloadValidationError::MultiplePostExecTxs { .. } => {
43+
Self::MultipleTransactions
44+
}
45+
PostExecPayloadValidationError::PostExecTxNotLast { .. } => Self::NotLast,
46+
PostExecPayloadValidationError::BlockNumberMismatch { .. } => Self::BlockNumberMismatch,
47+
}
48+
}
49+
}
50+
51+
/// Records a `PostExec` structural validation failure.
52+
///
53+
/// Callers must classify failures with [`PostExecValidationFailureReason`] so the `reason` label
54+
/// remains low-cardinality.
55+
pub fn record_post_exec_validation_failure(reason: PostExecValidationFailureReason) {
56+
#[cfg(feature = "std")]
57+
{
58+
describe_counter!(
59+
POST_EXEC_VALIDATION_FAILURES,
60+
"PostExec structural validation failures by reason"
61+
);
62+
counter!(POST_EXEC_VALIDATION_FAILURES, "reason" => reason.as_str()).increment(1);
63+
}
64+
#[cfg(not(feature = "std"))]
65+
let _ = reason;
66+
}
67+
68+
#[cfg(all(test, feature = "std"))]
69+
mod tests {
70+
use super::*;
71+
use metrics_util::debugging::{DebugValue, DebuggingRecorder, Snapshot};
72+
73+
fn failure_count(snapshot: Snapshot, reason: PostExecValidationFailureReason) -> u64 {
74+
snapshot
75+
.into_vec()
76+
.into_iter()
77+
.find_map(|(composite_key, _, _, value)| {
78+
let key = composite_key.key();
79+
let matches = key.name() == POST_EXEC_VALIDATION_FAILURES &&
80+
key.labels().any(|label| {
81+
label.key() == "reason" && label.value() == reason.as_str()
82+
});
83+
matches.then_some(match value {
84+
DebugValue::Counter(value) => value,
85+
_ => 0,
86+
})
87+
})
88+
.unwrap_or_default()
89+
}
90+
91+
#[test]
92+
fn records_reason_labeled_post_exec_validation_failure() {
93+
let recorder = DebuggingRecorder::new();
94+
let snapshotter = recorder.snapshotter();
95+
metrics::with_local_recorder(&recorder, || {
96+
record_post_exec_validation_failure(
97+
PostExecValidationFailureReason::BlockNumberMismatch,
98+
);
99+
});
100+
101+
assert_eq!(
102+
failure_count(
103+
snapshotter.snapshot(),
104+
PostExecValidationFailureReason::BlockNumberMismatch
105+
),
106+
1
107+
);
108+
}
109+
110+
#[test]
111+
fn maps_structural_errors_to_stable_reasons() {
112+
for (error, expected) in [
113+
(
114+
PostExecPayloadValidationError::UnexpectedPostExecTx { tx_index: 0 },
115+
PostExecValidationFailureReason::SdmInactive,
116+
),
117+
(
118+
PostExecPayloadValidationError::MultiplePostExecTxs {
119+
first_index: 0,
120+
duplicate_index: 1,
121+
},
122+
PostExecValidationFailureReason::MultipleTransactions,
123+
),
124+
(
125+
PostExecPayloadValidationError::PostExecTxNotLast { tx_index: 0, last_index: 1 },
126+
PostExecValidationFailureReason::NotLast,
127+
),
128+
(
129+
PostExecPayloadValidationError::BlockNumberMismatch {
130+
payload_block_number: 1,
131+
block_number: 2,
132+
},
133+
PostExecValidationFailureReason::BlockNumberMismatch,
134+
),
135+
] {
136+
assert_eq!(PostExecValidationFailureReason::from(&error), expected);
137+
}
138+
}
139+
}

rust/op-reth/crates/node/src/engine.rs

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use alloy_consensus::BlockHeader;
2-
use alloy_primitives::B256;
2+
use alloy_primitives::{B256, Bytes};
33
use alloy_rpc_types_engine::{ExecutionPayloadEnvelopeV2, ExecutionPayloadV1};
4+
use op_alloy_consensus::{POST_EXEC_TX_TYPE_ID, PostExecPayload};
45
use op_alloy_rpc_types_engine::{
56
OpExecutionData, OpExecutionPayloadEnvelope, OpExecutionPayloadEnvelopeV3,
67
OpExecutionPayloadEnvelopeV4,
@@ -17,6 +18,9 @@ use reth_node_api::{
1718
validate_version_specific_fields,
1819
};
1920
use reth_optimism_consensus::isthmus;
21+
use reth_optimism_evm::metrics::{
22+
PostExecValidationFailureReason, record_post_exec_validation_failure,
23+
};
2024
use reth_optimism_forks::OpHardforks;
2125
use reth_optimism_payload_builder::{
2226
OpExecData, OpExecutionPayloadValidator, OpPayloadAttrs, OpPayloadTypes,
@@ -123,6 +127,14 @@ where
123127
}
124128
}
125129

130+
fn has_invalid_post_exec_encoding(transactions: &[Bytes]) -> bool {
131+
transactions.iter().any(|encoded| {
132+
let bytes = encoded.as_ref();
133+
bytes.first() == Some(&POST_EXEC_TX_TYPE_ID) &&
134+
PostExecPayload::from_rlp_bytes(&bytes[1..]).is_err()
135+
})
136+
}
137+
126138
impl<P, Tx, ChainSpec, Types> PayloadValidator<Types> for OpEngineValidator<P, Tx, ChainSpec>
127139
where
128140
P: StateProviderFactory + Unpin + 'static,
@@ -162,6 +174,11 @@ where
162174
&self,
163175
payload: OpExecData,
164176
) -> Result<SealedBlock<Self::Block>, NewPayloadError> {
177+
if has_invalid_post_exec_encoding(payload.0.payload.transactions()) {
178+
record_post_exec_validation_failure(
179+
PostExecValidationFailureReason::InvalidEncodingOrSchema,
180+
);
181+
}
165182
self.inner.ensure_well_formed_payload(payload.0).map_err(NewPayloadError::other)
166183
}
167184
}
@@ -335,6 +352,25 @@ mod test {
335352
}};
336353
}
337354

355+
#[test]
356+
fn detects_invalid_post_exec_encoding_or_schema() {
357+
let valid_payload = PostExecPayload {
358+
version: op_alloy_consensus::POST_EXEC_PAYLOAD_VERSION,
359+
block_number: 1,
360+
gas_refund_entries: vec![],
361+
};
362+
let mut valid = vec![POST_EXEC_TX_TYPE_ID];
363+
valid.extend_from_slice(&valid_payload.to_rlp_bytes());
364+
365+
let mut invalid_schema = vec![POST_EXEC_TX_TYPE_ID];
366+
invalid_schema
367+
.extend_from_slice(&PostExecPayload { version: 2, ..valid_payload }.to_rlp_bytes());
368+
369+
assert!(!has_invalid_post_exec_encoding(&[Bytes::from(valid)]));
370+
assert!(has_invalid_post_exec_encoding(&[Bytes::from(invalid_schema)]));
371+
assert!(!has_invalid_post_exec_encoding(&[Bytes::from_static(&[0x02, 0x01])]));
372+
}
373+
338374
fn get_attributes(
339375
eip_1559_params: Option<B64>,
340376
min_base_fee: Option<u64>,

rust/op-reth/crates/payload/src/builder.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use reth_metrics::{
2929
};
3030
use reth_optimism_evm::{
3131
ConfigurePostExecEvm, PostExecExecutorExt, PostExecMode, PreRefundGasUsed,
32+
metrics::record_post_exec_validation_failure,
3233
};
3334
use reth_optimism_forks::OpHardforks;
3435
use reth_optimism_primitives::{L2_TO_L1_MESSAGE_PASSER_ADDRESS, OpTransaction};
@@ -899,7 +900,10 @@ where
899900
next_block_number,
900901
sdm_active,
901902
)
902-
.map_err(PayloadBuilderError::other)
903+
.map_err(|error| {
904+
record_post_exec_validation_failure((&error).into());
905+
PayloadBuilderError::other(error)
906+
})
903907
}
904908

905909
/// Decides this payload's SDM post-exec mode: *produce*, *verify*, or `Disabled`.

0 commit comments

Comments
 (0)