[review-mirror] M2 epic (upstream #17): TWAP + EthFlow + manifests#54
[review-mirror] M2 epic (upstream #17): TWAP + EthFlow + manifests#54jeffersonBastos wants to merge 25 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
Per ADR-0001 (module.toml schema), authored for the two M2
modules:
twap-monitor / module.toml
- capabilities.required = ["logging", "local-store", "chain",
"cow-api"] — matches the Rust imports the BLEU-826/827/828
paths exercise.
- [[subscription]] log on Sepolia (chain_id 11155111) against
ComposableCoW (0xfdaFc9d1902f4e0b84f65F49f244b32b31013b74)
with topic-0 keccak256(
"ConditionalOrderCreated(address,(address,bytes32,bytes))"
) = 0x2cceac5555b0ca45a3744ced542f54b56ad2eb45e521962372eef212a2cbf361.
- [[subscription]] block on Sepolia for the BLEU-827 poll loop.
ethflow-watcher / module.toml
- Same capability set (chain reserved for a future eth_call —
e.g. read the EthFlow refund pointer — without churning the
manifest).
- [[subscription]] log on Sepolia against CoWSwapEthFlow
production (0xbA3cB449bD2B4ADddBc894D8697F5170800EAdeC) with
topic-0 keccak256(
"OrderPlacement(address,(address,address,address,uint256,uint256,
uint32,bytes32,uint256,bytes32,bool,bytes32,bytes32),
(uint8,bytes),bytes)"
) = 0xcf5f9de2984132265203b5c335b25727702ca77262ff622e136baa7362bf1da9.
Both [capabilities.http].allow stay empty: all outbound HTTP
flows through the cow-api capability, which routes via the
host's pinned orderbook URL.
The content hash field is the 0.2 placeholder all-zero sha256;
0.3 will validate it against the loaded component bytes.
Linear: BLEU-834. Ref ADR-0001.
jeffersonBastos
left a comment
There was a problem hiding this comment.
M2 mirror review (= upstream #17)
Blocker: this PR does not contain the M2 module source it describes. modules/twap-monitor/ and modules/ethflow-watcher/ contain only module.toml (no Cargo.toml, no src/, no tests), and the two modules are not workspace members (Cargo.toml still lists only crates/nexum-engine and modules/example). So the description's "20 unit tests / 5 integration tests" and "cargo clippy --workspace clean" cannot hold against this tree. The head branch is feat/m2-module-manifests-bleu-834 (the BLEU-834 manifests ticket); the implementation branches (BLEU-826..833) were never merged into it.
Fix: re-aggregate to include modules/*/{Cargo.toml,src/} plus the two workspace-member entries, then re-confirm clippy/tests.
The reviewable M2 delta that IS present is the two module.toml manifests plus the cow-rs patch bump. The cow-rs patch (rev 57f5f55, BLEU-822/823) is well justified. Inline notes on the manifests below.
| [[subscription]] | ||
| kind = "log" | ||
| chain_id = 11155111 | ||
| address = "0xbA3cB449bD2B4ADddBc894D8697F5170800EAdeC" |
There was a problem hiding this comment.
verify: this subscription is pinned to chain_id = 11155111 (Sepolia), so the address must be the Sepolia CoWSwapEthFlow deployment. Please also soften the comment above: unlike ComposableCoW's CREATE2 address, EthFlow has had multiple per-network and per-version deployments, so "identical on every chain" should not carry into M5 multi-chain config. Confirm this value against the live Sepolia EthFlow.
There was a problem hiding this comment.
Done in #56 (stacked on this branch).
Address itself stays unchanged: 0xbA3cB449bD2B4ADddBc894D8697F5170800EAdeC is verified - matches cow-rs::ETH_FLOW_PRODUCTION AND was observed firing OrderPlacement events live during the COW-1064 dry-run on 2026-06-18 + every load-test run since.
The comment was softened: now cites cowprotocol/ethflowcontract/networks.prod.json as the source, contrasts with ComposableCoW's CREATE2 canonical address, and explicitly warns that M5 multi-chain config MUST re-check per chain_id rather than carry this value over.
| # imports logging, local-store and cow-api today; `chain` is declared | ||
| # because a follow-up may add an eth_call (e.g. to read the EthFlow | ||
| # refund pointer) without churning the manifest. | ||
| required = ["logging", "local-store", "chain", "cow-api"] |
There was a problem hiding this comment.
least-privilege: chain is listed as required, but the comment says the module does not call it yet (a follow-up "may" add an eth_call). Prefer omitting it, or moving it to optional, until BLEU-855 actually uses chain. Declaring an unused capability as required widens the grant for no current need.
There was a problem hiding this comment.
Done in #56 (stacked on this branch). chain moved from required to optional:
-required = ["logging", "local-store", "chain", "cow-api"]
-optional = []
+required = ["logging", "local-store", "cow-api"]
+optional = ["chain"]The comment now frames optional as "future-proofing without widening the required grant"; when BLEU-855 (or any other follow-up) actually calls chain, it can use it without a manifest change.
| [[subscription]] | ||
| kind = "log" | ||
| chain_id = 1 | ||
| address = "0xC92E8bdf79f0507f65a392b0ab4667716BFE0110" # ComposableCoW |
There was a problem hiding this comment.
This documented example contradicts the shipped manifest: it shows chain_id = 1 and 0xC92E8bdf...0110, but modules/twap-monitor/module.toml uses chain_id = 11155111 and the canonical ComposableCoW 0xfdaFc9d1...3b74. 0xC92E8... is not ComposableCoW. Align the README example with the shipped Sepolia manifest.
There was a problem hiding this comment.
Good catch - that was the GPv2VaultRelayer address (scripts/lib.sh has it as GPV2_VAULT_RELAYER), not ComposableCoW. Fixed in #56 (stacked on this branch): swapped to the canonical 0xfdaFc9d1902f4e0b84f65F49f244b32b31013b74 with an expanded inline comment noting it is the same on every supported chain.
Kept chain_id = 1 so the README example reads as a "conceptual mainnet" snippet; the shipped Sepolia manifest in modules/twap-monitor/module.toml uses chain_id = 11155111 with the same address, which is fine since ComposableCoW is CREATE2 canonical.
Three threads from the internal review mirror of upstream nullislabs/shepherd PR #17: 1. ethflow-watcher/module.toml capabilities: move `chain` from required to optional. The comment on the original manifest already said the module does not call `chain` today; declaring it as required widened the grant for a capability the module does not exercise. Optional keeps "future-proofing" (BLEU-855 can use it without manifest churn) without violating least-privilege. 2. ethflow-watcher/module.toml subscription comment: soften the "identical on every chain" claim. cow-rs::ETH_FLOW_PRODUCTION is identical across chains today, but unlike ComposableCoW's CREATE2 address EthFlow has had multiple per-network and per-version deployments historically. Multi-chain config in M5 must re-check per `chain_id` instead of assuming the address carries. Address itself stays unchanged: 0xbA3cB449bD2B4ADddBc894D8697F5170800EAdeC is verified against the live Sepolia deployment (event firing observed in the COW-1064 dry-run on 2026-06-18 + cow-rs canonical constant + multiple load-test runs). 3. README.md module manifest example: the documented `address` field said `0xC92E8bdf79f0507f65a392b0ab4667716BFE0110` labeled "ComposableCoW", but that is the GPv2VaultRelayer (per scripts/lib.sh). ComposableCoW is `0xfdaFc9d1902f4e0b84f65F49f244b32b31013b74`. Fixed the address; expanded the comment to clarify it is the canonical CREATE2 address (same on every supported chain). Stays on `feat/m2-module-manifests-bleu-834` as a stacked branch so upstream PR #17 + internal mirror PR #54 can see the fixes as a separate, atomic commit. AI assistance disclosure: drafted by Claude (Opus 4.7, 1M context).
…ance) Filtered subset of the compliance applied in PRs #66/#67 of bleu/nullis-shepherd, restricted to files that exist on the M2 epic head. M3+ files (shepherd-sdk, examples, backtest, deploy artifacts) and M4-coupled hunks (ProviderError typed-source variants, JoinSet reconnect tasks, supervisor restart helper) are skipped — they land via their own upstream PRs. Brings M2 epic in line with the repo-wide rust rubric (typed errors, no anyhow in libs, em-dash sweep, #[non_exhaustive] on public error enums). AI Assistance: Claude Code (Opus 4.7) applied a filtered patch derived from PR #67 in the bleu fork. A human (Bruno) is accountable for the result.
|
Work landed via |
Three threads from the internal review mirror of upstream nullislabs/shepherd PR #17: 1. ethflow-watcher/module.toml capabilities: move `chain` from required to optional. The comment on the original manifest already said the module does not call `chain` today; declaring it as required widened the grant for a capability the module does not exercise. Optional keeps "future-proofing" (BLEU-855 can use it without manifest churn) without violating least-privilege. 2. ethflow-watcher/module.toml subscription comment: soften the "identical on every chain" claim. cow-rs::ETH_FLOW_PRODUCTION is identical across chains today, but unlike ComposableCoW's CREATE2 address EthFlow has had multiple per-network and per-version deployments historically. Multi-chain config in M5 must re-check per `chain_id` instead of assuming the address carries. Address itself stays unchanged: 0xbA3cB449bD2B4ADddBc894D8697F5170800EAdeC is verified against the live Sepolia deployment (event firing observed in the COW-1064 dry-run on 2026-06-18 + cow-rs canonical constant + multiple load-test runs). 3. README.md module manifest example: the documented `address` field said `0xC92E8bdf79f0507f65a392b0ab4667716BFE0110` labeled "ComposableCoW", but that is the GPv2VaultRelayer (per scripts/lib.sh). ComposableCoW is `0xfdaFc9d1902f4e0b84f65F49f244b32b31013b74`. Fixed the address; expanded the comment to clarify it is the canonical CREATE2 address (same on every supported chain). Stays on `feat/m2-module-manifests-bleu-834` as a stacked branch so upstream PR #17 + internal mirror PR #54 can see the fixes as a separate, atomic commit.
Three threads from the internal review mirror of upstream nullislabs/shepherd PR #17: 1. ethflow-watcher/module.toml capabilities: move `chain` from required to optional. The comment on the original manifest already said the module does not call `chain` today; declaring it as required widened the grant for a capability the module does not exercise. Optional keeps "future-proofing" (BLEU-855 can use it without manifest churn) without violating least-privilege. 2. ethflow-watcher/module.toml subscription comment: soften the "identical on every chain" claim. cow-rs::ETH_FLOW_PRODUCTION is identical across chains today, but unlike ComposableCoW's CREATE2 address EthFlow has had multiple per-network and per-version deployments historically. Multi-chain config in M5 must re-check per `chain_id` instead of assuming the address carries. Address itself stays unchanged: 0xbA3cB449bD2B4ADddBc894D8697F5170800EAdeC is verified against the live Sepolia deployment (event firing observed in the COW-1064 dry-run on 2026-06-18 + cow-rs canonical constant + multiple load-test runs). 3. README.md module manifest example: the documented `address` field said `0xC92E8bdf79f0507f65a392b0ab4667716BFE0110` labeled "ComposableCoW", but that is the GPv2VaultRelayer (per scripts/lib.sh). ComposableCoW is `0xfdaFc9d1902f4e0b84f65F49f244b32b31013b74`. Fixed the address; expanded the comment to clarify it is the canonical CREATE2 address (same on every supported chain). Stays on `feat/m2-module-manifests-bleu-834` as a stacked branch so upstream PR #17 + internal mirror PR #54 can see the fixes as a separate, atomic commit.
Three threads from the internal review mirror of upstream nullislabs/shepherd PR #17: 1. ethflow-watcher/module.toml capabilities: move `chain` from required to optional. The comment on the original manifest already said the module does not call `chain` today; declaring it as required widened the grant for a capability the module does not exercise. Optional keeps "future-proofing" (BLEU-855 can use it without manifest churn) without violating least-privilege. 2. ethflow-watcher/module.toml subscription comment: soften the "identical on every chain" claim. cow-rs::ETH_FLOW_PRODUCTION is identical across chains today, but unlike ComposableCoW's CREATE2 address EthFlow has had multiple per-network and per-version deployments historically. Multi-chain config in M5 must re-check per `chain_id` instead of assuming the address carries. Address itself stays unchanged: 0xbA3cB449bD2B4ADddBc894D8697F5170800EAdeC is verified against the live Sepolia deployment (event firing observed in the COW-1064 dry-run on 2026-06-18 + cow-rs canonical constant + multiple load-test runs). 3. README.md module manifest example: the documented `address` field said `0xC92E8bdf79f0507f65a392b0ab4667716BFE0110` labeled "ComposableCoW", but that is the GPv2VaultRelayer (per scripts/lib.sh). ComposableCoW is `0xfdaFc9d1902f4e0b84f65F49f244b32b31013b74`. Fixed the address; expanded the comment to clarify it is the canonical CREATE2 address (same on every supported chain). Stays on `feat/m2-module-manifests-bleu-834` as a stacked branch so upstream PR #17 + internal mirror PR #54 can see the fixes as a separate, atomic commit.
Three threads from the internal review mirror of upstream nullislabs/shepherd PR #17: 1. ethflow-watcher/module.toml capabilities: move `chain` from required to optional. The comment on the original manifest already said the module does not call `chain` today; declaring it as required widened the grant for a capability the module does not exercise. Optional keeps "future-proofing" (BLEU-855 can use it without manifest churn) without violating least-privilege. 2. ethflow-watcher/module.toml subscription comment: soften the "identical on every chain" claim. cow-rs::ETH_FLOW_PRODUCTION is identical across chains today, but unlike ComposableCoW's CREATE2 address EthFlow has had multiple per-network and per-version deployments historically. Multi-chain config in M5 must re-check per `chain_id` instead of assuming the address carries. Address itself stays unchanged: 0xbA3cB449bD2B4ADddBc894D8697F5170800EAdeC is verified against the live Sepolia deployment (event firing observed in the COW-1064 dry-run on 2026-06-18 + cow-rs canonical constant + multiple load-test runs). 3. README.md module manifest example: the documented `address` field said `0xC92E8bdf79f0507f65a392b0ab4667716BFE0110` labeled "ComposableCoW", but that is the GPv2VaultRelayer (per scripts/lib.sh). ComposableCoW is `0xfdaFc9d1902f4e0b84f65F49f244b32b31013b74`. Fixed the address; expanded the comment to clarify it is the canonical CREATE2 address (same on every supported chain). Stays on `feat/m2-module-manifests-bleu-834` as a stacked branch so upstream PR #17 + internal mirror PR #54 can see the fixes as a separate, atomic commit.
Three threads from the internal review mirror of upstream nullislabs/shepherd PR #17: 1. ethflow-watcher/module.toml capabilities: move `chain` from required to optional. The comment on the original manifest already said the module does not call `chain` today; declaring it as required widened the grant for a capability the module does not exercise. Optional keeps "future-proofing" (BLEU-855 can use it without manifest churn) without violating least-privilege. 2. ethflow-watcher/module.toml subscription comment: soften the "identical on every chain" claim. cow-rs::ETH_FLOW_PRODUCTION is identical across chains today, but unlike ComposableCoW's CREATE2 address EthFlow has had multiple per-network and per-version deployments historically. Multi-chain config in M5 must re-check per `chain_id` instead of assuming the address carries. Address itself stays unchanged: 0xbA3cB449bD2B4ADddBc894D8697F5170800EAdeC is verified against the live Sepolia deployment (event firing observed in the COW-1064 dry-run on 2026-06-18 + cow-rs canonical constant + multiple load-test runs). 3. README.md module manifest example: the documented `address` field said `0xC92E8bdf79f0507f65a392b0ab4667716BFE0110` labeled "ComposableCoW", but that is the GPv2VaultRelayer (per scripts/lib.sh). ComposableCoW is `0xfdaFc9d1902f4e0b84f65F49f244b32b31013b74`. Fixed the address; expanded the comment to clarify it is the canonical CREATE2 address (same on every supported chain). Stays on `feat/m2-module-manifests-bleu-834` as a stacked branch so upstream PR #17 + internal mirror PR #54 can see the fixes as a separate, atomic commit.
Three threads from the internal review mirror of upstream nullislabs/shepherd PR #17: 1. ethflow-watcher/module.toml capabilities: move `chain` from required to optional. The comment on the original manifest already said the module does not call `chain` today; declaring it as required widened the grant for a capability the module does not exercise. Optional keeps "future-proofing" (BLEU-855 can use it without manifest churn) without violating least-privilege. 2. ethflow-watcher/module.toml subscription comment: soften the "identical on every chain" claim. cow-rs::ETH_FLOW_PRODUCTION is identical across chains today, but unlike ComposableCoW's CREATE2 address EthFlow has had multiple per-network and per-version deployments historically. Multi-chain config in M5 must re-check per `chain_id` instead of assuming the address carries. Address itself stays unchanged: 0xbA3cB449bD2B4ADddBc894D8697F5170800EAdeC is verified against the live Sepolia deployment (event firing observed in the COW-1064 dry-run on 2026-06-18 + cow-rs canonical constant + multiple load-test runs). 3. README.md module manifest example: the documented `address` field said `0xC92E8bdf79f0507f65a392b0ab4667716BFE0110` labeled "ComposableCoW", but that is the GPv2VaultRelayer (per scripts/lib.sh). ComposableCoW is `0xfdaFc9d1902f4e0b84f65F49f244b32b31013b74`. Fixed the address; expanded the comment to clarify it is the canonical CREATE2 address (same on every supported chain). Stays on `feat/m2-module-manifests-bleu-834` as a stacked branch so upstream PR #17 + internal mirror PR #54 can see the fixes as a separate, atomic commit.
Internal review mirror of upstream nullislabs#17 — opened so we review, comment, and apply fixes in the fork before it faces the MFW repo.
7da5636(feat/m2-module-manifests-bleu-834)7ab804a(bleu main = the merge-base of the upstream PR, so the diff here is identical to upstream)module.tomlfiles but not the TWAP/EthFlow module source (src/), so the modules are not workspace members and the body's "20 unit tests / 5 integration tests" cannot be exercised from this tree. Review surface only — do not merge.