Repository navigation
snapshot: check that every reflected field is restored by the importer - #187
Merged
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
#184 restored four snapshot fields by hand. Nothing in the build or the tests would have stopped the next one from being forgotten the same way — and nothing had caught these two:
pm_oracle_object.banned_byandpm_oracle_object.active_marketshave been in the struct and inFC_REFLECTsince HF14 (#124), so every snapshot written since contains them — the importer never copied them. On a restore a banned oracle comes back with an emptybanned_by, andpm_unbanmatches on that field, so the ban can never be lifted; theactive_marketsgauge is seeded once fromdgpo.pm_active_markets_seeded, which does not re-run, so it stays at 0 for good.What
scripts/check_snapshot_import_coverage.py— for every object the snapshot plugin creates or modifies from the snapshot file, compare the fields listed inFC_REFLECTwith the fields the importer body assigns, and name what is missing, with itsplugin.cppline.fc::to_variant), soFC_REFLECTis the file's field set; the importer is hand-written, and that is where fields disappear.FC_REFLECT/FC_REFLECT_DERIVEDare parsed fromlibraries/chain/include/**andlibraries/chain/hardfork.d/*.hf(the hardfork object lives there), with inheritance resolved.obj.field = …must not count as an assignment.fc::from_variantare exempt by construction (reflection copies everything).obj.f = …: also container fills (obj.buffer.resize(),obj.flat_set.insert()) and shared_string setters.db.modify()bodies count only if they read the snapshot variant. The in-place repairs (LIB promotion, ballot reconciliation, self-healing penalties) must not look like restore evidence, or a field missing from the importer would appear covered.FC_REFLECTare listed inNO_REFLECT_EXPORTEDwith a reason (proposal_object: custom export because ofbuffer_type/bip::flat_set). A type that is missing reflection and missing from that table makes the run blind (2), not clean.Codes:
0every reflected field is assigned,1a field is missing,2blindness (nothing parsed, or a type could not be judged).plugins/snapshot/plugin.cpp— importbanned_byandactive_marketsunder the samecontains()guard as the neighbouring optional fields..github/workflows/tests.yml— run the guard, selftest first, as the second step ofconsensus-and-pm: a second, and it needs no submodules.Proof
--selftest: 5 arms — covered → 0, missing field → 1, unknown type → 2, hand-written type listed → 0, empty importer → 2. All green.masterbefore this branch: names exactly the four seed flags from snapshot: restore PM seed flags; docs: HF16 readiness check #184.banned_byandactive_markets, the two fixed here.rc=0, 28 objects, 23 judged field by field, 4 copied by reflection, 1 out of scope.banned_byassignment → names onlybanned_by.Scope boundary
The guard judges what
FC_REFLECTdescribes.proposal_objectis exported by hand as well, so it is out of scope by name, not by silence: a hand-written exporter can still drop a new field on both sides, and catching that needs the same comparison on the export side. That is not part of this PR.