Contracts: credit claimable balances instead of pushing transfers on never-throw paths - #558
Contracts: credit claimable balances instead of pushing transfers on never-throw paths#558heifner wants to merge 11 commits into
Conversation
…never-throw paths sysio.token::transfer calls require_recipient on both sides, and apply_context::exec dispatches notified receivers with no exception isolation, so an assert inside a recipient's on_notify handler aborts the entire transaction including every parent inline action. A handler that burns CPU instead of asserting is the same vector, so the handler must not run at all rather than be tolerated. That made every pushed payout on a never-throw path abortable by its recipient. Three sites sat in the sysio.epoch::advance inline subtree, where an abort halted epoch advancement, emissions accrual and outbound envelope construction chain-wide: the swap-from-WIRE refund in uwrit::drainfwq, the producer / batch-op / category payouts in sysio.system::payepoch, and the WIRE-chain collateral remits in sysio.opreg. The refund case was permanent and cost one min_fromwire_amount escrow: the drain's queue-row erase rolled back with the abort, so the offending row survived to re-block every later epoch. A fourth, sysio.reserv::paywire, stalled OPP consensus dispatch. All four now credit a claimable balance and transfer nothing. The recipient pulls it with claimpay / claimwire / claimremit, which carry the claimant's own authority, so a hostile handler blocks only its own payout. This generalizes the pattern sysio.dclaim already used, into a shared sysio.opp.common/claimable.hpp. Credits saturate at the sysio::asset magnitude limit rather than the integer limit, so a credited balance is always payable; claim actions erase the row before queueing the transfer, closing the re-entrancy window a notify handler would otherwise use. Unclaimed pay still sits in the treasury's token balance while being fully owed, so sysio.system::fundclaim's balance cap and sysio.epoch's emissions readiness gate both reserve it -- otherwise the treasury double-commits and a later claim fails on overdraw. sysio.reserv's claimable set is unbounded and caller-influenced, so those rows carry an expiry swept back to the treasury; the bounded producer and operator sets do not expire. Existing tests that asserted recipient balances after a payout now claim first and assert the same end state, so they cover the credit and the pull together. Treasury conservation becomes "balance decrease + outstanding claims == distributed", which collapses to the original assertion once every recipient has claimed.
…le-payouts # Conflicts: # contracts/sysio.reserv/src/sysio.reserv.cpp # contracts/sysio.reserv/sysio.reserv.wasm
|
Hub UX companion opened: Wire-Network/wire-hub-webapp#159. It surfaces the aggregate |
…le-payouts
Master's swap-fee-distribution work (underwriter + reserve-owner fee payouts,
the reservcfg emissions dial, quote settlement) landed across the same
sysio.reserv / sysio.system surface this branch converts to claimable payouts.
Conflicts resolved by keeping master's fee MODEL and this branch's payout
MECHANISM.
Contracts:
* sysio.reserv::paywire — credit `wire_out` via `credit_wire_claim` (this
branch) while routing the fee through master's 3-arg
`route_wire_fee(self, fee, underwriter)`. Header docs merged the same way:
master's owner-fee/underwriter-accrual/emissions-dial description plus this
branch's credit-not-push rationale.
* sysio.reserv::refundwire — same shape, with master's
`route_wire_fee(self, fee, name{})` (a revert has no winning underwriter).
* sysio.reserv header/impl — union of both sides: this branch's `claimwire` +
`wireclaims` table alongside master's `setconfig`, `setrsvfee`, `rsvfeebal`,
`claimrsvfee`, `uwfeebal`, `claimuwfee`, `uwfees` and `reservcfg`. The
rewards_bucket custody invariant now carries master's uw/owner accrual terms
AND this branch's `Σ wireclaims.balance`.
* sysio.system::payepoch — master deleted the producer fee-share split, so the
stale `emis_pay`/`fee_pay`/`fee_to_producers` arm is gone; the producer payout
is master's single `pay` credited via `credit_pay`.
Tests — three cases merged cleanly as text but not as behaviour:
* emissions_tests payepoch_pays_swap_fee_to_active_batch_operator and the
pay_cadence mid-period case (both new on master) asserted a PUSHED balance
delta; payepoch now credits `payclaims`, so they use the existing
`get_wire_balance_paid` helper, which claims first.
* reserv_tests refundwire_routes_revert_fee expected reserv custody to drop by
the emissions half (992). Master replaced the fixed 50/50 rewards/emissions
split with the `fee_emissions_share_bps` dial defaulting to 0 — the same
fixture's own comment says so — so with no `setconfig` call nothing leaves
custody at refund time and the balance stays 1000.
Artifacts: sysio.reserv.{wasm,abi} and sysio.system.wasm are rebuilt from the
merged source (both had conflicting source in this commit). Every other wasm is
byte-identical to origin/master's committed artifact — adopted by the merge, not
rebuilt locally.
contracts_unit_test: 623 cases, *** No errors detected
Change-Id: I4dc02b4335ad339e570b2684e577c3512de6b274
…le-payouts Change-Id: If788b30fd190eed7a6980345f581a96037417e49
| // chain-wide. Pay is credited here instead and pulled later by `claimpay`, which carries the | ||
| // claimant's own authority -- so a hostile recipient blocks only its own payout. | ||
| // | ||
| // The set is bounded (ranks 1..standby_end_rank, plus batch-op group members and the two category |
There was a problem hiding this comment.
The recipient set is bounded only concurrently, not across the lifetime of this table. Producers and batch operators can churn; every departed account that does not call claimpay leaves a payclaims row forever, paid for by sysio. The same assumption is made for remitclaims after a terminated operator is pruned. This makes system-funded claim storage grow with historical participants. Please add a retention/cleanup path or a payer model that does not turn unclaimed historical payouts into permanent system state.
There was a problem hiding this comment.
You're right, and the comment was wrong to claim otherwise — fixed in c453c0c.
"Bounded" held only concurrently. Producers and batch operators churn, a terminated operator is pruned, and every departed account that never calls claimpay / claimremit leaves its row billed to the sysio RAM pool forever, so storage grows with historical participants rather than with the live set. Both inline comments now state that plainly instead of asserting a bound that isn't there.
What landed is the schema half, not the policy half. pay_claim and remit_claim now carry expires_at_sec plus a byexpiry expiry-ordered secondary index, maintained by claimable::credit on every credit — the same shape sysio.reserv::wire_claim already uses. Nothing expires: no sweep_expired is wired against either table, so the stamp is recorded and ignored, and behaviour is unchanged.
Splitting it that way was deliberate. The schema is free to add before launch and expensive after, and landing it now means a later retention pass inherits real age data instead of starting its clock the day the field is added — which matters here, because refreshing on every credit is exactly what makes the stamp a "last had activity" signal rather than a "row created" one, so an account still being paid never looks stale.
What I did not do is wire the sweep, because the remaining question isn't a RAM one. These two tables hold earned pay and returned collateral, so sweeping means forfeiting them — which is why they were left unswept while wireclaims (unbounded and caller-influenced) was not. That needs a decision, and the in-tree precedents don't settle it: sysio.dclaim::pending_claims forfeits earned staking rewards after 180d by default (10y max, reverting to the capital fund), while wireclaims uses 1 year to the treasury. PAY_CLAIM_WINDOW_SEC / REMIT_CLAIM_WINDOW_SEC are set to 1 year purely to compute the recorded stamp and carry no commitment.
That decision, wiring the sweep, and choosing a sink for reclaimed value are tracked in WIRE-339 (low priority — it can land any time now that the schema is in). Worth noting the destinations differ: payclaims needs no transfer at all, since the WIRE already sits in the treasury balance and a sweep would only decrement payclaimtot, whereas remitclaims holds CORE_SYM in opreg custody and needs an explicit sink.
ABI note: both rows gain a uint32 expires_at_sec and a byexpiry uint128 secondary index, so the wire-libraries-ts regenerate this PR already needs covers these too.
contracts_unit_test: 623 cases, *** No errors detected
Review finding (huangminghuang, PR #558): both tables were documented as holding a BOUNDED recipient set and therefore needing no expiry. That is true only concurrently, not across the table's lifetime. Producers and batch operators churn, and a terminated operator is `prune`d, so every departed account that never calls `claimpay` / `claimremit` leaves a row billed to the sysio RAM pool forever — system-funded storage growing with historical participants rather than with the live set. The justification was wrong; both comments now say so. The rows now carry `expires_at_sec` plus a `byexpiry` expiry-ordered secondary index, maintained by `claimable::credit` on every credit — the same shape `sysio.reserv::wire_claim` already uses. NOTHING EXPIRES. No `sweep_expired` is wired against either table, so the stamp is recorded and ignored; behaviour is unchanged. The schema is landed now because it is free before launch and expensive after, and because it means a later retention pass inherits real age data instead of starting its clock the day the field is added. Refreshing on every credit is what makes the stamp a "last had activity" signal, so an account still being paid never looks stale. `PAY_CLAIM_WINDOW_SEC` / `REMIT_CLAIM_WINDOW_SEC` are 1 year, mirroring `WIRE_CLAIM_WINDOW_SEC` so the claimable tables age on one scale. They are provisional and carry no commitment: whether earned pay and returned collateral should be forfeited at all — and over what window — is an economic decision, not a RAM one, and stays open in WIRE-339 along with wiring the sweep and picking a sink for reclaimed value. ABI: `pay_claim` and `remit_claim` each gain a `uint32 expires_at_sec` and a `byexpiry` uint128 secondary index, so the wire-libraries-ts companion regenerate this PR already needs covers these too. contracts_unit_test: 623 cases, *** No errors detected Change-Id: Iea369b188489b6aeee045ca3e2046b5d010920df
huangminghuang
left a comment
There was a problem hiding this comment.
Approved. The remaining claim-retention risk is explicitly accepted and tracked in WIRE-339.
…le-payouts The only conflict was the binary contracts/sysio.opreg/sysio.opreg.wasm. Both sides had rebuilt it: #549 refreshed it through sysio.uwrit.hpp, which opreg's include path pulls in, while this branch changed opreg's own source. Resolved by rebuilding from the merged tree rather than taking either side. Rebuilt every contract whose source this PR modifies — sysio.epoch, sysio.opreg, sysio.reserv, sysio.system, sysio.uwrit, and the new block_transfer test contract. Their committed binaries did not match what their own source compiles to, and CI builds with BUILD_SYSTEM_CONTRACTS=OFF and executes the committed .wasm, so those stale binaries were what the suite actually tested. Every ABI is byte-identical before and after, so the regenerated SDK types in wire-libraries-ts#66 are unaffected. sysio.chalg, sysio.dclaim and sysio.msgch are left exactly as the merge produced them: this PR does not touch their source.
| // accounts rather than attacker-controlled ones, so they are not the live threat -- but routing | ||
| // them through the same claimable path removes a latent halt should either ever carry code, and | ||
| // keeps every payepoch payout on one mechanism. | ||
| credit_pay(get_self(), CAPEX_OPERATIONS_ACCOUNT, capex_amount, memo::capex); |
There was a problem hiding this comment.
[P1] Keep the automatic payout path for the T5 bucket accounts. claimpay requires sysio.ops / sysio.gov authority, but the documented production bootstrap creates all remaining sysio.* accounts with RAM only and zero NET/CPU; the updated fixture explicitly skips these two claims for that same reason. Neither account has a contract/claimer that can run this action, so capex and governance allocations are left permanently in payclaims while payclaimtot reserves the backing WIRE. These recipients are protocol-controlled rather than hostile handlers, so retain the direct transfer here or add a funded, authorized claim mechanism as part of the bootstrap.
There was a problem hiding this comment.
Fixed in 6e17468 — both buckets are pushed again, and you were right that neither could ever run the claim.
Digging into why produced a stronger reason than "the bootstrap creates them with RAM only". It is structural:
sysio.roa::is_sysio_accountmatches any account whose prefix issysio, and bothset_reslimitandincrease_reslimitthen forcenet_weightandcpu_weightto zero for it. Funding them is not merely undone by the documented bootstrap — no ROA policy can give them transaction resources at all, because the contract zeroes whatever weights it is passed.- The mechanism
sysio.*accounts actually use instead is a privileged contract emitting the action inline on their behalf.sysio.dclaimis the precedent:fundclaimisrequire_auth-gated exactly likeclaimpay, and dclaim's own privileged contract emits it.sysio.opsandsysio.govare pure holding accounts with no contract, so nothing can emit it for them.
So the pull model has no reachable claim path for these two, and crediting them converts an earned allocation into a permanently growing payclaimtot reservation of WIRE that nobody can move — worse than the halt risk it was guarding against.
Pushing is the right call for the reason you gave: the pull model exists because a payout to an account the protocol does not control lets that account's notify handler abort advance, and these are protocol-owned accounts with no code. This PR already makes the identical exception for sysio.dclaim via fundclaim, so this is one consistent rule rather than a special case. The constraint that follows lands on the accounts rather than on payepoch, and I have written it there: any contract governance ever deploys on either MUST NOT assert or burn CPU in an on_notify("sysio.token::transfer") handler.
Treasury accounting needed no change and I verified it stays consistent rather than assuming: fundclaim's claims_reserve and sysio.epoch's readiness gate both read payclaimtot.outstanding, and this pay now leaves the treasury balance instead of being reserved inside it — both sides move together, so there is no double-commit and no under-reservation.
Tests move back to balance deltas for the two buckets, and claim_pay's skip for them is deleted: it existed only to model accounts that could not sign, which is no longer a case the fixture needs. payclaims now contains exactly the recipients that can actually claim.
contracts_unit_test: 639 cases, *** No errors detected
| reserve::wire_claim{.account = recipient}, amount, | ||
| now_sec + reserve::WIRE_CLAIM_WINDOW_SEC); | ||
|
|
||
| // On-write retention: sweep a bounded number of rows that aged out before adding more. Runs |
There was a problem hiding this comment.
[P2] Give expired wireclaims a deterministic sweep trigger. This only sweeps while a later paywire or refundwire credit occurs. If traffic stops after caller-influenced claim rows are created, no action ever revisits them after the one-year deadline; claimwire also does not reject an expired row. That leaves both the system-funded unbounded table and the WIRE it reserves indefinitely. Add a bounded permissionless/epoch-driven sweep (and enforce expiry when claiming), rather than relying on future settlement traffic.
There was a problem hiding this comment.
Fixed in c546b32 — the sweep now has an unconditional trigger, and the deadline is enforced at the claim.
sysio.reserv::sweepclaims(max_rows) (auth sysio.epoch or self) is inlined by sysio.epoch::advance with MAX_CLAIM_SWEEP_PER_EPOCH, sized like the other advance-inlined bounds so a backlog drains across epochs rather than threatening advance's CPU deadline. The sweep body is factored into one helper shared with credit_wire_claim, so the opportunistic trigger and the epoch trigger cannot drift on what "expired" means or where the reclaimed WIRE goes.
You were right that the claim path needed it too, and the reason is worth stating: the sweep is bounded and best-effort, so a row can outlive its expiry by many epochs while it waits its turn. Without a check at claimwire the window would have meant "swept eventually" rather than "claimable until", and a balance the retention policy had already extinguished would still pay out in the meantime. It now refuses with a message that says the row is forfeit and pending sweep, rather than silently paying.
One thing worth flagging, because it changes the shape of the fix: the inline is guarded on is_account(RESERV_ACCOUNT). My first cut was unguarded and failed 124 test cases with inline action's code account sysio.reserv does not exist — the emissions fixtures advance epochs without ever creating that account. Dispatching an inline action to an absent code account is a hard action_validate_exception, and inside advance that is a chain-wide epoch stall, so an unguarded call would have traded a growing table for the exact failure mode this PR exists to remove. sysio.reserv is not a precondition for advancing an epoch, and nothing can have accrued a wireclaim before it exists, so skipping the sweep in that state is precisely correct rather than merely defensive.
Tests:
expired_wire_claim_is_swept_without_further_traffic— credits a claim, ages past the window with NO further credits so the opportunistic sweep never fires, then asserts the claim is refused, a zero budget is a no-op, and the sweep reclaims the row to the treasury leaving nothing to claim. I verified it is a real regression: with the claim-time check removed it fails on the refusal assertion.sweepclaims_leaves_live_rows_alone— a live row survives the sweep and still claims normally.
Gap I did not close: no test asserts that advance actually inlines this. The reserv fixture cannot reach the epoch machinery, and a dedicated dispatch test would need to age a claim a full year inside a fixture whose epoch and lock timing assume otherwise. Breakage is not silent today — any missing or malformed inline aborts advance and fails the many cases that advance, which is exactly how the guard bug surfaced — but a future deletion of the inline would go unnoticed. Happy to add the dispatch-level test if you want that covered.
contracts_unit_test: 641 cases, *** No errors detected
Review finding (huangminghuang, PR #558): `sysio.ops` and `sysio.gov` were credited like every other recipient, but neither can ever claim, so their pay accrued in `payclaims` forever while `payclaimtot` reserved the backing WIRE. They cannot claim for a structural reason, not a bootstrap oversight. `claimpay` needs `require_auth(account_name)`, and a `sysio.*` account can neither sign nor be acted for here: * `sysio.roa::is_sysio_account` matches any account whose prefix is `sysio`, and both `set_reslimit` and `increase_reslimit` then force `net_weight` and `cpu_weight` to zero for it. No ROA policy can give these two the resources to send a transaction -- the contract zeroes whatever weights are passed. * The mechanism `sysio.*` accounts actually use is a privileged contract emitting the action inline on their behalf (`sysio.dclaim` does exactly this for `fundclaim`, which is likewise `require_auth`-gated). `sysio.ops` and `sysio.gov` are pure holding accounts with no contract, so nothing can emit it for them. So the pull model has no reachable claim path for these two, and crediting them converts earned allocations into a permanently growing reservation of WIRE that nobody can move. They are pushed instead. The pull model exists because a payout to an account the protocol does not control lets that account's transfer-notify handler abort `advance`; these are protocol-owned accounts holding no code, so no handler exists. `fundclaim` already makes the same exception for `sysio.dclaim` on the same reasoning. The standing constraint that follows is on the ACCOUNTS, not on payepoch: any contract governance ever deploys on either MUST NOT assert or burn CPU in an `on_notify("sysio.token::transfer")` handler. Treasury accounting needs no change and stays consistent: `fundclaim`'s `claims_reserve` and `sysio.epoch`'s readiness gate both read `payclaimtot.outstanding`, and this pay now leaves the treasury balance instead of being reserved in it -- both sides move together, so there is no double-commit and no under-reservation. Tests: the two buckets go back to balance-based assertions, and `claim_pay`'s skip for them is deleted -- it existed only because they could not sign, which is no longer a case the fixture has to model. `payclaims` now holds exactly the recipients that CAN claim. contracts_unit_test: 639 cases, *** No errors detected Change-Id: I71b262c790f9b72783d56f9b3fe13edb1ba88288
…the deadline Review finding (huangminghuang, PR #558): the retention window only closed as a side effect of a later credit. `credit_wire_claim` sweeps after crediting, so with settlement traffic stopped nothing revisits an aged-out row -- leaving the system-funded table and the WIRE it reserves outstanding indefinitely. And `claimwire` did not check expiry, so the deadline was never binding on a claim. Two changes: * `sysio.reserv::sweepclaims(max_rows)` (auth: `sysio.epoch` or self), inlined by `sysio.epoch::advance` with `MAX_CLAIM_SWEEP_PER_EPOCH`. That makes the sweep unconditional rather than traffic-dependent. The budget is sized like the other advance-inlined bounds so a backlog drains across epochs instead of threatening advance's CPU deadline. The sweep body is factored into one `sweep_expired_wire_claims` helper shared with the credit path, so the two triggers cannot drift on what "expired" means or where the WIRE goes. * `claimwire` refuses a row past `expires_at_sec`. The sweep is bounded and best-effort, so an expired row can wait many epochs for its turn; without this the deadline meant "swept eventually" rather than "claimable until", and a forfeit balance still paid out in the meantime. The inline is GUARDED on `is_account(RESERV_ACCOUNT)`. Dispatching to an absent code account is a hard `action_validate_exception`, and inside `advance` that is a chain-wide epoch stall. `sysio.reserv` is not a precondition for advancing an epoch -- a chain can advance before reserves are ever deployed -- and nothing can have accrued a wireclaim in that state, so skipping is exactly right rather than merely defensive. The unguarded first cut failed 124 cases with "inline action's code account sysio.reserv does not exist"; the emissions fixtures advance without ever creating that account. Tests: * `expired_wire_claim_is_swept_without_further_traffic` — credits a claim, ages past the window with NO further credits (so the opportunistic sweep never fires), asserts the claim is refused, a zero budget is a no-op, and the sweep then reclaims the row to the treasury and leaves nothing to claim. Verified as a real regression: without the claim-time check it fails on the refusal. * `sweepclaims_leaves_live_rows_alone` — a live row survives the sweep and still claims normally. contracts_unit_test: 641 cases, *** No errors detected Change-Id: I5a788ed53df6915ba94629af8383d98651f1f271
| // | ||
| // The row is erased before the transfer is queued (inside pay_out), so a notify handler that | ||
| // re-enters claimpay finds no row and cannot double spend. The outstanding-total counter is | ||
| // decremented in lockstep, releasing the reserve that fundclaim and the epoch gate hold against it. |
There was a problem hiding this comment.
[P1] Reserve outstanding epoch pay against node-owner withdrawals too. payclaimtot protects unclaimed pay in the epoch gate and fundclaim, but claimnodedis (lines 450-479) still spends from the same gross sysio WIRE balance. For example, after payepoch credits 60 while 100 remains in sysio, a vested node owner can withdraw 50; that transfer succeeds, leaving only 50 backing the 60 already owed, so the later claimpay overdraws and future pay epochs remain balance-blocked. Before this PR the epoch pay had already left the treasury, so this competing outflow is newly exposed. Reject or cap that withdrawal against balance - payclaimtot.outstanding (and audit any other non-payclaim WIRE outflow), with a regression test proving existing claims remain payable.
There was a problem hiding this comment.
Fixed in 8469473 — claimnodedis now checks against balance - payclaimtot.outstanding.
Your walkthrough is exactly the failure, and your last sentence is the part that makes it this PR's problem: before payouts became claimable, epoch pay had already left the treasury by the time a node claim ran, so there was nothing outstanding for it to spend into. Making pay claimable created a window where two independent draws target the same balance and only one of them knows about the other.
I rejected rather than capped. Capping would mean either transferring less than info.claimable while claimed advanced by the full amount — silently destroying the difference — or threading a partial amount through the vesting accounting, which is a lot of new surface for a case that resolves itself. Rejecting keeps claimed exactly in step with what was transferred, leaves the claim fully available, and claimnodedis is user-initiated so the throw reaches the caller who asked.
I took the "audit any other non-payclaim WIRE outflow" seriously rather than fixing only the site you named. Every treasury outflow in sysio.system:
claimpay— pays frompayclaimsand decrementsoutstandingin lockstep. Self-consistent.fundclaim— already subtractsclaims_reserve.payepoch's category-bucket pushes — run inside the epoch readiness gate, which already reservesoutstanding.claimnodedis— the one unreserved draw.
So this was the only gap, not the first of several.
claimnodedis_reserves_outstanding_epoch_pay covers it end to end: a fully vested T1 owner, epoch pay credited through the real payepoch path, and the treasury drained to exactly one unit short of pay + claim. The withdrawal is refused and nothing is spent; funding that one missing unit lets the same claim through; and afterwards the epoch pay is still fully backed and really is payable — the property the reserve exists to protect, asserted by actually pulling it. Verified as a real regression: with the check removed the withdrawal succeeds.
| const uint32_t now_sec = static_cast<uint32_t>(current_time_point().sec_since_epoch()); | ||
|
|
||
| reserve::wireclaims_t claims(self); | ||
| sysio::opp::claimable::credit( |
There was a problem hiding this comment.
[P2] Do not revive a forfeited claim when the account is credited again. claimwire rejects as soon as now >= expires_at_sec, but this upsert first adds the new payout to an expired balance and refreshes its timestamp; the sweep below then sees the row as live. If an expired row is still waiting behind the bounded epoch backlog, any later paywire or refundwire for that account makes both the forfeited old amount and the new amount claimable again, and periodic small credits can keep the system-funded row alive indefinitely. Before accumulating, detect a recipient row expired at now, reclaim/erase its old balance to the treasury, then create the fresh claim; add a re-credit-after-expiry test.
There was a problem hiding this comment.
Fixed in 8469473 — credit_wire_claim settles an already-forfeited row before accumulating onto it.
You identified the interaction precisely: claimwire refuses at now >= expires_at_sec, but claimable::credit upserts, so a later credit for the same account would add to that refused balance and refresh its stamp, and the sweep would then see a live row. The old amount becomes claimable again, and periodic small credits keep a system-funded row alive indefinitely — the exact thing the window exists to prevent.
It now detects a row expired at now, transfers its balance to the treasury and erases it, then creates the fresh claim. A credit can only ever start a new claim; it can never resurrect an old one.
On the policy, since it decides the shape: expiry exists so abandoned rows do not hold RAM forever. An account still being credited is not abandoned, so its NEW claim legitimately gets a full window — that part of the upsert behaviour is wanted, and I deliberately did not make a re-credit inherit the old (already lapsed) deadline. What it must not do is un-forfeit the balance the previous window already closed on. Reclaim-then-credit gives both.
recredit_after_expiry_does_not_revive_the_forfeited_balance covers it: credit 150, age past the window WITHOUT sweeping so the forfeited row is still sitting there, then credit 40. Only the 40 is claimable, the 150 went to the treasury rather than accumulating into a 190 balance with a fresh window, and the new claim is live so the recipient is not penalised for the later payout.
contracts_unit_test: 643 cases, *** No errors detected
| // are ever deployed (every emissions-only test fixture does exactly that). | ||
| // Nothing can have accrued a wireclaim in that state, so skipping the sweep | ||
| // is precisely correct rather than merely safe. | ||
| if (is_account(RESERV_ACCOUNT)) { |
There was a problem hiding this comment.
[P1] Keep the reclamation path reachable when this epoch is balance-blocked. advance returns on a failed emissions gate at lines 349-361, before this call. If a pay epoch is BALANCE_INSUFFICIENT while expired wireclaims hold enough forfeited WIRE to cover the shortfall, every retry exits before transferring that WIRE back to sysio, so the gate remains stuck even though the contract already controls the needed funds. sweepclaims only accepts epoch or reserv authority, so an ordinary keeper cannot break the cycle and privileged intervention is required. Run the bounded maintenance reclaim before the economic gate, expose a safely capped permissionless trigger, or otherwise let gate-blocked retries execute it; add a regression where expired claims make a blocked epoch payable.
There was a problem hiding this comment.
Fixed in 8469473 — the sweep now runs BEFORE the emissions gate.
You are right, and it was self-inflicted: I had placed it with the other maintenance sweeps (chklocks, pruneuwreqs), which all sit after the gate. That is fine for them because none of them can change the gate's verdict. This one can — what it reclaims lands in the very balance the gate measures — so putting it in the same block created a cycle where a BALANCE_INSUFFICIENT epoch returns before reclaiming the forfeited WIRE that would cover its own shortfall, and every retry repeats it. Your point about authority closes the escape hatch: sweepclaims takes only epoch or reserv authority, so no ordinary keeper could break it either.
It is maintenance, not economics, so ahead of the gate is where it belongs. Safe there for three reasons I checked rather than assumed: it is budget-bounded, it never throws past its auth check, and it touches no epoch state — so a blocked epoch that sweeps and still cannot pay is exactly as blocked as before, minus some expired rows. Nothing about the gate's own inputs is disturbed except the balance, which is the point.
I did not add the permissionless trigger you offered as an alternative. Running it before the gate makes the reclaim unconditional on every advance attempt, including the retries of a blocked epoch, so the cycle cannot form in the first place — a keeper-callable escape valve would be a second path to the same outcome. Happy to add one if you would rather have a manual lever independent of the epoch tick.
Worth noting for the record: the guard on this inline is load-bearing and was found the hard way. The first version was unguarded and failed 124 cases with inline action's code account sysio.reserv does not exist — the emissions fixtures advance epochs without ever creating that account. Moving the call earlier does not change that; it is still wrapped in is_account(RESERV_ACCOUNT).
I have NOT added the regression you asked for — "expired claims make a blocked epoch payable". It needs a fixture holding a balance-blocked epoch and expired wireclaims whose forfeited total covers exactly the shortfall, which is a more elaborate setup than the two sweep tests; the reordering itself is covered only in the sense that the full suite still passes. Say the word and I will build it.
There was a problem hiding this comment.
Yes, please add that regression. This placement is load-bearing cross-contract control flow, while the current reserv tests call sweepclaims directly and never exercise the real advance path. Since action.send() queues the inline action, the expected blocked-path behavior is: the first advance records the gate block and then executes the queued reclaim; the next chkcons retry observes the increased treasury balance and advances. A test that leaves an expired claim covering the shortfall and proves the epoch advances without a manual sweep or top-up would pin the actual guarantee and catch either moving this call back below the gate or losing the retry.
There was a problem hiding this comment.
Added in 264fe11 — expired_wire_claims_unblock_a_balance_blocked_epoch, in contracts/tests/emissions_tests.cpp.
It pins the guarantee in exactly the two-attempt shape you described:
- a reserve seeded from the treasury, one
refundwirecredit, thenproduce_block(fc::days(366))with no further credit, socredit_wire_claim's opportunistic sweep can never fire — the epoch inline is the only thing that can collect the row; - the treasury drained to exactly
period_emission - FORFEIT, i.e. short by precisely the forfeited claim; - first advance —
blocklog[1]recordsEMISSIONS_BLOCK_REASON_BALANCE_INSUFFICIENTwithattempted_emission == period_emissionandt5state.last_epoch_indexstill 0, and the queued reclaim runs anyway: the claim row is gone and the treasury is exactlyperiod_emission; - second advance — no manual
sweepclaims, no top-up: the epoch advances to 1,blocklog[1]is pruned,last_epoch_indexis 1.
I verified it is a real regression rather than assuming. Moving the inline back below the gate (where the other maintenance sweeps live) fails it at 0u == wire_claimable("lapseduser"_n) with [0 != 100000000000] — the blocked epoch returns without ever reclaiming, which is the cycle the placement exists to prevent. It would equally catch losing the retry, since the second advance is what asserts the epoch actually moves.
Reaching the epoch machinery from a reserv-aware fixture turned out not to need new scaffolding: deploy_reserv() already existed in the emissions fixture for the swap-fee fold-in test, and regreserve is legal there because the fixture is still inside the epoch-0 bootstrap window. The only addition is a wire_claimable() reader for sysio.reserv::wireclaims.
contracts_unit_test: 644 cases, *** No errors detected
Three review findings (huangminghuang, PR #558). Two are exposures this PR itself created; the third is a deadlock in the sweep it added. 1. `claimnodedis` now reserves outstanding epoch pay. Node-owner vesting draws on the SAME `sysio` WIRE balance that backs unclaimed `payclaims` rows, but spent the gross balance. Credit 60 of epoch pay against a 100 balance, let a vested owner withdraw 50, and only 50 backs the 60 owed: the later `claimpay` cannot pay out and `payepoch` stays balance-blocked from then on. The exposure is new -- before payouts became claimable, epoch pay had already left the treasury by the time this ran. It now checks against `balance - payclaimtot.outstanding`, the same reserve `fundclaim` and the epoch gate hold. Rejecting rather than capping keeps `claimed` exactly in step with what was transferred; the claim stays fully available and succeeds once claims are pulled or emissions refill. Audited the other treasury outflows: `claimpay` decrements `outstanding` in lockstep, `fundclaim` already reserves it, and payepoch's category-bucket pushes run inside the gate that reserves it. `claimnodedis` was the only unreserved one. 2. `sweepclaims` moved AHEAD of the emissions gate in `advance`. `advance` returns early when the gate reports BALANCE_INSUFFICIENT, which sat before the sweep -- so an epoch blocked for want of WIRE never reclaimed forfeited `wireclaims` that could cover the shortfall, and every retry took the same path. The contract would sit on the funds needed to unblock itself, and since `sweepclaims` takes only epoch or reserv authority no keeper could break the cycle. It is maintenance, not economics, and what it reclaims lands in the very balance the gate measures, so it belongs before the gate. Safe there: bounded, never-throwing past its auth check, and it touches no epoch state -- a blocked epoch that sweeps and still cannot pay is exactly as blocked, minus some expired rows. 3. `credit_wire_claim` settles an already-forfeited row before accumulating. `claimable::credit` upserts, so crediting an account whose row was past its window but still queued behind the bounded sweep would add to the forfeited balance and refresh its stamp -- reviving an amount `claimwire` had been refusing, and letting a trickle of small credits keep a system-funded row alive indefinitely. It now reclaims the expired balance to the treasury and erases the row first, so a credit can only ever start a FRESH claim. Expiry exists so abandoned rows do not hold RAM forever; an account still being credited is not abandoned, so its new claim legitimately gets a full window. What that must not do is un-forfeit what the last window closed on. Tests -- all three verified as real regressions by reverting each fix: * `claimnodedis_reserves_outstanding_epoch_pay` -- vested owner, real payepoch credit, treasury left one unit short of pay + claim: the withdrawal is refused and nothing is spent; funding the missing unit lets it through; the epoch pay is still fully backed afterwards and really is payable. Without the reserve the withdrawal succeeds. * `recredit_after_expiry_does_not_revive_the_forfeited_balance` -- only the new amount is claimable, the old one went to the treasury. * `expired_wire_claim_is_swept_without_further_traffic` (from the previous commit) continues to cover the sweep itself. contracts_unit_test: 643 cases, *** No errors detected Change-Id: I3a4140e0eb3ffe9aeae78a48ab6ffcf05afd6fd1
| // `sysio.reserv` is not a precondition for advancing an epoch -- a chain can advance before | ||
| // reserves are ever deployed -- and nothing can have accrued a wireclaim in that state, so | ||
| // skipping is precisely correct rather than merely defensive. | ||
| if (is_account(RESERV_ACCOUNT)) { |
There was a problem hiding this comment.
[P1] Define a safe multi-contract rollout order. This call and the new pay-claim reader create two unsafe mixed-version states. If sysio.epoch is upgraded while sysio.reserv is still old, is_account passes but the old CDT dispatcher asserts on the unknown sweepclaims action, so every advance aborts. If sysio.system is upgraded before epoch, new payepoch starts retaining WIRE in payclaims while the old gate still counts that backing as spendable, so a later pay period can double-commit it and leave claims underfunded. Deploy sysio.reserv then sysio.epoch then sysio.system, or batch/feature-gate the upgrades atomically; document and exercise that transition so release tooling cannot choose an unsafe order.
There was a problem hiding this comment.
Fixed in 264fe11 — the order is documented, and the write-up is new in this PR: docs/contract-upgrade-order.md.
Both hazards you name are real, and I verified the mechanism rather than taking it on faith: the CDT-generated dispatcher ends every contract's apply with default: if (r != "sysio"_n.value) sysio_assert_code(false, 1);, so an inline action a stale callee does not implement asserts — and inside advance that is a chain-wide epoch stall, exactly as you describe.
Two things came out of writing it up, and both change the shape of the answer.
The hazard is systemic, not specific to sweepclaims. advance inlines into six contracts — sysio.reserv (sweepclaims), sysio.uwrit (chklocks, pruneuwreqs, drainfwq), sysio.opreg (recorddel, termcheck, flushwtdw), sysio.chalg (slashop), sysio.msgch (queueout, buildenv), sysio (accrueepoch, payepoch) — plus everything those inline transitively. pruneuwreqs (SEC-129) carried the identical exposure when it landed. A guard on this one call fixes one edge of N; a deploy procedure fixes the class.
sysio is the exception in that dispatcher, and it is the worse one. The if (r != "sysio"_n.value) means an action sysio.system does not implement is silently IGNORED rather than asserted. So the sysio.epoch-new / sysio.system-old direction does not halt — it drops the effect. An accrueepoch that never runs looks exactly like a chain that is working. Table reads have no dispatcher at all, so a reader meeting a stale writer sees the field default; that direction is always silent.
The doc therefore leads with your second remedy rather than an ordering rule: ship a release's system contracts in ONE sysio.msig transaction — every setcode/setabi commits or fails together, so there is no mixed-version window to order. The staged fallback is sysio.reserv → sysio.epoch → sysio.system with your two edges as the reasons, sysio.opreg/sysio.uwrit free to land anywhere (they gain no inlined action and no other contract reads their new tables), and the general rule for future changes: a contract gaining an advance-inlined action deploys BEFORE sysio.epoch; a contract whose new state the gate must reserve deploys AFTER it. The downgrade direction is written down too, since it inherits both rules reversed.
Every intermediate state of that order is safe, and I checked the one that looked risky rather than asserting it: a new sysio.epoch under an old sysio.system reads a payclaimtot that has no row yet — that read is contract-side KV, so it never consults the stale ABI, and get_or_default yields a zero reserve, which is the correct answer while nothing is credited.
What I did not do is add a structural guard, and that was a decision rather than an oversight. The cheap version — gate the inline on sysio.reserv's wireclaims being non-empty, since only the new build writes that table — would make epoch-before-reserv safe automatically in any order. I left it out for three reasons: it closes 1 of the N edges above while the procedure closes all of them; it needs a layout-compatible mirror of wire_claim inside sysio.epoch (the same duplication rewards_bucket already carries, a drift surface with its own pinning test); and pre-release, every cluster is bootstrapped fresh from one build, so the mixed-version state is a launch-time concern rather than a live one. Say the word and I will add it.
On "so release tooling cannot choose an unsafe order": there is no in-tree upgrade tool to constrain — the harness bootstraps fresh clusters and deploys the whole set in one pass — so the procedure is the artifact. And a genuine mixed-version test would need a stale wasm vendored into the tree, an artifact nobody can rebuild from source, which is exactly what this repo does not commit. The nearest in-tree evidence is the guard already there and the 124 cases that failed the first time the inline went out unguarded.
| void claimnodedis(const sysio::name& account_name); | ||
|
|
||
| /** | ||
| * Claim epoch pay credited by payepoch (producer, standby, batch-operator or category |
There was a problem hiding this comment.
[P3] Update the public claim semantics after restoring bucket pushes. claimpay now covers producers, standbys, and batch operators only; sysio.ops and sysio.gov are transferred directly. The PR description repeats this stale category-claim behavior in both the action table and UX section, and it still describes the sweep as credit-path-only despite the new sweepclaims action. Please update these API docs and the full PR description to match the current head so downstream SDK/UI work does not implement the superseded flow.
There was a problem hiding this comment.
Fixed in 264fe11, and the PR description is rewritten against the current head.
sysio.system.hpp's claimpay doc no longer advertises a category share, and it now states what actually happens to sysio.ops / sysio.gov and why no claim path can ever reach them — sysio.roa::is_sysio_account forces net_weight/cpu_weight to zero for every sysio-prefixed account, and neither carries a contract that could emit the claim inline the way sysio.dclaim does. It points at the standing on_notify constraint recorded at the push site in emissions.cpp rather than restating it.
The description changes, since "the full PR description" covers a fair amount of surface:
- the
claimpayrow drops the category-bucket share, and the "all three are permissionless" framing now has a second table forsweepclaims— it is maintenance rather than a payout, and takessysio.epoch-or-self authority, so listing it with the claims would have been wrong in two ways; - the UX bullet on the buckets is inverted: they are still paid directly, with the reason and the standing constraint on any contract governance ever deploys there;
- the expiry section is rewritten around the four things that now enforce the window — the epoch-inlined
sweepclaimsand why it sits before the gate, the opportunistic credit-path sweep, the claim-time refusal, and settle-before-recredit — instead of describing it as credit-path-only; payclaims/remitclaimsmove from "Expires: no" to "stamped, not enforced (WIRE-339)", which is what the schema actually says now;- treasury accounting gains
claimnodedisand the audit of the othersysio.systemoutflows; - a new "Rollout order" section for #3784561690, pointing at
docs/contract-upgrade-order.md; - the testing section lists the retention and reserve tests by name, and the merge-order note records that Add some addtional webauthn tests #66 has been regenerated from this branch's ABI.
The vulnerability table also drops "category accounts" from the payepoch row, since after the fix those are the recipients that are deliberately still pushed.
| /// runs inline inside `advance`, where an abort stalls epoch progress | ||
| /// chain-wide. An oversized backlog drains across later epochs. | ||
| [[sysio::action]] | ||
| void sweepclaims(uint32_t max_rows); |
There was a problem hiding this comment.
[P3] Regenerate the advertised SDK companion from this ABI. The current wire-libraries-ts#66 head (ea302db) predates this action: it contains claimwire but has neither a sweepclaims action type nor a sysio.reserv action-list entry. Regenerate that PR from the current 8469473 ABI before following the documented merge sequence, otherwise the companion described in this PR is already stale.
There was a problem hiding this comment.
Done — wire-libraries-ts#66 is regenerated from this branch's current ABI and pushed (1129fb0).
The diff is exactly the gap you found: SysioReservSweepclaimsAction, plus sweepclaims in the reserv action list in SysioContractDefinitions. The expires_at_sec fields were already there from the earlier regen, so nothing else in the file moved.
I checked that "nothing else moved" rather than assuming it: before generating, I diffed all ten sysio.*.abi in the source tree against a fresh build of this branch and they are byte-identical, so the committed ABIs the generator consumed are the current ones. (The .wasm artifacts differ from the committed copies by 90–350 bytes on this host, including for contracts this branch never touches, so that is local toolchain drift rather than anything from these commits — I left every artifact alone rather than shipping a binary whose provenance is my build environment.)
The branch also picked up origin/master, which it was 8 commits behind, and pnpm build is green with the reserv action list confirmed in both the cjs and esm lib/ output rather than only in source.
…gate sweep Three review follow-ups, no behaviour change. `docs/contract-upgrade-order.md` (new) records why a release's system contracts are only correct as a set. `advance` inlines into six contracts and the emissions gate reads two more, and the CDT dispatcher asserts on an action a stale callee does not implement -- inside `advance` that is a chain-wide epoch stall. `sysio` is the exception and the worse one: an action `sysio.system` does not implement is silently ignored rather than asserted, so a skew there drops the effect instead of halting. The doc leads with the one-transaction msig deploy (no mixed-version window at all), then the staged fallback reserv -> epoch -> system with the reason for each edge, the general rule for future changes, and the downgrade direction. The sweepclaims inline site points at it. `expired_wire_claims_unblock_a_balance_blocked_epoch` pins the placement of that inline ABOVE the emissions gate. It ages a wireclaim past its window with no further credit (so the opportunistic sweep cannot fire), leaves the treasury short by exactly the forfeited amount, and asserts the two-attempt guarantee: the first advance records BALANCE_INSUFFICIENT and still executes the queued reclaim, and the next retry advances with no manual sweepclaims and no top-up. Verified as a real regression -- moving the call below the gate fails it on the un-reclaimed claim. The fixture gains a `wire_claimable()` reader. `claimpay`'s header doc no longer advertises a category share: the T5 buckets are pushed, because ROA zeroes net/cpu for every sysio-prefixed account and neither carries a contract that could emit the claim inline, so no claim path can reach them. contracts_unit_test: 644 cases, *** No errors detected Change-Id: I3cce89f67e82b4de7080aabb31e5b7a3007a3249
| that never runs looks exactly like a chain that is working. | ||
|
|
||
| Table reads carry no dispatcher at all, so a reader that meets a stale writer | ||
| sees the field's default rather than an error. That direction is always silent. |
There was a problem hiding this comment.
[P2] Do not assume missing fields deserialize to defaults. get_or_default returns its fallback only when the entire KV key is absent. If a stale writer has an existing row encoded with an older, shorter schema, the new reader still attempts to decode that row; the streaming path underflows, while the fixed-serializable kv::global path copies sizeof(T) from a short read rather than filling the tail from the default object. The current payclaimtot transition is safe because the whole new singleton is absent, not because an added field would default. Please scope this statement to that case and require an explicit compatible layout or migration for additions to existing rows.
There was a problem hiding this comment.
Fixed in 8aaa053 — the statement was too broad and is now split by case.
You are right about the mechanism: get_or_default returns its fallback only when the whole KV key is absent. If a row EXISTS under an older, shorter encoding, the new reader still decodes those bytes — the streaming path underflows, and the fixed-serializable kv::global path copies sizeof(T) from a short read rather than filling the tail from the default object. "Reader meets stale writer → sees the default" was describing one case as if it were the rule.
The doc now says the failure mode depends on whether the ROW exists, not whether the FIELD does:
- whole KV key absent →
get_or_defaultyields the default. Quietly correct, and it is the case thepayclaimtottransition actually lands in — the new singleton does not exist yet under an oldsysio.system. - row exists in an older, shorter encoding → the decode runs against those bytes: streaming underflows,
kv::globalcopiessizeof(T)from a short read. A field ADDED to an EXISTING row therefore does not default — it needs a layout-compatible encoding or an explicit migration, decided per change.
The staged-rollout section's safety argument was relying on the loose version too, so I tightened it to say what it actually depends on: payclaimtot's key does not exist yet, which is the absent-key case rather than the short-decode one, with a pointer back to the split.
| callee. | ||
|
|
||
| **The rule: deploy a release's system contracts in ONE transaction.** A single | ||
| `sysio.msig` proposal carrying every `setcode`/`setabi` action commits or fails |
There was a problem hiding this comment.
[P2] Use an executable system-contract deployment recipe. The proposed every setcode/setabi transaction cannot be submitted under the repository defaults in two independent ways. Non-sysio system accounts must go through sysio.roa::setsyscode / setsysabi so giftram reconciles their finite RAM quota; raw native actions bypass that path, and this PR grows sysio.reserv.wasm by 6,040 bytes (and sysio.opreg.wasm by 2,447) while a correctly provisioned account retains only the 1,144-byte creation allowance, so the raw deployment fails RAM validation. Separately, the five changed WASMs total 579,408 bytes, already above the default 524,288-byte max_transaction_net_usage before any ABI or action wrapping; msig::propose receives the complete inner transaction before it chunks storage, so chunking does not bypass input NET. Please specify the ROA deployment actions and either atomically package only the compatibility-coupled trio after preflighting its packed size, stage the uncoupled contracts, or document a tested limits-change path.
There was a problem hiding this comment.
Fixed in 8aaa053 — the doc now carries an executable recipe instead of "one transaction", and both blockers you name are real. I re-derived every number rather than taking them:
- 5 changed WASMs = 579,408 B (epoch 75,579 + opreg 88,451 + reserv 84,026 + system 174,138 + uwrit 157,214) against
default_max_transaction_net_usage= 524,288 (config::default_max_block_net_usage / 2,libraries/chain/include/sysio/chain/config.hpp:56-58). Over before ABIs or wrapping. - reserv +6,040 B, opreg +2,447 B — matches your figures exactly.
sysio.roa::setsyscode/setsysabiare documented incontracts/sysio.roa/sysio.roa.hpp:210-232as setting code/abi and gifting the exact RAM out of sysio's pool viagiftram, measured after, re-callable so a smaller re-deploy reclaims. A rawsetcodeskips exactly that reconciliation.
So the headline claim was wrong in two independent ways, and the fix is not a caveat — it changes what the atomic unit IS. The doc now says: deploy each compatibility-coupled SET in one transaction, and the coupled set is usually smaller than the release.
For this release the coupled set is the trio the edges actually couple:
| code | abi | total | |
|---|---|---|---|
sysio.epoch |
75,579 | 6,758 | 82,337 |
sysio.system |
174,138 | 61,916 | 236,054 |
sysio.reserv |
84,026 | 24,647 | 108,673 |
| trio | 333,743 | 93,321 | 427,064 |
~97 KB of headroom — enough, but the doc says preflight the packed size of the actual proposal rather than trusting that table, since it is a snapshot of one release and raw bytes are not packed NET. sysio.opreg and sysio.uwrit are uncoupled and stage in their own transaction, which is what keeps the trio under the ceiling.
For the future case where a coupled set does not fit, the doc names your two options and rules out the third: split off whatever is genuinely uncoupled, or raise max_transaction_net_usage via setparams first, as a tested path — proposed and confirmed before the deploy proposal, never assumed to work on the day.
Your note that msig::propose receives the complete inner transaction before it chunks storage is in there too, because "propose it through msig" is the obvious wrong inference from the old text.
| A downgrade is an upgrade with the versions swapped, so it inherits both rules in | ||
| reverse: roll back `sysio.epoch` **before** `sysio.reserv`, or the surviving new | ||
| `advance` will inline `sweepclaims` into a contract that no longer implements it. | ||
| Rolling back `sysio.system` while `payclaims` rows exist strands them — the rows |
There was a problem hiding this comment.
[P1] Preserve every outstanding liability before downgrading. The old sysio.reserv also removes claimwire while existing wireclaims still represent WIRE already withheld from recipients, and the old sysio.opreg removes claimremit while remitclaims still represent debited operator collateral; both sets of funds become unreachable just like payclaims. In addition, rolling epoch back while the new system remains live lets payepoch keep creating claims that the old gate does not reserve. A safe procedure must first quiesce credit writers and drain or migrate all three claim tables, then use the coupled code order sysio.system -> sysio.epoch -> sysio.reserv and only roll opreg back once remits are handled; without such a migration, a live chain with uncooperative claimants is not safely downgradeable.
There was a problem hiding this comment.
Fixed in 8aaa053 — the section was rewritten around your point, which reframes it: a downgrade is a data-migration problem first, and a code-ordering problem second. The old paragraph treated it as the upgrade run backwards and named only payclaims, which is the least of the three.
All three claim tables are now stated as what they are — value already taken from someone's spendable position and parked behind an action the old build does not have:
| Table | Contract | Paid out by | Stranded when that contract rolls back |
|---|---|---|---|
payclaims |
sysio.system |
claimpay |
earned epoch pay |
wireclaims |
sysio.reserv |
claimwire |
swap payouts + refunds already withheld from recipients |
remitclaims |
sysio.opreg |
claimremit |
debited operator collateral |
The live-writer hazard you identified is called out separately, because it has no upgrade counterpart: rolling sysio.epoch back while the new sysio.system stays deployed lets payepoch keep CREATING claims the reverted gate does not reserve, so the treasury resumes double-committing while the unreachable pile grows.
The procedure is yours: quiesce the credit writers → drain or migrate all three claim tables → roll back the coupled trio in the order sysio.system → sysio.epoch → sysio.reserv → roll sysio.opreg back only once its remits are handled. I noted explicitly that step 2 is the one that actually gates the rollback and cannot be completed unilaterally, since a claimant who never claims holds it open.
And the conclusion is stated rather than implied: a live chain with uncooperative claimants is not safely downgradeable by code order alone. If a rollback has to happen anyway, the outstanding balances are a liability to settle deliberately, not something the deploy sequence absorbs.
| know to reserve, and the gate authorizes what the treasury cannot cover. | ||
|
|
||
| Both rules say the same thing from opposite ends: the *reader/caller* must never | ||
| be older than the *writer/callee* it depends on. |
There was a problem hiding this comment.
[P2] Keep the two version constraints in their actual directions. Rule 1 requires callee_version >= caller_version: the new callee must land before the new caller. Rule 2 requires reader_version >= writer_version: the new reader must land before the new writer. The combined sentence says the caller must never be older than its callee, which is the opposite of Rule 1 and would guide a future rollout toward the epoch-new/callee-old state that aborts every advance. State the call edge and table edge separately rather than presenting them as one inequality.
There was a problem hiding this comment.
Fixed in 8aaa053. You are right, and the sentence was mine — I collapsed two inequalities that point in opposite directions into one line for symmetry, and the half that broke is the half that matters: it would steer a rollout straight into the epoch-new / callee-old state that aborts every advance.
The summary is gone. The two edges are now stated separately, with their directions:
- Call edge —
callee_version >= caller_version. The contract that RECEIVES an inlined action upgrades first, because the new caller emits an action the old callee cannot dispatch. - Table edge —
reader_version >= writer_version. The contract that READS the new state upgrades first, because the new writer commits state the old reader does not know to account for.
And the doc now says explicitly that they cannot be collapsed into one inequality, plus what following the inverted form produces — so the next person to reach for a tidy one-liner has the reason not to.
| // A swap-from-WIRE refund credits a claimable balance that nobody ever pulls. | ||
| constexpr uint64_t FORFEIT = 100'000'000'000ULL; | ||
| create_user_accounts({ "lapseduser"_n }); | ||
| BOOST_REQUIRE_EQUAL( success(), push_reserv_action(UWRIT, "refundwire"_n, mvo() |
There was a problem hiding this comment.
[P3] Back the simulated refund with separate escrow. regreserve transfers RESERVE_SEED into custody and books that entire amount into reserve_wire_amount; the direct refundwire call does not debit the reserve row because production reaches it only after swapfromwire has deposited additional in-flight escrow. Here no such deposit occurs, so sweeping FORFEIT leaves the token balance at RESERVE_SEED - FORFEIT while the reserve row still reports RESERVE_SEED: the test unblocks emissions by consuming registered reserve liquidity, not by returning forfeited escrow. Transfer a separate FORFEIT into reserv (or drive the real swap-from-WIRE path) before crediting the claim, and assert the post-sweep custody invariant.
There was a problem hiding this comment.
Fixed in 8aaa053. You are right, and the flaw was load-bearing on what the test claimed to prove: regreserve books RESERVE_SEED into reserve_wire_amount as well as moving it into custody, and refundwire credits without debiting that row because production reaches it only after swapfromwire has deposited the user's in-flight escrow on top. With no such deposit, the sweep handed the treasury registered reserve liquidity — so the epoch unblocked itself by breaking reserv's custody invariant, not by reclaiming forfeited escrow. Green for the wrong reason.
The test now funds the escrow separately before crediting: a plain FORFEIT transfer from the treasury into sysio.reserv, modelling the in-flight deposit, asserted at RESERVE_SEED + FORFEIT before the refund. Custody then reads reserve_wire_amount (RESERVE_SEED) + Σ wireclaims (FORFEIT), which is the invariant's own decomposition.
And the post-sweep custody assertion you asked for is in: after the blocked advance reclaims the row, wire_balance(sysio.reserv) == RESERVE_SEED exactly — the escrow left, the booked liquidity did not. That is the assertion that makes the test mean what its name says.
The shortfall arithmetic is unchanged (the drain targets period_emission - FORFEIT off the live balance, so it self-adjusts), and I re-ran the negative control against the REVISED test rather than assuming it carried over: with the inline moved back below the gate it still fails at 0u == wire_claimable("lapseduser") with [0 != 100000000000].
contracts_unit_test: 644 cases, *** No errors detected
Review round on the upgrade-order doc and the reclaim test. Still no behaviour change. The doc's headline was not executable. Raw `setcode`/`setabi` bypasses `sysio.roa::setsyscode`/`setsysabi`, which gift the exact RAM out of sysio's pool via `giftram` -- and this release grows sysio.reserv by 6,040 bytes and sysio.opreg by 2,447 against a system account's small creation allowance. The whole release also does not fit: five changed WASMs total 579,408 bytes against a 524,288-byte max_transaction_net_usage, and `msig::propose` takes the complete inner transaction before it chunks storage. So the atomic unit is the compatibility-coupled TRIO (epoch + system + reserv, 427,064 bytes with ABIs), uncoupled contracts stage separately, and the doc says to preflight the packed size rather than trust the table. Two statements in it were wrong. The summary sentence inverted the call edge -- it read "caller must never be older than the callee", which is backwards and would steer a rollout into the epoch-new/callee-old state that aborts every advance; the call edge (callee >= caller) and table edge (reader >= writer) are now stated separately with why they cannot be merged. And "a stale read sees the default" only holds when the whole KV key is absent: an existing row in a shorter encoding underflows the streaming path, or copies sizeof(T) from a short read on `kv::global`. Downgrades were reframed as a data-migration problem. All three claim tables hold value only the new code can pay out -- `wireclaims` and `remitclaims` strand exactly like `payclaims` -- and rolling epoch back under a live new system lets payepoch keep creating claims the reverted gate does not reserve. The procedure is quiesce, drain or migrate, then system -> epoch -> reserv, with opreg last. `expired_wire_claims_unblock_a_balance_blocked_epoch` was green for the wrong reason: `regreserve` books its seed into `reserve_wire_amount`, and `refundwire` does not debit that row, so the sweep was handing the treasury registered reserve liquidity rather than forfeited escrow. It now funds the in-flight escrow separately before crediting and asserts custody returns to exactly the booked amount after the sweep. contracts_unit_test: 644 cases, *** No errors detected Change-Id: I95d8d3fcaba298625a4266de979710501e45dcb4
| Two repository defaults make "just `setcode` everything in one transaction" | ||
| fail, and both bite this release specifically. | ||
|
|
||
| **1. System contracts deploy through `sysio.roa`, not through raw `setcode`.** |
There was a problem hiding this comment.
[P1] Keep the root sysio deployment native. This rule still cannot be applied to the full trio: sysio.system is code on the root sysio account, which is the RAM pool itself, while setsyscode / setsysabi are for separate finite-quota system accounts. Self-targeting giftram moves the positive delta from the sysio reslimit row into sysio.acct, then set_resource_limits(sysio, sram - delta) and add_system_resources(sysio, delta) operate on the same account and restore its chain quota, leaving the ROA ledger lower than the actual quota. The production bootstrap therefore explicitly deploys system on sysio with raw native actions because giftram cannot self-target. Build the same atomic msig transaction with native setcode / setabi for root sysio, and ROA setsyscode / setsysabi only for sysio.epoch and sysio.reserv (and the other separate-account contracts).
| Upgrading them one at a time creates windows in which a new caller meets an old | ||
| callee. | ||
|
|
||
| **The rule: deploy each compatibility-coupled SET in ONE transaction.** A single |
There was a problem hiding this comment.
[P3] Bring the PR rollout summary forward with this rewrite. The PR description still tells operators to put the entire release into one sysio.msig transaction carrying every raw setcode / setabi, which is the exact recipe this commit now explains exceeds default NET and bypasses RAM reconciliation for separate system accounts. Update the merge-record description to say the compatibility-coupled trio is atomic, opreg/uwrit stage separately, and the trio mixes native actions for root sysio with ROA actions for the separate accounts once the deployment exception above is fixed.
Closes SEC-150.
The vulnerability
sysio.token::transfercallsrequire_recipienton both sides, andapply_context::execdispatches notified receivers in a bare loop with no exception isolation. An assert inside a recipient'son_notify("sysio.token::transfer")handler therefore aborts the entire transaction, including every parent inline action. Any account cansetcodesuch a handler —contracts/test_contracts/reject_all.wasmhas done exactly this in-tree for years. A handler that burns CPU instead of asserting is the same vector, so the handler must not run at all rather than be tolerated.That made every pushed payout on a never-throw path abortable by its own recipient. The contracts pre-validate every reachable
check()sosysio.epoch::advanceand the OPP inbound dispatch chain cannot abort, but that discipline stops at the contract's own guards: once value is pushed to an account the protocol does not control, the counterparty decides whether the transaction commits.Four sites were reachable this way. Three sit in the
sysio.epoch::advanceinline subtree, where an abort halts epoch advancement, emissions accrual and outbound envelope construction chain-wide:uwrit::drainfwq->reserv::refundwiresysio.system::payepochsysio.opregWIRE-chain remitssysio.reserv::paywireThe refund case is the worst of them and the cheapest to mount. The drain's queue-row erase rolls back with the abort, so the offending row survives to re-block every later epoch — an indefinite halt bought for one
min_fromwire_amountescrow, paid once. The termination case never converges either: the operator blocks its own remit, so the termination never commits and each later advance retries it.The fix
All four sites now credit a claimable balance and transfer nothing. The recipient pulls it with an action carrying its own authority, so a hostile handler blocks only its own payout. This generalizes the pattern
sysio.dclaimalready used (creditpending_claimson the never-throw path, pay out from a separate user-initiatedclaim) into a sharedsysio.opp.common/claimable.hpp.The one deliberate exception is a payout to an account the protocol itself controls:
payepoch's T5 category buckets andfundclaim's transfer tosysio.dclaimare still pushed. See UX changes.New actions
The three claims are permissionless and self-authorized: the caller may only claim its own balance.
sysio.system::claimpay(account_name)payepochsysio.reserv::claimwire(account)paywire,refundwiresysio.opreg::claimremit(account)Each drains the caller's row in full — there is no partial claim — and throws
no epoch pay to claim/no claimable WIRE for this account/no claimable remit for this accountwhen nothing is owed.claimwireadditionally refuses a row past its retention deadline.A fourth action is maintenance rather than a payout, and is not permissionless:
sysio.reserv::sweepclaims(max_rows)sysio.epochor selfmax_rowsexpiredwireclaimsrows and returns their WIRE to the treasurysysio.epoch::advanceevery epochNew tables
payclaimssysio.systempayclaimtotsysio.systemwireclaimssysio.reservremitclaimssysio.opregpayclaims/wireclaims/remitclaimsare readable to show a "you have X to claim" balance before the user sends anything.UX changes
Anything that previously waited for WIRE to arrive in an account after one of these events must now claim it. Balances no longer move on their own.
sysio.reservcustody until the recipient callsclaimwire. Wallets and bridge UIs need a "claim" step, or a relayer that claims on the user's behalf.claimwire.payepochno longer moves WIRE into their accounts. Any monitoring that reads producer balances as a proxy for "was I paid" must readpayclaimsinstead, or claim first.sysio.opsandsysio.gov. They are pushed rather than credited because no claim path can ever reach them: a claim needsrequire_auth(account_name),sysio.roa::is_sysio_accountforcesnet_weight/cpu_weightto zero for everysysio-prefixed account so neither can pay for a transaction, and unlikesysio.dclaimneither carries a contract that could emit the claim inline. Crediting them would convert an earned allocation into a permanently growingpayclaimtotreservation nobody can move. They are protocol-owned holding accounts with no code, so the notify-handler threat the pull model defends against does not exist for them — the same exceptionfundclaimalready makes forsysio.dclaim. The constraint this puts on the accounts is written at the push site: any contract governance ever deploys onsysio.opsorsysio.govMUST NOT assert or burn CPU in anon_notify("sysio.token::transfer")handler.remitclaims. This is independent ofprune— a pruned operator row does not strand the collateral.wireclaimsrows expire after one year and are swept back to the treasury, on an epoch-driven trigger rather than only on settlement traffic.payclaimsandremitclaimsrecord a stamp but nothing expires them today.Nothing changes for outpost-chain remits, which still flow through
WITHDRAW_REMITattestations, or forsysio.dclaimstaking rewards, which were already claim-based.Implementation details worth review
sysio::assetmagnitude limit rather than the integer limit, so a credited balance is always payable. Saturating atUINT64_MAXwould merely move the abort from credit time to claim time and strand the balance permanently.opreg::depositalready applies.Treasury accounting
Unclaimed pay still sits in the treasury's token balance while being fully owed, so every gate that spends against that balance reserves it:
sysio.system::fundclaim's balance cap,sysio.epoch's emissions readiness gate, andsysio.system::claimnodedis, which now refuses a node-owner withdrawal that would eat intopayclaimtot.outstanding. Without this the treasury double-commits and a later claim fails on overdraw, stranding earned pay. The outstanding total is a maintained counter (payclaimtot) rather than a table scan, because the epoch gate reads it on every advance.claimnodedisrejects rather than caps: capping would either transfer less thaninfo.claimablewhileclaimedadvanced by the full amount, silently destroying the difference, or thread a partial amount through the vesting accounting. It is user-initiated, so the throw reaches the caller who asked. Every other treasury outflow insysio.systemwas audited in the same pass —claimpaydecrementsoutstandingin lockstep,fundclaimalready subtracted the reserve, andpayepoch's category pushes run inside the gate that reserves it.sysio.epoch's blocklog now reportssysio_balanceas the spendable amount for the same reason — an operator readingBALANCE_INSUFFICIENTneeds the shortfall against what can really be paid.Expiry
sysio.reserv's claimable set is unbounded and caller-influenced, and the rows bill to the sysio RAM pool, so they carry a one-year window. The producer and operator sets are bounded per-epoch and hold earned pay and returned collateral, so they are stamped but not swept. The window is a documented constant rather than config, to keep the swap-settlement surface unchanged; it is a RAM backstop, not an economic lever.Four pieces enforce the
wireclaimswindow, and the sweep itself is one implementation shared by both triggers so they cannot drift on what "expired" means or where the reclaimed WIRE goes:sweepclaims, inlined bysysio.epoch::advancewithMAX_CLAIM_SWEEP_PER_EPOCH. This is what makes the deadline hold when settlement stops. It is placed before the emissions gate, deliberately: what it reclaims lands in the very balance the gate measures, so ordering it after the gate lets aBALANCE_INSUFFICIENTepoch return without reclaiming the forfeited WIRE that would cover its own shortfall — on every retry, withsweepclaimstaking only epoch/reserv authority so no ordinary keeper could break the cycle. It is budget-bounded, never throws past its auth check, touches no epoch state, and is guarded onis_account(sysio.reserv)(an inline to an absent code account is a hardaction_validate_exception, which insideadvanceis a chain-wide stall).credit_wire_claim's opportunistic sweep on the write path — bounded and best-effort; it only fires while credits keep arriving.claimwirerefuses an expired row. The sweep is bounded, so a row can outlive its expiry for many epochs while it waits its turn; without this the window would mean "swept eventually" rather than "claimable until", and a balance the policy had already extinguished would still pay out in the meantime.paywire/refundwirewould add to a refused balance and refresh its stamp, and a trickle of small credits could keep a system-funded row alive indefinitely.payclaimsandremitclaimscarryexpires_at_secplus abyexpiryindex maintained on every credit, and nothing reads them yet. The schema is free to add before launch and expensive after, and landing it now means a later retention pass inherits real age data instead of starting its clock the day the field is added — refreshing on every credit makes the stamp a "last had activity" signal, so an account still being paid never looks stale. Wiring a sweep there means forfeiting earned pay and returned collateral, which is a policy decision rather than a RAM one, and the in-tree precedents disagree (sysio.dclaim::pending_claimsforfeits to the capital fund after 180d;wireclaimsuses a year to the treasury). That decision is tracked in WIRE-339.Rollout order
These contracts are not independent deployables:
advanceinlines actions into six of them, and the emissions gate reads two more contracts' tables. Deploy a release's system contracts in ONEsysio.msigtransaction — everysetcode/setabicommits or fails together, so no mixed-version window exists.If a staged rollout is forced, the order for this release is
sysio.reserv→sysio.epoch→sysio.system(sysio.opregandsysio.uwritmay land anywhere — they gain no inlined action and no other contract reads their new tables):sysio.reservbeforesysio.epoch— the newadvanceinlinessysio.reserv::sweepclaims, guarded only on the account existing. An oldsysio.reservbuild has the account and not the action, and the CDT dispatcher asserts on an action it does not implement, so every advance would abort — a chain-wide epoch stall.sysio.epochbeforesysio.system— the newpayepochretains WIRE inpayclaims; the old gate counts that backing as spendable and could double-commit it, leaving credited claims underfunded.Every intermediate state of that order is safe, and the general rule (a contract gaining an
advance-inlined action deploys beforesysio.epoch; a contract whose new state the gate must reserve deploys after it), the full edge list, and the downgrade direction are written up indocs/contract-upgrade-order.md, new in this PR.Testing
A new
contracts/test_contracts/block_transfercontract asserts on every incoming transfer, and three regression tests park it on each attack surface — a producer, a terminating operator, and a swap-from-WIRE refund recipient — asserting the system path completes, the blocker is still credited in full, and the only thing it can break is its own claim.Retention and reserve coverage added on top of that:
expired_wire_claim_is_swept_without_further_traffic— ages a claim past the window with no further credits, so the opportunistic sweep never fires: the claim is refused, a zero budget is a no-op, and the sweep reclaims the row to the treasury.sweepclaims_leaves_live_rows_alone— a live row survives the sweep and still claims normally.recredit_after_expiry_does_not_revive_the_forfeited_balance— a later credit starts a fresh claim; the forfeited balance went to the treasury rather than accumulating with a new window.claimnodedis_reserves_outstanding_epoch_pay— a treasury one unit short of pay + claim refuses the node-owner withdrawal, spends nothing, and lets it through once funded; the epoch pay is still really payable afterwards.expired_wire_claims_unblock_a_balance_blocked_epoch— aBALANCE_INSUFFICIENTepoch whose shortfall is exactly covered by an expired claim advances on the next attempt with no manualsweepclaimsand no top-up. This pins the pre-gate placement of the inline and the queued-inline semantics: the first advance records the block and performs the reclaim, and the next retry sees the larger balance. Verified as a real regression — moving the call back below the gate fails the test.Existing tests that asserted recipient balances after a payout now claim first and assert the same end state, so they cover the credit and the pull together. Treasury conservation becomes
balance decrease + outstanding claims == distributed, which collapses to the original assertion once every recipient has claimed.Follow-ups required before this is usable end to end
This changes payout semantics, so two repos need companion changes and the swap-to-WIRE e2e flow fails until they land:
flow-swap-to-wirepolls the recipient's WIRE balance until a deadline, which can no longer become true without aclaimwire. It now waits on the claimable balance and claims.WireClientgainsclaimWire/claimPayand their claimable-balance readers.sdk-core/src/types/SysioContractTypes.tsregenerated for the new actions and tables, or SDK consumers cannot call them.The other flows were checked and need no change:
flow-swap-from-wirenever exercises a revert, sorefundwireis not on its path;flow-reserve-lifecycle's WIRE assertions cover thematchreserveescrow, which still pushes from the user; andflow-emissions-soakcoverssysio.dclaimstaker claims, which were already pull-based.The Ethereum and Solana outposts are unaffected: the change is depot-side, and outpost remits still flow through
WITHDRAW_REMITattestations.paywireis the P1 site rather than one of the P0 halts, and it is the one carrying the user-facing swap change described above. It is the same vulnerability — a hostile swap recipient really can stall consensus dispatch — so it is fixed the same way here.Cross-repo companions and merge order
SysioContractTypesfor the new actions and tablesflow-swap-to-wireclaims the credited payout instead of polling a token balanceMerge this PR first — it is self-contained and the others consume its ABI. #66 has been regenerated from this branch's current ABI, so it carries
sweepclaimsand theexpires_at_sec/byexpiryadditions alongside the three claim actions. #66 and #62 are independent of each other and can land in either order once this is in: #62 reads the new tables raw rather than through the typed accessor, so it does not wait on an@wireio/sdk-corerelease of #66. Between this merging and #62 landing,flow-swap-to-wirefails against master. #159 is stacked on wire-hub-webapp#144 and unblocks once this and #66 are in.