fix: upgrade handler missing burn permissions for feecollector account

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]
 

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() {
@@ -52,7 +51,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),
 		)
 	}
 }

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)

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),
 	)
 }

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)
 		}

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)
 		}

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

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)
 		}

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)
 		}

app/upgrades/4_0_1/upgrades.go

@@ -0,0 +1,40 @@
+package upgrade4_0_1
+
+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"
+)
+
+const Name = "v4.0.1"
+
+var Upgrade = upgrades.Upgrade{
+	UpgradeName: Name,
+	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")
+			}
+			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)
+		}
+	},
+	StoreUpgrades: storetypes.StoreUpgrades{},
+}

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()
+		})
+	}
+}

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

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,
 		},