diff --git a/crates/pipe-exec-layer-ext-v2/execute/src/lib.rs b/crates/pipe-exec-layer-ext-v2/execute/src/lib.rs index 7e626a9c86..5ee2f2e294 100644 --- a/crates/pipe-exec-layer-ext-v2/execute/src/lib.rs +++ b/crates/pipe-exec-layer-ext-v2/execute/src/lib.rs @@ -855,14 +855,36 @@ impl Core { let validator_result = SystemTxnResult { result: execution_result, txn }; if !validator_result.result.is_success() { - error!(target: "execute_ordered_block", - index=?index, - is_dkg=?is_dkg, - block_number=?block_number, - gas_used=?validator_result.result.gas_used(), - output=?validator_result.result.output(), - "validator system transaction reverted" - ); + // Classify the revert by severity. A recoverable revert — e.g. a duplicate + // oracle attestation rejected by NativeOracle's sequential-nonce replay guard + // (NonceNotSequential) — is expected and must log at WARN, not ERROR, so it + // does not look like a failure or trip alerting. Unknown/fatal reverts stay ERROR. + let recoverable = validator_result + .result + .output() + .and_then(|out| onchain_config::errors::decode_revert_error(out)) + .is_some_and(|e| { + e.severity == onchain_config::errors::ErrorSeverity::Recoverable + }); + if recoverable { + warn!(target: "execute_ordered_block", + index=?index, + is_dkg=?is_dkg, + block_number=?block_number, + gas_used=?validator_result.result.gas_used(), + output=?validator_result.result.output(), + "validator system transaction reverted (recoverable)" + ); + } else { + error!(target: "execute_ordered_block", + index=?index, + is_dkg=?is_dkg, + block_number=?block_number, + gas_used=?validator_result.result.gas_used(), + output=?validator_result.result.output(), + "validator system transaction reverted" + ); + } } else { // DKG transactions may trigger epoch change if is_dkg { diff --git a/crates/pipe-exec-layer-ext-v2/execute/src/onchain_config/errors.rs b/crates/pipe-exec-layer-ext-v2/execute/src/onchain_config/errors.rs index 2538c0795a..3f26088694 100644 --- a/crates/pipe-exec-layer-ext-v2/execute/src/onchain_config/errors.rs +++ b/crates/pipe-exec-layer-ext-v2/execute/src/onchain_config/errors.rs @@ -53,7 +53,16 @@ sol! { error DKGNotInitialized(); // -------------------- NativeOracle Errors -------------------- - /// @notice Nonce must be strictly increasing for each source + /// @notice Oracle nonce must be sequential (== currentNonce + 1) for each source. + /// Thrown by NativeOracle._updateNonce when a duplicate / already-committed + /// attestation is replayed; matches NativeOracle.sol (selector 0x32e429cd). + /// This is what the currently deployed contract reverts with. + error NonceNotSequential(uint32 sourceType, uint256 sourceId, uint128 expectedNonce, uint128 providedNonce); + + /// @notice Legacy oracle-nonce error (selector 0xc778b1a4). The deployed contract + /// no longer throws this — `NonceNotSequential` superseded it — but it is kept in + /// the decode table so that a node running against an older contract build still + /// classifies the revert as Recoverable instead of falling through to ERROR. error NonceNotIncreasing(uint32 sourceType, uint256 sourceId, uint128 currentNonce, uint128 providedNonce); /// @notice Batch arrays have mismatched lengths @@ -114,6 +123,26 @@ pub enum SystemTxnExecutionResult { /// Decode a revert output into a known error type /// /// Uses 4-byte selector matching for O(1) lookup, then decodes only the matched error. +/// +/// Severity classification (drives the log level at the call sites — Fatal => ERROR, +/// Recoverable => WARN). Keep this list in sync with the match arms below: +/// +/// - **Fatal** (a real protocol violation / bug — should never happen in normal operation): +/// - `Unauthorized` — system call from a non-system caller. +/// - `TimestampMustAdvance` / `TimestampMustEqual` — block timestamp invariant broken. +/// - `ValidatorIndexOutOfBounds` — validator index outside the active set. +/// - `ReconfigurationNotInitialized` / `DKGNotInitialized` — required module not initialized. +/// - `OracleBatchArrayLengthMismatch` — malformed oracle batch (nonces/payloads/gas mismatched). +/// +/// - **Recoverable** (expected under benign races / replays — safe to ignore, logged at WARN): +/// - `ReconfigurationNotInProgress` / `ReconfigurationInProgress` — epoch-transition state race. +/// - `DKGNotInProgress` / `DKGInProgress` — DKG-session state race. +/// - `NonceNotSequential` — duplicate/already-committed oracle attestation rejected by the +/// sequential-nonce replay guard (current contract; selector 0x32e429cd). +/// - `NonceNotIncreasing` — legacy form of the same oracle-nonce replay rejection, superseded +/// by `NonceNotSequential`; kept for compatibility with older contract builds. +/// +/// An unknown selector returns `None`, which the call sites log at ERROR. pub fn decode_revert_error(output: &Bytes) -> Option { // Need at least 4 bytes for selector if output.len() < 4 { @@ -214,12 +243,27 @@ pub fn decode_revert_error(output: &Bytes) -> Option { severity: ErrorSeverity::Recoverable, }), + s if s == NonceNotSequential::SELECTOR => { + let err = NonceNotSequential::abi_decode(output).ok()?; + Some(SystemTxnError { + name: "NonceNotSequential".into(), + details: format!( + "Oracle nonce not sequential (duplicate/replayed attestation rejected by the nonce guard): sourceType={}, sourceId={}, expected={}, provided={}", + err.sourceType, err.sourceId, err.expectedNonce, err.providedNonce + ), + severity: ErrorSeverity::Recoverable, + }) + } + + // Legacy oracle-nonce error. The deployed contract emits NonceNotSequential + // instead, but keep this arm so a node run against an older contract build + // still classifies the replay rejection as Recoverable. s if s == NonceNotIncreasing::SELECTOR => { let err = NonceNotIncreasing::abi_decode(output).ok()?; Some(SystemTxnError { name: "NonceNotIncreasing".into(), details: format!( - "Oracle nonce not increasing: sourceType={}, sourceId={}, current={}, provided={}", + "Oracle nonce not increasing (legacy; superseded by NonceNotSequential): sourceType={}, sourceId={}, current={}, provided={}", err.sourceType, err.sourceId, err.currentNonce, err.providedNonce ), severity: ErrorSeverity::Recoverable, @@ -318,7 +362,27 @@ mod tests { } #[test] - fn test_decode_nonce_not_increasing() { + fn test_decode_nonce_not_sequential() { + // expected = currentNonce + 1, provided = currentNonce: the duplicate-attestation + // replay shape seen in production (selector 0x32e429cd, classified Recoverable). + let error = NonceNotSequential { + sourceType: 0, + sourceId: alloy_primitives::U256::from(1), + expectedNonce: 11, + providedNonce: 10, + }; + let encoded = error.abi_encode(); + let result = decode_revert_error(&encoded.into()); + + assert!(result.is_some()); + let err = result.unwrap(); + assert_eq!(err.name, "NonceNotSequential"); + assert_eq!(err.severity, ErrorSeverity::Recoverable); + } + + #[test] + fn test_decode_nonce_not_increasing_legacy() { + // Legacy selector (0xc778b1a4), kept for older contract builds; still Recoverable. let error = NonceNotIncreasing { sourceType: 1, sourceId: alloy_primitives::U256::from(42),