From 31d901fed2d065bbd9f84d7b9c2dc5f113ac4a0b Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Wed, 3 Jul 2019 12:58:59 -0700 Subject: [PATCH 01/13] extended permissions for module accounts --- simapp/app.go | 13 +++++--- x/auth/test_common.go | 24 ++++++++++----- x/distribution/keeper/test_common.go | 9 ++++-- x/genaccounts/genesis_account.go | 32 +++++++++---------- x/gov/test_common.go | 8 +++-- x/mint/internal/keeper/test_common.go | 9 ++++-- x/slashing/app_test.go | 8 +++-- x/slashing/test_common.go | 8 +++-- x/staking/app_test.go | 8 +++-- x/staking/keeper/test_common.go | 8 +++-- x/supply/exported/exported.go | 3 +- x/supply/internal/keeper/account.go | 26 ++++++++-------- x/supply/internal/keeper/bank.go | 9 +++--- x/supply/internal/keeper/bank_test.go | 33 +++++++++++++++++--- x/supply/internal/keeper/keeper.go | 22 +++++-------- x/supply/internal/keeper/test_common.go | 13 ++++++-- x/supply/internal/types/account.go | 34 +++++++++++++-------- x/supply/internal/types/account_test.go | 32 +++++++++++++++++-- x/supply/internal/types/permissions.go | 15 +++++---- x/supply/internal/types/permissions_test.go | 33 ++++++++++++++++++++ 20 files changed, 245 insertions(+), 102 deletions(-) create mode 100644 x/supply/internal/types/permissions_test.go diff --git a/simapp/app.go b/simapp/app.go index 89d7c124bd78..9ebf9de23c52 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -144,15 +144,20 @@ func NewSimApp(logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest bo crisisSubspace := app.paramsKeeper.Subspace(crisis.DefaultParamspace) // account permissions - basicModuleAccs := []string{auth.FeeCollectorName, distr.ModuleName} - minterModuleAccs := []string{mint.ModuleName} - burnerModuleAccs := []string{staking.BondedPoolName, staking.NotBondedPoolName, gov.ModuleName} + maccPerms := map[string][]string{ + auth.FeeCollectorName: []string{supply.Basic}, + distr.ModuleName: []string{supply.Basic}, + mint.ModuleName: []string{supply.Minter}, + staking.BondedPoolName: []string{supply.Burner}, + staking.NotBondedPoolName: []string{supply.Burner}, + gov.ModuleName: []string{supply.Burner}, + } // add keepers app.accountKeeper = auth.NewAccountKeeper(app.cdc, app.keyAccount, authSubspace, auth.ProtoBaseAccount) app.bankKeeper = bank.NewBaseKeeper(app.accountKeeper, bankSubspace, bank.DefaultCodespace) app.supplyKeeper = supply.NewKeeper(app.cdc, app.keySupply, app.accountKeeper, - app.bankKeeper, supply.DefaultCodespace, basicModuleAccs, minterModuleAccs, burnerModuleAccs) + app.bankKeeper, supply.DefaultCodespace, maccPerms) stakingKeeper := staking.NewKeeper(app.cdc, app.keyStaking, app.tkeyStaking, app.supplyKeeper, stakingSubspace, staking.DefaultCodespace) app.mintKeeper = mint.NewKeeper(app.cdc, app.keyMint, mintSubspace, &stakingKeeper, app.supplyKeeper, auth.FeeCollectorName) diff --git a/x/auth/test_common.go b/x/auth/test_common.go index eef914aebe2b..6c674c8da842 100644 --- a/x/auth/test_common.go +++ b/x/auth/test_common.go @@ -3,9 +3,9 @@ package auth import ( abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/crypto" dbm "github.com/tendermint/tendermint/libs/db" "github.com/tendermint/tendermint/libs/log" - "github.com/tendermint/tendermint/crypto" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/store" @@ -25,22 +25,30 @@ type testInput struct { // moduleAccount defines an account for modules that holds coins on a pool type moduleAccount struct { *types.BaseAccount - Name string `json:"name"` // name of the module - Permission string `json:"permission"` // permission of module account (minter/burner/holder) + Name string `json:"name"` // name of the module + Permissions []string `json:"permissions"` // permissions of module account (minter/burner/holder) } +// HasPermission returns whether or not the module account has permission. +func (ma moduleAccount) HasPermission(permission string) bool { + for _, perm := range ma.Permissions { + if perm == permission { + return true + } + } + return false +} // GetName returns the the name of the holder's module func (ma moduleAccount) GetName() string { return ma.Name } -// GetPermission returns permission granted to the module account (holder/minter/burner) -func (ma moduleAccount) GetPermission() string { - return ma.Permission +// GetPermissions returns permissions granted to the module account (holder/minter/burner) +func (ma moduleAccount) GetPermissions() []string { + return ma.Permissions } - func setupTestInput() testInput { db := dbm.NewMemDB() @@ -128,7 +136,7 @@ func (sk DummySupplyKeeper) GetModuleAccount(ctx sdk.Context, moduleName string) macc := &moduleAccount{ BaseAccount: &baseAcc, Name: moduleName, - Permission: "basic", + Permissions: []string{"basic"}, } maccI := (sk.ak.NewAccount(ctx, macc)).(exported.ModuleAccountI) diff --git a/x/distribution/keeper/test_common.go b/x/distribution/keeper/test_common.go index e54655c91eba..43e255313141 100644 --- a/x/distribution/keeper/test_common.go +++ b/x/distribution/keeper/test_common.go @@ -120,8 +120,13 @@ func CreateTestInputAdvanced(t *testing.T, isCheckTx bool, initPower int64, ctx := sdk.NewContext(ms, abci.Header{ChainID: "foochainid"}, isCheckTx, log.NewNopLogger()) accountKeeper := auth.NewAccountKeeper(cdc, keyAcc, pk.Subspace(auth.DefaultParamspace), auth.ProtoBaseAccount) bankKeeper := bank.NewBaseKeeper(accountKeeper, pk.Subspace(bank.DefaultParamspace), bank.DefaultCodespace) - supplyKeeper := supply.NewKeeper(cdc, keySupply, accountKeeper, bankKeeper, supply.DefaultCodespace, - []string{auth.FeeCollectorName, types.ModuleName}, []string{}, []string{staking.NotBondedPoolName, staking.BondedPoolName}) + maccPerms := map[string][]string{ + auth.FeeCollectorName: []string{supply.Basic}, + types.ModuleName: []string{supply.Basic}, + staking.NotBondedPoolName: []string{supply.Burner}, + staking.BondedPoolName: []string{supply.Burner}, + } + supplyKeeper := supply.NewKeeper(cdc, keySupply, accountKeeper, bankKeeper, supply.DefaultCodespace, maccPerms) sk := staking.NewKeeper(cdc, keyStaking, tkeyStaking, supplyKeeper, pk.Subspace(staking.DefaultParamspace), staking.DefaultCodespace) sk.SetParams(ctx, staking.DefaultParams()) diff --git a/x/genaccounts/genesis_account.go b/x/genaccounts/genesis_account.go index bb5dc8ab7414..e816fabe5037 100644 --- a/x/genaccounts/genesis_account.go +++ b/x/genaccounts/genesis_account.go @@ -27,8 +27,8 @@ type GenesisAccount struct { EndTime int64 `json:"end_time"` // vesting end time (UNIX Epoch time) // module account fields - ModuleName string `json:"module_name"` // name of the module account - ModulePermission string `json:"module_permission"` // permission of module account + ModuleName string `json:"module_name"` // name of the module account + ModulePermissions []string `json:"module_permissions"` // permissions of module account } // Validate checks for errors on the vesting and module account parameters @@ -53,20 +53,20 @@ func (ga GenesisAccount) Validate() error { // NewGenesisAccountRaw creates a new GenesisAccount object func NewGenesisAccountRaw(address sdk.AccAddress, coins, vestingAmount sdk.Coins, vestingStartTime, vestingEndTime int64, - module, permission string) GenesisAccount { + module string, permissions ...string) GenesisAccount { return GenesisAccount{ - Address: address, - Coins: coins, - Sequence: 0, - AccountNumber: 0, // ignored set by the account keeper during InitGenesis - OriginalVesting: vestingAmount, - DelegatedFree: sdk.Coins{}, // ignored - DelegatedVesting: sdk.Coins{}, // ignored - StartTime: vestingStartTime, - EndTime: vestingEndTime, - ModuleName: module, - ModulePermission: permission, + Address: address, + Coins: coins, + Sequence: 0, + AccountNumber: 0, // ignored set by the account keeper during InitGenesis + OriginalVesting: vestingAmount, + DelegatedFree: sdk.Coins{}, // ignored + DelegatedVesting: sdk.Coins{}, // ignored + StartTime: vestingStartTime, + EndTime: vestingEndTime, + ModuleName: module, + ModulePermissions: permissions, } } @@ -102,7 +102,7 @@ func NewGenesisAccountI(acc authexported.Account) (GenesisAccount, error) { gacc.EndTime = acc.GetEndTime() case supplyexported.ModuleAccountI: gacc.ModuleName = acc.GetName() - gacc.ModulePermission = acc.GetPermission() + gacc.ModulePermissions = acc.GetPermissions() } return gacc, nil @@ -131,7 +131,7 @@ func (ga *GenesisAccount) ToAccount() auth.Account { // module accounts if ga.ModuleName != "" { - return supply.NewModuleAccount(bacc, ga.ModuleName, ga.ModulePermission) + return supply.NewModuleAccount(bacc, ga.ModuleName, ga.ModulePermissions...) } return bacc diff --git a/x/gov/test_common.go b/x/gov/test_common.go index d5ec1628bc2b..d524c6969660 100644 --- a/x/gov/test_common.go +++ b/x/gov/test_common.go @@ -59,8 +59,12 @@ func getMockApp(t *testing.T, numGenAccs int, genState GenesisState, genAccs []a bk := bank.NewBaseKeeper(mApp.AccountKeeper, mApp.ParamsKeeper.Subspace(bank.DefaultParamspace), bank.DefaultCodespace) - supplyKeeper := supply.NewKeeper(mApp.Cdc, keySupply, mApp.AccountKeeper, bk, supply.DefaultCodespace, - []string{}, []string{}, []string{types.ModuleName, staking.NotBondedPoolName, staking.BondedPoolName}) + maccPerms := map[string][]string{ + types.ModuleName: []string{supply.Burner}, + staking.NotBondedPoolName: []string{supply.Burner}, + staking.BondedPoolName: []string{supply.Burner}, + } + supplyKeeper := supply.NewKeeper(mApp.Cdc, keySupply, mApp.AccountKeeper, bk, supply.DefaultCodespace, maccPerms) sk := staking.NewKeeper(mApp.Cdc, keyStaking, tKeyStaking, supplyKeeper, pk.Subspace(staking.DefaultParamspace), staking.DefaultCodespace) keeper := NewKeeper(mApp.Cdc, keyGov, pk, pk.Subspace("testgov"), supplyKeeper, sk, DefaultCodespace, rtr) diff --git a/x/mint/internal/keeper/test_common.go b/x/mint/internal/keeper/test_common.go index f9be3d040b04..c4f33a229a35 100644 --- a/x/mint/internal/keeper/test_common.go +++ b/x/mint/internal/keeper/test_common.go @@ -57,8 +57,13 @@ func newTestInput(t *testing.T) testInput { paramsKeeper := params.NewKeeper(types.ModuleCdc, keyParams, tkeyParams, params.DefaultCodespace) accountKeeper := auth.NewAccountKeeper(types.ModuleCdc, keyAcc, paramsKeeper.Subspace(auth.DefaultParamspace), auth.ProtoBaseAccount) bankKeeper := bank.NewBaseKeeper(accountKeeper, paramsKeeper.Subspace(bank.DefaultParamspace), bank.DefaultCodespace) - supplyKeeper := supply.NewKeeper(types.ModuleCdc, keySupply, accountKeeper, bankKeeper, supply.DefaultCodespace, - []string{auth.FeeCollectorName}, []string{types.ModuleName}, []string{staking.NotBondedPoolName, staking.BondedPoolName}) + maccPerms := map[string][]string{ + auth.FeeCollectorName: []string{supply.Basic}, + types.ModuleName: []string{supply.Minter}, + staking.NotBondedPoolName: []string{supply.Burner}, + staking.BondedPoolName: []string{supply.Burner}, + } + supplyKeeper := supply.NewKeeper(types.ModuleCdc, keySupply, accountKeeper, bankKeeper, supply.DefaultCodespace, maccPerms) supplyKeeper.SetSupply(ctx, supply.NewSupply(sdk.Coins{})) stakingKeeper := staking.NewKeeper( diff --git a/x/slashing/app_test.go b/x/slashing/app_test.go index 40096b1aad76..f720107bfc44 100644 --- a/x/slashing/app_test.go +++ b/x/slashing/app_test.go @@ -36,8 +36,12 @@ func getMockApp(t *testing.T) (*mock.App, staking.Keeper, Keeper) { keySupply := sdk.NewKVStoreKey(supply.StoreKey) bankKeeper := bank.NewBaseKeeper(mapp.AccountKeeper, mapp.ParamsKeeper.Subspace(bank.DefaultParamspace), bank.DefaultCodespace) - supplyKeeper := supply.NewKeeper(mapp.Cdc, keySupply, mapp.AccountKeeper, bankKeeper, supply.DefaultCodespace, - []string{auth.FeeCollectorName}, []string{}, []string{staking.NotBondedPoolName, staking.BondedPoolName}) + maccPerms := map[string][]string{ + auth.FeeCollectorName: []string{supply.Basic}, + staking.NotBondedPoolName: []string{supply.Burner}, + staking.BondedPoolName: []string{supply.Burner}, + } + supplyKeeper := supply.NewKeeper(mapp.Cdc, keySupply, mapp.AccountKeeper, bankKeeper, supply.DefaultCodespace, maccPerms) stakingKeeper := staking.NewKeeper(mapp.Cdc, keyStaking, tkeyStaking, supplyKeeper, mapp.ParamsKeeper.Subspace(staking.DefaultParamspace), staking.DefaultCodespace) keeper := NewKeeper(mapp.Cdc, keySlashing, stakingKeeper, mapp.ParamsKeeper.Subspace(DefaultParamspace), DefaultCodespace) mapp.Router().AddRoute(staking.RouterKey, staking.NewHandler(stakingKeeper)) diff --git a/x/slashing/test_common.go b/x/slashing/test_common.go index 310f28f3e4d8..80e00302e660 100644 --- a/x/slashing/test_common.go +++ b/x/slashing/test_common.go @@ -77,8 +77,12 @@ func createTestInput(t *testing.T, defaults Params) (sdk.Context, bank.Keeper, s accountKeeper := auth.NewAccountKeeper(cdc, keyAcc, paramsKeeper.Subspace(auth.DefaultParamspace), auth.ProtoBaseAccount) bk := bank.NewBaseKeeper(accountKeeper, paramsKeeper.Subspace(bank.DefaultParamspace), bank.DefaultCodespace) - supplyKeeper := supply.NewKeeper(cdc, keySupply, accountKeeper, bk, supply.DefaultCodespace, - []string{auth.FeeCollectorName}, []string{}, []string{staking.NotBondedPoolName, staking.BondedPoolName}) + maccPerms := map[string][]string{ + auth.FeeCollectorName: []string{supply.Basic}, + staking.NotBondedPoolName: []string{supply.Burner}, + staking.BondedPoolName: []string{supply.Burner}, + } + supplyKeeper := supply.NewKeeper(cdc, keySupply, accountKeeper, bk, supply.DefaultCodespace, maccPerms) totalSupply := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, initTokens.MulRaw(int64(len(addrs))))) supplyKeeper.SetSupply(ctx, supply.NewSupply(totalSupply)) diff --git a/x/staking/app_test.go b/x/staking/app_test.go index 2cfc14efda94..2f538bd3bf27 100644 --- a/x/staking/app_test.go +++ b/x/staking/app_test.go @@ -26,8 +26,12 @@ func getMockApp(t *testing.T) (*mock.App, Keeper) { keySupply := sdk.NewKVStoreKey(supply.StoreKey) bankKeeper := bank.NewBaseKeeper(mApp.AccountKeeper, mApp.ParamsKeeper.Subspace(bank.DefaultParamspace), bank.DefaultCodespace) - supplyKeeper := supply.NewKeeper(mApp.Cdc, keySupply, mApp.AccountKeeper, bankKeeper, supply.DefaultCodespace, - []string{auth.FeeCollectorName}, []string{}, []string{types.NotBondedPoolName, types.BondedPoolName}) + maccPerms := map[string][]string{ + auth.FeeCollectorName: []string{supply.Basic}, + types.NotBondedPoolName: []string{supply.Burner}, + types.BondedPoolName: []string{supply.Burner}, + } + supplyKeeper := supply.NewKeeper(mApp.Cdc, keySupply, mApp.AccountKeeper, bankKeeper, supply.DefaultCodespace, maccPerms) keeper := NewKeeper(mApp.Cdc, keyStaking, tkeyStaking, supplyKeeper, mApp.ParamsKeeper.Subspace(DefaultParamspace), DefaultCodespace) mApp.Router().AddRoute(RouterKey, NewHandler(keeper)) diff --git a/x/staking/keeper/test_common.go b/x/staking/keeper/test_common.go index 28a9944f6192..aecf52f8547c 100644 --- a/x/staking/keeper/test_common.go +++ b/x/staking/keeper/test_common.go @@ -124,8 +124,12 @@ func CreateTestInput(t *testing.T, isCheckTx bool, initPower int64) (sdk.Context bank.DefaultCodespace, ) - supplyKeeper := supply.NewKeeper(cdc, keySupply, accountKeeper, bk, supply.DefaultCodespace, - []string{auth.FeeCollectorName}, []string{}, []string{types.NotBondedPoolName, types.BondedPoolName}) + maccPerms := map[string][]string{ + auth.FeeCollectorName: []string{supply.Basic}, + types.NotBondedPoolName: []string{supply.Burner}, + types.BondedPoolName: []string{supply.Burner}, + } + supplyKeeper := supply.NewKeeper(cdc, keySupply, accountKeeper, bk, supply.DefaultCodespace, maccPerms) initTokens := sdk.TokensFromConsensusPower(initPower) initCoins := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, initTokens)) diff --git a/x/supply/exported/exported.go b/x/supply/exported/exported.go index c8f701ee4219..3139836d9555 100644 --- a/x/supply/exported/exported.go +++ b/x/supply/exported/exported.go @@ -6,5 +6,6 @@ import "github.com/cosmos/cosmos-sdk/x/auth/exported" type ModuleAccountI interface { exported.Account GetName() string - GetPermission() string + GetPermissions() []string + HasPermission(string) bool } diff --git a/x/supply/internal/keeper/account.go b/x/supply/internal/keeper/account.go index 96a4ea3f52f7..4fea4f5103ed 100644 --- a/x/supply/internal/keeper/account.go +++ b/x/supply/internal/keeper/account.go @@ -6,7 +6,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/supply/internal/types" ) -// GetModuleAddress returns a an address based on the name +// GetModuleAddress returns a an address based on the name func (k Keeper) GetModuleAddress(moduleName string) sdk.AccAddress { permAddr, ok := k.permAddrs[moduleName] if !ok { @@ -15,20 +15,20 @@ func (k Keeper) GetModuleAddress(moduleName string) sdk.AccAddress { return permAddr.address } -// GetModuleAddressAndPermission returns an address and permission based on the name -func (k Keeper) GetModuleAddressAndPermission(moduleName string) (addr sdk.AccAddress, permission string) { +// GetModuleAddressAndPermissions returns an address and permissions based on the name +func (k Keeper) GetModuleAddressAndPermissions(moduleName string) (addr sdk.AccAddress, permissions []string) { permAddr, ok := k.permAddrs[moduleName] if !ok { - return nil, "" + return addr, permissions } - return permAddr.address, permAddr.permission + return permAddr.address, permAddr.permissions } -// GetModuleAccount gets the module account to the auth account store -func (k Keeper) GetModuleAccountAndPermission(ctx sdk.Context, moduleName string) (exported.ModuleAccountI, string) { - addr, perm := k.GetModuleAddressAndPermission(moduleName) +// GetModuleAccountiAndPermissions gets the module account to the auth account store +func (k Keeper) GetModuleAccountAndPermissions(ctx sdk.Context, moduleName string) (exported.ModuleAccountI, []string) { + addr, perms := k.GetModuleAddressAndPermissions(moduleName) if addr == nil { - return nil, "" + return nil, []string{} } acc := k.ak.GetAccount(ctx, addr) @@ -37,20 +37,20 @@ func (k Keeper) GetModuleAccountAndPermission(ctx sdk.Context, moduleName string if !ok { panic("account is not a module account") } - return macc, perm + return macc, perms } // create a new module account - macc := types.NewEmptyModuleAccount(moduleName, perm) + macc := types.NewEmptyModuleAccount(moduleName, perms...) maccI := (k.ak.NewAccount(ctx, macc)).(exported.ModuleAccountI) // set the account number k.SetModuleAccount(ctx, maccI) - return maccI, perm + return maccI, perms } // GetModuleAccount gets the module account to the auth account store func (k Keeper) GetModuleAccount(ctx sdk.Context, moduleName string) exported.ModuleAccountI { - acc, _ := k.GetModuleAccountAndPermission(ctx, moduleName) + acc, _ := k.GetModuleAccountAndPermissions(ctx, moduleName) return acc } diff --git a/x/supply/internal/keeper/bank.go b/x/supply/internal/keeper/bank.go index f8277566cee5..665bbcf3836b 100644 --- a/x/supply/internal/keeper/bank.go +++ b/x/supply/internal/keeper/bank.go @@ -84,13 +84,13 @@ func (k Keeper) MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) sdk } // create the account if it doesn't yet exist - acc, perm := k.GetModuleAccountAndPermission(ctx, moduleName) + acc, _ := k.GetModuleAccountAndPermissions(ctx, moduleName) addr := acc.GetAddress() if addr == nil { return sdk.ErrUnknownAddress(fmt.Sprintf("module account %s does not exist", moduleName)) } - if perm != types.Minter { + if !acc.HasPermission(types.Minter) { panic(fmt.Sprintf("Account %s does not have permissions to mint tokens", moduleName)) } @@ -116,12 +116,13 @@ func (k Keeper) BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) sdk panic("cannot burn empty coins") } - addr, perm := k.GetModuleAddressAndPermission(moduleName) + acc, _ := k.GetModuleAccountAndPermissions(ctx, moduleName) + addr := acc.GetAddress() if addr == nil { return sdk.ErrUnknownAddress(fmt.Sprintf("module account %s does not exist", moduleName)) } - if perm != types.Burner { + if !acc.HasPermission(types.Burner) { panic(fmt.Sprintf("Account %s does not have permissions to burn tokens", moduleName)) } diff --git a/x/supply/internal/keeper/bank_test.go b/x/supply/internal/keeper/bank_test.go index e7c33e9a7b21..0f16faa56beb 100644 --- a/x/supply/internal/keeper/bank_test.go +++ b/x/supply/internal/keeper/bank_test.go @@ -12,9 +12,10 @@ import ( const initialPower = int64(100) var ( - holderAcc = types.NewEmptyModuleAccount(types.Basic, types.Basic) - burnerAcc = types.NewEmptyModuleAccount(types.Burner, types.Burner) - minterAcc = types.NewEmptyModuleAccount(types.Minter, types.Minter) + holderAcc = types.NewEmptyModuleAccount(types.Basic, types.Basic) + burnerAcc = types.NewEmptyModuleAccount(types.Burner, types.Burner) + minterAcc = types.NewEmptyModuleAccount(types.Minter, types.Minter) + multiPermAcc = types.NewEmptyModuleAccount(multiPerm, types.Basic, types.Burner, types.Minter) initTokens = sdk.TokensFromConsensusPower(initialPower) initCoins = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, initTokens)) @@ -78,6 +79,7 @@ func TestMintCoins(t *testing.T) { keeper.SetModuleAccount(ctx, burnerAcc) keeper.SetModuleAccount(ctx, minterAcc) + keeper.SetModuleAccount(ctx, multiPermAcc) initialSupply := keeper.GetSupply(ctx) @@ -88,6 +90,14 @@ func TestMintCoins(t *testing.T) { require.Equal(t, initCoins, getCoinsByName(ctx, keeper, types.Minter)) require.Equal(t, initialSupply.Total.Add(initCoins), keeper.GetSupply(ctx).Total) + // test same functionality on module account with multiple permissions + initialSupply = keeper.GetSupply(ctx) + + err = keeper.MintCoins(ctx, multiPermAcc.GetName(), initCoins) + require.NoError(t, err) + require.Equal(t, initCoins, getCoinsByName(ctx, keeper, multiPermAcc.GetName())) + require.Equal(t, initialSupply.Total.Add(initCoins), keeper.GetSupply(ctx).Total) + require.Panics(t, func() { keeper.MintCoins(ctx, types.Burner, initCoins) }) } @@ -103,8 +113,7 @@ func TestBurnCoins(t *testing.T) { initialSupply.Inflate(initCoins) keeper.SetSupply(ctx, initialSupply) - err = keeper.BurnCoins(ctx, "", initCoins) - require.Error(t, err) + require.Panics(t, func() { keeper.BurnCoins(ctx, "", initCoins) }) err = keeper.BurnCoins(ctx, types.Burner, initialSupply.Total) require.Error(t, err) @@ -113,4 +122,18 @@ func TestBurnCoins(t *testing.T) { require.NoError(t, err) require.Equal(t, sdk.Coins(nil), getCoinsByName(ctx, keeper, types.Burner)) require.Equal(t, initialSupply.Total.Sub(initCoins), keeper.GetSupply(ctx).Total) + + // test same functionality on module account with multiple permissions + initialSupply = keeper.GetSupply(ctx) + initialSupply.Inflate(initCoins) + keeper.SetSupply(ctx, initialSupply) + + err = multiPermAcc.SetCoins(initCoins) + require.NoError(t, err) + keeper.SetModuleAccount(ctx, multiPermAcc) + + err = keeper.BurnCoins(ctx, multiPermAcc.GetName(), initCoins) + require.NoError(t, err) + require.Equal(t, sdk.Coins(nil), getCoinsByName(ctx, keeper, multiPermAcc.GetName())) + require.Equal(t, initialSupply.Total.Sub(initCoins), keeper.GetSupply(ctx).Total) } diff --git a/x/supply/internal/keeper/keeper.go b/x/supply/internal/keeper/keeper.go index 22980d8be879..1d7401dac3e8 100644 --- a/x/supply/internal/keeper/keeper.go +++ b/x/supply/internal/keeper/keeper.go @@ -20,32 +20,26 @@ type Keeper struct { } type permAddr struct { - permission string // basic/minter/burner - address sdk.AccAddress + permissions []string // basic/minter/burner + address sdk.AccAddress } // NewpermAddr creates a new permAddr object -func newPermAddr(permission, name string) permAddr { +func newPermAddr(name string, permissions []string) permAddr { return permAddr{ - permission: permission, - address: types.NewModuleAddress(name), + permissions: permissions, + address: types.NewModuleAddress(name), } } // NewKeeper creates a new Keeper instance func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, ak types.AccountKeeper, bk types.BankKeeper, - codespace sdk.CodespaceType, basicModuleAccs, minterModuleAccs, burnerModuleAccs []string) Keeper { + codespace sdk.CodespaceType, maccPerms map[string][]string) Keeper { // set the addresses permAddrs := make(map[string]permAddr) - for _, name := range basicModuleAccs { - permAddrs[name] = newPermAddr(types.Basic, name) - } - for _, name := range minterModuleAccs { - permAddrs[name] = newPermAddr(types.Minter, name) - } - for _, name := range burnerModuleAccs { - permAddrs[name] = newPermAddr(types.Burner, name) + for name, perms := range maccPerms { + permAddrs[name] = newPermAddr(name, perms) } return Keeper{ diff --git a/x/supply/internal/keeper/test_common.go b/x/supply/internal/keeper/test_common.go index 24e39b7c70db..2fc8ee9bded9 100644 --- a/x/supply/internal/keeper/test_common.go +++ b/x/supply/internal/keeper/test_common.go @@ -21,6 +21,9 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) +// nolint: deadcode unused +var multiPerm = "multiple permissions account" + // nolint: deadcode unused // create a codec used only for testing func makeTestCodec() *codec.Codec { @@ -71,7 +74,13 @@ func createTestInput(t *testing.T, isCheckTx bool, initPower int64, nAccs int64) initialCoins := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, valTokens)) createTestAccs(ctx, int(nAccs), initialCoins, &ak) - keeper := NewKeeper(cdc, keySupply, ak, bk, DefaultCodespace, []string{types.Basic}, []string{types.Minter}, []string{types.Burner}) + maccPerms := map[string][]string{ + types.Basic: []string{types.Basic}, + types.Minter: []string{types.Minter}, + types.Burner: []string{types.Burner}, + multiPerm: []string{types.Basic, types.Minter, types.Burner}, + } + keeper := NewKeeper(cdc, keySupply, ak, bk, DefaultCodespace, maccPerms) totalSupply := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, valTokens.MulRaw(nAccs))) keeper.SetSupply(ctx, types.NewSupply(totalSupply)) @@ -91,4 +100,4 @@ func createTestAccs(ctx sdk.Context, numAccs int, initialCoins sdk.Coins, ak *au ak.SetAccount(ctx, &acc) } return -} \ No newline at end of file +} diff --git a/x/supply/internal/types/account.go b/x/supply/internal/types/account.go index 55a1bf1bd237..95047031810e 100644 --- a/x/supply/internal/types/account.go +++ b/x/supply/internal/types/account.go @@ -17,8 +17,8 @@ var _ exported.ModuleAccountI = (*ModuleAccount)(nil) // ModuleAccount defines an account for modules that holds coins on a pool type ModuleAccount struct { *authtypes.BaseAccount - Name string `json:"name"` // name of the module - Permission string `json:"permission"` // permission of module account (minter/burner/holder) + Name string `json:"name"` // name of the module + Permissions []string `json:"permissions"` // permissions of module account (minter/burner/holder) } // NewModuleAddress creates an AccAddress from the hash of the module's name @@ -26,44 +26,54 @@ func NewModuleAddress(name string) sdk.AccAddress { return sdk.AccAddress(crypto.AddressHash([]byte(name))) } -func NewEmptyModuleAccount(name, permission string) *ModuleAccount { +func NewEmptyModuleAccount(name string, permissions ...string) *ModuleAccount { moduleAddress := NewModuleAddress(name) baseAcc := authtypes.NewBaseAccountWithAddress(moduleAddress) - if err := validatePermissions(permission); err != nil { + if err := validatePermissions(permissions); err != nil { panic(err) } return &ModuleAccount{ BaseAccount: &baseAcc, Name: name, - Permission: permission, + Permissions: permissions, } } // NewModuleAccount creates a new ModuleAccount instance func NewModuleAccount(ba *authtypes.BaseAccount, - name, permission string) *ModuleAccount { + name string, permissions ...string) *ModuleAccount { - if err := validatePermissions(permission); err != nil { + if err := validatePermissions(permissions); err != nil { panic(err) } return &ModuleAccount{ BaseAccount: ba, Name: name, - Permission: permission, + Permissions: permissions, } } +// HasPermission returns whether or not the module account has permission. +func (ma ModuleAccount) HasPermission(permission string) bool { + for _, perm := range ma.Permissions { + if perm == permission { + return true + } + } + return false +} + // GetName returns the the name of the holder's module func (ma ModuleAccount) GetName() string { return ma.Name } // GetPermission returns permission granted to the module account (holder/minter/burner) -func (ma ModuleAccount) GetPermission() string { - return ma.Permission +func (ma ModuleAccount) GetPermissions() []string { + return ma.Permissions } // SetPubKey - Implements Account @@ -94,7 +104,7 @@ func (ma ModuleAccount) MarshalYAML() (interface{}, error) { AccountNumber uint64 Sequence uint64 Name string - Permission string + Permissions []string }{ Address: ma.Address, Coins: ma.Coins, @@ -102,7 +112,7 @@ func (ma ModuleAccount) MarshalYAML() (interface{}, error) { AccountNumber: ma.AccountNumber, Sequence: ma.Sequence, Name: ma.Name, - Permission: ma.Permission, + Permissions: ma.Permissions, }) if err != nil { diff --git a/x/supply/internal/types/account_test.go b/x/supply/internal/types/account_test.go index d239b53e6667..a3928c515ce5 100644 --- a/x/supply/internal/types/account_test.go +++ b/x/supply/internal/types/account_test.go @@ -13,7 +13,7 @@ import ( func TestModuleAccountMarshalYAML(t *testing.T) { name := "test" - moduleAcc := NewEmptyModuleAccount(name, Basic) + moduleAcc := NewEmptyModuleAccount(name, Basic, Minter, Burner) moduleAddress := sdk.AccAddress(crypto.AddressHash([]byte(name))) bs, err := yaml.Marshal(moduleAcc) require.NoError(t, err) @@ -25,9 +25,35 @@ func TestModuleAccountMarshalYAML(t *testing.T) { accountnumber: 0 sequence: 0 name: %s - permission: %s -`, moduleAddress, name, Basic) + permissions: + - %s + - %s + - %s +`, moduleAddress, name, Basic, Minter, Burner) require.Equal(t, want, string(bs)) require.Equal(t, want, moduleAcc.String()) } + +func TestHasPermissions(t *testing.T) { + name := "test" + macc := NewEmptyModuleAccount(name, Basic, Minter, Burner) + cases := []struct { + permission string + expectHas bool + }{ + {Basic, true}, + {Minter, true}, + {Burner, true}, + {"other", false}, + } + + for i, tc := range cases { + hasPerm := macc.HasPermission(tc.permission) + if tc.expectHas { + require.True(t, hasPerm, "test case #%d", i) + } else { + require.False(t, hasPerm, "test case #%d", i) + } + } +} diff --git a/x/supply/internal/types/permissions.go b/x/supply/internal/types/permissions.go index d2198e039498..2a50360ba325 100644 --- a/x/supply/internal/types/permissions.go +++ b/x/supply/internal/types/permissions.go @@ -10,11 +10,14 @@ const ( ) // validate the input permissions -func validatePermissions(permission string) error { - switch permission { - case Basic, Minter, Burner: - return nil - default: - return fmt.Errorf("invalid module permission %s", permission) +func validatePermissions(permissions []string) error { + for _, perm := range permissions { + switch perm { + case Basic, Minter, Burner: + continue + default: + return fmt.Errorf("invalid module permission %s", perm) + } } + return nil } diff --git a/x/supply/internal/types/permissions_test.go b/x/supply/internal/types/permissions_test.go new file mode 100644 index 000000000000..88afdc56c6b4 --- /dev/null +++ b/x/supply/internal/types/permissions_test.go @@ -0,0 +1,33 @@ +package types + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestValidatePermissions(t *testing.T) { + cases := []struct { + name string + permissions []string + expectPass bool + }{ + {"no permissions", []string{}, true}, + {"one permission", []string{Basic}, true}, + {"multiple permissions", []string{Basic, Minter, Burner}, true}, + {"invalid permission", []string{"other"}, false}, + {"multiple invalid permissions", []string{Burner, "other", "invalid"}, false}, + } + + for i, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := validatePermissions(tc.permissions) + if tc.expectPass { + require.NoError(t, err, "test case #%d", i) + } else { + require.Error(t, err, "test case #%d", i) + } + }) + } + +} From cef1308935014214b088e2ecf7ec83bb9bcb1bf3 Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Wed, 3 Jul 2019 13:18:23 -0700 Subject: [PATCH 02/13] updated docs and added pending log --- .pending/breaking/sdk/4652-Extend-supply-p | 1 + docs/spec/supply/01_concepts.md | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) create mode 100644 .pending/breaking/sdk/4652-Extend-supply-p diff --git a/.pending/breaking/sdk/4652-Extend-supply-p b/.pending/breaking/sdk/4652-Extend-supply-p new file mode 100644 index 000000000000..2426ca2bb8d0 --- /dev/null +++ b/.pending/breaking/sdk/4652-Extend-supply-p @@ -0,0 +1 @@ +#4652 Extend supply permissions to allow for multiple permissions per ModuleAccount. diff --git a/docs/spec/supply/01_concepts.md b/docs/spec/supply/01_concepts.md index a9dd83067248..f93bcdfd2008 100644 --- a/docs/spec/supply/01_concepts.md +++ b/docs/spec/supply/01_concepts.md @@ -32,7 +32,8 @@ The `ModuleAccount` interface is defined as follows: type ModuleAccount interface { auth.Account // same methods as the Account interface GetName() string // name of the module; used to obtain the address - GetPermission() string // permission of module account (minter/burner/holder) + GetPermissions() string // permissions of module account (minter/burner/holder) + HasPermission(string) bool } ``` @@ -51,7 +52,7 @@ Each `ModuleAccount` has a different set of permissions that provide different object capabilities to perform certain actions. Permissions need to be registered upon the creation of the supply `Keeper` so that every time a `ModuleAccount` calls the allowed functions, the `Keeper` can lookup the -permission to that specific account and perform or not the action. +permissions to that specific account and perform or not the action. The available permissions are: From d4eeedab6947363f608a8e7f8b6424d50b87b4e1 Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Wed, 3 Jul 2019 15:29:34 -0700 Subject: [PATCH 03/13] address some of @fedekunze pr comments, move permaddr to types/ --- docs/spec/supply/01_concepts.md | 6 +-- x/supply/internal/keeper/account.go | 6 +-- x/supply/internal/keeper/keeper.go | 19 ++------- x/supply/internal/types/account.go | 4 +- x/supply/internal/types/permissions.go | 47 ++++++++++++++++++++- x/supply/internal/types/permissions_test.go | 21 ++++++++- 6 files changed, 77 insertions(+), 26 deletions(-) diff --git a/docs/spec/supply/01_concepts.md b/docs/spec/supply/01_concepts.md index f93bcdfd2008..2375f18b13b5 100644 --- a/docs/spec/supply/01_concepts.md +++ b/docs/spec/supply/01_concepts.md @@ -30,9 +30,9 @@ The `ModuleAccount` interface is defined as follows: ```go type ModuleAccount interface { - auth.Account // same methods as the Account interface - GetName() string // name of the module; used to obtain the address - GetPermissions() string // permissions of module account (minter/burner/holder) + auth.Account // same methods as the Account interface + GetName() string // name of the module; used to obtain the address + GetPermissions() []string // permissions of module account HasPermission(string) bool } ``` diff --git a/x/supply/internal/keeper/account.go b/x/supply/internal/keeper/account.go index 4fea4f5103ed..7968cf568daf 100644 --- a/x/supply/internal/keeper/account.go +++ b/x/supply/internal/keeper/account.go @@ -12,7 +12,7 @@ func (k Keeper) GetModuleAddress(moduleName string) sdk.AccAddress { if !ok { return nil } - return permAddr.address + return permAddr.GetAddress() } // GetModuleAddressAndPermissions returns an address and permissions based on the name @@ -21,10 +21,10 @@ func (k Keeper) GetModuleAddressAndPermissions(moduleName string) (addr sdk.AccA if !ok { return addr, permissions } - return permAddr.address, permAddr.permissions + return permAddr.GetAddress(), permAddr.GetPermissions() } -// GetModuleAccountiAndPermissions gets the module account to the auth account store +// GetModuleAccountAndPermissions gets the module account to the auth account store func (k Keeper) GetModuleAccountAndPermissions(ctx sdk.Context, moduleName string) (exported.ModuleAccountI, []string) { addr, perms := k.GetModuleAddressAndPermissions(moduleName) if addr == nil { diff --git a/x/supply/internal/keeper/keeper.go b/x/supply/internal/keeper/keeper.go index 1d7401dac3e8..82ff7af6553e 100644 --- a/x/supply/internal/keeper/keeper.go +++ b/x/supply/internal/keeper/keeper.go @@ -16,20 +16,7 @@ type Keeper struct { storeKey sdk.StoreKey ak types.AccountKeeper bk types.BankKeeper - permAddrs map[string]permAddr -} - -type permAddr struct { - permissions []string // basic/minter/burner - address sdk.AccAddress -} - -// NewpermAddr creates a new permAddr object -func newPermAddr(name string, permissions []string) permAddr { - return permAddr{ - permissions: permissions, - address: types.NewModuleAddress(name), - } + permAddrs map[string]types.PermAddr } // NewKeeper creates a new Keeper instance @@ -37,9 +24,9 @@ func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, ak types.AccountKeeper, bk ty codespace sdk.CodespaceType, maccPerms map[string][]string) Keeper { // set the addresses - permAddrs := make(map[string]permAddr) + permAddrs := make(map[string]types.PermAddr) for name, perms := range maccPerms { - permAddrs[name] = newPermAddr(name, perms) + permAddrs[name] = types.NewPermAddr(name, perms) } return Keeper{ diff --git a/x/supply/internal/types/account.go b/x/supply/internal/types/account.go index 95047031810e..dd58389a4c40 100644 --- a/x/supply/internal/types/account.go +++ b/x/supply/internal/types/account.go @@ -18,7 +18,7 @@ var _ exported.ModuleAccountI = (*ModuleAccount)(nil) type ModuleAccount struct { *authtypes.BaseAccount Name string `json:"name"` // name of the module - Permissions []string `json:"permissions"` // permissions of module account (minter/burner/holder) + Permissions []string `json:"permissions"` // permissions of module account } // NewModuleAddress creates an AccAddress from the hash of the module's name @@ -71,7 +71,7 @@ func (ma ModuleAccount) GetName() string { return ma.Name } -// GetPermission returns permission granted to the module account (holder/minter/burner) +// GetPermissions returns permissions granted to the module account func (ma ModuleAccount) GetPermissions() []string { return ma.Permissions } diff --git a/x/supply/internal/types/permissions.go b/x/supply/internal/types/permissions.go index 2a50360ba325..1432cc28c2cb 100644 --- a/x/supply/internal/types/permissions.go +++ b/x/supply/internal/types/permissions.go @@ -1,6 +1,10 @@ package types -import "fmt" +import ( + "fmt" + + sdk "github.com/cosmos/cosmos-sdk/types" +) // permissions const ( @@ -9,6 +13,47 @@ const ( Burner = "burner" ) +// PermAddr defines the permissions for an address +type PermAddr struct { + permissions []string + address sdk.AccAddress +} + +// NewPermAddr creates a new PermAddr object +func NewPermAddr(name string, permissions []string) PermAddr { + return PermAddr{ + permissions: permissions, + address: NewModuleAddress(name), + } +} + +// AddPermissions adds the permission to the list of granted permissions. +func (pa *PermAddr) AddPermissions(permissions ...string) { + pa.permissions = append(pa.permissions, permissions...) +} + +// RemovePermission removes the permission from the list of granted permissions +// or returns an error if the permission is has not been granted. +func (pa *PermAddr) RemovePermission(permission string) error { + for i, perm := range pa.permissions { + if perm == permission { + pa.permissions = append(pa.permissions[:i], pa.permissions[i+1:]...) + return nil + } + } + return fmt.Errorf("cannot remove non granted permission %s", permission) +} + +// GetAddress returns the address of the PermAddr object +func (pa PermAddr) GetAddress() sdk.AccAddress { + return pa.address +} + +// GetPermissions returns the permissions granted to the address +func (pa PermAddr) GetPermissions() []string { + return pa.permissions +} + // validate the input permissions func validatePermissions(permissions []string) error { for _, perm := range permissions { diff --git a/x/supply/internal/types/permissions_test.go b/x/supply/internal/types/permissions_test.go index 88afdc56c6b4..038f93a7f2b8 100644 --- a/x/supply/internal/types/permissions_test.go +++ b/x/supply/internal/types/permissions_test.go @@ -6,6 +6,26 @@ import ( "github.com/stretchr/testify/require" ) +func TestRemovePermissions(t *testing.T) { + name := "test" + permAddr := NewPermAddr(name, []string{}) + require.Empty(t, permAddr.GetPermissions()) + + permAddr.AddPermissions(Basic, Minter, Burner) + require.Equal(t, []string{Basic, Minter, Burner}, permAddr.GetPermissions(), "did not add permissions") + + err := permAddr.RemovePermission("random") + require.Error(t, err, "did not error on removing nonexistent permission") + + err = permAddr.RemovePermission(Burner) + require.NoError(t, err, "failed to remove permission") + require.Equal(t, []string{Basic, Minter}, permAddr.GetPermissions(), "does not have correct permissions") + + err = permAddr.RemovePermission(Basic) + require.NoError(t, err, "failed to remove permission") + require.Equal(t, []string{Minter}, permAddr.GetPermissions(), "does not have correct permissions") +} + func TestValidatePermissions(t *testing.T) { cases := []struct { name string @@ -29,5 +49,4 @@ func TestValidatePermissions(t *testing.T) { } }) } - } From bdf58f3173cc55af1a321530140cca46fb62e690 Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Wed, 3 Jul 2019 16:04:02 -0700 Subject: [PATCH 04/13] add staking permission --- x/distribution/keeper/test_common.go | 8 ++++---- x/gov/test_common.go | 8 ++++---- x/slashing/app_test.go | 8 ++++---- x/slashing/test_common.go | 8 ++++---- x/staking/app_test.go | 8 ++++---- x/staking/keeper/test_common.go | 8 ++++---- x/supply/alias.go | 1 + x/supply/internal/keeper/bank.go | 14 ++++++++++++-- x/supply/internal/types/permissions.go | 9 +++++---- 9 files changed, 42 insertions(+), 30 deletions(-) diff --git a/x/distribution/keeper/test_common.go b/x/distribution/keeper/test_common.go index 43e255313141..26aaa5f7351e 100644 --- a/x/distribution/keeper/test_common.go +++ b/x/distribution/keeper/test_common.go @@ -123,8 +123,8 @@ func CreateTestInputAdvanced(t *testing.T, isCheckTx bool, initPower int64, maccPerms := map[string][]string{ auth.FeeCollectorName: []string{supply.Basic}, types.ModuleName: []string{supply.Basic}, - staking.NotBondedPoolName: []string{supply.Burner}, - staking.BondedPoolName: []string{supply.Burner}, + staking.NotBondedPoolName: []string{supply.Burner, supply.Staking}, + staking.BondedPoolName: []string{supply.Burner, supply.Staking}, } supplyKeeper := supply.NewKeeper(cdc, keySupply, accountKeeper, bankKeeper, supply.DefaultCodespace, maccPerms) @@ -145,8 +145,8 @@ func CreateTestInputAdvanced(t *testing.T, isCheckTx bool, initPower int64, // create module accounts feeCollectorAcc := supply.NewEmptyModuleAccount(auth.FeeCollectorName, supply.Basic) - notBondedPool := supply.NewEmptyModuleAccount(staking.NotBondedPoolName, supply.Burner) - bondPool := supply.NewEmptyModuleAccount(staking.BondedPoolName, supply.Burner) + notBondedPool := supply.NewEmptyModuleAccount(staking.NotBondedPoolName, supply.Burner, supply.Staking) + bondPool := supply.NewEmptyModuleAccount(staking.BondedPoolName, supply.Burner, supply.Staking) distrAcc := supply.NewEmptyModuleAccount(types.ModuleName, supply.Basic) keeper.supplyKeeper.SetModuleAccount(ctx, feeCollectorAcc) diff --git a/x/gov/test_common.go b/x/gov/test_common.go index d524c6969660..b0a501f389fc 100644 --- a/x/gov/test_common.go +++ b/x/gov/test_common.go @@ -61,8 +61,8 @@ func getMockApp(t *testing.T, numGenAccs int, genState GenesisState, genAccs []a maccPerms := map[string][]string{ types.ModuleName: []string{supply.Burner}, - staking.NotBondedPoolName: []string{supply.Burner}, - staking.BondedPoolName: []string{supply.Burner}, + staking.NotBondedPoolName: []string{supply.Burner, supply.Staking}, + staking.BondedPoolName: []string{supply.Burner, supply.Staking}, } supplyKeeper := supply.NewKeeper(mApp.Cdc, keySupply, mApp.AccountKeeper, bk, supply.DefaultCodespace, maccPerms) sk := staking.NewKeeper(mApp.Cdc, keyStaking, tKeyStaking, supplyKeeper, pk.Subspace(staking.DefaultParamspace), staking.DefaultCodespace) @@ -112,8 +112,8 @@ func getInitChainer(mapp *mock.App, keeper Keeper, stakingKeeper staking.Keeper, // set module accounts govAcc := supply.NewEmptyModuleAccount(types.ModuleName, supply.Burner) - notBondedPool := supply.NewEmptyModuleAccount(staking.NotBondedPoolName, supply.Burner) - bondPool := supply.NewEmptyModuleAccount(staking.BondedPoolName, supply.Burner) + notBondedPool := supply.NewEmptyModuleAccount(staking.NotBondedPoolName, supply.Burner, supply.Staking) + bondPool := supply.NewEmptyModuleAccount(staking.BondedPoolName, supply.Burner, supply.Staking) supplyKeeper.SetModuleAccount(ctx, govAcc) supplyKeeper.SetModuleAccount(ctx, notBondedPool) diff --git a/x/slashing/app_test.go b/x/slashing/app_test.go index f720107bfc44..82ba0659e048 100644 --- a/x/slashing/app_test.go +++ b/x/slashing/app_test.go @@ -38,8 +38,8 @@ func getMockApp(t *testing.T) (*mock.App, staking.Keeper, Keeper) { bankKeeper := bank.NewBaseKeeper(mapp.AccountKeeper, mapp.ParamsKeeper.Subspace(bank.DefaultParamspace), bank.DefaultCodespace) maccPerms := map[string][]string{ auth.FeeCollectorName: []string{supply.Basic}, - staking.NotBondedPoolName: []string{supply.Burner}, - staking.BondedPoolName: []string{supply.Burner}, + staking.NotBondedPoolName: []string{supply.Burner, supply.Staking}, + staking.BondedPoolName: []string{supply.Burner, supply.Staking}, } supplyKeeper := supply.NewKeeper(mapp.Cdc, keySupply, mapp.AccountKeeper, bankKeeper, supply.DefaultCodespace, maccPerms) stakingKeeper := staking.NewKeeper(mapp.Cdc, keyStaking, tkeyStaking, supplyKeeper, mapp.ParamsKeeper.Subspace(staking.DefaultParamspace), staking.DefaultCodespace) @@ -70,8 +70,8 @@ func getInitChainer(mapp *mock.App, keeper staking.Keeper, accountKeeper types.A return func(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain { // set module accounts feeCollector := supply.NewEmptyModuleAccount(auth.FeeCollectorName, supply.Basic) - notBondedPool := supply.NewEmptyModuleAccount(types.NotBondedPoolName, supply.Burner) - bondPool := supply.NewEmptyModuleAccount(types.BondedPoolName, supply.Burner) + notBondedPool := supply.NewEmptyModuleAccount(types.NotBondedPoolName, supply.Burner, supply.Staking) + bondPool := supply.NewEmptyModuleAccount(types.BondedPoolName, supply.Burner, supply.Staking) supplyKeeper.SetModuleAccount(ctx, feeCollector) supplyKeeper.SetModuleAccount(ctx, bondPool) diff --git a/x/slashing/test_common.go b/x/slashing/test_common.go index 80e00302e660..1a27a0d1f73d 100644 --- a/x/slashing/test_common.go +++ b/x/slashing/test_common.go @@ -79,8 +79,8 @@ func createTestInput(t *testing.T, defaults Params) (sdk.Context, bank.Keeper, s bk := bank.NewBaseKeeper(accountKeeper, paramsKeeper.Subspace(bank.DefaultParamspace), bank.DefaultCodespace) maccPerms := map[string][]string{ auth.FeeCollectorName: []string{supply.Basic}, - staking.NotBondedPoolName: []string{supply.Burner}, - staking.BondedPoolName: []string{supply.Burner}, + staking.NotBondedPoolName: []string{supply.Burner, supply.Staking}, + staking.BondedPoolName: []string{supply.Burner, supply.Staking}, } supplyKeeper := supply.NewKeeper(cdc, keySupply, accountKeeper, bk, supply.DefaultCodespace, maccPerms) @@ -92,8 +92,8 @@ func createTestInput(t *testing.T, defaults Params) (sdk.Context, bank.Keeper, s // set module accounts feeCollectorAcc := supply.NewEmptyModuleAccount(auth.FeeCollectorName, supply.Basic) - notBondedPool := supply.NewEmptyModuleAccount(staking.NotBondedPoolName, supply.Burner) - bondPool := supply.NewEmptyModuleAccount(staking.BondedPoolName, supply.Burner) + notBondedPool := supply.NewEmptyModuleAccount(staking.NotBondedPoolName, supply.Burner, supply.Staking) + bondPool := supply.NewEmptyModuleAccount(staking.BondedPoolName, supply.Burner, supply.Staking) supplyKeeper.SetModuleAccount(ctx, feeCollectorAcc) supplyKeeper.SetModuleAccount(ctx, bondPool) diff --git a/x/staking/app_test.go b/x/staking/app_test.go index 2f538bd3bf27..cb4921e3143a 100644 --- a/x/staking/app_test.go +++ b/x/staking/app_test.go @@ -28,8 +28,8 @@ func getMockApp(t *testing.T) (*mock.App, Keeper) { bankKeeper := bank.NewBaseKeeper(mApp.AccountKeeper, mApp.ParamsKeeper.Subspace(bank.DefaultParamspace), bank.DefaultCodespace) maccPerms := map[string][]string{ auth.FeeCollectorName: []string{supply.Basic}, - types.NotBondedPoolName: []string{supply.Burner}, - types.BondedPoolName: []string{supply.Burner}, + types.NotBondedPoolName: []string{supply.Burner, supply.Staking}, + types.BondedPoolName: []string{supply.Burner, supply.Staking}, } supplyKeeper := supply.NewKeeper(mApp.Cdc, keySupply, mApp.AccountKeeper, bankKeeper, supply.DefaultCodespace, maccPerms) keeper := NewKeeper(mApp.Cdc, keyStaking, tkeyStaking, supplyKeeper, mApp.ParamsKeeper.Subspace(DefaultParamspace), DefaultCodespace) @@ -61,8 +61,8 @@ func getInitChainer(mapp *mock.App, keeper Keeper, accountKeeper types.AccountKe // set module accounts feeCollector := supply.NewEmptyModuleAccount(auth.FeeCollectorName, supply.Basic) - notBondedPool := supply.NewEmptyModuleAccount(types.NotBondedPoolName, supply.Burner) - bondPool := supply.NewEmptyModuleAccount(types.BondedPoolName, supply.Burner) + notBondedPool := supply.NewEmptyModuleAccount(types.NotBondedPoolName, supply.Burner, supply.Staking) + bondPool := supply.NewEmptyModuleAccount(types.BondedPoolName, supply.Burner, supply.Staking) supplyKeeper.SetModuleAccount(ctx, feeCollector) supplyKeeper.SetModuleAccount(ctx, bondPool) diff --git a/x/staking/keeper/test_common.go b/x/staking/keeper/test_common.go index aecf52f8547c..331081509594 100644 --- a/x/staking/keeper/test_common.go +++ b/x/staking/keeper/test_common.go @@ -126,8 +126,8 @@ func CreateTestInput(t *testing.T, isCheckTx bool, initPower int64) (sdk.Context maccPerms := map[string][]string{ auth.FeeCollectorName: []string{supply.Basic}, - types.NotBondedPoolName: []string{supply.Burner}, - types.BondedPoolName: []string{supply.Burner}, + types.NotBondedPoolName: []string{supply.Burner, supply.Staking}, + types.BondedPoolName: []string{supply.Burner, supply.Staking}, } supplyKeeper := supply.NewKeeper(cdc, keySupply, accountKeeper, bk, supply.DefaultCodespace, maccPerms) @@ -142,8 +142,8 @@ func CreateTestInput(t *testing.T, isCheckTx bool, initPower int64) (sdk.Context // set module accounts feeCollectorAcc := supply.NewEmptyModuleAccount(auth.FeeCollectorName, supply.Basic) - notBondedPool := supply.NewEmptyModuleAccount(types.NotBondedPoolName, supply.Burner) - bondPool := supply.NewEmptyModuleAccount(types.BondedPoolName, supply.Burner) + notBondedPool := supply.NewEmptyModuleAccount(types.NotBondedPoolName, supply.Burner, supply.Staking) + bondPool := supply.NewEmptyModuleAccount(types.BondedPoolName, supply.Burner, supply.Staking) err = notBondedPool.SetCoins(totalSupply) require.NoError(t, err) diff --git a/x/supply/alias.go b/x/supply/alias.go index ca0b0627d76c..77ee6f8b2956 100644 --- a/x/supply/alias.go +++ b/x/supply/alias.go @@ -18,6 +18,7 @@ const ( Basic = types.Basic Minter = types.Minter Burner = types.Burner + Staking = types.Staking ) var ( diff --git a/x/supply/internal/keeper/bank.go b/x/supply/internal/keeper/bank.go index 665bbcf3836b..03065cddd0c1 100644 --- a/x/supply/internal/keeper/bank.go +++ b/x/supply/internal/keeper/bank.go @@ -55,11 +55,16 @@ func (k Keeper) DelegateCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk recipientModule string, amt sdk.Coins) sdk.Error { // create the account if it doesn't yet exist - recipientAddr := k.GetModuleAccount(ctx, recipientModule).GetAddress() + acc, _ := k.GetModuleAccountAndPermissions(ctx, recipientModule) + recipientAddr := acc.GetAddress() if recipientAddr == nil { panic(fmt.Sprintf("module account %s isn't able to be created", recipientModule)) } + if !acc.HasPermission(types.Staking) { + panic(fmt.Sprintf("Account %s does not have permissions to delegate coins", recipientModule)) + } + return k.bk.DelegateCoins(ctx, senderAddr, recipientAddr, amt) } @@ -68,11 +73,16 @@ func (k Keeper) DelegateCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk func (k Keeper) UndelegateCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) sdk.Error { - senderAddr := k.GetModuleAddress(senderModule) + acc, _ := k.GetModuleAccountAndPermissions(ctx, senderModule) + senderAddr := acc.GetAddress() if senderAddr == nil { return sdk.ErrUnknownAddress(fmt.Sprintf("module account %s does not exist", senderModule)) } + if !acc.HasPermission(types.Staking) { + panic(fmt.Sprintf("Account %s does not have permissions to undelegate coins", senderModule)) + } + return k.bk.UndelegateCoins(ctx, senderAddr, recipientAddr, amt) } diff --git a/x/supply/internal/types/permissions.go b/x/supply/internal/types/permissions.go index 1432cc28c2cb..ddf17214fa61 100644 --- a/x/supply/internal/types/permissions.go +++ b/x/supply/internal/types/permissions.go @@ -8,9 +8,10 @@ import ( // permissions const ( - Basic = "basic" - Minter = "minter" - Burner = "burner" + Basic = "basic" + Minter = "minter" + Burner = "burner" + Staking = "staking" ) // PermAddr defines the permissions for an address @@ -58,7 +59,7 @@ func (pa PermAddr) GetPermissions() []string { func validatePermissions(permissions []string) error { for _, perm := range permissions { switch perm { - case Basic, Minter, Burner: + case Basic, Minter, Burner, Staking: continue default: return fmt.Errorf("invalid module permission %s", perm) From 198313d735112a13395374564f14dce006cb28be Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Fri, 5 Jul 2019 14:32:22 -0400 Subject: [PATCH 05/13] Update x/supply/internal/keeper/bank.go --- x/supply/internal/keeper/bank.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/supply/internal/keeper/bank.go b/x/supply/internal/keeper/bank.go index 03065cddd0c1..f8d045f7cf98 100644 --- a/x/supply/internal/keeper/bank.go +++ b/x/supply/internal/keeper/bank.go @@ -62,7 +62,7 @@ func (k Keeper) DelegateCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk } if !acc.HasPermission(types.Staking) { - panic(fmt.Sprintf("Account %s does not have permissions to delegate coins", recipientModule)) + panic(fmt.Sprintf("module account %s does not have permissions to delegate coins", recipientModule)) } return k.bk.DelegateCoins(ctx, senderAddr, recipientAddr, amt) From 5738e883a1997ae92bea94f39d05a925ec1efaa5 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Fri, 5 Jul 2019 14:32:36 -0400 Subject: [PATCH 06/13] Update x/supply/internal/keeper/bank.go --- x/supply/internal/keeper/bank.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/supply/internal/keeper/bank.go b/x/supply/internal/keeper/bank.go index f8d045f7cf98..87cfdf217d4a 100644 --- a/x/supply/internal/keeper/bank.go +++ b/x/supply/internal/keeper/bank.go @@ -80,7 +80,7 @@ func (k Keeper) UndelegateCoinsFromModuleToAccount(ctx sdk.Context, senderModule } if !acc.HasPermission(types.Staking) { - panic(fmt.Sprintf("Account %s does not have permissions to undelegate coins", senderModule)) + panic(fmt.Sprintf("module account %s does not have permissions to undelegate coins", senderModule)) } return k.bk.UndelegateCoins(ctx, senderAddr, recipientAddr, amt) From 87eff5cdcdc8232448652ff89736edf852477b5c Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Fri, 5 Jul 2019 14:32:55 -0400 Subject: [PATCH 07/13] Update x/supply/internal/keeper/bank.go --- x/supply/internal/keeper/bank.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/supply/internal/keeper/bank.go b/x/supply/internal/keeper/bank.go index 87cfdf217d4a..9cab8d57128a 100644 --- a/x/supply/internal/keeper/bank.go +++ b/x/supply/internal/keeper/bank.go @@ -133,7 +133,7 @@ func (k Keeper) BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) sdk } if !acc.HasPermission(types.Burner) { - panic(fmt.Sprintf("Account %s does not have permissions to burn tokens", moduleName)) + panic(fmt.Sprintf("module account %s does not have permissions to burn tokens", moduleName)) } _, err := k.bk.SubtractCoins(ctx, addr, amt) From 1316e2fa02b4fcc88a4a616083326ad219802ec5 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Fri, 5 Jul 2019 14:33:08 -0400 Subject: [PATCH 08/13] Update x/supply/internal/keeper/bank.go --- x/supply/internal/keeper/bank.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/supply/internal/keeper/bank.go b/x/supply/internal/keeper/bank.go index 9cab8d57128a..9a97ce9609b1 100644 --- a/x/supply/internal/keeper/bank.go +++ b/x/supply/internal/keeper/bank.go @@ -101,7 +101,7 @@ func (k Keeper) MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) sdk } if !acc.HasPermission(types.Minter) { - panic(fmt.Sprintf("Account %s does not have permissions to mint tokens", moduleName)) + panic(fmt.Sprintf("module account %s does not have permissions to mint tokens", moduleName)) } _, err := k.bk.AddCoins(ctx, addr, amt) From e8827dd8d2bb95bef071d0c839b1ad42f2f23686 Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Fri, 5 Jul 2019 12:02:36 -0700 Subject: [PATCH 09/13] address pr review comments --- .pending/breaking/sdk/4652-Extend-supply-p | 1 - docs/spec/supply/01_concepts.md | 1 + simapp/app.go | 7 ++-- x/auth/test_common.go | 14 +++---- x/supply/internal/keeper/bank.go | 8 ++-- x/supply/internal/keeper/bank_test.go | 14 +++++-- x/supply/internal/keeper/keeper.go | 13 ++++++ x/supply/internal/keeper/keeper_test.go | 17 ++++++++ x/supply/internal/keeper/test_common.go | 6 ++- x/supply/internal/types/account.go | 22 +++++++++- x/supply/internal/types/account_test.go | 20 +++++++++ x/supply/internal/types/permissions.go | 29 +++++-------- x/supply/internal/types/permissions_test.go | 45 +++++++++++---------- 13 files changed, 134 insertions(+), 63 deletions(-) delete mode 100644 .pending/breaking/sdk/4652-Extend-supply-p diff --git a/.pending/breaking/sdk/4652-Extend-supply-p b/.pending/breaking/sdk/4652-Extend-supply-p deleted file mode 100644 index 2426ca2bb8d0..000000000000 --- a/.pending/breaking/sdk/4652-Extend-supply-p +++ /dev/null @@ -1 +0,0 @@ -#4652 Extend supply permissions to allow for multiple permissions per ModuleAccount. diff --git a/docs/spec/supply/01_concepts.md b/docs/spec/supply/01_concepts.md index 2375f18b13b5..3d71e6e4b569 100644 --- a/docs/spec/supply/01_concepts.md +++ b/docs/spec/supply/01_concepts.md @@ -59,3 +59,4 @@ The available permissions are: - `Basic`: is allowed to only transfer its coins to other accounts. - `Minter`: allows for a module to mint a specific amount of coins as well as perform the `Basic` permissioned actions. - `Burner`: allows for a module to burn a specific amount of coins as well as perform the `Basic` permissioned actions. +- `Staking`: allows for a module to delegate and undelegate a specific amount of coins. diff --git a/simapp/app.go b/simapp/app.go index 9ebf9de23c52..0855d9e58e13 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -148,16 +148,15 @@ func NewSimApp(logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest bo auth.FeeCollectorName: []string{supply.Basic}, distr.ModuleName: []string{supply.Basic}, mint.ModuleName: []string{supply.Minter}, - staking.BondedPoolName: []string{supply.Burner}, - staking.NotBondedPoolName: []string{supply.Burner}, + staking.BondedPoolName: []string{supply.Burner, supply.Staking}, + staking.NotBondedPoolName: []string{supply.Burner, supply.Staking}, gov.ModuleName: []string{supply.Burner}, } // add keepers app.accountKeeper = auth.NewAccountKeeper(app.cdc, app.keyAccount, authSubspace, auth.ProtoBaseAccount) app.bankKeeper = bank.NewBaseKeeper(app.accountKeeper, bankSubspace, bank.DefaultCodespace) - app.supplyKeeper = supply.NewKeeper(app.cdc, app.keySupply, app.accountKeeper, - app.bankKeeper, supply.DefaultCodespace, maccPerms) + app.supplyKeeper = supply.NewKeeper(app.cdc, app.keySupply, app.accountKeeper, app.bankKeeper, supply.DefaultCodespace, maccPerms) stakingKeeper := staking.NewKeeper(app.cdc, app.keyStaking, app.tkeyStaking, app.supplyKeeper, stakingSubspace, staking.DefaultCodespace) app.mintKeeper = mint.NewKeeper(app.cdc, app.keyMint, mintSubspace, &stakingKeeper, app.supplyKeeper, auth.FeeCollectorName) diff --git a/x/auth/test_common.go b/x/auth/test_common.go index 6c674c8da842..c308b180ef67 100644 --- a/x/auth/test_common.go +++ b/x/auth/test_common.go @@ -25,13 +25,13 @@ type testInput struct { // moduleAccount defines an account for modules that holds coins on a pool type moduleAccount struct { *types.BaseAccount - Name string `json:"name"` // name of the module - Permissions []string `json:"permissions"` // permissions of module account (minter/burner/holder) + name string `json:"name"` // name of the module + permissions []string `json:"permissions"` // permissions of module account (minter/burner/holder) } // HasPermission returns whether or not the module account has permission. func (ma moduleAccount) HasPermission(permission string) bool { - for _, perm := range ma.Permissions { + for _, perm := range ma.permissions { if perm == permission { return true } @@ -41,12 +41,12 @@ func (ma moduleAccount) HasPermission(permission string) bool { // GetName returns the the name of the holder's module func (ma moduleAccount) GetName() string { - return ma.Name + return ma.name } // GetPermissions returns permissions granted to the module account (holder/minter/burner) func (ma moduleAccount) GetPermissions() []string { - return ma.Permissions + return ma.permissions } func setupTestInput() testInput { @@ -135,8 +135,8 @@ func (sk DummySupplyKeeper) GetModuleAccount(ctx sdk.Context, moduleName string) // create a new module account macc := &moduleAccount{ BaseAccount: &baseAcc, - Name: moduleName, - Permissions: []string{"basic"}, + name: moduleName, + permissions: []string{"basic"}, } maccI := (sk.ak.NewAccount(ctx, macc)).(exported.ModuleAccountI) diff --git a/x/supply/internal/keeper/bank.go b/x/supply/internal/keeper/bank.go index 03065cddd0c1..9a97ce9609b1 100644 --- a/x/supply/internal/keeper/bank.go +++ b/x/supply/internal/keeper/bank.go @@ -62,7 +62,7 @@ func (k Keeper) DelegateCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk } if !acc.HasPermission(types.Staking) { - panic(fmt.Sprintf("Account %s does not have permissions to delegate coins", recipientModule)) + panic(fmt.Sprintf("module account %s does not have permissions to delegate coins", recipientModule)) } return k.bk.DelegateCoins(ctx, senderAddr, recipientAddr, amt) @@ -80,7 +80,7 @@ func (k Keeper) UndelegateCoinsFromModuleToAccount(ctx sdk.Context, senderModule } if !acc.HasPermission(types.Staking) { - panic(fmt.Sprintf("Account %s does not have permissions to undelegate coins", senderModule)) + panic(fmt.Sprintf("module account %s does not have permissions to undelegate coins", senderModule)) } return k.bk.UndelegateCoins(ctx, senderAddr, recipientAddr, amt) @@ -101,7 +101,7 @@ func (k Keeper) MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) sdk } if !acc.HasPermission(types.Minter) { - panic(fmt.Sprintf("Account %s does not have permissions to mint tokens", moduleName)) + panic(fmt.Sprintf("module account %s does not have permissions to mint tokens", moduleName)) } _, err := k.bk.AddCoins(ctx, addr, amt) @@ -133,7 +133,7 @@ func (k Keeper) BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) sdk } if !acc.HasPermission(types.Burner) { - panic(fmt.Sprintf("Account %s does not have permissions to burn tokens", moduleName)) + panic(fmt.Sprintf("module account %s does not have permissions to burn tokens", moduleName)) } _, err := k.bk.SubtractCoins(ctx, addr, amt) diff --git a/x/supply/internal/keeper/bank_test.go b/x/supply/internal/keeper/bank_test.go index 0f16faa56beb..f293108ed664 100644 --- a/x/supply/internal/keeper/bank_test.go +++ b/x/supply/internal/keeper/bank_test.go @@ -12,10 +12,11 @@ import ( const initialPower = int64(100) var ( - holderAcc = types.NewEmptyModuleAccount(types.Basic, types.Basic) - burnerAcc = types.NewEmptyModuleAccount(types.Burner, types.Burner) - minterAcc = types.NewEmptyModuleAccount(types.Minter, types.Minter) - multiPermAcc = types.NewEmptyModuleAccount(multiPerm, types.Basic, types.Burner, types.Minter) + holderAcc = types.NewEmptyModuleAccount(types.Basic, types.Basic) + burnerAcc = types.NewEmptyModuleAccount(types.Burner, types.Burner) + minterAcc = types.NewEmptyModuleAccount(types.Minter, types.Minter) + multiPermAcc = types.NewEmptyModuleAccount(multiPerm, types.Basic, types.Burner, types.Minter) + randomPermAcc = types.NewEmptyModuleAccount(randomPerm, "random") initTokens = sdk.TokensFromConsensusPower(initialPower) initCoins = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, initTokens)) @@ -80,11 +81,14 @@ func TestMintCoins(t *testing.T) { keeper.SetModuleAccount(ctx, burnerAcc) keeper.SetModuleAccount(ctx, minterAcc) keeper.SetModuleAccount(ctx, multiPermAcc) + keeper.SetModuleAccount(ctx, randomPermAcc) initialSupply := keeper.GetSupply(ctx) require.Panics(t, func() { keeper.MintCoins(ctx, "", initCoins) }) + require.Panics(t, func() { keeper.MintCoins(ctx, randomPerm, initCoins) }) + err := keeper.MintCoins(ctx, types.Minter, initCoins) require.NoError(t, err) require.Equal(t, initCoins, getCoinsByName(ctx, keeper, types.Minter)) @@ -115,6 +119,8 @@ func TestBurnCoins(t *testing.T) { require.Panics(t, func() { keeper.BurnCoins(ctx, "", initCoins) }) + require.Panics(t, func() { keeper.BurnCoins(ctx, randomPerm, initialSupply.Total) }) + err = keeper.BurnCoins(ctx, types.Burner, initialSupply.Total) require.Error(t, err) diff --git a/x/supply/internal/keeper/keeper.go b/x/supply/internal/keeper/keeper.go index 82ff7af6553e..67c149b67dbd 100644 --- a/x/supply/internal/keeper/keeper.go +++ b/x/supply/internal/keeper/keeper.go @@ -7,6 +7,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/supply/exported" "github.com/cosmos/cosmos-sdk/x/supply/internal/types" ) @@ -60,3 +61,15 @@ func (k Keeper) SetSupply(ctx sdk.Context, supply types.Supply) { b := k.cdc.MustMarshalBinaryLengthPrefixed(supply) store.Set(SupplyKey, b) } + +// ValidatePermissions validates that the module account has been granted +// permissions within its set of allowed permissions. +func (k Keeper) ValidatePermissions(macc exported.ModuleAccountI) error { + permAddr := k.permAddrs[macc.GetName()] + for _, perm := range macc.GetPermissions() { + if !permAddr.HasPermission(perm) { + return fmt.Errorf("invalid module permission %s", perm) + } + } + return nil +} diff --git a/x/supply/internal/keeper/keeper_test.go b/x/supply/internal/keeper/keeper_test.go index 4e458eeb48e9..fe2ce3dce2f6 100644 --- a/x/supply/internal/keeper/keeper_test.go +++ b/x/supply/internal/keeper/keeper_test.go @@ -20,3 +20,20 @@ func TestSupply(t *testing.T) { require.Equal(t, expectedTotal, total) } + +func TestValidatePermissions(t *testing.T) { + nAccs := int64(0) + initialPower := int64(100) + _, _, keeper := createTestInput(t, false, initialPower, nAccs) + + err := keeper.ValidatePermissions(multiPermAcc) + require.NoError(t, err) + + err = keeper.ValidatePermissions(randomPermAcc) + require.NoError(t, err) + + // add unregistered permissions + randomPermAcc.AddPermissions("other") + err = keeper.ValidatePermissions(randomPermAcc) + require.Error(t, err) +} diff --git a/x/supply/internal/keeper/test_common.go b/x/supply/internal/keeper/test_common.go index 2fc8ee9bded9..285492510cd1 100644 --- a/x/supply/internal/keeper/test_common.go +++ b/x/supply/internal/keeper/test_common.go @@ -22,7 +22,10 @@ import ( ) // nolint: deadcode unused -var multiPerm = "multiple permissions account" +var ( + multiPerm = "multiple permissions account" + randomPerm = "random permission" +) // nolint: deadcode unused // create a codec used only for testing @@ -79,6 +82,7 @@ func createTestInput(t *testing.T, isCheckTx bool, initPower int64, nAccs int64) types.Minter: []string{types.Minter}, types.Burner: []string{types.Burner}, multiPerm: []string{types.Basic, types.Minter, types.Burner}, + randomPerm: []string{"random"}, } keeper := NewKeeper(cdc, keySupply, ak, bk, DefaultCodespace, maccPerms) totalSupply := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, valTokens.MulRaw(nAccs))) diff --git a/x/supply/internal/types/account.go b/x/supply/internal/types/account.go index dd58389a4c40..98dc4d16a214 100644 --- a/x/supply/internal/types/account.go +++ b/x/supply/internal/types/account.go @@ -30,7 +30,7 @@ func NewEmptyModuleAccount(name string, permissions ...string) *ModuleAccount { moduleAddress := NewModuleAddress(name) baseAcc := authtypes.NewBaseAccountWithAddress(moduleAddress) - if err := validatePermissions(permissions); err != nil { + if err := validatePermissions(permissions...); err != nil { panic(err) } @@ -45,7 +45,7 @@ func NewEmptyModuleAccount(name string, permissions ...string) *ModuleAccount { func NewModuleAccount(ba *authtypes.BaseAccount, name string, permissions ...string) *ModuleAccount { - if err := validatePermissions(permissions); err != nil { + if err := validatePermissions(permissions...); err != nil { panic(err) } @@ -56,6 +56,24 @@ func NewModuleAccount(ba *authtypes.BaseAccount, } } +// AddPermissions adds the permissions to the module account's list of granted +// permissions. +func (ma *ModuleAccount) AddPermissions(permissions ...string) { + ma.Permissions = append(ma.Permissions, permissions...) +} + +// RemovePermission removes the permission from the list of granted permissions +// or returns an error if the permission is has not been granted. +func (ma *ModuleAccount) RemovePermission(permission string) error { + for i, perm := range ma.Permissions { + if perm == permission { + ma.Permissions = append(ma.Permissions[:i], ma.Permissions[i+1:]...) + return nil + } + } + return fmt.Errorf("cannot remove non granted permission %s", permission) +} + // HasPermission returns whether or not the module account has permission. func (ma ModuleAccount) HasPermission(permission string) bool { for _, perm := range ma.Permissions { diff --git a/x/supply/internal/types/account_test.go b/x/supply/internal/types/account_test.go index a3928c515ce5..214c847a7620 100644 --- a/x/supply/internal/types/account_test.go +++ b/x/supply/internal/types/account_test.go @@ -35,6 +35,26 @@ func TestModuleAccountMarshalYAML(t *testing.T) { require.Equal(t, want, moduleAcc.String()) } +func TestRemovePermissions(t *testing.T) { + name := "test" + macc := NewEmptyModuleAccount(name) + require.Empty(t, macc.GetPermissions()) + + macc.AddPermissions(Basic, Minter, Burner) + require.Equal(t, []string{Basic, Minter, Burner}, macc.GetPermissions(), "did not add permissions") + + err := macc.RemovePermission("random") + require.Error(t, err, "did not error on removing nonexistent permission") + + err = macc.RemovePermission(Burner) + require.NoError(t, err, "failed to remove permission") + require.Equal(t, []string{Basic, Minter}, macc.GetPermissions(), "does not have correct permissions") + + err = macc.RemovePermission(Basic) + require.NoError(t, err, "failed to remove permission") + require.Equal(t, []string{Minter}, macc.GetPermissions(), "does not have correct permissions") +} + func TestHasPermissions(t *testing.T) { name := "test" macc := NewEmptyModuleAccount(name, Basic, Minter, Burner) diff --git a/x/supply/internal/types/permissions.go b/x/supply/internal/types/permissions.go index ddf17214fa61..00882ba6a053 100644 --- a/x/supply/internal/types/permissions.go +++ b/x/supply/internal/types/permissions.go @@ -2,6 +2,7 @@ package types import ( "fmt" + "strings" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -28,21 +29,14 @@ func NewPermAddr(name string, permissions []string) PermAddr { } } -// AddPermissions adds the permission to the list of granted permissions. -func (pa *PermAddr) AddPermissions(permissions ...string) { - pa.permissions = append(pa.permissions, permissions...) -} - -// RemovePermission removes the permission from the list of granted permissions -// or returns an error if the permission is has not been granted. -func (pa *PermAddr) RemovePermission(permission string) error { - for i, perm := range pa.permissions { +// HasPermission returns whether the PermAddr contains permission. +func (pa PermAddr) HasPermission(permission string) bool { + for _, perm := range pa.permissions { if perm == permission { - pa.permissions = append(pa.permissions[:i], pa.permissions[i+1:]...) - return nil + return true } } - return fmt.Errorf("cannot remove non granted permission %s", permission) + return false } // GetAddress returns the address of the PermAddr object @@ -55,14 +49,11 @@ func (pa PermAddr) GetPermissions() []string { return pa.permissions } -// validate the input permissions -func validatePermissions(permissions []string) error { +// performs basic permission validation +func validatePermissions(permissions ...string) error { for _, perm := range permissions { - switch perm { - case Basic, Minter, Burner, Staking: - continue - default: - return fmt.Errorf("invalid module permission %s", perm) + if strings.TrimSpace(perm) == "" { + return fmt.Errorf("module permission is empty") } } return nil diff --git a/x/supply/internal/types/permissions_test.go b/x/supply/internal/types/permissions_test.go index 038f93a7f2b8..7449cf1a7084 100644 --- a/x/supply/internal/types/permissions_test.go +++ b/x/supply/internal/types/permissions_test.go @@ -6,24 +6,28 @@ import ( "github.com/stretchr/testify/require" ) -func TestRemovePermissions(t *testing.T) { - name := "test" - permAddr := NewPermAddr(name, []string{}) - require.Empty(t, permAddr.GetPermissions()) +func TestHasPermission(t *testing.T) { + emptyPermAddr := NewPermAddr("empty", []string{}) + has := emptyPermAddr.HasPermission(Basic) + require.False(t, has) - permAddr.AddPermissions(Basic, Minter, Burner) - require.Equal(t, []string{Basic, Minter, Burner}, permAddr.GetPermissions(), "did not add permissions") - - err := permAddr.RemovePermission("random") - require.Error(t, err, "did not error on removing nonexistent permission") - - err = permAddr.RemovePermission(Burner) - require.NoError(t, err, "failed to remove permission") - require.Equal(t, []string{Basic, Minter}, permAddr.GetPermissions(), "does not have correct permissions") + cases := []struct { + permission string + expectHas bool + }{ + {Basic, true}, + {Minter, true}, + {Burner, true}, + {Staking, true}, + {"random", false}, + {"", false}, + } + permAddr := NewPermAddr("test", []string{Basic, Minter, Burner, Staking}) + for i, tc := range cases { + has = permAddr.HasPermission(tc.permission) + require.Equal(t, tc.expectHas, has, "test case #%d", i) + } - err = permAddr.RemovePermission(Basic) - require.NoError(t, err, "failed to remove permission") - require.Equal(t, []string{Minter}, permAddr.GetPermissions(), "does not have correct permissions") } func TestValidatePermissions(t *testing.T) { @@ -33,15 +37,14 @@ func TestValidatePermissions(t *testing.T) { expectPass bool }{ {"no permissions", []string{}, true}, - {"one permission", []string{Basic}, true}, - {"multiple permissions", []string{Basic, Minter, Burner}, true}, - {"invalid permission", []string{"other"}, false}, - {"multiple invalid permissions", []string{Burner, "other", "invalid"}, false}, + {"valid permission", []string{Basic}, true}, + {"invalid permission", []string{""}, false}, + {"invalid and valid permission", []string{Basic, ""}, false}, } for i, tc := range cases { t.Run(tc.name, func(t *testing.T) { - err := validatePermissions(tc.permissions) + err := validatePermissions(tc.permissions...) if tc.expectPass { require.NoError(t, err, "test case #%d", i) } else { From 0772d29d47c1b380e94f89b85fd4c1b027e99d8e Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Sat, 6 Jul 2019 10:35:05 -0400 Subject: [PATCH 10/13] Update x/supply/internal/keeper/bank.go --- x/supply/internal/keeper/bank.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/supply/internal/keeper/bank.go b/x/supply/internal/keeper/bank.go index 4aac5c09ff10..65d204ad56f2 100644 --- a/x/supply/internal/keeper/bank.go +++ b/x/supply/internal/keeper/bank.go @@ -61,7 +61,7 @@ func (k Keeper) DelegateCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk } if !recipientAcc.HasPermission(types.Staking) { - panic(fmt.Sprintf("module account %s does not have permissions to delegate coins", recipientModule)) + panic(fmt.Sprintf("module account %s does not have permissions to receive delegated coins", recipientModule)) } return k.bk.DelegateCoins(ctx, senderAddr, recipientAcc.GetAddress(), amt) From e22f70c958b15377f976c93c4366b74d65d71dde Mon Sep 17 00:00:00 2001 From: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Date: Sat, 6 Jul 2019 17:49:55 +0200 Subject: [PATCH 11/13] Apply suggestions from code review --- x/auth/test_common.go | 2 +- x/mint/internal/keeper/test_common.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x/auth/test_common.go b/x/auth/test_common.go index c308b180ef67..70bd8e8ae00f 100644 --- a/x/auth/test_common.go +++ b/x/auth/test_common.go @@ -44,7 +44,7 @@ func (ma moduleAccount) GetName() string { return ma.name } -// GetPermissions returns permissions granted to the module account (holder/minter/burner) +// GetPermissions returns permissions granted to the module account func (ma moduleAccount) GetPermissions() []string { return ma.permissions } diff --git a/x/mint/internal/keeper/test_common.go b/x/mint/internal/keeper/test_common.go index c4f33a229a35..c26908ce2e06 100644 --- a/x/mint/internal/keeper/test_common.go +++ b/x/mint/internal/keeper/test_common.go @@ -60,8 +60,8 @@ func newTestInput(t *testing.T) testInput { maccPerms := map[string][]string{ auth.FeeCollectorName: []string{supply.Basic}, types.ModuleName: []string{supply.Minter}, - staking.NotBondedPoolName: []string{supply.Burner}, - staking.BondedPoolName: []string{supply.Burner}, + staking.NotBondedPoolName: []string{supply.Burner, supply.Staking}, + staking.BondedPoolName: []string{supply.Burner, supply.Staking}, } supplyKeeper := supply.NewKeeper(types.ModuleCdc, keySupply, accountKeeper, bankKeeper, supply.DefaultCodespace, maccPerms) supplyKeeper.SetSupply(ctx, supply.NewSupply(sdk.Coins{})) From d36686b64192a6e4f6c10f9933b500a00a22ef6a Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Sat, 6 Jul 2019 09:51:14 -0700 Subject: [PATCH 12/13] apply @fedekunze review suggestions --- x/supply/internal/keeper/keeper.go | 6 +++--- x/supply/internal/types/permissions.go | 20 ++++++++++---------- x/supply/internal/types/permissions_test.go | 4 ++-- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/x/supply/internal/keeper/keeper.go b/x/supply/internal/keeper/keeper.go index 67c149b67dbd..c2208df2ee8e 100644 --- a/x/supply/internal/keeper/keeper.go +++ b/x/supply/internal/keeper/keeper.go @@ -17,7 +17,7 @@ type Keeper struct { storeKey sdk.StoreKey ak types.AccountKeeper bk types.BankKeeper - permAddrs map[string]types.PermAddr + permAddrs map[string]types.PermissionsForAddress } // NewKeeper creates a new Keeper instance @@ -25,9 +25,9 @@ func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, ak types.AccountKeeper, bk ty codespace sdk.CodespaceType, maccPerms map[string][]string) Keeper { // set the addresses - permAddrs := make(map[string]types.PermAddr) + permAddrs := make(map[string]types.PermissionsForAddress) for name, perms := range maccPerms { - permAddrs[name] = types.NewPermAddr(name, perms) + permAddrs[name] = types.NewPermissionsForAddress(name, perms) } return Keeper{ diff --git a/x/supply/internal/types/permissions.go b/x/supply/internal/types/permissions.go index 00882ba6a053..29f9b5e8f22a 100644 --- a/x/supply/internal/types/permissions.go +++ b/x/supply/internal/types/permissions.go @@ -15,22 +15,22 @@ const ( Staking = "staking" ) -// PermAddr defines the permissions for an address -type PermAddr struct { +// PermissionsForAddress defines all the registered permissions for an address +type PermissionsForAddress struct { permissions []string address sdk.AccAddress } -// NewPermAddr creates a new PermAddr object -func NewPermAddr(name string, permissions []string) PermAddr { - return PermAddr{ +// NewPermissionsForAddress creates a new PermissionsForAddresess object +func NewPermissionsForAddress(name string, permissions []string) PermissionsForAddress { + return PermissionsForAddress{ permissions: permissions, address: NewModuleAddress(name), } } -// HasPermission returns whether the PermAddr contains permission. -func (pa PermAddr) HasPermission(permission string) bool { +// HasPermission returns whether the PermissionsForAddress contains permission. +func (pa PermissionsForAddress) HasPermission(permission string) bool { for _, perm := range pa.permissions { if perm == permission { return true @@ -39,13 +39,13 @@ func (pa PermAddr) HasPermission(permission string) bool { return false } -// GetAddress returns the address of the PermAddr object -func (pa PermAddr) GetAddress() sdk.AccAddress { +// GetAddress returns the address of the PermissionsForAddress object +func (pa PermissionsForAddress) GetAddress() sdk.AccAddress { return pa.address } // GetPermissions returns the permissions granted to the address -func (pa PermAddr) GetPermissions() []string { +func (pa PermissionsForAddress) GetPermissions() []string { return pa.permissions } diff --git a/x/supply/internal/types/permissions_test.go b/x/supply/internal/types/permissions_test.go index 7449cf1a7084..2612d3e0edb0 100644 --- a/x/supply/internal/types/permissions_test.go +++ b/x/supply/internal/types/permissions_test.go @@ -7,7 +7,7 @@ import ( ) func TestHasPermission(t *testing.T) { - emptyPermAddr := NewPermAddr("empty", []string{}) + emptyPermAddr := NewPermissionsForAddress("empty", []string{}) has := emptyPermAddr.HasPermission(Basic) require.False(t, has) @@ -22,7 +22,7 @@ func TestHasPermission(t *testing.T) { {"random", false}, {"", false}, } - permAddr := NewPermAddr("test", []string{Basic, Minter, Burner, Staking}) + permAddr := NewPermissionsForAddress("test", []string{Basic, Minter, Burner, Staking}) for i, tc := range cases { has = permAddr.HasPermission(tc.permission) require.Equal(t, tc.expectHas, has, "test case #%d", i) From e513d2479defe69d9f99b36ed24123d64034b206 Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Sat, 6 Jul 2019 10:01:58 -0700 Subject: [PATCH 13/13] fix spelling --- x/supply/internal/types/permissions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/supply/internal/types/permissions.go b/x/supply/internal/types/permissions.go index 29f9b5e8f22a..ffb8cf492bba 100644 --- a/x/supply/internal/types/permissions.go +++ b/x/supply/internal/types/permissions.go @@ -21,7 +21,7 @@ type PermissionsForAddress struct { address sdk.AccAddress } -// NewPermissionsForAddress creates a new PermissionsForAddresess object +// NewPermissionsForAddress creates a new PermissionsForAddress object func NewPermissionsForAddress(name string, permissions []string) PermissionsForAddress { return PermissionsForAddress{ permissions: permissions,