Skip to content

Commit

Permalink
Port double upgrade fix (#9694)
Browse files Browse the repository at this point in the history
closes: #9682

## Description
Ports the change from #9683 to master, adding some extra checks since master has a more complex upgrade name structure.
Update some docs following the refactor in #9575

### Security Considerations
None

### Scaling Considerations
None

### Documentation Considerations
None

### Testing Considerations
This PR originally added an a3p-integration test, but it cannot work without modification to the agd binary just for the purpose of testing this (if the original binary already contains support for the upgrade name, it attempts an upgrade in place, which fails as we don't intend to support this)

### Upgrade Considerations
Ensure we have the ability to release and apply multiple release candidates for the same version.
  • Loading branch information
mergify[bot] committed Aug 31, 2024
2 parents f6011d0 + d5c3380 commit 074c43c
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 16 deletions.
2 changes: 1 addition & 1 deletion MAINTAINERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ The Release Owner and other appropriate stakeholders must agree on:

- [ ] When a new release is planned, create a new branch from branch `release-mainnet1B` with a name like `dev-$releaseShortLabel` (example: `dev-upgrade-8`). This can be done from the command line or the [GitHub Branches UI](https://github.com/Agoric/agoric-sdk/branches).
- [ ] Initialize the new branch for the planned upgrade:
- [ ] In **golang/cosmos/app/app.go**, update the `upgradeName` constants and the associated upgrade handler function name to correspond with the [_**upgrade name**_](#assign-release-parameters).
- [ ] In **golang/cosmos/app/upgrade.go**, update the `upgradeName` constants and the associated upgrade handler function name to correspond with the [_**upgrade name**_](#assign-release-parameters).
Remove from the function any logic specific to the previous upgrade (e.g., core proposals).
- [ ] Ensure that **a3p-integration/package.json** has an object-valued `agoricSyntheticChain` property with `fromTag` set to the [agoric-3-proposals Docker images](https://github.com/Agoric/agoric-3-proposals/pkgs/container/agoric-3-proposals) tag associated with the previous release
(example: `use-upgrade-7`).
Expand Down
2 changes: 1 addition & 1 deletion a3p-integration/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ For a chain software upgrade proposal, the `type` is `"Software Upgrade Proposal
particular, it can have a `coreProposals` field which instructs the chain
software to run other core proposals in addition to the one configured in the
chain software's upgrade handler (see `CoreProposalSteps` in
`/golang/cosmos/app/app.go`).
`/golang/cosmos/app/upgrade.go`).
- See **Generating core-eval submissions** below for details.

For an (evolving) example, see `a:upgrade-next` in master.
Expand Down
11 changes: 7 additions & 4 deletions a3p-integration/proposals/e:upgrade-next/README.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
# Proposal to upgrade the chain software

The `UNRELEASED_A3P_INTEGRATION` software upgrade may include core proposals defined in
its upgrade handler. See `CoreProposalSteps` in the `unreleasedUpgradeHandler`
in [golang/cosmos/app/app.go](../../../golang/cosmos/app/app.go).
The `UNRELEASED_A3P_INTEGRATION` software upgrade may include core proposals
defined in its upgrade handler. See `CoreProposalSteps` in the
`unreleasedUpgradeHandler` in
[golang/cosmos/app/upgrade.go](../../../golang/cosmos/app/upgrade.go).

This test proposal may also include `coreProposals` in its `upgradeInfo`, which
are executed after those defined in that upgrade handler. See `agoricProposal`
in [package.json](./package.json).

The "binaries" property of `upgradeInfo` is now required since Cosmos SDK 0.46, however it cannot be computed for an unreleased upgrade. To disable the check, `releaseNotes` is set to `false`.
The "binaries" property of `upgradeInfo` is now required since Cosmos SDK 0.46,
however it cannot be computed for an unreleased upgrade. To disable the check,
`releaseNotes` is set to `false`.
9 changes: 7 additions & 2 deletions golang/cosmos/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -873,18 +873,23 @@ func NewAgoricApp(
app.SetBeginBlocker(app.BeginBlocker)
app.SetEndBlocker(app.EndBlocker)

for name := range upgradeNamesOfThisVersion {
for _, name := range upgradeNamesOfThisVersion {
app.UpgradeKeeper.SetUpgradeHandler(
name,
unreleasedUpgradeHandler(app, name),
)
}

// At this point we don't have a way to read from the store, so we have to
// rely on data saved by the x/upgrade module in the previous software.
upgradeInfo, err := app.UpgradeKeeper.ReadUpgradeInfoFromDisk()
if err != nil {
panic(err)
}
if upgradeNamesOfThisVersion[upgradeInfo.Name] && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) {
// Store migrations can only run once, so we use a notion of "primary upgrade
// name" to trigger them. Testnets may end up upgrading from one rc to
// another, which shouldn't re-run store upgrades.
if isPrimaryUpgradeName(upgradeInfo.Name) && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) {
storeUpgrades := storetypes.StoreUpgrades{
Added: []string{},
Deleted: []string{},
Expand Down
67 changes: 61 additions & 6 deletions golang/cosmos/app/upgrade.go
Original file line number Diff line number Diff line change
@@ -1,22 +1,70 @@
package gaia

import (
"fmt"

"github.com/Agoric/agoric-sdk/golang/cosmos/vm"
swingsetkeeper "github.com/Agoric/agoric-sdk/golang/cosmos/x/swingset/keeper"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
)

var upgradeNamesOfThisVersion = map[string]bool{
"UNRELEASED_BASIC": true, // no-frills
"UNRELEASED_A3P_INTEGRATION": true,
"UNRELEASED_main": true,
"UNRELEASED_devnet": true,
var upgradeNamesOfThisVersion = []string{
"UNRELEASED_BASIC", // no-frills
"UNRELEASED_A3P_INTEGRATION",
"UNRELEASED_main",
"UNRELEASED_devnet",
"UNRELEASED_REAPPLY",
}

// isUpgradeNameOfThisVersion returns whether the provided plan name is a
// known upgrade name of this software version
func isUpgradeNameOfThisVersion(name string) bool {
for _, upgradeName := range upgradeNamesOfThisVersion {
if upgradeName == name {
return true
}
}
return false
}

// validUpgradeName is an identity function that asserts the provided name
// is an upgrade name of this software version. It can be used as a sort of
// dynamic enum check.
func validUpgradeName(name string) string {
if !isUpgradeNameOfThisVersion(name) {
panic(fmt.Errorf("invalid upgrade name: %s", name))
}
return name
}

// isPrimaryUpgradeName returns wether the provided plan name is considered a
// primary for the purpose of applying store migrations for the first upgrade
// of this version.
// It is expected that only primary plan names are used for non testing chains.
func isPrimaryUpgradeName(name string) bool {
if name == "" {
// An empty upgrade name can happen if there are no upgrade in progress
return false
}
switch name {
case validUpgradeName("UNRELEASED_BASIC"),
validUpgradeName("UNRELEASED_A3P_INTEGRATION"),
validUpgradeName("UNRELEASED_main"),
validUpgradeName("UNRELEASED_devnet"):
return true
case validUpgradeName("UNRELEASED_REAPPLY"):
return false
default:
panic(fmt.Errorf("unexpected upgrade name %s", validUpgradeName(name)))
}
}

// isFirstTimeUpgradeOfThisVersion looks up in the upgrade store whether no
// upgrade plan name of this version have previously been applied.
func isFirstTimeUpgradeOfThisVersion(app *GaiaApp, ctx sdk.Context) bool {
for name := range upgradeNamesOfThisVersion {
for _, name := range upgradeNamesOfThisVersion {
if app.UpgradeKeeper.GetDoneHeight(ctx, name) != 0 {
return false
}
Expand All @@ -34,6 +82,13 @@ func unreleasedUpgradeHandler(app *GaiaApp, targetUpgrade string) func(sdk.Conte
// These CoreProposalSteps are not idempotent and should only be executed
// as part of the first upgrade using this handler on any given chain.
if isFirstTimeUpgradeOfThisVersion(app, ctx) {
// The storeUpgrades defined in app.go only execute for the primary upgrade name
// If we got here and this first upgrade of this version does not use the
// primary upgrade name, stores have not been initialized correctly.
if !isPrimaryUpgradeName(plan.Name) {
return module.VersionMap{}, fmt.Errorf("cannot run %s as first upgrade", plan.Name)
}

// Each CoreProposalStep runs sequentially, and can be constructed from
// one or more modules executing in parallel within the step.
CoreProposalSteps = []vm.CoreProposalStep{
Expand Down
3 changes: 1 addition & 2 deletions packages/deploy-script-support/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ objects.
The script describes how to build the core proposal. For
`agoric-3-proposals` and uploading to the chain, the script must be named in the
`CoreProposalSteps` section in
[`app.go`](https://github.com/Agoric/agoric-sdk/blob/b13743a2cccf0cb63a412b54384435596d4e81ea/golang/cosmos/app/app.go#L881),
`CoreProposalSteps` section in [`upgrade.go`](../../golang/cosmos/app/upgrade.go),
and its `defaultProposalBuilder` will be invoked directly.
Script files should export `defaultProposalBuilder` and a `default` function
Expand Down

0 comments on commit 074c43c

Please sign in to comment.