diff --git a/.github/workflows/nightly-e2e.yml b/.github/workflows/nightly-e2e.yml index b279c59714..ffc8ff363f 100644 --- a/.github/workflows/nightly-e2e.yml +++ b/.github/workflows/nightly-e2e.yml @@ -310,6 +310,23 @@ jobs: - name: E2E active set changes run: go run ./tests/e2e/... --tc active-set-changes + inactive-provider-validators-on-consumer-test: + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/setup-go@v5 + with: + go-version: "1.22" + - uses: actions/checkout@v4 + - name: Checkout LFS objects + run: git lfs checkout + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version: "1.22" # The Go version to download (if necessary) and use. + - name: E2E inactive provider validators on consumer + run: go run ./tests/e2e/... --tc inactive-provider-validators-on-consumer + nightly-test-fail: needs: - happy-path-test diff --git a/tests/e2e/steps_inactive_vals.go b/tests/e2e/steps_inactive_vals.go index 3fbdbe19d0..7e6f7a3b4a 100644 --- a/tests/e2e/steps_inactive_vals.go +++ b/tests/e2e/steps_inactive_vals.go @@ -13,8 +13,8 @@ import ( // high-level, this test does: // - start the provider chain // - start a consumer chain -// - check that non-consensus validators do not get slashed for downtime; and that they don't get rewards -// - check that active validators *do* get slashed for downtime, and don't get rewards while they are down +// - check that non-consensus validators do not get slashed for downtime on the provider; and that they don't get rewards +// - check that active validators *do* get slashed for downtime on the provider, and don't get rewards while they are down // - check that non-consensus validators *do* get jailed for downtime on consumer chains // - check that non-consensus validators *become* consensus validators when they have enough power func stepsInactiveProviderValidators() []Step { @@ -43,7 +43,9 @@ func stepsInactiveProviderValidators() []Step { }, Rewards: &Rewards{ IsNativeDenom: true, // check for rewards in the provider denom - IsIncrementalReward: true, // check current rewards, because alice gets one block of rewards due to being in the genesis + IsIncrementalReward: true, // we need to get incremental rewards + // if we would look at total rewards, alice would trivially also get rewards, + // because she gets rewards in the first block due to being in the genesis IsRewarded: map[ValidatorID]bool{ ValidatorID("alice"): false, ValidatorID("bob"): true, @@ -142,7 +144,6 @@ func stepsInactiveProviderValidators() []Step { }, State: State{ ChainID("provi"): ChainState{ - // check that now every validator got rewarded since the first block Rewards: &Rewards{ IsNativeDenom: true, // check for rewards in the provider denom IsIncrementalReward: true, // check rewards for currently produced blocks only diff --git a/x/ccv/no_valupdates_genutil/doc.go b/x/ccv/no_valupdates_genutil/doc.go index 466804b6cc..08ad53a8cb 100644 --- a/x/ccv/no_valupdates_genutil/doc.go +++ b/x/ccv/no_valupdates_genutil/doc.go @@ -1,8 +1,8 @@ /* Package staking defines a "wrapper" module around the Cosmos SDK's native -x/staking module. In other words, it provides the exact same functionality as +x/genutil module. In other words, it provides the exact same functionality as the native module in that it simply embeds the native module. However, it -overrides `EndBlock` which will return no validator set updates. Instead, +overrides `InitGenesis` which will return no validator set updates. Instead, it is assumed that some other module will provide the validator set updates. */ package genutil diff --git a/x/ccv/provider/keeper/genesis_test.go b/x/ccv/provider/keeper/genesis_test.go index 179c741959..07fd2ba229 100644 --- a/x/ccv/provider/keeper/genesis_test.go +++ b/x/ccv/provider/keeper/genesis_test.go @@ -154,7 +154,6 @@ func TestInitAndExportGenesis(t *testing.T) { ) mocks.MockStakingKeeper.EXPECT().GetBondedValidatorsByPower(gomock.Any()).Return( - // return an empty set of validators, since they are not tested against here []stakingtypes.Validator{ provVal, }, nil).AnyTimes() diff --git a/x/ccv/provider/keeper/provider_consensus.go b/x/ccv/provider/keeper/provider_consensus.go index ffccd19ed1..462c4d4ae3 100644 --- a/x/ccv/provider/keeper/provider_consensus.go +++ b/x/ccv/provider/keeper/provider_consensus.go @@ -21,7 +21,7 @@ func (k Keeper) SetLastProviderConsensusValidator( } // SetLastProviderConsensusValSet resets the stored last validator set sent to the consensus engine on the provider -// to the provided nextValidators. +// to the provided `nextValidators“. func (k Keeper) SetLastProviderConsensusValSet(ctx sdk.Context, nextValidators []types.ConsensusValidator) { k.setValSet(ctx, []byte{types.LastProviderConsensusValsPrefix}, nextValidators) } @@ -59,6 +59,7 @@ func (k Keeper) GetLastTotalProviderConsensusPower( return k.getTotalPower(ctx, []byte{types.LastProviderConsensusValsPrefix}) } +// CreateProviderConsensusValidator creates a new ConsensusValidator from the given staking validator func (k Keeper) CreateProviderConsensusValidator(ctx sdk.Context, val stakingtypes.Validator) (types.ConsensusValidator, error) { consAddr, err := val.GetConsAddr() if err != nil { diff --git a/x/ccv/provider/keeper/provider_consensus_test.go b/x/ccv/provider/keeper/provider_consensus_test.go index 389aa53981..329fd10f13 100644 --- a/x/ccv/provider/keeper/provider_consensus_test.go +++ b/x/ccv/provider/keeper/provider_consensus_test.go @@ -3,6 +3,7 @@ package keeper_test import ( "testing" + "cosmossdk.io/math" "github.com/cometbft/cometbft/proto/tendermint/crypto" "github.com/stretchr/testify/require" @@ -100,3 +101,24 @@ func TestDeleteLastProviderConsensusValSet(t *testing.T) { storedValidators := providerKeeper.GetLastProviderConsensusValSet(ctx) require.Empty(t, storedValidators, "validator set should be empty") } + +func TestGetLastTotalProviderConsensusPower(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + validator1 := types.ConsensusValidator{ + ProviderConsAddr: []byte("providerConsAddr1"), + Power: 2, + PublicKey: &crypto.PublicKey{}, + } + validator2 := types.ConsensusValidator{ + ProviderConsAddr: []byte("providerConsAddr2"), + Power: 3, + PublicKey: &crypto.PublicKey{}, + } + nextValidators := []types.ConsensusValidator{validator1, validator2} + providerKeeper.SetLastProviderConsensusValSet(ctx, nextValidators) + // Get the total power of the last stored validator set + totalPower := providerKeeper.GetLastTotalProviderConsensusPower(ctx) + expectedTotalPower := math.NewInt(5) + require.Equal(t, expectedTotalPower, totalPower, "total power does not match") +} diff --git a/x/ccv/provider/keeper/relay.go b/x/ccv/provider/keeper/relay.go index 2ae796adfa..9254347c3d 100644 --- a/x/ccv/provider/keeper/relay.go +++ b/x/ccv/provider/keeper/relay.go @@ -180,9 +180,9 @@ func (k Keeper) EndBlockVSU(ctx sdk.Context) ([]abci.ValidatorUpdate, error) { // The function returns the difference between the current validator set and the next validator set as a list of `abci.ValidatorUpdate` objects. func (k Keeper) ProviderValidatorUpdates(ctx sdk.Context) []abci.ValidatorUpdate { // get the bonded validators from the staking module - bondedValidators, error := k.stakingKeeper.GetBondedValidatorsByPower(ctx) - if error != nil { - panic(fmt.Errorf("failed to get bonded validators: %w", error)) + bondedValidators, err := k.stakingKeeper.GetBondedValidatorsByPower(ctx) + if err != nil { + panic(fmt.Errorf("failed to get bonded validators: %w", err)) } // get the last validator set sent to consensus diff --git a/x/ccv/provider/keeper/relay_test.go b/x/ccv/provider/keeper/relay_test.go index 3d4e2ca7a5..98ddaab3fe 100644 --- a/x/ccv/provider/keeper/relay_test.go +++ b/x/ccv/provider/keeper/relay_test.go @@ -775,7 +775,7 @@ func TestEndBlockVSU(t *testing.T) { testkeeper.SetupMocksForLastBondedValidatorsExpectation(mocks.MockStakingKeeper, 5, lastValidators, powers, -1) sort.Slice(lastValidators, func(i, j int) bool { - return lastValidators[i].ConsensusPower(sdk.DefaultPowerReduction) > + return lastValidators[i].GetConsensusPower(sdk.DefaultPowerReduction) > lastValidators[j].GetConsensusPower(sdk.DefaultPowerReduction) }) mocks.MockStakingKeeper.EXPECT().GetBondedValidatorsByPower(gomock.Any()).Return(lastValidators, nil).AnyTimes() diff --git a/x/ccv/provider/keeper/validator_set_storage.go b/x/ccv/provider/keeper/validator_set_storage.go index 31c3235419..3e271fec86 100644 --- a/x/ccv/provider/keeper/validator_set_storage.go +++ b/x/ccv/provider/keeper/validator_set_storage.go @@ -77,10 +77,10 @@ func (k Keeper) isValidator(ctx sdk.Context, prefix []byte, providerAddr types.P // getValSet returns all the validators stored under the given prefix. func (k Keeper) getValSet( ctx sdk.Context, - key []byte, + prefix []byte, ) (validators []types.ConsensusValidator) { store := ctx.KVStore(k.storeKey) - iterator := storetypes.KVStorePrefixIterator(store, key) + iterator := storetypes.KVStorePrefixIterator(store, prefix) defer iterator.Close() for ; iterator.Valid(); iterator.Next() { diff --git a/x/ccv/provider/keeper/validator_set_update.go b/x/ccv/provider/keeper/validator_set_update.go index c3a866fd42..091d11e7b0 100644 --- a/x/ccv/provider/keeper/validator_set_update.go +++ b/x/ccv/provider/keeper/validator_set_update.go @@ -12,8 +12,8 @@ import ( ccv "github.com/cosmos/interchain-security/v5/x/ccv/types" ) -// GetConsumerChainKey returns the store key for consumer validators of the consumer chain with `chainID` -func (k Keeper) GetConsumerChainKey(ctx sdk.Context, chainID string) []byte { +// GetConsumerChainConsensusValidatorsKey returns the store key for consumer validators of the consumer chain with `chainID` +func (k Keeper) GetConsumerChainConsensusValidatorsKey(ctx sdk.Context, chainID string) []byte { return types.ChainIdWithLenKey(types.ConsumerValidatorBytePrefix, chainID) } @@ -23,13 +23,13 @@ func (k Keeper) SetConsumerValidator( chainID string, validator types.ConsensusValidator, ) { - k.setValidator(ctx, k.GetConsumerChainKey(ctx, chainID), validator) + k.setValidator(ctx, k.GetConsumerChainConsensusValidatorsKey(ctx, chainID), validator) } // SetConsumerValSet resets the current consumer validators with the `nextValidators` computed by // `FilterValidators` and hence this method should only be called after `FilterValidators` has completed. func (k Keeper) SetConsumerValSet(ctx sdk.Context, chainID string, nextValidators []types.ConsensusValidator) { - k.setValSet(ctx, k.GetConsumerChainKey(ctx, chainID), nextValidators) + k.setValSet(ctx, k.GetConsumerChainConsensusValidatorsKey(ctx, chainID), nextValidators) } // DeleteConsumerValidator removes consumer validator with `providerAddr` address @@ -38,7 +38,7 @@ func (k Keeper) DeleteConsumerValidator( chainID string, providerConsAddr types.ProviderConsAddress, ) { - k.deleteValidator(ctx, k.GetConsumerChainKey(ctx, chainID), providerConsAddr) + k.deleteValidator(ctx, k.GetConsumerChainConsensusValidatorsKey(ctx, chainID), providerConsAddr) } // DeleteConsumerValSet deletes all the stored consumer validators for chain `chainID` @@ -46,13 +46,13 @@ func (k Keeper) DeleteConsumerValSet( ctx sdk.Context, chainID string, ) { - k.deleteValSet(ctx, k.GetConsumerChainKey(ctx, chainID)) + k.deleteValSet(ctx, k.GetConsumerChainConsensusValidatorsKey(ctx, chainID)) } // IsConsumerValidator returns `true` if the consumer validator with `providerAddr` exists for chain `chainID` // and `false` otherwise func (k Keeper) IsConsumerValidator(ctx sdk.Context, chainID string, providerAddr types.ProviderConsAddress) bool { - return k.isValidator(ctx, k.GetConsumerChainKey(ctx, chainID), providerAddr) + return k.isValidator(ctx, k.GetConsumerChainConsensusValidatorsKey(ctx, chainID), providerAddr) } // GetConsumerValSet returns all the consumer validators for chain `chainID` @@ -60,7 +60,7 @@ func (k Keeper) GetConsumerValSet( ctx sdk.Context, chainID string, ) []types.ConsensusValidator { - return k.getValSet(ctx, k.GetConsumerChainKey(ctx, chainID)) + return k.getValSet(ctx, k.GetConsumerChainConsensusValidatorsKey(ctx, chainID)) } // DiffValidators compares the current and the next epoch's consumer validators and returns the `ValidatorUpdate` diff diff --git a/x/ccv/provider/migrations/vX/migrations.go b/x/ccv/provider/migrations/vX/migrations.go new file mode 100644 index 0000000000..9040c82ee2 --- /dev/null +++ b/x/ccv/provider/migrations/vX/migrations.go @@ -0,0 +1,16 @@ +package v6 + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + providerkeeper "github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper" +) + +// InitializeMaxValidatorsForExistingConsumers initializes the max validators +// parameter for existing consumers to the MaxProviderConsensusValidators parameter. +// This is necessary to avoid those consumer chains having an excessive amount of validators. +func InitializeMaxValidatorsForExistingConsumers(ctx sdk.Context, providerKeeper providerkeeper.Keeper) { + maxVals := providerKeeper.GetParams(ctx).MaxProviderConsensusValidators + for _, chainID := range providerKeeper.GetAllRegisteredConsumerChainIDs(ctx) { + providerKeeper.SetValidatorSetCap(ctx, chainID, uint32(maxVals)) + } +} diff --git a/x/ccv/provider/types/keys.go b/x/ccv/provider/types/keys.go index 2e1315ff93..dba1b84254 100644 --- a/x/ccv/provider/types/keys.go +++ b/x/ccv/provider/types/keys.go @@ -187,7 +187,7 @@ const ( // minimum power required to be in the top N per consumer chain. MinimumPowerInTopNBytePrefix - // LastProviderConsensusValsPrefix is byte prefix for storing the last validator set + // LastProviderConsensusValsPrefix is the byte prefix for storing the last validator set // sent to the consensus engine of the provider chain LastProviderConsensusValsPrefix diff --git a/x/ccv/provider/types/keys_test.go b/x/ccv/provider/types/keys_test.go index 3f6df5cb0a..e6c16dfa7e 100644 --- a/x/ccv/provider/types/keys_test.go +++ b/x/ccv/provider/types/keys_test.go @@ -62,6 +62,7 @@ func getAllKeyPrefixes() []byte { providertypes.ConsumerRewardsAllocationBytePrefix, providertypes.ConsumerCommissionRatePrefix, providertypes.MinimumPowerInTopNBytePrefix, + providertypes.LastProviderConsensusValsPrefix, providertypes.ParametersByteKey, } }