chore(upgrade/v1.12.0): persist post-1.11.1 defaults and anchor Everlight clock#131
Open
mateeullahmalik wants to merge 1 commit intomasterfrom
Open
chore(upgrade/v1.12.0): persist post-1.11.1 defaults and anchor Everlight clock#131mateeullahmalik wants to merge 1 commit intomasterfrom
mateeullahmalik wants to merge 1 commit intomasterfrom
Conversation
…ght clock The v1.12.0 upgrade handler previously only ran module migrations. After v1.11.1-hotfix several PRs landed that introduced new params and per-chain anchors which were not persisted at activation: - LEP-5 (#103, action): SvcChallengeCount, SvcMinChunksForChallenge. - Everlight plus LEP-4 (#113, supernode): RewardDistribution, MetricsUpdateIntervalBlocks, MetricsGracePeriodBlocks, MetricsFreshnessMaxBlocks, MinSupernodeVersion, MinCpu/Mem/StorageGB, MaxCpu/Mem/StorageUsagePercent, RequiredOpenPorts. New state keys LastDistributionHeight / TotalDistributed / SNDistState. Module account permissions changed to Minter+Burner+Staking. - LEP-6 (#117, audit): full StorageTruth params block plus ConsensusVersion bump 1 to 2 with registered v1 to v2 migration (covered by RunMigrations). This commit: 1. Adds Params.WithDefaults() in x/action/v1/types and applies it inside keeper.GetParams and keeper.SetParams (sibling-asymmetry fix; supernode and audit already had this). 2. Rewrites app/upgrades/v1_12_0/upgrade.go to: - Run consensusparams.EnsurePresent (defensive). - Run module migrations (audit v1 to v2 fires here). - Persist action params via SetParams(GetParams) so LEP-5 defaults land in the on-disk blob (no ConsensusVersion bump; soft-compat path). - Persist supernode params with WithDefaults likewise (Everlight plus LEP-4). - Anchor SetLastDistributionHeight(ctx.BlockHeight()) so the first Everlight payout fires one PaymentPeriodBlocks AFTER upgrade height instead of on the very next block. - Call SupernodeKeeper.EnsureModuleAccount(ctx) so the supernode ModuleAccount carries the updated Minter+Burner+Staking permissions. - Burn StorageTruthEnforcementMode = SHADOW on audit params (LEP-6 activation default; explicit so it does not depend on WithDefaults promoting UNSPECIFIED). 3. Adds EnsureModuleAccount to the SupernodeKeeper expected-keeper interface and regenerates the mock. 4. Adds activation-policy constants everlightEnabled (true) and storageTruthEnforcementMode (SHADOW) with regression tests pinning them. No module ConsensusVersion is bumped: action and supernode changes are purely additive params with safe defaults reachable via WithDefaults. Audit v1 to v2 migration is unchanged and runs via RunMigrations. go build ./..., go vet ./..., and unit, integration, and system tests in tests/{integration,system}/* all pass.
Contributor
There was a problem hiding this comment.
Pull request overview
Completes the v1.12.0 upgrade handler so that post-v1.11.1-hotfix additive defaults and activation-time anchors are persisted immediately at upgrade height (not deferred until the next governance write), and updates the action params read/write path to normalize LEP-5 SVC defaults.
Changes:
- Extend the
v1.12.0upgrade handler to persist Action/Supernode/Audit defaults, anchor Everlight’s distribution clock, and refresh the Supernode module account. - Add
Params.WithDefaults()tox/actionand apply it on keeper param reads/writes, with unit tests. - Extend the
SupernodeKeeperexpected interface (and regenerated mocks) to exposeEnsureModuleAccountfor upgrade wiring.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
app/upgrades/v1_12_0/upgrade.go |
Persists default params at upgrade time; anchors Everlight last distribution height; ensures module account; sets audit enforcement mode. |
app/upgrades/v1_12_0/upgrade_test.go |
Pins activation-policy constants via unit tests. |
x/action/v1/types/params.go |
Adds Params.WithDefaults() for LEP-5 additive SVC fields. |
x/action/v1/keeper/params.go |
Applies WithDefaults() on params read and write. |
x/action/v1/types/params_with_defaults_test.go |
Tests WithDefaults() behavior for zero vs non-zero values. |
x/supernode/v1/types/expected_keepers.go |
Adds EnsureModuleAccount to SupernodeKeeper interface. |
x/supernode/v1/mocks/expected_keepers_mock.go |
Regenerated gomock outputs reflecting interface changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+105
to
+121
| // WithDefaults returns a copy of the params with any zero-value fields populated | ||
| // from module defaults. Older genesis blobs and on-chain params written before | ||
| // new fields existed (e.g. LEP-5 SVC params) read back as zero; reading via | ||
| // WithDefaults keeps behaviour consistent without requiring a ConsensusVersion | ||
| // bump for purely additive params with safe defaults. | ||
| // | ||
| // Note: WithDefaults does NOT call Validate. Callers that want strict | ||
| // 0-as-unset semantics (genesis init, MsgUpdateParams) should apply | ||
| // WithDefaults() before Validate(). | ||
| func (p Params) WithDefaults() Params { | ||
| if p.SvcChallengeCount == 0 { | ||
| p.SvcChallengeCount = DefaultSVCChallengeCount | ||
| } | ||
| if p.SvcMinChunksForChallenge == 0 { | ||
| p.SvcMinChunksForChallenge = DefaultSVCMinChunksForChallenge | ||
| } | ||
| return p |
Comment on lines
+76
to
+77
| // All four keepers are required by this handler. Fail loudly on any | ||
| // wiring regression rather than nil-deref mid-upgrade. |
Contributor
|
@roomote code review |
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.
What & Why
The v1.12.0 upgrade handler previously ran module migrations only. Several PRs merged after
v1.11.1-hotfixintroduced new params and per-chain anchors that were not persisted at activation:x/actionSvcChallengeCount,SvcMinChunksForChallengex/supernodeRewardDistribution{...}, full LEP-4 metrics block, new state keys (LastDistributionHeight,TotalDistributed,SNDistState), module-account perms nowMinter+Burner+Stakingx/auditStorageTruth*params block; ConsensusVersion 1→2 with registered migrationKeepLastEpochEntries; ❌ enforcement mode not explicitly burnedSymptoms without this PR:
lumerad q action paramsandlumerad q supernode paramsreturn zeros on the new fields until next governance write.lastDistHeight=0andcurrentHeight - 0 ≥ PaymentPeriodBlocksfor any chain past 100,800 blocks — almost certainly not the intended activation policy.WithDefaultspromotion, which intentionally does NOT promoteUNSPECIFIEDtoSHADOW.Changes
app/upgrades/v1_12_0/upgrade.go— completed handlerconsensusparams.EnsurePresent(defensive, matches v1.10.1/v1.11.x pattern).RunMigrations(audit v1→v2 fires here; bumpsKeepLastEpochEntriesfloor).ActionKeeper.SetParams(ctx, ActionKeeper.GetParams(ctx))— burns LEP-5 defaults to disk via the newWithDefaultspath.SupernodeKeeper.SetParams(ctx, GetParams(ctx).WithDefaults())— burns Everlight + LEP-4 defaults.SupernodeKeeper.SetLastDistributionHeight(ctx, ctx.BlockHeight())— anchors Everlight clock at upgrade height. First payout fires onePaymentPeriodBlocksafter upgrade, not on the next block.SupernodeKeeper.EnsureModuleAccount(ctx)— refreshes ModuleAccount withMinter+Burner+Stakingperms.audit.StorageTruthEnforcementMode = SHADOWexplicitly (matches v1.11.0'sauditScEnabled = trueprecedent).Activation-policy constants (with regression tests pinning them):
x/action/v1/types/params.goandkeeper/params.goParams.WithDefaults()(sibling-asymmetry fix — supernode and audit already had this since v1.11.x).keeper.GetParamsappliesWithDefaults()on read;keeper.SetParamson write.Params.SvcChallengeCountno longer see0post-upgrade. (Previously masked by the ad-hoc fallback inkeeper.GetSVCParams.)x/supernode/v1/types/expected_keepers.go(+ regenerated mock)EnsureModuleAccount(ctx sdk.Context)to theSupernodeKeeperinterface so the upgrade handler can call it throughappParams.AppUpgradeParams.SupernodeKeeper.go generatere-ran cleanly onexpected_keepers_mock.go.What is intentionally NOT done
ConsensusVersionbump for action or supernode. Both modules' changes are purely additive params with safe zero-default semantics; bumping would force a near-empty per-module migration with the same effect as the in-handlerSetParams. Lumera precedent (this PR's behaviour) follows v1.11.1's auditmin_disk_free_percentfloor pattern.StoreUpgrades.Added/Deleted: Everlight uses the existingsupernodestore; LEP-6 uses the existingauditstore.Risks
SetParamson a module already at correct values is a wasted writelastDistHeight = upgradeHeightdelays first payoutEnsureModuleAccountno-ops if account already correctWithDefaultson action params changes runtime semantics for callers readingParamsdirectlykeeper.GetSVCParamsalready returned via fallback; verified allSvcChallengeCount/SvcMinChunksForChallengeconsumersRollback
Revert this commit and
lumerad rollback. No store schema change, no migration replay required.Tests
app/upgrades/v1_12_0/upgrade_test.go— pins activation constants (everlightEnabled, enforcement mode) plus existing semver / store-safety / handler-non-nil tests.x/action/v1/types/params_with_defaults_test.go— new:WithDefaultsfills zero SVC fields and preserves non-zero values.go build ./...✅go vet ./...✅app/upgrades/...,x/action/...,x/supernode/...,x/audit/...✅tests/integration/{action,audit,bank,everlight,gov,staking,supernode,wasm}andtests/system/{action,audit,supernode,wasm}✅Devnet rehearsal
Recommend a
lumera-upgrade-rehearsalrunv1.11.1 → v1.12.0andv1.10.1 → v1.12.0before mainnet — the handler is keeper-heavy and the existingapp/upgrades/upgrades_test.go::TestSetupUpgradesAndHandlerssmoke test cannot exercise it (skipped, same as v1.11.0/v1.11.1).