[Feature]: Have multiple upgrades in upgrade-info.json and support it in Cosmovisor

[freak12techno]

Summary

It would be lovely to have upgrade-info.json as an array of objects instead of a single object, storing the info on multiple upcoming upgrades.

Problem Definition

Imagine a life of a person maintaining an archive node and willing to sync it from scratch, and a case when there are multiple consequent non-governance upgrade via halt-height. For simplicity let's say there's a chain which only had 2 halt-height upgrades and 0 governance ones.
Here's the sequence of steps that a person willing to sync an archive node from scratch should do (with Cosmovisor latest version:

1. prepare the binary for the first upgrade
2. use Cosmovisor's add-upgrade command with --height
3. wait for the height to be reached and the binary auto-replaced
4. do steps 2 and 3 for the second upgrade
5. wait till it's applied.

If this person forgets to do step 4 or fails to do so because of any reason (for example, the window between two upgrades being too small), then their node would result in AppHash error due to consensus mismatch.

Therefore, it'd be nice to be able to allow to stage multiple upgrades on upgrade-info.json.
As a result of this, a node operator would be able to build and stage all upgrades at once and then just chill and wait until the node is synced without any need to do anything extra during the sync process.

Proposed Feature

This is how I see it:

1. Changing the upgrade-info.json structure so that it'd be array of objects instead of a single object.
2. When adding an upgrade info, instead of overwriting this file we can read it as an array, push the new upgrade info then overwrite it again.
3. When applying the upgrade, probably remove its entry from the file.
4. Support this on Cosmovisor side.

[jdubpark]

Apps currently set store loaders like this:

for _, upgrade := range Upgrades {
    if upgradeInfo.Name == upgrade.UpgradeName {
        storeUpgrades := upgrade.StoreUpgrades
        app.SetStoreLoader(upgradetypes.UpgradeStoreLoader(upgradeInfo.Height, &storeUpgrades))
    }
}

which needs to be changed if we were to not restart the binary. We need to set the next upgrade's store loader after the previous upgrade is complete.

Additionally, we would need to modify the pre-mature handler check in the upgrade module

// if we have a pending upgrade, but it is not yet time, make sure we did not
// set the handler already
if k.HasHandler(plan.Name) {
    downgradeMsg := fmt.Sprintf("BINARY UPDATED BEFORE TRIGGER! UPGRADE \"%s\" - in binary but not executed on chain. Downgrade your binary", plan.Name)
    ctx.Logger().Error(downgradeMsg)
    panic(downgradeMsg)
}

to allow setting upgrade handlers before their upgrade heights (as an option).

[psiphi5]

Seeing as this is currently unassigned, I'd be willing to take this on.

[julienrbrt]

Changing the upgrade-info.json is an x/upgrade concern. This feature request doesn't seem to be an x/upgrade but mainly a Cosmovisor UX improvement.

I'd say we actually shouldn't change the upgrade-info.json but update cosmovisor to maybe read take multiple upgrade-info.json (maybe they should be annotated with a number, and cosmovisor read them in that order, or in another way i don't know)

[psiphi5]

Changing the upgrade-info.json is an x/upgrade concern. This feature request doesn't seem to be an x/upgrade but mainly a Cosmovisor UX improvement.

I'd say we actually shouldn't change the upgrade-info.json but update cosmovisor to maybe read take multiple upgrade-info.json (maybe they should be annotated with a number, and cosmovisor read them in that order, or in another way i don't know)

That makes sense to me. Would it be preferable to change the add-upgrade command to take a list of upgrades instead of just one:

add-upgrade <upgrade-name-1> <exec-path-1> <upgrade-name-2> <exec-path-2> ... <upgrade-name-n> <exec-path-n>

Or would it be better to add a new command entirely? Something like add-batch-upgrades?

[julienrbrt]

I like add-batch-upgrades personally, as it isn't something you always need, best to split the concerns 👍
Are you willing to implement it?

[psiphi5]

I like add-batch-upgrades personally, as it isn't something you always need, best to split the concerns 👍 Are you willing to implement it?

Yep, I'm willing to take this on.

[psiphi5]

I like add-batch-upgrades personally, as it isn't something you always need, best to split the concerns 👍 Are you willing to implement it?

Hello, I spent some time trying to map out a way to implement this feature and wrote some prototype code, here's how I see it:

• Add a new command with the format: add-batch-upgrade <upgrade1-name>:<path-to-exec1>:<upgrade1-height> <upgrade2-name>:<path-to-exec2>:<upgrade2-height> .. <upgradeN-name>:<path-to-execN>:<upgradeN-height>, this will loop through all the arguments and call the already-existing AddUpgrade function (this function would have to be modified a bit so that it can save the upgrade-info.json file in an arbitrary location along with other minor changes). Once all the individual files are written, they'll all be concatenated to form a single file upgrade-info.batch.json.
• Start a watcher goroutine when cosmovisor run is ran that'll read the batch upgrade file and will watch the current height and swap out the upgrade-info.json file with the next one at the appropriate time.