Module account permissions fix (#701)

* initial draft

* fix log msg formatting

* fix mod account type

* sync permissions at a block time

* update the update time

x/bep3/abci.go

@@ -1,12 +1,20 @@
 package bep3
 
 import (
+	"fmt"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
 // BeginBlocker on every block expires outdated atomic swaps and removes closed
 // swap from long term storage (default storage time of 1 week)
 func BeginBlocker(ctx sdk.Context, k Keeper) {
+	if ctx.BlockTime().After(ModulePermissionsUpgradeTime) {
+		err := k.EnsureModuleAccountPermissions(ctx)
+		if err != nil {
+			k.Logger(ctx).Error(fmt.Sprintf("couldn't update module account permissions: %v", err))
+		}
+	}
 	k.UpdateTimeBasedSupplyLimits(ctx)
 	k.UpdateExpiredAtomicSwaps(ctx)
 	k.DeleteClosedAtomicSwapsFromLongtermStorage(ctx)

x/bep3/alias.go

@@ -121,6 +121,7 @@ var (
 	DefaultMinBlockLock             = types.DefaultMinBlockLock
 	DefaultMaxBlockLock             = types.DefaultMaxBlockLock
 	DefaultPreviousBlockTime        = types.DefaultPreviousBlockTime
+	ModulePermissionsUpgradeTime    = types.ModulePermissionsUpgradeTime
 )
 
 type (

x/bep3/keeper/integration_test.go

@@ -4,6 +4,8 @@ import (
 	"time"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/auth"
+	authexported "github.com/cosmos/cosmos-sdk/x/auth/exported"
 
 	"github.com/tendermint/tendermint/crypto"
 	tmtime "github.com/tendermint/tendermint/types/time"
@@ -29,6 +31,11 @@ func c(denom string, amount int64) sdk.Coin { return sdk.NewInt64Coin(denom, amo
 func cs(coins ...sdk.Coin) sdk.Coins        { return sdk.NewCoins(coins...) }
 func ts(minOffset int) int64                { return tmtime.Now().Add(time.Duration(minOffset) * time.Minute).Unix() }
 
+func NewAuthGenStateFromAccs(accounts ...authexported.GenesisAccount) app.GenesisState {
+	authGenesis := auth.NewGenesisState(auth.DefaultParams(), accounts)
+	return app.GenesisState{auth.ModuleName: auth.ModuleCdc.MustMarshalJSON(authGenesis)}
+}
+
 func NewBep3GenStateMulti(deputyAddress sdk.AccAddress) app.GenesisState {
 	bep3Genesis := types.GenesisState{
 		Params: types.Params{

x/bep3/keeper/keeper.go

@@ -8,6 +8,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/store/prefix"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/params/subspace"
+	"github.com/cosmos/cosmos-sdk/x/supply"
 	"github.com/tendermint/tendermint/libs/log"
 
 	"github.com/kava-labs/kava/x/bep3/types"
@@ -46,6 +47,19 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger {
 	return ctx.Logger().With("module", fmt.Sprintf("x/%s", types.ModuleName))
 }
 
+// EnsureModuleAccountPermissions syncs the bep3 module account's permissions with those in the supply keeper.
+func (k Keeper) EnsureModuleAccountPermissions(ctx sdk.Context) error {
+	maccI := k.supplyKeeper.GetModuleAccount(ctx, types.ModuleName)
+	macc, ok := maccI.(*supply.ModuleAccount)
+	if !ok {
+		return fmt.Errorf("expected %s account to be a module account type", types.ModuleName)
+	}
+	_, perms := k.supplyKeeper.GetModuleAddressAndPermissions(types.ModuleName)
+	macc.Permissions = perms
+	k.supplyKeeper.SetModuleAccount(ctx, macc)
+	return nil
+}
+
 // ------------------------------------------
 //				Atomic Swaps
 // ------------------------------------------

x/bep3/keeper/keeper_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/stretchr/testify/suite"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/supply"
 
 	abci "github.com/tendermint/tendermint/abci/types"
 	tmtime "github.com/tendermint/tendermint/types/time"
@@ -30,7 +31,6 @@ func (suite *KeeperTestSuite) SetupTest() {
 	config := sdk.GetConfig()
 	app.SetBech32AddressPrefixes(config)
 	suite.ResetChain()
-	return
 }
 
 func (suite *KeeperTestSuite) ResetChain() {
@@ -43,6 +43,37 @@ func (suite *KeeperTestSuite) ResetChain() {
 	suite.keeper = keeper
 }
 
+func (suite *KeeperTestSuite) TestEnsureModuleAccountPermissions() {
+	suite.app.InitializeFromGenesisStates(
+		NewAuthGenStateFromAccs(
+			supply.NewEmptyModuleAccount(types.ModuleName), // no permisions
+		),
+	)
+	supplyKeeper := suite.app.GetSupplyKeeper()
+	testCoins := cs(c("busd", 1000_00_000_000))
+
+	// Ensure there are no minting and burning permissions.
+	// This calls mint/burn instead of checking permissions as the supply module can report permissions incorrectly.
+	// Using a panic check because MintCoins panics when permissions are incorrect.
+	suite.Panics(func() {
+		err := supplyKeeper.MintCoins(suite.ctx, types.ModuleName, testCoins)
+		if err != nil {
+			panic(err)
+		}
+		err = supplyKeeper.BurnCoins(suite.ctx, types.ModuleName, testCoins)
+		if err != nil {
+			panic(err)
+		}
+	})
+
+	err := suite.keeper.EnsureModuleAccountPermissions(suite.ctx)
+	suite.NoError(err)
+
+	err = supplyKeeper.MintCoins(suite.ctx, types.ModuleName, testCoins)
+	suite.NoError(err)
+	err = supplyKeeper.BurnCoins(suite.ctx, types.ModuleName, testCoins)
+	suite.NoError(err)
+}
 func (suite *KeeperTestSuite) TestGetSetAtomicSwap() {
 	suite.ResetChain()
 

x/bep3/types/expected_keepers.go

@@ -10,6 +10,8 @@ import (
 type SupplyKeeper interface {
 	GetModuleAddress(name string) sdk.AccAddress
 	GetModuleAccount(ctx sdk.Context, moduleName string) supplyexported.ModuleAccountI
+	GetModuleAddressAndPermissions(moduleName string) (sdk.AccAddress, []string)
+	SetModuleAccount(ctx sdk.Context, macc supplyexported.ModuleAccountI)
 
 	SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
 	SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error

x/bep3/types/keys.go

@@ -1,6 +1,8 @@
 package types
 
 import (
+	"time"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
@@ -26,6 +28,9 @@ const (
 
 // Key prefixes
 var (
+	// ModulePermissionsUpgradeTime is the block time after which the bep3 module account's permissions are synced with the supply module.
+	ModulePermissionsUpgradeTime time.Time = time.Date(2020, 11, 3, 10, 0, 0, 0, time.UTC)
+
 	AtomicSwapKeyPrefix             = []byte{0x00} // prefix for keys that store AtomicSwaps
 	AtomicSwapByBlockPrefix         = []byte{0x01} // prefix for keys of the AtomicSwapsByBlock index
 	AtomicSwapLongtermStoragePrefix = []byte{0x02} // prefix for keys of the AtomicSwapLongtermStorage index