Doc: Contracts: document audited designs and pin three of them in tests by heifner · Pull Request #651 · Wire-Network/wire-sysio

heifner · 2026-09-28T16:38:19Z

Documentation and tests for four CertiK findings whose code needs no change.

Unlocked ROA grants. A time_block at or below the current block issues an unlocked grant; addpolicy, reducepolicy and the ROA overview now say so.
network_gen. Only generation 0 is accepted, because only the one-shot activateroa writes it. The three policy actions document that, the policies table notes the funding generation a future rotation must record, and a new test pins the rejection of generation 1.
Finalizer key re-registration. A new test registers a deleted key again and checks it receives a new id.
Held producer pay (the code fix was feat(sysio.system): derive producer rank from a score, and demote on missed rounds (WIRE-367) #599). A terminated producer keeps its held blocks and is paid them once it cancels any withdrawal still queued from before termination, registers and bonds again; a slashed producer's are forfeited. The unpaid_blocks comment and the producer guide now say so, and two new tests run that path, one with a stale withdrawal.

Notes

CertiK WNS-37 / WIRE-395, WNS-39 / WIRE-397, WNS-46 / WIRE-380, WNS-49 / WIRE-399.
WNS-59 / WIRE-409 is no longer covered here: [WNS-59] Chalg: refuse dispute ballots once the tally has a winner #690 fixed it and documents the votedispute ballot rule.
Comments, docs and tests only: every contract wasm and abi rebuilds byte-identical to the committed files.

addpolicy stores time_block unchecked, so a value at or below the current block issues an unlocked grant that reducepolicy can take back at once. activateroa, node-owner registration and bootstrap tooling rely on that. Say so on addpolicy and in the ROA overview, which described every grant as time-committed, and correct reducepolicy's brief, which said it was callable only after time_block when the check admits the call at time_block too. network_gen is written only by the one-shot activateroa, so the policy actions accept only generation 0. Document that on addpolicy, expandpolicy and reducepolicy, and note on the policies table that a row does not record its funding generation, which a future rotation must add. A new test has all three actions reject generation 1 while the same calls at generation 0 go through. A new finalizer-key test deletes a producer's only key and registers it again: the key comes back under a new id, since key ids are never reused. A terminated operator can settle, re-register and collect its held blocks, so the unpaid_blocks comment no longer says otherwise, and the producer guide now says so and states that a slashed producer's held blocks are forfeited. votedispute now notes that ballots stay open past the deadline on purpose: the deadline only relaxes the tally, and an envelope dispute cannot lapse the way an underwriter challenge does. The header changes are comments only; no contract wasm or abi changes.

huangminghuang · 2026-09-29T17:19:14Z

 re-registering before your next round costs you nothing provided you still meet the admission
 requirements; if governance raised a collateral minimum while you were parked, top up first.

+A terminated producer that settles its collateral and re-registers is paid them too. Slashing is the


Termination settlement and regoperator re-registration are not sufficient to make this producer payable. terminate_inline drains the old balances, and non-bootstrapped regoperator creates a fresh UNKNOWN operator row with no collateral; payepoch only pays once is_schedulable sees an ACTIVE producer operator. Please say that the producer must re-post the required bonds (and reactivate its producer row if it was parked) before the held blocks can be paid. An end-to-end terminate → settle → re-register → rebond → payout test would also pin this recovery path.

Agreed — fixed in 1245b25: the guide now says termination credits the bond to the claimremit claim and regoperator starts a new UNKNOWN registration with no bond, so the producer must post every required bond again, and call regproducer if its row was parked, before the held blocks are paid. terminated_producer_is_paid_held_blocks_once_bonded_again pins the path: terminate, claimremit, regoperator, two payouts that pay nothing and keep the count, then deposit and a payout that pays every held block.

…and-tests votedispute takes master's comment: #690 settled WNS-59 and documents the ballot rule, so this branch's note is dropped. The unpaid_blocks comment keeps master's wording without "or terminated".

…ks are paid Termination credits the bond to the operator's claim, and regoperator starts a new UNKNOWN registration with no bond, so re-registering alone does not make a producer payable. The producer guide now says to post every required bond again, and to call regproducer if the row was parked; unpaid_blocks lists termination among what the count survives. A new test terminates a bonded producer, pulls the returned bond and registers again: the payouts before the new bond pay nothing and keep the count, and the payout after it pays every held block. Master is merged in. Change-Id: I98b4bf7e8764ff3745694ba3695b55e2fad864c2

huangminghuang · 2026-10-23T18:25:33Z


+Termination does not forfeit them either, but re-registering alone does not make you payable:
+termination credits your whole bond to your `claimremit` claim, and `regoperator` starts a new
+`UNKNOWN` registration with no bond. Post every required bond again, and call `regproducer` if your


[P2] Clear stale withdrawals before promising payout after rebond. Termination returns and zeroes the bond but deliberately leaves queued withdrawals. Those rows are account-scoped across regoperator, so available_inline still subtracts an old request from the new registration; posting only the required bond can therefore leave the producer UNKNOWN, and a later flushwtdw can debit fresh collateral before payepoch. Please document that outstanding withdrawals must be cancelled or flushed first (or clear/generation-tag them in the contract), and cover immediate re-registration/rebonding with a pending withdrawal.

Agreed — fixed in 41fadc7: the guide now says to cancel any withdrawal still queued from before termination with cancelwtdw before bonding again, since the queue is per account and would reserve part of the new bond and pay out of it. stale_withdrawal_holds_back_a_rebonded_producer_until_cancelled queues a withdrawal, terminates, re-registers and posts the full bond while the request is queued: the producer stays UNKNOWN until cancelwtdw, and the next payout pays its held blocks. This PR stays docs and tests; clearing the account's queued withdrawals at termination would be the contract-side fix.

huangminghuang · 2026-10-23T18:25:34Z

+   BOOST_REQUIRE_GT( held, 0u );
+
+   // A payout the producer is not payable at pays it nothing and keeps its count.
+   const auto require_unpaid_payout = [&] {


[P2] Use the required assertion-helper name. The binding naming rule requires assertion helpers to use assert* and explicitly bans require*. Please rename this lambda to assert_unpaid_payout at its declaration and both call sites.

Agreed — fixed in 41fadc7: it is now the fixture's assert_unpaid_payout.

huangminghuang · 2026-10-23T18:25:35Z

+
+   const auto target = names[0];
+   BOOST_REQUIRE_EQUAL( success(), terminate_operator(target) );
+   BOOST_REQUIRE_EQUAL( "OPERATOR_STATUS_TERMINATED", get_opreg_operator(target)["status"].as_string() );


[P2] Keep OperatorStatus typed. The enum invariant requires closed-set values, including fc::variant reads, to remain typed. Please decode with .as<OperatorStatus>() and compare against OperatorStatus::OPERATOR_STATUS_TERMINATED; apply the same change to the UNKNOWN and ACTIVE assertions below.

Agreed — fixed in 41fadc7: all three status checks compare .as<OperatorStatus>() with OperatorStatus::OPERATOR_STATUS_*, through the fixture's operator_status.

huangminghuang · 2026-10-23T18:25:36Z

+
+   // Pull the returned bond and register again: the new registration holds no bond.
+   const asset unclaimed = get_wire_balance(target);
+   BOOST_REQUIRE_EQUAL( success(), push_opreg_action(target, "claimremit"_n,


[P2] Name the contract action and ABI-field identifiers. The no-magic-literals invariant specifically requires account/table/action/field identifiers to be named constexpr values grouped by contract. Please introduce or reuse constants for claimremit, regoperator, deposit, their field keys, and the later compute_amount lookup.

Agreed — fixed in 41fadc7: opreg_action, opreg_table, opreg_field, epoch_field and epochlog_field name every action, table and field the held-pay tests use, and credit_collateral now pushes deposit through them as well.

huangminghuang · 2026-10-23T18:25:37Z

+   wait_for_producer_schedule();
+   produce_complete_cycles(3, 2);
+
+   const uint32_t start = head_secs() - ONE_EPOCH - 1;


[P2] Name the semantic timing values. The one-second overdue offset, both 130-block waits, and epoch-log index 3 encode test behavior rather than trivial indexes or loop bounds. Please replace them with descriptive constants, or derive the epoch index from observed state, as required by the no-magic-literals invariant.

Agreed — fixed in 41fadc7: the offset is t5_start_overdue_secs, the waits are cross_epoch_boundary() (one epoch of slots plus epoch_crossing_margin_blocks), and the payout check reads the paid epoch from epochstate instead of assuming 3.

huangminghuang · 2026-10-23T18:25:39Z

+   BOOST_REQUIRE_EQUAL( success(), regproducer(alice) );
+
+   BOOST_REQUIRE_EQUAL( success(), register_finalizer_key(alice, finalizer_key_1, pop_1) );
+   const uint64_t first_id = get_finalizer_info(alice)["active_key_id"].as_uint64();


[P2] Reuse the named fixture and name ABI-field keys. This test introduces raw table-field strings and later repeats alice1111111 even though the file already defines the same account as alice. Please reuse that fixture spelling and introduce or reuse constants for active_key_id, finalizer_key_count, active_key_binary, finalizer_name, and finalizer_key.

Agreed — fixed in 41fadc7: the test uses alice and a finalizer_field group for the five keys.

huangminghuang · 2026-10-23T18:25:40Z

+   produce_block();
+
+   BOOST_CHECK_EXCEPTION(
+      add_roa_policy(issuer, user, "2.0000 SYS", "2.0000 SYS", "2.0000 SYS", 0, future_gen),


[P2] Name the policy values and field identifiers. The repeated SYS quantities and net_weight/cpu_weight/ram_weight keys are semantic values and contract fields, so the no-magic-literals invariant requires descriptive constants rather than repeated raw strings.

Agreed — fixed in 41fadc7: granted, step and unlocked_time_block name the values, and a policy_field group names the three weight keys.

…and-tests

…ncelled Termination leaves queued withdrawals in place, and the queue is per account, so a request from before termination reserves part of the new registration's bond and is paid out of it when it matures. The producer guide now says to cancel it with cancelwtdw before bonding again, and a new test rebonds a terminated producer while its request is queued: it stays UNKNOWN until the request is cancelled, and the next payout pays its held blocks. The held-pay tests now share a fixture, keep OperatorStatus typed, name their opreg actions and fields and their timing values, and read the paid epoch from epochstate. The network_gen and finalizer-key tests name their quantities and field keys and use the alice fixture. Master is merged in. Change-Id: Iacf1d505441a609cdc192b65da97f2ec62fd5c4b

Brings in #651, #659 and #701. sysio.epoch.wasm is rebuilt from the merged sources, and every other contract matches its committed wasm and ABI. MAX_OPERATORS_PER_EPOCH keeps the cap of 7 with #659's cost wording, one recorddel and termcheck per member. #659's shared rotation walk now checks the window roster: a member terminated as its group expires is no longer listed.

Brings in #692 (merged as a62f42c, superseding this branch's copies of its commits), #592, #659, #658, #651 and the rest of master. - `buildenv` keeps #592's retention: shipped rows are erased once packed, the chain links come from the newest `byoutepoch` row, and the acknowledged-prefix sweep runs before the inbound PROCESSED sweep. - `shipped_attestations` reads the newest `outenvelopes` row, since unacknowledged emits now stay, and the new readers decode with `fixture_abi_yield_function()` after #684's sweep. - Two locals in `buildenv` are renamed so they no longer shadow (`by_queue`, `inbound`). - `sysio.msgch.wasm` rebuilt.

heifner requested a review from a team September 28, 2026 16:38

heifner changed the title ~~Contracts: document audited designs and pin two of them in tests~~ Doc: Contracts: document audited designs and pin two of them in tests Sep 28, 2026

huangminghuang reviewed Sep 29, 2026

View reviewed changes

heifner added 2 commits October 23, 2026 14:03

Merge remote-tracking branch 'origin/master' into fix/wns-audit-docs-…

9dd5cff

…and-tests votedispute takes master's comment: #690 settled WNS-59 and documents the ballot rule, so this branch's note is dropped. The unpaid_blocks comment keeps master's wording without "or terminated".

heifner changed the title ~~Doc: Contracts: document audited designs and pin two of them in tests~~ Doc: Contracts: document audited designs and pin three of them in tests Oct 23, 2026

heifner requested a review from huangminghuang October 23, 2026 18:04

huangminghuang reviewed Oct 23, 2026

View reviewed changes

heifner added 2 commits October 23, 2026 17:11

Merge remote-tracking branch 'origin/master' into fix/wns-audit-docs-…

5b9c450

…and-tests

heifner requested a review from huangminghuang October 23, 2026 21:12

huangminghuang approved these changes Oct 23, 2026

View reviewed changes

heifner merged commit ce7e4af into master Oct 24, 2026
13 checks passed

Conversation

heifner commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

heifner commented Sep 28, 2026 •

edited

Loading