Skip to content

Commit

Permalink
Remove Basic Permission (cosmos#4722)
Browse files Browse the repository at this point in the history
Module account has "basic" permissions.
Since it is never checked against, we just delete it.

Closes: cosmos#4702
  • Loading branch information
colin-axner authored and Alessio Treglia committed Jul 16, 2019
1 parent d3bb9f5 commit 8c7ed19
Show file tree
Hide file tree
Showing 14 changed files with 39 additions and 42 deletions.
5 changes: 2 additions & 3 deletions docs/spec/supply/01_concepts.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ permissions to that specific account and perform or not the action.

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.
- `Minter`: allows for a module to mint a specific amount of coins.
- `Burner`: allows for a module to burn a specific amount of coins.
- `Staking`: allows for a module to delegate and undelegate a specific amount of coins.
4 changes: 2 additions & 2 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ func NewSimApp(logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest bo

// account permissions
maccPerms := map[string][]string{
auth.FeeCollectorName: []string{supply.Basic},
distr.ModuleName: []string{supply.Basic},
auth.FeeCollectorName: nil,
distr.ModuleName: nil,
mint.ModuleName: []string{supply.Minter},
staking.BondedPoolName: []string{supply.Burner, supply.Staking},
staking.NotBondedPoolName: []string{supply.Burner, supply.Staking},
Expand Down
8 changes: 4 additions & 4 deletions x/distribution/keeper/test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ func CreateTestInputAdvanced(t *testing.T, isCheckTx bool, initPower int64,
accountKeeper := auth.NewAccountKeeper(cdc, keyAcc, pk.Subspace(auth.DefaultParamspace), auth.ProtoBaseAccount)
bankKeeper := bank.NewBaseKeeper(accountKeeper, pk.Subspace(bank.DefaultParamspace), bank.DefaultCodespace)
maccPerms := map[string][]string{
auth.FeeCollectorName: []string{supply.Basic},
types.ModuleName: []string{supply.Basic},
auth.FeeCollectorName: nil,
types.ModuleName: nil,
staking.NotBondedPoolName: []string{supply.Burner, supply.Staking},
staking.BondedPoolName: []string{supply.Burner, supply.Staking},
}
Expand All @@ -144,10 +144,10 @@ func CreateTestInputAdvanced(t *testing.T, isCheckTx bool, initPower int64,
}

// create module accounts
feeCollectorAcc := supply.NewEmptyModuleAccount(auth.FeeCollectorName, supply.Basic)
feeCollectorAcc := supply.NewEmptyModuleAccount(auth.FeeCollectorName)
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)
distrAcc := supply.NewEmptyModuleAccount(types.ModuleName)

keeper.supplyKeeper.SetModuleAccount(ctx, feeCollectorAcc)
keeper.supplyKeeper.SetModuleAccount(ctx, notBondedPool)
Expand Down
4 changes: 2 additions & 2 deletions x/mint/internal/keeper/test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func newTestInput(t *testing.T) testInput {
accountKeeper := auth.NewAccountKeeper(types.ModuleCdc, keyAcc, paramsKeeper.Subspace(auth.DefaultParamspace), auth.ProtoBaseAccount)
bankKeeper := bank.NewBaseKeeper(accountKeeper, paramsKeeper.Subspace(bank.DefaultParamspace), bank.DefaultCodespace)
maccPerms := map[string][]string{
auth.FeeCollectorName: []string{supply.Basic},
auth.FeeCollectorName: nil,
types.ModuleName: []string{supply.Minter},
staking.NotBondedPoolName: []string{supply.Burner, supply.Staking},
staking.BondedPoolName: []string{supply.Burner, supply.Staking},
Expand All @@ -72,7 +72,7 @@ func newTestInput(t *testing.T) testInput {
mintKeeper := NewKeeper(types.ModuleCdc, keyMint, paramsKeeper.Subspace(types.DefaultParamspace), &stakingKeeper, supplyKeeper, auth.FeeCollectorName)

// set module accounts
feeCollectorAcc := supply.NewEmptyModuleAccount(auth.FeeCollectorName, supply.Basic)
feeCollectorAcc := supply.NewEmptyModuleAccount(auth.FeeCollectorName)
minterAcc := supply.NewEmptyModuleAccount(types.ModuleName, supply.Minter)
notBondedPool := supply.NewEmptyModuleAccount(staking.NotBondedPoolName, supply.Burner)
bondPool := supply.NewEmptyModuleAccount(staking.BondedPoolName, supply.Burner)
Expand Down
4 changes: 2 additions & 2 deletions x/slashing/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ 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},
auth.FeeCollectorName: nil,
staking.NotBondedPoolName: []string{supply.Burner, supply.Staking},
staking.BondedPoolName: []string{supply.Burner, supply.Staking},
}
Expand Down Expand Up @@ -69,7 +69,7 @@ func getEndBlocker(keeper staking.Keeper) sdk.EndBlocker {
func getInitChainer(mapp *mock.App, keeper staking.Keeper, accountKeeper types.AccountKeeper, supplyKeeper types.SupplyKeeper) sdk.InitChainer {
return func(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
// set module accounts
feeCollector := supply.NewEmptyModuleAccount(auth.FeeCollectorName, supply.Basic)
feeCollector := supply.NewEmptyModuleAccount(auth.FeeCollectorName)
notBondedPool := supply.NewEmptyModuleAccount(types.NotBondedPoolName, supply.Burner, supply.Staking)
bondPool := supply.NewEmptyModuleAccount(types.BondedPoolName, supply.Burner, supply.Staking)

Expand Down
4 changes: 2 additions & 2 deletions x/slashing/test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ 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},
auth.FeeCollectorName: nil,
staking.NotBondedPoolName: []string{supply.Burner, supply.Staking},
staking.BondedPoolName: []string{supply.Burner, supply.Staking},
}
Expand All @@ -91,7 +91,7 @@ func createTestInput(t *testing.T, defaults Params) (sdk.Context, bank.Keeper, s
genesis := staking.DefaultGenesisState()

// set module accounts
feeCollectorAcc := supply.NewEmptyModuleAccount(auth.FeeCollectorName, supply.Basic)
feeCollectorAcc := supply.NewEmptyModuleAccount(auth.FeeCollectorName)
notBondedPool := supply.NewEmptyModuleAccount(staking.NotBondedPoolName, supply.Burner, supply.Staking)
bondPool := supply.NewEmptyModuleAccount(staking.BondedPoolName, supply.Burner, supply.Staking)

Expand Down
4 changes: 2 additions & 2 deletions x/staking/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ 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},
auth.FeeCollectorName: nil,
types.NotBondedPoolName: []string{supply.Burner, supply.Staking},
types.BondedPoolName: []string{supply.Burner, supply.Staking},
}
Expand Down Expand Up @@ -60,7 +60,7 @@ func getInitChainer(mapp *mock.App, keeper Keeper, accountKeeper types.AccountKe
mapp.InitChainer(ctx, req)

// set module accounts
feeCollector := supply.NewEmptyModuleAccount(auth.FeeCollectorName, supply.Basic)
feeCollector := supply.NewEmptyModuleAccount(auth.FeeCollectorName)
notBondedPool := supply.NewEmptyModuleAccount(types.NotBondedPoolName, supply.Burner, supply.Staking)
bondPool := supply.NewEmptyModuleAccount(types.BondedPoolName, supply.Burner, supply.Staking)

Expand Down
4 changes: 2 additions & 2 deletions x/staking/keeper/test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func CreateTestInput(t *testing.T, isCheckTx bool, initPower int64) (sdk.Context
)

maccPerms := map[string][]string{
auth.FeeCollectorName: []string{supply.Basic},
auth.FeeCollectorName: nil,
types.NotBondedPoolName: []string{supply.Burner, supply.Staking},
types.BondedPoolName: []string{supply.Burner, supply.Staking},
}
Expand All @@ -141,7 +141,7 @@ func CreateTestInput(t *testing.T, isCheckTx bool, initPower int64) (sdk.Context
keeper.SetParams(ctx, types.DefaultParams())

// set module accounts
feeCollectorAcc := supply.NewEmptyModuleAccount(auth.FeeCollectorName, supply.Basic)
feeCollectorAcc := supply.NewEmptyModuleAccount(auth.FeeCollectorName)
notBondedPool := supply.NewEmptyModuleAccount(types.NotBondedPoolName, supply.Burner, supply.Staking)
bondPool := supply.NewEmptyModuleAccount(types.BondedPoolName, supply.Burner, supply.Staking)

Expand Down
1 change: 0 additions & 1 deletion x/supply/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ const (
StoreKey = types.StoreKey
RouterKey = types.RouterKey
QuerierRoute = types.QuerierRoute
Basic = types.Basic
Minter = types.Minter
Burner = types.Burner
Staking = types.Staking
Expand Down
12 changes: 6 additions & 6 deletions x/supply/internal/keeper/bank_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ import (
const initialPower = int64(100)

var (
holderAcc = types.NewEmptyModuleAccount(types.Basic, types.Basic)
holderAcc = types.NewEmptyModuleAccount(holder)
burnerAcc = types.NewEmptyModuleAccount(types.Burner, types.Burner)
minterAcc = types.NewEmptyModuleAccount(types.Minter, types.Minter)
multiPermAcc = types.NewEmptyModuleAccount(multiPerm, types.Basic, types.Burner, types.Minter)
multiPermAcc = types.NewEmptyModuleAccount(multiPerm, types.Burner, types.Minter, types.Staking)
randomPermAcc = types.NewEmptyModuleAccount(randomPerm, "random")

initTokens = sdk.TokensFromConsensusPower(initialPower)
Expand Down Expand Up @@ -44,7 +44,7 @@ func TestSendCoins(t *testing.T) {
keeper.SetModuleAccount(ctx, burnerAcc)
ak.SetAccount(ctx, baseAcc)

err = keeper.SendCoinsFromModuleToModule(ctx, "", types.Basic, initCoins)
err = keeper.SendCoinsFromModuleToModule(ctx, "", holderAcc.GetName(), initCoins)
require.Error(t, err)

require.Panics(t, func() {
Expand All @@ -54,12 +54,12 @@ func TestSendCoins(t *testing.T) {
err = keeper.SendCoinsFromModuleToAccount(ctx, "", baseAcc.GetAddress(), initCoins)
require.Error(t, err)

err = keeper.SendCoinsFromModuleToAccount(ctx, types.Basic, baseAcc.GetAddress(), initCoins.Add(initCoins))
err = keeper.SendCoinsFromModuleToAccount(ctx, holderAcc.GetName(), baseAcc.GetAddress(), initCoins.Add(initCoins))
require.Error(t, err)

err = keeper.SendCoinsFromModuleToModule(ctx, types.Basic, types.Burner, initCoins)
err = keeper.SendCoinsFromModuleToModule(ctx, holderAcc.GetName(), types.Burner, initCoins)
require.NoError(t, err)
require.Equal(t, sdk.Coins(nil), getCoinsByName(ctx, keeper, types.Basic))
require.Equal(t, sdk.Coins(nil), getCoinsByName(ctx, keeper, holderAcc.GetName()))
require.Equal(t, initCoins, getCoinsByName(ctx, keeper, types.Burner))

err = keeper.SendCoinsFromModuleToAccount(ctx, types.Burner, baseAcc.GetAddress(), initCoins)
Expand Down
5 changes: 3 additions & 2 deletions x/supply/internal/keeper/test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
var (
multiPerm = "multiple permissions account"
randomPerm = "random permission"
holder = "holder"
)

// nolint: deadcode unused
Expand Down Expand Up @@ -78,10 +79,10 @@ func createTestInput(t *testing.T, isCheckTx bool, initPower int64, nAccs int64)
createTestAccs(ctx, int(nAccs), initialCoins, &ak)

maccPerms := map[string][]string{
types.Basic: []string{types.Basic},
holder: nil,
types.Minter: []string{types.Minter},
types.Burner: []string{types.Burner},
multiPerm: []string{types.Basic, types.Minter, types.Burner},
multiPerm: []string{types.Minter, types.Burner, types.Staking},
randomPerm: []string{"random"},
}
keeper := NewKeeper(cdc, keySupply, ak, bk, DefaultCodespace, maccPerms)
Expand Down
16 changes: 8 additions & 8 deletions x/supply/internal/types/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (

func TestModuleAccountMarshalYAML(t *testing.T) {
name := "test"
moduleAcc := NewEmptyModuleAccount(name, Basic, Minter, Burner)
moduleAcc := NewEmptyModuleAccount(name, Minter, Burner, Staking)
moduleAddress := sdk.AccAddress(crypto.AddressHash([]byte(name)))
bs, err := yaml.Marshal(moduleAcc)
require.NoError(t, err)
Expand All @@ -29,7 +29,7 @@ func TestModuleAccountMarshalYAML(t *testing.T) {
- %s
- %s
- %s
`, moduleAddress, name, Basic, Minter, Burner)
`, moduleAddress, name, Minter, Burner, Staking)

require.Equal(t, want, string(bs))
require.Equal(t, want, moduleAcc.String())
Expand All @@ -40,29 +40,29 @@ func TestRemovePermissions(t *testing.T) {
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")
macc.AddPermissions(Minter, Burner, Staking)
require.Equal(t, []string{Minter, Burner, Staking}, 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")
require.Equal(t, []string{Minter, Staking}, macc.GetPermissions(), "does not have correct permissions")

err = macc.RemovePermission(Basic)
err = macc.RemovePermission(Staking)
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)
macc := NewEmptyModuleAccount(name, Staking, Minter, Burner)
cases := []struct {
permission string
expectHas bool
}{
{Basic, true},
{Staking, true},
{Minter, true},
{Burner, true},
{"other", false},
Expand Down
1 change: 0 additions & 1 deletion x/supply/internal/types/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (

// permissions
const (
Basic = "basic"
Minter = "minter"
Burner = "burner"
Staking = "staking"
Expand Down
9 changes: 4 additions & 5 deletions x/supply/internal/types/permissions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,20 @@ import (

func TestHasPermission(t *testing.T) {
emptyPermAddr := NewPermissionsForAddress("empty", []string{})
has := emptyPermAddr.HasPermission(Basic)
has := emptyPermAddr.HasPermission(Minter)
require.False(t, has)

cases := []struct {
permission string
expectHas bool
}{
{Basic, true},
{Minter, true},
{Burner, true},
{Staking, true},
{"random", false},
{"", false},
}
permAddr := NewPermissionsForAddress("test", []string{Basic, Minter, Burner, Staking})
permAddr := NewPermissionsForAddress("test", []string{Minter, Burner, Staking})
for i, tc := range cases {
has = permAddr.HasPermission(tc.permission)
require.Equal(t, tc.expectHas, has, "test case #%d", i)
Expand All @@ -37,9 +36,9 @@ func TestValidatePermissions(t *testing.T) {
expectPass bool
}{
{"no permissions", []string{}, true},
{"valid permission", []string{Basic}, true},
{"valid permission", []string{Minter}, true},
{"invalid permission", []string{""}, false},
{"invalid and valid permission", []string{Basic, ""}, false},
{"invalid and valid permission", []string{Staking, ""}, false},
}

for i, tc := range cases {
Expand Down

0 comments on commit 8c7ed19

Please sign in to comment.