diff --git a/CHANGELOG.md b/CHANGELOG.md index a75f6041..9b125a02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Fix missing ICA controller configuration [#257](https://github.com/atomone-hub/atomone/pull/257) - Fix wrapper converters for `x/gov` [#276](https://github.com/atomone-hub/atomone/pull/276) - Add min-stake filtering for cosmos-sdk votes in the gov ante handler [#279](https://github.com/atomone-hub/atomone/pull/279) +- Oversight DAO and Steering DAO addresses cannot be the same [#309](https://github.com/atomone-hub/atomone/pull/309) ### DEPENDENCIES diff --git a/x/coredaos/keeper/msg_server_test.go b/x/coredaos/keeper/msg_server_test.go index 86a10916..973ca3e6 100644 --- a/x/coredaos/keeper/msg_server_test.go +++ b/x/coredaos/keeper/msg_server_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "strings" "testing" "time" @@ -18,10 +19,11 @@ import ( func TestMsgServerUpdateParams(t *testing.T) { timeDuration := time.Duration(1) - testAcc := simtestutil.CreateRandomAccounts(3) + testAcc := simtestutil.CreateRandomAccounts(4) bondedAcc := testAcc[0].String() unbondingAcc := testAcc[1].String() unbondedAcc := testAcc[2].String() + unbondedAcc2 := testAcc[3].String() tests := []struct { name string @@ -188,16 +190,44 @@ func TestMsgServerUpdateParams(t *testing.T) { Authority: "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn", Params: types.Params{ SteeringDaoAddress: unbondedAcc, - OversightDaoAddress: unbondedAcc, + OversightDaoAddress: unbondedAcc2, VotingPeriodExtensionDuration: &timeDuration, }, }, setupMocks: func(ctx sdk.Context, m *testutil.Mocks) { // Address is not bonded or in unbonding - m.StakingKeeper.EXPECT().GetDelegatorBonded(ctx, sdk.MustAccAddressFromBech32(unbondedAcc)).Return(math.NewInt(0), nil).Times(2) - m.StakingKeeper.EXPECT().GetDelegatorUnbonding(ctx, sdk.MustAccAddressFromBech32(unbondedAcc)).Return(math.NewInt(0), nil).Times(2) + m.StakingKeeper.EXPECT().GetDelegatorBonded(ctx, sdk.MustAccAddressFromBech32(unbondedAcc)).Return(math.NewInt(0), nil) + m.StakingKeeper.EXPECT().GetDelegatorUnbonding(ctx, sdk.MustAccAddressFromBech32(unbondedAcc)).Return(math.NewInt(0), nil) + m.StakingKeeper.EXPECT().GetDelegatorBonded(ctx, sdk.MustAccAddressFromBech32(unbondedAcc2)).Return(math.NewInt(0), nil) + m.StakingKeeper.EXPECT().GetDelegatorUnbonding(ctx, sdk.MustAccAddressFromBech32(unbondedAcc2)).Return(math.NewInt(0), nil) }, }, + { + name: "steeringdao and oversight same address", + msg: &types.MsgUpdateParams{ + Authority: "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn", + Params: types.Params{ + SteeringDaoAddress: unbondedAcc, + OversightDaoAddress: unbondedAcc, + VotingPeriodExtensionDuration: &timeDuration, + }, + }, + expectedErr: "steering DAO address and oversight DAO address cannot be the same: " + unbondedAcc, + setupMocks: func(ctx sdk.Context, m *testutil.Mocks) {}, + }, + { + name: "steeringdao and oversight same address but different case", + msg: &types.MsgUpdateParams{ + Authority: "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn", + Params: types.Params{ + SteeringDaoAddress: unbondedAcc, + OversightDaoAddress: strings.ToUpper(unbondedAcc), + VotingPeriodExtensionDuration: &timeDuration, + }, + }, + expectedErr: "steering DAO address and oversight DAO address cannot be the same: " + unbondedAcc, + setupMocks: func(ctx sdk.Context, m *testutil.Mocks) {}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/x/coredaos/types/params.go b/x/coredaos/types/params.go index dea3ebcf..67185846 100644 --- a/x/coredaos/types/params.go +++ b/x/coredaos/types/params.go @@ -42,20 +42,31 @@ func DefaultParams() Params { // Validate validates the set of params func (p Params) ValidateBasic() error { + var ( + steeringDaoAddr sdk.AccAddress + oversightDaoAddr sdk.AccAddress + err error + ) + // Steering DAO address can only be empty (disabled) or a valid Bech32 address if p.SteeringDaoAddress != "" { - if _, err := sdk.AccAddressFromBech32(p.SteeringDaoAddress); err != nil { + if steeringDaoAddr, err = sdk.AccAddressFromBech32(p.SteeringDaoAddress); err != nil { return fmt.Errorf("invalid steering DAO address: %s: %w", p.SteeringDaoAddress, err) } } // Oversight DAO address can only be empty (disabled) or a valid Bech32 address if p.OversightDaoAddress != "" { - if _, err := sdk.AccAddressFromBech32(p.OversightDaoAddress); err != nil { + if oversightDaoAddr, err = sdk.AccAddressFromBech32(p.OversightDaoAddress); err != nil { return fmt.Errorf("invalid oversight DAO address: %s: %w", p.OversightDaoAddress, err) } } + // Oversight DAO cannot be the same as the Steering DAO + if p.SteeringDaoAddress != "" && p.OversightDaoAddress != "" && steeringDaoAddr.Equals(oversightDaoAddr) { + return fmt.Errorf("steering DAO address and oversight DAO address cannot be the same: %s", p.SteeringDaoAddress) + } + // VotingPeriodExtensionDuration must be a positive duration if p.VotingPeriodExtensionDuration == nil { return fmt.Errorf("voting period extension duration must not be nil")