From 35a82acbd07ecd51cb22c6621760ba64f46eca90 Mon Sep 17 00:00:00 2001 From: Ruaridh Date: Wed, 28 Oct 2020 01:06:44 +0000 Subject: [PATCH] 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 | 8 ++++++++ x/bep3/alias.go | 1 + x/bep3/keeper/integration_test.go | 7 +++++++ x/bep3/keeper/keeper.go | 14 +++++++++++++ x/bep3/keeper/keeper_test.go | 33 ++++++++++++++++++++++++++++++- x/bep3/types/expected_keepers.go | 2 ++ x/bep3/types/keys.go | 5 +++++ 7 files changed, 69 insertions(+), 1 deletion(-) diff --git a/x/bep3/abci.go b/x/bep3/abci.go index 14eb2980..364ed817 100644 --- a/x/bep3/abci.go +++ b/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) diff --git a/x/bep3/alias.go b/x/bep3/alias.go index 5acf9cc1..1c04d7c3 100644 --- a/x/bep3/alias.go +++ b/x/bep3/alias.go @@ -121,6 +121,7 @@ var ( DefaultMinBlockLock = types.DefaultMinBlockLock DefaultMaxBlockLock = types.DefaultMaxBlockLock DefaultPreviousBlockTime = types.DefaultPreviousBlockTime + ModulePermissionsUpgradeTime = types.ModulePermissionsUpgradeTime ) type ( diff --git a/x/bep3/keeper/integration_test.go b/x/bep3/keeper/integration_test.go index 4077fef0..705ed42c 100644 --- a/x/bep3/keeper/integration_test.go +++ b/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{ diff --git a/x/bep3/keeper/keeper.go b/x/bep3/keeper/keeper.go index 036230e1..42040f4d 100644 --- a/x/bep3/keeper/keeper.go +++ b/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 // ------------------------------------------ diff --git a/x/bep3/keeper/keeper_test.go b/x/bep3/keeper/keeper_test.go index e7f1ea43..f6449c7a 100644 --- a/x/bep3/keeper/keeper_test.go +++ b/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() diff --git a/x/bep3/types/expected_keepers.go b/x/bep3/types/expected_keepers.go index d6b9ba01..ef78fd5c 100644 --- a/x/bep3/types/expected_keepers.go +++ b/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 diff --git a/x/bep3/types/keys.go b/x/bep3/types/keys.go index b7b67de4..5ea7ab74 100644 --- a/x/bep3/types/keys.go +++ b/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