Skip to content

HF16 readiness: count the quorum by schedule slots - #191

Merged
On1x merged 2 commits into
masterfrom
fix/hf16-readiness-quorum
Oct 27, 2026
Merged

On1x merged 2 commits into
masterfrom
fix/hf16-readiness-quorum

Conversation

@chiliec

@chiliec chiliec commented Oct 26, 2026

Copy link
Copy Markdown
Member

Why

Mainnet (api.viz.world, 2026-10-26) schedules 13 validators. Validators with a null signing key are not scheduled, and the tally in update_validator_schedule walks only current_shuffled_validators[0 .. num_scheduled_validators). So 17 votes for 4.2.0 cannot be reached: 12 of the 13 already vote 4.2.0 and next_hardfork stays 4.1.0. HF16 does not activate on 2026-10-31 until at least 4 more validators set a key and vote 4.2.0.

lindsay and monsterhigh already run 4.2.0 with a null key, so a validator_update is enough for them. jackvote is scheduled but on 4.1.2.

The readiness doc from #184 says "17 of the 21 scheduled" and counts get_validators_by_vote ["", 21], which also lists null-key validators. No test caught this: testnet builds set CHAIN_HARDFORK_REQUIRED_VALIDATORS = 1.

What

  • scripts/hardfork_readiness.py: repeats the chain's recount from get_validator_schedule (slots, committee skipped during emergency, grouped by version and time, first group with ≥17 wins). It lists scheduled validators voting otherwise and null-key validators voting the leading version, then cross-checks against get_next_scheduled_hardfork. Exit 0 = quorum reached, 1 = not yet, 2 = disagrees with the chain, or the API answer is unusable.
  • --selftest covers 16 vs 17, 13 slots, committee skip, per-slot counting and the hex schedule encoding. It also pins REQUIRED/REPEAT to config.hpp. Runs in the CI offline step.
  • docs/consensus/hf16.md + ru: quorum is counted in schedule slots; how to run the script; manual check via get_validator_schedule. zh-CN has no readiness section, so it is unchanged.

No chain code touched.

Verification

  • --selftest: 9/9 pass. Setting REQUIRED = 16 fails the config arm.
  • Live run against api.viz.world: 13 slots, 4.2.0 12/17, recount "none" agrees with chain next_hardfork 4.1.0.

…ule slots

Mainnet schedules 13 validators (null-key validators are not scheduled), so the
17-slot tally cannot promote 4.2.0. The readiness doc counted 'upgraded of the
top 21 by vote', which hides that. hardfork_readiness.py repeats the chain's
recount from get_validator_schedule and cross-checks next_hardfork; its
--selftest runs in CI and pins the quorum constants to config.hpp.
@On1x

On1x commented Oct 26, 2026

Copy link
Copy Markdown
Member

Thanks — the core of this is right, and the diagnosis it argues for is confirmed on the live chain. But there are four problems, and two of them are in the tool an operator will use to judge HF16 readiness.

1. "recount at shuffle #N" is not the recount block

scripts/hardfork_readiness.py:100 prints next_shuffle_block_num as the block where the chain recounts, and the docstring at :24-25 tells the operator to rerun after it. Neither holds.

  • The chain recounts only when head_block_num() % (CHAIN_MAX_VALIDATORS * CHAIN_BLOCK_VALIDATOR_REPEAT) == 0, i.e. every 21 blocks (libraries/chain/database.cpp:3540).
  • The field is set to head_block_num() + num_scheduled_validators (database.cpp:3782-3783) — "height at which the schedule was last built + how many slots it has". It is write-only: git grep next_shuffle_block_num on master @ d5e937bd gives four assignments (593, 3782, 3844, 6734), the struct field and one FC_REFLECT — and no reads at all. Nothing in the chain consults it to decide when to recount.
  • The two coincide only while num_scheduled_validators == 21. As soon as there are fewer slots — exactly the scenario this PR exists for — the printed block is not a recount block, and once the head passes it the label names a block in the past.

Live, a few minutes ago:

head #84623539, current hardfork 4.1.0, emergency: no
scheduled slots: 16 (recount at shuffle #84623548), quorum 17

84623539 % 21 = 7, so the next recount is #84623553, not #84623548. And 84623548 % 21 = 16 — the printed block is one on which nothing is recounted. Earlier, inside the 21-block window, the same line reported a block four blocks in the past while the actual assembly happened at a different height.

Suggested fix — one expression, and it gives MAX_VALIDATORS a reason to be pinned in the selftest alongside REQUIRED and REPEAT (:164):

MAX_VALIDATORS = 21            # CHAIN_MAX_VALIDATORS
next_recount = (dgp["head_block_number"] // (MAX_VALIDATORS * REPEAT) + 1) * (MAX_VALIDATORS * REPEAT)

Or drop the parenthetical entirely: the exit code does not depend on it, and a wrong block number is worse than none.

Note where the assumption probably comes from: database.cpp:6724-6728 says the emergency hybrid override "runs on the next schedule update" and that otherwise "next_shuffle_block_num is still N blocks away" — which also implies a reader that does not exist. Worth a separate cleanup, out of scope here.

2. The docs state the opposite of what get_active_validators does

docs/consensus/hf16.md:30 and @l10n/ru/docs/consensus/hf16.md:30: "get_active_validators returns the whole schedule array. Neither is the set the tally counts."

  • The handler returns only the non-empty slots (plugins/validator_api/plugin.cpp:30-43, aliased at :206); the underlying array is fixed-size (CHAIN_MAX_VALIDATORS * REPEAT, validator_objects.hpp:131) and its tail is zeroed on every rebuild (database.cpp:3754-3757).
  • With REPEAT = 1 and no emergency consensus, that set is exactly the tally's set: the first num_scheduled_validators non-empty slots. committee is present in both and skipped by both only under emergency.
  • Live check at the same head: get_active_validators → 16 names, num_scheduled_validators → 16, sets identical.

So the previous advice — "take the names from get_active_validators, drop committee, look each one up" — was correct, and this PR replaces it with a wrong warning. I would keep the advice and name the real boundary: it differs only under emergency consensus, and when REPEAT > 1 (active lists every slot; the tally takes every REPEAT-th, counting a validator once).

3. Off by one: activation happens in the recount block itself

docs:11 / @l10n/ru ...:11: "activates at the first block after the recount".

Within a block, update_validator_schedule() (database.cpp:6153) runs before process_hardforks() (:6183), and process_hardforks applies as soon as next_hardfork_time <= head_block_time() (database_hardfork.cpp:155). With a live_time already in the past, the fork applies in the very block that promoted it. "the first block after the recount" → "in the recount block".

4. The "voting but unscheduled" hint can be silent for the case it exists for

:115 builds that list from get_validators_by_vote, which stops at the first validator with zero votes (plugin.cpp:120: while (itr != vote_idx.end() && result.size() < limit && itr->votes > 0)). A zero-vote validator with a null key never appears there — and a null key is precisely why validators fall out of the schedule.

Live: get_validator_count = 103, get_validators_by_vote ["", 100] = 91, and uuid is missing from the list while sitting in the schedule (support slots are chosen by virtual_scheduled_time, not by votes; database.cpp:3596-3620).

The section is not wrong today (every voting validator is scheduled), but the boundary should be named in the script and in the docs — or the list built from get_validator_count + pagination instead. The same sentence in the docs ("also lists unscheduled validators with a null key") is only true for those with votes > 0.

Minor

  • --selftest returns 1 on failure, and 1 also means "quorum not reached". A red selftest therefore reads as "not ready". Suggest 4 for "the check itself is broken".
  • Run outside the repo tree, --selftest hits the unguarded open(...) at :162 → traceback with rc 1, the same collision — and downloading the script is a natural way to use it.
  • votes = {…} | {…} at :147 requires Python ≥ 3.9. Fine on ubuntu-latest; operators are not necessarily on it.
  • "until 1 more validators set a signing key" (:102-103) — plural.
  • The docstring pins a dated snapshot ("On 2026-10-26 mainnet scheduled 13 validators"); by the time I ran it, it was 16. Add "at the time of writing" or make the example explicitly timestamped.
  • REQUIRED is read from config.hpp, not from the node. Against a build that overrides it (testnet N=1 in CI) the script will honestly report DISAGREE. That is fine for mainnet, but one sentence would prevent a false alarm; the selftest pinning the constant currently reads as "the node agrees".

What is correct (so it does not get refactored away)

  • The tally mirrors the chain loop: REPEAT step over current_shuffled_validators[0 .. num_scheduled_validators), committee skipped only under emergency, grouping by (hardfork_version_vote, hardfork_time_vote), ascending order, first group ≥ 17, otherwise fallback to the current version (database.cpp:3653-3725).
  • "A validator with a null signing key is not scheduled, so its vote does not count" is right (database.cpp:3604-3606).
  • Exit codes 0/1/2, and 2 for blindness rather than "not ready", are the right split; the cross-check against get_next_scheduled_hardfork is the right shape.
  • The selftest is real and it actually runs in CI — tests.yml:43, job consensus-and-pm passes. Not a test written and forgotten.
  • The live output matched an independent recount one-for-one: 16 slots, 15 votes for 4.2.0, jackvote on 4.1.0, chain in agreement, rc 1.

Readiness, not about the patch

At head #84623538: 16 scheduled slots, 15 of them voting 4.2.0 @ 2026-10-31T00:00:00. The threshold is 17, so it currently takes two more votes and one more scheduled slot — even if all 16 slots voted 4.2.0, 16 < 17. The PR's central claim is correct, and worth repeating in the description: this is why "17 upgraded validators out of 21" is the wrong metric.

… 4; docs: get_active_validators is the tally set, fork applies in the recount block
@chiliec

chiliec commented Oct 27, 2026

Copy link
Copy Markdown
Member Author

Thanks. Every point checks out against the tree. Fixed in fe842c4:

  1. Recount block. The script prints next recount at #N with N = (head // 21 + 1) * 21, and the comment notes that next_shuffle_block_num is write-only. MAX_VALIDATORS is now pinned in the selftest (as TOP + SUPPORT from config.hpp). The docstring says "rerun after the printed recount block". I left the stale comment at database.cpp:6724-6728 alone, as you suggested.
  2. get_active_validators. I was wrong: it filters empty slots and the tail is zeroed. The docs restore it as the manual check, and the REPEAT>1 / emergency boundary is in the script docstring. get_validators_by_vote is now described as the wrong set for both reasons.
  3. Off by one. The docs say the fork applies "in the recount block itself" and that the chain recounts every 21 blocks.
  4. votes > 0. The script label and the docs name the boundary. I didn't paginate: validator ids are not contiguous (get_validators [102] is null with a count of 103), so a complete list costs about 100 calls to get_validator_by_account. That's not worth it for a hint.

Minors: a failed selftest now exits 4, and outside the repo tree the config arms are skipped instead of throwing a traceback. Also fixed: the | merge (Python 3.9+ only), the plural, the dated docstring example ("at the time of writing"), and a sentence that the quorum constants are production ones, so a testnet node reports DISAGREE.

Live at #84634402: 16 slots, 15/17 for 4.2.0, jackvote on 4.1.0, and the chain agrees. It needs one more scheduled slot and two more votes.

@On1x
On1x merged commit d92b9da into master Oct 27, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants