WIRE-342: send the OPERATORS roster only when it changes - #568
Conversation
Generated-output-only companion to Wire-Network/wire-sysio#568, which adds the `sysio.epoch::rosterdig` table -- the per-outpost digest of the last OPERATORS attestation queued for that outpost, so the roster ships only when its content changes instead of on every epoch. Adds `SysioEpochRosterDigestEntryType` / `SysioEpochRosterDigestKeyType` and registers `rosterdig` on the epoch contract's tables and in `SysioContractDefinitions`. Nothing else in the ABI surface moved. Change-Id: I43cb504a510b9752405aedc491ac9793ae0d009b
`sysio.epoch::advance` re-encoded the entire `sysio.opreg::operators` table -- every status, including never-bonded UNKNOWN and never-erased TERMINATED rows -- and fanned it to every active outpost on EVERY epoch, unconditionally. The roster changes on registration/activation/slash; advance runs every epoch. So the depot re-shipped ~10 KB of byte-identical data per outpost per epoch, consuming roughly a third of every 32 KiB envelope, to say nothing. Re-sending bought nothing. An outpost cannot miss an envelope and continue -- both enforce strict epoch sequencing (ETH OPPInbound.sol:429, SOL epoch_in.rs:63-66), so a missed envelope stalls rather than diverges. Ethereum's handler has no skip path at all. Solana's three skips are pure functions of the payload, so a byte-identical re-send reproduces them exactly. No outpost re-converges via the rebroadcast. Six changes: 1. Send on change. A per-outpost digest of the encoded payload (`epoch::rosterdig`) gates `queueout`. The full roster still ships when it changes, so ETH's generation-stamp model and SOL's destructive replace are untouched. Absence of a row is what makes a newly activated outpost receive it unconditionally. No periodic re-send: every case where an outpost could want a roster it was already sent is an operational act needing depot coordination anyway (re-init resets the epoch cursor alongside the registry). 2. Filter to ACTIVE + SLASHED via the existing `bystatus` index. SLASHED must stay: `opreg::slash` flips the row BEFORE emitting OPERATOR_ACTION(SLASH), and SOL resolves a slash's target through this roster, so ACTIVE-only would silently no-op every slash. Excluding UNKNOWN is what makes a never-bonded registration cost zero envelope bytes. 3. Enforce `op_config.max_available_*` in `regoperator`. Declared, validated and persisted since the registry landed -- and read by nothing. Bootstrapped operators bypass it. 4. Cross-validate those ceilings against the envelope budget in `opreg::setconfig`, so a governance change cannot produce a roster no envelope can carry. `sysio.epoch` already does this for the sibling BATCH_OPERATOR_GROUPS attestation. 5. `buildenv`'s single-attestation guard now reports att_id, bytes and budget. It aborts `advance` inline, halting epoch advancement chain-wide, so it should not be anonymous. 6. Bound the per-operator authex address walk. The envelope budget moves to `sysio.opp.common/opp_envelope_budget.hpp` so epoch derives its ceiling from it rather than restating a consensus-visible cap. msgch re-exposes the values as class members (aliases, not copies) and its call sites are unchanged. Artifacts: epoch/opreg/msgch only -- the three with source changes. epoch.wasm carries +112 bytes from the CDT kv-header change (PR #107) that also moves several untouched contracts; those belong to a separate toolchain-refresh commit. opreg +3691 and msgch +2033 are entirely this change; verified against a same-CDT build of master. Full contracts_unit_test --sys-vm: No errors detected. Change-Id: I9d1ea243859297860de0336061ebab60c727e3a0
b8ed6a8 to
aa54789
Compare
`advance` builds the OPERATORS roster from ACTIVE + SLASHED. The SLASHED half is load-bearing and had no test: `sysio.opreg::slash` flips the row to SLASHED before emitting OPERATOR_ACTION(SLASH), and the Solana slash handler resolves its target THROUGH the roster the outpost holds -- so narrowing the filter to ACTIVE-only would turn every subsequent slash into a silent no-op on the outpost rather than a loud failure. Verified by mutation: dropping OPERATOR_STATUS_SLASHED from the scan fails this case. Asserts against the outbound ENVELOPE rather than the `attestations` rows, because `buildenv` consumes those rows as it bundles them -- they no longer exist once an advance returns. The envelope is also the stronger surface: it is the exact payload the outpost consumes. Test-only; no contract source, .wasm or .abi changes. Change-Id: I00e7bb6a0005ac4b8e8c9d8886976811fa2cf4dd
huangminghuang
left a comment
There was a problem hiding this comment.
OCR delegation review: four inline findings were validated against the current PR head and the latest Solana outpost limits/handlers.
|
|
||
| /// Ceiling on the total registered (non-bootstrapped) operator count, derived | ||
| /// from the envelope budget that `sysio.msgch` owns rather than restating it. | ||
| static constexpr uint32_t MAX_ROSTER_OPERATORS = |
There was a problem hiding this comment.
[P1] Bound the roster by the Solana entry cap
MAX_ROSTER_OPERATORS derives to 158 from the envelope byte budget, but the latest Solana outpost hard-caps OperatorRegistry at 32 entries and skips the entire OPERATORS attestation when that count is exceeded, retaining its previous roster (cap, whole-attestation skip). The shipped max_available_* sum is 105, so a configuration accepted here can deterministically leave Solana stale as soon as 33 active/slashed operators have SVM links. Please bound the actual roster by the strictest outpost cap or coordinate a Solana capacity increase.
There was a problem hiding this comment.
Agreed and fixed in 572b203b — this was the right catch, and it exposed a second bug next to it.
MAX_ROSTER_OPERATORS (fixed 158) is now max_roster_operators(outpost_count) = min(envelope capacity, MAX_OUTPOST_ROSTER_ENTRIES = 128), called from setconfig with a live count.
The second bug: ROSTER_BYTES_PER_OPERATOR was pinned to the two-address shape while the address walk allows eight, so the byte half silently overstated capacity as soon as a third outpost registered. Both halves now derive from the live active-outpost count:
| outposts | bytes/op | by bytes | ceiling | binding |
|---|---|---|---|---|
| 2 (launch) | 102 | 158 | 128 | entry cap |
| 3 | 141 | 114 | 114 | bytes |
| 4 | 180 | 89 | 89 | bytes |
So at the launch shape the depot can no longer accept a configuration Solana would refuse to seat, and the bound tightens on its own as outposts are added rather than going stale.
Dependency, stated on the PR: 128 comes from wire-solana#442, which is not merged — next is still 32. Worth noting the mismatch predates this PR: the shipped max_available_* defaults already sum to 105, so today they exceed the merged Solana cap by 3x regardless of anything here. #442 should land first; if it lands on a different number, this constant follows it.
I also confirmed the cap counts SVM-linked entries only (inbound.rs None => continue), which per outpost-three-concerns.md is effectively the whole roster since every operator bonds on every outpost.
| // SLASHED MUST stay in: `opreg::slash` flips the row to SLASHED *before* it emits | ||
| // OPERATOR_ACTION(SLASH), and the Solana outpost resolves a slash's target through | ||
| // this roster — so an ACTIVE-only roster would silently no-op every slash. | ||
| for (const auto status : { OperatorStatus::OPERATOR_STATUS_ACTIVE, |
There was a problem hiding this comment.
[P1] Ensure a target is published before its slash action
Excluding UNKNOWN leaves an operator absent from every outpost until its first ACTIVE/SLASHED roster. If it remains UNKNOWN across advances, then becomes ACTIVE and is slashed before the next advance, opreg::slash queues OPERATOR_ACTION(SLASH) first; this roster is queued later, so FIFO packing makes Solana process the slash without a mapping, return OperatorNotFound, and consume the action before the roster arrives. Retaining SLASHED therefore does not close the first-publication race. Ensure the target roster precedes any dependent action, or make slash application independent of the cached roster.
There was a problem hiding this comment.
Confirmed the mechanism, and I agree the gap is real — but I think the fix belongs outpost-side, so I have not changed it here.
Verified on the depot: opreg::slash calls emit_slash_attestation inline mid-epoch, the roster is queued from advance at the epoch boundary, and buildenv walks the READY status index in att_id order. So the slash genuinely does precede a roster queued later in the same envelope. And you are right that retaining SLASHED does not close it — that carve-out keeps a published operator present, it does not publish a new one.
Where I differ is the remedy. "Ensure the target roster precedes any dependent action" is not something the depot can do without restructuring when slashes are emitted: the slash attestation is minted at slash time, and the roster only exists as a queued attestation at advance. Reordering would mean either deferring emit_slash_attestation to advance (changing existing, unrelated behaviour) or having opreg reach into sysio.epochs roster gate.
Your second alternative — make slash application independent of the cached roster — is outpost-side and composes with the tombstone work already in wire-solana#444, which is touching exactly this code (process_slash_action + the rebuild loop). I would rather it land there than contort the depot.
On reachability, for the record: for BATCH operators the window looks closed, since a slash requires delivery, which requires scheduling, which happens at the same advance that publishes the roster. It looks plausible for UNDERWRITERS now that WIRE-297 has landed an underwriting-fault slash path that does not depend on scheduling. I have not proven either.
Filing a SOL ticket for the outpost-side fix and linking it here unless you would rather it be tracked under WIRE-342.
| case OperatorType::OPERATOR_TYPE_PRODUCER: return cfg.max_available_producers; | ||
| case OperatorType::OPERATOR_TYPE_BATCH: return cfg.max_available_batch_ops; | ||
| case OperatorType::OPERATOR_TYPE_UNDERWRITER: return cfg.max_available_underwriters; | ||
| default: return 0; |
There was a problem hiding this comment.
[P1] Challenger registration is not privileged
The comment's assumption is false: regoperator explicitly accepts OPERATOR_TYPE_CHALLENGER, and a non-privileged caller only has to authorize its own account. Returning 0 here therefore lets arbitrary challenger registrations bypass the new count while every row is billed to the sysio RAM pool. Across attacker-controlled accounts, the system-funded table remains unbounded. Require sysio.opreg authority for challenger registration or add and enforce a challenger ceiling.
There was a problem hiding this comment.
The premise is right but I do not think the roster consequence follows — pushing back on this one.
You are correct that registration_ceiling_for_type returns 0 for CHALLENGER and that regoperator accepts the type with only self-authorization. But a self-registered challenger cannot reach the OPERATORS roster, on three independent counts:
regoperatorsetsstatus = is_bootstrapped ? ACTIVE : UNKNOWN(sysio.opreg.cpp:440), andis_bootstrappedrequiresrequire_auth(get_self())— so a non-privileged caller lands UNKNOWN.meets_role_minreturns false for CHALLENGER via itsdefault:— there is noreq_*_collatfor the type, so eligibility can never be satisfied.- The eligibility handler switch returns early for CHALLENGER, so no
processprod/processbatch/processuwpath exists to flip it ACTIVE.
It therefore stays UNKNOWN permanently, and this PRs ACTIVE+SLASHED filter excludes UNKNOWN — so a challenger consumes zero envelope bytes and cannot contribute to either threshold. That is the same closure the filter gives never-bonded registrations of any type, which advance_ignores_never_bonded_registrations pins.
The half I agree with is the RAM one: the row is billed to the sysio pool and nothing bounds challenger registrations. That is real, but it is pre-existing, orthogonal to the roster/envelope defect this PR addresses, and would not be fixed by a challenger ceiling alone (the same unbilled-row argument applies to UNKNOWN registrations of every type). Happy to file it as its own ticket — it wants a billing or admission fix, not a roster one.
I have corrected the misleading comment you flagged: it claimed the envelope headroom "absorbs" bootstrapped and CHALLENGER registrations, which was wrong on both counts. It now states why challengers cannot reach the roster, and bootstrapped operators are bounded explicitly (see the P2 thread).
| // Bootstrapped operators bypass it, consistently with their other privileged paths | ||
| // (`bootstrapped-operator-invariants.md`): they are the genesis seed the chain cannot | ||
| // start without, and they are privileged-registered. | ||
| if (!is_bootstrapped) { |
There was a problem hiding this comment.
[P2] Bootstrap bypass defeats the hard roster bound
Bootstrapped operators land ACTIVE immediately and are serialized into OPERATORS, yet this bypass excludes them from every max_available_* check. The validated sum therefore is not a hard safety invariant: privileged bootstrap/governance registrations can still exceed both the envelope allocation and downstream entry caps. Apply a global actual-roster bound even on the bootstrap path; bootstrap's collateral/termination exemptions do not require bypassing a consensus-liveness capacity limit.
There was a problem hiding this comment.
Agreed and fixed in 572b203b. Your framing is the part I had wrong — bootstrap's exemptions are economic, and a consensus-liveness capacity limit is not the same class of thing. Exceeding it costs the operator nothing and wedges the outpost for everyone.
regoperator now applies a global actual-roster bound to every registration, bootstrapped included: it counts ACTIVE + SLASHED (exactly the set advance serializes, via the bystatus index, bounded by the ceiling) and refuses the registration that would cross it. The per-type ceilings keep their bootstrap bypass — that one is genuinely about the genesis seed being registerable under a mis-set config — but they are no longer load-bearing for safety.
Test: regoperator_global_roster_bound_applies_to_bootstrapped fills the roster to the ceiling with bootstrapped registrations (each lands ACTIVE, so each occupies a roster slot) and asserts the next one is refused. That is the case your finding names, and it fails without the change.
One implementation note worth flagging since it is a hot path: the bound is a bounded scan (stops at the ceiling), so regoperator now does two O(n) walks — the existing per-type count plus this one — with n capped at 128. Materializing a roster counter instead would remove both, but that means maintaining it across every status transition (activation, slash, termination, re-registration), which is a larger change than this finding warrants. Flagging it rather than silently accepting the cost.
…-send-on-change Change-Id: Iba30f92e05999bf095517a8f7a340e0ec31aae79 # Conflicts: # contracts/sysio.epoch/sysio.epoch.wasm # contracts/sysio.opreg/sysio.opreg.wasm
Review findings from huangminghuang on #568. **The ceiling was derived from bytes alone, which is necessary but not sufficient.** An outpost that cannot seat the roster it receives skips the WHOLE attestation and retains its previous one, so the roster silently stops tracking the depot. Solana's `OperatorRegistry` is a fixed-size PDA -- 32 entries on `next`, raised to 128 by wire-solana#442 (SOL-385), which explicitly defers the depot-side number here. Ethereum applies no count cap, so Solana is the binding one. `MAX_ROSTER_OPERATORS` (a fixed 158) is replaced by `max_roster_operators(outpost_count)` = min(envelope capacity, 128 entries). **The bytes half was also wrong the moment a third outpost registers.** `ROSTER_BYTES_PER_OPERATOR` was pinned to the two-address shape while the address walk allows eight, so the derivation silently overstated capacity as outposts grow. It is now parameterised on the live active-outpost count, since an operator carries one address per registered outpost. At the launch shape (2) the ENTRY cap binds at 128; at 3 it is 114 and at 4 it is 89, where bytes take over. **The configured sum was not a safety invariant.** Bootstrapped registrations land ACTIVE immediately -- so they are in the roster at once -- yet bypassed the per-type ceilings entirely. `regoperator` now applies a global ACTIVE+SLASHED roster bound to EVERY registration. Bootstrap's exemptions from collateral and termination are economic (`bootstrapped-operator-invariants.md`); exceeding a transport/seating ceiling is not a cost to the operator, it wedges the outpost for everyone. `is_active_outpost` moves to the chains header: it was already duplicated between `sysio.epoch.cpp` and `sysio.opreg.cpp`, and the ceiling derivation would have been a third copy. `active_outpost_count()` joins it as the shared census. 658/658 contract unit tests. Change-Id: I110c2f2eee572330d6b78ce87ba7ba33ebe5fb1c
Updated — master merged, review addressedRebased onto current master ( Review (
The ceiling fix also closed a bug next to it: Merge dependency
Worth being explicit that the mismatch is not introduced here: the shipped Verification
|
Converted to draft — holding for WIRE-361WIRE-361 measures the budget this PR sizes against, and the number is wrong. The ceiling here derives from the DECLARED envelope cap (32,768 B). WIRE-361 measured the envelope that actually finalizes under EIP-7825 with every costly regime coinciding: 9,472 B — 29% of declared. The roster is charged against that, not the nominal cap.
So the allowance reserved here is 1.7x the whole envelope that can finalize, and a full 128-operator dual-address roster exceeds it outright. WIRE-361 reaches the same place independently: at the realistic dual-address shape its Two things follow. The share-divisor rationale in this PR is false as written. A registration ceiling cannot resolve this. A ceiling safe against 9,472 B is well under 100, but the shipped Send-on-change reduces frequency — most epochs now carry no roster at all — but on any change epoch the FULL roster still ships, so this PR does not satisfy WIRE-361's acceptance criterion that throughput be flat in cumulative registration count. What is ready to fold into WIRE-361All of it is green (658/658 contract unit tests; the pre-merge head was 14/14 e2e on run
Also: WIRE-361 carries one stale noteIts closing "Related, filed separately" says @huangminghuang — your four findings are all addressed in-thread; the P1 on the Solana entry cap is what surfaced this, since chasing the right bound is what led to the executable-envelope number. |
Closes WIRE-342 (depot side).
The problem
sysio.epoch::advancere-encoded the entiresysio.opreg::operatorstable — everystatus, including never-bonded
UNKNOWNand never-erasedTERMINATEDrows — and fanned itto every active outpost on every epoch, unconditionally. No dirty flag, no digest, no
watermark:
sysio.epoch.cpp:628-694was a bare scope with no guard.The roster changes on registration/activation/slash.
advanceruns every epoch. So thedepot re-shipped ~10 KB of byte-identical data per outpost per epoch, consuming roughly a
third of every 32 KiB envelope, to communicate nothing.
The rebroadcast bought nothing — this is what the fix turns on
The obvious justification is re-convergence, as documented for the sibling
BATCH_OPERATOR_GROUPS. It does not hold here:sequencing — ETH
OPPInbound.sol:429, SOLepoch_in.rs:63-66. A missed envelope is astall, not a silent divergence.
OPPInboundLib.handleOperatorsdecodes and appliesunconditionally. There is nothing to recover from.
bytes, so a byte-identical re-send reproduces the same skip forever. The condition clears
only when the roster changes — exactly when send-on-change sends anyway.
What changed
epoch::rosterdig) gatesqueueout. Thefull roster still ships when it changes, so ETH's generation-stamp model and SOL's
destructive replace behave exactly as before. A newly activated outpost has no digest
row, and that absence is what makes it receive the roster unconditionally.
bystatusindex. SLASHED must stayin:
opreg::slashflips the row before emittingOPERATOR_ACTION(SLASH), and SOLresolves a slash's target through this roster — ACTIVE-only would silently no-op every
slash.
op_config.max_available_*inregoperator— declared, validated andpersisted since the registry landed, and read by nothing. Bootstrapped operators bypass it.
opreg::setconfig.buildenv's single-attestation guard now reportsatt_id,bytes,budget. Itaborts
advanceinline — halting epoch advancement chain-wide — so it should not be anonymous.The envelope budget moves to
sysio.opp.common/opp_envelope_budget.hppsosysio.epochderives its roster ceiling from it rather than restating a consensus-visible cap.
msgchre-exposes the values as class members (aliases, not copies); its call sites are unchanged.
Effect
A static epoch now queues one attestation instead of two. Composed with the filter, a
never-bonded registration produces zero envelope traffic — closing the zero-collateral
growth path the ticket describes.
MAX_ROSTER_OPERATORSderives to 158 (32,232 B budget ÷ 2 share ÷ 102 B per operator atthe realistic two-address shape). Shipped defaults sum to 105.
Validation
contracts_unit_test -- --sys-vm: No errors detected.sysio.opreg_tests, 3 insysio.dispatch_tests) covering send-on-change,the never-bonded case, per-outpost delivery to a newly activated outpost, the ceilings, the
bootstrapped bypass, and TERMINATED not consuming a slot.
advancesilently does nothing —
advancegate-blocks on emissions before reaching the roster block,and the first version of these tests passed for exactly that wrong reason.
Artifacts
Only
epoch/opreg/msgch— the three with source changes. Verified against a same-CDTbuild of
master:epochopregmsgchepoch.wasmunavoidably carries +112 bytes from the CDTkv::globalchange (wire-cdt #107),which also moves several contracts this PR does not touch. Those belong to a separate
toolchain-refresh commit, not this one.
Companion
SysioContractTypesregen for the newrosterdigtable: Wire-Network/wire-libraries-ts#70.Merge order: this PR, then the companion.
Not covered
The
buildenvmessage and the address-cap bound compile and are exercised by the full sweep,but neither has a test that drives its specific behaviour — that needs an oversized-attestation
fixture and an account with more than 8 authex links.