From 6e79bfc5daf6acaa38ff1ce70b86674610c3d967 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Wed, 17 Jul 2024 15:22:52 +0200 Subject: [PATCH 1/7] Change wiring for mint and gov to use ProviderKeeper instead of StakingKeeper --- app/provider/app.go | 52 ++++++------ testutil/keeper/mocks.go | 73 +++++++++++++++++ x/ccv/provider/keeper/keeper.go | 4 + .../keeper/staking_keeper_interface.go | 81 +++++++++++++++++++ x/ccv/types/expected_keepers.go | 11 +++ 5 files changed, 198 insertions(+), 23 deletions(-) create mode 100644 x/ccv/provider/keeper/staking_keeper_interface.go diff --git a/app/provider/app.go b/app/provider/app.go index 37b26b491c..c96ab8d2e9 100644 --- a/app/provider/app.go +++ b/app/provider/app.go @@ -373,15 +373,6 @@ func New( authcodec.NewBech32Codec(sdk.GetConfig().GetBech32ValidatorAddrPrefix()), authcodec.NewBech32Codec(sdk.GetConfig().GetBech32ConsensusAddrPrefix()), ) - app.MintKeeper = mintkeeper.NewKeeper( - appCodec, - runtime.NewKVStoreService(keys[minttypes.StoreKey]), - app.StakingKeeper, - app.AccountKeeper, - app.BankKeeper, - authtypes.FeeCollectorName, - authtypes.NewModuleAddress(govtypes.ModuleName).String(), - ) app.DistrKeeper = distrkeeper.NewKeeper( appCodec, runtime.NewKVStoreService(keys[distrtypes.StoreKey]), @@ -456,19 +447,6 @@ func New( runtime.ProvideCometInfoService(), ) - govConfig := govtypes.DefaultConfig() - app.GovKeeper = govkeeper.NewKeeper( - appCodec, - runtime.NewKVStoreService(keys[govtypes.StoreKey]), - app.AccountKeeper, - app.BankKeeper, - app.StakingKeeper, - app.DistrKeeper, - app.MsgServiceRouter(), - govConfig, - authtypes.NewModuleAddress(govtypes.ModuleName).String(), - ) - app.ProviderKeeper = ibcproviderkeeper.NewKeeper( appCodec, keys[providertypes.StoreKey], @@ -483,13 +461,41 @@ func New( app.AccountKeeper, app.DistrKeeper, app.BankKeeper, - *app.GovKeeper, + govkeeper.Keeper{}, // will be set after the GovKeeper is created authtypes.NewModuleAddress(govtypes.ModuleName).String(), authcodec.NewBech32Codec(sdk.GetConfig().GetBech32ValidatorAddrPrefix()), authcodec.NewBech32Codec(sdk.GetConfig().GetBech32ConsensusAddrPrefix()), authtypes.FeeCollectorName, ) + govConfig := govtypes.DefaultConfig() + app.GovKeeper = govkeeper.NewKeeper( + appCodec, + runtime.NewKVStoreService(keys[govtypes.StoreKey]), + app.AccountKeeper, + app.BankKeeper, + app.StakingKeeper, + app.DistrKeeper, + app.MsgServiceRouter(), + govConfig, + authtypes.NewModuleAddress(govtypes.ModuleName).String(), + ) + + // set the GovKeeper in the ProviderKeeper + app.ProviderKeeper.SetGovKeeper(*app.GovKeeper) + + app.MintKeeper = mintkeeper.NewKeeper( + appCodec, + runtime.NewKVStoreService(keys[minttypes.StoreKey]), + // use the ProviderKeeper as StakingKeeper for mint + // because minting should be based on the consensus-active validators + app.ProviderKeeper, + app.AccountKeeper, + app.BankKeeper, + authtypes.FeeCollectorName, + authtypes.NewModuleAddress(govtypes.ModuleName).String(), + ) + // gov router must be set after the provider keeper is created // otherwise the provider keeper will not be able to handle proposals (will be nil) govRouter := govv1beta1.NewRouter() diff --git a/testutil/keeper/mocks.go b/testutil/keeper/mocks.go index 7424e8bc63..600f0596d9 100644 --- a/testutil/keeper/mocks.go +++ b/testutil/keeper/mocks.go @@ -9,6 +9,7 @@ import ( reflect "reflect" time "time" + address "cosmossdk.io/core/address" math "cosmossdk.io/math" types "cosmossdk.io/store/types" types0 "github.com/cometbft/cometbft/abci/types" @@ -62,6 +63,21 @@ func (mr *MockStakingKeeperMockRecorder) BondDenom(ctx interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BondDenom", reflect.TypeOf((*MockStakingKeeper)(nil).BondDenom), ctx) } +// BondedRatio mocks base method. +func (m *MockStakingKeeper) BondedRatio(ctx context.Context) (math.LegacyDec, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BondedRatio", ctx) + ret0, _ := ret[0].(math.LegacyDec) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// BondedRatio indicates an expected call of BondedRatio. +func (mr *MockStakingKeeperMockRecorder) BondedRatio(ctx interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BondedRatio", reflect.TypeOf((*MockStakingKeeper)(nil).BondedRatio), ctx) +} + // Delegation mocks base method. func (m *MockStakingKeeper) Delegation(ctx context.Context, addr types1.AccAddress, valAddr types1.ValAddress) (types3.DelegationI, error) { m.ctrl.T.Helper() @@ -287,6 +303,34 @@ func (mr *MockStakingKeeperMockRecorder) IsValidatorJailed(ctx, addr interface{} return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsValidatorJailed", reflect.TypeOf((*MockStakingKeeper)(nil).IsValidatorJailed), ctx, addr) } +// IterateBondedValidatorsByPower mocks base method. +func (m *MockStakingKeeper) IterateBondedValidatorsByPower(arg0 context.Context, arg1 func(int64, types3.ValidatorI) bool) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IterateBondedValidatorsByPower", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// IterateBondedValidatorsByPower indicates an expected call of IterateBondedValidatorsByPower. +func (mr *MockStakingKeeperMockRecorder) IterateBondedValidatorsByPower(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IterateBondedValidatorsByPower", reflect.TypeOf((*MockStakingKeeper)(nil).IterateBondedValidatorsByPower), arg0, arg1) +} + +// IterateDelegations mocks base method. +func (m *MockStakingKeeper) IterateDelegations(ctx context.Context, delegator types1.AccAddress, fn func(int64, types3.DelegationI) bool) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IterateDelegations", ctx, delegator, fn) + ret0, _ := ret[0].(error) + return ret0 +} + +// IterateDelegations indicates an expected call of IterateDelegations. +func (mr *MockStakingKeeperMockRecorder) IterateDelegations(ctx, delegator, fn interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IterateDelegations", reflect.TypeOf((*MockStakingKeeper)(nil).IterateDelegations), ctx, delegator, fn) +} + // IterateLastValidatorPowers mocks base method. func (m *MockStakingKeeper) IterateLastValidatorPowers(ctx context.Context, cb func(types1.ValAddress, int64) bool) error { m.ctrl.T.Helper() @@ -447,6 +491,21 @@ func (mr *MockStakingKeeperMockRecorder) SlashWithInfractionReason(ctx, consAddr return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SlashWithInfractionReason", reflect.TypeOf((*MockStakingKeeper)(nil).SlashWithInfractionReason), ctx, consAddr, infractionHeight, power, slashFactor, infraction) } +// StakingTokenSupply mocks base method. +func (m *MockStakingKeeper) StakingTokenSupply(ctx context.Context) (math.Int, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "StakingTokenSupply", ctx) + ret0, _ := ret[0].(math.Int) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// StakingTokenSupply indicates an expected call of StakingTokenSupply. +func (mr *MockStakingKeeperMockRecorder) StakingTokenSupply(ctx interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "StakingTokenSupply", reflect.TypeOf((*MockStakingKeeper)(nil).StakingTokenSupply), ctx) +} + // UnbondingCanComplete mocks base method. func (m *MockStakingKeeper) UnbondingCanComplete(ctx context.Context, id uint64) error { m.ctrl.T.Helper() @@ -505,6 +564,20 @@ func (mr *MockStakingKeeperMockRecorder) Validator(ctx, addr interface{}) *gomoc return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Validator", reflect.TypeOf((*MockStakingKeeper)(nil).Validator), ctx, addr) } +// ValidatorAddressCodec mocks base method. +func (m *MockStakingKeeper) ValidatorAddressCodec() address.Codec { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ValidatorAddressCodec") + ret0, _ := ret[0].(address.Codec) + return ret0 +} + +// ValidatorAddressCodec indicates an expected call of ValidatorAddressCodec. +func (mr *MockStakingKeeperMockRecorder) ValidatorAddressCodec() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ValidatorAddressCodec", reflect.TypeOf((*MockStakingKeeper)(nil).ValidatorAddressCodec)) +} + // ValidatorByConsAddr mocks base method. func (m *MockStakingKeeper) ValidatorByConsAddr(ctx context.Context, consAddr types1.ConsAddress) (types3.ValidatorI, error) { m.ctrl.T.Helper() diff --git a/x/ccv/provider/keeper/keeper.go b/x/ccv/provider/keeper/keeper.go index 1bbd724154..7a7591e26b 100644 --- a/x/ccv/provider/keeper/keeper.go +++ b/x/ccv/provider/keeper/keeper.go @@ -143,6 +143,10 @@ func (k Keeper) mustValidateFields() { // ccv.PanicIfZeroOrNil(k.govKeeper, "govKeeper") // 17 } +func (k *Keeper) SetGovKeeper(govKeeper govkeeper.Keeper) { + k.govKeeper = govKeeper +} + // Logger returns a module-specific logger. func (k Keeper) Logger(ctx context.Context) log.Logger { sdkCtx := sdk.UnwrapSDKContext(ctx) diff --git a/x/ccv/provider/keeper/staking_keeper_interface.go b/x/ccv/provider/keeper/staking_keeper_interface.go new file mode 100644 index 0000000000..a491853fea --- /dev/null +++ b/x/ccv/provider/keeper/staking_keeper_interface.go @@ -0,0 +1,81 @@ +package keeper + +import ( + "context" + + "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" +) + +// Iterates over the consensus-active validators by power. +// The same as IterateBondedValidatorsByPower in the StakingKeeper, +// but only returns the first MaxProviderConsensusValidators validators. +// This is used to implement the interface of the staking keeper to interface with +// modules that need to reference the consensus-active validators. +func (k Keeper) IterateBondedValidatorsByPower(ctx context.Context, fn func(index int64, validator stakingtypes.ValidatorI) (stop bool)) error { + maxProviderConsensusVals := k.GetMaxProviderConsensusValidators(sdk.UnwrapSDKContext(ctx)) + counter := int64(0) + return k.stakingKeeper.IterateBondedValidatorsByPower(ctx, func(index int64, validator stakingtypes.ValidatorI) (stop bool) { + if counter >= maxProviderConsensusVals { + return true + } + counter++ + return fn(index, validator) + }) +} + +// Gets the amount of bonded tokens, which is equal +// to the amount of tokens of the consensus-active validators. +// The same as TotalBondedTokens, but only counts +// tokens of the first MaxProviderConsensusValidators validators. +// This is used to implement the interface of the staking keeper to interface with +// modules that need to reference the consensus-active validators. +func (k Keeper) TotalBondedTokens(ctx context.Context) (math.Int, error) { + // iterate through the bonded validators + totalBondedTokens := math.ZeroInt() + + k.IterateBondedValidatorsByPower(ctx, func(_ int64, validator stakingtypes.ValidatorI) (stop bool) { + tokens := validator.GetTokens() + totalBondedTokens = totalBondedTokens.Add(tokens) + return false + }) + + return totalBondedTokens, nil +} + +// The same as IterateDelegations in the StakingKeeper. +// Necessary to implement the interface of the staking keeper to interface with +// other modules. +func (k Keeper) IterateDelegations(ctx context.Context, delegator sdk.AccAddress, fn func(index int64, delegation stakingtypes.DelegationI) (stop bool)) error { + return k.stakingKeeper.IterateDelegations(ctx, delegator, fn) +} + +// The same as StakingTotalSupply in the StakingKeeper. +// Necessary to implement the interface of the staking keeper to interface with +// other modules. +func (k Keeper) StakingTokenSupply(ctx context.Context) (math.Int, error) { + return k.stakingKeeper.StakingTokenSupply(ctx) +} + +// Gets the ratio of tokens staked to validators active in the consensus +// to the total supply of tokens. +// Same as BondedRatio in the StakingKeeper, but only counts +// tokens of the first MaxProviderConsensusValidators bonded validators. +func (k Keeper) BondedRatio(ctx context.Context) (math.LegacyDec, error) { + totalSupply, err := k.StakingTokenSupply(ctx) + if err != nil { + return math.LegacyZeroDec(), err + } + + bondedTokens, err := k.TotalBondedTokens(ctx) + if err != nil { + return math.LegacyZeroDec(), err + } + + if !totalSupply.IsPositive() { + return math.LegacyZeroDec(), nil + } + + return math.LegacyNewDecFromInt(bondedTokens).QuoInt(totalSupply), nil +} diff --git a/x/ccv/types/expected_keepers.go b/x/ccv/types/expected_keepers.go index e57487df5f..280863daa6 100644 --- a/x/ccv/types/expected_keepers.go +++ b/x/ccv/types/expected_keepers.go @@ -4,6 +4,7 @@ import ( context "context" "time" + addresscodec "cosmossdk.io/core/address" capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types" transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" @@ -57,6 +58,16 @@ type StakingKeeper interface { GetRedelegationByUnbondingID(ctx context.Context, id uint64) (stakingtypes.Redelegation, error) GetValidatorByUnbondingID(ctx context.Context, id uint64) (stakingtypes.Validator, error) GetBondedValidatorsByPower(ctx context.Context) ([]stakingtypes.Validator, error) + ValidatorAddressCodec() addresscodec.Codec + IterateDelegations( + ctx context.Context, delegator sdk.AccAddress, + fn func(index int64, delegation stakingtypes.DelegationI) (stop bool), + ) error + IterateBondedValidatorsByPower( + context.Context, func(index int64, validator stakingtypes.ValidatorI) (stop bool), + ) error + StakingTokenSupply(ctx context.Context) (math.Int, error) + BondedRatio(ctx context.Context) (math.LegacyDec, error) } // SlashingKeeper defines the contract expected to perform ccv slashing From 900a54fd460eaccfcc03f90e8372c39453331c18 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Wed, 17 Jul 2024 17:09:11 +0200 Subject: [PATCH 2/7] Add test for IterateBondedValidatorsByPower --- .../keeper/staking_keeper_interface_test.go | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 x/ccv/provider/keeper/staking_keeper_interface_test.go diff --git a/x/ccv/provider/keeper/staking_keeper_interface_test.go b/x/ccv/provider/keeper/staking_keeper_interface_test.go new file mode 100644 index 0000000000..99d5529d87 --- /dev/null +++ b/x/ccv/provider/keeper/staking_keeper_interface_test.go @@ -0,0 +1,96 @@ +package keeper_test + +import ( + "sort" + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/staking/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + testkeeper "github.com/cosmos/interchain-security/v5/testutil/keeper" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" +) + +// Tests that IterateBondedValidatorsByPower returns the correct validators, +// namely the ones with most power, and stops when the max number of validators is reached. +func TestIterateBondedValidatorsByPower(t *testing.T) { + tests := []struct { + name string + maxValidators int64 + powers []int64 + expectedPowers []int64 + }{ + { + name: "no validators", + maxValidators: 5, + powers: []int64{}, + expectedPowers: []int64{}, + }, + { + name: "validators less than max", + maxValidators: 5, + powers: []int64{10, 20, 30}, + expectedPowers: []int64{10, 20, 30}, // get all validators back + }, + { + name: "validators more than max", + maxValidators: 2, + powers: []int64{10, 20, 30}, + expectedPowers: []int64{30, 20}, // get the top 2 validators back + }, + { + name: "validators equal to max", + maxValidators: 3, + powers: []int64{10, 20, 30}, + expectedPowers: []int64{30, 20, 10}, // get all validators back + }, + { + name: "validators with equal powers", + maxValidators: 3, + powers: []int64{10, 10, 10, 20, 20, 30, 30}, + expectedPowers: []int64{30, 30, 20}, // get the top 3 validators back + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + counter := int64(0) + vals, _ := createStakingValidatorsAndMocks(ctx, mocks, tt.powers...) + + params := providerKeeper.GetParams(ctx) + params.MaxProviderConsensusValidators = tt.maxValidators + providerKeeper.SetParams(ctx, params) + + // sort vals by descending number of tokens + sort.Slice( + vals, + func(i, j int) bool { + return vals[i].GetTokens().Int64() > vals[j].GetTokens().Int64() + }, + ) + + mocks.MockStakingKeeper.EXPECT().IterateBondedValidatorsByPower(gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx sdk.Context, cb func(int64, stakingtypes.ValidatorI) bool) error { + for i, val := range vals { + if stop := cb(int64(i), val); stop { + break + } + } + return nil + }) + actualValPowers := []int64{} + err := providerKeeper.IterateBondedValidatorsByPower(ctx, func(index int64, validator types.ValidatorI) (stop bool) { + counter++ + actualValPowers = append(actualValPowers, validator.GetTokens().Int64()) + return false + }) + require.NoError(t, err) + // we don't check that the elements exactly match; just matching the powers is good enough + require.ElementsMatch(t, tt.expectedPowers, actualValPowers) + }) + } +} From a7a8c9636af8dc83cba4e5ce7859d84b5147d8e6 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Thu, 18 Jul 2024 09:35:55 +0200 Subject: [PATCH 3/7] Rewire GovKeeper --- app/provider/app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/provider/app.go b/app/provider/app.go index c96ab8d2e9..f791c4c953 100644 --- a/app/provider/app.go +++ b/app/provider/app.go @@ -474,7 +474,7 @@ func New( runtime.NewKVStoreService(keys[govtypes.StoreKey]), app.AccountKeeper, app.BankKeeper, - app.StakingKeeper, + app.ProviderKeeper, app.DistrKeeper, app.MsgServiceRouter(), govConfig, From d6bcfe0bcf93661b5c230a27395ccdd365c3b07f Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Thu, 18 Jul 2024 10:21:01 +0200 Subject: [PATCH 4/7] Fix docstrings --- x/ccv/provider/keeper/staking_keeper_interface.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/x/ccv/provider/keeper/staking_keeper_interface.go b/x/ccv/provider/keeper/staking_keeper_interface.go index a491853fea..60786bb030 100644 --- a/x/ccv/provider/keeper/staking_keeper_interface.go +++ b/x/ccv/provider/keeper/staking_keeper_interface.go @@ -8,7 +8,7 @@ import ( stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ) -// Iterates over the consensus-active validators by power. +// IterateBondedValidatorsByPower iterates over the consensus-active validators by power. // The same as IterateBondedValidatorsByPower in the StakingKeeper, // but only returns the first MaxProviderConsensusValidators validators. // This is used to implement the interface of the staking keeper to interface with @@ -25,10 +25,9 @@ func (k Keeper) IterateBondedValidatorsByPower(ctx context.Context, fn func(inde }) } -// Gets the amount of bonded tokens, which is equal -// to the amount of tokens of the consensus-active validators. -// The same as TotalBondedTokens, but only counts -// tokens of the first MaxProviderConsensusValidators validators. +// TotalBondedTokens gets the amount of tokens of the consensus-active validators. +// The same as TotalBondedTokens in the StakingKeeper, but only counts bonded tokens +// of the first MaxProviderConsensusValidators bonded validators. // This is used to implement the interface of the staking keeper to interface with // modules that need to reference the consensus-active validators. func (k Keeper) TotalBondedTokens(ctx context.Context) (math.Int, error) { @@ -44,21 +43,21 @@ func (k Keeper) TotalBondedTokens(ctx context.Context) (math.Int, error) { return totalBondedTokens, nil } -// The same as IterateDelegations in the StakingKeeper. +// IterateDelegations is the same as IterateDelegations in the StakingKeeper. // Necessary to implement the interface of the staking keeper to interface with // other modules. func (k Keeper) IterateDelegations(ctx context.Context, delegator sdk.AccAddress, fn func(index int64, delegation stakingtypes.DelegationI) (stop bool)) error { return k.stakingKeeper.IterateDelegations(ctx, delegator, fn) } -// The same as StakingTotalSupply in the StakingKeeper. +// StakingTokenSupply is the same as StakingTotalSupply in the StakingKeeper. // Necessary to implement the interface of the staking keeper to interface with // other modules. func (k Keeper) StakingTokenSupply(ctx context.Context) (math.Int, error) { return k.stakingKeeper.StakingTokenSupply(ctx) } -// Gets the ratio of tokens staked to validators active in the consensus +// BondedRatio gets the ratio of tokens staked to validators active in the consensus // to the total supply of tokens. // Same as BondedRatio in the StakingKeeper, but only counts // tokens of the first MaxProviderConsensusValidators bonded validators. From e77d510f5811922ab7c66a678254abf0f9c17361 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Thu, 18 Jul 2024 10:21:09 +0200 Subject: [PATCH 5/7] Test other modified functions --- .../keeper/staking_keeper_interface_test.go | 91 +++++++++++++------ 1 file changed, 63 insertions(+), 28 deletions(-) diff --git a/x/ccv/provider/keeper/staking_keeper_interface_test.go b/x/ccv/provider/keeper/staking_keeper_interface_test.go index 99d5529d87..510756bf12 100644 --- a/x/ccv/provider/keeper/staking_keeper_interface_test.go +++ b/x/ccv/provider/keeper/staking_keeper_interface_test.go @@ -4,6 +4,7 @@ import ( "sort" "testing" + "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/staking/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" @@ -12,44 +13,67 @@ import ( "github.com/stretchr/testify/require" ) -// Tests that IterateBondedValidatorsByPower returns the correct validators, -// namely the ones with most power, and stops when the max number of validators is reached. -func TestIterateBondedValidatorsByPower(t *testing.T) { +// TestStakingKeeperInterface tests +// * IterateBondedValidatorsByPower +// * TotalBondedTokens +// * BondedRatio +func TestStakingKeeperInterface(t *testing.T) { tests := []struct { - name string - maxValidators int64 - powers []int64 - expectedPowers []int64 + name string + maxValidators int64 + powers []int64 + expectedPowers []int64 + mockTotalSupply int64 + expectedTotalBonded math.Int + expectedBondedRatio math.LegacyDec }{ { - name: "no validators", - maxValidators: 5, - powers: []int64{}, - expectedPowers: []int64{}, + name: "no validators", + maxValidators: 5, + powers: []int64{}, + expectedPowers: []int64{}, + mockTotalSupply: 100, + expectedTotalBonded: math.ZeroInt(), + expectedBondedRatio: math.LegacyZeroDec(), }, { - name: "validators less than max", - maxValidators: 5, - powers: []int64{10, 20, 30}, - expectedPowers: []int64{10, 20, 30}, // get all validators back + name: "validators less than max", + maxValidators: 5, + powers: []int64{10, 20, 30}, + expectedPowers: []int64{10, 20, 30}, // get all validators back + mockTotalSupply: 100, + expectedTotalBonded: math.NewInt(60), + expectedBondedRatio: math.LegacyNewDec(60).Quo(math.LegacyNewDec(100)), // 60% bonded }, { - name: "validators more than max", - maxValidators: 2, - powers: []int64{10, 20, 30}, - expectedPowers: []int64{30, 20}, // get the top 2 validators back + name: "validators more than max", + maxValidators: 2, + powers: []int64{10, 20, 30}, + expectedPowers: []int64{30, 20}, // get the top 2 validators back + mockTotalSupply: 100, + // 30 + 20 = 50 + expectedTotalBonded: math.NewInt(50), + expectedBondedRatio: math.LegacyNewDec(50).Quo(math.LegacyNewDec(100)), // 50% bonded }, { - name: "validators equal to max", - maxValidators: 3, - powers: []int64{10, 20, 30}, - expectedPowers: []int64{30, 20, 10}, // get all validators back + name: "validators equal to max", + maxValidators: 3, + powers: []int64{10, 20, 30}, + expectedPowers: []int64{30, 20, 10}, // get all validators back + mockTotalSupply: 60, + // 30 + 20 + 10 = 60 + expectedTotalBonded: math.NewInt(60), + expectedBondedRatio: math.LegacyNewDec(100).Quo(math.LegacyNewDec(100)), // 100% bonded }, { - name: "validators with equal powers", - maxValidators: 3, - powers: []int64{10, 10, 10, 20, 20, 30, 30}, - expectedPowers: []int64{30, 30, 20}, // get the top 3 validators back + name: "validators with equal powers", + maxValidators: 3, + powers: []int64{10, 10, 10, 20, 20, 30, 30}, + expectedPowers: []int64{30, 30, 20}, // get the top 3 validators back + mockTotalSupply: 1000, + // 30 + 30 + 20 = 80 + expectedTotalBonded: math.NewInt(80), + expectedBondedRatio: math.LegacyNewDec(8).Quo(math.LegacyNewDec(100)), // 8% bonded }, } @@ -81,7 +105,7 @@ func TestIterateBondedValidatorsByPower(t *testing.T) { } } return nil - }) + }).AnyTimes() actualValPowers := []int64{} err := providerKeeper.IterateBondedValidatorsByPower(ctx, func(index int64, validator types.ValidatorI) (stop bool) { counter++ @@ -91,6 +115,17 @@ func TestIterateBondedValidatorsByPower(t *testing.T) { require.NoError(t, err) // we don't check that the elements exactly match; just matching the powers is good enough require.ElementsMatch(t, tt.expectedPowers, actualValPowers) + + // check bonded ratio and total bonded + mocks.MockStakingKeeper.EXPECT().StakingTokenSupply(gomock.Any()).Return(math.NewInt(tt.mockTotalSupply), nil).AnyTimes() + + totalBonded, err := providerKeeper.TotalBondedTokens(ctx) + require.NoError(t, err) + require.Equal(t, tt.expectedTotalBonded, totalBonded) + + bondedRatio, err := providerKeeper.BondedRatio(ctx) + require.NoError(t, err) + require.Equal(t, tt.expectedBondedRatio, bondedRatio) }) } } From 2008840d2bfa125adcdbe07e53f1abbca4e60a03 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Thu, 18 Jul 2024 10:58:10 +0200 Subject: [PATCH 6/7] Start writing some testing scenarios --- .../adr-017-allowing-inactive-validators.md | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/docs/docs/adrs/adr-017-allowing-inactive-validators.md b/docs/docs/adrs/adr-017-allowing-inactive-validators.md index 52d8d9328a..42eec9978b 100644 --- a/docs/docs/adrs/adr-017-allowing-inactive-validators.md +++ b/docs/docs/adrs/adr-017-allowing-inactive-validators.md @@ -75,6 +75,46 @@ disables the main feature described in this ADR. Additional risk mitigations are to increase the active set size slowly, and to monitor the effects on the network closely. For the first iteration, we propose to increase the active set size to 200 validators (while keeping the consensus validators to 180), thus letting the 20 validators with the most stake outside of the active set validate on consumer chains. +## Testing Scenarios + +In the following, +- bonded validators refers to all validators that have bonded stake, +- active validators refers to the validators that take part in consensus, +- inactive validators refers to bonded validators that are not active validators. + +### Scenario 1: Inactive validators should not be considered by governance + +Inactive validators should not be considered for the purpose of governance. +In particular, the governance module should not allow inactive validators to vote on proposals, +and the quorum depends only on the stake bonded by active validators. + +This can be tested by creating a governance proposal, then trying to vote on it with inactive validators. +The proposal should not pass. +Afterwards, we create another proposal and vote on it with active validators, too. +Then, the proposal should pass. + +### Scenario 2: Inactive validators should not get rewards from the provider chain + +Inactive validators should not get rewards from the provider chain. + +This can be tested by starting a provider chain with inactive validators and checking the rewards of inactive validators. + +### Scenario 3: Inactive validators should get rewards from consumer chains + +An inactive validator that is validating on a consumer chain should receive rewards in the consumer chain token. + +### Scenario 4: Inactive validators should not get slashed/jailed for downtime on the provider chain + +This can be tested by having an inactive validator go offline on the provider chain for long enough to accrue downtime. +The validator should be neither slashed nor jailed for downtime. + +### Scenario 5: Inactive validators *should* get jailed for downtime on the provider chain + +This can be tested by having an inactive validator go offline on a consumer chain for long enough to accrue downtime. +The consumer chain should send a SlashPacket to the provider chain, which should jail the validator. + +* **Mint**: + ## Consequences ### Positive From 1a1a1a9dafa2d92ad00daa097f550e981bec067c Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Thu, 18 Jul 2024 11:27:01 +0200 Subject: [PATCH 7/7] Add TotalBondedTokens to expected staking keeper interface --- testutil/keeper/mocks.go | 15 +++++++++++++++ x/ccv/types/expected_keepers.go | 1 + 2 files changed, 16 insertions(+) diff --git a/testutil/keeper/mocks.go b/testutil/keeper/mocks.go index 600f0596d9..8be9418b97 100644 --- a/testutil/keeper/mocks.go +++ b/testutil/keeper/mocks.go @@ -506,6 +506,21 @@ func (mr *MockStakingKeeperMockRecorder) StakingTokenSupply(ctx interface{}) *go return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "StakingTokenSupply", reflect.TypeOf((*MockStakingKeeper)(nil).StakingTokenSupply), ctx) } +// TotalBondedTokens mocks base method. +func (m *MockStakingKeeper) TotalBondedTokens(ctx context.Context) (math.Int, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "TotalBondedTokens", ctx) + ret0, _ := ret[0].(math.Int) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// TotalBondedTokens indicates an expected call of TotalBondedTokens. +func (mr *MockStakingKeeperMockRecorder) TotalBondedTokens(ctx interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TotalBondedTokens", reflect.TypeOf((*MockStakingKeeper)(nil).TotalBondedTokens), ctx) +} + // UnbondingCanComplete mocks base method. func (m *MockStakingKeeper) UnbondingCanComplete(ctx context.Context, id uint64) error { m.ctrl.T.Helper() diff --git a/x/ccv/types/expected_keepers.go b/x/ccv/types/expected_keepers.go index 280863daa6..9d4ece168f 100644 --- a/x/ccv/types/expected_keepers.go +++ b/x/ccv/types/expected_keepers.go @@ -68,6 +68,7 @@ type StakingKeeper interface { ) error StakingTokenSupply(ctx context.Context) (math.Int, error) BondedRatio(ctx context.Context) (math.LegacyDec, error) + TotalBondedTokens(ctx context.Context) (math.Int, error) } // SlashingKeeper defines the contract expected to perform ccv slashing