Opp: source outpost identity from sysio.chains instead of operator config - #579
Opp: source outpost identity from sysio.chains instead of operator config#579heifner wants to merge 2 commits into
Conversation
…nfig Every operator declared each outpost's remote contract addresses in its own config, which bought coordination rather than security: the addresses are not verifiable on WIRE either way, and an attacker who can rewrite the registry row can already setcode sysio.msgch. Commit 9671150 dropped the on-chain binding for the regchain ABI churn it cost, not for a security gain, and pre-launch that cost is near zero. sysio.chains rows carry an outpost_addrs struct again, widened to cover both daemons: OPP, OPPInbound, OperatorRegistry, and the source-deposit contract. EVM names a contract per role; SVM is one program in opp_addr with the role fields required empty; WIRE is empty. setoutpost replaces the set for a remote redeploy. Values are format-validated per kind and bounded so nothing unbounded reaches sysio-billed state. The RPC client for a chain is now the one registered under that chain's own sysio.chains code, replacing the batch operator's external_chain_id match; create_outpost_client still asserts the client's verified eth_chainId equals the row's, so a client registered under the wrong code is rejected. That removes batch-outpost, batch-sol-client-id, and both underwriter-{eth,sol}-outpost options: the underwriter now serves every active non-depot row. An elected group must deliver on every active chain, so a chain with no configured client — or a chain kind this build cannot relay — is fatal for the batch operator rather than skipped: it names them and quits, after the sync gate where sysio.chains is readable. Missing addresses stay non-fatal, being governance state fixable with setoutpost. Both daemons rebuild a handle when the row's address changes, instead of relaying to an address the outpost has moved off. get_client in both client plugins used map::at, throwing std::out_of_range on an unknown id rather than returning null; that made create_outpost_client's own Unknown-client-id assert and verify_source_deposit_sol's null guard unreachable. Both now use find. get_client_by_chain_id and find_endpoint_coverage_gap lose their last callers and are removed.
| } | ||
| // An SVM outpost is one program serving every role, so the role fields | ||
| // are empty on its row and resolve back to opp_addr. | ||
| ep.commit_addr = depot_chains::resolve_role_addr(operator_registry_addr, opp_addr); |
There was a problem hiding this comment.
[P1] Do not fall back to opp_addr for missing EVM roles. setoutpost allows partial EVM address sets, but this helper maps a missing operator_registry_addr or source_deposit_addr to the OPP address. The non-empty fallback passes preflight and sends commit or verification calls to the wrong contract. Restrict the fallback to SVM rows, or require every EVM role address.
There was a problem hiding this comment.
Confirmed and fixed in f86cb32.
You are right that the danger is specifically the non-empty wrong value: setoutpost validates each EVM field independently, so a row can legitimately carry the OPP address while its OperatorRegistry is still undeployed. Substituting opp_addr there produced a well-formed address that sailed through the "carries no remote contract addresses" preflight and then targeted the OPP contract for uw_commit. Failing to commit would have been visible; committing to the wrong contract is not.
resolve_role_addr now takes the deployment shape rather than inferring it from emptiness:
inline std::string resolve_role_addr(std::string_view role_addr,
std::string_view opp_addr,
bool single_program) {
return single_program ? std::string{opp_addr} : std::string{role_addr};
}
SVM resolves every role to opp_addr; EVM resolves a role to its own field and nothing else, so an unset EVM role stays empty and preflight check 2b fails closed. I took the "restrict the fallback to SVM" option rather than requiring a complete EVM set, so the contract's register-then-configure path keeps working.
Added per_role_outpost_never_falls_back_to_opp_addr to test_chains_registry.cpp pinning exactly this — 5 cases in that suite now.
| wlog("underwriter_plugin: outpost chain {} has no remote contract address on its " | ||
| "sysio.chains row — not wiring until sysio.chains::setoutpost supplies one", | ||
| code_str); | ||
| continue; |
There was a problem hiding this comment.
[P1] Drop cached clients when setoutpost removes the address. When commit_addr becomes empty, this branch logs and continues but retains outpost_by_chain and wired_commit_addrs. A later destination-leg submission therefore still commits to the retired deployment, despite the registry no longer naming it. Erase or invalidate the cached handle before returning.
There was a problem hiding this comment.
Confirmed and fixed in f86cb32.
The branch now erases both outpost_by_chain and wired_commit_addrs before continuing, and distinguishes the two cases in the log — retiring a live handle is a different event from never having wired one:
if (outpost_by_chain.erase(chain_code) > 0) {
wlog("... no longer carries a remote contract address — retired the wired client ...");
} else {
wlog("... has no remote contract address on its sysio.chains row ...");
}
wired_commit_addrs.erase(chain_code);
wired_commit_addrs had to go too, otherwise a later setoutpost restoring the same address would hit the already-wired short-circuit and never rebuild the handle we just dropped.
One adjacent case I deliberately left as-is, so it is a conscious choice rather than an oversight: the branch above it still skips a chain that has dropped out of the active registry entirely, keeping its handle. Deactivation is not address retirement — the deployment is still the one the registry last named — and dropping the client mid-flight would strand an in-progress leg. Say the word if you would rather that one invalidate as well.
| wlog("batch_operator: failed to build outpost_client for outpost {}: {}", | ||
| op.id, e.to_string()); | ||
| code_str, e.to_string()); | ||
| continue; |
There was a problem hiding this comment.
[P1] Treat client-factory exceptions as unserviceable. A registered Ethereum client with the wrong chain ID throws from create_outpost_client, but this catch only logs and continues, so the entry never reaches unserviceable and the process does not quit. That violates the active-chain fail-fast policy and leaves an elected operator unable to deliver.
There was a problem hiding this comment.
Confirmed and fixed in f86cb32.
The catch now records the chain as unserviceable rather than logging past it:
} catch (const fc::exception& e) {
unserviceable.emplace_back(code_str,
std::format("outpost client could not be built: {}", e.top_message()));
continue;
}
Worth noting how reachable this was. The chain-id assert in create_outpost_client is the main check that a client registered under a chain code actually belongs to that chain — it is what makes the new code-keyed lookup safe. So the one failure that specifically catches a mis-registered client was also the one that got swallowed, leaving an elected operator running and silently undeliverable on that chain. The fail-fast policy documented right above the loop only held for the cases that returned rather than threw.
The reason string is per-chain, so the shutdown log names which chain and why, alongside the missing-client and unsupported-kind entries.
…ble clients Three review findings, all cases where a daemon kept going with wrong or stale outpost state instead of failing closed. resolve_role_addr fell back to opp_addr whenever a role field was empty, which is only correct for a single-program SVM outpost. setoutpost accepts a PARTIAL EVM set, so a row carrying the OPP address before its OperatorRegistry is deployed would resolve commit_addr to the OPP contract — a plausible, non-empty, wrong address that passes the is-it-configured preflight and then sends uw_commit to the wrong contract. The helper now takes the deployment shape explicitly: SVM resolves every role to opp_addr, EVM resolves a role to its own field and nothing else, so an unset EVM role stays empty and the preflight fails closed. wire_outpost_clients logged and continued when a row lost its address, keeping the handle in outpost_by_chain and wired_commit_addrs. Because setoutpost replaces the whole set, clearing a role retires that deployment, and a later destination leg would still have committed to it. The cached handle and its recorded address are now erased. build_opp_jobs swallowed client-factory exceptions, so a client registered under a chain code whose eth_chainId disagrees with the row never reached the unserviceable list and the node did not quit — the exact fail-fast policy the function documents. Factory failures now count as unserviceable.
E2E: greenhttps://github.com/Wire-Network/wire-platform-build-system/actions/runs/32400635346 Flow selection is quoted because a run that selects zero flows also reports success — these three genuinely executed. Run against a throwaway integration branch merging #578 into this one. The cluster-tool PR strips both PRs' flags in a single change, so pairing this branch alone with it would leave What the pass actually establishes, beyond "swaps work":
Caveat: that run predates the three P1 fixes (f86cb32). Those paths are fail-closed handling for misconfigurations e2e does not create, and the happy path through Two earlier runs failed in the build, both my error and both fixed: sdk-core has a hand-written |
Full E2E flow set: 13/13 greenhttps://github.com/Wire-Network/wire-platform-build-system/actions/runs/32409486702 The discovery line is quoted deliberately: a run that selects zero flows also reports success, so the Branch set — the combined branch carries BOTH wire-sysio PRs, because wire-tools-ts#77 strips both PRs' flags in one change and pairing either alone with it would fail on branch skew rather than on the change:
Only What the pass establishes beyond the earlier 3-flow run
Every one of these depends on the daemons having read their outpost addresses off Merge order#578 and this PR, then wire-libraries-ts#72 plus its sdk-core publish, then wire-tools-ts#77. Note #72 is no longer a pure type regeneration — it also carries the hand-written |
2 participants
Every operator declared each outpost's remote contract addresses in its own config. That bought coordination rather than security: WIRE cannot verify an address either way, and an attacker who can rewrite the registry row can already
setcodesysio.msgch, so the on-chain form grants no capability the privileged authority lacks.9671150f6adropped the on-chain binding for theregchainABI churn it cost — not for a security gain — and pre-launch that cost is near zero.Registry
sysio.chainsrows carry anoutpost_addrsstruct again, widened to serve both daemons: OPP, OPPInbound, OperatorRegistry, and the source-deposit contract. EVM names a contract per role; SVM is one program inopp_addrwith the role fields required empty; WIRE is empty.setoutpostreplaces the whole set when a remote contract is redeployed. Values are format-validated against the row's kind and bounded, so nothing unbounded reachessysio-billed state. Rows may register with empty addresses and be configured later.Wiring
The RPC client for a chain is the one registered under that chain's own
sysio.chainscode, replacing the batch operator'sexternal_chain_idmatch.create_outpost_clientstill asserts the client's verifiedeth_chainIdequals the row's, so a client registered under the wrong code is rejected rather than relayed through. Nothing about an outpost is declared per node any more, which removes--batch-outpost,--batch-sol-client-id, and both--underwriter-{eth,sol}-outpost; the underwriter serves every active non-depot row.An elected group must deliver on every active chain, so a chain with no configured client — or a chain kind this build cannot relay — is fatal for the batch operator rather than skipped: it names what it cannot serve and quits, after the sync gate where
sysio.chainsis readable. Missing addresses stay non-fatal, being governance state fixable withsetoutpostwithout touching a node. Both daemons rebuild a handle when the row's address changes, rather than relaying to an address the outpost has moved off.Incidental fix
get_clientin both client plugins usedmap::at, throwingstd::out_of_rangeon an unknown id instead of returning null. That madecreate_outpost_client's own "Unknown ethereum client id" assert andverify_source_deposit_sol'sif (!entry || !entry->client)guard unreachable. Both now usefind.get_client_by_chain_idandfind_endpoint_coverage_gaplose their last callers and are removed.Cross-repo
BREAKING for cluster-managed nodes: appbase rejects unknown options, so the cluster-tool change must land with this.
SysioContractTypesfor theoutpost_addrstype and thesetoutpostactionsetoutpostseeding stepMerge order: this and #578, then the sdk-core publish (
@wireio/sdk-coreis a published dependency in wire-tools-ts, not a workspace link), then wire-tools-ts#77.