Skip to content

fix(fsm): preserve RetiredCommittees across genesis export/import - #586

Open
Alicepoltora wants to merge 1 commit into
canopy-network:developmentfrom
Alicepoltora:fix/genesis-retired-committees
Open

fix(fsm): preserve RetiredCommittees across genesis export/import#586
Alicepoltora wants to merge 1 commit into
canopy-network:developmentfrom
Alicepoltora:fix/genesis-retired-committees

Conversation

@Alicepoltora

Copy link
Copy Markdown

Description

GenesisState serializes through a hand-written JSON codec (the genesisState struct in fsm/genesis.go). That struct omits RetiredCommittees, and because GenesisState defines its own MarshalJSON/UnmarshalJSON, the proto tag on the real field is never used.

The result is that retired committees do not survive a genesis round trip:

  • ExportState correctly populates genesis.RetiredCommittees (fsm/genesis.go).
  • The field is silently dropped on marshal, so it never reaches genesis.json.
  • On the way back in, SetRetiredCommittees(genesis.RetiredCommittees) inside NewStateFromGenesis always 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 in GetSubsidizedCommittees and therefore starts receiving freshly minted tokens from FundCommitteeRewardPools again, and HandleCertificateResults stops rejecting its certificates. RetireCommittee is 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

  • Added the RetiredCommittees field to the genesisState JSON codec struct, with a comment explaining why it must be serialized.
  • Wired it through GenesisState.MarshalJSON and GenesisState.UnmarshalJSON.
  • Added TestGenesisJSONRoundTripPreservesRetiredCommittees, which marshals a GenesisState carrying 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

  • I have tested the changes locally and verified they work as intended.
  • I have appropriately titled my branch 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.)
  • I have run re-built the web-wallet and/or block explorer (if applicable). (Not applicable - Go only.)
  • I have run npm run prettier to format the web-wallet and/or block explorer (if applicable). (Not applicable.)
  • I have updated documentation (if applicable). (Not applicable.)
  • I have included tests for the changes (if applicable).

Additional Notes

Verification performed on development:

  • The new test fails before the fix (the marshaled JSON contains no retiredCommittees key) 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 and gofmt reports no changes for both touched files.
  • go vet ./fsm/ surfaces only pre-existing warnings (in account_test.go, older genesis_test.go table cases, and generated message.pb.go); nothing from this change.

Two small things I noticed while here but deliberately left out to keep this PR single-purpose:

  1. CONTRIBUTING.md asks contributors to update .docs/CHANGELOG.md, but neither that file nor the .docs directory exists in the repository. Glad to add a CHANGELOG in a separate PR if you want one.
  2. The same genesisState codec also drops the export-only Committees field. I left it alone since it is a different concern; happy to follow up if it is also unintended.

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.

1 participant