From 2e1fb485cd5d73ddb7c7a1564d8a1c3f0da29346 Mon Sep 17 00:00:00 2001
From: Julien Robert <julien@rbrt.fr>
Date: Tue, 23 Aug 2022 18:21:41 +0200
Subject: [PATCH] fix: use module permission from app_config in `app.go`
 (#12997)

---
 simapp/app.go        |  39 +++--
 simapp/app_config.go | 335 ++++++++++++++++++++++---------------------
 simapp/app_legacy.go |  22 +--
 simapp/app_test.go   |  10 +-
 x/bank/module.go     |   6 +-
 5 files changed, 211 insertions(+), 201 deletions(-)

diff --git a/simapp/app.go b/simapp/app.go
index 88dcbdc7e6..579b786065 100644
--- a/simapp/app.go
+++ b/simapp/app.go
@@ -121,17 +121,6 @@ var (
 		vesting.AppModuleBasic{},
 		nftmodule.AppModuleBasic{},
 	)
-
-	// module account permissions
-	maccPerms = map[string][]string{
-		authtypes.FeeCollectorName:     nil,
-		distrtypes.ModuleName:          nil,
-		minttypes.ModuleName:           {authtypes.Minter},
-		stakingtypes.BondedPoolName:    {authtypes.Burner, authtypes.Staking},
-		stakingtypes.NotBondedPoolName: {authtypes.Burner, authtypes.Staking},
-		govtypes.ModuleName:            {authtypes.Burner},
-		nft.ModuleName:                 nil,
-	}
 )
 
 var (
@@ -386,10 +375,30 @@ func (app *SimApp) RegisterAPIRoutes(apiSvr *api.Server, apiConfig config.APICon
 }
 
 // GetMaccPerms returns a copy of the module account permissions
+//
+// NOTE: This is solely to be used for testing purposes.
 func GetMaccPerms() map[string][]string {
-	dupMaccPerms := make(map[string][]string)
-	for k, v := range maccPerms {
-		dupMaccPerms[k] = v
+	dup := make(map[string][]string)
+	for _, perms := range moduleAccPerms {
+		dup[perms.Account] = perms.Permissions
 	}
-	return dupMaccPerms
+
+	return dup
+}
+
+// BlockedAddresses returns all the app's blocked account addresses.
+func BlockedAddresses() map[string]bool {
+	result := make(map[string]bool)
+
+	if len(blockAccAddrs) > 0 {
+		for _, addr := range blockAccAddrs {
+			result[addr] = true
+		}
+	} else {
+		for addr := range GetMaccPerms() {
+			result[addr] = true
+		}
+	}
+
+	return result
 }
diff --git a/simapp/app_config.go b/simapp/app_config.go
index d9dc7f17bd..b2c140018d 100644
--- a/simapp/app_config.go
+++ b/simapp/app_config.go
@@ -47,171 +47,176 @@ import (
 	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
 )
 
-// Alternatively the AppConfig can be defined as a YAML or a JSON file.
-// e.g. https://github.com/cosmos/cosmos-sdk/blob/91b1d83f1339e235a1dfa929ecc00084101a19e3/simapp/app.yaml
+var (
+	// module account permissions
+	moduleAccPerms = []*authmodulev1.ModuleAccountPermission{
+		{Account: authtypes.FeeCollectorName},
+		{Account: distrtypes.ModuleName},
+		{Account: minttypes.ModuleName, Permissions: []string{authtypes.Minter}},
+		{Account: stakingtypes.BondedPoolName, Permissions: []string{authtypes.Burner, stakingtypes.ModuleName}},
+		{Account: stakingtypes.NotBondedPoolName, Permissions: []string{authtypes.Burner, stakingtypes.ModuleName}},
+		{Account: govtypes.ModuleName, Permissions: []string{authtypes.Burner}},
+		{Account: nft.ModuleName},
+	}
 
-var AppConfig = appconfig.Compose(&appv1alpha1.Config{
-	Modules: []*appv1alpha1.ModuleConfig{
-		{
-			Name: "runtime",
-			Config: appconfig.WrapAny(&runtimev1alpha1.Module{
-				AppName: "SimApp",
-				// During begin block slashing happens after distr.BeginBlocker so that
-				// there is nothing left over in the validator fee pool, so as to keep the
-				// CanWithdrawInvariant invariant.
-				// NOTE: staking module is required if HistoricalEntries param > 0
-				// NOTE: capability module's beginblocker must come before any modules using capabilities (e.g. IBC)
-				BeginBlockers: []string{
-					upgradetypes.ModuleName,
-					capabilitytypes.ModuleName,
-					minttypes.ModuleName,
-					distrtypes.ModuleName,
-					slashingtypes.ModuleName,
-					evidencetypes.ModuleName,
-					stakingtypes.ModuleName,
-					authtypes.ModuleName,
-					banktypes.ModuleName,
-					govtypes.ModuleName,
-					crisistypes.ModuleName,
-					genutiltypes.ModuleName,
-					authz.ModuleName,
-					feegrant.ModuleName,
-					nft.ModuleName,
-					group.ModuleName,
-					paramstypes.ModuleName,
-					vestingtypes.ModuleName,
-				},
-				EndBlockers: []string{
-					crisistypes.ModuleName,
-					govtypes.ModuleName,
-					stakingtypes.ModuleName,
-					capabilitytypes.ModuleName,
-					authtypes.ModuleName,
-					banktypes.ModuleName,
-					distrtypes.ModuleName,
-					slashingtypes.ModuleName,
-					minttypes.ModuleName,
-					genutiltypes.ModuleName,
-					evidencetypes.ModuleName,
-					authz.ModuleName,
-					feegrant.ModuleName,
-					nft.ModuleName,
-					group.ModuleName,
-					paramstypes.ModuleName,
-					upgradetypes.ModuleName,
-					vestingtypes.ModuleName,
-				},
-				OverrideStoreKeys: []*runtimev1alpha1.StoreKeyConfig{
-					{
-						ModuleName: authtypes.ModuleName,
-						KvStoreKey: "acc",
+	// blocked account addresses
+	blockAccAddrs = []string{
+		authtypes.FeeCollectorName,
+		distrtypes.ModuleName,
+		minttypes.ModuleName,
+		stakingtypes.BondedPoolName,
+		stakingtypes.NotBondedPoolName,
+		nft.ModuleName,
+		// We allow the following module accounts to receive funds:
+		// govtypes.ModuleName
+	}
+
+	// application configuration (used by depinject)
+	AppConfig = appconfig.Compose(&appv1alpha1.Config{
+		Modules: []*appv1alpha1.ModuleConfig{
+			{
+				Name: "runtime",
+				Config: appconfig.WrapAny(&runtimev1alpha1.Module{
+					AppName: "SimApp",
+					// During begin block slashing happens after distr.BeginBlocker so that
+					// there is nothing left over in the validator fee pool, so as to keep the
+					// CanWithdrawInvariant invariant.
+					// NOTE: staking module is required if HistoricalEntries param > 0
+					// NOTE: capability module's beginblocker must come before any modules using capabilities (e.g. IBC)
+					BeginBlockers: []string{
+						upgradetypes.ModuleName,
+						capabilitytypes.ModuleName,
+						minttypes.ModuleName,
+						distrtypes.ModuleName,
+						slashingtypes.ModuleName,
+						evidencetypes.ModuleName,
+						stakingtypes.ModuleName,
+						authtypes.ModuleName,
+						banktypes.ModuleName,
+						govtypes.ModuleName,
+						crisistypes.ModuleName,
+						genutiltypes.ModuleName,
+						authz.ModuleName,
+						feegrant.ModuleName,
+						nft.ModuleName,
+						group.ModuleName,
+						paramstypes.ModuleName,
+						vestingtypes.ModuleName,
 					},
-				},
-			}),
+					EndBlockers: []string{
+						crisistypes.ModuleName,
+						govtypes.ModuleName,
+						stakingtypes.ModuleName,
+						capabilitytypes.ModuleName,
+						authtypes.ModuleName,
+						banktypes.ModuleName,
+						distrtypes.ModuleName,
+						slashingtypes.ModuleName,
+						minttypes.ModuleName,
+						genutiltypes.ModuleName,
+						evidencetypes.ModuleName,
+						authz.ModuleName,
+						feegrant.ModuleName,
+						nft.ModuleName,
+						group.ModuleName,
+						paramstypes.ModuleName,
+						upgradetypes.ModuleName,
+						vestingtypes.ModuleName,
+					},
+					OverrideStoreKeys: []*runtimev1alpha1.StoreKeyConfig{
+						{
+							ModuleName: authtypes.ModuleName,
+							KvStoreKey: "acc",
+						},
+					},
+				}),
+			},
+			{
+				Name: authtypes.ModuleName,
+				Config: appconfig.WrapAny(&authmodulev1.Module{
+					Bech32Prefix:             "cosmos",
+					ModuleAccountPermissions: moduleAccPerms,
+				}),
+			},
+			{
+				Name:   vestingtypes.ModuleName,
+				Config: appconfig.WrapAny(&vestingmodulev1.Module{}),
+			},
+			{
+				Name: banktypes.ModuleName,
+				Config: appconfig.WrapAny(&bankmodulev1.Module{
+					BlockedModuleAccountsOverride: blockAccAddrs,
+				}),
+			},
+			{
+				Name:   stakingtypes.ModuleName,
+				Config: appconfig.WrapAny(&stakingmodulev1.Module{}),
+			},
+			{
+				Name:   slashingtypes.ModuleName,
+				Config: appconfig.WrapAny(&slashingmodulev1.Module{}),
+			},
+			{
+				Name:   paramstypes.ModuleName,
+				Config: appconfig.WrapAny(&paramsmodulev1.Module{}),
+			},
+			{
+				Name:   "tx",
+				Config: appconfig.WrapAny(&txmodulev1.Module{}),
+			},
+			{
+				Name:   genutiltypes.ModuleName,
+				Config: appconfig.WrapAny(&genutilmodulev1.Module{}),
+			},
+			{
+				Name:   authz.ModuleName,
+				Config: appconfig.WrapAny(&authzmodulev1.Module{}),
+			},
+			{
+				Name:   upgradetypes.ModuleName,
+				Config: appconfig.WrapAny(&upgrademodulev1.Module{}),
+			},
+			{
+				Name:   distrtypes.ModuleName,
+				Config: appconfig.WrapAny(&distrmodulev1.Module{}),
+			},
+			{
+				Name: capabilitytypes.ModuleName,
+				Config: appconfig.WrapAny(&capabilitymodulev1.Module{
+					SealKeeper: true,
+				}),
+			},
+			{
+				Name:   evidencetypes.ModuleName,
+				Config: appconfig.WrapAny(&evidencemodulev1.Module{}),
+			},
+			{
+				Name:   minttypes.ModuleName,
+				Config: appconfig.WrapAny(&mintmodulev1.Module{}),
+			},
+			{
+				Name: group.ModuleName,
+				Config: appconfig.WrapAny(&groupmodulev1.Module{
+					MaxExecutionPeriod: durationpb.New(time.Second * 1209600),
+					MaxMetadataLen:     255,
+				}),
+			},
+			{
+				Name:   nft.ModuleName,
+				Config: appconfig.WrapAny(&nftmodulev1.Module{}),
+			},
+			{
+				Name:   feegrant.ModuleName,
+				Config: appconfig.WrapAny(&feegrantmodulev1.Module{}),
+			},
+			{
+				Name:   govtypes.ModuleName,
+				Config: appconfig.WrapAny(&govmodulev1.Module{}),
+			},
+			{
+				Name:   crisistypes.ModuleName,
+				Config: appconfig.WrapAny(&crisismodulev1.Module{}),
+			},
 		},
-		{
-			Name: authtypes.ModuleName,
-			Config: appconfig.WrapAny(&authmodulev1.Module{
-				Bech32Prefix: "cosmos",
-				ModuleAccountPermissions: []*authmodulev1.ModuleAccountPermission{
-					{Account: authtypes.FeeCollectorName},
-					{Account: distrtypes.ModuleName},
-					{Account: minttypes.ModuleName, Permissions: []string{authtypes.Minter}},
-					{Account: stakingtypes.BondedPoolName, Permissions: []string{authtypes.Burner, stakingtypes.ModuleName}},
-					{Account: stakingtypes.NotBondedPoolName, Permissions: []string{authtypes.Burner, stakingtypes.ModuleName}},
-					{Account: govtypes.ModuleName, Permissions: []string{authtypes.Burner}},
-					{Account: nft.ModuleName},
-				},
-			}),
-		},
-		{
-			Name:   vestingtypes.ModuleName,
-			Config: appconfig.WrapAny(&vestingmodulev1.Module{}),
-		},
-		{
-			Name: banktypes.ModuleName,
-			Config: appconfig.WrapAny(&bankmodulev1.Module{
-				BlockedModuleAccountsOverride: []string{
-					authtypes.FeeCollectorName,
-					distrtypes.ModuleName,
-					minttypes.ModuleName,
-					stakingtypes.BondedPoolName,
-					stakingtypes.NotBondedPoolName,
-					govtypes.ModuleName,
-					nft.ModuleName,
-					// We allow the following module accounts to receive funds:
-					// govtypes.ModuleName
-				},
-			}),
-		},
-		{
-			Name:   stakingtypes.ModuleName,
-			Config: appconfig.WrapAny(&stakingmodulev1.Module{}),
-		},
-		{
-			Name:   slashingtypes.ModuleName,
-			Config: appconfig.WrapAny(&slashingmodulev1.Module{}),
-		},
-		{
-			Name:   paramstypes.ModuleName,
-			Config: appconfig.WrapAny(&paramsmodulev1.Module{}),
-		},
-		{
-			Name:   "tx",
-			Config: appconfig.WrapAny(&txmodulev1.Module{}),
-		},
-		{
-			Name:   genutiltypes.ModuleName,
-			Config: appconfig.WrapAny(&genutilmodulev1.Module{}),
-		},
-		{
-			Name:   authz.ModuleName,
-			Config: appconfig.WrapAny(&authzmodulev1.Module{}),
-		},
-		{
-			Name:   upgradetypes.ModuleName,
-			Config: appconfig.WrapAny(&upgrademodulev1.Module{}),
-		},
-		{
-			Name:   distrtypes.ModuleName,
-			Config: appconfig.WrapAny(&distrmodulev1.Module{}),
-		},
-		{
-			Name: capabilitytypes.ModuleName,
-			Config: appconfig.WrapAny(&capabilitymodulev1.Module{
-				SealKeeper: true,
-			}),
-		},
-		{
-			Name:   evidencetypes.ModuleName,
-			Config: appconfig.WrapAny(&evidencemodulev1.Module{}),
-		},
-		{
-			Name:   minttypes.ModuleName,
-			Config: appconfig.WrapAny(&mintmodulev1.Module{}),
-		},
-		{
-			Name: group.ModuleName,
-			Config: appconfig.WrapAny(&groupmodulev1.Module{
-				MaxExecutionPeriod: durationpb.New(time.Second * 1209600),
-				MaxMetadataLen:     255,
-			}),
-		},
-		{
-			Name:   nft.ModuleName,
-			Config: appconfig.WrapAny(&nftmodulev1.Module{}),
-		},
-		{
-			Name:   feegrant.ModuleName,
-			Config: appconfig.WrapAny(&feegrantmodulev1.Module{}),
-		},
-		{
-			Name:   govtypes.ModuleName,
-			Config: appconfig.WrapAny(&govmodulev1.Module{}),
-		},
-		{
-			Name:   crisistypes.ModuleName,
-			Config: appconfig.WrapAny(&crisismodulev1.Module{}),
-		},
-	},
-})
+	})
+)
diff --git a/simapp/app_legacy.go b/simapp/app_legacy.go
index 440bcd108a..e4fed7bca9 100644
--- a/simapp/app_legacy.go
+++ b/simapp/app_legacy.go
@@ -304,6 +304,15 @@ func NewSimApp(
 	*/
 	app.GroupKeeper = groupkeeper.NewKeeper(keys[group.StoreKey], appCodec, app.MsgServiceRouter(), app.AccountKeeper, groupConfig)
 
+	// get skipUpgradeHeights from the app options
+	skipUpgradeHeights := map[int64]bool{}
+	for _, h := range cast.ToIntSlice(appOpts.Get(server.FlagUnsafeSkipUpgrades)) {
+		skipUpgradeHeights[int64(h)] = true
+	}
+	homePath := cast.ToString(appOpts.Get(flags.FlagHome))
+	// set the governance module account as the authority for conducting upgrades
+	app.UpgradeKeeper = upgradekeeper.NewKeeper(skipUpgradeHeights, keys[upgradetypes.StoreKey], appCodec, homePath, app.BaseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())
+
 	// Register the proposal types
 	// Deprecated: Avoid adding new handlers, instead use the new proposal flow
 	// by granting the governance module the right to execute the message.
@@ -328,15 +337,6 @@ func NewSimApp(
 		),
 	)
 
-	// get skipUpgradeHeights from the app options
-	skipUpgradeHeights := map[int64]bool{}
-	for _, h := range cast.ToIntSlice(appOpts.Get(server.FlagUnsafeSkipUpgrades)) {
-		skipUpgradeHeights[int64(h)] = true
-	}
-	homePath := cast.ToString(appOpts.Get(flags.FlagHome))
-	// set the governance module account as the authority for conducting upgrades
-	app.UpgradeKeeper = upgradekeeper.NewKeeper(skipUpgradeHeights, keys[upgradetypes.StoreKey], appCodec, homePath, app.BaseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())
-
 	// RegisterUpgradeHandlers is used for registering any on-chain upgrades
 	app.RegisterUpgradeHandlers()
 
@@ -642,6 +642,8 @@ func (app *SimApp) RegisterTendermintService(clientCtx client.Context) {
 }
 
 // GetMaccPerms returns a copy of the module account permissions
+//
+// NOTE: This is solely to be used for testing purposes.
 func GetMaccPerms() map[string][]string {
 	dupMaccPerms := make(map[string][]string)
 	for k, v := range maccPerms {
@@ -654,7 +656,7 @@ func GetMaccPerms() map[string][]string {
 // ModuleAccountAddrsLegacy returns all the app's module account addresses.
 func ModuleAccountAddrsLegacy() map[string]bool {
 	modAccAddrs := make(map[string]bool)
-	for acc := range maccPerms {
+	for acc := range GetMaccPerms() {
 		modAccAddrs[authtypes.NewModuleAddress(acc).String()] = true
 	}
 
diff --git a/simapp/app_test.go b/simapp/app_test.go
index aa83fa1f4f..9d2df38066 100644
--- a/simapp/app_test.go
+++ b/simapp/app_test.go
@@ -2,6 +2,7 @@ package simapp
 
 import (
 	"encoding/json"
+	"fmt"
 	"os"
 	"testing"
 
@@ -46,11 +47,11 @@ func TestSimAppExportAndBlockedAddrs(t *testing.T) {
 		AppOpts: simtestutil.NewAppOptionsWithFlagHome(DefaultNodeHome),
 	})
 
-	for acc := range maccPerms {
+	for acc := range BlockedAddresses() {
 		require.True(
 			t,
 			app.BankKeeper.BlockedAddr(app.AccountKeeper.GetModuleAddress(acc)),
-			"ensure that blocked addresses are properly set in bank keeper",
+			fmt.Sprintf("ensure that blocked addresses are properly set in bank keeper: %s should be blocked", acc),
 		)
 	}
 
@@ -63,11 +64,6 @@ func TestSimAppExportAndBlockedAddrs(t *testing.T) {
 	require.NoError(t, err, "ExportAppStateAndValidators should not have an error")
 }
 
-func TestGetMaccPerms(t *testing.T) {
-	dup := GetMaccPerms()
-	require.Equal(t, maccPerms, dup, "duplicated module account permissions differed from actual module account permissions")
-}
-
 func TestRunMigrations(t *testing.T) {
 	db := dbm.NewMemDB()
 	logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout))
diff --git a/x/bank/module.go b/x/bank/module.go
index 91a77ecbb5..41745f528f 100644
--- a/x/bank/module.go
+++ b/x/bank/module.go
@@ -12,7 +12,6 @@ import (
 	gwruntime "github.com/grpc-ecosystem/grpc-gateway/runtime"
 	"github.com/spf13/cobra"
 	abci "github.com/tendermint/tendermint/abci/types"
-	"github.com/tendermint/tendermint/crypto"
 
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/codec"
@@ -232,10 +231,9 @@ func provideModule(in bankInputs) bankOutputs {
 	// Default behavior for blockedAddresses is to regard any module mentioned in
 	// AccountKeeper's module account permissions as blocked.
 	blockedAddresses := make(map[string]bool)
-	if len(in.Config.BlockedModuleAccountsOverride) != 0 {
+	if len(in.Config.BlockedModuleAccountsOverride) > 0 {
 		for _, moduleName := range in.Config.BlockedModuleAccountsOverride {
-			addr := sdk.AccAddress(crypto.AddressHash([]byte(moduleName)))
-			blockedAddresses[addr.String()] = true
+			blockedAddresses[authtypes.NewModuleAddress(moduleName).String()] = true
 		}
 	} else {
 		for _, permission := range in.AccountKeeper.GetModulePermissions() {