From 183f6158243eab4c216486de053a4af9b34594c6 Mon Sep 17 00:00:00 2001 From: Spoorthi Satheesha <9302666+spoo-bar@users.noreply.github.com> Date: Wed, 30 Aug 2023 20:06:11 +0530 Subject: [PATCH 1/7] passing account keeper to upgrade handlers --- app/app_upgrades.go | 2 +- app/app_upgrades_test.go | 3 ++- app/app_upgrades_util_test.go | 2 +- app/upgrades/06/upgrades.go | 3 ++- app/upgrades/1_0_0_rc_4/upgrades.go | 3 ++- app/upgrades/2_0_0/upgrades.go | 3 ++- app/upgrades/3_0_0/upgrades.go | 3 ++- app/upgrades/4_0_0/upgrades.go | 3 ++- app/upgrades/latest/upgrades.go | 3 ++- app/upgrades/upgrades.go | 3 ++- 10 files changed, 18 insertions(+), 10 deletions(-) diff --git a/app/app_upgrades.go b/app/app_upgrades.go index 15b9080e..036b3bc6 100644 --- a/app/app_upgrades.go +++ b/app/app_upgrades.go @@ -52,7 +52,7 @@ func (app *ArchwayApp) setUpgradeHandlers() { for _, u := range Upgrades { app.UpgradeKeeper.SetUpgradeHandler( u.UpgradeName, - u.CreateUpgradeHandler(app.mm, app.configurator), + u.CreateUpgradeHandler(app.mm, app.configurator, app.AccountKeeper), ) } } diff --git a/app/app_upgrades_test.go b/app/app_upgrades_test.go index e7cfb66a..08b1f96b 100644 --- a/app/app_upgrades_test.go +++ b/app/app_upgrades_test.go @@ -7,6 +7,7 @@ import ( storeTypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" + "github.com/cosmos/cosmos-sdk/x/auth/keeper" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" "github.com/stretchr/testify/require" @@ -44,7 +45,7 @@ func TestUpgrades(t *testing.T) { proposalExecuted := false fauxUpgrade := upgrades.Upgrade{ UpgradeName: "test-upgrade", - CreateUpgradeHandler: func(manager *module.Manager, configurator module.Configurator) upgradetypes.UpgradeHandler { + CreateUpgradeHandler: func(manager *module.Manager, configurator module.Configurator, _ keeper.AccountKeeper) upgradetypes.UpgradeHandler { return func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { proposalExecuted = true return manager.RunMigrations(ctx, configurator, fromVM) diff --git a/app/app_upgrades_util_test.go b/app/app_upgrades_util_test.go index 3a3ba197..bed7eccd 100644 --- a/app/app_upgrades_util_test.go +++ b/app/app_upgrades_util_test.go @@ -7,6 +7,6 @@ import "github.com/archway-network/archway/app/upgrades" func (app *ArchwayApp) AddUpgradeHandler(upgrade upgrades.Upgrade) { app.UpgradeKeeper.SetUpgradeHandler( upgrade.UpgradeName, - upgrade.CreateUpgradeHandler(app.mm, app.configurator), + upgrade.CreateUpgradeHandler(app.mm, app.configurator, app.AccountKeeper), ) } diff --git a/app/upgrades/06/upgrades.go b/app/upgrades/06/upgrades.go index 64400d80..df9042f9 100644 --- a/app/upgrades/06/upgrades.go +++ b/app/upgrades/06/upgrades.go @@ -4,6 +4,7 @@ import ( storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" + "github.com/cosmos/cosmos-sdk/x/auth/keeper" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" ibcfeetypes "github.com/cosmos/ibc-go/v4/modules/apps/29-fee/types" @@ -14,7 +15,7 @@ const Name = "v0.6.0" var Upgrade = upgrades.Upgrade{ UpgradeName: Name, - CreateUpgradeHandler: func(mm *module.Manager, cfg module.Configurator) upgradetypes.UpgradeHandler { + CreateUpgradeHandler: func(mm *module.Manager, cfg module.Configurator, _ keeper.AccountKeeper) upgradetypes.UpgradeHandler { return func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { return mm.RunMigrations(ctx, cfg, fromVM) } diff --git a/app/upgrades/1_0_0_rc_4/upgrades.go b/app/upgrades/1_0_0_rc_4/upgrades.go index d38055ce..4fb3fe65 100644 --- a/app/upgrades/1_0_0_rc_4/upgrades.go +++ b/app/upgrades/1_0_0_rc_4/upgrades.go @@ -4,6 +4,7 @@ import ( storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" + "github.com/cosmos/cosmos-sdk/x/auth/keeper" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" "github.com/archway-network/archway/app/upgrades" @@ -13,7 +14,7 @@ const Name = "v1.0.0-rc.4" var Upgrade = upgrades.Upgrade{ UpgradeName: Name, - CreateUpgradeHandler: func(mm *module.Manager, cfg module.Configurator) upgradetypes.UpgradeHandler { + CreateUpgradeHandler: func(mm *module.Manager, cfg module.Configurator, _ keeper.AccountKeeper) upgradetypes.UpgradeHandler { return func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { return mm.RunMigrations(ctx, cfg, fromVM) } diff --git a/app/upgrades/2_0_0/upgrades.go b/app/upgrades/2_0_0/upgrades.go index 28b383d2..20235670 100644 --- a/app/upgrades/2_0_0/upgrades.go +++ b/app/upgrades/2_0_0/upgrades.go @@ -5,6 +5,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" + "github.com/cosmos/cosmos-sdk/x/auth/keeper" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" @@ -23,7 +24,7 @@ const Name = "v2.0.0" var Upgrade = upgrades.Upgrade{ UpgradeName: Name, - CreateUpgradeHandler: func(mm *module.Manager, cfg module.Configurator) upgradetypes.UpgradeHandler { + CreateUpgradeHandler: func(mm *module.Manager, cfg module.Configurator, _ keeper.AccountKeeper) upgradetypes.UpgradeHandler { return func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { // Set Initial Consensus Version diff --git a/app/upgrades/3_0_0/upgrades.go b/app/upgrades/3_0_0/upgrades.go index fedd63f5..0e9f4f61 100644 --- a/app/upgrades/3_0_0/upgrades.go +++ b/app/upgrades/3_0_0/upgrades.go @@ -4,6 +4,7 @@ import ( storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" + "github.com/cosmos/cosmos-sdk/x/auth/keeper" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" "github.com/archway-network/archway/app/upgrades" @@ -13,7 +14,7 @@ const Name = "v3.0.0" var Upgrade = upgrades.Upgrade{ UpgradeName: Name, - CreateUpgradeHandler: func(mm *module.Manager, cfg module.Configurator) upgradetypes.UpgradeHandler { + CreateUpgradeHandler: func(mm *module.Manager, cfg module.Configurator, _ keeper.AccountKeeper) upgradetypes.UpgradeHandler { return func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { return mm.RunMigrations(ctx, cfg, fromVM) } diff --git a/app/upgrades/4_0_0/upgrades.go b/app/upgrades/4_0_0/upgrades.go index 8424c179..3598ad19 100644 --- a/app/upgrades/4_0_0/upgrades.go +++ b/app/upgrades/4_0_0/upgrades.go @@ -4,6 +4,7 @@ import ( storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" + "github.com/cosmos/cosmos-sdk/x/auth/keeper" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" "github.com/archway-network/archway/app/upgrades" @@ -13,7 +14,7 @@ const Name = "v4.0.0" var Upgrade = upgrades.Upgrade{ UpgradeName: Name, - CreateUpgradeHandler: func(mm *module.Manager, cfg module.Configurator) upgradetypes.UpgradeHandler { + CreateUpgradeHandler: func(mm *module.Manager, cfg module.Configurator, _ keeper.AccountKeeper) upgradetypes.UpgradeHandler { return func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { return mm.RunMigrations(ctx, cfg, fromVM) } diff --git a/app/upgrades/latest/upgrades.go b/app/upgrades/latest/upgrades.go index 58b1d608..98ea9092 100644 --- a/app/upgrades/latest/upgrades.go +++ b/app/upgrades/latest/upgrades.go @@ -4,6 +4,7 @@ import ( storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" + "github.com/cosmos/cosmos-sdk/x/auth/keeper" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" "github.com/archway-network/archway/app/upgrades" @@ -15,7 +16,7 @@ const Name = "latest" var Upgrade = upgrades.Upgrade{ UpgradeName: Name, - CreateUpgradeHandler: func(mm *module.Manager, cfg module.Configurator) upgradetypes.UpgradeHandler { + CreateUpgradeHandler: func(mm *module.Manager, cfg module.Configurator, appKeepers keeper.AccountKeeper) upgradetypes.UpgradeHandler { return func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { return mm.RunMigrations(ctx, cfg, fromVM) } diff --git a/app/upgrades/upgrades.go b/app/upgrades/upgrades.go index 07e6ca81..38d330d4 100644 --- a/app/upgrades/upgrades.go +++ b/app/upgrades/upgrades.go @@ -3,6 +3,7 @@ package upgrades import ( "github.com/cosmos/cosmos-sdk/store/types" "github.com/cosmos/cosmos-sdk/types/module" + "github.com/cosmos/cosmos-sdk/x/auth/keeper" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" ) @@ -15,7 +16,7 @@ type Upgrade struct { UpgradeName string // CreateUpgradeHandler defines the function that creates an upgrade handler - CreateUpgradeHandler func(*module.Manager, module.Configurator) upgradetypes.UpgradeHandler + CreateUpgradeHandler func(*module.Manager, module.Configurator, keeper.AccountKeeper) upgradetypes.UpgradeHandler // Store upgrades, should be used for any new modules introduced, new modules deleted, or store names renamed. StoreUpgrades types.StoreUpgrades From 91ca9dcd1a316ee5c1b3af6db8527f33e3ddf2e5 Mon Sep 17 00:00:00 2001 From: Spoorthi Satheesha <9302666+spoo-bar@users.noreply.github.com> Date: Wed, 30 Aug 2023 20:49:25 +0530 Subject: [PATCH 2/7] adding the upgrade handler to provide burn permission to feecollector module account --- app/upgrades/latest/upgrades.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/app/upgrades/latest/upgrades.go b/app/upgrades/latest/upgrades.go index 98ea9092..ac80bfc6 100644 --- a/app/upgrades/latest/upgrades.go +++ b/app/upgrades/latest/upgrades.go @@ -1,10 +1,13 @@ package upgradelatest import ( + "fmt" + storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" "github.com/cosmos/cosmos-sdk/x/auth/keeper" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" "github.com/archway-network/archway/app/upgrades" @@ -16,8 +19,16 @@ const Name = "latest" var Upgrade = upgrades.Upgrade{ UpgradeName: Name, - CreateUpgradeHandler: func(mm *module.Manager, cfg module.Configurator, appKeepers keeper.AccountKeeper) upgradetypes.UpgradeHandler { + CreateUpgradeHandler: func(mm *module.Manager, cfg module.Configurator, accountKeeper keeper.AccountKeeper) upgradetypes.UpgradeHandler { return func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { + fcAccount := accountKeeper.GetModuleAccount(ctx, authtypes.FeeCollectorName) + account, ok := fcAccount.(*authtypes.ModuleAccount) + if !ok { + return nil, fmt.Errorf("feeCollector account is not *authtypes.ModuleAccount") + } + account.Permissions = append(account.Permissions, authtypes.Burner) + accountKeeper.SetModuleAccount(ctx, account) + return mm.RunMigrations(ctx, cfg, fromVM) } }, From 929346b39dd2738d5d064f1de0ab2e41dd52f63b Mon Sep 17 00:00:00 2001 From: Spoorthi Satheesha <9302666+spoo-bar@users.noreply.github.com> Date: Wed, 30 Aug 2023 20:56:16 +0530 Subject: [PATCH 3/7] bumping version to v4.0.1 --- app/app_upgrades.go | 5 ++--- app/upgrades/{latest => 4_0_1}/upgrades.go | 4 +--- interchaintest/setup.go | 2 +- 3 files changed, 4 insertions(+), 7 deletions(-) rename app/upgrades/{latest => 4_0_1}/upgrades.go (92%) diff --git a/app/app_upgrades.go b/app/app_upgrades.go index 036b3bc6..316a2d82 100644 --- a/app/app_upgrades.go +++ b/app/app_upgrades.go @@ -11,7 +11,7 @@ import ( upgrade2_0_0 "github.com/archway-network/archway/app/upgrades/2_0_0" upgrade3_0_0 "github.com/archway-network/archway/app/upgrades/3_0_0" upgrade4_0_0 "github.com/archway-network/archway/app/upgrades/4_0_0" - upgradelatest "github.com/archway-network/archway/app/upgrades/latest" + upgrade4_0_1 "github.com/archway-network/archway/app/upgrades/4_0_1" ) // UPGRADES @@ -22,8 +22,7 @@ var Upgrades = []upgrades.Upgrade{ upgrade2_0_0.Upgrade, // v2.0.0 upgrade3_0_0.Upgrade, // v3.0.0 upgrade4_0_0.Upgrade, // v4.0.0 - - upgradelatest.Upgrade, // latest - This upgrade handler is used for all the current changes to the protocol + upgrade4_0_1.Upgrade, // v4.0.1 } func (app *ArchwayApp) setupUpgrades() { diff --git a/app/upgrades/latest/upgrades.go b/app/upgrades/4_0_1/upgrades.go similarity index 92% rename from app/upgrades/latest/upgrades.go rename to app/upgrades/4_0_1/upgrades.go index ac80bfc6..20a26281 100644 --- a/app/upgrades/latest/upgrades.go +++ b/app/upgrades/4_0_1/upgrades.go @@ -13,9 +13,7 @@ import ( "github.com/archway-network/archway/app/upgrades" ) -// This upgrade handler is used for all the current changes to the protocol - -const Name = "latest" +const Name = "v4.0.1" var Upgrade = upgrades.Upgrade{ UpgradeName: Name, diff --git a/interchaintest/setup.go b/interchaintest/setup.go index a2001417..65789468 100644 --- a/interchaintest/setup.go +++ b/interchaintest/setup.go @@ -9,7 +9,7 @@ import ( const ( initialVersion = "v4.0.0" // The last release of the chain. The one the mainnet is running on - upgradeName = "latest" // The next upgrade name. Should match the upgrade handler. + upgradeName = "v4.0.1" // The next upgrade name. Should match the upgrade handler. chainName = "archway" ) From e79c56e9dc5228136fbfc210adc81b94014b3ae0 Mon Sep 17 00:00:00 2001 From: Spoorthi Satheesha <9302666+spoo-bar@users.noreply.github.com> Date: Thu, 31 Aug 2023 02:56:00 +0530 Subject: [PATCH 4/7] Update CHANGELOG.md --- CHANGELOG.md | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 04937e9e..3500b18c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,19 +28,11 @@ Contains bug fixes. Contains all the PRs that improved the code without changing the behaviours. --> -## [Unreleased] - -### Added - -### Changed - -### Deprecated - -### Removed +## [v4.0.1] ### Fixed -### Improvements +- [#437](https://github.com/archway-network/archway/pull/437) - adding upgrade handler with missing burn permissions for feecollector account ## [v4.0.0] From 0ba9b7e7b974e6f9940994522a9103ed157d1ea3 Mon Sep 17 00:00:00 2001 From: Spoorthi Satheesha <9302666+spoo-bar@users.noreply.github.com> Date: Thu, 31 Aug 2023 09:54:00 +0530 Subject: [PATCH 5/7] add upgrade test --- app/upgrades/4_0_1/upgrades.go | 10 ++- app/upgrades/4_0_1/upgrades_test.go | 94 +++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 app/upgrades/4_0_1/upgrades_test.go diff --git a/app/upgrades/4_0_1/upgrades.go b/app/upgrades/4_0_1/upgrades.go index 20a26281..5abe290c 100644 --- a/app/upgrades/4_0_1/upgrades.go +++ b/app/upgrades/4_0_1/upgrades.go @@ -1,4 +1,4 @@ -package upgradelatest +package upgrade4_0_1 import ( "fmt" @@ -24,7 +24,13 @@ var Upgrade = upgrades.Upgrade{ if !ok { return nil, fmt.Errorf("feeCollector account is not *authtypes.ModuleAccount") } - account.Permissions = append(account.Permissions, authtypes.Burner) + if !account.HasPermission(authtypes.Burner) { + account.Permissions = append(account.Permissions, authtypes.Burner) + } + err := accountKeeper.ValidatePermissions(account) + if err != nil { + return nil, fmt.Errorf("Could not validate feeCollectors permissions") + } accountKeeper.SetModuleAccount(ctx, account) return mm.RunMigrations(ctx, cfg, fromVM) diff --git a/app/upgrades/4_0_1/upgrades_test.go b/app/upgrades/4_0_1/upgrades_test.go new file mode 100644 index 00000000..5b3293a4 --- /dev/null +++ b/app/upgrades/4_0_1/upgrades_test.go @@ -0,0 +1,94 @@ +package upgrade4_0_1_test + +import ( + "fmt" + "testing" + + e2eTesting "github.com/archway-network/archway/e2e/testing" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" + "github.com/stretchr/testify/suite" + abci "github.com/tendermint/tendermint/abci/types" +) + +type UpgradeTestSuite struct { + suite.Suite + + archway *e2eTesting.TestChain +} + +func (s *UpgradeTestSuite) SetupTest() { + s.archway = e2eTesting.NewTestChain(s.T(), 1) +} + +func TestUpgradeTestSuite(t *testing.T) { + suite.Run(t, new(UpgradeTestSuite)) +} + +const ( + dummyUpgradeHeight = 5 +) + +func (suite *UpgradeTestSuite) TestUpgrade() { + testCases := []struct { + name string + pre_upgrade func() + post_upgrade func() + }{ + { + "Feecollector does not have burn permissions, we ensure upgrade happens and account gets the burn permissions", + func() { + accountKeeper := suite.archway.GetApp().AccountKeeper + fcAccount := accountKeeper.GetModuleAccount(suite.archway.GetContext(), authtypes.FeeCollectorName) + + account, ok := fcAccount.(*authtypes.ModuleAccount) + suite.Require().True(ok) + account.Permissions = []string{} + accountKeeper.SetModuleAccount(suite.archway.GetContext(), account) + + fcAccount = accountKeeper.GetModuleAccount(suite.archway.GetContext(), authtypes.FeeCollectorName) + suite.Require().False(fcAccount.HasPermission(authtypes.Burner)) + }, + func() { + accountKeeper := suite.archway.GetApp().AccountKeeper + fcAccount := accountKeeper.GetModuleAccount(suite.archway.GetContext(), authtypes.FeeCollectorName) + suite.Require().True(fcAccount.HasPermission(authtypes.Burner)) + }, + }, + { + "Feecollector already has burn permissions, we ensure upgrade happens smoothly", + func() { + accountKeeper := suite.archway.GetApp().AccountKeeper + fcAccount := accountKeeper.GetModuleAccount(suite.archway.GetContext(), authtypes.FeeCollectorName) + suite.Require().True(fcAccount.HasPermission(authtypes.Burner)) + }, + func() { + accountKeeper := suite.archway.GetApp().AccountKeeper + fcAccount := accountKeeper.GetModuleAccount(suite.archway.GetContext(), authtypes.FeeCollectorName) + suite.Require().True(fcAccount.HasPermission(authtypes.Burner)) + }, + }, + } + + for _, tc := range testCases { + suite.Run(fmt.Sprintf("Case %s", tc.name), func() { + suite.SetupTest() // reset + + tc.pre_upgrade() + + ctx := suite.archway.GetContext().WithBlockHeight(dummyUpgradeHeight - 1) + plan := upgradetypes.Plan{Name: "v4.0.1", Height: dummyUpgradeHeight} + upgradekeeper := suite.archway.GetApp().UpgradeKeeper + err := upgradekeeper.ScheduleUpgrade(ctx, plan) + suite.Require().NoError(err) + _, exists := upgradekeeper.GetUpgradePlan(ctx) + suite.Require().True(exists) + ctx = ctx.WithBlockHeight(dummyUpgradeHeight) + suite.Require().NotPanics(func() { + suite.archway.GetApp().BeginBlocker(ctx, abci.RequestBeginBlock{}) + }) + + tc.post_upgrade() + }) + } +} From 515a23edb5e738e6e619b63dc005e8469ef575c3 Mon Sep 17 00:00:00 2001 From: Spoorthi Satheesha <9302666+spoo-bar@users.noreply.github.com> Date: Thu, 31 Aug 2023 10:39:10 +0530 Subject: [PATCH 6/7] Adding interchain test - TestFeeCollectorBurnChainUpgrade --- interchaintest/chain_upgrade_test.go | 12 +-- interchaintest/chain_v401_upgrade_test.go | 108 ++++++++++++++++++++++ interchaintest/go.mod | 4 +- 3 files changed, 116 insertions(+), 8 deletions(-) create mode 100644 interchaintest/chain_v401_upgrade_test.go diff --git a/interchaintest/chain_upgrade_test.go b/interchaintest/chain_upgrade_test.go index 7174a67c..b1a20c0c 100644 --- a/interchaintest/chain_upgrade_test.go +++ b/interchaintest/chain_upgrade_test.go @@ -24,9 +24,9 @@ func TestChainUpgrade(t *testing.T) { t.Skip("skipping in short mode") } - archwayChain, client, ctx := startChain(t) + archwayChain, client, ctx := startChain(t, initialVersion) chainUser := fundChainUser(t, ctx, archwayChain) - haltHeight := submitUpgradeProposalAndVote(t, ctx, archwayChain, chainUser) + haltHeight := submitUpgradeProposalAndVote(t, ctx, upgradeName, archwayChain, chainUser) height, err := archwayChain.Height(ctx) require.NoError(t, err, "cound not fetch height before upgrade") @@ -63,7 +63,7 @@ func TestChainUpgrade(t *testing.T) { require.NoError(t, err, "chain did not produce blocks after upgrade") } -func submitUpgradeProposalAndVote(t *testing.T, ctx context.Context, archwayChain *cosmos.CosmosChain, chainUser ibc.Wallet) uint64 { +func submitUpgradeProposalAndVote(t *testing.T, ctx context.Context, nextUpgradeName string, archwayChain *cosmos.CosmosChain, chainUser ibc.Wallet) uint64 { height, err := archwayChain.Height(ctx) // The current chain height require.NoError(t, err, "error fetching height before submit upgrade proposal") @@ -72,7 +72,7 @@ func submitUpgradeProposalAndVote(t *testing.T, ctx context.Context, archwayChai proposal := cosmos.SoftwareUpgradeProposal{ Deposit: "10000000000" + archwayChain.Config().Denom, Title: "Test upgrade", - Name: upgradeName, + Name: nextUpgradeName, Description: "Every PR we perform a upgrade check to ensure nothing breaks", Height: haltHeight, } @@ -94,13 +94,13 @@ func fundChainUser(t *testing.T, ctx context.Context, archwayChain *cosmos.Cosmo return users[0] } -func startChain(t *testing.T) (*cosmos.CosmosChain, *client.Client, context.Context) { +func startChain(t *testing.T, startingVersion string) (*cosmos.CosmosChain, *client.Client, context.Context) { numOfVals := 1 cf := interchaintest.NewBuiltinChainFactory(zaptest.NewLogger(t), []*interchaintest.ChainSpec{ { Name: chainName, ChainName: "archway-1", - Version: initialVersion, + Version: startingVersion, ChainConfig: archwayConfig, NumValidators: &numOfVals, }, diff --git a/interchaintest/chain_v401_upgrade_test.go b/interchaintest/chain_v401_upgrade_test.go new file mode 100644 index 00000000..9fae1ab0 --- /dev/null +++ b/interchaintest/chain_v401_upgrade_test.go @@ -0,0 +1,108 @@ +package interchaintest + +import ( + "context" + "testing" + "time" + + "gopkg.in/yaml.v2" + + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/strangelove-ventures/interchaintest/v4/chain/cosmos" + "github.com/strangelove-ventures/interchaintest/v4/testutil" + "github.com/stretchr/testify/require" +) + +// TestAccountBurnChainUpgrade was written specifically to test for the following issue : https://github.com/orgs/archway-network/discussions/6 +// To run this test, you will need the following heighliner images +// heighliner build --org archway-network --repo archway --dockerfile cosmos --build-target "make build" --build-env "BUILD_TAGS=muslc" --binaries "build/archwayd" --git-ref v2.0.0 --tag v2.0.0 -c archway +// heighliner build --org archway-network --repo archway --dockerfile cosmos --build-target "make build" --build-env "BUILD_TAGS=muslc" --binaries "build/archwayd" --git-ref v4.0.0 --tag v4.0.0 -c archway +// heighliner build --org archway-network --repo archway --dockerfile cosmos --build-target "make build" --build-env "BUILD_TAGS=muslc" --binaries "build/archwayd" --git-ref v4.0.1 --tag v4.0.1 -c archway +func TestFeeCollectorBurnChainUpgrade(t *testing.T) { + if testing.Short() { + t.Skip("skipping in short mode") + } + + // Starting the chain with v2.0.0. Starting at v2.0.0 because bug only happens when we have upgraded to v4.0.0. Does not happen when we start from there. + archwayChain, client, ctx := startChain(t, "v2.0.0") + chainUser := fundChainUser(t, ctx, archwayChain) + + timeoutCtx, timeoutCtxCancel := context.WithTimeout(ctx, time.Second*45) + defer timeoutCtxCancel() + + // waiting for chain starting + testutil.WaitForBlocks(timeoutCtx, 1, archwayChain) + + // Ensuring feecollector does not have burn permissions in v2.0.0 + queryRes2 := getModuleAccount(t, ctx, authtypes.FeeCollectorName, archwayChain) + require.Len(t, queryRes2.Account.Permissions, 0, "feecollector should not have burn permissions in v2.0.0") + + // Upgrading to v4.0.0 => Not directly upgrading to v4.0.1 to simulate how things went on constantine. + haltHeight := submitUpgradeProposalAndVote(t, ctx, "v4.0.0", archwayChain, chainUser) + height, err := archwayChain.Height(ctx) + require.NoError(t, err, "cound not fetch height before upgrade") + testutil.WaitForBlocks(timeoutCtx, int(haltHeight-height)+1, archwayChain) + height, err = archwayChain.Height(ctx) + require.NoError(t, err, "could not fetch height after chain should have halted") + require.Equal(t, int(haltHeight), int(height), "height is not equal to halt height") + err = archwayChain.StopAllNodes(ctx) + require.NoError(t, err, "could not stop node(s)") + archwayChain.UpgradeVersion(ctx, client, chainName, "v4.0.0") + err = archwayChain.StartAllNodes(ctx) + require.NoError(t, err, "could not start upgraded node(s)") + timeoutCtx, timeoutCtxCancel = context.WithTimeout(ctx, time.Second*45) + defer timeoutCtxCancel() + err = testutil.WaitForBlocks(timeoutCtx, int(blocksAfterUpgrade), archwayChain) + require.NoError(t, err, "chain did not produce blocks after upgrade") + + // Ensuring feecollector does not have burn permissions in v4.0.0 + queryRes4 := getModuleAccount(t, ctx, authtypes.FeeCollectorName, archwayChain) + require.Len(t, queryRes4.Account.Permissions, 0, "feecollector should not have burn permissions in v4.0.0") + + // Upgrading to v4.0.1 + haltHeight = submitUpgradeProposalAndVote(t, ctx, "v4.0.1", archwayChain, chainUser) + height, err = archwayChain.Height(ctx) + require.NoError(t, err, "cound not fetch height before upgrade") + testutil.WaitForBlocks(timeoutCtx, int(haltHeight-height)+1, archwayChain) + height, err = archwayChain.Height(ctx) + require.NoError(t, err, "could not fetch height after chain should have halted") + require.Equal(t, int(haltHeight), int(height), "height is not equal to halt height") + err = archwayChain.StopAllNodes(ctx) + require.NoError(t, err, "could not stop node(s)") + archwayChain.UpgradeVersion(ctx, client, chainName, "v4.0.1") + err = archwayChain.StartAllNodes(ctx) + require.NoError(t, err, "could not start upgraded node(s)") + timeoutCtx, timeoutCtxCancel = context.WithTimeout(ctx, time.Second*45) + defer timeoutCtxCancel() + err = testutil.WaitForBlocks(timeoutCtx, int(blocksAfterUpgrade), archwayChain) + require.NoError(t, err, "chain did not produce blocks after upgrade") + + // Ensuring feecollector HAS burn permissions in v4.0.1 + queryRes401 := getModuleAccount(t, ctx, authtypes.FeeCollectorName, archwayChain) + require.Len(t, queryRes401.Account.Permissions, 1, "feecollector should have one permissions in v4.0.1") + require.Equal(t, authtypes.Burner, queryRes401.Account.Permissions[0], "feecollector should have burn permissions in v4.0.1") +} + +func getModuleAccount(t *testing.T, ctx context.Context, moduleAccountName string, archwayChain *cosmos.CosmosChain) QueryModuleAccountResponse { + cmd := []string{ + "archwayd", "q", "auth", "module-account", moduleAccountName, + "--node", archwayChain.GetRPCAddress(), + "--home", archwayChain.HomeDir(), + "--chain-id", archwayChain.Config().ChainID, + } + stdout, _, err := archwayChain.Exec(ctx, cmd, nil) + require.NoError(t, err, "could not fetch the fee collector account") + queryRes := QueryModuleAccountResponse{} + err = yaml.Unmarshal(stdout, &queryRes) + require.NoError(t, err, "could not unmarshal query module account respons") + return queryRes +} + +type QueryModuleAccountResponse struct { + Account AccountData `json:"account"` +} + +type AccountData struct { + Name string `json:"name` + Permissions []string `json:"permissions` +} diff --git a/interchaintest/go.mod b/interchaintest/go.mod index 1c3a5760..c4e047ac 100644 --- a/interchaintest/go.mod +++ b/interchaintest/go.mod @@ -12,10 +12,12 @@ replace ( ) require ( + github.com/cosmos/cosmos-sdk v0.45.16 github.com/docker/docker v24.0.1+incompatible github.com/strangelove-ventures/interchaintest/v4 v4.0.0-20230628165518-1f80499fa0cd github.com/stretchr/testify v1.8.4 go.uber.org/zap v1.24.0 + gopkg.in/yaml.v2 v2.4.0 ) require ( @@ -47,7 +49,6 @@ require ( github.com/cosmos/btcutil v1.0.5 // indirect github.com/cosmos/cosmos-db v0.0.0-20221226095112-f3c38ecb5e32 // indirect github.com/cosmos/cosmos-proto v1.0.0-beta.2 // indirect - github.com/cosmos/cosmos-sdk v0.45.16 // indirect github.com/cosmos/go-bip39 v1.0.0 // indirect github.com/cosmos/gorocksdb v1.2.0 // indirect github.com/cosmos/iavl v0.19.5 // indirect @@ -153,7 +154,6 @@ require ( google.golang.org/grpc v1.55.0 // indirect google.golang.org/protobuf v1.30.0 // indirect gopkg.in/ini.v1 v1.67.0 // indirect - gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect lukechampine.com/uint128 v1.2.0 // indirect modernc.org/cc/v3 v3.40.0 // indirect From d5c0a85fb78dc868d21af188def0aa1b1fac41a6 Mon Sep 17 00:00:00 2001 From: Spoorthi Satheesha <9302666+spoo-bar@users.noreply.github.com> Date: Thu, 31 Aug 2023 10:46:14 +0530 Subject: [PATCH 7/7] fixing linting --- app/upgrades/4_0_1/upgrades_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/upgrades/4_0_1/upgrades_test.go b/app/upgrades/4_0_1/upgrades_test.go index 5b3293a4..7c6c2818 100644 --- a/app/upgrades/4_0_1/upgrades_test.go +++ b/app/upgrades/4_0_1/upgrades_test.go @@ -4,11 +4,12 @@ import ( "fmt" "testing" - e2eTesting "github.com/archway-network/archway/e2e/testing" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" "github.com/stretchr/testify/suite" abci "github.com/tendermint/tendermint/abci/types" + + e2eTesting "github.com/archway-network/archway/e2e/testing" ) type UpgradeTestSuite struct {