diff --git a/modules/twap-monitor/src/strategy.rs b/modules/twap-monitor/src/strategy.rs index 07810300..79c81c78 100644 --- a/modules/twap-monitor/src/strategy.rs +++ b/modules/twap-monitor/src/strategy.rs @@ -11,8 +11,8 @@ use alloy_primitives::{Address, B256, Bytes, keccak256}; use alloy_sol_types::{SolCall, SolEvent, SolValue}; use cowprotocol::{ - COMPOSABLE_COW, ComposableCoW::ConditionalOrderCreated, ConditionalOrderParams, GPv2OrderData, - OrderCreation, Signature, + COMPOSABLE_COW, Chain, ComposableCoW::ConditionalOrderCreated, ConditionalOrderParams, + GPv2OrderData, OrderCreation, Signature, }; use shepherd_sdk::chain::{eth_call_params, parse_eth_call_result}; use shepherd_sdk::cow::{PollOutcome, RetryAction, classify_api_error, gpv2_to_order_data}; @@ -329,6 +329,29 @@ fn submit_ready( watch_key: &str, now_epoch_s: u64, ) -> Result<(), HostError> { + // COW-1085: short-circuit if the orderbook UID for this exact + // (order, owner, chain) tuple is already in our local-store as + // `submitted:`. The poll-tick can re-fire `Ready` for the same + // TWAP child in successive blocks — `getTradeableOrderWithSignature` + // does not know shepherd already POSTed it — and re-submitting + // wastes an appData GET + submit_order call and emits a + // misleading `DuplicatedOrder` Warn. The UID computation is + // deterministic from on-chain inputs (and matches what the + // orderbook derives server-side from the signed payload), so we + // can check before doing any network work. We also reuse the + // computed value below as the `submitted:{uid}` marker key, so + // the read and write paths agree. + let client_uid_hex = compute_uid_hex(chain_id, order, owner); + if let Some(uid_hex) = client_uid_hex.as_deref() + && host.get(&format!("submitted:{uid_hex}"))?.is_some() + { + host.log( + LogLevel::Info, + &format!("twap {uid_hex} already submitted; skipping poll re-submit"), + ); + return Ok(()); + } + // COW-1074: cow-swap UI (and other clients) sign TWAPs with a // non-empty `appData` hash that points at a JSON document held // by the orderbook's app_data registry. Hard-coding @@ -385,10 +408,30 @@ fn submit_ready( } }; match host.submit_order(chain_id, &body) { - Ok(uid) => { - let key = format!("submitted:{uid}"); + Ok(server_uid) => { + // Prefer the client-computed UID for the marker key so the + // idempotency check at the top of `submit_ready` reads what + // we wrote (COW-1085). In production the server-returned + // UID is the same value (both sides derive it from the + // signed `OrderData` via the canonical + // `digest || owner || valid_to` layout); a divergence + // would be a protocol-level bug worth surfacing rather + // than silently splitting the keyspace. + let marker_uid = client_uid_hex.as_deref().unwrap_or(server_uid.as_str()); + let key = format!("submitted:{marker_uid}"); // Empty marker - presence of the key is the receipt. host.set(&key, b"")?; + if let Some(client_uid) = client_uid_hex.as_deref() + && client_uid != server_uid + { + host.log( + LogLevel::Warn, + &format!( + "twap UID divergence: client={client_uid} server={server_uid} \ + (marker stored under client UID for idempotency consistency)" + ), + ); + } host.log(LogLevel::Info, &format!("submitted {key}")); } Err(err) => { @@ -398,6 +441,23 @@ fn submit_ready( Ok(()) } +/// Compute the orderbook UID hex (`0x` + 112 hex chars) for the given +/// on-chain (order, owner, chain) tuple, mirroring what `submit_order` +/// will deduce server-side. Used by [`submit_ready`] to short-circuit +/// poll-tick re-submissions of an already-submitted TWAP child +/// (COW-1085). +/// +/// Returns `None` if the chain id is unsupported by `cowprotocol::Chain` +/// or the order carries an unknown enum marker — both cases also stop +/// the regular submit path downstream, so the caller can fall through +/// to the normal flow and let it surface the appropriate diagnostic. +fn compute_uid_hex(chain_id: u64, order: &GPv2OrderData, owner: Address) -> Option { + let chain = Chain::try_from(chain_id).ok()?; + let domain = chain.settlement_domain(); + let order_data = gpv2_to_order_data(order)?; + Some(format!("{}", order_data.uid(&domain, owner))) +} + // ---- BLEU-829: OrderPostError -> retry action ---- fn apply_submit_retry( @@ -888,11 +948,80 @@ mod tests { on_block(&host, sample_block(1_000)).unwrap(); + let expected_uid = compute_uid_hex(SEPOLIA, &ready_order, owner) + .expect("Sepolia is supported + canonical markers"); assert_eq!(host.chain.call_count(), 1); assert_eq!(host.cow_api.call_count(), 1); assert!( - host.store.snapshot().contains_key("submitted:0xfeedface"), - "expected submitted:{{uid}} marker" + host.store + .snapshot() + .contains_key(&format!("submitted:{expected_uid}")), + "expected submitted:{{client_uid}} marker (COW-1085: marker key now uses the client-computed UID, not the server-returned one, so the idempotency check at the top of submit_ready reads what we wrote)" + ); + // The MockHost orderbook stub returns `0xfeedface` instead of + // the canonical UID; this asserts the strategy logs a Warn + // about the divergence (real orderbooks would not diverge). + assert!( + host.logging.contains("twap UID divergence"), + "expected divergence Warn when mock orderbook returns a non-canonical UID" + ); + } + + /// COW-1085 regression guard: when `getTradeableOrderWithSignature` + /// returns the same Ready tuple in consecutive poll-ticks (the + /// on-chain conditional order does not know shepherd already + /// POSTed it), the second tick must NOT call `submit_order` + /// again. Without the guard the orderbook responds with + /// `DuplicatedOrder` and a Warn fires for what is in fact + /// correct, finished work. The guard is the `submitted:{uid}` + /// short-circuit at the top of `submit_ready`. + #[test] + fn poll_ready_skips_submit_when_submitted_uid_already_in_store() { + let host = MockHost::new(); + let owner = address!("0011223344556677889900AABBCCDDEEFF001122"); + let params = sample_params(); + seed_watch(&host, owner, ¶ms); + + let ready_order = submittable_order(); + let signature: Bytes = hex!("c0ffeec0ffeec0ffee").to_vec().into(); + let wire = (ready_order.clone(), signature.clone()).abi_encode_params(); + host.chain.respond_to( + "eth_call", + programmed_eth_call_params(owner, ¶ms), + Ok(quoted_hex(&wire)), + ); + + // Seed the marker that a previous successful poll-tick would + // have written. The poll path must read this and skip; the + // orderbook submit must not be attempted. + let already_submitted_uid = compute_uid_hex(SEPOLIA, &ready_order, owner) + .expect("Sepolia is supported + canonical markers"); + host.store + .set(&format!("submitted:{already_submitted_uid}"), b"") + .expect("seed submitted marker"); + + on_block(&host, sample_block(1_000)).unwrap(); + + assert_eq!( + host.chain.call_count(), + 1, + "poll still consults the chain to see Ready", + ); + assert_eq!( + host.cow_api.call_count(), + 0, + "submit_order must NOT be called when submitted:{{uid}} already exists", + ); + assert_eq!( + host.cow_api.request_calls().len(), + 0, + "appData resolve must NOT be called either — the guard short-circuits early", + ); + assert!( + host.logging.contains(&format!( + "twap {already_submitted_uid} already submitted; skipping poll re-submit" + )), + "expected the idempotency-skip Info log line", ); } @@ -953,9 +1082,13 @@ mod tests { 1, "exactly one app_data resolve", ); + let expected_uid = compute_uid_hex(SEPOLIA, &ready_order, owner) + .expect("Sepolia is supported + canonical markers"); assert!( - host.store.snapshot().contains_key("submitted:0xfeedface"), - "submitted:{{uid}} marker must be written after a successful resolve+submit" + host.store + .snapshot() + .contains_key(&format!("submitted:{expected_uid}")), + "submitted:{{client_uid}} marker must be written after a successful resolve+submit (COW-1085)" ); }