Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: update x/upgrade keeper.DumpUpgradeInfoToDisk to support Plan.Info #10532

Merged
merged 3 commits into from Nov 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -37,6 +37,9 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

### Improvements
* (x/upgrade) [\#10532](https://github.com/cosmos/cosmos-sdk/pull/10532) Add `keeper.DumpUpgradeInfoWithInfoToDisk` to include `Plan.Info` in the upgrade-info file.

### Bug Fixes

* [\#10414](https://github.com/cosmos/cosmos-sdk/pull/10414) Use `sdk.GetConfig().GetFullBIP44Path()` instead `sdk.FullFundraiserPath` to generate key
Expand Down
2 changes: 1 addition & 1 deletion x/upgrade/abci.go
Expand Up @@ -42,7 +42,7 @@ func BeginBlocker(k keeper.Keeper, ctx sdk.Context, _ abci.RequestBeginBlock) {
if !k.HasHandler(plan.Name) {
// Write the upgrade info to disk. The UpgradeStoreLoader uses this info to perform or skip
// store migrations.
err := k.DumpUpgradeInfoToDisk(ctx.BlockHeight(), plan.Name)
err := k.DumpUpgradeInfoWithInfoToDisk(ctx.BlockHeight(), plan.Name, plan.Info)
if err != nil {
panic(fmt.Errorf("unable to write upgrade info to filesystem: %s", err.Error()))
}
Expand Down
30 changes: 26 additions & 4 deletions x/upgrade/keeper/keeper.go
Expand Up @@ -325,23 +325,35 @@ func (k Keeper) IsSkipHeight(height int64) bool {
return k.skipUpgradeHeights[height]
}

// DumpUpgradeInfoToDisk writes upgrade information to UpgradeInfoFileName.
// DumpUpgradeInfoToDisk writes upgrade information to UpgradeInfoFileName. The function
// doesn't save the `Plan.Info` data, hence it won't support auto download functionality
// by cosmvisor.
// NOTE: this function will be update in the next release.
func (k Keeper) DumpUpgradeInfoToDisk(height int64, name string) error {
return k.DumpUpgradeInfoWithInfoToDisk(height, name, "")
}

// Deprecated: DumpUpgradeInfoWithInfoToDisk writes upgrade information to UpgradeInfoFileName.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

adding a new function which is deprecated

Choose a reason for hiding this comment

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

Ok

// `info` should be provided and contain Plan.Info data in order to support
// auto download functionality by cosmovisor and other tools using upgarde-info.json
// (GetUpgradeInfoPath()) file.
func (k Keeper) DumpUpgradeInfoWithInfoToDisk(height int64, name string, info string) error {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I hate this kind of names.... but couldn't find better one

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like it either lol. What about *WithPlan? Also, why is a new function being introduced and marked as deprecated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The whole thing is a Plan (name, height, info...).
In original solution I overloaded the DumpUpgradeInfoToDisk using additional vararg (that made it backward compatible). I agree with Aaron, that overloading doesn't smell good. see: #10532 (comment)

upgradeInfoFilePath, err := k.GetUpgradeInfoPath()
if err != nil {
return err
}

upgradeInfo := store.UpgradeInfo{
upgradeInfo := upgradeInfo{
Name: name,
Height: height,
Info: info,
}
info, err := json.Marshal(upgradeInfo)
bz, err := json.Marshal(upgradeInfo)
if err != nil {
return err
}

return ioutil.WriteFile(upgradeInfoFilePath, info, 0600)
return ioutil.WriteFile(upgradeInfoFilePath, bz, 0600)
}

// GetUpgradeInfoPath returns the upgrade info file path
Expand Down Expand Up @@ -388,3 +400,13 @@ func (k Keeper) ReadUpgradeInfoFromDisk() (store.UpgradeInfo, error) {

return upgradeInfo, nil
}

// upgradeInfo is stripped types.Plan structure used to dump upgrade plan data.
type upgradeInfo struct {
// Name has types.Plan.Name value
Name string `json:"name,omitempty"`
// Height has types.Plan.Height value
Height int64 `json:"height,omitempty"`
// Height has types.Plan.Height value
Info string `json:"info,omitempty"`
}