Skip to content

feat(cow): source the registry address from module [config] - #666

Merged
mfw78 merged 1 commit into
mainfrom
cow/651-registry-config
Sep 1, 2026
Merged

mfw78 merged 1 commit into
mainfrom
cow/651-registry-config

Conversation

@mfw78

@mfw78 mfw78 commented Aug 3, 2026 •

Copy link
Copy Markdown
Contributor

What

Sources the ComposableCoW registry address from the twap-monitor manifest [config] and deletes the compiled cowprotocol::COMPOSABLE_COW constant from the module, with no fallback of any kind.

modules/twap-monitor/src/keeper.rs gains KeeperConfig { registry: Address }, parsed through nexum_sdk::config::get_required and held in a nexum_sdk::config::Slot, with a registry field threaded through TwapSource into poll_one's eth_call_params.
modules/twap-monitor/src/lib.rs parses and stores in init, so a parse failure is the init error.
on_block reads the slot and delegates to poll_block, which takes the registry explicitly; before init it is a typed Fault::Internal refusal, which is the SDK's classification for ConfigError::NotInitialized.
modules/twap-monitor/component.toml gains [config] with registry = "0xfdaFc9d1902f4e0b84f65F49f244b32b31013b74", the current Sepolia registry, so behaviour is unchanged.

Why

Closes #651: the converged ccow-monitor knows exactly one contract, the registry, and hard-coding the constant coupled the module to a single network and put a cow-rs rev on the critical path.

This is the first car of the convergence train and changes the mechanism only; the address cutover to the mainnet fork is #652's atomic swap.
That cutover is why the car matters more than its diff suggests: the fork registry is at 0xf9ba6F64c9b41Df1cEe76A50e2039D3847064232, a different address from the constant, so until the address is config-sourced, pointing at the fork means editing a constant in nullislabs/cow-rs and doing a cross-repo rev bump to move an address.
After this it is a one-line manifest edit.

A parity test pins [config].registry to every event trigger address and to the test fixture constant, so the #652 swap must move all three together.
It is kept separate from module_manifests.rs because the two guard different failures: this one is an internal-consistency invariant, that three places agree; that one is an external-fact invariant, that the address is a specific known deployment and its start_block belongs to it.

Notes for review

Two deliberate behaviour changes came with adopting the SDK rather than hand-rolling.

The uninitialised-dispatch refusal is Fault::Internal, not Fault::Unavailable.
config.rs states the rationale: NotInitialized means init returned Ok without storing, a guest bug that never recovers, so it is not a transient unavailability.

Slot is write-once, so a test cannot clear it.
Rather than keep a mutable static to suit the tests, tests call poll_block with an explicit registry and never touch the static; only two tests touch CONFIG, and nextest gives each its own process.
The cost is that those two are nextest-only, where an earlier revision of this PR was runner-independent under cargo test --lib.
just test is nextest and the only cargo test the repo asks for is --doc, so this matches how the repo runs, but it is a real narrowing and worth a reviewer's opinion.

Testing

Rebased onto main after #675 landed the nexum-runtime migration, so the manifest is component.toml, triggers are [[trigger]] on = "event", and the Watch vocabulary is Commitment throughout.

  • just build-modules: green.
  • cargo nextest run --workspace --all-features: 182 passed, 1 skipped.
  • cargo nextest run -p twap-monitor: 36 passed.
  • cargo fmt --all --check and cargo clippy --workspace --all-targets --all-features -- -D warnings: clean.

AI Assistance: Claude Fable used for the original implementation via a structured workflow; Claude Opus used for the red-team review (9 findings, 7 fixed, 1 duplicate-merged, 1 rejected as out-of-scope checksum policy); Claude Code used for the post-migration rebase and the nexum_sdk::config adoption.

@mfw78
mfw78 marked this pull request as ready for review August 3, 2026 23:52
@mfw78
mfw78 force-pushed the cow/651-registry-config branch from 1aa7891 to acf0e06 Compare August 3, 2026 23:56
@mfw78
mfw78 force-pushed the cow/651-registry-config branch from acf0e06 to 336789c Compare September 1, 2026 00:18
@mfw78

mfw78 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main (acf0e06 to 336789c) after shepherd#675 landed the migration.

The manifest moved underneath this PR, so the [config] block relocated from module.toml to component.toml, and the parity test now reads [[trigger]] tables filtered on on = "event" rather than [[subscription]] filtered on kind = "chain-log". Four conflicts in keeper.rs were the same shape: this PR's substance over the migration's renamed API, so nexum_sdk::events::ChainLogParts became nexum_sdk::sol_events::LogParts and watch.key() became commitment.key(). One straggler, seed_watch, is now seed_commitment.

On the test overlap with module_manifests.rs from shepherd#675: keeping both, because they guard different failures. This PR's test is an internal-consistency invariant, that [config].registry, every event trigger address and the test fixture are one address. The module_manifests.rs test is an external-fact invariant, that the address is a specific known deployment and its start_block belongs to that deployment. Collapsing them would lose one or the other.

Verified: 182 tests pass workspace-wide (173 plus this PR's 9), fmt and clippy clean at -D warnings, just build-modules green. COMPOSABLE_COW no longer appears anywhere in the module.

Worth recording why this car matters more than its diff suggests. The fork registry is at 0xf9ba6F64c9b41Df1cEe76A50e2039D3847064232, a different address from the compiled-in constant. Until the address is config-sourced, pointing at the fork means changing a constant in nullislabs/cow-rs and doing a cross-repo rev bump to move an address. After this, it is a one-line manifest edit. That is why shepherd#652 is blocked on shepherd#651.

AI Assistance: Claude Code used for the rebase, the schema migration of the parity test, and verification.

The keeper learns the ComposableCoW registry from its manifest
[config] instead of the compiled cowprotocol::COMPOSABLE_COW
constant. A missing or malformed registry key is a hard init error;
there is no fallback of any kind. A manifest parity test pins
[config].registry to the chain-log subscription address pins (and the
test fixture constant) until the #652 cutover moves them together.

Closes #651
@mfw78
mfw78 force-pushed the cow/651-registry-config branch from 336789c to ba5ab94 Compare September 1, 2026 00:29
@mfw78

mfw78 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Adopted nexum_sdk::config and trimmed the comments (336789c to ba5ab94).

The hand-rolled config machinery is gone. The SDK already provides all of it:

  • Slot<T>, a write-once OnceLock holder that is const fn new() so a static can hold one, replaces RwLock<Option<KeeperConfig>> plus store_config, config, clear_config and the test mutex guard.
  • config::get_required replaces the manual key loop.
  • ConfigError with From<ConfigError> for Fault replaces the hand-built Fault::InvalidInput strings.

Slot is the sanctioned idiom: it is documented in the SDK's crate root with a doctest, and the runtime's own example modules hold parsed config in a static OnceLock<Settings> the same way.

Two consequences worth calling out, both deliberate.

The refusal fault changed from Unavailable to Internal. That is the SDK's classification, with its rationale in config.rs: NotInitialized means init returned Ok without storing, which is a guest bug that never recovers, so it is not a transient unavailability. The test now asserts Internal.

Slot is write-once, so a test cannot clear it. Rather than keep a mutable static to suit the tests, on_block now reads CONFIG and delegates to poll_block, which takes the registry explicitly. Tests call poll_block and never touch the static at all, which is the better shape regardless: the static belongs at the real entry point, not threaded through the test surface. Only two tests touch CONFIG, and each gets a fresh process under nextest.

The trade this makes: those two tests are nextest-only, because cargo test --lib shares one process across the whole binary and the slot cannot be reset. just test is nextest, and the only cargo test the repo asks for is --doc, so this fits how the repo actually runs. Flagging it because the earlier version was deliberately runner-independent.

Also dropped the two message-text assertions in favour of asserting the variant and that the message names the key. Pinning the exact string coupled the test to upstream's wording.

Verified: 182 tests pass workspace-wide, fmt and clippy clean at -D warnings, just build-modules green.

AI Assistance: Claude Code used for the SDK adoption and verification.

@mfw78
mfw78 merged commit 12dab27 into main Sep 1, 2026
6 checks passed
@mfw78
mfw78 deleted the cow/651-registry-config branch September 1, 2026 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cow: source the registry address from module [config]

1 participant