Repository navigation
snapshot: restore PM seed flags; docs: HF16 readiness check - #184
Conversation
import_dynamic_global_properties rebuilt dgp field by field and skipped pm_frozen_counters_seeded, pm_frozen_counters_reseeded_v1, pm_active_markets_seeded and pm_oracle_gauges_seeded. Export is reflection-based and carries them, so a restored node re-ran the one-time seeds on its first block and its later snapshots disagreed with the network. Read them with the same absent-defaults pattern as round_anchor_*. hf16.md (en/ru): "Verify readiness" section. Activation needs 17 of 21 validators voting 4.2.0 @ 1793404800; shows get_next_scheduled_hardfork and get_validators_by_vote and what to count in each.
On1x
left a comment
On1x
left a comment
There was a problem hiding this comment.
Verified the premise and the fix against the tree, not just the description.
The gap is exactly as stated. dynamic_global_property_object has 36 members; import_dynamic_global_properties assigns 34 of them. The two left over are id (set via set_next_id before the loop) and precisely the four seed flags this PR adds. So after this change the field-by-field importer covers every reflected member of the struct — no fifth flag is missing.
Export really is reflection-based, so the asymmetry was one-sided: EXPORT_INDEX writes with fc::to_variant(*itr, v) (plugin.cpp:1287) and all four flags sit in FC_REFLECT (global_property_object.hpp:256-259). A restore therefore dropped state that every snapshot carried.
Absent-key default = false matches the existing round_anchor_* pattern and is the right default for pre-fix snapshots: a node restored from an old file re-runs an idempotent, display-only seed once, which is the pre-fix behaviour, not a new one.
Docs claims check out. CHAIN_HARDFORK_REQUIRED_VALIDATORS is 17 (config.hpp:50). The tally in database.cpp:3663-3690 promotes next_hardfork when a (version, time) pair reaches 17 and resets it to the current version otherwise, so 'if the 17th vote lands late, the fork activates at that block with no announced moment' is accurate, and so is 'until then it mirrors the current version'. One precision nit, non-blocking: the tally walks the scheduled validators (one per repeat slot) and skips the emergency committee validator, so counting via get_validators_by_vote ["", 21] is only equivalent while the schedule is the top 21 by vote.
Nit, non-blocking: contains(x) && v[x].as_bool() throws if the key is present but null; snapshots never write null there, so it is safe today, but !is_null() would express the defensive intent.
CI on the branch is green (4 jobs pass, 2 path-filter skips).
One thing this PR does not fix, and nothing in CI would catch: a future field added to the struct and forgotten in this importer. That is the class that produced this bug, and it recurs silently. A mechanical guard — compare reflected members against obj.X = assignments for every field-by-field importer — is worth a follow-up PR; I can write it if wanted.
… tally in HF16 readiness check
|
Thanks for checking it against the tree. Both nits are addressed in 01e1fe0:
There are no other changes. As before, the snapshot TU wasn't compiled locally, so CI is the build check. |
A snapshot is exported by reflection, but the importer is written by hand, field by field. A field added to a chain object and forgotten in the importer is dropped silently on restore -- exactly what happened to the four PM seed flags in #184: a restored node re-seeded on its first block and its later snapshots disagreed with the network. scripts/check_snapshot_import_coverage.py compares, for every object the snapshot plugin creates or modifies from the file, the fields listed in FC_REFLECT against the fields the body assigns; objects copied with fc::from_variant are exempt by construction. db.modify() bodies that repair in place (LIB promotion, ballot reconciliation, self-healing) are not counted as import evidence. The same script finds two more fields of the same class on master: pm_oracle_object.banned_by and pm_oracle_object.active_markets, both present since HF14 and never copied, so a restore made a ban un-liftable and reset the live-market gauge to zero for good. Imported here under the same contains() guard as the neighbouring optional fields, so the check can gate CI. Codes: 0 every reflected field is assigned, 1 a field is missing (named with its plugin.cpp line), 2 blindness (a type could not be judged, e.g. no FC_REFLECT and not in the declared hand-written list) -- "could not check" is never reported as clean. --selftest proves the arms and both blindness paths. tested: selftest 5/5 green; on master the script names the four #184 flags; on the #184 tree it names only banned_by/active_markets; with the banned_by assignment removed from this tree it names only banned_by.
A snapshot is exported by reflection, but the importer is written by hand, field by field. A field added to a chain object and forgotten in the importer is dropped silently on restore -- exactly what happened to the four PM seed flags in #184: a restored node re-seeded on its first block and its later snapshots disagreed with the network. scripts/check_snapshot_import_coverage.py compares, for every object the snapshot plugin creates or modifies from the file, the fields listed in FC_REFLECT against the fields the body assigns; objects copied with fc::from_variant are exempt by construction. db.modify() bodies that repair in place (LIB promotion, ballot reconciliation, self-healing) are not counted as import evidence. The same script finds two more fields of the same class on master: pm_oracle_object.banned_by and pm_oracle_object.active_markets, both present since HF14 and never copied, so a restore made a ban un-liftable and reset the live-market gauge to zero for good. Imported here under the same contains() guard as the neighbouring optional fields, so the check can gate CI. Codes: 0 every reflected field is assigned, 1 a field is missing (named with its plugin.cpp line), 2 blindness (a type could not be judged, e.g. no FC_REFLECT and not in the declared hand-written list) -- "could not check" is never reported as clean. --selftest proves the arms and both blindness paths. tested: selftest 5/5 green; on master the script names the four #184 flags; on the #184 tree it names only banned_by/active_markets; with the banned_by assignment removed from this tree it names only banned_by.
|
@On1x thanks for merging. Yes, please go ahead with the reflected-members guard for the field-by-field importers. That's the class that caused this bug, and nothing catches it today. Happy to review it when it's up. |
Follow-up to #183 (the commit was pushed to that branch minutes after the merge, so it is re-based here on master).
Snapshot
import_dynamic_global_propertiesinplugins/snapshot/plugin.cpprebuilds dgp field by field and skipped four flags:pm_frozen_counters_seeded,pm_frozen_counters_reseeded_v1,pm_active_markets_seeded,pm_oracle_gauges_seeded. Export is reflection-based and already carries them. Effect before this fix: a restored node re-ran the one-time seeds on its first block, and its own later snapshots carried flags that differ from the network. The seeds are idempotent and display-only, so no consensus impact. Flags absent in the file read as false, same pattern asround_anchor_*.Docs
docs/consensus/hf16.mdand the ru copy gain a "Verify readiness" section. Activation needs 17 of 21 scheduled validators voting 4.2.0 at 1793404800; if fewer have upgraded by 2026-10-31 00:00 UTC the fork fires at the block where the 17th vote lands. The section showsdatabase_api.get_next_scheduled_hardforkandvalidator_api.get_validators_by_vote ["", 21]and what to count in each.Verification
Snapshot TU syntax-checked with the recorded local compile flags. No P2P or consensus paths touched.