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

v4/upgrade handler #384

Closed
wants to merge 12 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -1002,6 +1002,8 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino
paramsKeeper.Subspace(icacallbacksmoduletypes.ModuleName)
// this line is used by starport scaffolding # stargate/app/paramSubspace

paramsKeeper.Subspace(claimtypes.ModuleName)
paramsKeeper.Subspace(authz.ModuleName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need it to initialize params store for modules

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unlike other data, module params are stored differently. The prefixed subspace of the parameter store is manage by params module. Some modules need their params private for another, we can modifier it

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, thanks for the explanation!

return paramsKeeper
}

Expand Down
19 changes: 19 additions & 0 deletions app/apptesting/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ package apptesting
import (
"strings"

abci "github.com/tendermint/tendermint/abci/types"
ibctransfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"

"github.com/cosmos/cosmos-sdk/baseapp"
sdk "github.com/cosmos/cosmos-sdk/types"
minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
icatypes "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/types"
transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
Expand Down Expand Up @@ -311,3 +313,20 @@ func (s *AppTestHelper) ICS20PacketAcknowledgement() channeltypes.Acknowledgemen
ack := channeltypes.NewResultAcknowledgement(s.MarshalledICS20PacketData())
return ack
}

func (s *AppTestHelper) ConfirmUpgradeSucceededs(upgradeName string, upgradeHeight int64) {
contextBeforeUpgrade := s.Ctx().WithBlockHeight(upgradeHeight - 1)
contextAtUpgrade := s.Ctx().WithBlockHeight(upgradeHeight)

plan := upgradetypes.Plan{Name: upgradeName, Height: upgradeHeight}
err := s.App.UpgradeKeeper.ScheduleUpgrade(contextBeforeUpgrade, plan)
s.Require().NoError(err)

plan, exists := s.App.UpgradeKeeper.GetUpgradePlan(contextBeforeUpgrade)
s.Require().True(exists)

s.Require().NotPanics(func() {
beginBlockRequest := abci.RequestBeginBlock{}
s.App.BeginBlocker(contextAtUpgrade, beginBlockRequest)
})
}
12 changes: 12 additions & 0 deletions app/upgrades.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import (

v2 "github.com/Stride-Labs/stride/v3/app/upgrades/v2"
v3 "github.com/Stride-Labs/stride/v3/app/upgrades/v3"
v4 "github.com/Stride-Labs/stride/v3/app/upgrades/v4"
claimtypes "github.com/Stride-Labs/stride/v3/x/claim/types"
authzkeeper "github.com/cosmos/cosmos-sdk/x/authz/keeper"
)

func (app *StrideApp) setupUpgradeHandlers() {
Expand All @@ -24,6 +26,12 @@ func (app *StrideApp) setupUpgradeHandlers() {
v3.CreateUpgradeHandler(app.mm, app.configurator, app.ClaimKeeper),
)

// v4 upgrade handler
app.UpgradeKeeper.SetUpgradeHandler(
v4.UpgradeName,
v4.CreateUpgradeHandler(app.mm, app.configurator),
)

upgradeInfo, err := app.UpgradeKeeper.ReadUpgradeInfoFromDisk()
if err != nil {
panic(fmt.Errorf("Failed to read upgrade info from disk: %w", err))
Expand All @@ -41,6 +49,10 @@ func (app *StrideApp) setupUpgradeHandlers() {
storeUpgrades = &storetypes.StoreUpgrades{
Added: []string{claimtypes.StoreKey},
}
case "v4":
storeUpgrades = &storetypes.StoreUpgrades{
Added: []string{authzkeeper.StoreKey},
}
}

if storeUpgrades != nil {
Expand Down
64 changes: 64 additions & 0 deletions app/upgrades/v3/upgrades_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package v3_test

import (
"fmt"
"testing"

"github.com/stretchr/testify/suite"

"github.com/Stride-Labs/stride/v3/app/apptesting"
)
var (
airdropIdentifiers = []string{"stride", "gaia", "osmosis", "juno", "stars"}
)
const dummyUpgradeHeight = 5

type UpgradeTestSuite struct {
apptesting.AppTestHelper
}

func (s *UpgradeTestSuite) SetupTest() {
s.Setup()
}

func TestKeeperTestSuite(t *testing.T) {
suite.Run(t, new(UpgradeTestSuite))
}

func (suite *UpgradeTestSuite) TestUpgrade() {
testCases := []struct {
msg string
preUpdate func()
update func()
postUpdate func()
expPass bool
}{
{
"Test that upgrade does not panic",
func() {
suite.Setup()
},
func() {
suite.ConfirmUpgradeSucceededs("v3", dummyUpgradeHeight)

// make sure claim record was set
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any way to verify authz was properly initialized here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to export module genesis then compare it with defaultGenState. Believe we do not modify the init state

Copy link
Contributor

Choose a reason for hiding this comment

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

that is what my new computer is for.... I'm going to export-upgrade -- and offer that to every chain that Notional validates before upgrades via ci.

Copy link
Contributor

Choose a reason for hiding this comment

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

Notional consistently proving your value

afterCtx := suite.Ctx().WithBlockHeight(dummyUpgradeHeight)
for _, identifier := range(airdropIdentifiers) {
claimRecords := suite.App.ClaimKeeper.GetClaimRecords(afterCtx, identifier)
suite.Require().NotEqual(0, len(claimRecords))
}
},
func() {
},
true,
},
}

for _, tc := range testCases {
suite.Run(fmt.Sprintf("Case %s", tc.msg), func() {
tc.preUpdate()
tc.update()
tc.postUpdate()
})
}
}
6 changes: 6 additions & 0 deletions app/upgrades/v4/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Upgrade v4 Changelog

1. Add authz storeKey to StoreLoader
2. Add authz & claim modules to params subspaces


27 changes: 27 additions & 0 deletions app/upgrades/v4/upgrades.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package v4

import (

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
)

// Note: ensure these values are properly set before running upgrade
var (
UpgradeName = "v4"
)

// CreateUpgradeHandler creates an SDK upgrade handler for v3
func CreateUpgradeHandler(
mm *module.Manager,
configurator module.Configurator,
) upgradetypes.UpgradeHandler {
return func(ctx sdk.Context, _ upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
newVm, err := mm.RunMigrations(ctx, configurator, vm)
if err != nil {
return newVm, err
}
return newVm, nil
}
}
63 changes: 63 additions & 0 deletions app/upgrades/v4/upgrades_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package v4_test

import (
"fmt"
"testing"

"github.com/stretchr/testify/suite"

"github.com/Stride-Labs/stride/v3/app/apptesting"
authz "github.com/cosmos/cosmos-sdk/x/authz"
)

const dummyUpgradeHeight = 5

type UpgradeTestSuite struct {
apptesting.AppTestHelper
}

func (s *UpgradeTestSuite) SetupTest() {
s.Setup()
}

func TestKeeperTestSuite(t *testing.T) {
suite.Run(t, new(UpgradeTestSuite))
}

func (suite *UpgradeTestSuite) TestUpgrade() {
testCases := []struct {
msg string
preUpdate func()
update func()
postUpdate func()
expPass bool
}{
{
"Test that upgrade does not panic",
func() {
suite.Setup()
},
func() {
suite.ConfirmUpgradeSucceededs("v4", dummyUpgradeHeight)

// make sure authz module was init
afterCtx := suite.Ctx().WithBlockHeight(dummyUpgradeHeight)
actGenState := suite.App.AuthzKeeper.ExportGenesis(afterCtx)
expGenState := authz.DefaultGenesisState()
suite.Require().NotNil(actGenState)
suite.Require().Equal(&expGenState, &actGenState)
},
func() {
},
true,
},
}

for _, tc := range testCases {
suite.Run(fmt.Sprintf("Case %s", tc.msg), func() {
tc.preUpdate()
tc.update()
tc.postUpdate()
})
}
}