Fix - Upgrade Handler 1.0 Lacks Idempotency for Dev Credit Minting#107
Open
ashishchandr70 wants to merge 13 commits intorelease/1.0from
Open
Fix - Upgrade Handler 1.0 Lacks Idempotency for Dev Credit Minting#107ashishchandr70 wants to merge 13 commits intorelease/1.0from
ashishchandr70 wants to merge 13 commits intorelease/1.0from
Conversation
* Upgrade to 1.0 and fix gosec issues * Added acltypes.StoreKey to registered upgrade handlers * Added more store upgrades * Added remaining custom stores to storeUpgrades * Removing ccvprovider types store key * Changed genesis module init order * use non-consumer modules * WIP for handling the InitGenesis related error * manually initialize provider module (#81) * Renamed upgrade to match go proposal passed in staging * Renamed upgrade back to 1.0 --------- Co-authored-by: Ashish Chandra <ashish@saga.xyz> Co-authored-by: Brian Luk <brian6.dev@gmail.com> Co-authored-by: Brian <45702419+lukitsbrian@users.noreply.github.com>
The setup fee in LaunchChainlet() was added to deposit without capturing the returned coin, so the escrow account was funded with only the epoch deposit. This fix assigns the result of deposit.Add(setupfee) back to deposit before calling NewChainletAccount.
* use platform validator set for chainlet token distributions * Add fallback to staking validators when no platform validators configured - If PlatformValidators param is empty, fall back to GetValidators() - Add guard against division by zero if no validators exist - Fixes potential panic when PlatformValidators is not set * Fix lint errors in billing module * Fix params test: use nil instead of empty slice for PlatformValidators default
* fix: complete genesis export/import for all modules Multiple custom modules only serialized params in ExportGenesis, leaving their keeper KV state out of every exported genesis. This caused data loss during export/import upgrades, state sync bootstraps, or genesis restarts. Changes: - x/chainlet: Export/import chainlets, chainlet stacks, and chainlet count - x/escrow: Export/import chainlet accounts, denomination pools, and funders - x/billing: Export/import billing history and validator payout history - x/peers: Export/import peer data and chain counters For each module: - Extended genesis protobuf to include full KV store contents - Updated ExportGenesis to iterate store prefixes and serialize data - Updated InitGenesis to rehydrate from serialized data - Added validation for duplicate entries - Updated tests to cover new fields and validation
# Conflicts: # x/chainlet/keeper/ibc.go
Contributor
|
Why would we trigger the upgrade multiple times? |
Contributor
Author
In normal course, we would not. But in case it gets triggered more than once, this scenario comes into play. It is a precautionary PR. |
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
3 participants
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.
Problem
The upgrade handler in app/upgrades/1.0/upgrades.go (section 5, lines 131-162) is not idempotent. The "One-shot dev credit mint + cleanup" section can execute multiple times if:
Impact
Recommended Solution
Add an idempotency check at the start of section 5:
This ensures: