diff --git a/crates/composable-cow/src/fork.rs b/crates/composable-cow/src/fork.rs index 026c89a0..ba3ab230 100644 --- a/crates/composable-cow/src/fork.rs +++ b/crates/composable-cow/src/fork.rs @@ -212,34 +212,47 @@ sol! { } } -/// Classify a failed poll `eth_call`. Every reachable revert is -/// deterministic on-chain state, so all are terminal; a re-`create` -/// re-indexes through its own event. A payload-free failure is the -/// transport, not the contract, so it stays retryable. +/// What a poll revert says about the commitment. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Refusal { + /// The contract refused with a selector this build can read. + Named(Selector), + /// The node executed the call and refused with nothing readable. + /// + /// The poll interface is closed: a generator answers through + /// `GeneratorResult` codes and the registry's own refusals carry + /// selectors. An empty payload is outside that, so it names a + /// defect. It does not say whose, and the candidates are not alike: + /// a codeless owner is the registration's, a gas cap too low for the + /// handler or a wrong registry address is the operator's. + Unattributed, + /// The call never reached the node. + Transport, +} + +/// Classify a poll revert. Attribution of [`Refusal::Unattributed`] +/// needs a second chain read, so it happens at the call site. #[must_use] -pub fn classify_revert(err: &ChainError) -> Verdict { +pub fn classify_revert(err: &ChainError) -> Refusal { let ChainError::Rpc(rpc) = err else { - // `ChainError` is `#[non_exhaustive]`: transport faults and any - // future case are payload-free, so they stay retryable. - return Verdict::TryNextBlock { - reason: Selector::ZERO, - }; - }; - let Some(data) = rpc.data.as_deref() else { - return Verdict::TryNextBlock { - reason: Selector::ZERO, - }; + return Refusal::Transport; }; - let Some(reason) = data.get(..4).map(Selector::from_slice) else { - return Verdict::TryNextBlock { - reason: Selector::ZERO, - }; - }; - // Unrecognised still means the contract refused; a handler `Panic` - // lands here. - Verdict::Invalid { reason } + rpc.data + .as_deref() + .and_then(|data| data.get(..4)) + .map_or(Refusal::Unattributed, |s| { + Refusal::Named(Selector::from_slice(s)) + }) } +/// How long a payload-free revert takes the commitment out of the +/// rotation. +/// +/// Long enough that the poll budget is not spent on a deterministic +/// failure, short enough that a commitment recovers on its own if the +/// cause was the node rather than the registration. +pub const PAYLOAD_FREE_BACKOFF_S: u64 = 3_600; + /// Selectors the classifier recognises, for logging and tests. #[must_use] pub fn is_residual_selector(selector: Selector) -> bool { @@ -591,7 +604,7 @@ mod tests { mod residual_tests { use alloy_primitives::fixed_bytes; use alloy_sol_types::SolError; - use nexum_sdk::host::RpcError; + use nexum_sdk::host::{Fault, RpcError}; use super::*; @@ -614,11 +627,8 @@ mod residual_tests { ] .map(Selector::from) { - let verdict = classify_revert(&reverted(Some(selector.to_vec()))); - assert!( - matches!(verdict, Verdict::Invalid { reason } if reason == selector), - "{selector:?} produced {verdict:?}", - ); + let refusal = classify_revert(&reverted(Some(selector.to_vec()))); + assert_eq!(refusal, Refusal::Named(selector), "{selector:?}"); assert!(is_residual_selector(selector)); } } @@ -628,22 +638,33 @@ mod residual_tests { fn an_unrecognised_selector_is_terminal() { let panic_selector = Selector::new([0x4e, 0x48, 0x7b, 0x71]); assert!(!is_residual_selector(panic_selector)); - assert!(matches!( + assert_eq!( classify_revert(&reverted(Some(panic_selector.to_vec()))), - Verdict::Invalid { .. } - )); + Refusal::Named(panic_selector), + ); } + /// The closed poll interface specifies neither of these, so both + /// name a defect the caller has to attribute before acting. #[test] - fn a_payload_free_failure_stays_retryable() { - assert!(matches!( - classify_revert(&reverted(None)), - Verdict::TryNextBlock { .. } - )); - assert!(matches!( - classify_revert(&reverted(Some(vec![1, 2]))), - Verdict::TryNextBlock { .. } - )); + fn an_unreadable_payload_is_not_attributed_here() { + for data in [None, Some(vec![1, 2])] { + assert_eq!( + classify_revert(&reverted(data.clone())), + Refusal::Unattributed, + "{data:?}", + ); + } + } + + /// A transport fault never reached the node, so it says nothing + /// about the contract. + #[test] + fn a_transport_fault_is_its_own_class() { + assert_eq!( + classify_revert(&ChainError::Fault(Fault::Unavailable("node down".into()))), + Refusal::Transport, + ); } /// A rename upstream must fail here, not silently reclassify a diff --git a/crates/composable-cow/src/lib.rs b/crates/composable-cow/src/lib.rs index b56da0a9..5add6f41 100644 --- a/crates/composable-cow/src/lib.rs +++ b/crates/composable-cow/src/lib.rs @@ -16,7 +16,7 @@ pub mod poll; #[cfg(feature = "run")] pub mod run; -pub use fork::{Mapped, PollResult, Suppressed, classify_revert, map_verdict, to_verdict}; +pub use fork::{Mapped, PollResult, Refusal, Suppressed, classify_revert, map_verdict, to_verdict}; pub use poll::{NextPoll, Verdict}; #[cfg(feature = "run")] pub use run::run; diff --git a/modules/ccow-monitor/src/keeper.rs b/modules/ccow-monitor/src/keeper.rs index 9f1fcea4..ce0daf7e 100644 --- a/modules/ccow-monitor/src/keeper.rs +++ b/modules/ccow-monitor/src/keeper.rs @@ -11,7 +11,9 @@ use alloy_primitives::{Address, B256, Bytes, Selector, keccak256}; use alloy_sol_types::{SolCall, SolEvent, SolValue}; use composable::ConditionalOrderParams; -use composable_cow::fork::{classify_revert, decode_poll_return, map_verdict, to_verdict}; +use composable_cow::fork::{ + PAYLOAD_FREE_BACKOFF_S, Refusal, classify_revert, decode_poll_return, map_verdict, to_verdict, +}; use composable_cow::{Verdict, run}; use cow_venue::CowClient; // The poll path receives the order inside `PollResult`, so the bare type @@ -628,31 +630,72 @@ fn poll_one( to_verdict(map_verdict(&result, &signature), valid_to) }, ), - Err(err) => { - let outcome = classify_revert(&err); - match &err { - ChainError::Fault(fault) => { - tracing::warn!("eth_call failed ({fault}); retrying next block"); + Err(err) => match classify_revert(&err) { + Refusal::Transport => { + tracing::warn!("eth_call failed ({err}); retrying next block"); + Verdict::TryNextBlock { + reason: Selector::ZERO, } - // A permanent drop deserves its cause on the record: the - // selector and the node's message are unrecoverable once - // the commitment is gone. - ChainError::Rpc(rpc) if matches!(outcome, Verdict::Invalid { .. }) => { - let selector = rpc - .data - .as_deref() - .and_then(|data| data.get(..4)) - .map(|s| format!("{:#x}", Selector::from_slice(s))) - .unwrap_or_else(|| "none".to_string()); - tracing::warn!( - "eth_call reverted permanently (selector {selector}, {}); \ - dropping commitment", - rpc.message, - ); - } - _ => {} } - outcome + // A permanent drop deserves its cause on the record: the + // selector is unrecoverable once the commitment is gone. + Refusal::Named(reason) => { + tracing::warn!( + "eth_call reverted permanently (selector {reason:#x}, {err}); \ + dropping commitment" + ); + Verdict::Invalid { reason } + } + Refusal::Unattributed => attribute_unreadable(host, tick, owner, &err), + }, + } +} + +/// Decide who an unreadable refusal belongs to. +/// +/// The registry probes the owner for ERC-1271 support before it builds a +/// signature, and a codeless owner makes that probe revert in the +/// caller's own frame with nothing attached. So an empty payload plus an +/// owner with no code is the registration answering for itself, and no +/// later poll of this commitment changes it. +/// +/// Every other cause points outward, at a gas cap too low for the +/// handler or a registry address that is not the fork. Those are the +/// operator's, they are the same for every commitment, and removing a +/// watch set is not how to report them. +fn attribute_unreadable( + host: &H, + tick: &Tick, + owner: &Address, + err: &ChainError, +) -> Verdict { + let params = format!(r#"["{owner:#x}","latest"]"#); + let code = host + .request(tick.chain_id, "eth_getCode", ¶ms) + .ok() + .and_then(|json| parse_eth_call_result(&json)); + match code.as_deref() { + Some([]) => { + tracing::error!( + "dropping commitment: owner {owner:#x} has no code, so the registry cannot \ + build an ERC-1271 signature for it ({err})" + ); + Verdict::Invalid { + reason: Selector::ZERO, + } + } + // Includes the read failing: without an answer this is not + // attributable, and the safe reading of that is the loud one. + _ => { + tracing::error!( + "poll refused with no readable payload and owner {owner:#x} has code; \ + check the poll gas cap and the registry address ({err}); \ + backing off {PAYLOAD_FREE_BACKOFF_S}s" + ); + Verdict::WaitTimestamp { + wait_until: tick.epoch_s.saturating_add(PAYLOAD_FREE_BACKOFF_S), + reason: Selector::ZERO, + } } } } @@ -1873,6 +1916,90 @@ mod tests { assert!(!store.keys().any(|k| k.starts_with("submitted:"))); } + /// A codeless owner cannot answer the registry's ERC-1271 probe, so + /// the poll reverts in the caller's own frame with nothing attached. + /// That is the registration answering for itself. + #[test] + fn an_unreadable_refusal_with_a_codeless_owner_drops() { + use nexum_sdk::host::RpcError; + + let host = MockHost::new(); + let venue = MockVenue::default(); + let owner = address!("0011223344556677889900AABBCCDDEEFF001122"); + let params = sample_params(); + let key = seed_commitment(&host, owner, ¶ms); + + host.chain.respond_to( + "eth_call", + programmed_eth_call_params(owner, ¶ms), + Err(ChainError::Rpc(RpcError { + code: 3, + message: "execution reverted".into(), + data: None, + })), + ); + host.chain.respond_to( + "eth_getCode", + format!(r#"["{owner:#x}","latest"]"#), + Ok("\"0x\"".to_owned()), + ); + + let (result, logs) = capture_tracing(|| dispatch(&host, &venue, sample_block(1_000))); + result.unwrap(); + + assert!( + !host.store.snapshot().contains_key(&key), + "the commitment is removed", + ); + assert!( + logs.any(|e| e.message.contains("has no code")), + "and the drop says why", + ); + } + + /// Every other cause of an unreadable refusal points outward, at the + /// gas cap or the registry address. Those are the same for every + /// commitment, so removing one reports nothing and loses a watch. + #[test] + fn an_unreadable_refusal_with_a_contract_owner_backs_off() { + use nexum_sdk::host::RpcError; + + let host = MockHost::new(); + let venue = MockVenue::default(); + let owner = address!("0011223344556677889900AABBCCDDEEFF001122"); + let params = sample_params(); + let key = seed_commitment(&host, owner, ¶ms); + + host.chain.respond_to( + "eth_call", + programmed_eth_call_params(owner, ¶ms), + Err(ChainError::Rpc(RpcError { + code: 3, + message: "out of gas".into(), + data: None, + })), + ); + host.chain.respond_to( + "eth_getCode", + format!(r#"["{owner:#x}","latest"]"#), + Ok("\"0x60806040\"".to_owned()), + ); + + let (result, logs) = capture_tracing(|| dispatch(&host, &venue, sample_block(1_000))); + result.unwrap(); + + let store = host.store.snapshot(); + assert!(store.contains_key(&key), "the commitment survives"); + assert!( + store.keys().any(|k| k.starts_with("next_epoch:")), + "and is gated forward: {store:?}", + ); + assert!( + logs.any(|e| e.message.contains("gas cap")), + "and the operator is pointed at the likely cause", + ); + } + #[test] fn poll_invalid_drops_commitment_and_gates() { // A residual revert must delete the commitment and any stale