Skip to content

fix(provider/genesis): provider genesis round-trip preserves per-consumer state#44

Open
giunatale wants to merge 9 commits into
mainfrom
giunatale/genesis-fix
Open

fix(provider/genesis): provider genesis round-trip preserves per-consumer state#44
giunatale wants to merge 9 commits into
mainfrom
giunatale/genesis-fix

Conversation

@giunatale
Copy link
Copy Markdown
Contributor

Provider ExportGenesis/InitGenesis didn't faithfully round-trip the keeper's per-consumer state: owner addresses, metadata, init params, and the spawn/removal queues were silently dropped on a state-export upgrade. Validate also rejected the empty client_id that ExportGenesis was emitting for pre-launch consumers, so any chain with a REGISTERED or INITIALIZED consumer couldn't be re-imported at all.

Added owner_address, metadata, init_params, removal_time to the ConsumerState proto. Rewrote Validate as a phase-aware switch matching the keeper's actual per-phase invariants.
The DELETED phase still requires metadata/init_params, mirroring the existing "kept for explorers" policy in DeleteConsumerChain.

NOTE: personally not a fan of this data retention policy: this may cause state-bloating for deleted chains which can become especially detrimental for a permissionless model, but I tried to stay consistent with what was done so far.

ExportGenesis now iterates all consumer ids including STOPPED/DELETED.
InitGenesis re-derives the spawn/removal queues and equivocation min height from per-consumer fields. the Debt state isn't exported either since the first BeginBlock after import re-derives it from the fee pool balance.

The e2e test depends on PR #43 because of a stale legacy iterator in prepForZeroHeightGenesis. Until that PR is merged the e2e test will fail.

Comment thread x/vaas/provider/keeper/genesis.go Outdated
Comment thread x/vaas/provider/keeper/genesis.go Outdated
Comment thread x/vaas/provider/keeper/genesis.go Outdated
Comment thread x/vaas/provider/keeper/genesis.go Outdated
Comment thread x/vaas/provider/keeper/genesis.go
giunatale added 9 commits May 20, 2026 16:37
REGISTERED/INITIALIZED consumers legitimately have an empty client_id
(no IBC client created yet). Make Validate() enforce per-phase
invariants matching what the keeper actually persists: pre-launch
states accept empty client_id; LAUNCHED requires it; STOPPED also
requires removal_time; DELETED keeps owner/metadata/init_params per
the explorer-preservation policy in DeleteConsumerChain.
ExportGenesis now iterates all consumer ids (not just active phases) so
that STOPPED and DELETED consumers are preserved across state-export
software upgrades. Each ConsumerState carries the four new per-consumer
fields read from their dedicated keeper collections; the spawn queue,
removal queue, equivocation min height, and debt state are intentionally
not emitted — InitGenesis re-derives them.
Reads owner/metadata/init_params/removal_time/client_id/consumer_genesis
into their dedicated keeper collections, then re-derives the spawn-time
queue (from INITIALIZED consumers' init_params.spawn_time), the
removal-time queue (from STOPPED consumers' removal_time), and the
equivocation_evidence_min_height (from LAUNCHED/STOPPED consumers'
init_params.initial_height.revision_height). ConsumerDebt is left
unset and re-derived by the first BeginBlock after import.
Add TestGenesisRoundTrip which seeds keeper A with all five consumer
phases (including derived spawn/removal queues and equivocation min
heights), exports, validates, imports into a fresh keeper B, re-exports,
and asserts deep equality — proving the round-trip is a fixed point.

Also fix InitGenesis to skip AppendPendingVSCPackets when the incoming
list is empty, preventing a spurious store entry that caused
GetPendingVSCPackets to return nil instead of []{}  on re-export and
broke the round-trip equality check.
@giunatale giunatale force-pushed the giunatale/genesis-fix branch from 8ad353d to 04dea7d Compare May 20, 2026 14:39
@giunatale
Copy link
Copy Markdown
Contributor Author

Addressed your f/b and also migrated consumer_id to uint64 as I think it's much better/cleaner this way

Copy link
Copy Markdown
Contributor

@tbruyelle tbruyelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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