Repository navigation
feat(sysio.system): derive producer rank from a score, and demote on missed rounds (WIRE-367) - #599
Conversation
…missed rounds (WIRE-367) Producer rank stops being a governance write and becomes position in a score-ordered index. `producer_info.rank` is replaced by `rank_score`, a packed key of two tier bits over one weighted composite: adding a scoring factor is a new weight field on the `prodscorecfg` singleton, never a re-layout of an unbounded table. Three factors ship live — collateral (linear, uncapped, min across the required pairs, on `slashable_now`), participation, and snapshot attestations; relay / api / benchmark ship at weight 0 pending an attestation path. `setrank` and `assign_producer_ranks` are gone. Missing rounds is a separate model, not just a factor. `onblock` walks the active schedule between the previous block's producer and this one's — the existing counters record presence only, so absence left no trace — and `max_consecutive_missed_rounds` (default 3) sets a demoted tier no score can climb out of. `regproducer` is the single door back, from a voluntary `unregprod` park and from demotion alike; there is no cooldown and no expiry. Rank is now ONE predicate — active row, ACTIVE PRODUCER operator, active finalizer key — shared by all four consumers, each counting schedulable entries while walking rather than taking the first N index slots. `compute` sinks every non-ACTIVE producer into the demoted tier, which is what bounds those walks over a permissionless table. A weight change or a `req_prod_collat` change invalidates every stored score, so `onblock` drains a bounded rescore cursor inside the throttle it already pays for. sysio.opreg cannot cheaply notify sysio.system, so the collateral half is detected by comparing a `scored_collateral_stamp` on the global against the live config. Also: `opreg::deposit` now rejects bootstrapped operators, matching `depositinle`; `regoperator` re-evaluates eligibility so a producer's score is correct from registration; and `termcheck` records why an indefinitely-demoted producer stays demoted rather than being terminated. `regsnapprov`'s capacity check becomes unreachable — `max_snap_providers` is defined as `max_snap_provider_rank`, and derived positions are necessarily distinct where the stored ordinal could repeat. It stays as a structural guard. Making producers schedulable also made the snapshot-attestation fixture expensive: once finalizer keys are registered, `update_ranked_producers` proposes a five-finalizer policy and the node signs plus verifies a vote on every block of the `block_spacing` advance. The fixture now takes a cadence-period count and builds that history in its constructor — before the system contract is deployed and before any key is registered — so those blocks run no contract code under the one-finalizer genesis policy, and `vote_block_num` asserts rather than silently advancing at the expensive price. Two height-precondition negatives need no history at all; two purging tests build both periods up front instead of one mid-test. The suite is its own ctest entry now, so the other 685 cases no longer queue behind it: 580 s and 854 s in parallel against a 2700 s budget, where one entry previously ran the whole binary to the edge of it. WIRE-382 tracks making `block_spacing` configurable, which retires the advance entirely. Change-Id: I86b251fd0915615946acb72eb2a6e69f690cb8a3
…free (WIRE-367) Review fixes on #599: - getpeerkeys ranks eligible operators (active producers row + ACTIVE opreg PRODUCER) without requiring a finalizer key, so BP gossip still reaches a producer scheduled through setprods; is_schedulable stays the schedule / pay predicate. Restores auto_bp_gossip_peering_test. - The snapshot factor reaches the index: producer_rank::rescore at credit and at the payepoch reset. rescore_generation -> rescore_pending (boolean). - Tests: mul_sat saturation, the 22-position bootstrap displacement boundary, a partial miss lowering the composite, peer discovery without a finalizer key. Pay model (agreed 2026-09-03): - Producers are paid PER BLOCK: the active slice of the producer pool is spread over the period's block slots (the nominal count, raised to the blocks produced when a period runs long) and every schedulable producer is credited that rate per block. Missed blocks stay in the treasury; nothing is redistributed to the producers that showed up. - Standbys draw a retainer from standby_bps (new emitcfg field, 8% in the harness) of the pool, split into fixed per-position shares decaying linearly from position 22. A vacant position pays nobody. - No forfeiture: a producer that is not schedulable at a payepoch (parked, keyless, demoted) is neither paid nor reset, and its blocks are paid at the first payepoch where it is. Demotion stays at three consecutive zero-block rounds; regproducer remains the only door back. - Removed: eligible_rounds / current_round_blocks / last_block_num, the 6-of-12 round threshold, the weight walk, and the global total_unpaid_blocks sequence-stamp counter. - sysio.epoch deserializes emitcfg and reads pay_cadence_epochs, which now follows the new field, so its wasm is refreshed with the layout. The Python TestHarness emitcfg payload carries standby_bps. Change-Id: Ifd5a306d2d1aa5d02f1d33afc44c0dca12e6d165
…fication (WIRE-367) sysio.system scores producer rank against opreg's producer collateral minimums, but it learned of a change only by comparing a second-granular config stamp inside onblock's throttle, so two setconfig calls landing inside one second could leave the second one unscored. sysio.opreg::setconfig now require_recipient()s sysio.system, and an on_notify handler opens the sweep the moment the action lands. The stamp field, its detector, and the throttle probe are gone; the global row loses scored_collateral_stamp (ABI change; sysio.system wasm/abi and sysio.opreg wasm rebuilt). collateral_minimum_change_opens_rescore_sweep now asserts the sweep is pending as soon as setconfig lands and that a second change reopens it. Change-Id: If7387f7f8b97698d8bc9bcbe6631fababcc48c32
…tarts on re-entry (WIRE-367) Review fixes on the score-based ranking: - The demoted tier now bounds every rank walk by LIVE standing. A parked (`unregprod`) row scores into the demoted tier and is rescored in the same action; sysio.opreg dispatches its `processprod` notification from `slash` and `terminate_inline` too (`notify_producer_standing`), and the eligibility sink notifies even for a terminal row while still refusing to transition it. Before this a parked, slashed or terminated producer kept its healthy-tier key until some unrelated event rescored it -- skipped by every walk but visited by all of them, and contradicting the comments that said otherwise. - `rescore` zeroes `snapshot_attestations` when a row re-enters the walk. `payepoch` resets the counter only on rows it visits and never visits the demoted tier, so a credit earned before a demotion, park or de-collateralization rode back in as a stale factor. The miss streak is deliberately kept: resetting it would let withdraw/cancel toggling dodge a demotion. - The collateral factor reads the bonded balance only. The sysio.uwrit `locksums` subtraction could never hit -- an account holds one operator row of one type and only active underwriters carry locks -- so the read, the uwrit include and the two include dirs it needed are gone. - payepoch comments corrected: a terminated operator can settle, re-register and be paid its carried blocks; a slashed one never returns. Tests: the park test asserts the tier sinks at once and is restored by regproducer; `slash_and_termination_sink_the_key_at_once`; `re_entering_the_walk_restarts_the_snapshot_credit` (snapshot suite). ABIs unchanged; sysio.system and sysio.opreg wasm rebuilt. Change-Id: I41d1f1ed2aeee437f84c1afa7f8788d37a965d6f
…hot service as a tiebreak (WIRE-367) Three review outcomes, decided together: - A demoted producer that is STILL in the active schedule now clears its demotion by producing a block, not only by pushing `regproducer`. Demotion and rescheduling are separate events, and the `min_schedule_size` floor can hold the gap open indefinitely: when demotions drop the schedulable count below it, the rebuild retains the last good schedule and the demoted producers keep producing under it. Without this they produce for nothing -- `payepoch` stops at the demoted tier -- until every operator re-registers by hand, which is what a mass outage would have caused. A producer the schedule has already dropped never reaches this path and still needs `regproducer`. - `snapshot_weight` defaults to a tenth of the collateral weight. At parity one quorum attestation moved the composite by as much as an entire minimum bond, so among producers bonded near each other the credit decided the top-21 boundary and the pay-period reset decided it back, proposing a producer schedule and a finalizer policy each way for no change in real standing. Snapshot service should separate producers the collateral term has left tied. - Documented the participation penalty's lasting consequence, which is intended rather than an oversight: the streak clears only by producing, so a producer whose penalty drops it below the active schedule holds that penalty until it re-registers or outbids on collateral. Clearing the streak for producers outside the schedule would let a boundary producer flap in and out at every rebuild. Test: `producing_while_still_scheduled_clears_a_demotion` demotes one of four producers, so three remain schedulable and the floor forces the schedule to be retained, then shows the demoted producer recovering by producing. Change-Id: I943155f8b3ef3e020d10599d08a6b1dc7c277e83
…IRE-367) Three phases on the existing 5-node fixture, exercising what no single-process contract test can reach: miss attribution against real block production, and recovery against real finality. - Stop one keyed producer's node and assert it is demoted at exactly `max_consecutive_missed_rounds`, that no producer which kept producing is charged a miss, and that LIB keeps advancing with one of four finalizers absent -- which is what lets the chain run long enough to demote it. - Assert the `min_schedule_size` floor RETAINS the demoted producer rather than publishing a short schedule: one demotion leaves three schedulable against a floor of four, so `update_ranked_producers` keeps the last good schedule and the demoted producer holds its slot. - Relaunch the node and assert producing a block clears both the demotion and the miss streak, with no `regproducer` and no operator intervention. The second and third phases are the same window: on a real outage the gap between "demoted" and "rescheduled" can stay open indefinitely, and recovery by producing is what closes it. Observed on the run: demotion 80s after the node stopped, the schedule retained all four producers, recovery 19s after relaunch. 390s total, well inside the default ctest timeout. Change-Id: Ie1a0dfd0f29539202061687e4281732eed50d0d4
The collateral-backed path onto the schedule is entirely self-service, and nothing described it end to end. This walks an operator through it: link the outpost addresses, register as a producer operator, bond on every required chain through the outpost contracts, register the block-signing and finalizer keys, run the node -- and then nothing, because ranking schedules them on its own. It also covers what an operator has to reason about afterwards: how the score is weighted and why collateral is the minimum across chains rather than a sum, that pay is per block with a standby retainer and no forfeiture, that a missed round is a whole unproduced window, that demotion clears either by producing while still scheduled or by re-registering, and that a single miss short of demotion can still cost a slot because the streak only clears by producing. Change-Id: Ie48f0f19ed2e771bd27f1a8d5c50146c763f2aa6
| // before it is ready is demoted again within max_consecutive_missed_rounds rounds, | ||
| // which is self-correcting. A demoted producer that is still in the active schedule | ||
| // recovers on its own by producing -- see `record_round_participation`. | ||
| info.is_demoted = false; |
There was a problem hiding this comment.
[P1] Re-registration can erase every missed round without proving liveness
The existing-row upsert clears both is_demoted and consecutive_missed_rounds on every regproducer or regproducer2 call. Both actions are repeatable with only producer authorization and accept the same key, so an offline but transaction-capable operator can resubmit after every one or two misses, or immediately after demotion, and restore the healthy tier without producing a block. That bypasses the core demotion mechanism indefinitely. Please make block production the only way to clear the streak, or introduce a recovery state, proof, or cooldown that cannot be reset repeatedly.
There was a problem hiding this comment.
[P1] Residual on b569e00: re-registration still erases the miss-rate evidence\n\nThe latest change preserves consecutive_missed_rounds, but every repeatable regproducer/regproducer2 still zeros rounds_in_window, missed_rounds_in_window, and miss_window_open_ms. A producer that alternates misses with produced rounds, or chronically delivers short rounds (which do not advance the consecutive streak), can re-register before the minimum rate sample and remain eligible indefinitely. Please reset the window only on a genuine demoted-to-recovered transition, not on an ordinary active-row upsert.
There was a problem hiding this comment.
Confirmed and fixed in 4fce5f4 — and it was worse than described. Short rounds feed the rate gate ALONE (a short round deliberately does not advance the consecutive counter), so a producer delivering a fraction of every round accrues evidence only in the window, and a free repeatable wipe left none of it. The cron loop the streak was protected from applied in full to the rate gate.
The window now resets only on a genuine demoted-to-recovered transition. Two tests: regproducer_does_not_launder_the_miss_window drives the actual loop — alternate miss/produce, re-register after each — and asserts the gate still fires; regproducer_clears_the_window_of_a_demoted_producer pins the reset that IS kept, since a demoted producer is unscheduled, observes no rounds, and could otherwise never improve its recorded rate.
| // The walk is bounded by the demoted tier. `regproducer` is permissionless, so the table is | ||
| // unbounded -- but producer_rank::compute sinks every non-ACTIVE producer operator into the | ||
| // demoted tier, which sorts last, so the scan stops before the spam tail. | ||
| for( auto it = idx.cbegin(); it != idx.cend() && top_producers.size() < max_producers; ++it ) { |
There was a problem hiding this comment.
[P1] Healthy-tier rows do not bound these scans
An ACTIVE producer operator remains in the healthy tier even without a finalizer key, so this loop can skip an arbitrary number of bonded but unschedulable rows before finding 21 producers. Producer admission is permissionless and max_available_producers is stored but not enforced. The same assumption appears in payepoch, which walks every non-demoted row. Enough such rows can exhaust action CPU during the periodic onblock rebuild or inline epoch payout. Please enforce a hard safe bound on active candidates or paginate and decouple these walks.
There was a problem hiding this comment.
[P2] The new hard ceiling makes debt beyond row 500 permanently unreachable\n\nThe CPU bound fixes the unbounded scan, but every payepoch restarts from prodrank.begin(). Registration/ACTIVE producer count is not otherwise capped, so a producer can earn blocks while top-21 and then be displaced below row 500 before payout. While ordering stays stable, no later payout reaches or clears that earned debt; retaining unpaid_blocks therefore does not provide eventual settlement. Please paginate with a persistent cursor or settle block debt through a separately bounded index.
There was a problem hiding this comment.
Confirmed, and accepted with the promise corrected rather than the bound changed.
The reasoning is economic: a producer bonded and unpaid does not stay bonded — the capital moves to where it earns — so a standing population of 500+ ranked-but-unpaid producers is not a state the system settles into. Past what is paid there is nothing to look at.
Worth separating out the common case, which is not rank at all: a temporarily unschedulable producer (key rotation, transient opreg UNKNOWN) sits in the demoted tier that the walk breaks at, and its carried blocks settle at ANY position once it is schedulable again. The genuinely unreachable case is narrower than the general one — earning while top-21, then being displaced 500+ positions before the next payout, which needs a mass onboarding between two payouts rather than a slide.
So 4fce5f4 drops the "nothing is ever forfeited" claim from the operator guide. It now says blocks produced are not forfeited, and states plainly that settling held blocks requires being back within the pay walk's reach.
| uint32_t position = 0; | ||
| for (auto i = idx.cbegin(); i != idx.cend() && resp.size() < max_return; ++i) { | ||
| if (i->rank > max_rank) | ||
| if (producer_rank::tier_of(i->rank_score) == producer_tier::demoted) |
There was a problem hiding this comment.
[P1] Preserve actual schedule members in peer discovery
This result is based only on eligible-operator rank, not the active or pending schedules. A producer demoted but retained by the four-member schedule floor is omitted even though its next block is supposed to clear demotion. Conversely, 30 higher-scoring ACTIVE producers without finalizer keys can fill the response even though none can enter the ranked schedule. Because a nonempty cache refresh removes omitted keys, this can evict real scheduled BPs and disable or reject automatic BP gossip. Please seed and deduplicate active and pending schedule members first, then fill remaining capacity with ranked candidates.
| // sysio.system scores producer rank on the ratio of posted collateral to these minimums, so | ||
| // every stored score is stale the moment they move. Tell it on the same channel processprod | ||
| // uses; it opens a bounded rescore sweep on the notification. | ||
| require_recipient(opreg::SYSTEM_ACCOUNT); |
There was a problem hiding this comment.
[P1] Reevaluate existing operators before activating new collateral minima
setconfig writes the new requirement vectors and only asks sysio.system to rescore. It never reevaluates existing operator statuses. Raising a minimum therefore leaves a now-undercollateralized producer ACTIVE; the score sweep lowers its composite but compute and is_schedulable still trust that stale status, so it can remain healthy, scheduled, and payable indefinitely. Lowering a minimum likewise leaves newly qualified UNKNOWN rows excluded. Please stage the config behind a bounded eligibility sweep or make authoritative consumers check the live minima.
There was a problem hiding this comment.
[P1] Residual on b569e00: the live producer ratio fixes only part of this\n\nA raised producer minimum is now caught by producer_score, but compute checks the stored ACTIVE status before evaluating that ratio. Lowering the minimum therefore cannot promote existing UNKNOWN producers. Batch operators and underwriters have no equivalent live-minimum check, so raising their minima leaves underbonded rows ACTIVE and still eligible for epoch selection or underwriter authorization until some balance mutation happens. The config change still needs a bounded status reconciliation or authoritative live checks for every role.
There was a problem hiding this comment.
Confirmed on all three counts. The decision for this PR is to accept and document rather than reconcile, so flagging that explicitly rather than marking it fixed.
One correction to the finding, though: the underwriter half is narrower than it looks. sysio.uwrit::try_select_winner refuses any candidate whose LIVE available_via_mirrors does not cover src + dst, so every underwritten leg stays collateral-backed at its real value — the minimum is an eligibility floor, not a capacity control. The exposure that remains is the batch-operator one: an underbonded op keeps serving in groups with a smaller slash-at-risk than governance just set.
Accepted because that exposure is one role in one direction, self-corrects on any balance movement (reevaluate_eligibility fires on deposit, withdraw, withdraw-flush, slash and terminate), and is auditable off-chain by comparing balances against op_config. The operator guide now states that a minimum change binds new registrations immediately and existing operators on their next balance movement, and that a lowered minimum needs a balance touch to take effect.
A bounded reconciliation sweep in opreg following the flushwthdw shape is the right permanent fix for all three roles and both directions — but it is opreg work rather than producer ranking, and deciding what happens to an operator whose status flips mid-epoch deserves its own review.
| if( timestamp.slot - _global.get().last_producer_schedule_update.slot > 120 ) { | ||
| // Drain any pending rescore BEFORE rebuilding, so the rebuild sees the freshest scores it | ||
| // can. A sweep spans several ticks; the schedule is proposed from a partially-rescored | ||
| // index in the meantime, which is safe because the tiers -- not the composite -- decide |
There was a problem hiding this comment.
[P2] Do not publish ranks from mixed score generations
Only 32 primary-key rows are rescored before update_ranked_producers consumes the secondary index. The comment says tiers make the partial state safe, but composite order selects the top 21 and standby positions within each tier. For example, reducing collateral_weight to zero makes the first batch lose that term while untouched rows retain their old inflated scores, so schedules, retainers, snapshot eligibility, and peer ranks match neither configuration for multiple ticks. Please stage scores by generation and atomically activate them after the sweep, or defer every rank consumer until one coherent generation is ready.
| require_auth( get_self() ); | ||
|
|
||
| producer_rank::producer_score_config_t weights_tbl( get_self() ); | ||
| weights_tbl.set( weights, get_self() ); |
There was a problem hiding this comment.
[P2] Apply a lowered miss threshold to existing streaks
setscorecfg stores the new max_consecutive_missed_rounds and opens only a score sweep. The sweep recomputes participation, but tier_for still uses the persisted is_demoted flag. A producer whose streak already meets the lower threshold can therefore stay healthy indefinitely, especially after falling outside the schedule where no later missed-round event can flip the flag. Please reconcile streak-to-demotion state during the sweep, with explicit semantics for threshold increases.
There was a problem hiding this comment.
Fixed in 9c19796. The ordering was what blocked this: while regproducer pardoned a producer but preserved its streak, re-deriving the flag would have re-demoted that producer on the very next sweep, and — unscheduled — it could never produce the block that was its only other way out. Gating the pardon on schedule exit let the pardon clear the streak too, and the sweep now re-derives is_demoted from warrants_demotion(streak, weights).
Semantics are symmetric, per your point about threshold increases: the streak is the evidence and the flag caches it, so a lowered limit binds on streaks that already breach it and a raised one releases the rows now under it. One expression, both directions.
a_raised_miss_threshold_reconciles_an_existing_demotion covers it on a producer carried off the schedule with its demotion intact — the case with no other remedy, since off-schedule it observes no rounds. It exercises the raise direction deliberately: a producer with a live streak is by definition still scheduled, so it serves a round and resets the streak before any sweep can run, which leaves the lowered direction with no deterministic setup in this fixture.
| distributed_to_producers += pay; | ||
| } | ||
| for (const auto& entry : entries) { | ||
| int64_t pay = static_cast<int64_t>( |
There was a problem hiding this comment.
[P2] Keep block entitlement when integer pay rounds to zero
A producer with unpaid blocks is added to to_reset even when active_pool multiplied by blocks divided by slot_divisor rounds to zero, so the later reset erases those blocks without crediting anything. A valid 60-second period with a one-unit active pool and 120 nominal slots demonstrates the loss. This contradicts the stated no-forfeiture invariant. Please retain the blocks or carry a per-producer fractional remainder until the entitlement is representable.
There was a problem hiding this comment.
[P1] The zero-rounding fix can now over-distribute the active pool\n\nOn b569e00, first-pass-zero rows are removed from slot_divisor but the final loop still pays them if that smaller divisor makes them nonzero. A reachable carried-debt state with nominal_slots=120, active_pool=1, and two payable rows holding 120 blocks each first removes all 240 blocks, obtains a final divisor of 120, and then credits both rows 1: two units of claims from a one-unit pool. Please keep first-pass-excluded rows carried for this payout, or recompute the payable set while enforcing the pool total.
There was a problem hiding this comment.
Confirmed — your arithmetic is exact, and this one was mine. Fixed in 4fce5f4: the first pass now DECIDES the payable set and the second only prices it, so a row excluded from the divisor is excluded from payment and its blocks carry to the next payout.
The comment I had written argued for the bug ("a row that crosses back over the threshold is simply paid, which is the outcome we want") without accounting for those blocks no longer being in the divisor. Test a_tiny_pool_never_credits_more_than_it_holds asserts a one-unit pool can never credit more than one unit.
| - **Park** with `unregprod`. Your bond is untouched and your operator status stays `ACTIVE`; you | ||
| simply hold no schedule position. `regproducer` brings you back at the position your collateral | ||
| earns. | ||
| - **Withdraw** with `sysio.opreg::withdraw`, which queues the request; `cancelwtdw` cancels it |
There was a problem hiding this comment.
[P2] This withdrawal path cannot release the outpost bonds described above
The guide tells operators to bond on Ethereum and Solana, but sysio.opreg::withdraw is hardcoded to the WIRE balance on WIRE_TOKEN. Following this instruction records an insufficient-balance failure and leaves the outpost collateral untouched. Please direct operators to each holding outpost withdrawal entry point, which then reaches withdrawinle on WIRE.
Master's WIRE collateral symbol fix (#600) landed on the same files this branch touches. Resolutions: - sysio.opreg.cpp: kept this branch's forward declaration of `reevaluate_eligibility`, took master's removal of the slug-literal using-directive, which its shared `opp::wire::` constants made redundant. - emissions.cpp: both sides deleted a constant at the same spot. Master dropped `WIRE_SYMBOL` for the shared `opp::wire::asset_symbol`; this branch dropped `ACTIVE_PRODUCER_COUNT` with the per-round pay threshold. Neither survives. - emissions_tests.cpp: kept this branch's `standby_bps` payload field and took master's parameterized `epoch_log_retention_count`. - sysio.system.{wasm,abi}, sysio.opreg.wasm, sysio.epoch.wasm: regenerated by rebuilding rather than merged. The contracts master changed and this branch does not are byte-identical to master. contracts_unit_test 604s and contracts_snapshot_attest_test 914s both pass on the merge alone, before any review-comment work goes on top. Change-Id: I812debaff65b56eab5200ef0ec1cabd63762c225
… discovery follows the schedule (WIRE-367) Two PR review findings, plus the contract artifacts the master merge should have carried. - payepoch cleared a producer's block count whenever it had blocks, but credited pay only when the amount was positive. Integer division means a real block count over a small pool floors to zero, so the count was consumed while nothing was paid -- destroying work that was actually done, which is the one thing this model promises never to do. The reset is now driven by whether the BLOCK portion credited, computed apart from the standby retainer, so a standby whose retainer paid but whose block pay floored to zero keeps its blocks too. An uncredited count carries exactly as an unpayable row's does. - getpeerkeys ranked candidates and stopped at the demoted tier, so it could omit a producer that is currently producing blocks. `update_peer_keys` returns early only on an EMPTY response, so a non-empty one ERASES every producer it omits: omitting a live producer evicts it from the BP peer map and cuts it out of the gossip mesh. A demoted producer retained by the min_schedule_size floor is exactly that case -- it holds its slot and its next block is what clears the demotion, yet it sorts into the tier the walk stops at. Peer discovery now seeds from the active schedule first, deduplicates, then fills the remainder by rank. Seeding is keyed by NAME, so a producer scheduled through setprods with no producers row at all is discoverable too. - The merge commit captured the pre-merge wasm/abi: they were staged during conflict resolution and the rebuild that followed was never re-staged. The binaries here are built from the merged source, which is what CI runs. Both tests were confirmed to FAIL without their fix: the pay test reports "uncredited blocks were consumed: had 15, now 1", and the peer test reports a scheduled producer omitted and would-be evicted. Change-Id: I2ee413a8dca20d453b74ff382837ca915d45065e
…the batch-operator gates (WIRE-367) Producers now answer to the same shape of availability test as batch operators in `sysio.opreg::termcheck`: a consecutive run says "you are offline right now", a miss rate over a rolling window says "you are chronically unreliable", and either demotes. The consequence stays DEMOTION rather than termination, and the settings are mirrored into `prodscorecfg` under producer names rather than borrowing the termination-named fields. - `producer_info` gains the window it is measured over: rounds observed, rounds missed, and when the window opened. Only rounds the producer was actually SCHEDULED for are counted, and a round observed after the window has lapsed opens a fresh one -- so time off the schedule accrues nothing and stale counts cannot greet a producer on its return. That is the resurrection CertiK raised as WNS-47, designed out at the source rather than patched. - `record_missed_round` becomes `record_round_outcome`, called for BOTH outcomes so the window rolls on produced rounds too. - The rate gate needs a minimum sample before it may fire, DERIVED rather than configured: the count at which the two gates agree. Below it the consecutive gate is strictly stricter, so the rate gate would add nothing except the power to demote on a first missed round -- at a sample of one, a single miss is a 100% rate. - `regproducer` clears the demotion and opens a fresh window but NOT the miss streak. It costs only a signature and may be repeated, so clearing the streak would let an offline operator cron its way back to healthy and never produce. The window does reset, because otherwise a rate-demoted producer can never recover: demoted it is unscheduled, so it observes no rounds and its rate is frozen. The consecutive gate is what defeats the cron loop and it is untouched. - Producing a block clears the streak and RE-DERIVES the demotion rather than forcing it false. One good round must not pardon the rate gate, or a producer missing half its rounds would clear its demotion every time it managed one. - A weight or threshold change reconciles stored demotions against the live thresholds, but only during the sweep the change itself opens, so an ordinary rescore cannot undo a re-registration. A lowered threshold binds on existing streaks; a raised one pardons nobody. Also removes the re-entry heuristic added earlier in this PR. It cleared the period's snapshot credit when a row's tier moved out of demoted, which cannot tell a stale credit from one granted in the same block -- so the ordinary demote/recover cycle destroyed live credits. The credit is now consumed at the events that actually leave the pay walk: demotion and `unregprod`. Field ORDER note: both structs declare new fields last, matching the tail of their SYSLIB_SERIALIZE. The ABI is generated from the declarations while the contract serializes in the macro's order; a disagreement decodes every later field from its neighbour's bytes, silently and plausibly. Tests: the rate gate demoting without a consecutive run and declining to fire below its minimum sample; `regproducer` leaving the streak standing; the park consuming the period's snapshot credit. contracts_unit_test 568s and contracts_snapshot_attest_test 846s both pass. Change-Id: I25a3bf98e989c4620e185e1c491e16cc5ae653c7
…ank from a half-swept index (WIRE-367) Two PR review findings, plus the documentation they change. - `sysio.opreg::setconfig` rewrites the requirement vectors and re-evaluates nobody: an operator's status is only ever re-derived when its own BALANCE moves. Raising a producer minimum therefore left producers below it ACTIVE, and scoring trusted that status, so they stayed schedulable and payable on a bond the chain no longer accepts. Scoring now tests the live minimum itself. It costs no extra reads. `collateral_factor` is already the ratio of posted bond to required minimum across every required pair, so a value below `score_scale` IS "short on at least one pair" -- the question `meets_role_min` answers, asked of numbers already in hand. Calling opreg's predicate instead would drag its pending-withdraw walk, unbounded per account, onto a path that runs for every scored row, and would reintroduce the `sysio.uwrit` dependency removed earlier in this PR. Bootstrapped producers are exempt, exactly as they are in `meets_role_min`. Only the config case needs catching here; a balance movement already re-evaluates status in opreg and notifies this contract. Convergence is by the sweep `setconfig` opens, so it is bounded rather than immediate. - `update_ranked_producers` no longer publishes from a half-swept index. A weight or minimum change invalidates every stored score at once and the sweep rewrites them a bounded batch per tick, so while it drains the index holds two configurations at once. Order within a tier picks the top 21 and the standby band, so ranking off that mixture proposed a schedule and a finalizer policy matching NEITHER configuration, repeatedly, as the sweep advanced. The throttle stamp still advances when the rebuild is skipped, or `onblock` would re-enter on every block for the sweep's duration. `payepoch` deliberately does not wait: deferring it would withhold a period's pay for a configuration change, and block pay is a per-producer count that does not read the walk order. Only the standby retainer reads position, so the exposure is one period's retainer. Docs: the withdrawal path was wrong -- an outpost bond releases through that outpost's own entry point, while `opreg::withdraw` takes an account and an amount and applies to the WIRE-native balance. Adds the tier table and states that healthy sorts ahead of bootstrapped, which is what makes genesis producers an always-on backup that yields slots as real producers arrive. Describes both demotion gates, why re-registering does not clear the streak, and why the period's snapshot credit does not survive leaving the pay walk. contracts_unit_test and contracts_snapshot_attest_test both pass, run sequentially: this host intermittently fails an unrelated councl case when the two run in parallel, and that case passes in isolation every time. Change-Id: I76710e092f75f8f3ab3c64f4114822b6bdc7df2e
…sort last (WIRE-367) The last PR review finding: the healthy tier did not bound the rank walks. An ACTIVE producer operator stayed in it without a finalizer key, so a walk looking for 21 schedulable producers could skip an arbitrary number of rows that can never qualify -- on `onblock`'s schedule rebuild and INLINE in the epoch payout, where an overrun stalls the chain. `regproducer` is permissionless and the table is unbounded, so nothing capped that. Two mechanisms, deliberately: - A producer with no ACTIVE finalizer key now scores into the demoted tier. It could not take part in finality, so it can never be scheduled, and it has no business sitting in a tier the walks traverse. This is the real bound: the healthy and bootstrapped tiers now hold only rows that could actually be scheduled. - Both walks additionally stop after a fixed number of rows EXAMINED. That is the belt to the braces: a walk running inline in an epoch advance should never depend for its CPU cost on a predicate holding. Stopping early is safe only because of the no-forfeiture rule -- a row the walk never reaches is neither paid nor reset, exactly like an unpayable one, so its blocks carry to the next payout. That reasoning is recorded at the bound, because a later change to the carry would turn a safe cap into silent loss. Peer discovery is deliberately unaffected, and the ordering is load-bearing: it seeds from the ACTIVE SCHEDULE before it ranks anything, so a producer scheduled through `setprods` without a finalizer key stays reachable by BP gossip. Sinking these rows is only safe because that landed first; the two tests point at each other so neither can be changed alone. `regfinkey`, `actfinkey` and `delfinkey` now rescore. Making the finalizer key a scoring input means every event that changes the answer has to move the stored key with it -- otherwise a stale demoted-tier score sits at the front of the index and stops the walks before the producers behind it. That is the third factor in this PR to need its event wired up, after collateral (opreg's notification) and the demotion flag (the sweep). A new scoring factor is not finished when the factor is computed; it is finished when every event that moves it rescores. Test: a producer bonded at TEN times its peers, registered, ACTIVE in opreg, but with no finalizer key holds no rank position and is not scheduled. The tier-ordering test now gives its bootstrapped producer a finalizer key, without which it is unschedulable and the comparison it makes is not the one it names. Both suites pass, run sequentially. Change-Id: I697120e8691953e9ea4a18e408a6df965d1f7899
…e (WIRE-367) The guide's recovery list already said `regproducer` does not clear the consecutive streak, but a later paragraph still told operators that calling it clears the miss penalty. That was true before the streak was made to survive re-registration and is not true now: re-registering returns a producer to the healthy tier without restoring its participation factor, and since a producer cannot produce without a slot, collateral is the only lever that works from outside the schedule. Say that, rather than promising a door that is closed. Two eligibility rules had no coverage at all. Losing the finalizer key now costs a position outright however large the bond, because a producer the chain cannot schedule scores into the demoted tier. And the collateral minimum is a governance setting that can be raised after a bond is posted, which leaves the registration ACTIVE and the balance untouched while the producer holds no rank until it tops up -- reaching the table through the background rescore, with no rebuild published until that finishes. Also: the schedule throttle is 120 block SLOTS, not blocks. Change-Id: I47c2b31be507c4bbc91508ebf3a0cf4c59b35d26
… rounds (WIRE-367) The permanent lockout first. `regproducer` deliberately preserves the consecutive streak, so a pardoned producer sits at (is_demoted false, streak >= threshold) -- and the config sweep re-derived BOTH gates on that row, re-demoting it with no new miss. Unscheduled, it could never produce the block that is its only other door back, so every later sweep undid `regproducer` again. The sweep now reconciles the RATE gate only, which is the gate that genuinely cannot self-correct: a producer off the schedule observes no rounds, so its recorded rate never improves. The consecutive gate loses nothing by waiting -- it asks "are you offline right now", which only an observed round answers, and a lowered limit binds on the next miss. Short rounds now count. A round was charged only when the WHOLE window went unproduced, so a producer could deliver one block of twelve, read as fully available, and hold a top-21 slot indefinitely. A round below `min_blocks_per_round` (new, default 6) is charged against the RATE gate, leaving the consecutive gate for rounds that produced nothing -- a total outage stays caught fast while a degraded node gets the window to recover in. The block count comes from the difference between the round's first and last block heights, both already in hand, so the 11-of-12 blocks that do not change producer do no extra work. `onblock` also stopped paying for rescores it did not need: the round path rescored unconditionally, at two cross-contract sysio.opreg reads apiece, even when no scoring input had moved. It now rescores only when the streak, the demotion flag or the snapshot credit actually changed. The schedule no longer waits for a config sweep. Deferring it put both the producer schedule and the finalizer policy behind an unbounded, permissionless table whose RAM this contract pays for -- anyone registering faster than the drain could freeze both indefinitely, during which a slashed or terminated producer keeps its slot and its finality weight. Ranking converges instead; a briefly mixed ordering is a far smaller harm than a schedule that cannot be rebuilt. Also: - `producer_info::set_demoted` makes consuming the snapshot credit a property of the transition rather than of one call site. Two of the three sites that raised the flag did not clear it, so a credit rode back in at full marks after `regproducer` and outranked producers that actually attested. `credit_snapshot_attestations` now skips demoted rows for the same reason, and `rmvproducer` -- the one is_active path this work had left unrescored -- clears it and sinks its key. - `setscorecfg` validates. `max_consecutive_missed_rounds = 0` reads as "disable the consecutive gate" and instead collapsed the rate gate's derived minimum sample to zero, demoting every producer on its first missed slot. - The `snapshot_ranked_producers` and `getpeerkeys` walks got the row ceiling the other two already had. The first is reached from a user-signed write and re-walked per entry; the second runs on the node's main thread. - Blocks carried by an unpayable row no longer inflate a later period's divisor, and the payepoch reset joins on a binary search rather than a linear scan. - `actfinkey`'s rescore moved above the pre-Savanna early return. - The live-collateral gate's rationale corrected: it measures posted balance, like the score, and deliberately not `available` -- withdraw and cancelwtdw are free, so subtracting a queued withdraw would let an operator oscillate its own eligibility. Change-Id: I17b1459a5bb3aa12c9b49daa6c7d201360e282ab
…te (WIRE-367) Output of `contracts/tools/generate-sysio-contract-types.py -B . -O /tmp/ctt -P snake -f` against the ABI from Wire-Network/wire-sysio#599. Two fields: - `prodscorecfg.min_blocks_per_round` -- blocks a producer must deliver inside its own round for that round to count as served. Below it the round is charged against the miss RATE, so a producer can no longer hold a slot indefinitely while delivering a fraction of its window. - `global.round_start_block` -- the height the current producer's round began at. Every block between it and the next producer's first block belongs to that producer by construction, so the difference IS its delivered block count, with no per-block counter. Both are declared at the tail of their structs, matching the serializer order. Change-Id: I2f9ed567b7083e1cb7278ab97af03067e521e8e8
huangminghuang
left a comment
There was a problem hiding this comment.
Additional findings on the current head. These inline comments cover issues not already tracked in the existing review threads; residual issues from those threads are posted as replies there.
| /// NOT a rank -- `rank` is position in the "prodrank" index among schedulable producers, | ||
| /// derived by iteration. Defaults to the demoted tier's worst score so a registered but | ||
| /// never-scored row can never outrank a scored one. | ||
| uint64_t rank_score = producer_rank::unscored(); |
There was a problem hiding this comment.
[P1] Add an upgrade path for existing producers and global rows
This widens persisted rank from uint32_t to uint64_t in place and replaces the existing producer tail. The same header also removes total_unpaid_blocks from the middle of the persisted global row before appending new fields. KV deserialization is positional, so rows already on chain will consume neighboring bytes as the new fields and/or underflow; onblock reads global immediately, making a populated-chain deployment capable of aborting block processing. Please add an explicit migration or a versioned compatibility decoder rather than reading the old bytes through the new structs.
There was a problem hiding this comment.
Nothing deployed, we do not need an upgrade path.
| // rest funds the per-block rate active producers are paid at. Each standby POSITION | ||
| // (22..standby_end_rank) holds a fixed, linearly decaying share of this slice; a vacant | ||
| // position's share stays in the treasury rather than flowing to the standbys present. | ||
| uint16_t standby_bps; |
There was a problem hiding this comment.
[P1] Migrate emitcfg instead of inserting into its serialized layout
standby_bps is inserted before the existing epoch_log_retention_count and pay_cadence_epochs fields. An existing row therefore shifts those two values and is two bytes too short for this decoder. Please use an explicit migration/versioned representation (or an actually compatible tail-extension strategy) before rollout.
There was a problem hiding this comment.
Nothing deployed, we do not need an upgrade path.
There was a problem hiding this comment.
Same ruling as the sibling thread — @heifner: nothing deployed, so there is no existing emitcfg row whose fields could shift.
| // then re-enters at full marks the moment `regproducer` lifts the tier, outranking producers | ||
| // that actually served the period it returns into. A demoted producer that keeps voting is | ||
| // not a fault to reject, just service that earns no rating, so this skips silently. | ||
| if (producers.get(key).is_demoted) continue; |
There was a problem hiding this comment.
[P2] Gate snapshot credit on current schedulability, not only is_demoted
unregprod leaves the provider mapping and is_demoted == false, so the retained provider can still vote and this helper credits an inactive producer. The rescore parks that row below the pay walk, where the counter is never reset, and a later regproducer restores it with service credit earned while parked. Losing the finalizer key or opreg eligibility has the same lifecycle. Please test/gate the live schedulable predicate here, or consume credit on every transition out of the walk.
There was a problem hiding this comment.
Fixed in 4fce5f4 — you are right that is_demoted was the wrong predicate. unregprod parks a row by clearing is_active and letting the rescore sink it by TIER, so the flag stays false and every parked producer was still credited; losing a finalizer key or opreg eligibility did the same. The gate is now producer_rank::is_schedulable against the live row.
Test a_parked_producer_earns_no_snapshot_credit (991f955) parks a producer, keeps it voting into a quorum, and asserts it earns nothing while the still-schedulable producer is credited.
| // cfg.pay_cadence_epochs, which a mid-period change makes disagree with the accrual -- | ||
| // at one slot per block interval. uint64: a 30-day epoch times a large cadence overflows | ||
| // uint32. | ||
| const uint64_t nominal_slots = |
There was a problem hiding this comment.
[P2] Preserve nominal slots across epoch-duration changes
This applies the current epoch duration to every epoch accrued in the open pay period. For example, a two-epoch period containing a 60-second epoch (120 slots) followed by a 120-second epoch (240 slots) has 360 nominal slots, but this computes 480 and pays only 75% of the active pool under full production. Persist accumulated nominal slots per accrual, or settle the period before applying a duration change.
There was a problem hiding this comment.
Fixed in 4fce5f4 by accruing the divisor the same way the pool already is. t5_state gains pending_nominal_slots; accrueepoch adds each epoch's slots at the duration in force for THAT epoch, and payepoch consumes and resets it alongside pending_emission_amount.
Test nominal_slots_accrue_at_each_epochs_own_duration (991f955) accrues one 60s epoch, doubles the duration, accrues a second, and asserts the total is 120 + 240 rather than the 240 x 2 the payout-time formula produced.
| "snapshot_target_attestations must be positive" ); | ||
|
|
||
| producer_rank::producer_score_config_t weights_tbl( get_self() ); | ||
| weights_tbl.set( weights, get_self() ); |
There was a problem hiding this comment.
[P2] Reject min_blocks_per_round above the fixed round size
The standard producer round has 12 slots, but this config path accepts any uint32_t. Setting 13 makes every fully produced standard round count as short and eventually rate-demotes every healthy producer. Please retain zero as the disabled value and otherwise require the threshold to be at most blocks_per_round.
There was a problem hiding this comment.
Fixed in 4fce5f4: setscorecfg now checks min_blocks_per_round <= blocks_per_round (12), with zero still the disabled spelling. Covered by setscorecfg_rejects_a_configuration_that_inverts_the_rate_gate, which asserts 13 is rejected and both 12 and 0 accepted.
Fair catch — that action gained five other bounds in the same commit that introduced this field, and I missed the one field the commit added.
| governance setting, so it can be raised after you have bonded: if that happens your registration | ||
| stays `ACTIVE` and nothing is taken from you, but you hold no rank until you top up to the new | ||
| minimum. A raised minimum reaches the table through a background rescore rather than all at once, | ||
| and the schedule is not rebuilt until that finishes. |
There was a problem hiding this comment.
[P3] Align the guide with the implemented rescore behavior
The current implementation intentionally rebuilds the schedule after each partial rescore drain and accepts mixed-generation ordering while the sweep converges. This sentence promises the opposite behavior, so operators will reason incorrectly about when a configuration change becomes visible.
There was a problem hiding this comment.
Fixed in 4fce5f4. The guide now says the schedule keeps being rebuilt while the sweep drains and that ranking converges over a few rounds rather than switching in one step. That sentence predated removing the publish freeze and I did not revisit it afterwards.
`regproducer` no longer launders the miss RATE. It cleared the window on every upsert, and short rounds made that a full bypass of the gate: a producer delivering a fraction of every round never advances the CONSECUTIVE counter -- a short round feeds the rate gate alone -- so a free, repeatable re-registration erased the only surviving evidence against it. The window now resets only on a genuine demoted-to-recovered transition, which is the case the reset exists for (a demoted producer is unscheduled, observes no rounds, and could otherwise never improve its recorded rate). The zero-rounding correction could over-distribute the pool. Removing a row's blocks from the divisor raises every remaining block's pay, so an excluded row can cross back over the rounding threshold on the second pass -- and paying it there pays for blocks the divisor no longer counts. With nominal_slots=120, active_pool=1 and two rows of 120 blocks, both round to zero at the first divisor, both leave it, the divisor falls to 120, and both would be credited 1: two units out of a one-unit pool. The first pass now DECIDES the payable set and the second only prices it. The pay divisor accrues with the pool. `payepoch` applied the CURRENT epoch duration to every epoch of the period, so a period spanning a duration change mis-sized it -- a 60s epoch followed by a 120s one is 360 slots but computed as 480, paying 75% of the active pool under full production. `t5_state` gains `pending_nominal_slots`, accumulated by `accrueepoch` at each epoch's own duration exactly as `pending_emission_amount` is, and reset with it. Also: - `setscorecfg` bounds `min_blocks_per_round` at the round size. Above it every fully produced round counts as SHORT and the rate gate demotes the network -- the same class of foot-gun the other checks in that action exist to stop, in the field the previous commit introduced. - Snapshot credit gates on the LIVE schedulable predicate rather than `is_demoted`. `unregprod` parks a row by clearing `is_active` and letting the rescore sink it by TIER; the flag stays false, so the old gate credited every parked producer, and losing a finalizer key did the same. - The operator guide drops three claims that are no longer true: that the schedule waits for a rescore sweep (it converges instead), that a minimum change binds existing operators on its own (it binds them on their next balance movement), and that nothing is EVER forfeited (settling held blocks requires being back within the pay walk's reach). `sysio.epoch.wasm` moves with no sysio.epoch source change: it deserializes `t5_state` through `sysio.system/emissions.hpp`, so the new field rebuilds it. Same coupling `standby_bps` had on `emitcfg` earlier in this branch. The field is declared last, so nothing sysio.epoch reads shifts position. Change-Id: Iabfac683c596f21a618cdcb4f7a0ce0393f504f6
…ount (WIRE-367) Output of `contracts/tools/generate-sysio-contract-types.py -B . -O /tmp/ctt -P snake -f` against the ABI from Wire-Network/wire-sysio#599. One field: - `t5state.pending_nominal_slots` — block slots the open pay period is entitled to, accumulated as each epoch accrues at that epoch's own duration. The pay divisor now builds the same way the pool does; computing it at payout from the current duration applied today's value to epochs that ran under a different one, mis-sizing any period that spans a duration change. Declared at the tail of the struct, matching the serializer order. Change-Id: Ie63369dedcb0e33c3990a172f5884f301ccd2e19
…crual (WIRE-367) Two fixes from the review round landed without tests of their own. `a_parked_producer_earns_no_snapshot_credit` pins the predicate: `unregprod` parks a row by clearing `is_active` and letting the rescore sink it by TIER, so `is_demoted` stays false and the old gate credited every parked producer. The test parks one, keeps it voting into a quorum, and asserts it earns nothing while the still-schedulable producer is credited. `nominal_slots_accrue_at_each_epochs_own_duration` accrues one 60s epoch, doubles the duration, accrues a second, and asserts the accumulator holds 120 + 240 slots rather than the 240 x 2 the old payout-time formula produced. Change-Id: Ic6c61924717bf234ce2c1abb9aeded4bdef39496
huangminghuang
left a comment
There was a problem hiding this comment.
Re-reviewed 991f955. I disregarded persisted-layout/backward-compatibility concerns as requested for the pre-launch stage. The new commits fix the ordinary re-registration reset, payout-divisor over-distribution, mixed-duration nominal-slot accrual, and min_blocks_per_round bound, but the remaining findings below prevent approval.
| // fraction of every round never advances the CONSECUTIVE counter (a short round feeds | ||
| // the rate gate alone), so if `regproducer` also cleared the window there would be no | ||
| // surviving evidence against it at all, and the rate gate could never fire. | ||
| if( info.is_demoted ) { |
There was a problem hiding this comment.
[P1] Reset the rate window only after the producer has actually left the schedule
This treats every demoted row as the documented dropped-producer recovery case, but regproducer/regproducer2 never checks whether the producer has left the active schedule. A producer demoted by the rate-only path can submit regproducer immediately, before the next schedule rebuild, and this branch clears both is_demoted and the entire rate window without a produced block or any time off-schedule. Repeating that after each demotion makes chronic short-round delivery unpenalizable; preserving the consecutive streak does not help because short rounds deliberately leave it at zero. Gate this reset on actual schedule removal (or another recovery condition that cannot be invoked while the producer still holds a slot), and cover demotion followed by immediate re-registration.
| // no rounds, so nothing accrued while it was away -- and if it stayed away longer than the | ||
| // window, its old counts lapse rather than greeting it on return. That is the WNS-47 | ||
| // shape (stale per-period state resurrecting on re-entry) designed out at the source. | ||
| if( p.miss_window_open_ms == 0 |
There was a problem hiding this comment.
[P1] Implement the advertised rolling miss window
Once the elapsed time reaches the configured window, this discards every prior observation at once, so the implementation is a tumbling bucket rather than the rolling window promised by the config, guide, and PR description. Boundary-straddling short rounds can therefore exceed the limit in a real duration-sized window while neither bucket exceeds it. For example, with a five-round minimum and a 40% limit, GGGSS | SSGGG leaves each bucket at 40%, but the sliding five-round window after the first two rounds of the new bucket is GSSSS (80%); short rounds do not advance the consecutive gate. Expire observations as they age out (or use an equivalent rolling representation) and add a boundary-straddling test.
| // false throughout -- and losing a finalizer key or opreg eligibility does the same. Gating | ||
| // on the flag would credit every one of them. A producer outside the walk that keeps voting | ||
| // is not a fault to reject, just service that earns no rating, so this skips silently. | ||
| if (!producer_rank::is_schedulable(producers.get(key), finalizers)) continue; |
There was a problem hiding this comment.
[P2] Exclude demoted-tier producers from snapshot credit
is_schedulable checks the active producer row, ACTIVE producer opreg status, and finalizer key, but it does not check is_demoted or the row's rank tier. A miss-demoted producer therefore still passes this gate and can keep voting through its retained provider mapping. Its credit then accumulates while payepoch stops before the demoted tier, and regproducer restores the row without clearing snapshot_attestations, bringing that stale score back into ranking. Gate credit on the same tier-plus-live predicate used by the pay walk and add demoted-producer coverage; the new parked-producer test exercises only is_active == false.
| }, | ||
| weights); | ||
|
|
||
| // The period's snapshot credit is cleared at the EVENTS that drop a producer out of the |
There was a problem hiding this comment.
[P2] Consume snapshot credit on every pay-walk exit
Demotion and unregprod are not the only events that drop a row out of the pay walk. Deleting the last finalizer key, losing ACTIVE producer-opreg eligibility, or falling below a raised live collateral minimum all make compute return unscored(), but those paths only call rescore and never clear an existing snapshot_attestations value. Because payepoch stops at the demoted/unscored tier, the value can survive across pay periods and reappear after re-keying, top-up, or status recovery. Detect and consume every eligible-to-ineligible transition at its event (without consuming a fresh credit on the reverse transition), and test exit -> pay epoch -> return for finalizer/opreg eligibility.
| // A pool of one unit against a full period of slots: every producer's block pay floors to zero | ||
| // on the first pass, so every row leaves the divisor. If the second pass then priced them at | ||
| // the collapsed divisor, each would be credited a whole unit out of a one-unit pool. | ||
| const int64_t tiny_emission = 1; |
There was a problem hiding this comment.
[P2] Make this regression test reach the over-distribution path
With the fixture defaults, compute_bps == 4000 and producer_bps == 7000, so tiny_emission == 1 produces compute_amount == 0, producer_pool == 0, and active_pool == 0. Both the old and fixed implementations credit zero, so reverting the new block_payable guard still passes this test. One three-producer cycle also leaves total produced blocks below the 120-slot nominal divisor, so the divisor cannot collapse in the way described above. Configure a nonzero one-unit active pool, create at least two excluded rows whose combined blocks exceed the nominal slots, and assert producer credit against the actual active pool so the pre-fix second pass demonstrably fails.
…siduals (WIRE-367) The miss window was a TUMBLING bucket, not the rolling one the config, the guide and the PR description all promised -- and `sysio.opreg::termcheck`, which this was said to mirror, is genuinely rolling: it keeps per-delivery rows and ages them out. A single bucket emptied at its duration reads under the limit in each half while the trailing window does not, and `miss_window_open_ms` is on-chain, so the boundary can be targeted deliberately: `GGGSS | SSGGG` is 40% in both buckets and 80% across the join. Short rounds never touch the consecutive gate, so nothing else would catch it. `producer_info` now carries the previous bucket, `roll_miss_window` carries the current one into it (clearing both past two full durations, so a long absence still starts fresh), and `weighted_miss_window` counts the previous bucket in proportion to how much of it the trailing window still covers. `breaches_miss_rate` is the one place that weighting is applied, so every gate reads the same number. An estimate rather than a per-observation truth -- the trade for O(1) state on a path `onblock` runs, where opreg's log shape is affordable only because it sits on a once-per-epoch path. `regproducer` answers the CONSECUTIVE gate and nothing else. Gating the window reset on `is_demoted` alone was still a loop: a producer demoted on rate can re-register immediately, before any rebuild drops it, and repeating that after every demotion made chronic short-round delivery unpunishable. The window is now rolled only when it has genuinely LAPSED, and the flag re-derived from what remains -- so a rate demotion is served out without a slot, while time still heals it and it can never become a lockout. Snapshot credit gates on the pay walk's own pair: tier AND live schedulability. Neither alone works, and the previous change traded one hole for the other -- `is_demoted` misses a parked row (`unregprod` sinks by tier, leaving the flag false), while `is_schedulable` misses a demoted one (it reads the row, opreg status and finalizer key, never the tier). The credit is also consumed inside `rescore`, on the transition OUT of the walk. Demotion and `unregprod` clear it at their own sites, but `delfinkey`, an opreg status change and a raised collateral minimum all sink a row through `compute` alone, and `payepoch` stops at that tier -- so a credit leaving by one of those doors was never reset and returned at full marks. One direction only: leaving carries no ambiguity, where inferring on the way IN cannot tell a stale credit from one earned in the same block (the reason the earlier re-entry heuristic was removed). Tests. The over-distribution regression test could not observe the bug it was written for: at `compute_bps` 4000 and `producer_bps` 7000 an emission of 1 left `active_pool` at 0, so every row credited nothing with or without the fix. Rebuilt at an emission of 5 -- the smallest that survives the splits to a pool of exactly 1 -- with enough rotations that each producer holds a full period of blocks, plus a precondition assert so it fails loudly rather than silently passing if the divisor cannot collapse. It and the credit-gate test were both verified to FAIL with their fixes reverted. `regproducer_clears_the_window_of_a_demoted_producer` asserted the rule this change replaces; it is now `regproducer_rolls_the_window_only_once_it_has_lapsed` and pins both halves -- the record survives an un-lapsed re-registration, and a lapsed window rolls. The second half is new coverage: it is what keeps a rate demotion from being permanent. Change-Id: Ifbff320eda4151a38d13b5345fe60dbc31b1695f
…indow (WIRE-367) Output of `contracts/tools/generate-sysio-contract-types.py -B . -O /tmp/ctt -P snake -f` against the ABI from Wire-Network/wire-sysio#599. Two fields on `producers`: - `prev_rounds_in_window` - `prev_missed_rounds_in_window` The previous bucket of the miss window. A single bucket emptied at its duration is a TUMBLING window: each half can read under the configured rate while the trailing window does not, and the bucket's open time is on-chain, so the boundary is targetable. Carrying the previous bucket and weighting it by how much of it the window still covers closes that in O(1) state. Declared at the tail of the struct, matching the serializer order. Change-Id: I624ec0834b9f144d5cf703628744932621a66d0d
…leness (WIRE-367) Demotion was two gates over a two-bucket rolling window. It is now one threshold: a round is SERVED at `min_blocks_per_round` blocks or more, anything less increments the streak, and the streak demotes. Scored once per round at the transition, where the block count is known. Snapshot credit carries the pay period it was earned in and `compute` ignores a stale one, replacing a write-time invariant that had to be cleared at six exits and was missed twice. Inert before T5 init, where there is no pay anyway. Pay divisor back to one pass. The two-pass correction fixed dilution that needs a sub-subunit per-block rate, and could overdraw the pool doing it. Removed: the 5 window fields and 2 config knobs, `weighted_miss_window`, `roll_miss_window`, `breaches_miss_rate`, `exceeds_miss_rate`, `rate_gate_minimum_sample`, `miss_window_view`, `set_demoted`, `record_short_round`, `reconcile_and_rescore`, the credit gate, and 10 tests that pinned mechanisms no longer present. `skip_round_of` finished the previous producer's window rather than truncating it -- harmless when only whole-window misses counted, a demoted bystander now. Change-Id: I15a511bd3e5a1e65f7fe593805cadc0662ab43be
getpeerkeys no longer returns a peer key for an out-of-schedule producer, so a gossip message can never mark the manual connection is_bp_peer. Assert on the connection itself; the "stale gossip peer dropped" half gets strictly stronger. Change-Id: Ibe2c8ad5b509451a43f90889649e306893b4a024
…row (WIRE-367) - collateral_factor skips a zero min_bond rather than asserting: the path runs under onblock, where a throw silently costs block counting, miss attribution and every schedule rebuild. - active_finalizer() returns the row in one read; is_schedulable and the schedule rebuild share it, so a rebuild does one finalizer read per producer, not three. - regsnapprov walks the ranked producers once and threads it to the capacity prune, instead of walking twice on one user-signed action. - accrueepoch reads the epoch duration without asserting the config exists. - getpeerkeys_tests covers the rank walk. The existing two cases register no finalizer key, so every row was demoted-tier and both passed on the schedule seed alone; verified failing with the walk disabled. - Comment fixes: a divisor correction the code no longer makes, an orphaned apply_demotion block, stacked doc blocks that left rescore undocumented, the payclaims recipient bound, and the walk's two predicates. ABI unchanged. Change-Id: I36bf630432fb0300700eaac441070db202021381
huangminghuang
left a comment
There was a problem hiding this comment.
Re-reviewed current head 905f622d0f15, with back-compatibility and persisted-layout concerns intentionally out of scope. All current-head CI checks pass, but I found the three current-head issues below, so I am not approving. The existing unresolved finding about lowered max_consecutive_missed_rounds not reconciling persisted is_demoted also still applies: #599 (comment)
| // streak would let an offline operator cron its way back and never serve a round. The | ||
| // streak clears by SERVING one. No cooldown -- a producer that returns unready is | ||
| // demoted again on its next unserved round. | ||
| info.is_demoted = false; |
There was a problem hiding this comment.
[P1] Require an actual schedule exit before clearing demotion
Preserving the streak does not prevent the cron bypass described in this comment, because tier_for consults only is_demoted. Once the streak is at the threshold, each next unserved round sets this flag again, but the offline operator can submit repeatable regproducer immediately afterward and before the next rebuild, restoring the healthy/bootstrap tier; repeating that once per round lets a sufficiently bonded operator remain scheduled without ever serving. The new test calls regproducer only once and therefore codifies the clear without exercising the repeat loop. Please gate this recovery on the producer actually having left the active schedule (or another non-repeatable recovery state), and cover repeated miss → re-register → rebuild.
There was a problem hiding this comment.
Confirmed and fixed in 9c19796. You were right that preserving the streak did not close this: tier_for reads the flag, not the streak, so each unserved round re-raised it and an immediate regproducer cleared it again before the next rebuild.
The pardon is now gated on absence from get_active_producers(). A producer that still holds a slot recovers only by serving a round, so the cron loop has nothing left to clear. Off the schedule there are no rounds left to serve, so that pardon clears the streak along with the flag — which also removes the reason the streak had to survive it in the first place.
regproducer_cannot_pardon_a_scheduled_producer drives the repeat loop rather than a single call: four producers keep the schedulable count under min_schedule_size, so the floor holds the target in the schedule, and it misses three rounds, re-registers, and asserts still demoted — three times over, with the streak accumulating across the attempts instead of restarting.
regproducer_pardons_a_producer_the_schedule_dropped pins the case that is still allowed, and it now asserts the demotion actually survives the schedule exit. Worth flagging that the old test was passing vacuously: a demoted producer keeps producing under the retained schedule and serves a round — recovering — before the rebuild ever drops it, so the assertions held for free.
| // overrun stalls the chain, so its cost may not depend on the demoted tier actually | ||
| // bounding it. A row past the ceiling is neither paid nor reset -- the same treatment an | ||
| // unpayable row gets -- so its blocks carry rather than vanish. | ||
| if (++examined > max_rank_walk_rows) break; |
There was a problem hiding this comment.
[P2] Expire snapshot-derived rank beyond the payout cap
Rows after this ceiling are neither reset nor rescored, yet period_start_epoch advances later in this action. Their persisted rank_score can therefore keep the prior period's snapshot term even though a fresh rescore would ignore it via snapshot_period. If higher-ranked producers later leave, such a row can move into a schedule or standby-paying position without any event that rescored it, so stale service affects the next period. Since snapshot credit is bounded by the provider set, please rescore credited providers at the boundary (or use another bounded expiry mechanism) and add a regression with a credited row beyond max_rank_walk_rows.
There was a problem hiding this comment.
Fixed in 9c19796. payepoch now rescores the mapped providers still holding credit, after period_start_epoch has moved, so the stale term stops scoring — bounded by max_snap_providers, as you suggested.
One correction to the scope: max_rank_walk_rows is not the reachable end of this. The walk breaks at the demoted tier before it reaches the ceiling, so any parked or sunk row carrying credit hits the same hole without needing 500 rows ahead of it. The fix covers both, and the comment now says so.
No regression test, and I would rather say that than imply coverage. It needs a fixture with both snapshot voting and the pay machinery, and neither existing one has both: block_spacing is 25000, which snapshot_attest_tester only affords by building those blocks before the system contract is deployed, while payepoch requires sysio.epoch authorization and a prior accrueepoch that fixture has no setup for. Building that seemed out of proportion here, but I am happy to add it if you would rather the fix not land uncovered.
| to recover in — so a bad hour costs you nothing, while a chronic pattern of half-served rounds | ||
| demotes you. | ||
|
|
||
| Two separate tests can demote you, and either is enough. They are the same pair of gates the |
There was a problem hiding this comment.
[P2] Rewrite the guide and PR body for the current model
This head deleted the rate/window gate: every round below min_blocks_per_round now increments the same consecutive streak, and only a served round (at least the configured threshold), not one block, clears it. This section still promises two gates, a rolling 24-hour rate, short-round-only rate handling, and a fresh rate window from regproducer. Lines 161–165 also say leaving the pay walk consumes snapshot credit, while the new period-stamped implementation deliberately retains same-period credit. The PR description repeats the removed rate-sweep, set_demoted, and snapshot-exit mechanics. Please update these operator/reviewer-facing descriptions to match the code.
There was a problem hiding this comment.
Fixed in 9c19796. "Staying in the schedule" is rewritten for the rule the current head actually implements: a round is served at min_blocks_per_round or better, anything less increments the same streak whether it delivered nothing or nearly enough, and only a served round clears it. The two-gate table, the rolling 24-hour rate and the short-round-only handling are gone, along with the claim that regproducer starts a fresh rate window.
The two doors back are now stated as the schedule deciding between them, which is also the behaviour change in this push. The snapshot paragraph is corrected as well: credit is stamped with its pay period and expires at the boundary, rather than being consumed when a producer leaves the pay walk.
PR description updated to match — the rate-sweep paragraph, the set_demoted sentence, and the snapshot-exit consequence.
sysio.opreg.wasm conflicted on the binary only; every source hunk merged clean. Rebuilt from the merged source rather than taking a side. The other 15 contracts reproduce byte-identically under the same toolchain.
regproducer cleared is_demoted unconditionally, so an offline operator could re-register between each demotion and the next rebuild and hold its slot without ever producing. The pardon now requires absence from get_active_producers(). A producer that still holds a slot recovers the way it always could, by serving a round; off the schedule there are no rounds left to serve, so that pardon clears the miss streak along with the flag. Clearing the streak there is what makes it safe to re-derive, which the rescore sweep now does. tier_for reads the flag rather than the streak, so a changed max_consecutive_missed_rounds never reached a producer already past it -- and one off the schedule observes no rounds, so no later miss could flip it either. payepoch's walk breaks at the demoted tier and again at max_rank_walk_rows, so a credited row past either is never reset while period_start_epoch moves on. Its stored rank_score went on scoring a rating whose period had ended; the boundary now rescores the mapped providers still holding credit, bounded by max_snap_providers. The operator guide still described the two-gate rate model that 47d3d02 replaced with a single consecutive rule, and said snapshot credit is consumed on leaving the pay walk rather than expiring with its period.
huangminghuang
left a comment
There was a problem hiding this comment.
Re-reviewed current head 9c19796f19. The regproducer pardon gate, threshold reconciliation, and operator-guide updates address the earlier findings. One snapshot-period cleanup hole remains, so I’m not approving this head yet.
| // mapped provider, and that table is capped at max_snap_providers, so rescoring the ones still | ||
| // holding it settles both cases. Must follow the t5s.set above: the score reads the period | ||
| // boundary this action just moved. | ||
| snap_providers_table snap_providers(get_self()); |
There was a problem hiding this comment.
[P2] Expire credits after provider mappings are pruned
This sweep only sees producers that still have a snapprovs row. However, prune_stale_snapshot_providers_if_full can erase a mapping when its producer falls out of the top 30 without clearing snapshot_attestations. If that credited producer is then beyond the 500-row pay walk, neither cleanup path rescores it after period_start_epoch advances, so its cached rank_score can continue carrying expired snapshot credit and can affect scheduling if higher rows later leave. Please track credited producers independently of the live mapping set, or clear/rescore the producer when pruning, and add a prune-to-period-boundary regression test.
There was a problem hiding this comment.
Confirmed and fixed in 60dff97. The boundary sweep finds credited rows through snapprovs, so a mapping erased for capacity orphaned the credit on the producer row exactly as you describe.
Taking your second option: prune_stale_snapshot_providers_if_full now consumes the credit and rescores before erasing, inside the loop it already walks. Worth noting that rescoring alone would not have closed it — within the same period compute still counts the credit, so the row would look correct right up to the boundary, where there is still no mapping for the sweep to find. The credit has to go at the moment the mapping does.
I went this way rather than tracking credited producers independently because that means new persistent state which itself needs bounding and its own pruning — more surface for the same class of hole, to preserve a rating that describes a producer that is no longer a rank-eligible provider. Scope is one call site: the other erase in regsnapprov is immediately followed by an emplace for the same producer, so no mapping disappears there.
pruning_a_provider_consumes_its_snapshot_credit covers it. It needed a fixture with both halves — capacity producers and a chain advanced to an attestable height — so it pairs capacity_producers() with cadence_periods = 1: credit a producer by vote, park it to make the mapping stale, let a full-table registration prune it, and assert the credit is gone.
One correction to my earlier reply on this thread, which was wrong on the record: I said a parked row with credit hits this hole. It does not — compute returns unscored() for an inactive row, so a parked producer carries no composite and no snapshot term at all. The rows that do carry it are miss-demoted-but-schedulable ones and schedulable rows past max_rank_walk_rows. That is also why the test asserts the cleared counter rather than a moved rank_score: on the parked row this fixture can drive into the prune, there is no composite in which to observe the term leaving. The point about the demoted tier being reachable well before 500 rows stands.
| uint32_t eligible_rounds = 0; // rounds meeting >= min_blocks threshold (per epoch) | ||
|
|
||
| uint64_t by_rank()const { return rank; } | ||
| /// Rounds this producer was scheduled for and produced nothing in, consecutively. Reset to 0 |
There was a problem hiding this comment.
[P3] Describe the served-round threshold accurately
This says only a zero-block round increments the streak and that producing any block clears it, but record_round_result now increments for every round below min_blocks_per_round and clears only when that threshold is met. The adjacent is_demoted comment has the same “producing a block” wording. Please align these field docs with the implemented served/unserved rule.
There was a problem hiding this comment.
Fixed in 60dff97. Both field docs now state the served/unserved rule: consecutive_missed_rounds counts rounds scheduled and not served at min_blocks_per_round or better — so a round that delivered some but too few blocks counts exactly as an empty one — and clears by serving a round rather than by producing a block.
is_demoted is corrected the same way, and also picks up the gate added in this push: regproducer clears it only once the schedule has dropped the producer, and is refused as a pardon while it still holds a slot. The old wording implied regproducer always clears it.
Also extended the snapshot_attestations doc to note the prune path now consumes it, per the sibling thread.
… pruned (WIRE-367) payepoch expires stale snapshot credit by walking snapprovs, so a mapping erased for capacity orphaned the credit still on the producer row: nothing rescores a producer the pay walk does not reach, and with no mapping the boundary sweep could not find it either. prune_stale_snapshot_providers_if_full now consumes the credit and rescores before erasing, inside the loop it already walks. Rescoring alone would not close it -- within the same period compute still counts the credit, so the row looks correct until the boundary, where there is again no mapping to find it by. Only one call site is affected: the other erase in regsnapprov is immediately followed by an emplace for the same producer. The producer_info field docs still described the pre-served/unserved rule -- consecutive_missed_rounds counting only empty rounds and clearing on any block, and is_demoted cleared by regproducer unconditionally rather than only once the schedule has dropped the producer. Also corrects a comment claiming a parked row can carry a stale snapshot term. It cannot: compute returns unscored() for an inactive row, so there is no composite. The rows that carry one are miss-demoted-but-schedulable rows and schedulable rows past max_rank_walk_rows.
huangminghuang
left a comment
There was a problem hiding this comment.
Re-reviewed current head 60dff9705e. The earlier prune and field-documentation findings are partly addressed, but a pending vote can recreate credit after its provider mapping is removed, reopening the same period-boundary cleanup hole. I’m not approving this head yet.
Additional documentation cleanup: contracts/sysio.system/EMISSIONS.md:71-73 still describes the removed eligible-round payout model; producer_rank.hpp:206-208 still says one produced block clears demotion; and snapshot_attest.cpp:116 still says no exit consumes snapshot credit. Current-head CI is still in progress, and the PR’s cited e2e run predates this head.
| // still on the row would orphan it -- nothing else rescores a producer the pay walk does | ||
| // not reach. Consume it here instead: the row is being pruned precisely because it is no | ||
| // longer a rank-eligible provider, so the rating no longer describes anything it can do. | ||
| if (producer_itr && producer_itr->snapshot_attestations > 0) { |
There was a problem hiding this comment.
[P2] Prevent pending votes from recreating orphaned credit
Clearing existing credit here does not cover a vote recorded before the prune. snapvotes.voters retains producer identities; if another provider later completes that tuple, credit_snapshot_attestations credits every saved voter without checking snapprovs, so this producer regains credit after its mapping is gone. For an active producer beyond max_rank_walk_rows, payepoch then reaches it through neither the pay walk nor the provider sweep, and the expired term can remain cached in rank_score. The new K=1 test finalizes before pruning and cannot catch this sequence. Please cover vote → prune → later quorum → period boundary and either require a current mapping when crediting or track credited producers independently.
There was a problem hiding this comment.
Reverted in f34561a. You are right that this reopened the hole, and chasing it further turned up something that changes the whole picture — so I have reverted the fix rather than patched it again.
Every consumer of the prodrank index breaks at the demoted tier: update_ranked_producers, this pay walk, snapshot_ranked_producers and getpeerkeys. So a demoted row's stale snapshot term orders it only among rows nothing reads, and an inactive row is unscored() and carries no composite at all. The harm needs a SCHEDULABLE row past max_rank_walk_rows — 500+ ranked schedulable producers.
That makes two of my earlier claims on these threads wrong, and I would rather retract them plainly than leave them standing:
- I said a parked row with credit carries a stale term. It does not;
computereturnsunscored()for an inactive row. - I said the demoted-tier break made this reachable well before 500 rows. The rows are reachable; the harm is not, because nothing reads order within that tier.
So the boundary sweep and the prune consumption were both buying very little. Against that, the sweep put an inline cost in every payout, prune consumption made a producer's rating depend on an unrelated producer triggering a capacity prune, and — as you found — a vote recorded before a prune re-credits at quorum with no live mapping. Both are reverted in the commit above. Staleness stays read-time via snapshot_period, and the walk's ceiling now documents the residual bound and what it does not affect.
This leaves your original finding accepted-and-documented rather than fixed, which is the same resolution the other bounded-walk finding on this PR reached. If you would rather see it actually closed, the honest way is a persistent cursor over the pay walk rather than a sweep bolted to the boundary — but that is its own change, and it needs the 500-producer case to be real first.
| Snapshot service is optional. If you want it, register a snapshot provider account with | ||
| `regsnapprov` and vote snapshot hashes with `votesnaphash`. Only votes that reach quorum are | ||
| credited, so registering alone earns nothing. The credit is a rating of the CURRENT pay period and | ||
| is stamped with it, so it expires at the period boundary rather than being consumed when you leave: |
There was a problem hiding this comment.
[P3] Document the new early-expiry condition
This still promises that snapshot credit expires only at the pay-period boundary and survives a park-and-return within the period, but a full-table regsnapprov now clears that credit when it prunes the mapping. That makes survival depend on an unrelated producer registering. Either preserve boundary-only expiry with cleanup tracking independent of live mappings, or document the prune-forfeiture exception here and in the PR description.
There was a problem hiding this comment.
Resolved in f34561a. Resolved by reverting the prune consumption rather than by documenting it — see the sibling thread. With nothing clearing credit early, this paragraph is accurate as written: expiry is at the pay-period boundary, and a park-and-return inside one period keeps the credit.
You were right that the alternative was unattractive. Making survival depend on whether an unrelated producer happens to trigger a capacity prune is arbitrary, and that was the strongest argument for dropping the mechanism instead of documenting around it.
snapshot_attest.cpp's "No exit has to consume it" is also correct again for the same reason, so I have left it — it only became wrong when the prune change introduced an exit that consumed credit. Flagging that explicitly so it does not read as overlooked.
…ng for it (WIRE-367) The payepoch boundary sweep and the prune-time credit consumption both existed to stop a stored rank_score carrying a snapshot term whose pay period had ended. The harm they prevent is far narrower than it looked. Every consumer of the prodrank index breaks at the demoted tier, so a demoted row's stale term orders it only among rows nothing reads, and an inactive row is unscored() and carries no term at all. What is left is a SCHEDULABLE row past max_rank_walk_rows, which takes 500+ ranked schedulable producers to reach. Against that, the sweep added an inline cost to every payout, and consuming credit at prune time made a producer's rating depend on whether an unrelated producer happened to trigger a capacity prune -- contradicting the boundary-only expiry the operator guide states. It also opened a hole of its own: a vote recorded before a prune credits its producer when a later vote reaches quorum, with no live mapping to gate it. Accept and document instead, as the other bounded-walk findings on this branch were. Staleness stays read-time via snapshot_period. The walk's ceiling now carries the limitation, and says what is NOT affected, which is most of it. Also corrects two docs that predate the current model: EMISSIONS.md still described the eligible-round payout, and producer_rank.hpp still said a demoted producer recovers by producing one block or by re-registering unconditionally.
|
On the documentation items from the review body, at
The PR description is updated to match: the boundary-sweep claim is replaced with the bound now accepted at the walk's ceiling. On the e2e gate — you are right that the cited run predates this head, and it predates the three heads since. That link should not be read as covering |
huangminghuang
left a comment
There was a problem hiding this comment.
Re-reviewed f34561a with backward compatibility intentionally out of scope. Reverting the prune-time cleanup removes the pending-vote regression, but the original cached-ordering defect remains actionable; see the inline finding.
| // Its snapshot counter is not reset either, and nothing rescores it, so a SCHEDULABLE row | ||
| // past this ceiling keeps a stored rank_score carrying last period's snapshot term until | ||
| // some unrelated event rescores it -- `compute` would drop the credit as stale, but the | ||
| // stored key is not recomputed. Accepted rather than swept: reaching it takes 500+ ranked |
There was a problem hiding this comment.
[P2] Expire cached snapshot scores beyond the walk cap
This remains a live ordering bug, not merely an unsupported 501st row. A producer can earn snapshot credit, then be pushed to row 501 by 500 higher-scored schedulable producers before the boundary. This payout skips it and advances the period. If just one higher row later exits, the stale key can put that producer at row 500 ahead of a candidate whose live score lies between its base and snapshot-boosted scores; the next payout can process the wrong row's carried blocks and include them in the shared divisor. With more churn, the same expired key can enter the active producer/finalizer set or standby band. Nothing rescores the row when entries above it move, and serving with an already-zero streak leaves it untouched. Since 500 is a scan guard rather than an enforced producer cap, documenting the inconsistency does not make the index safe. Please expire these keys through bounded persistent/lazy work or enforce the hard cap.
There was a problem hiding this comment.
Not fixing this one, and I want to be direct about why rather than argue the mechanism — your analysis of it is correct.
The scenario needs 500 schedulable producers ranked above the affected row, as your own description says: bonded on every required chain, ACTIVE in opreg, and holding an active finalizer key, all scoring higher. This network is not expected to exceed roughly 35 producers. At that size the pay walk never reaches its ceiling, every row is rescored at every payout, and no key is ever stale — the defect is unreachable rather than merely unlikely.
You are right that 500 is a scan guard rather than an enforced cap, so nothing structurally prevents it. That is a real gap, and the place to close it is the cap, not the expiry: sysio.opreg's max_available_producers is stored and never enforced, which is the same root the earlier unbounded-walk thread landed on. That belongs in its own change against opreg rather than in producer ranking, and it wants the 500-producer case to be plausible first.
So this is an accepted, documented bound for launch, on the same basis as the other bounded-walk finding on this PR. The comment at the walk's ceiling states it, and states what it does not affect: a demoted row's stale term is inert because every walk breaks at that tier, and an inactive row is unscored() and carries no term at all. If the network ever approaches that scale, this needs revisiting together with the walk cap itself — the cap is the load-bearing assumption, not the expiry.
| Snapshot service is optional. If you want it, register a snapshot provider account with | ||
| `regsnapprov` and vote snapshot hashes with `votesnaphash`. Only votes that reach quorum are | ||
| credited, so registering alone earns nothing. The credit is a rating of the CURRENT pay period and | ||
| is stamped with it, so it expires at the period boundary rather than being consumed when you leave: |
There was a problem hiding this comment.
[P3] Keep the operator docs consistent with the retained bound
This categorical promise conflicts with the accepted behavior documented in emissions.cpp: a schedulable row beyond the 500-row walk can retain a prior-period snapshot term in its stored key, and that term becomes observable if the row later moves back into range. Either fix expiry as above or qualify this guide. Please also clean up the related stale comments: snapshot_attest.hpp:151-153 says operator-registry status is not consulted even though registration now walks is_schedulable, and producer_rank.hpp:151-154 / producer_pay.cpp:68-70 still describe any/first produced block as clearing the streak rather than a served round.
There was a problem hiding this comment.
The three stale comments are fixed in 8e9cfb8:
snapshot_attest.hpp— you are right, and it was backwards: registration walksis_schedulable, so opreg status and the finalizer key are both consulted. The comment now says so instead of claiming the opposite.producer_rank.hpp— the streak clears on a SERVED round atmin_blocks_per_roundor better, andregproducerpardons only once the schedule has dropped the producer.producer_pay.cpp— the mid-round early return said the miss counter "was already cleared on the first block of this round", which is the old per-block model. Nothing is scored mid-round; a round is judged once, at the transition where its block count is known.
Comment-only — the rebuilt wasm is byte-identical.
On qualifying the operator guide: leaving it as written, because I think it is already accurate at the level it speaks. It promises that the credit does not survive the pay-period boundary, and that holds unconditionally — compute ignores a count whose snapshot_period is not current, at any rank. What can survive past the walk's ceiling is a stale cached sort key, which is an implementation detail the guide never makes a claim about. Qualifying an operator-facing document with a condition that needs 500+ schedulable producers would mislead more readers than it informs.
…und model (WIRE-367) snapshot_attest.hpp said operator-registry status is deliberately not consulted at snapshot-provider registration. It is: the rank walk that gates registration tests is_schedulable, which reads both opreg status and the finalizer key. producer_rank.hpp and producer_pay.cpp still described a produced BLOCK clearing the miss streak. A round clears it by being SERVED at min_blocks_per_round or better, and regproducer pardons only once the schedule has dropped the producer. Comment-only: the rebuilt contract wasm is byte-identical.
Closes WIRE-367.
Producer rank stops being a governance write and becomes position in a score-ordered index, and producer pay stops being a per-round threshold and becomes per block produced. A candidate could previously complete every self-service step and reach
OPERATOR_STATUS_ACTIVEyet never be scheduled, because rank was only ever written by hand.The on-ramp this completes
A stranger can now bond on Ethereum and Solana and be scheduled, with no vote and nobody's approval. Only the bootstrapped flag needs the registry's authority; everything else is self-signed:
sysio.authex::createlinkon each chain, so an outpost deposit is attributable to the WIRE account.sysio.opreg::regoperator(account, OPERATOR_TYPE_PRODUCER, is_bootstrapped = false).ACTIVEand is rescored on the bond it posted.regproducer+regfinkey.flow-producer-registrationdrives that path against real outposts. It registers the producer and its finalizer key before any collateral and asserts no schedule ever names it, so entry is demonstrably driven by the bond; at the end it withdraws the Ethereum bond and asserts the producer leaves. In between the account produces a real block — also the first end-to-end coverage ofregfinkey→set_proposed_finalizers.New:
docs/becoming-a-block-producer.md, the operator-facing guide.What changed
rank_scorereplacesrank— two tier bits (healthy / bootstrapped / demoted) over a weighted composite. Tier always beats score.relay/api/benchmarkship at zero pending an attestation path.onblocknow walks the schedule between the previous block's producer and this one's, scoring each round once. A round is served atmin_blocks_per_roundor better; three unserved in a row demotes.regproduceris the door back and clears the streak with the flag — it is refused as a pardon while the producer still holds a slot, or an absent operator could clear the flag on a timer between each demotion and the next rebuild.is_schedulable(active row, ACTIVE operator, finalizer key) governs ranking, pay and snapshot eligibility.is_eligible_operatordrops the key requirement and is what peer discovery walks, seeded from the chain's own active schedule.regproduceris permissionless, socomputesinks every unschedulable producer into the demoted tier, which sorts last — a walk that breaks there has seen every schedulable producer, withmax_rank_walk_rowsas backstop.onblockdrains a bounded cursor rather than rewriting an unbounded table inline. The rebuild does not wait for it: ranking converges over a few ticks, because deferring would let anyone freeze the schedule and finalizer policy by registering. The drain also re-derives the demotion flag from the streak, so a changed threshold reaches producers already past it.setscorecfgvalidates its fields, including rejecting amin_blocks_per_roundabove the round size.Pay is per block produced
The old per-round threshold is gone, along with
eligible_rounds,current_round_blocks,last_block_numand the globaltotal_unpaid_blocks.onblockincrementsunpaid_blocks.payepochsplits the producer pool into active and standby slices (standby_bps, default 800):active_pool × blocks / divisor, the divisor being the period's nominal slots raised to blocks actually produced when a period runs long. A missed block is not counted, so its pay stays in the treasury rather than going to whoever did show up.standby_end_rankdraw a per-position share decaying linearly. A vacant position pays nobody.payepochat which it is payable. Settling does require being back within the walk's reach.Snapshot credit is stamped with the pay period that earned it and expires at the boundary rather than being consumed on leaving the walk. One bound is accepted rather than swept for: the walk breaks at the demoted tier and at
max_rank_walk_rows, so a row past either is not reset, and a schedulable row past the ceiling keeps a stored key carrying the previous period's term until something rescores it. Reaching that takes 500+ ranked schedulable producers; a demoted row's stale term is inert, since every walk breaks at that tier and nothing reads the order within it, and an inactive row isunscored()and carries no term at all.One shape is deliberate: a miss short of demotion still costs a slot if it drops the producer below the active set. More collateral outranks the penalty, and serving clears it. A producer that missed is worth less than one that did not, and the way back is an explicit assertion of readiness rather than the passage of time.
Test cost
Making producers schedulable made the snapshot-attestation fixture expensive: with finalizer keys registered, the node signs and verifies a vote on every block of the
block_spacingadvance. The fixture now builds that history in its constructor, before the system contract is deployed, so those blocks run no contract code. The suite is also its own ctest entry.TIMEOUTWIRE-382 tracks making
block_spacingconfigurable, which retires the advance entirely.Validation
contracts_unit_testandcontracts_snapshot_attest_test, with cases covering each review round: pay that rounds to zero keeps its blocks, an unserved round counts against the producer, a raised collateral minimum sinks the producers now below it, a bonded producer without a finalizer key holds no rank,regproducercannot pardon a producer that still holds a slot, and a changed miss threshold reconciles an existing demotion.producer_rank_teston a live 5-node cluster, covering demotion and recovery — what a single-process contract test cannot reach. It stops a scheduled producer's node, asserts demotion at exactly the threshold with no miss charged to the producers that kept going, and asserts themin_schedule_sizefloor retains it rather than publishing a short schedule.producer-registrationandemissions-soak— the latter being where genesis producers newly drawing producer pay would have surfaced.Stack
Merge first. Downstream, in order:
producer-registrationflowNote for whoever merges: this repo's ruleset requires approval from the
cicdteam specifically, and auto-merge is disabled repo-wide.