Skip to content

Commit

Permalink
refactor: only use authority as admin (no params)
Browse files Browse the repository at this point in the history
  • Loading branch information
Reecepbcups committed Jul 16, 2024
1 parent 3ddd5eb commit 955113d
Show file tree
Hide file tree
Showing 15 changed files with 131 additions and 435 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ poad tx poa set-power [validator] [amount] [--unsafe]
# - any admin can modify the list at any time
# - there must be at least one admin in the list at all times
# - allow_validator_self_exit is a bool to allow validators to force remove themselves from the set.
poad tx poa update-params [admin1,admin2,admin3,...] [allow_validator_self_exit]
poad tx poa update-params [allow_validator_self_exit]

# (admin) Update the staking module params
# - unbondingTime is the time that a validator must wait to unbond (ex: 336h)
Expand Down
233 changes: 49 additions & 184 deletions api/v1/params.pulsar.go

Large diffs are not rendered by default.

16 changes: 4 additions & 12 deletions client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,29 +161,21 @@ func NewRemovePendingCmd() *cobra.Command {

func NewUpdateParamsCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "update-params [admin1,admin2,admin3,...] [allow-validator-self-exit-bool]",
Use: "update-params [allow-validator-self-exit-bool]",
Short: "update the PoA module params",
Args: cobra.ExactArgs(2),
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientTxContext(cmd)
if err != nil {
return err
}

admins := strings.Split(args[0], ",")
for _, admin := range admins {
_, err = sdk.AccAddressFromBech32(admin)
if err != nil {
return fmt.Errorf("AccAddressFromBech32 failed: %w", err)
}
}

allowGracefulExit, err := strconv.ParseBool(args[1])
allowGracefulExit, err := strconv.ParseBool(args[0])
if err != nil {
return fmt.Errorf("strconv.ParseBool failed: %w", err)
}

p, err := poa.NewParams(admins, allowGracefulExit)
p, err := poa.NewParams(allowGracefulExit)
if err != nil {
return fmt.Errorf("NewParams failed: %w", err)
}
Expand Down
11 changes: 2 additions & 9 deletions e2e/helpers/poa.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,20 +106,13 @@ func GetPOAPending(t *testing.T, ctx context.Context, chain *cosmos.CosmosChain)
return res
}

func POAUpdateParams(t *testing.T, ctx context.Context, chain *cosmos.CosmosChain, user ibc.Wallet, admins []string, gracefulExit bool) (TxResponse, error) {
// admin1,admin2,admin3
adminList := ""
for _, admin := range admins {
adminList += admin + ","
}
adminList = adminList[:len(adminList)-1]

func POAUpdateParams(t *testing.T, ctx context.Context, chain *cosmos.CosmosChain, user ibc.Wallet, gracefulExit bool) (TxResponse, error) {
gracefulParam := "true"
if !gracefulExit {
gracefulParam = "false"
}

cmd := TxCommandBuilder(ctx, chain, []string{"tx", "poa", "update-params", adminList, gracefulParam}, user.KeyName())
cmd := TxCommandBuilder(ctx, chain, []string{"tx", "poa", "update-params", gracefulParam}, user.KeyName())
return ExecuteTransaction(ctx, chain, cmd)
}

Expand Down
13 changes: 5 additions & 8 deletions e2e/poa_gov_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,15 @@ func testGovernance(t *testing.T, ctx context.Context, chain *cosmos.CosmosChain
&poa.MsgUpdateParams{
Sender: GovModuleAddress,
Params: poa.Params{
Admins: []string{acc0.FormattedAddress(), GovModuleAddress, RandAcc},
AllowValidatorSelfExit: false,
},
},
}

propId := helpers.SubmitParamChangeProp(t, ctx, chain, acc0, updatedParams, GovModuleAddress, 25)
helpers.ValidatorVote(t, ctx, chain, propId, cosmos.ProposalVoteYes, 30)

require.Len(t, helpers.GetPOAParams(t, ctx, chain).Admins, 3, "Admins should be 3")
require.True(t, helpers.GetPOAParams(t, ctx, chain).AllowValidatorSelfExit, "AllowValidatorSelfExit should be true")
})

t.Run("success: gov proposal validator change", func(t *testing.T) {
Expand All @@ -91,7 +91,7 @@ func testUpdatePOAParams(t *testing.T, ctx context.Context, chain *cosmos.Cosmos
t.Log("\n===== TEST UPDATE POA PARAMS =====")

t.Run("fail: update-params message from a non authorized user", func(t *testing.T) {
tx, err = helpers.POAUpdateParams(t, ctx, chain, incorrectUser, []string{incorrectUser.FormattedAddress()}, true)
tx, err = helpers.POAUpdateParams(t, ctx, chain, incorrectUser, true)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -140,8 +140,7 @@ func testUpdatePOAParams(t *testing.T, ctx context.Context, chain *cosmos.Cosmos
})

t.Run("success: update-params message from an authorized user with cli.", func(t *testing.T) {
newAdmins := []string{acc0.FormattedAddress(), GovModuleAddress, RandAcc, incorrectUser.FormattedAddress()}
tx, err = helpers.POAUpdateParams(t, ctx, chain, acc0, newAdmins, true)
tx, err = helpers.POAUpdateParams(t, ctx, chain, acc0, true)
if err != nil {
t.Fatal(err)
}
Expand All @@ -151,9 +150,7 @@ func testUpdatePOAParams(t *testing.T, ctx context.Context, chain *cosmos.Cosmos
require.EqualValues(t, txRes.Code, 0)

p := helpers.GetPOAParams(t, ctx, chain)
for _, admin := range newAdmins {
require.Contains(t, p.Admins, admin)
}
require.False(t, p.AllowValidatorSelfExit) // TODO: check this
})

}
12 changes: 6 additions & 6 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import (
)

var (
ErrStakingActionNotAllowed = sdkerrors.Register(ModuleName, 1, "staking actions are now allowed on this chain")
ErrPowerBelowMinimum = sdkerrors.Register(ModuleName, 2, "power must be above 1_000_000")
ErrNotAnAuthority = sdkerrors.Register(ModuleName, 3, "not an authority")
ErrUnsafePower = sdkerrors.Register(ModuleName, 4, "unsafe: msg.Power is >30%% of total power, set unsafe=true to override")
ErrMustProvideAtLeastOneAddress = sdkerrors.Register(ModuleName, 5, "must provide at least one address")
ErrValidatorSelfRemoval = sdkerrors.Register(ModuleName, 6, "validator is not allowed to remove themselves")
ErrStakingActionNotAllowed = sdkerrors.Register(ModuleName, 1, "staking actions are now allowed on this chain")
ErrPowerBelowMinimum = sdkerrors.Register(ModuleName, 2, "power must be above 1_000_000")
ErrNotAnAuthority = sdkerrors.Register(ModuleName, 3, "not an authority")
ErrUnsafePower = sdkerrors.Register(ModuleName, 4, "unsafe: msg.Power is >30%% of total power, set unsafe=true to override")
// ErrMustProvideAtLeastOneAddress = sdkerrors.Register(ModuleName, 5, "must provide at least one address")
ErrValidatorSelfRemoval = sdkerrors.Register(ModuleName, 6, "validator is not allowed to remove themselves")
)
9 changes: 2 additions & 7 deletions keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestInitGenesis(t *testing.T) {
})

t.Run("custom params", func(t *testing.T) {
p, err := poa.NewParams([]string{fixture.addrs[0].String(), fixture.addrs[1].String()}, true)
p, err := poa.NewParams(true)
require.NoError(err)

data := &poa.GenesisState{
Expand All @@ -44,13 +44,8 @@ func TestInitGenesis(t *testing.T) {
require.Equal(p, params)
})

t.Run("bad params", func(t *testing.T) {
_, err := poa.NewParams(nil, false)
require.Error(err)
})

t.Run("pending validator export", func(t *testing.T) {
p, err := poa.NewParams([]string{fixture.addrs[0].String(), fixture.addrs[1].String()}, true)
p, err := poa.NewParams(true)
require.NoError(err)

acc := GenAcc()
Expand Down
31 changes: 3 additions & 28 deletions keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,38 +101,13 @@ func (k Keeper) GetBankKeeper() BankKeeper {
return k.bankKeeper
}

// GetAdmins returns the module's administrators with delegation of power control.
func (k Keeper) GetAdmins(ctx context.Context) []string {
p, err := k.GetParams(ctx)
if err != nil {
return []string{}
}

admins := p.Admins

found := false
for _, auth := range admins {
if auth == k.authority {
found = true
break
}
}
if !found {
admins = append(admins, k.authority)
}

return admins
// GetAdmin returns the module's administrators with delegation of power control (authority)
func (k Keeper) GetAdmin(ctx context.Context) string {
return k.authority
}

// IsAdmin checks if the given address is an admin.
func (k Keeper) IsAdmin(ctx context.Context, fromAddr string) bool {
for _, auth := range k.GetAdmins(ctx) {
if auth == fromAddr {
return true
}
}

// the main authority may already be in the GetAdmins list. If not, we check here.
return k.authority == fromAddr
}

Expand Down
36 changes: 19 additions & 17 deletions keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,16 @@ import (
poamodule "github.com/strangelove-ventures/poa/module"
)

var maccPerms = map[string][]string{
authtypes.FeeCollectorName: nil,
stakingtypes.BondedPoolName: {authtypes.Burner, authtypes.Staking},
stakingtypes.NotBondedPoolName: {authtypes.Burner, authtypes.Staking},
minttypes.ModuleName: {authtypes.Minter},
govtypes.ModuleName: {authtypes.Burner},
}
var (
authorityAddr = "cosmos15ky9du8a2wlstz6fpx3p4mqpjyrm5cgqjwl8sq"
maccPerms = map[string][]string{
authtypes.FeeCollectorName: nil,
stakingtypes.BondedPoolName: {authtypes.Burner, authtypes.Staking},
stakingtypes.NotBondedPoolName: {authtypes.Burner, authtypes.Staking},
minttypes.ModuleName: {authtypes.Minter},
govtypes.ModuleName: {authtypes.Burner},
}
)

type testFixture struct {
suite.Suite
Expand All @@ -64,8 +67,8 @@ type testFixture struct {
bankkeeper bankkeeper.BaseKeeper
mintkeeper mintkeeper.Keeper

addrs []sdk.AccAddress
govModAddr string
addrs []sdk.AccAddress
authorityAddr string
}

func SetupTest(t *testing.T, baseValShares int64) *testFixture {
Expand All @@ -77,7 +80,7 @@ func SetupTest(t *testing.T, baseValShares int64) *testFixture {
logger := log.NewTestLogger(t)
encCfg := moduletestutil.MakeTestEncodingConfig()

f.govModAddr = authtypes.NewModuleAddress(govtypes.ModuleName).String()
f.authorityAddr = authorityAddr
f.addrs = simtestutil.CreateIncrementalAccounts(3)

key := storetypes.NewKVStoreKey(poa.ModuleName)
Expand All @@ -90,7 +93,7 @@ func SetupTest(t *testing.T, baseValShares int64) *testFixture {
registerBaseSDKModules(f, encCfg, storeService, logger, require)

// Setup POA Keeper.
f.k = keeper.NewKeeper(encCfg.Codec, storeService, f.stakingKeeper, f.slashingKeeper, f.bankkeeper, logger, "")
f.k = keeper.NewKeeper(encCfg.Codec, storeService, f.stakingKeeper, f.slashingKeeper, f.bankkeeper, logger, authorityAddr)
f.msgServer = keeper.NewMsgServerImpl(f.k)
f.queryServer = keeper.NewQueryServerImpl(f.k)
f.appModule = poamodule.NewAppModule(encCfg.Codec, f.k)
Expand All @@ -117,7 +120,6 @@ func (f *testFixture) InitPoAGenesis(t *testing.T) {
t.Helper()

genState := poa.NewGenesisState()
genState.Params.Admins = []string{f.addrs[0].String(), f.govModAddr}
require.NoError(t, f.k.InitGenesis(f.ctx, genState))
}

Expand All @@ -134,21 +136,21 @@ func registerBaseSDKModules(
authtypes.ProtoBaseAccount,
maccPerms,
authcodec.NewBech32Codec(sdk.Bech32MainPrefix), sdk.Bech32MainPrefix,
f.govModAddr,
f.authorityAddr,
)

// Bank Keeper.
f.bankkeeper = bankkeeper.NewBaseKeeper(
encCfg.Codec, storeService,
f.accountkeeper,
nil,
f.govModAddr, logger,
f.authorityAddr, logger,
)

// Staking Keeper.
f.stakingKeeper = stakingkeeper.NewKeeper(
encCfg.Codec, storeService,
f.accountkeeper, f.bankkeeper, f.govModAddr,
f.accountkeeper, f.bankkeeper, f.authorityAddr,
authcodec.NewBech32Codec(sdk.Bech32PrefixValAddr),
authcodec.NewBech32Codec(sdk.Bech32PrefixConsAddr),
)
Expand All @@ -159,7 +161,7 @@ func registerBaseSDKModules(
f.slashingKeeper = slashingkeeper.NewKeeper(
encCfg.Codec, encCfg.Amino, storeService,
f.stakingKeeper,
f.govModAddr,
f.authorityAddr,
)
err = f.slashingKeeper.SetParams(f.ctx, slashingtypes.DefaultParams())
require.NoError(err)
Expand All @@ -169,7 +171,7 @@ func registerBaseSDKModules(
f.mintkeeper = mintkeeper.NewKeeper(
encCfg.Codec, storeService,
f.stakingKeeper, f.accountkeeper, f.bankkeeper,
authtypes.FeeCollectorName, f.govModAddr,
authtypes.FeeCollectorName, f.authorityAddr,
)
}

Expand Down
2 changes: 1 addition & 1 deletion keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func NewMsgServerImpl(keeper Keeper) poa.MsgServer {

func (ms msgServer) SetPower(ctx context.Context, msg *poa.MsgSetPower) (*poa.MsgSetPowerResponse, error) {
if isAdmin := ms.k.IsAdmin(ctx, msg.Sender); !isAdmin {
return nil, errorsmod.Wrapf(poa.ErrNotAnAuthority, "sender %s is not an authority. allowed: %+v", msg.Sender, ms.k.GetAdmins(ctx))
return nil, errorsmod.Wrapf(poa.ErrNotAnAuthority, "sender %s is not an authority. allowed: %+v", msg.Sender, ms.k.GetAdmin(ctx))
}

if err := msg.Validate(ms.k.GetValidatorAddressCodec()); err != nil {
Expand Down
14 changes: 3 additions & 11 deletions keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,12 @@ func TestUpdateParams(t *testing.T) {
},
expectErrMsg: "not an authority",
},
{
name: "set invalid admins",
request: &poa.MsgUpdateParams{
Sender: f.govModAddr,
Params: poa.Params{},
},
expectErrMsg: poa.ErrMustProvideAtLeastOneAddress.Error(),
},
{
name: "set valid params",
request: &poa.MsgUpdateParams{
Sender: f.govModAddr,
Sender: f.authorityAddr,
Params: poa.Params{
Admins: []string{f.addrs[0].String()},
AllowValidatorSelfExit: true,
},
},
},
Expand Down Expand Up @@ -96,7 +88,7 @@ func TestUpdateStakingParams(t *testing.T) {
{
name: "set valid params",
request: &poa.MsgUpdateStakingParams{
Sender: f.govModAddr,
Sender: f.authorityAddr,
Params: poa.DefaultStakingParams(),
},
expectErrMsg: "",
Expand Down
24 changes: 0 additions & 24 deletions keeper/query_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,30 +67,6 @@ func TestParamsQuery(t *testing.T) {
},
expected: poa.DefaultParams(),
},
{
name: "two admins",
request: &poa.MsgUpdateParams{
Sender: f.govModAddr,
Params: poa.Params{
Admins: []string{f.govModAddr, f.addrs[0].String()},
},
},
expected: poa.Params{
Admins: []string{f.govModAddr, f.addrs[0].String()},
},
},
{
name: "duplicate admins",
request: &poa.MsgUpdateParams{
Sender: f.govModAddr,
Params: poa.Params{
Admins: []string{f.govModAddr, f.govModAddr},
},
},
expected: poa.Params{
Admins: []string{f.govModAddr, f.govModAddr},
},
},
}

for _, tc := range testCases {
Expand Down
Loading

0 comments on commit 955113d

Please sign in to comment.