refactor: rename orchestrator/contributor naming to router/operator - #186
Conversation
BreadrichEngels
left a comment
There was a problem hiding this comment.
Clean post-#184 cleanup — the orchestrator/contributor → router/operator rename is applied consistently across the coupled surfaces, which is the thing that matters for a rename like this. LGTM.
Traced the couplings that would actually break if a rename were half-applied, and they all move together:
- File renames are consistent end to end.
public_orchestrator.json→public_router.json(node volume mounts + the--routerarg + node/router READMEs) androuter_orchestrator.json→router_key.json(router volume +--key-file+ thelocal-integrationCI invocation + router README). The renamed files aresimilarity index 100%, so content is untouched and theRouterConfigstruct still parses the sameg2_*/address/portfields (serde keys unchanged). submitter.rsis a pure variable rename (contributor→operator) in the bitmap→operator resolution loop — no logic change to the block-pinned-read path.- Removing the
CONTRIBUTOR_*_KEYFILEenv vars is safe — compose mounts eachtestacc*.private.bls.key.jsonto/app/key.jsondirectly, so those vars were dead. And thescripts/READMErewrite corrects real staleness (drops the submodule step, fixes ports to 3001-3003/4000, thecommonware-avs-scriptspackage name, and the two workflow files).
One thing worth validating (the unit tests can't catch it): the --orchestrator → --router change is a CLI-interface rename between docker-compose.yml and the node binary. Your cargo test covers the code, but a compose↔binary flag mismatch only shows up at runtime — and local-integration-test.yml "pulls prebuilt node images." If that image is a stale tag built before this commit, it still expects --orchestrator and clap will reject the new --router flag, failing the run. Please confirm the node image the compose e2e pulls is built from this branch (or run the docker-compose / local-integration e2e once) so the flag rename is exercised end-to-end, not just the unit suite.
Minor: deleting config/orchestrator.json is fine per your repo-wide grep, but an in-repo grep can't see external deploy/ops scripts — just worth a glance that nothing outside the repo referenced that path (it's only a local test key, so low stakes either way).
CI isn't reporting on the head commit yet — confirm fmt/clippy/check/the 56 tests are green. Approving.
Summary
Orchestrator/Contributornaming from the deleted star-topology system (CLI flags, struct/fn/var names, config filenames) to the currentrouter/operatorterminology used everywhere else post-feat: integrate commonware-consensus aggregation engine #184config/orchestrator.json, an unreferenced duplicate ofrouter_key.json(formerlyrouter_orchestrator.json)CONTRIBUTOR_1/2/3_KEYFILEentries fromexample.env(unused since nodes now get keys via direct file mounts, not env-var indirection)scripts/README.md, which still described a pre-monorepo workflow (submodules, wrong ports, wrong CI file/branches, wrong package name)Test plan
cargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warningscargo check --workspace --all-targetscargo test --workspace(56 passed, 0 failed)orchestrator/contributorreturns no hits outside intentional identifiers