fix(fsm): preserve RetiredCommittees across genesis export/import - #586
Open
Alicepoltora wants to merge 1 commit into
Open
fix(fsm): preserve RetiredCommittees across genesis export/import#586Alicepoltora wants to merge 1 commit into
Alicepoltora wants to merge 1 commit into
Conversation
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.
Description
GenesisStateserializes through a hand-written JSON codec (thegenesisStatestruct infsm/genesis.go). That struct omitsRetiredCommittees, and becauseGenesisStatedefines its ownMarshalJSON/UnmarshalJSON, the proto tag on the real field is never used.The result is that retired committees do not survive a genesis round trip:
ExportStatecorrectly populatesgenesis.RetiredCommittees(fsm/genesis.go).genesis.json.SetRetiredCommittees(genesis.RetiredCommittees)insideNewStateFromGenesisalways receives an empty slice.So after a state export ->
genesis.json-> restart (the documented upgrade/fork path), every previously retired committee is live again: it re-qualifies inGetSubsidizedCommitteesand therefore starts receiving freshly minted tokens fromFundCommitteeRewardPoolsagain, andHandleCertificateResultsstops rejecting its certificates.RetireCommitteeis documented as making a committee non-subsidized "for eternity", but that property currently does not survive an export/import.This is not attacker-controlled - it affects operators and governance performing an export-import upgrade.
Related Issues
None that I could find - I searched open and closed issues and PRs for
RetiredCommittees/ genesis serialization and found nothing covering this. Happy to open an issue first if you prefer that flow for changes like this.Changes Made
RetiredCommitteesfield to thegenesisStateJSON codec struct, with a comment explaining why it must be serialized.GenesisState.MarshalJSONandGenesisState.UnmarshalJSON.TestGenesisJSONRoundTripPreservesRetiredCommittees, which marshals aGenesisStatecarrying retired committees and asserts they are still present after unmarshaling.The change is intentionally limited to the serialization gap; no behavior outside the codec is touched.
Checklist
issue-#<issue-number>. (No issue exists for this; the branch is named after the fix. Happy to rename if you'd like an issue opened first.)npm run prettierto format the web-wallet and/or block explorer (if applicable). (Not applicable.)Additional Notes
Verification performed on
development:retiredCommitteeskey) and passes after it.go test ./fsm/passes in full.go test $(go list ./... | grep -v cmd/auto-update)- i.e. the CI test command - passes across the repository.go build ./...succeeds andgofmtreports no changes for both touched files.go vet ./fsm/surfaces only pre-existing warnings (inaccount_test.go, oldergenesis_test.gotable cases, and generatedmessage.pb.go); nothing from this change.Two small things I noticed while here but deliberately left out to keep this PR single-purpose:
CONTRIBUTING.mdasks contributors to update.docs/CHANGELOG.md, but neither that file nor the.docsdirectory exists in the repository. Glad to add a CHANGELOG in a separate PR if you want one.genesisStatecodec also drops the export-onlyCommitteesfield. I left it alone since it is a different concern; happy to follow up if it is also unintended.