[review-mirror] M3 epic (upstream #18): SDK + examples + tutorial + QA#55
Closed
jeffersonBastos wants to merge 70 commits into
Closed
[review-mirror] M3 epic (upstream #18): SDK + examples + tutorial + QA#55jeffersonBastos wants to merge 70 commits into
jeffersonBastos wants to merge 70 commits into
Conversation
Adds the dependencies the 0.2 host backends need: - cowprotocol (1.0.0-alpha) for the cow-api submission path (OrderBookApi, OrderCreation, OrderUid, Chain). - alloy-provider / -rpc-client / -transport-ws / -primitives (1.5) for the chain JSON-RPC dispatch. The reqwest feature on alloy-provider engages connect_http; the pubsub/ws features back eth_subscribe-class methods. - redb (2) for local-store. Same crate cowprotocol's own watch-tower picked, so the dep tree does not bifurcate when both are used in the same workspace. - reqwest (0.12, rustls-tls) — direct, so the import survives any future cowprotocol feature rearrangement. - tracing + tracing-subscriber (env-filter + fmt) — replaces the 0.1 eprintln! debug log so the engine can drop into a structured log pipeline without re-instrumenting every host call. - thiserror (2) — typed error enums in each backend. - tempfile + wiremock as dev-deps for the host backend tests. Adds engine.example.toml documenting the [engine] state_dir + per- chain RPC URLs the chain backend reads at boot; data/ is now ignored so a local run does not leave the redb file in tree.
Replaces the 0.2 Unsupported stubs with working backends. Each
capability lives in its own host submodule so the trait impls in
main.rs stay thin (dispatch + project the backend's typed error
onto HostError).
cow_api::submit_order
- Parses the guest's bytes as JSON cowprotocol::OrderCreation.
- Dispatches via cowprotocol::OrderBookApi::post_order.
- Returns the assigned OrderUid as a 0x-prefixed hex string.
cow_api::request
- REST passthrough. The base URL is whichever URL the pool's
OrderBookApi client carries — so OrderBookApi::new_with_base_url
overrides (staging, wiremock) flow through transparently.
- Method/path validated host-side; orderbook 4xx/5xx bodies are
surfaced verbatim so the guest can decode {errorType,description}.
chain::request
- Raw JSON-RPC dispatch over an alloy DynProvider opened from
engine.toml at boot. WebSocket URLs engage pubsub (eth_subscribe);
HTTP URLs use the HTTP transport. Params are passed as
serde_json::RawValue so alloy does not re-encode.
- request-batch falls back to per-call dispatch (same shape as the
earlier stub but now backed by real RPC).
local_store
- redb file under engine_config.engine.state_dir.
- Single shared table. Per-module namespacing is enforced
host-side via [len:u8][module_name][raw_key] prefix on every
key. list_keys strips the prefix before returning to the guest.
logging
- Routes through tracing::event! tagged with module=<namespace>.
- Engine boot installs an EnvFilter-based subscriber; RUST_LOG
overrides the engine.toml log_level.
identity / remote-store / messaging / http stay at Unsupported per
the 0.2 roadmap (keystore / Swarm / Waku land in 0.3).
Tests (14, all green):
- cow_orderbook: pool default chains, unknown-chain typing, REST
GET passthrough, relative-path resolution, unknown-method
rejection, submit_order round-trip — last three under wiremock
so the full HTTP path is exercised without hitting api.cow.fi.
- provider_pool: empty pool surfaces UnknownChain.
- local_store: roundtrip, namespace isolation, delete, list_keys
prefix-stripping, empty-namespace rejection.
End-to-end against modules/example: example.wasm loads under the
new wiring, logs init + on_event through the tracing pipeline.
… death (BLEU-813-817)
…er-pool, supervisor (BLEU-821)
…interfaces (BLEU-819)
…ed_crate_dependencies, drop redundant map_err)
PR #9 specific: - main: warn + return when block/log streams end (WebSocket dropped) - supervisor: simplify dispatch_block by extracting chain_id before move - supervisor: temp_local_store returns (TempDir, LocalStore) instead of leaking - README: correct engine.toml chain syntax to [chains.<id>] with rpc_url Rebased from PR #8: - local_store_redb: table.range() instead of iter() for O(matching) keys - provider_pool: dedupe method clone on the success path - main: hex_encode writes into the pre-allocated buffer - cow_orderbook: drop blank line nit - manifest: collapse nested if and use ? operator (clippy) - alloy_rpc_client / alloy_transport(_ws) imports as _ to satisfy unused_crate_dependencies.
Move the manifest.rs monolith into a directory module with four focused submodules (types, load, capabilities, error). Includes the Subscription enum and the four PR #9 tests for subscription parsing. Behaviour unchanged - pure code motion.
main.rs went from 739 lines of mixed bootstrap + 8 Host trait impls +
CLI parser + event loop to ~125 lines of pure orchestration. New
layout:
- bindings.rs: wasmtime::component::bindgen!() moved out so other
modules can name the generated types.
- cli.rs: Cli struct + manual parser.
- host/state.rs: HostState + WasiView impl.
- host/error.rs: unimplemented / internal_error / hex_encode helpers.
- host/impls/{chain,cow_api,identity,local_store,remote_store,messaging,
logging,clock,random,http,types}.rs: one Host trait impl per file.
- runtime/limits.rs: DEFAULT_FUEL_PER_EVENT + DEFAULT_MEMORY_LIMIT.
- runtime/event_loop.rs: open_block_streams, open_log_streams, run,
wait_for_shutdown_signal, TaggedBlockStream, TaggedLogStream.
Adding a new capability is now a single new file under host/impls/
rather than a 60-80 line diff in main.rs.
local_store_redb.rs was 89% tests, cow_orderbook.rs was 60%, and supervisor.rs was 32% (205 lines absolute). Promote each to a directory module with the test suite living in a sibling tests.rs so impl-side diffs stop competing with test churn for attention.
Carries PR #8 (host backends) + PR #9 (supervisor) + cowprotocol patch. Open upstream: nullislabs#15.
Open upstream: nullislabs#12. Resolved .gitignore by taking the PR #12 additions (.agents/, .claude/, skills-lock.json) plus PR #15's data/. # Conflicts: # .gitignore
Add modules/twap-monitor/ as a workspace member. Cargo.toml declares [lib] crate-type = ["cdylib"] for WASM Component output, and pulls the deps the TWAP module path needs: cowprotocol (default-features off — only typed primitives and OrderCreation surface needed), alloy-sol-types (event/return decoding lands in BLEU-826/827), and wit-bindgen. src/lib.rs binds against the shepherd:cow/shepherd world (event- module imports + cow-api). generate_all is required because the world include pulls nexum:host/types across packages — without it, wit-bindgen panics on the missing cross-package mapping. init and on_event are stubbed: init logs once; on_event is a no-op until the Event::Log / Event::Block dispatch lands in BLEU-826 / BLEU-827. Verification: cargo build --target wasm32-wasip2 --release -p twap-monitor emits a 65 KB .wasm. Engine load is gated on module.toml (BLEU-834).
…-826)
`on_event(Event::Logs)` decodes each log against
`ComposableCoW.ConditionalOrderCreated` via `alloy_sol_types`,
extracts `(owner, params)`, and writes `watch:{owner}:{params_hash}`
to local-store with the abi-encoded `ConditionalOrderParams` as
the value. BLEU-827 reads this back via `list-keys("watch:")` and
the value is exactly the `(handler, salt, staticInput)` tuple the
poll path passes to `getTradeableOrderWithSignature`.
Idempotency: `local_store::set` overwrites in place, so re-org
replay or overlapping subscription windows produce no observable
side effect.
Resilience: `decode_conditional_order_created` returns `None`
when topic0 does not match the event signature or the payload
fails ABI decoding. Adjacent events on the same subscription
(MerkleRootSet, SwapGuardSet) are silently skipped instead of
short-circuiting the batch. The fn is on plain slices so the
host-free unit tests cover well-formed / wrong-topic / empty-
topics without wit-bindgen scaffolding.
Block, Tick, and Message variants of `Event` are left unhandled
in this PR — `Event::Block` dispatch lands in BLEU-827 (poll
path); the other two are not used by this module.
Adds `alloy-primitives` as a direct dep so the topic/data plumbing
does not rely on alloy types leaking through `cowprotocol`'s
re-exports.
`cargo build --target wasm32-wasip2 --release -p twap-monitor`
emits a 96 KB .wasm (up from the 65 KB skeleton because of the
alloy + cowprotocol composable types now linked in).
`on_event(Event::Block)` walks every persisted watch, skips the
ones gated by a future `next_block:` / `next_epoch:` entry, and
dispatches the ready ones via `chain::request("eth_call",
[{to: COMPOSABLE_COW, data}, "latest"])` to
`ComposableCoW.getTradeableOrderWithSignature(owner, params,
"", [])`.
Returns:
- Successful return data → `<(GPv2OrderData, Bytes)>::abi_decode_params`
→ `PollOutcome::Ready { order, signature }`.
- Revert payload → `decode_revert` matches the four-byte selector
against the five `IConditionalOrder` errors:
OrderNotValid → DontTryAgain
PollNever → DontTryAgain
PollTryNextBlock → TryNextBlock
PollTryAtBlock(n) → TryOnBlock(n)
PollTryAtEpoch(t) → TryAtEpoch(t)
- Anything else falls back to TryNextBlock so a flaky RPC or
unmodelled require-revert is retried instead of dropped.
Decoder ABI: a local `abi::Params` struct mirrors the wire format
of `cowprotocol::ConditionalOrderParams` because sol! cannot cross
crate boundaries; the resulting call selector is byte-equal to the
real contract. The successful return path decodes into the
canonical `cowprotocol::GPv2OrderData` directly, so the 12-field
struct is not duplicated. `Ready` boxes the order to keep
`PollOutcome` cache-friendly (clippy::large_enum_variant).
Storage conventions (shared with BLEU-830, which writes these):
- `next_block:{owner}:{params_hash}` -> u64 LE — block number gate
- `next_epoch:{owner}:{params_hash}` -> u64 LE — Unix-seconds gate
Either / both / neither may be set; the watch polls when both pass.
`block.timestamp` is milliseconds per WIT, so we divide by 1000 to
compare against the `TryAtEpoch` (seconds) convention.
Host follow-up: the chain backend currently swallows alloy's
`RpcError::ErrorResp.data` (it becomes `host-error.message`,
unstructured). `poll_one` is wired to consume structured revert
hex via `host-error.data` once that lands — the `decode_revert_hex`
test locks the path. Until then, every revert defaults to
TryNextBlock, which is the safe choice.
Tests: 14 new (return round-trip, all five revert variants, hex
plumbing, eth_call JSON shape, watch-key round-trip, U256
saturation), keeping the 3 BLEU-826 regressions. `.wasm` grows
from 96 KB to 215 KB (serde_json + IConditionalOrder ABI + the
GPv2OrderData decode path linked in).
Linear: BLEU-827. Ref ADR-0006.
…828)
On `PollOutcome::Ready { order, signature }`, convert the
`GPv2OrderData` to the typed `OrderData` (maps the on-chain
bytes32 markers `kind` / `sellTokenBalance` / `buyTokenBalance`
via cowprotocol's `from_contract_bytes`), wrap the signature as
`Signature::Eip1271` (ComposableCoW returns the orderbook wire
form: raw verifier bytes, the orderbook re-prepends `from`
before settlement), and feed everything through
`OrderCreation::from_signed_order_data`. The body is then
serde-encoded and pushed to `cow_api::submit_order(chain_id,
body)`.
On success, persist `submitted:{uid}` in local-store as an
empty marker — presence of the key is the receipt; BLEU-830
may later attach metadata but the bare flag is enough to
suppress double submits.
Scope notes (deliberately deferred):
- `app_data` is hard-coded to `EMPTY_APP_DATA_JSON`.
Conditional orders that pin a real document on IPFS get
rejected by `from_signed_order_data` (digest mismatch) and
skipped with a Warn log instead of submitting a corrupt body.
Resolving the document is its own concern.
- Submission errors are logged. BLEU-829 wires
`OrderPostError::retry_hint` into this site so the backoff /
drop decision is data-driven.
- `from` is set to the watch owner (the address that emitted
`ConditionalOrderCreated`). The orderbook prepends this to
the EIP-1271 blob during settlement.
Tests: 7 new (gpv2_to_order_data marker mapping incl. zero-
receiver normalisation, unknown kind / balance marker
rejection; build_order_creation happy path with serde round-
trip; rejection of non-empty app_data and `from = ZERO`).
Total 24 host tests. `.wasm` 273 KB (was 215 KB; serde for
OrderCreation, the OrderData/Signature/SigningScheme modules,
and serde_with's runtime ride along).
Linear: BLEU-828. Ref ADR-0006 (modules build orders
themselves).
This was referenced Jun 22, 2026
…ance) Filtered subset of the compliance applied in PRs #66/#67 of bleu/nullis-shepherd, restricted to files that exist on the M3 epic head. M4/M5-only files (shepherd-backtest, baseline-latency tools, etc.) are skipped, and compliance hunks that depended on M4-introduced types/functions (reconnect tasks, JoinSet plumbing in event_loop, the M4-shape `ProviderError`/Rpc variant, the supervisor restart loop, env-var substitution in engine.toml) are skipped too - they only make sense once the underlying M4 code lands. Brings the M3 epic in line with the repo-wide rust rubric in the cases that do transfer cleanly: - crates/nexum-engine/src/manifest/error.rs: swap manual Display/Error impls for `thiserror::Error` derives, mark `ParseError` `#[non_exhaustive]`, carry source via `#[from]`. - crates/nexum-engine/src/manifest/load.rs: drop the `.map_err(ParseError::Io/Toml)` call sites that the `#[from]` impls now cover, swap `eprintln!` lines for structured `tracing::{info,warn}`. - crates/nexum-engine/src/manifest/mod.rs: doc tidy-up. - crates/nexum-engine/src/host/mod.rs: tighten submodule visibility from `pub` to `pub(crate)` (no out-of-crate users). - crates/nexum-engine/src/host/error.rs + host/impls/cow_api.rs: drop the bespoke `hex_encode` helper in favour of `alloy_primitives::hex::encode_prefixed`, already a dep on M3 and used elsewhere in `shepherd-sdk`. - crates/nexum-engine/src/engine_config.rs: introduce a trimmed `EngineConfigError` (Io + Toml only - the M5 `Substitute` variant covers an env-var-substitution path that does not exist on M3) and return it from `load_or_default` instead of `anyhow::Result`. `main.rs`'s `?` still works thanks to `From<EngineConfigError> for anyhow::Error`. - crates/shepherd-sdk/src/cow/error.rs: mark `RetryAction` `#[non_exhaustive]`. - modules/{twap-monitor,examples/stop-loss,ethflow-watcher}/src/strategy.rs: add a default arm to each `match RetryAction { ... }` that now needs one, treating unknown future variants conservatively (retry on next block / leave watch in place) instead of silently dropping the watch on an SDK bump. cargo fmt + cargo clippy --all-targets -D warnings + cargo test --workspace --all-features all green on the worktree. AI Assistance: Claude Code (Opus 4.7) prepared and applied this filtered patch under direct human review. The instruction \`STOP and report\` on any non-trivial deviation was followed for each hunk; the skipped hunks (M4-coupled) are documented in the propagation summary that accompanies this branch update. A human (Bruno) is accountable for the result.
|
Work landed via |
This was referenced Jun 24, 2026
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
12 review threads addressed end-to-end. Net diff is -720 lines despite adding ~200 lines of new helpers + tests, because the WitBindgenHost adapter deduplication alone wipes ~400 lines. Per-thread: #1 (balance-tracker architecture): refactored to match the M3 host-trait+adapter split the other 4 modules use. Created `strategy.rs` with `on_block(&impl Host, ...)`, moved check_one / fetch_balance / parse_balance_hex / parse_settings into it, converted parse_config to use SDK config helpers + typed HostError instead of String. Added 3 MockHost-driven tests covering first-seen-above-threshold, below-threshold-persist, and error-does-not-abort-loop. #2 + #3 (WitBindgenHost dedup): new `shepherd_sdk::bind_host_via_wit_bindgen!()` declarative macro. Single source of truth in `crates/shepherd-sdk/src/wit_bindgen_macro.rs`; the 4 trait impls + convert_err / sdk_err_into_wit / convert_level collapse to one macro invocation per module. Migrated all 5 modules (twap-monitor, ethflow-watcher, price-alert, stop-loss, balance-tracker). Each module's lib.rs lost ~80 lines. #4 (scale_decimal + config_get dup): new `shepherd_sdk::config` with `get_required`, `get_optional`, `scale_decimal`, and a typed `ConfigError` enum (host-neutral). price-alert + stop-loss consume the SDK helpers; their local duplicates were deleted. Module-level decimal-parsing tests removed (covered by 7 SDK tests + 4 proptest cases now). #5 (Chainlink dup): new `shepherd_sdk::chain::chainlink` with `read_latest_answer(host, chain_id, oracle, domain) -> Option<I256>`. Encapsulates the eth_call → parse → ABI decode flow + Warn logging. price-alert + stop-loss now call the helper; their local AggregatorV3 sol! definitions + read_oracle / on_block oracle plumbing was deleted. SDK ships with 3 StubHost tests covering happy path, host error, and garbage-hex. #6 (WIT world capability elision): added new "Capability enforcement vs. the WIT world" section to ADR-0009 documenting that price-alert + balance-tracker compile against the shepherd:cow/shepherd supertype but their manifests omit cow-api, and that boot success depends on wasm-tools' unused- import elision. Flagged as load-bearing; M5 macro hardening path documented. #7 (poll-time revert classification inert): filed COW-1082 for the host-side fix (forward structured eth_call error data into HostError.data; analogous to COW-1075 for orderbook). #8 (classify_api_error retry-default unbounded): filed COW-1083 for the rate-limit / max-retry follow-up on the backoff: marker. #9 (RetryAction::Backoff dead variant): no code change; replied to thread clarifying it is reserved API surface waiting on a richer upstream retry_hint shape (open question for mfw78). #10 (no proptest anywhere): added `proptest` to shepherd-sdk dev-dependencies. New `crates/shepherd-sdk/src/proptests.rs` with 6 properties covering eth_call_params/parse_eth_call_result round-trip, parse_eth_call_result rejection on unquoted input, config::scale_decimal round-trip + sign-preservation, U256 LE byte round-trip, and no-panic guards for decode_revert_hex + gpv2_to_order_data marker dispatch. #11 (ethflow chain capability least-privilege): moved `chain` from required to optional in `modules/ethflow-watcher/module.toml`, mirroring the M2 mirror fix already applied. #12 (ADR-0009 test-count census): dropped the "145 host tests (twap 20, ethflow 12, ...)" breakdown; kept the qualitative claim. CI is now the authoritative count. Drive-by: alloy-sol-types moved from regular to dev-dependencies in price-alert and stop-loss now that the Chainlink ABI helper is inside shepherd-sdk and the modules only use sol! in their test helpers. Validation: - cargo test --workspace: every crate green; 5 modules + SDK + sdk-test + engine all pass. 8 host tests gained on balance-tracker; 6 proptest props gained on shepherd-sdk; 3 Chainlink helper tests gained. - cargo clippy --workspace --all-targets -- -D warnings: clean. - cargo fmt --check: clean. - cargo build --target wasm32-wasip2 --release for all 5 modules: clean. - Zero em-dashes in source code added.
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
…W-1082) The chain backend previously dropped alloy's structured `RpcError::ErrorResp` payload on the floor — the formatted error string went into `HostError.message`, but `HostError.data` stayed `None` and `HostError.code` was hard-coded to `-32603`. That made the twap-monitor's poll-time revert classifier inert on real traffic: `OrderNotValid` / `PollNever` / `PollTryAtBlock` / `PollTryAtEpoch` all fell through to `TryNextBlock` because `decode_revert_hex` only fires on a non-empty `err.data`. This change wires the structured payload through end-to-end. - `crates/nexum-engine/src/host/provider_pool.rs`: when alloy's `provider.raw_request` fails with an `RpcError::ErrorResp`, the pool now captures both `payload.code` (as `Option<i64>` so we can distinguish "no ErrorResp" from "ErrorResp with code 0") and `payload.data` (as `Option<String>`, the JSON-encoded revert hex) and surfaces them on `ProviderError::Rpc`. Transport-side failures (timeout, websocket disconnect) leave both `None`. The two subscribe paths (`subscribe_blocks`, `subscribe_logs`) keep `code: None, data: None` since they don't carry an ErrorResp. - `crates/nexum-engine/src/host/impls/chain.rs`: extract the `ProviderError -> HostError` projection into a free helper `provider_error_to_host_error`. The `Rpc` arm forwards the structured `data` verbatim, preserves the node-reported code (saturating out-of-`i32` values to `-32603`), and falls back to `-32603` only when no `ErrorResp` was present. Five unit tests cover: revert with data, transport failure with `None`, out-of-range code, unknown-chain, and invalid-params. - `modules/twap-monitor/src/strategy.rs`: update the stale comment on the `decode_revert_hex` branch — that branch is now live on real traffic, the only `None` path is transport-level failures (which keep the safe `TryNextBlock` default). No incorrect order is ever submitted (the contract reverts; the orderbook never sees a bad body). The issue is pruning efficiency: a permanently dead TWAP watch was re-polled every block until a submit eventually failed for an unrelated reason, and the local-store filled with `watch:` entries the strategy could otherwise drop on the first revert. With this fix the SDK-side classifier dispatches `Drop` / gate on the first revert, matching the documented expectation in `docs/adr/0007-upstream-protocol-logic-to-cow-rs.md`. - 70/70 nexum-engine tests pass - 23/23 twap-monitor tests pass - 5/5 new chain.rs projection tests pass (revert-with-data, transport-fail, out-of-range-code, unknown-chain, invalid-params) - `cargo clippy -p nexum-engine -p twap-monitor --all-targets -- -D warnings` clean jeffersonBastos's PR #55 (M3 mirror) review, thread on `modules/twap-monitor/src/strategy.rs:189`. The mirror of this fix on the cow-api side is COW-1075 (already merged via PR #48).
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
…OW-1083)
The strategy's `apply_submit_retry` previously wrote an empty
`backoff:{uid}` marker on every retriable submit failure (including
the `TryNextBlock` fallback for unparseable orderbook envelopes). The
marker was a presence flag with no payload, so on every supervisor
reconnect / engine restart the same dead placement would retry
indefinitely — bounded only by log re-delivery frequency.
This change persists a per-UID retry count in the marker's value
(ASCII `u32`) and upgrades to `dropped:` after `MAX_BACKOFF_RETRIES =
5` consecutive retries. The upgrade emits a Warn-level log line so
the operator sees the structural issue (flaky CDN, indexer hiccup,
poisoned envelope) rather than silently accumulating retries.
- `modules/ethflow-watcher/src/strategy.rs`:
- New const `MAX_BACKOFF_RETRIES = 5`.
- New helper `read_backoff_count` that reads + parses the marker
payload; pre-COW-1083 empty markers decode to 0 so previously-set
backoff: rows still get a fresh attempt (no premature drop on
rollout).
- `apply_submit_retry`'s retriable branch now reads the prior
count, increments, and either writes the new count or upgrades
to `dropped:` (clearing the stale `backoff:`) at the cap.
- Cap-upgrade log line carries the retry-count and message: "...
after 5 retries on transient/unparseable rejection ...".
- 19/19 ethflow-watcher tests pass.
- New `submit_transient_error_at_cap_upgrades_to_dropped_warn`:
seeds `backoff:{uid} = "4"`, triggers a `data: None` rejection
(the unparseable case the issue names explicitly), asserts:
* `dropped:{uid}` is now set
* `backoff:{uid}` is cleared (single outcome marker at rest)
* exactly one Warn log line containing "ethflow dropped" +
"retries"
- New `submit_transient_error_with_legacy_empty_marker_resets_counter`:
backwards-compat — a pre-COW-1083 empty `b""` marker is treated
as count 0, bumped to "1" on first retry rather than prematurely
dropping. Protects in-flight backoffs across the rollout.
- Existing `submit_transient_error_writes_backoff_marker_and_returns`
extended with an assertion that the first retry persists
`backoff:{uid} = "1"`.
- `cargo clippy -p ethflow-watcher --all-targets -- -D warnings`
clean.
Surfaced by jeffersonBastos's PR #55 (M3 mirror) review, thread on
`crates/shepherd-sdk/src/cow/error.rs:82`. Latent in normal
operation (the host forwards parseable envelopes after COW-1075, so
`classify_api_error` returns `Drop` for permanent rejections), but
the gap fires when the orderbook returns a non-JSON 4xx body
(e.g. an HTML error page from a CDN) or if a future host change
accidentally drops the envelope again. Bounded retry semantics
close the latent risk without changing the safe-default
classification (still `TryNextBlock` on `None` data — that part is
explicitly out of scope per the issue).
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
12 review threads addressed end-to-end. Net diff is -720 lines despite adding ~200 lines of new helpers + tests, because the WitBindgenHost adapter deduplication alone wipes ~400 lines. Per-thread: #1 (balance-tracker architecture): refactored to match the M3 host-trait+adapter split the other 4 modules use. Created `strategy.rs` with `on_block(&impl Host, ...)`, moved check_one / fetch_balance / parse_balance_hex / parse_settings into it, converted parse_config to use SDK config helpers + typed HostError instead of String. Added 3 MockHost-driven tests covering first-seen-above-threshold, below-threshold-persist, and error-does-not-abort-loop. #2 + #3 (WitBindgenHost dedup): new `shepherd_sdk::bind_host_via_wit_bindgen!()` declarative macro. Single source of truth in `crates/shepherd-sdk/src/wit_bindgen_macro.rs`; the 4 trait impls + convert_err / sdk_err_into_wit / convert_level collapse to one macro invocation per module. Migrated all 5 modules (twap-monitor, ethflow-watcher, price-alert, stop-loss, balance-tracker). Each module's lib.rs lost ~80 lines. #4 (scale_decimal + config_get dup): new `shepherd_sdk::config` with `get_required`, `get_optional`, `scale_decimal`, and a typed `ConfigError` enum (host-neutral). price-alert + stop-loss consume the SDK helpers; their local duplicates were deleted. Module-level decimal-parsing tests removed (covered by 7 SDK tests + 4 proptest cases now). #5 (Chainlink dup): new `shepherd_sdk::chain::chainlink` with `read_latest_answer(host, chain_id, oracle, domain) -> Option<I256>`. Encapsulates the eth_call → parse → ABI decode flow + Warn logging. price-alert + stop-loss now call the helper; their local AggregatorV3 sol! definitions + read_oracle / on_block oracle plumbing was deleted. SDK ships with 3 StubHost tests covering happy path, host error, and garbage-hex. #6 (WIT world capability elision): added new "Capability enforcement vs. the WIT world" section to ADR-0009 documenting that price-alert + balance-tracker compile against the shepherd:cow/shepherd supertype but their manifests omit cow-api, and that boot success depends on wasm-tools' unused- import elision. Flagged as load-bearing; M5 macro hardening path documented. #7 (poll-time revert classification inert): filed COW-1082 for the host-side fix (forward structured eth_call error data into HostError.data; analogous to COW-1075 for orderbook). #8 (classify_api_error retry-default unbounded): filed COW-1083 for the rate-limit / max-retry follow-up on the backoff: marker. #9 (RetryAction::Backoff dead variant): no code change; replied to thread clarifying it is reserved API surface waiting on a richer upstream retry_hint shape (open question for mfw78). #10 (no proptest anywhere): added `proptest` to shepherd-sdk dev-dependencies. New `crates/shepherd-sdk/src/proptests.rs` with 6 properties covering eth_call_params/parse_eth_call_result round-trip, parse_eth_call_result rejection on unquoted input, config::scale_decimal round-trip + sign-preservation, U256 LE byte round-trip, and no-panic guards for decode_revert_hex + gpv2_to_order_data marker dispatch. #11 (ethflow chain capability least-privilege): moved `chain` from required to optional in `modules/ethflow-watcher/module.toml`, mirroring the M2 mirror fix already applied. #12 (ADR-0009 test-count census): dropped the "145 host tests (twap 20, ethflow 12, ...)" breakdown; kept the qualitative claim. CI is now the authoritative count. Drive-by: alloy-sol-types moved from regular to dev-dependencies in price-alert and stop-loss now that the Chainlink ABI helper is inside shepherd-sdk and the modules only use sol! in their test helpers. Validation: - cargo test --workspace: every crate green; 5 modules + SDK + sdk-test + engine all pass. 8 host tests gained on balance-tracker; 6 proptest props gained on shepherd-sdk; 3 Chainlink helper tests gained. - cargo clippy --workspace --all-targets -- -D warnings: clean. - cargo fmt --check: clean. - cargo build --target wasm32-wasip2 --release for all 5 modules: clean. - Zero em-dashes in source code added.
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
…W-1082) The chain backend previously dropped alloy's structured `RpcError::ErrorResp` payload on the floor — the formatted error string went into `HostError.message`, but `HostError.data` stayed `None` and `HostError.code` was hard-coded to `-32603`. That made the twap-monitor's poll-time revert classifier inert on real traffic: `OrderNotValid` / `PollNever` / `PollTryAtBlock` / `PollTryAtEpoch` all fell through to `TryNextBlock` because `decode_revert_hex` only fires on a non-empty `err.data`. This change wires the structured payload through end-to-end. - `crates/nexum-engine/src/host/provider_pool.rs`: when alloy's `provider.raw_request` fails with an `RpcError::ErrorResp`, the pool now captures both `payload.code` (as `Option<i64>` so we can distinguish "no ErrorResp" from "ErrorResp with code 0") and `payload.data` (as `Option<String>`, the JSON-encoded revert hex) and surfaces them on `ProviderError::Rpc`. Transport-side failures (timeout, websocket disconnect) leave both `None`. The two subscribe paths (`subscribe_blocks`, `subscribe_logs`) keep `code: None, data: None` since they don't carry an ErrorResp. - `crates/nexum-engine/src/host/impls/chain.rs`: extract the `ProviderError -> HostError` projection into a free helper `provider_error_to_host_error`. The `Rpc` arm forwards the structured `data` verbatim, preserves the node-reported code (saturating out-of-`i32` values to `-32603`), and falls back to `-32603` only when no `ErrorResp` was present. Five unit tests cover: revert with data, transport failure with `None`, out-of-range code, unknown-chain, and invalid-params. - `modules/twap-monitor/src/strategy.rs`: update the stale comment on the `decode_revert_hex` branch — that branch is now live on real traffic, the only `None` path is transport-level failures (which keep the safe `TryNextBlock` default). No incorrect order is ever submitted (the contract reverts; the orderbook never sees a bad body). The issue is pruning efficiency: a permanently dead TWAP watch was re-polled every block until a submit eventually failed for an unrelated reason, and the local-store filled with `watch:` entries the strategy could otherwise drop on the first revert. With this fix the SDK-side classifier dispatches `Drop` / gate on the first revert, matching the documented expectation in `docs/adr/0007-upstream-protocol-logic-to-cow-rs.md`. - 70/70 nexum-engine tests pass - 23/23 twap-monitor tests pass - 5/5 new chain.rs projection tests pass (revert-with-data, transport-fail, out-of-range-code, unknown-chain, invalid-params) - `cargo clippy -p nexum-engine -p twap-monitor --all-targets -- -D warnings` clean jeffersonBastos's PR #55 (M3 mirror) review, thread on `modules/twap-monitor/src/strategy.rs:189`. The mirror of this fix on the cow-api side is COW-1075 (already merged via PR #48).
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
…OW-1083)
The strategy's `apply_submit_retry` previously wrote an empty
`backoff:{uid}` marker on every retriable submit failure (including
the `TryNextBlock` fallback for unparseable orderbook envelopes). The
marker was a presence flag with no payload, so on every supervisor
reconnect / engine restart the same dead placement would retry
indefinitely — bounded only by log re-delivery frequency.
This change persists a per-UID retry count in the marker's value
(ASCII `u32`) and upgrades to `dropped:` after `MAX_BACKOFF_RETRIES =
5` consecutive retries. The upgrade emits a Warn-level log line so
the operator sees the structural issue (flaky CDN, indexer hiccup,
poisoned envelope) rather than silently accumulating retries.
- `modules/ethflow-watcher/src/strategy.rs`:
- New const `MAX_BACKOFF_RETRIES = 5`.
- New helper `read_backoff_count` that reads + parses the marker
payload; pre-COW-1083 empty markers decode to 0 so previously-set
backoff: rows still get a fresh attempt (no premature drop on
rollout).
- `apply_submit_retry`'s retriable branch now reads the prior
count, increments, and either writes the new count or upgrades
to `dropped:` (clearing the stale `backoff:`) at the cap.
- Cap-upgrade log line carries the retry-count and message: "...
after 5 retries on transient/unparseable rejection ...".
- 19/19 ethflow-watcher tests pass.
- New `submit_transient_error_at_cap_upgrades_to_dropped_warn`:
seeds `backoff:{uid} = "4"`, triggers a `data: None` rejection
(the unparseable case the issue names explicitly), asserts:
* `dropped:{uid}` is now set
* `backoff:{uid}` is cleared (single outcome marker at rest)
* exactly one Warn log line containing "ethflow dropped" +
"retries"
- New `submit_transient_error_with_legacy_empty_marker_resets_counter`:
backwards-compat — a pre-COW-1083 empty `b""` marker is treated
as count 0, bumped to "1" on first retry rather than prematurely
dropping. Protects in-flight backoffs across the rollout.
- Existing `submit_transient_error_writes_backoff_marker_and_returns`
extended with an assertion that the first retry persists
`backoff:{uid} = "1"`.
- `cargo clippy -p ethflow-watcher --all-targets -- -D warnings`
clean.
Surfaced by jeffersonBastos's PR #55 (M3 mirror) review, thread on
`crates/shepherd-sdk/src/cow/error.rs:82`. Latent in normal
operation (the host forwards parseable envelopes after COW-1075, so
`classify_api_error` returns `Drop` for permanent rejections), but
the gap fires when the orderbook returns a non-JSON 4xx body
(e.g. an HTML error page from a CDN) or if a future host change
accidentally drops the envelope again. Bounded retry semantics
close the latent risk without changing the safe-default
classification (still `TryNextBlock` on `None` data — that part is
explicitly out of scope per the issue).
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
12 review threads addressed end-to-end. Net diff is -720 lines despite adding ~200 lines of new helpers + tests, because the WitBindgenHost adapter deduplication alone wipes ~400 lines. Per-thread: #1 (balance-tracker architecture): refactored to match the M3 host-trait+adapter split the other 4 modules use. Created `strategy.rs` with `on_block(&impl Host, ...)`, moved check_one / fetch_balance / parse_balance_hex / parse_settings into it, converted parse_config to use SDK config helpers + typed HostError instead of String. Added 3 MockHost-driven tests covering first-seen-above-threshold, below-threshold-persist, and error-does-not-abort-loop. #2 + #3 (WitBindgenHost dedup): new `shepherd_sdk::bind_host_via_wit_bindgen!()` declarative macro. Single source of truth in `crates/shepherd-sdk/src/wit_bindgen_macro.rs`; the 4 trait impls + convert_err / sdk_err_into_wit / convert_level collapse to one macro invocation per module. Migrated all 5 modules (twap-monitor, ethflow-watcher, price-alert, stop-loss, balance-tracker). Each module's lib.rs lost ~80 lines. #4 (scale_decimal + config_get dup): new `shepherd_sdk::config` with `get_required`, `get_optional`, `scale_decimal`, and a typed `ConfigError` enum (host-neutral). price-alert + stop-loss consume the SDK helpers; their local duplicates were deleted. Module-level decimal-parsing tests removed (covered by 7 SDK tests + 4 proptest cases now). #5 (Chainlink dup): new `shepherd_sdk::chain::chainlink` with `read_latest_answer(host, chain_id, oracle, domain) -> Option<I256>`. Encapsulates the eth_call → parse → ABI decode flow + Warn logging. price-alert + stop-loss now call the helper; their local AggregatorV3 sol! definitions + read_oracle / on_block oracle plumbing was deleted. SDK ships with 3 StubHost tests covering happy path, host error, and garbage-hex. #6 (WIT world capability elision): added new "Capability enforcement vs. the WIT world" section to ADR-0009 documenting that price-alert + balance-tracker compile against the shepherd:cow/shepherd supertype but their manifests omit cow-api, and that boot success depends on wasm-tools' unused- import elision. Flagged as load-bearing; M5 macro hardening path documented. #7 (poll-time revert classification inert): filed COW-1082 for the host-side fix (forward structured eth_call error data into HostError.data; analogous to COW-1075 for orderbook). #8 (classify_api_error retry-default unbounded): filed COW-1083 for the rate-limit / max-retry follow-up on the backoff: marker. #9 (RetryAction::Backoff dead variant): no code change; replied to thread clarifying it is reserved API surface waiting on a richer upstream retry_hint shape (open question for mfw78). #10 (no proptest anywhere): added `proptest` to shepherd-sdk dev-dependencies. New `crates/shepherd-sdk/src/proptests.rs` with 6 properties covering eth_call_params/parse_eth_call_result round-trip, parse_eth_call_result rejection on unquoted input, config::scale_decimal round-trip + sign-preservation, U256 LE byte round-trip, and no-panic guards for decode_revert_hex + gpv2_to_order_data marker dispatch. #11 (ethflow chain capability least-privilege): moved `chain` from required to optional in `modules/ethflow-watcher/module.toml`, mirroring the M2 mirror fix already applied. #12 (ADR-0009 test-count census): dropped the "145 host tests (twap 20, ethflow 12, ...)" breakdown; kept the qualitative claim. CI is now the authoritative count. Drive-by: alloy-sol-types moved from regular to dev-dependencies in price-alert and stop-loss now that the Chainlink ABI helper is inside shepherd-sdk and the modules only use sol! in their test helpers. Validation: - cargo test --workspace: every crate green; 5 modules + SDK + sdk-test + engine all pass. 8 host tests gained on balance-tracker; 6 proptest props gained on shepherd-sdk; 3 Chainlink helper tests gained. - cargo clippy --workspace --all-targets -- -D warnings: clean. - cargo fmt --check: clean. - cargo build --target wasm32-wasip2 --release for all 5 modules: clean. - Zero em-dashes in source code added.
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
…W-1082) The chain backend previously dropped alloy's structured `RpcError::ErrorResp` payload on the floor — the formatted error string went into `HostError.message`, but `HostError.data` stayed `None` and `HostError.code` was hard-coded to `-32603`. That made the twap-monitor's poll-time revert classifier inert on real traffic: `OrderNotValid` / `PollNever` / `PollTryAtBlock` / `PollTryAtEpoch` all fell through to `TryNextBlock` because `decode_revert_hex` only fires on a non-empty `err.data`. This change wires the structured payload through end-to-end. - `crates/nexum-engine/src/host/provider_pool.rs`: when alloy's `provider.raw_request` fails with an `RpcError::ErrorResp`, the pool now captures both `payload.code` (as `Option<i64>` so we can distinguish "no ErrorResp" from "ErrorResp with code 0") and `payload.data` (as `Option<String>`, the JSON-encoded revert hex) and surfaces them on `ProviderError::Rpc`. Transport-side failures (timeout, websocket disconnect) leave both `None`. The two subscribe paths (`subscribe_blocks`, `subscribe_logs`) keep `code: None, data: None` since they don't carry an ErrorResp. - `crates/nexum-engine/src/host/impls/chain.rs`: extract the `ProviderError -> HostError` projection into a free helper `provider_error_to_host_error`. The `Rpc` arm forwards the structured `data` verbatim, preserves the node-reported code (saturating out-of-`i32` values to `-32603`), and falls back to `-32603` only when no `ErrorResp` was present. Five unit tests cover: revert with data, transport failure with `None`, out-of-range code, unknown-chain, and invalid-params. - `modules/twap-monitor/src/strategy.rs`: update the stale comment on the `decode_revert_hex` branch — that branch is now live on real traffic, the only `None` path is transport-level failures (which keep the safe `TryNextBlock` default). No incorrect order is ever submitted (the contract reverts; the orderbook never sees a bad body). The issue is pruning efficiency: a permanently dead TWAP watch was re-polled every block until a submit eventually failed for an unrelated reason, and the local-store filled with `watch:` entries the strategy could otherwise drop on the first revert. With this fix the SDK-side classifier dispatches `Drop` / gate on the first revert, matching the documented expectation in `docs/adr/0007-upstream-protocol-logic-to-cow-rs.md`. - 70/70 nexum-engine tests pass - 23/23 twap-monitor tests pass - 5/5 new chain.rs projection tests pass (revert-with-data, transport-fail, out-of-range-code, unknown-chain, invalid-params) - `cargo clippy -p nexum-engine -p twap-monitor --all-targets -- -D warnings` clean jeffersonBastos's PR #55 (M3 mirror) review, thread on `modules/twap-monitor/src/strategy.rs:189`. The mirror of this fix on the cow-api side is COW-1075 (already merged via PR #48).
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
…OW-1083)
The strategy's `apply_submit_retry` previously wrote an empty
`backoff:{uid}` marker on every retriable submit failure (including
the `TryNextBlock` fallback for unparseable orderbook envelopes). The
marker was a presence flag with no payload, so on every supervisor
reconnect / engine restart the same dead placement would retry
indefinitely — bounded only by log re-delivery frequency.
This change persists a per-UID retry count in the marker's value
(ASCII `u32`) and upgrades to `dropped:` after `MAX_BACKOFF_RETRIES =
5` consecutive retries. The upgrade emits a Warn-level log line so
the operator sees the structural issue (flaky CDN, indexer hiccup,
poisoned envelope) rather than silently accumulating retries.
- `modules/ethflow-watcher/src/strategy.rs`:
- New const `MAX_BACKOFF_RETRIES = 5`.
- New helper `read_backoff_count` that reads + parses the marker
payload; pre-COW-1083 empty markers decode to 0 so previously-set
backoff: rows still get a fresh attempt (no premature drop on
rollout).
- `apply_submit_retry`'s retriable branch now reads the prior
count, increments, and either writes the new count or upgrades
to `dropped:` (clearing the stale `backoff:`) at the cap.
- Cap-upgrade log line carries the retry-count and message: "...
after 5 retries on transient/unparseable rejection ...".
- 19/19 ethflow-watcher tests pass.
- New `submit_transient_error_at_cap_upgrades_to_dropped_warn`:
seeds `backoff:{uid} = "4"`, triggers a `data: None` rejection
(the unparseable case the issue names explicitly), asserts:
* `dropped:{uid}` is now set
* `backoff:{uid}` is cleared (single outcome marker at rest)
* exactly one Warn log line containing "ethflow dropped" +
"retries"
- New `submit_transient_error_with_legacy_empty_marker_resets_counter`:
backwards-compat — a pre-COW-1083 empty `b""` marker is treated
as count 0, bumped to "1" on first retry rather than prematurely
dropping. Protects in-flight backoffs across the rollout.
- Existing `submit_transient_error_writes_backoff_marker_and_returns`
extended with an assertion that the first retry persists
`backoff:{uid} = "1"`.
- `cargo clippy -p ethflow-watcher --all-targets -- -D warnings`
clean.
Surfaced by jeffersonBastos's PR #55 (M3 mirror) review, thread on
`crates/shepherd-sdk/src/cow/error.rs:82`. Latent in normal
operation (the host forwards parseable envelopes after COW-1075, so
`classify_api_error` returns `Drop` for permanent rejections), but
the gap fires when the orderbook returns a non-JSON 4xx body
(e.g. an HTML error page from a CDN) or if a future host change
accidentally drops the envelope again. Bounded retry semantics
close the latent risk without changing the safe-default
classification (still `TryNextBlock` on `None` data — that part is
explicitly out of scope per the issue).
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
12 review threads addressed end-to-end. Net diff is -720 lines despite adding ~200 lines of new helpers + tests, because the WitBindgenHost adapter deduplication alone wipes ~400 lines. Per-thread: #1 (balance-tracker architecture): refactored to match the M3 host-trait+adapter split the other 4 modules use. Created `strategy.rs` with `on_block(&impl Host, ...)`, moved check_one / fetch_balance / parse_balance_hex / parse_settings into it, converted parse_config to use SDK config helpers + typed HostError instead of String. Added 3 MockHost-driven tests covering first-seen-above-threshold, below-threshold-persist, and error-does-not-abort-loop. #2 + #3 (WitBindgenHost dedup): new `shepherd_sdk::bind_host_via_wit_bindgen!()` declarative macro. Single source of truth in `crates/shepherd-sdk/src/wit_bindgen_macro.rs`; the 4 trait impls + convert_err / sdk_err_into_wit / convert_level collapse to one macro invocation per module. Migrated all 5 modules (twap-monitor, ethflow-watcher, price-alert, stop-loss, balance-tracker). Each module's lib.rs lost ~80 lines. #4 (scale_decimal + config_get dup): new `shepherd_sdk::config` with `get_required`, `get_optional`, `scale_decimal`, and a typed `ConfigError` enum (host-neutral). price-alert + stop-loss consume the SDK helpers; their local duplicates were deleted. Module-level decimal-parsing tests removed (covered by 7 SDK tests + 4 proptest cases now). #5 (Chainlink dup): new `shepherd_sdk::chain::chainlink` with `read_latest_answer(host, chain_id, oracle, domain) -> Option<I256>`. Encapsulates the eth_call → parse → ABI decode flow + Warn logging. price-alert + stop-loss now call the helper; their local AggregatorV3 sol! definitions + read_oracle / on_block oracle plumbing was deleted. SDK ships with 3 StubHost tests covering happy path, host error, and garbage-hex. #6 (WIT world capability elision): added new "Capability enforcement vs. the WIT world" section to ADR-0009 documenting that price-alert + balance-tracker compile against the shepherd:cow/shepherd supertype but their manifests omit cow-api, and that boot success depends on wasm-tools' unused- import elision. Flagged as load-bearing; M5 macro hardening path documented. #7 (poll-time revert classification inert): filed COW-1082 for the host-side fix (forward structured eth_call error data into HostError.data; analogous to COW-1075 for orderbook). #8 (classify_api_error retry-default unbounded): filed COW-1083 for the rate-limit / max-retry follow-up on the backoff: marker. #9 (RetryAction::Backoff dead variant): no code change; replied to thread clarifying it is reserved API surface waiting on a richer upstream retry_hint shape (open question for mfw78). #10 (no proptest anywhere): added `proptest` to shepherd-sdk dev-dependencies. New `crates/shepherd-sdk/src/proptests.rs` with 6 properties covering eth_call_params/parse_eth_call_result round-trip, parse_eth_call_result rejection on unquoted input, config::scale_decimal round-trip + sign-preservation, U256 LE byte round-trip, and no-panic guards for decode_revert_hex + gpv2_to_order_data marker dispatch. #11 (ethflow chain capability least-privilege): moved `chain` from required to optional in `modules/ethflow-watcher/module.toml`, mirroring the M2 mirror fix already applied. #12 (ADR-0009 test-count census): dropped the "145 host tests (twap 20, ethflow 12, ...)" breakdown; kept the qualitative claim. CI is now the authoritative count. Drive-by: alloy-sol-types moved from regular to dev-dependencies in price-alert and stop-loss now that the Chainlink ABI helper is inside shepherd-sdk and the modules only use sol! in their test helpers. Validation: - cargo test --workspace: every crate green; 5 modules + SDK + sdk-test + engine all pass. 8 host tests gained on balance-tracker; 6 proptest props gained on shepherd-sdk; 3 Chainlink helper tests gained. - cargo clippy --workspace --all-targets -- -D warnings: clean. - cargo fmt --check: clean. - cargo build --target wasm32-wasip2 --release for all 5 modules: clean. - Zero em-dashes in source code added.
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
…W-1082) The chain backend previously dropped alloy's structured `RpcError::ErrorResp` payload on the floor — the formatted error string went into `HostError.message`, but `HostError.data` stayed `None` and `HostError.code` was hard-coded to `-32603`. That made the twap-monitor's poll-time revert classifier inert on real traffic: `OrderNotValid` / `PollNever` / `PollTryAtBlock` / `PollTryAtEpoch` all fell through to `TryNextBlock` because `decode_revert_hex` only fires on a non-empty `err.data`. This change wires the structured payload through end-to-end. - `crates/nexum-engine/src/host/provider_pool.rs`: when alloy's `provider.raw_request` fails with an `RpcError::ErrorResp`, the pool now captures both `payload.code` (as `Option<i64>` so we can distinguish "no ErrorResp" from "ErrorResp with code 0") and `payload.data` (as `Option<String>`, the JSON-encoded revert hex) and surfaces them on `ProviderError::Rpc`. Transport-side failures (timeout, websocket disconnect) leave both `None`. The two subscribe paths (`subscribe_blocks`, `subscribe_logs`) keep `code: None, data: None` since they don't carry an ErrorResp. - `crates/nexum-engine/src/host/impls/chain.rs`: extract the `ProviderError -> HostError` projection into a free helper `provider_error_to_host_error`. The `Rpc` arm forwards the structured `data` verbatim, preserves the node-reported code (saturating out-of-`i32` values to `-32603`), and falls back to `-32603` only when no `ErrorResp` was present. Five unit tests cover: revert with data, transport failure with `None`, out-of-range code, unknown-chain, and invalid-params. - `modules/twap-monitor/src/strategy.rs`: update the stale comment on the `decode_revert_hex` branch — that branch is now live on real traffic, the only `None` path is transport-level failures (which keep the safe `TryNextBlock` default). No incorrect order is ever submitted (the contract reverts; the orderbook never sees a bad body). The issue is pruning efficiency: a permanently dead TWAP watch was re-polled every block until a submit eventually failed for an unrelated reason, and the local-store filled with `watch:` entries the strategy could otherwise drop on the first revert. With this fix the SDK-side classifier dispatches `Drop` / gate on the first revert, matching the documented expectation in `docs/adr/0007-upstream-protocol-logic-to-cow-rs.md`. - 70/70 nexum-engine tests pass - 23/23 twap-monitor tests pass - 5/5 new chain.rs projection tests pass (revert-with-data, transport-fail, out-of-range-code, unknown-chain, invalid-params) - `cargo clippy -p nexum-engine -p twap-monitor --all-targets -- -D warnings` clean jeffersonBastos's PR #55 (M3 mirror) review, thread on `modules/twap-monitor/src/strategy.rs:189`. The mirror of this fix on the cow-api side is COW-1075 (already merged via PR #48).
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
…OW-1083)
The strategy's `apply_submit_retry` previously wrote an empty
`backoff:{uid}` marker on every retriable submit failure (including
the `TryNextBlock` fallback for unparseable orderbook envelopes). The
marker was a presence flag with no payload, so on every supervisor
reconnect / engine restart the same dead placement would retry
indefinitely — bounded only by log re-delivery frequency.
This change persists a per-UID retry count in the marker's value
(ASCII `u32`) and upgrades to `dropped:` after `MAX_BACKOFF_RETRIES =
5` consecutive retries. The upgrade emits a Warn-level log line so
the operator sees the structural issue (flaky CDN, indexer hiccup,
poisoned envelope) rather than silently accumulating retries.
- `modules/ethflow-watcher/src/strategy.rs`:
- New const `MAX_BACKOFF_RETRIES = 5`.
- New helper `read_backoff_count` that reads + parses the marker
payload; pre-COW-1083 empty markers decode to 0 so previously-set
backoff: rows still get a fresh attempt (no premature drop on
rollout).
- `apply_submit_retry`'s retriable branch now reads the prior
count, increments, and either writes the new count or upgrades
to `dropped:` (clearing the stale `backoff:`) at the cap.
- Cap-upgrade log line carries the retry-count and message: "...
after 5 retries on transient/unparseable rejection ...".
- 19/19 ethflow-watcher tests pass.
- New `submit_transient_error_at_cap_upgrades_to_dropped_warn`:
seeds `backoff:{uid} = "4"`, triggers a `data: None` rejection
(the unparseable case the issue names explicitly), asserts:
* `dropped:{uid}` is now set
* `backoff:{uid}` is cleared (single outcome marker at rest)
* exactly one Warn log line containing "ethflow dropped" +
"retries"
- New `submit_transient_error_with_legacy_empty_marker_resets_counter`:
backwards-compat — a pre-COW-1083 empty `b""` marker is treated
as count 0, bumped to "1" on first retry rather than prematurely
dropping. Protects in-flight backoffs across the rollout.
- Existing `submit_transient_error_writes_backoff_marker_and_returns`
extended with an assertion that the first retry persists
`backoff:{uid} = "1"`.
- `cargo clippy -p ethflow-watcher --all-targets -- -D warnings`
clean.
Surfaced by jeffersonBastos's PR #55 (M3 mirror) review, thread on
`crates/shepherd-sdk/src/cow/error.rs:82`. Latent in normal
operation (the host forwards parseable envelopes after COW-1075, so
`classify_api_error` returns `Drop` for permanent rejections), but
the gap fires when the orderbook returns a non-JSON 4xx body
(e.g. an HTML error page from a CDN) or if a future host change
accidentally drops the envelope again. Bounded retry semantics
close the latent risk without changing the safe-default
classification (still `TryNextBlock` on `None` data — that part is
explicitly out of scope per the issue).
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
12 review threads addressed end-to-end. Net diff is -720 lines despite adding ~200 lines of new helpers + tests, because the WitBindgenHost adapter deduplication alone wipes ~400 lines. Per-thread: #1 (balance-tracker architecture): refactored to match the M3 host-trait+adapter split the other 4 modules use. Created `strategy.rs` with `on_block(&impl Host, ...)`, moved check_one / fetch_balance / parse_balance_hex / parse_settings into it, converted parse_config to use SDK config helpers + typed HostError instead of String. Added 3 MockHost-driven tests covering first-seen-above-threshold, below-threshold-persist, and error-does-not-abort-loop. #2 + #3 (WitBindgenHost dedup): new `shepherd_sdk::bind_host_via_wit_bindgen!()` declarative macro. Single source of truth in `crates/shepherd-sdk/src/wit_bindgen_macro.rs`; the 4 trait impls + convert_err / sdk_err_into_wit / convert_level collapse to one macro invocation per module. Migrated all 5 modules (twap-monitor, ethflow-watcher, price-alert, stop-loss, balance-tracker). Each module's lib.rs lost ~80 lines. #4 (scale_decimal + config_get dup): new `shepherd_sdk::config` with `get_required`, `get_optional`, `scale_decimal`, and a typed `ConfigError` enum (host-neutral). price-alert + stop-loss consume the SDK helpers; their local duplicates were deleted. Module-level decimal-parsing tests removed (covered by 7 SDK tests + 4 proptest cases now). #5 (Chainlink dup): new `shepherd_sdk::chain::chainlink` with `read_latest_answer(host, chain_id, oracle, domain) -> Option<I256>`. Encapsulates the eth_call → parse → ABI decode flow + Warn logging. price-alert + stop-loss now call the helper; their local AggregatorV3 sol! definitions + read_oracle / on_block oracle plumbing was deleted. SDK ships with 3 StubHost tests covering happy path, host error, and garbage-hex. #6 (WIT world capability elision): added new "Capability enforcement vs. the WIT world" section to ADR-0009 documenting that price-alert + balance-tracker compile against the shepherd:cow/shepherd supertype but their manifests omit cow-api, and that boot success depends on wasm-tools' unused- import elision. Flagged as load-bearing; M5 macro hardening path documented. #7 (poll-time revert classification inert): filed COW-1082 for the host-side fix (forward structured eth_call error data into HostError.data; analogous to COW-1075 for orderbook). #8 (classify_api_error retry-default unbounded): filed COW-1083 for the rate-limit / max-retry follow-up on the backoff: marker. #9 (RetryAction::Backoff dead variant): no code change; replied to thread clarifying it is reserved API surface waiting on a richer upstream retry_hint shape (open question for mfw78). #10 (no proptest anywhere): added `proptest` to shepherd-sdk dev-dependencies. New `crates/shepherd-sdk/src/proptests.rs` with 6 properties covering eth_call_params/parse_eth_call_result round-trip, parse_eth_call_result rejection on unquoted input, config::scale_decimal round-trip + sign-preservation, U256 LE byte round-trip, and no-panic guards for decode_revert_hex + gpv2_to_order_data marker dispatch. #11 (ethflow chain capability least-privilege): moved `chain` from required to optional in `modules/ethflow-watcher/module.toml`, mirroring the M2 mirror fix already applied. #12 (ADR-0009 test-count census): dropped the "145 host tests (twap 20, ethflow 12, ...)" breakdown; kept the qualitative claim. CI is now the authoritative count. Drive-by: alloy-sol-types moved from regular to dev-dependencies in price-alert and stop-loss now that the Chainlink ABI helper is inside shepherd-sdk and the modules only use sol! in their test helpers. Validation: - cargo test --workspace: every crate green; 5 modules + SDK + sdk-test + engine all pass. 8 host tests gained on balance-tracker; 6 proptest props gained on shepherd-sdk; 3 Chainlink helper tests gained. - cargo clippy --workspace --all-targets -- -D warnings: clean. - cargo fmt --check: clean. - cargo build --target wasm32-wasip2 --release for all 5 modules: clean. - Zero em-dashes in source code added.
brunota20
added a commit
that referenced
this pull request
Jun 26, 2026
…W-1082) The chain backend previously dropped alloy's structured `RpcError::ErrorResp` payload on the floor — the formatted error string went into `HostError.message`, but `HostError.data` stayed `None` and `HostError.code` was hard-coded to `-32603`. That made the twap-monitor's poll-time revert classifier inert on real traffic: `OrderNotValid` / `PollNever` / `PollTryAtBlock` / `PollTryAtEpoch` all fell through to `TryNextBlock` because `decode_revert_hex` only fires on a non-empty `err.data`. This change wires the structured payload through end-to-end. - `crates/nexum-engine/src/host/provider_pool.rs`: when alloy's `provider.raw_request` fails with an `RpcError::ErrorResp`, the pool now captures both `payload.code` (as `Option<i64>` so we can distinguish "no ErrorResp" from "ErrorResp with code 0") and `payload.data` (as `Option<String>`, the JSON-encoded revert hex) and surfaces them on `ProviderError::Rpc`. Transport-side failures (timeout, websocket disconnect) leave both `None`. The two subscribe paths (`subscribe_blocks`, `subscribe_logs`) keep `code: None, data: None` since they don't carry an ErrorResp. - `crates/nexum-engine/src/host/impls/chain.rs`: extract the `ProviderError -> HostError` projection into a free helper `provider_error_to_host_error`. The `Rpc` arm forwards the structured `data` verbatim, preserves the node-reported code (saturating out-of-`i32` values to `-32603`), and falls back to `-32603` only when no `ErrorResp` was present. Five unit tests cover: revert with data, transport failure with `None`, out-of-range code, unknown-chain, and invalid-params. - `modules/twap-monitor/src/strategy.rs`: update the stale comment on the `decode_revert_hex` branch — that branch is now live on real traffic, the only `None` path is transport-level failures (which keep the safe `TryNextBlock` default). No incorrect order is ever submitted (the contract reverts; the orderbook never sees a bad body). The issue is pruning efficiency: a permanently dead TWAP watch was re-polled every block until a submit eventually failed for an unrelated reason, and the local-store filled with `watch:` entries the strategy could otherwise drop on the first revert. With this fix the SDK-side classifier dispatches `Drop` / gate on the first revert, matching the documented expectation in `docs/adr/0007-upstream-protocol-logic-to-cow-rs.md`. - 70/70 nexum-engine tests pass - 23/23 twap-monitor tests pass - 5/5 new chain.rs projection tests pass (revert-with-data, transport-fail, out-of-range-code, unknown-chain, invalid-params) - `cargo clippy -p nexum-engine -p twap-monitor --all-targets -- -D warnings` clean jeffersonBastos's PR #55 (M3 mirror) review, thread on `modules/twap-monitor/src/strategy.rs:189`. The mirror of this fix on the cow-api side is COW-1075 (already merged via PR #48).
brunota20
added a commit
that referenced
this pull request
Jun 26, 2026
…OW-1083)
The strategy's `apply_submit_retry` previously wrote an empty
`backoff:{uid}` marker on every retriable submit failure (including
the `TryNextBlock` fallback for unparseable orderbook envelopes). The
marker was a presence flag with no payload, so on every supervisor
reconnect / engine restart the same dead placement would retry
indefinitely — bounded only by log re-delivery frequency.
This change persists a per-UID retry count in the marker's value
(ASCII `u32`) and upgrades to `dropped:` after `MAX_BACKOFF_RETRIES =
5` consecutive retries. The upgrade emits a Warn-level log line so
the operator sees the structural issue (flaky CDN, indexer hiccup,
poisoned envelope) rather than silently accumulating retries.
- `modules/ethflow-watcher/src/strategy.rs`:
- New const `MAX_BACKOFF_RETRIES = 5`.
- New helper `read_backoff_count` that reads + parses the marker
payload; pre-COW-1083 empty markers decode to 0 so previously-set
backoff: rows still get a fresh attempt (no premature drop on
rollout).
- `apply_submit_retry`'s retriable branch now reads the prior
count, increments, and either writes the new count or upgrades
to `dropped:` (clearing the stale `backoff:`) at the cap.
- Cap-upgrade log line carries the retry-count and message: "...
after 5 retries on transient/unparseable rejection ...".
- 19/19 ethflow-watcher tests pass.
- New `submit_transient_error_at_cap_upgrades_to_dropped_warn`:
seeds `backoff:{uid} = "4"`, triggers a `data: None` rejection
(the unparseable case the issue names explicitly), asserts:
* `dropped:{uid}` is now set
* `backoff:{uid}` is cleared (single outcome marker at rest)
* exactly one Warn log line containing "ethflow dropped" +
"retries"
- New `submit_transient_error_with_legacy_empty_marker_resets_counter`:
backwards-compat — a pre-COW-1083 empty `b""` marker is treated
as count 0, bumped to "1" on first retry rather than prematurely
dropping. Protects in-flight backoffs across the rollout.
- Existing `submit_transient_error_writes_backoff_marker_and_returns`
extended with an assertion that the first retry persists
`backoff:{uid} = "1"`.
- `cargo clippy -p ethflow-watcher --all-targets -- -D warnings`
clean.
Surfaced by jeffersonBastos's PR #55 (M3 mirror) review, thread on
`crates/shepherd-sdk/src/cow/error.rs:82`. Latent in normal
operation (the host forwards parseable envelopes after COW-1075, so
`classify_api_error` returns `Drop` for permanent rejections), but
the gap fires when the orderbook returns a non-JSON 4xx body
(e.g. an HTML error page from a CDN) or if a future host change
accidentally drops the envelope again. Bounded retry semantics
close the latent risk without changing the safe-default
classification (still `TryNextBlock` on `None` data — that part is
explicitly out of scope per the issue).
jean-neiverth
pushed a commit
that referenced
this pull request
Jun 29, 2026
…W-1082) The chain backend previously dropped alloy's structured `RpcError::ErrorResp` payload on the floor — the formatted error string went into `HostError.message`, but `HostError.data` stayed `None` and `HostError.code` was hard-coded to `-32603`. That made the twap-monitor's poll-time revert classifier inert on real traffic: `OrderNotValid` / `PollNever` / `PollTryAtBlock` / `PollTryAtEpoch` all fell through to `TryNextBlock` because `decode_revert_hex` only fires on a non-empty `err.data`. This change wires the structured payload through end-to-end. - `crates/nexum-engine/src/host/provider_pool.rs`: when alloy's `provider.raw_request` fails with an `RpcError::ErrorResp`, the pool now captures both `payload.code` (as `Option<i64>` so we can distinguish "no ErrorResp" from "ErrorResp with code 0") and `payload.data` (as `Option<String>`, the JSON-encoded revert hex) and surfaces them on `ProviderError::Rpc`. Transport-side failures (timeout, websocket disconnect) leave both `None`. The two subscribe paths (`subscribe_blocks`, `subscribe_logs`) keep `code: None, data: None` since they don't carry an ErrorResp. - `crates/nexum-engine/src/host/impls/chain.rs`: extract the `ProviderError -> HostError` projection into a free helper `provider_error_to_host_error`. The `Rpc` arm forwards the structured `data` verbatim, preserves the node-reported code (saturating out-of-`i32` values to `-32603`), and falls back to `-32603` only when no `ErrorResp` was present. Five unit tests cover: revert with data, transport failure with `None`, out-of-range code, unknown-chain, and invalid-params. - `modules/twap-monitor/src/strategy.rs`: update the stale comment on the `decode_revert_hex` branch — that branch is now live on real traffic, the only `None` path is transport-level failures (which keep the safe `TryNextBlock` default). No incorrect order is ever submitted (the contract reverts; the orderbook never sees a bad body). The issue is pruning efficiency: a permanently dead TWAP watch was re-polled every block until a submit eventually failed for an unrelated reason, and the local-store filled with `watch:` entries the strategy could otherwise drop on the first revert. With this fix the SDK-side classifier dispatches `Drop` / gate on the first revert, matching the documented expectation in `docs/adr/0007-upstream-protocol-logic-to-cow-rs.md`. - 70/70 nexum-engine tests pass - 23/23 twap-monitor tests pass - 5/5 new chain.rs projection tests pass (revert-with-data, transport-fail, out-of-range-code, unknown-chain, invalid-params) - `cargo clippy -p nexum-engine -p twap-monitor --all-targets -- -D warnings` clean jeffersonBastos's PR #55 (M3 mirror) review, thread on `modules/twap-monitor/src/strategy.rs:189`. The mirror of this fix on the cow-api side is COW-1075 (already merged via PR #48).
jean-neiverth
pushed a commit
that referenced
this pull request
Jun 29, 2026
…OW-1083)
The strategy's `apply_submit_retry` previously wrote an empty
`backoff:{uid}` marker on every retriable submit failure (including
the `TryNextBlock` fallback for unparseable orderbook envelopes). The
marker was a presence flag with no payload, so on every supervisor
reconnect / engine restart the same dead placement would retry
indefinitely — bounded only by log re-delivery frequency.
This change persists a per-UID retry count in the marker's value
(ASCII `u32`) and upgrades to `dropped:` after `MAX_BACKOFF_RETRIES =
5` consecutive retries. The upgrade emits a Warn-level log line so
the operator sees the structural issue (flaky CDN, indexer hiccup,
poisoned envelope) rather than silently accumulating retries.
- `modules/ethflow-watcher/src/strategy.rs`:
- New const `MAX_BACKOFF_RETRIES = 5`.
- New helper `read_backoff_count` that reads + parses the marker
payload; pre-COW-1083 empty markers decode to 0 so previously-set
backoff: rows still get a fresh attempt (no premature drop on
rollout).
- `apply_submit_retry`'s retriable branch now reads the prior
count, increments, and either writes the new count or upgrades
to `dropped:` (clearing the stale `backoff:`) at the cap.
- Cap-upgrade log line carries the retry-count and message: "...
after 5 retries on transient/unparseable rejection ...".
- 19/19 ethflow-watcher tests pass.
- New `submit_transient_error_at_cap_upgrades_to_dropped_warn`:
seeds `backoff:{uid} = "4"`, triggers a `data: None` rejection
(the unparseable case the issue names explicitly), asserts:
* `dropped:{uid}` is now set
* `backoff:{uid}` is cleared (single outcome marker at rest)
* exactly one Warn log line containing "ethflow dropped" +
"retries"
- New `submit_transient_error_with_legacy_empty_marker_resets_counter`:
backwards-compat — a pre-COW-1083 empty `b""` marker is treated
as count 0, bumped to "1" on first retry rather than prematurely
dropping. Protects in-flight backoffs across the rollout.
- Existing `submit_transient_error_writes_backoff_marker_and_returns`
extended with an assertion that the first retry persists
`backoff:{uid} = "1"`.
- `cargo clippy -p ethflow-watcher --all-targets -- -D warnings`
clean.
Surfaced by jeffersonBastos's PR #55 (M3 mirror) review, thread on
`crates/shepherd-sdk/src/cow/error.rs:82`. Latent in normal
operation (the host forwards parseable envelopes after COW-1075, so
`classify_api_error` returns `Drop` for permanent rejections), but
the gap fires when the orderbook returns a non-JSON 4xx body
(e.g. an HTML error page from a CDN) or if a future host change
accidentally drops the envelope again. Bounded retry semantics
close the latent risk without changing the safe-default
classification (still `TryNextBlock` on `None` data — that part is
explicitly out of scope per the issue).
jean-neiverth
pushed a commit
that referenced
this pull request
Jun 30, 2026
…W-1082) The chain backend previously dropped alloy's structured `RpcError::ErrorResp` payload on the floor — the formatted error string went into `HostError.message`, but `HostError.data` stayed `None` and `HostError.code` was hard-coded to `-32603`. That made the twap-monitor's poll-time revert classifier inert on real traffic: `OrderNotValid` / `PollNever` / `PollTryAtBlock` / `PollTryAtEpoch` all fell through to `TryNextBlock` because `decode_revert_hex` only fires on a non-empty `err.data`. This change wires the structured payload through end-to-end. - `crates/nexum-engine/src/host/provider_pool.rs`: when alloy's `provider.raw_request` fails with an `RpcError::ErrorResp`, the pool now captures both `payload.code` (as `Option<i64>` so we can distinguish "no ErrorResp" from "ErrorResp with code 0") and `payload.data` (as `Option<String>`, the JSON-encoded revert hex) and surfaces them on `ProviderError::Rpc`. Transport-side failures (timeout, websocket disconnect) leave both `None`. The two subscribe paths (`subscribe_blocks`, `subscribe_logs`) keep `code: None, data: None` since they don't carry an ErrorResp. - `crates/nexum-engine/src/host/impls/chain.rs`: extract the `ProviderError -> HostError` projection into a free helper `provider_error_to_host_error`. The `Rpc` arm forwards the structured `data` verbatim, preserves the node-reported code (saturating out-of-`i32` values to `-32603`), and falls back to `-32603` only when no `ErrorResp` was present. Five unit tests cover: revert with data, transport failure with `None`, out-of-range code, unknown-chain, and invalid-params. - `modules/twap-monitor/src/strategy.rs`: update the stale comment on the `decode_revert_hex` branch — that branch is now live on real traffic, the only `None` path is transport-level failures (which keep the safe `TryNextBlock` default). No incorrect order is ever submitted (the contract reverts; the orderbook never sees a bad body). The issue is pruning efficiency: a permanently dead TWAP watch was re-polled every block until a submit eventually failed for an unrelated reason, and the local-store filled with `watch:` entries the strategy could otherwise drop on the first revert. With this fix the SDK-side classifier dispatches `Drop` / gate on the first revert, matching the documented expectation in `docs/adr/0007-upstream-protocol-logic-to-cow-rs.md`. - 70/70 nexum-engine tests pass - 23/23 twap-monitor tests pass - 5/5 new chain.rs projection tests pass (revert-with-data, transport-fail, out-of-range-code, unknown-chain, invalid-params) - `cargo clippy -p nexum-engine -p twap-monitor --all-targets -- -D warnings` clean jeffersonBastos's PR #55 (M3 mirror) review, thread on `modules/twap-monitor/src/strategy.rs:189`. The mirror of this fix on the cow-api side is COW-1075 (already merged via PR #48).
jean-neiverth
pushed a commit
that referenced
this pull request
Jun 30, 2026
…OW-1083)
The strategy's `apply_submit_retry` previously wrote an empty
`backoff:{uid}` marker on every retriable submit failure (including
the `TryNextBlock` fallback for unparseable orderbook envelopes). The
marker was a presence flag with no payload, so on every supervisor
reconnect / engine restart the same dead placement would retry
indefinitely — bounded only by log re-delivery frequency.
This change persists a per-UID retry count in the marker's value
(ASCII `u32`) and upgrades to `dropped:` after `MAX_BACKOFF_RETRIES =
5` consecutive retries. The upgrade emits a Warn-level log line so
the operator sees the structural issue (flaky CDN, indexer hiccup,
poisoned envelope) rather than silently accumulating retries.
- `modules/ethflow-watcher/src/strategy.rs`:
- New const `MAX_BACKOFF_RETRIES = 5`.
- New helper `read_backoff_count` that reads + parses the marker
payload; pre-COW-1083 empty markers decode to 0 so previously-set
backoff: rows still get a fresh attempt (no premature drop on
rollout).
- `apply_submit_retry`'s retriable branch now reads the prior
count, increments, and either writes the new count or upgrades
to `dropped:` (clearing the stale `backoff:`) at the cap.
- Cap-upgrade log line carries the retry-count and message: "...
after 5 retries on transient/unparseable rejection ...".
- 19/19 ethflow-watcher tests pass.
- New `submit_transient_error_at_cap_upgrades_to_dropped_warn`:
seeds `backoff:{uid} = "4"`, triggers a `data: None` rejection
(the unparseable case the issue names explicitly), asserts:
* `dropped:{uid}` is now set
* `backoff:{uid}` is cleared (single outcome marker at rest)
* exactly one Warn log line containing "ethflow dropped" +
"retries"
- New `submit_transient_error_with_legacy_empty_marker_resets_counter`:
backwards-compat — a pre-COW-1083 empty `b""` marker is treated
as count 0, bumped to "1" on first retry rather than prematurely
dropping. Protects in-flight backoffs across the rollout.
- Existing `submit_transient_error_writes_backoff_marker_and_returns`
extended with an assertion that the first retry persists
`backoff:{uid} = "1"`.
- `cargo clippy -p ethflow-watcher --all-targets -- -D warnings`
clean.
Surfaced by jeffersonBastos's PR #55 (M3 mirror) review, thread on
`crates/shepherd-sdk/src/cow/error.rs:82`. Latent in normal
operation (the host forwards parseable envelopes after COW-1075, so
`classify_api_error` returns `Drop` for permanent rejections), but
the gap fires when the orderbook returns a non-JSON 4xx body
(e.g. an HTML error page from a CDN) or if a future host change
accidentally drops the envelope again. Bounded retry semantics
close the latent risk without changing the safe-default
classification (still `TryNextBlock` on `None` data — that part is
explicitly out of scope per the issue).
jean-neiverth
pushed a commit
that referenced
this pull request
Jun 30, 2026
…W-1082) The chain backend previously dropped alloy's structured `RpcError::ErrorResp` payload on the floor — the formatted error string went into `HostError.message`, but `HostError.data` stayed `None` and `HostError.code` was hard-coded to `-32603`. That made the twap-monitor's poll-time revert classifier inert on real traffic: `OrderNotValid` / `PollNever` / `PollTryAtBlock` / `PollTryAtEpoch` all fell through to `TryNextBlock` because `decode_revert_hex` only fires on a non-empty `err.data`. This change wires the structured payload through end-to-end. - `crates/nexum-engine/src/host/provider_pool.rs`: when alloy's `provider.raw_request` fails with an `RpcError::ErrorResp`, the pool now captures both `payload.code` (as `Option<i64>` so we can distinguish "no ErrorResp" from "ErrorResp with code 0") and `payload.data` (as `Option<String>`, the JSON-encoded revert hex) and surfaces them on `ProviderError::Rpc`. Transport-side failures (timeout, websocket disconnect) leave both `None`. The two subscribe paths (`subscribe_blocks`, `subscribe_logs`) keep `code: None, data: None` since they don't carry an ErrorResp. - `crates/nexum-engine/src/host/impls/chain.rs`: extract the `ProviderError -> HostError` projection into a free helper `provider_error_to_host_error`. The `Rpc` arm forwards the structured `data` verbatim, preserves the node-reported code (saturating out-of-`i32` values to `-32603`), and falls back to `-32603` only when no `ErrorResp` was present. Five unit tests cover: revert with data, transport failure with `None`, out-of-range code, unknown-chain, and invalid-params. - `modules/twap-monitor/src/strategy.rs`: update the stale comment on the `decode_revert_hex` branch — that branch is now live on real traffic, the only `None` path is transport-level failures (which keep the safe `TryNextBlock` default). No incorrect order is ever submitted (the contract reverts; the orderbook never sees a bad body). The issue is pruning efficiency: a permanently dead TWAP watch was re-polled every block until a submit eventually failed for an unrelated reason, and the local-store filled with `watch:` entries the strategy could otherwise drop on the first revert. With this fix the SDK-side classifier dispatches `Drop` / gate on the first revert, matching the documented expectation in `docs/adr/0007-upstream-protocol-logic-to-cow-rs.md`. - 70/70 nexum-engine tests pass - 23/23 twap-monitor tests pass - 5/5 new chain.rs projection tests pass (revert-with-data, transport-fail, out-of-range-code, unknown-chain, invalid-params) - `cargo clippy -p nexum-engine -p twap-monitor --all-targets -- -D warnings` clean jeffersonBastos's PR #55 (M3 mirror) review, thread on `modules/twap-monitor/src/strategy.rs:189`. The mirror of this fix on the cow-api side is COW-1075 (already merged via PR #48).
jean-neiverth
pushed a commit
that referenced
this pull request
Jun 30, 2026
…OW-1083)
The strategy's `apply_submit_retry` previously wrote an empty
`backoff:{uid}` marker on every retriable submit failure (including
the `TryNextBlock` fallback for unparseable orderbook envelopes). The
marker was a presence flag with no payload, so on every supervisor
reconnect / engine restart the same dead placement would retry
indefinitely — bounded only by log re-delivery frequency.
This change persists a per-UID retry count in the marker's value
(ASCII `u32`) and upgrades to `dropped:` after `MAX_BACKOFF_RETRIES =
5` consecutive retries. The upgrade emits a Warn-level log line so
the operator sees the structural issue (flaky CDN, indexer hiccup,
poisoned envelope) rather than silently accumulating retries.
- `modules/ethflow-watcher/src/strategy.rs`:
- New const `MAX_BACKOFF_RETRIES = 5`.
- New helper `read_backoff_count` that reads + parses the marker
payload; pre-COW-1083 empty markers decode to 0 so previously-set
backoff: rows still get a fresh attempt (no premature drop on
rollout).
- `apply_submit_retry`'s retriable branch now reads the prior
count, increments, and either writes the new count or upgrades
to `dropped:` (clearing the stale `backoff:`) at the cap.
- Cap-upgrade log line carries the retry-count and message: "...
after 5 retries on transient/unparseable rejection ...".
- 19/19 ethflow-watcher tests pass.
- New `submit_transient_error_at_cap_upgrades_to_dropped_warn`:
seeds `backoff:{uid} = "4"`, triggers a `data: None` rejection
(the unparseable case the issue names explicitly), asserts:
* `dropped:{uid}` is now set
* `backoff:{uid}` is cleared (single outcome marker at rest)
* exactly one Warn log line containing "ethflow dropped" +
"retries"
- New `submit_transient_error_with_legacy_empty_marker_resets_counter`:
backwards-compat — a pre-COW-1083 empty `b""` marker is treated
as count 0, bumped to "1" on first retry rather than prematurely
dropping. Protects in-flight backoffs across the rollout.
- Existing `submit_transient_error_writes_backoff_marker_and_returns`
extended with an assertion that the first retry persists
`backoff:{uid} = "1"`.
- `cargo clippy -p ethflow-watcher --all-targets -- -D warnings`
clean.
Surfaced by jeffersonBastos's PR #55 (M3 mirror) review, thread on
`crates/shepherd-sdk/src/cow/error.rs:82`. Latent in normal
operation (the host forwards parseable envelopes after COW-1075, so
`classify_api_error` returns `Drop` for permanent rejections), but
the gap fires when the orderbook returns a non-JSON 4xx body
(e.g. an HTML error page from a CDN) or if a future host change
accidentally drops the envelope again. Bounded retry semantics
close the latent risk without changing the safe-default
classification (still `TryNextBlock` on `None` data — that part is
explicitly out of scope per the issue).
jean-neiverth
pushed a commit
that referenced
this pull request
Jun 30, 2026
…W-1082) The chain backend previously dropped alloy's structured `RpcError::ErrorResp` payload on the floor — the formatted error string went into `HostError.message`, but `HostError.data` stayed `None` and `HostError.code` was hard-coded to `-32603`. That made the twap-monitor's poll-time revert classifier inert on real traffic: `OrderNotValid` / `PollNever` / `PollTryAtBlock` / `PollTryAtEpoch` all fell through to `TryNextBlock` because `decode_revert_hex` only fires on a non-empty `err.data`. This change wires the structured payload through end-to-end. - `crates/nexum-engine/src/host/provider_pool.rs`: when alloy's `provider.raw_request` fails with an `RpcError::ErrorResp`, the pool now captures both `payload.code` (as `Option<i64>` so we can distinguish "no ErrorResp" from "ErrorResp with code 0") and `payload.data` (as `Option<String>`, the JSON-encoded revert hex) and surfaces them on `ProviderError::Rpc`. Transport-side failures (timeout, websocket disconnect) leave both `None`. The two subscribe paths (`subscribe_blocks`, `subscribe_logs`) keep `code: None, data: None` since they don't carry an ErrorResp. - `crates/nexum-engine/src/host/impls/chain.rs`: extract the `ProviderError -> HostError` projection into a free helper `provider_error_to_host_error`. The `Rpc` arm forwards the structured `data` verbatim, preserves the node-reported code (saturating out-of-`i32` values to `-32603`), and falls back to `-32603` only when no `ErrorResp` was present. Five unit tests cover: revert with data, transport failure with `None`, out-of-range code, unknown-chain, and invalid-params. - `modules/twap-monitor/src/strategy.rs`: update the stale comment on the `decode_revert_hex` branch — that branch is now live on real traffic, the only `None` path is transport-level failures (which keep the safe `TryNextBlock` default). No incorrect order is ever submitted (the contract reverts; the orderbook never sees a bad body). The issue is pruning efficiency: a permanently dead TWAP watch was re-polled every block until a submit eventually failed for an unrelated reason, and the local-store filled with `watch:` entries the strategy could otherwise drop on the first revert. With this fix the SDK-side classifier dispatches `Drop` / gate on the first revert, matching the documented expectation in `docs/adr/0007-upstream-protocol-logic-to-cow-rs.md`. - 70/70 nexum-engine tests pass - 23/23 twap-monitor tests pass - 5/5 new chain.rs projection tests pass (revert-with-data, transport-fail, out-of-range-code, unknown-chain, invalid-params) - `cargo clippy -p nexum-engine -p twap-monitor --all-targets -- -D warnings` clean jeffersonBastos's PR #55 (M3 mirror) review, thread on `modules/twap-monitor/src/strategy.rs:189`. The mirror of this fix on the cow-api side is COW-1075 (already merged via PR #48).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Internal review mirror of upstream nullislabs#18 — opened so we review, comment, and apply fixes in the fork before it faces the MFW repo.
b40d092(fix/supervisor-alive-on-init-err)7ab804a(bleu main = the merge-base of the upstream PR, so the diff here is identical to upstream)Review surface only — do not merge.