diff --git a/x/ccv/provider/keeper/keeper.go b/x/ccv/provider/keeper/keeper.go index ea6562fef9..aeb206a302 100644 --- a/x/ccv/provider/keeper/keeper.go +++ b/x/ccv/provider/keeper/keeper.go @@ -1190,14 +1190,17 @@ func (k Keeper) SetOptedIn( chainID string, providerAddr types.ProviderConsAddress, blockHeight uint64, + power uint64, ) { store := ctx.KVStore(k.storeKey) - // validator is considered opted in blockHeightBytes := make([]byte, 8) binary.BigEndian.PutUint64(blockHeightBytes, blockHeight) - store.Set(types.OptedInKey(chainID, providerAddr), blockHeightBytes) + powerBytes := make([]byte, 8) + binary.BigEndian.PutUint64(powerBytes, power) + + store.Set(types.OptedInKey(chainID, providerAddr), append(blockHeightBytes, powerBytes...)) } func (k Keeper) DeleteOptedIn( @@ -1242,7 +1245,8 @@ func (k Keeper) GetOptedIn( for ; iterator.Valid(); iterator.Next() { optedInValidators = append(optedInValidators, OptedInValidator{ ProviderAddr: types.NewProviderConsAddress(iterator.Key()[len(key):]), - BlockHeight: binary.BigEndian.Uint64(iterator.Value()), + BlockHeight: binary.BigEndian.Uint64(iterator.Value()[0:8]), + Power: binary.BigEndian.Uint64(iterator.Value()[8:]), }) } diff --git a/x/ccv/provider/keeper/keeper_test.go b/x/ccv/provider/keeper/keeper_test.go index 6eb28675ba..a3605a31fe 100644 --- a/x/ccv/provider/keeper/keeper_test.go +++ b/x/ccv/provider/keeper/keeper_test.go @@ -670,20 +670,24 @@ func TestGetOptedIn(t *testing.T) { { ProviderAddr: types.NewProviderConsAddress([]byte("providerAddr1")), BlockHeight: 1, + Power: 2, }, { ProviderAddr: types.NewProviderConsAddress([]byte("providerAddr2")), BlockHeight: 2, + Power: 3, }, { ProviderAddr: types.NewProviderConsAddress([]byte("providerAddr3")), BlockHeight: 3, + Power: 4, }, } for _, expectedOptedInValidator := range expectedOptedInValidators { providerKeeper.SetOptedIn(ctx, "chainID", - expectedOptedInValidator.ProviderAddr, expectedOptedInValidator.BlockHeight) + expectedOptedInValidator.ProviderAddr, expectedOptedInValidator.BlockHeight, + expectedOptedInValidator.Power) } actualOptedInValidators := providerKeeper.GetOptedIn(ctx, "chainID") @@ -710,10 +714,11 @@ func TestOptedIn(t *testing.T) { optedInValidator := keeper.OptedInValidator{ ProviderAddr: types.NewProviderConsAddress([]byte("providerAddr")), BlockHeight: 1, + Power: 2, } require.False(t, providerKeeper.IsOptedIn(ctx, "chainID", optedInValidator.ProviderAddr)) - providerKeeper.SetOptedIn(ctx, "chainID", optedInValidator.ProviderAddr, optedInValidator.BlockHeight) + providerKeeper.SetOptedIn(ctx, "chainID", optedInValidator.ProviderAddr, optedInValidator.BlockHeight, optedInValidator.Power) require.True(t, providerKeeper.IsOptedIn(ctx, "chainID", optedInValidator.ProviderAddr)) providerKeeper.DeleteOptedIn(ctx, "chainID", optedInValidator.ProviderAddr) require.False(t, providerKeeper.IsOptedIn(ctx, "chainID", optedInValidator.ProviderAddr)) diff --git a/x/ccv/provider/keeper/key_assignment.go b/x/ccv/provider/keeper/key_assignment.go index 465157ce1e..5b66fc7b86 100644 --- a/x/ccv/provider/keeper/key_assignment.go +++ b/x/ccv/provider/keeper/key_assignment.go @@ -554,7 +554,7 @@ func (k Keeper) MustApplyKeyAssignmentToValUpdates( ctx sdk.Context, chainID string, valUpdates []abci.ValidatorUpdate, - considerReplacement func(address types.ProviderConsAddress) bool, + considerKeyReplacement func(address types.ProviderConsAddress) bool, ) (newUpdates []abci.ValidatorUpdate) { for _, valUpdate := range valUpdates { providerAddrTmp, err := ccvtypes.TMCryptoPublicKeyToConsAddr(valUpdate.PubKey) @@ -611,7 +611,7 @@ func (k Keeper) MustApplyKeyAssignmentToValUpdates( providerAddr := types.NewProviderConsAddress(replacement.ProviderAddr) // only consider updates for validators that are considered here ... - if !considerReplacement(providerAddr) { + if !considerKeyReplacement(providerAddr) { return } k.DeleteKeyAssignmentReplacement(ctx, chainID, providerAddr) diff --git a/x/ccv/provider/keeper/partial_set_security.go b/x/ccv/provider/keeper/partial_set_security.go index ba6c371949..334afe8128 100644 --- a/x/ccv/provider/keeper/partial_set_security.go +++ b/x/ccv/provider/keeper/partial_set_security.go @@ -13,6 +13,8 @@ type OptedInValidator struct { ProviderAddr types.ProviderConsAddress // block height the validator opted in at BlockHeight uint64 + // power the validator had when it opted in + Power uint64 } func (k Keeper) HandleOptIn(ctx sdk.Context, chainID string, providerAddr types.ProviderConsAddress, consumerKey *string) error { @@ -22,20 +24,11 @@ func (k Keeper) HandleOptIn(ctx sdk.Context, chainID string, providerAddr types. "opting in to an unknown consumer chain, with id: %s", chainID) } - val, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, providerAddr.ToSdkConsAddr()) + validator, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, providerAddr.ToSdkConsAddr()) if !found { return errorsmod.Wrapf( types.ErrNoValidatorProviderAddress, "could not find validator with consensus address: %s", providerAddr.ToSdkConsAddr().Bytes()) - } else if !val.IsBonded() { - // FIXME: problematic ... - // Only active validators are allowed to opt in. Note that the fact that a validator is bonded when sending - // a `MsgOptIn` message does not guarantee that the validator would still be bonded when the validator actually - // opts in (e.g., at the end of a block or of an epoch). We recheck if validators are bonded in - // `GetToBeOptedInValidatorUpdates` before sending the partial set to a consumer chain. - return errorsmod.Wrapf( - types.ErrValidatorNotBonded, - "validator with consensus address: %s is not bonded", providerAddr.ToSdkConsAddr().Bytes()) } if k.IsToBeOptedOut(ctx, chainID, providerAddr) { @@ -52,11 +45,6 @@ func (k Keeper) HandleOptIn(ctx sdk.Context, chainID string, providerAddr types. return err } - validator, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, providerAddr.Address) - if !found { - return stakingtypes.ErrNoValidatorFound - } - err = k.AssignConsumerKey(ctx, chainID, validator, consumerTMPublicKey) if err != nil { return err @@ -86,6 +74,7 @@ func (k Keeper) HandleOptOut(ctx sdk.Context, chainID string, providerAddr types return nil } +// getValidatorsPublicKey is a helper function that returns the public key of the validator func (k Keeper) getValidatorsPublicKey(validator stakingtypes.Validator) (tmprotocrypto.PublicKey, error) { consAddr, err := validator.GetConsAddr() if err != nil { @@ -98,163 +87,171 @@ func (k Keeper) getValidatorsPublicKey(validator stakingtypes.Validator) (tmprot }, nil } -// GetToBeOptedInValidatorUpdates returns all the needed `ValidatorUpdate`s for validators that are to be opted in -// on consumer chain with `chainID` -func (k Keeper) GetToBeOptedInValidatorUpdates(ctx sdk.Context, chainID string) []abci.ValidatorUpdate { - var updates []abci.ValidatorUpdate - for _, val := range k.GetToBeOptedIn(ctx, chainID) { - validator, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, val.ToSdkConsAddr()) +// GetToBeOptedInValidators returns all the needed validators that are to be opted in +// on consumer chain with `chainID` and that are still bonded +func (k Keeper) GetToBeOptedInValidators(ctx sdk.Context, chainID string) []types.ProviderConsAddress { + var consAddresses []types.ProviderConsAddress + for _, addr := range k.GetToBeOptedIn(ctx, chainID) { + validator, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, addr.ToSdkConsAddr()) if !found { // a validator was successfully set to be opted in, but we cannot find this validator anymore - k.Logger(ctx).Error("could not find validator with consensus address: %s", val.ToSdkConsAddr().Bytes()) + k.Logger(ctx).Error("could not find to-be-opted-in invalidator with consensus address: %s", addr.ToSdkConsAddr().Bytes()) + continue } - // FIXME: bonded means in the active ... if !validator.IsBonded() { - // a validator might have started unbonding or unbonded since it asked to be opted in + // only consider validators that are in the active set continue } + consAddresses = append(consAddresses, addr) + } - pubKey, err := k.getValidatorsPublicKey(validator) - if err != nil { - k.Logger(ctx).Error("could not find validator with consensus address: %s", val.ToSdkConsAddr().Bytes()) + return consAddresses +} + +// GetNextOptedInValidators returns all the addresses of validators that are going to be opted in +// next on the consumer chain with `chainID` and a validator update would need to be sent for them +func (k Keeper) GetNextOptedInValidators(ctx sdk.Context, chainID string) []types.ProviderConsAddress { + var consAddresses []types.ProviderConsAddress + + // Create set that contains all the validators that are to be opted out so that we do not reintroduce opted-out + // validators when going through the already opted in validators. + isSetToBeOptedOut := make(map[string]bool) + for _, addr := range k.GetToBeOptedOut(ctx, chainID) { + isSetToBeOptedOut[addr.ToSdkConsAddr().String()] = true + } + + for _, val := range k.GetOptedIn(ctx, chainID) { + // go through all the validators that are currently opted in + validator, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, val.ProviderAddr.ToSdkConsAddr()) + if !found { + // a validator was opted in, but we cannot find this validator anymore + k.Logger(ctx).Error("could not find opted-in validator with consensus address: %s", + val.ProviderAddr.ToSdkConsAddr().Bytes()) continue } - updates = append(updates, abci.ValidatorUpdate{ - PubKey: pubKey, - Power: k.stakingKeeper.GetLastValidatorPower(ctx, validator.GetOperator()), - }) + if isSetToBeOptedOut[val.ProviderAddr.ToSdkConsAddr().String()] { + continue + } + + if val.Power == uint64(k.stakingKeeper.GetLastValidatorPower(ctx, validator.GetOperator())) { + // Only send an update if an opted-in validator had a power change since the lat time it was sent. + // If an opted-in validator is not in the active set anymore, then its new power is 0 and hence we + // do not consider this validator in the next set of opted in validators. + continue + } + + consAddresses = append(consAddresses, val.ProviderAddr) } - return updates + // append all the to-be-opted-in validators + consAddresses = append(consAddresses, k.GetToBeOptedInValidators(ctx, chainID)...) + return consAddresses } -// GetToBeOptedOutValidatorUpdates returns all the needed `ValidatorUpdate`s for validators that are to be opted out -// of the consumer chain with `chainID` -func (k Keeper) GetToBeOptedOutValidatorUpdates(ctx sdk.Context, chainID string) []abci.ValidatorUpdate { - var updates []abci.ValidatorUpdate - for _, val := range k.GetToBeOptedOut(ctx, chainID) { +// ComputePartialSetValidatorUpdates returns the partial set of `ValidatorUpdate`s that the provider chain sends to the +// consumer chain. Receives `nextOptedIn` that corresponds to the validators that would be opted in next and +// `toBeOptedOut` that contains the validators that would be opted out. +func (k Keeper) ComputePartialSetValidatorUpdates(ctx sdk.Context, nextOptedIn []types.ProviderConsAddress, + toBeOptedOut []types.ProviderConsAddress) []abci.ValidatorUpdate { + var partialSetUpdates []abci.ValidatorUpdate + + for _, val := range nextOptedIn { validator, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, val.ToSdkConsAddr()) if !found { - // a validator was successfully set to be opted in, but we cannot find this validator anymore - k.Logger(ctx).Error("could not find validator with consensus address: %s", val.ToSdkConsAddr().Bytes()) + // any validator in `nextOptedIn` should be found + k.Logger(ctx).Error("could not find validator with consensus address: %s", + val.ToSdkConsAddr().Bytes()) continue } pubKey, err := k.getValidatorsPublicKey(validator) if err != nil { + k.Logger(ctx).Error("could retrieve public key from validator with consensus address: %s", + val.ToSdkConsAddr().Bytes()) continue } - updates = append(updates, abci.ValidatorUpdate{ + // if a validator that was opted in, is not in the active set anymore, then `GetLastValidatorPower` returns 0 + partialSetUpdates = append(partialSetUpdates, abci.ValidatorUpdate{ PubKey: pubKey, - Power: 0, + Power: k.stakingKeeper.GetLastValidatorPower(ctx, validator.GetOperator()), }) } - return updates -} - -// ComputePartialSetValUpdates computes and returns the partial set `ValidatorUpdate`s for given chain with `chainID` -// and provided the `stakingUpdates` stemming from the staking module -func (k Keeper) ComputePartialSetValUpdates(ctx sdk.Context, chainID string, stakingUpdates []abci.ValidatorUpdate) []abci.ValidatorUpdate { - var partialSetUpdates []abci.ValidatorUpdate - - toBeOptedInValidatorUpdates := k.GetToBeOptedInValidatorUpdates(ctx, chainID) - toBeOptedOutValidatorUpdates := k.GetToBeOptedOutValidatorUpdates(ctx, chainID) - - partialSetUpdates = append(partialSetUpdates, toBeOptedInValidatorUpdates...) - partialSetUpdates = append(partialSetUpdates, toBeOptedOutValidatorUpdates...) - - // Create set that contains all the validators that are to be opted out so that we do not reintroduce opted-out - // validators when going through the already opted in validators. Opted out validators are already included - // in the partial set updates through `toBeOptedOutValidatorUpdates`. - isSetToBeOptedOut := make(map[string]bool) - for _, val := range toBeOptedOutValidatorUpdates { - isSetToBeOptedOut[val.PubKey.String()] = true - } - // Create set that contains all the validators that stem from `stakingUpdates` changes so that we only send - // validator updates for validators that had a change in their voting power. - isStakingUpdate := make(map[string]bool) - for _, val := range stakingUpdates { - isStakingUpdate[val.PubKey.String()] = true - } - - for _, val := range k.GetOptedIn(ctx, chainID) { - validator, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, val.ProviderAddr.ToSdkConsAddr()) + for _, addr := range toBeOptedOut { + // go through all the validators that are currently opted in + validator, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, addr.ToSdkConsAddr()) if !found { + // a validator was opted in, but we cannot find this validator anymore + k.Logger(ctx).Error("could not find opted-in validator with consensus address: %s", + addr.ToSdkConsAddr().Bytes()) continue } pubKey, err := k.getValidatorsPublicKey(validator) if err != nil { + k.Logger(ctx).Error("could retrieve public key from validator with consensus address: %s", + addr.ToSdkConsAddr().Bytes()) continue } - if isSetToBeOptedOut[pubKey.String()] { - // do not create a `ValidatorUpdate` for validators that opt out - continue - } - - if !isStakingUpdate[pubKey.String()] { - // only send an update if an opted in validator had a staking update from the staking module and - // hence a voting power change - continue - } - + // send power 0 so the validator is removed partialSetUpdates = append(partialSetUpdates, abci.ValidatorUpdate{ PubKey: pubKey, - Power: k.stakingKeeper.GetLastValidatorPower(ctx, validator.GetOperator()), + Power: 0, }) } return partialSetUpdates } -func (k Keeper) ResetPartialSet(ctx sdk.Context, chainID string) { - var optedOutOnes map[string]bool - for _, v := range k.GetToBeOptedIn(ctx, chainID) { - optedOutOnes[v.String()] = true +// ResetOptedInSet resets the opted-in validators with the newest set that was computed by +// `ComputePartialSetValidatorUpdates` and hence this method should only be called after +// `ComputePartialSetValidatorUpdates` has complete. Also, clears all the `ToBeOptedIn` and `ToBeOptedOut` sets. +func (k Keeper) ResetOptedInSet(ctx sdk.Context, chainID string) { + // Create set that contains all the validators that are to be opted out so that we do not consider opted-out + // validators when going through the already opted in validators. + isSetToBeOptedOut := make(map[string]bool) + for _, val := range k.GetToBeOptedOut(ctx, chainID) { + isSetToBeOptedOut[val.ToSdkConsAddr().String()] = true } - var allOfThem []types.ProviderConsAddress - for _, v := range k.GetOptedIn(ctx, chainID) { - // FOXME: - validator, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, v.ProviderAddr.ToSdkConsAddr()) + optedInValidators := k.GetOptedIn(ctx, chainID) + k.DeleteAllOptedIn(ctx, chainID) + + for _, val := range optedInValidators { + validator, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, val.ProviderAddr.ToSdkConsAddr()) if !found { - // probably an error + // any validator in `nextOptedIn` should be found + k.Logger(ctx).Error("could not find validator with consensus address: %s", + val.ProviderAddr.ToSdkConsAddr().Bytes()) continue } - if !validator.IsBonded() { + + if isSetToBeOptedOut[val.ProviderAddr.ToSdkConsAddr().String()] { + // do not create a `ValidatorUpdate` for validators that opted out continue } - // only still bonded ones ... - if optedOutOnes[v.ProviderAddr.String()] { - // here you would need ot remove + + power := uint64(k.stakingKeeper.GetLastValidatorPower(ctx, validator.GetOperator())) + if power == 0 { + // validator has unbonded continue } - allOfThem = append(allOfThem, v.ProviderAddr) + + // we only update the power of the validator, the height the validator first opted in at remains the same + k.SetOptedIn(ctx, chainID, val.ProviderAddr, val.BlockHeight, power) } - for _, v := range k.GetToBeOptedIn(ctx, chainID) { - // only still bonded ones ... - validator, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, v.ToSdkConsAddr()) + for _, addr := range k.GetToBeOptedInValidators(ctx, chainID) { + validator, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, addr.ToSdkConsAddr()) if !found { - // probably an error - continue - } - if !validator.IsBonded() { continue } - allOfThem = append(allOfThem, types.NewProviderConsAddress(v.Address)) - } - for _, v := range allOfThem { - if !k.IsOptedIn(ctx, chainID, v) { - k.SetOptedIn(ctx, chainID, v, uint64(ctx.BlockHeight())) - } else { - // leave the previous block height as is - } + // this validator was not in the opted-in validators before, so set its height to be the current one + k.SetOptedIn(ctx, chainID, addr, uint64(ctx.BlockHeight()), uint64(k.stakingKeeper.GetLastValidatorPower(ctx, validator.GetOperator()))) } k.DeleteAllToBeOptedIn(ctx, chainID) diff --git a/x/ccv/provider/keeper/partial_set_security_test.go b/x/ccv/provider/keeper/partial_set_security_test.go index eab9feec04..c5ff4c2b23 100644 --- a/x/ccv/provider/keeper/partial_set_security_test.go +++ b/x/ccv/provider/keeper/partial_set_security_test.go @@ -1,7 +1,6 @@ package keeper_test import ( - "fmt" abci "github.com/cometbft/cometbft/abci/types" tmprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" codectypes "github.com/cosmos/cosmos-sdk/codec/types" @@ -9,11 +8,13 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" testkeeper "github.com/cosmos/interchain-security/v4/testutil/keeper" + "github.com/cosmos/interchain-security/v4/x/ccv/provider/keeper" "github.com/cosmos/interchain-security/v4/x/ccv/provider/types" ccvtypes "github.com/cosmos/interchain-security/v4/x/ccv/types" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" "sort" + "strings" "testing" ) @@ -23,25 +24,13 @@ func TestHandleOptIn(t *testing.T) { providerAddr := types.NewProviderConsAddress([]byte("providerAddr")) - // mock `GetValidatorByConsAddr` that returns a `Bonded` validator for the `providerAddr` address and - // an `Unbonding` validator for any other address mocks.MockStakingKeeper.EXPECT(). GetValidatorByConsAddr(gomock.Any(), gomock.Any()). DoAndReturn(func(ctx sdk.Context, addr sdk.ConsAddress) (stakingtypes.Validator, bool) { - if addr.Equals(providerAddr.Address) { - pkAny, _ := codectypes.NewAnyWithValue(ed25519.GenPrivKeyFromSecret([]byte{1}).PubKey()) - return stakingtypes.Validator{ConsensusPubkey: pkAny, Status: stakingtypes.Bonded}, true - } else { - pkAny, _ := codectypes.NewAnyWithValue(ed25519.GenPrivKeyFromSecret([]byte{2}).PubKey()) - return stakingtypes.Validator{ConsensusPubkey: pkAny, Status: stakingtypes.Unbonding}, true - } + pkAny, _ := codectypes.NewAnyWithValue(ed25519.GenPrivKeyFromSecret([]byte{1}).PubKey()) + return stakingtypes.Validator{ConsensusPubkey: pkAny}, true }).AnyTimes() - // verify that a non-`Bonded` validator cannot opt in - unbondedProviderAddr := types.NewProviderConsAddress([]byte("unbondedProviderAddr")) - providerKeeper.SetProposedConsumerChain(ctx, "someChainID", 1) - require.Error(t, providerKeeper.HandleOptIn(ctx, "someChainID", unbondedProviderAddr, nil)) - // trying to opt in to a non-proposed and non-registered chain returns an error require.Error(t, providerKeeper.HandleOptIn(ctx, "unknownChainID", providerAddr, nil)) @@ -55,7 +44,7 @@ func TestHandleOptIn(t *testing.T) { require.False(t, providerKeeper.IsToBeOptedOut(ctx, "chainID", providerAddr)) // if validator (`providerAddr`) is already opted in, then the validator cannot be opted in - providerKeeper.SetOptedIn(ctx, "chainID", providerAddr, 1) + providerKeeper.SetOptedIn(ctx, "chainID", providerAddr, 1, 2) providerKeeper.HandleOptIn(ctx, "chainID", providerAddr, nil) require.False(t, providerKeeper.IsToBeOptedIn(ctx, "chainID", providerAddr)) } @@ -82,7 +71,7 @@ func TestHandleOptInWithConsumerKey(t *testing.T) { // for any other consensus address, we cannot find a validator return stakingtypes.Validator{}, false } - }).Times(3), + }).Times(2), } gomock.InOrder(calls...) @@ -133,34 +122,30 @@ func TestHandleOptOut(t *testing.T) { require.False(t, providerKeeper.IsToBeOptedOut(ctx, "chainID", providerAddr)) } -func TestGetToBeOptedInValidatorUpdates(t *testing.T) { +func TestGetToBeOptedInValidators(t *testing.T) { providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() - type TestValidator struct { - validator stakingtypes.Validator - power int64 - expectedValUpdate abci.ValidatorUpdate - } - chainID := "chainID" - var testValidators []TestValidator + var expectedAddresses []types.ProviderConsAddress + // set 10 validators as to-be-opted-in, but only 3 (= 10/3) of those are `Bonded` for i := int64(0); i < 10; i++ { // generate a consensus public key for the provider providerConsPubKey := ed25519.GenPrivKeyFromSecret([]byte{uint8(i)}).PubKey() consAddr := sdk.ConsAddress(providerConsPubKey.Address()) providerAddr := types.NewProviderConsAddress(consAddr) - foo := providerAddr.Address.Bytes() var providerValidatorAddr sdk.ValAddress - providerValidatorAddr = foo + providerValidatorAddr = providerAddr.Address.Bytes() var status stakingtypes.BondStatus if i%3 == 0 { status = stakingtypes.Unbonded } else if i%3 == 1 { status = stakingtypes.Bonded + // we only expect bonded validators to be returned by `GetToBeOptedInValidators` + expectedAddresses = append(expectedAddresses, providerAddr) } else { status = stakingtypes.Unbonding } @@ -172,116 +157,162 @@ func TestGetToBeOptedInValidatorUpdates(t *testing.T) { Status: status, } - consumerTMPublicKey := tmprotocrypto.PublicKey{ - Sum: &tmprotocrypto.PublicKey_Ed25519{ - Ed25519: consAddr.Bytes(), - }, - } - - expectedValUpdate := abci.ValidatorUpdate{PubKey: consumerTMPublicKey, Power: i} - - testValidators = append(testValidators, - TestValidator{validator: validator, power: i, expectedValUpdate: expectedValUpdate}) + mocks.MockStakingKeeper.EXPECT(). + GetValidatorByConsAddr(ctx, consAddr).Return(validator, true).AnyTimes() providerKeeper.SetToBeOptedIn(ctx, chainID, providerAddr) } - for _, val := range testValidators { - consAddr, _ := val.validator.GetConsAddr() + actualAddresses := providerKeeper.GetToBeOptedInValidators(ctx, chainID) + + // sort before comparing + sort.Slice(expectedAddresses, func(i int, j int) bool { + return strings.Compare(expectedAddresses[i].String(), expectedAddresses[j].String()) < 0 + }) + sort.Slice(actualAddresses, func(i int, j int) bool { + return strings.Compare(actualAddresses[i].String(), actualAddresses[j].String()) < 0 + }) + require.Equal(t, expectedAddresses, actualAddresses) +} + +func TestGetNextOptedInValidators(t *testing.T) { + providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + chainID := "chainID" + + // create 10 validators + var providerAddresses []types.ProviderConsAddress + for i := 0; i < 10; i++ { + // generate a consensus public key for the provider + providerConsPubKey := ed25519.GenPrivKeyFromSecret([]byte{uint8(i)}).PubKey() + consAddr := sdk.ConsAddress(providerConsPubKey.Address()) + providerAddr := types.NewProviderConsAddress(consAddr) + providerAddresses = append(providerAddresses, providerAddr) + + var providerValidatorAddr sdk.ValAddress + providerValidatorAddr = providerAddr.Address.Bytes() + + pkAny, _ := codectypes.NewAnyWithValue(providerConsPubKey) + validator := stakingtypes.Validator{ + OperatorAddress: providerValidatorAddr.String(), + ConsensusPubkey: pkAny, + Status: stakingtypes.Bonded, + } + mocks.MockStakingKeeper.EXPECT(). - GetValidatorByConsAddr(ctx, consAddr).Return(val.validator, true).AnyTimes() + GetValidatorByConsAddr(ctx, consAddr).Return(validator, true).AnyTimes() + mocks.MockStakingKeeper.EXPECT(). - GetLastValidatorPower(ctx, val.validator.GetOperator()).Return(val.power).AnyTimes() + GetLastValidatorPower(ctx, providerValidatorAddr).Return(int64(i)).AnyTimes() } - var expectedValUpdates []abci.ValidatorUpdate - for _, val := range testValidators { - if !val.validator.IsBonded() { - continue - } - expectedValUpdates = append(expectedValUpdates, val.expectedValUpdate) + // first 3 validators (i.e., 0, 1, and 2) are already opted in but with the same power as they currently have + // and therefore should not be returned by `GetNextOptedInValidators` + for i := 0; i < 3; i++ { + providerKeeper.SetOptedIn(ctx, chainID, providerAddresses[i], 1, uint64(i)) + + } + + // validators 3, 4, 5 and 6 are already opted in but with a different voting power + // and therefore should be returned by `GetNextOptedInValidators` (unless opted out -- see below) + for i := 3; i < 7; i++ { + providerKeeper.SetOptedIn(ctx, chainID, providerAddresses[i], 1, uint64(i+1)) + } + + // validators 8 and 9 are to be opted in + for i := 8; i < 10; i++ { + providerKeeper.SetToBeOptedIn(ctx, chainID, providerAddresses[i]) + } + + // first 5 validators are to be opted out and hence validators 3 and 4 won't be returned + // by `GetNextOptedInValidators` + for i := 0; i < 5; i++ { + providerKeeper.SetToBeOptedOut(ctx, chainID, providerAddresses[i]) } - actualValUpdates := providerKeeper.GetToBeOptedInValidatorUpdates(ctx, chainID) + expectedAddresses := []types.ProviderConsAddress{ + providerAddresses[5], + providerAddresses[6], + providerAddresses[8], + providerAddresses[9], + } + actualAddresses := providerKeeper.GetNextOptedInValidators(ctx, chainID) // sort before comparing - sort.Slice(expectedValUpdates, func(i int, j int) bool { - if expectedValUpdates[i].PubKey.Compare(expectedValUpdates[j].PubKey) < 0 { - return true - } else if expectedValUpdates[i].PubKey.Compare(expectedValUpdates[j].PubKey) == 0 { - return expectedValUpdates[i].Power < expectedValUpdates[j].Power - } - return false + sort.Slice(expectedAddresses, func(i int, j int) bool { + return strings.Compare(expectedAddresses[i].String(), expectedAddresses[j].String()) < 0 }) - sort.Slice(actualValUpdates, func(i int, j int) bool { - if actualValUpdates[i].PubKey.Compare(actualValUpdates[j].PubKey) < 0 { - return true - } else if actualValUpdates[i].PubKey.Compare(actualValUpdates[j].PubKey) == 0 { - return actualValUpdates[i].Power < actualValUpdates[j].Power - } - return false + sort.Slice(actualAddresses, func(i int, j int) bool { + return strings.Compare(actualAddresses[i].String(), actualAddresses[j].String()) < 0 }) - - require.Equal(t, expectedValUpdates, actualValUpdates) + require.Equal(t, expectedAddresses, actualAddresses) } -func TestGetToBeOptedOutValidatorUpdates(t *testing.T) { +func TestComputePartialSetValidatorUpdates(t *testing.T) { providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() - type TestValidator struct { - validator stakingtypes.Validator - power int64 - expectedValUpdate abci.ValidatorUpdate - } - - chainID := "chainID" - - var testValidators []TestValidator - for i := int64(0); i < 10; i++ { + // create 10 validators + var providerAddresses []types.ProviderConsAddress + var pubKeys []tmprotocrypto.PublicKey + for i := 0; i < 10; i++ { // generate a consensus public key for the provider providerConsPubKey := ed25519.GenPrivKeyFromSecret([]byte{uint8(i)}).PubKey() consAddr := sdk.ConsAddress(providerConsPubKey.Address()) providerAddr := types.NewProviderConsAddress(consAddr) + providerAddresses = append(providerAddresses, providerAddr) - foo := providerAddr.Address.Bytes() var providerValidatorAddr sdk.ValAddress - providerValidatorAddr = foo + providerValidatorAddr = providerAddr.Address.Bytes() pkAny, _ := codectypes.NewAnyWithValue(providerConsPubKey) validator := stakingtypes.Validator{ OperatorAddress: providerValidatorAddr.String(), ConsensusPubkey: pkAny, + Status: stakingtypes.Bonded, } - consumerTMPublicKey := tmprotocrypto.PublicKey{ + pubKey := tmprotocrypto.PublicKey{ Sum: &tmprotocrypto.PublicKey_Ed25519{ Ed25519: consAddr.Bytes(), }, } + pubKeys = append(pubKeys, pubKey) - expectedValUpdate := abci.ValidatorUpdate{PubKey: consumerTMPublicKey, Power: 0} + mocks.MockStakingKeeper.EXPECT(). + GetValidatorByConsAddr(ctx, consAddr).Return(validator, true).AnyTimes() - testValidators = append(testValidators, - TestValidator{validator: validator, power: i, expectedValUpdate: expectedValUpdate}) + mocks.MockStakingKeeper.EXPECT(). + GetLastValidatorPower(ctx, providerValidatorAddr).Return(int64(i)).AnyTimes() + } - providerKeeper.SetToBeOptedOut(ctx, chainID, providerAddr) + // first 6 validators to opt in + var nextOptedIn []types.ProviderConsAddress + for i := 0; i < 6; i++ { + nextOptedIn = append(nextOptedIn, providerAddresses[i]) } - for _, val := range testValidators { - consAddr, _ := val.validator.GetConsAddr() - mocks.MockStakingKeeper.EXPECT(). - GetValidatorByConsAddr(ctx, consAddr).Return(val.validator, true).AnyTimes() + var optedOut []types.ProviderConsAddress + for i := 6; i < 10; i++ { + optedOut = append(optedOut, providerAddresses[i]) } - var expectedValUpdates []abci.ValidatorUpdate - for _, val := range testValidators { - expectedValUpdates = append(expectedValUpdates, val.expectedValUpdate) + expectedValUpdates := []abci.ValidatorUpdate{ + {PubKey: pubKeys[0], Power: 0}, + {PubKey: pubKeys[1], Power: 1}, + {PubKey: pubKeys[2], Power: 2}, + {PubKey: pubKeys[3], Power: 3}, + {PubKey: pubKeys[4], Power: 4}, + {PubKey: pubKeys[5], Power: 5}, + {PubKey: pubKeys[6], Power: 0}, + {PubKey: pubKeys[7], Power: 0}, + {PubKey: pubKeys[8], Power: 0}, + {PubKey: pubKeys[9], Power: 0}, } - actualValUpdates := providerKeeper.GetToBeOptedOutValidatorUpdates(ctx, chainID) + actualValUpdates := providerKeeper.ComputePartialSetValidatorUpdates(ctx, nextOptedIn, optedOut) - // sort before comparing sort.Slice(expectedValUpdates, func(i int, j int) bool { if expectedValUpdates[i].PubKey.Compare(expectedValUpdates[j].PubKey) < 0 { return true @@ -300,18 +331,24 @@ func TestGetToBeOptedOutValidatorUpdates(t *testing.T) { }) require.Equal(t, expectedValUpdates, actualValUpdates) + } -func createValidators(bondStatutes []stakingtypes.BondStatus, powers []int64) (validators []stakingtypes.Validator, - valUpdates []abci.ValidatorUpdate) { - if len(bondStatutes) != len(powers) { - return - } +func TestResetOptedInSet(t *testing.T) { + providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + chainID := "chainID" - for i := 0; i < len(bondStatutes); i++ { + // create 10 validators + var providerAddresses []types.ProviderConsAddress + var pubKeys []tmprotocrypto.PublicKey + for i := 0; i < 10; i++ { + // generate a consensus public key for the provider providerConsPubKey := ed25519.GenPrivKeyFromSecret([]byte{uint8(i)}).PubKey() consAddr := sdk.ConsAddress(providerConsPubKey.Address()) providerAddr := types.NewProviderConsAddress(consAddr) + providerAddresses = append(providerAddresses, providerAddr) var providerValidatorAddr sdk.ValAddress providerValidatorAddr = providerAddr.Address.Bytes() @@ -320,75 +357,70 @@ func createValidators(bondStatutes []stakingtypes.BondStatus, powers []int64) (v validator := stakingtypes.Validator{ OperatorAddress: providerValidatorAddr.String(), ConsensusPubkey: pkAny, - Status: bondStatutes[i], + Status: stakingtypes.Bonded, } - validators = append(validators, validator) - consumerTMPublicKey := tmprotocrypto.PublicKey{ + pubKey := tmprotocrypto.PublicKey{ Sum: &tmprotocrypto.PublicKey_Ed25519{ Ed25519: consAddr.Bytes(), }, } + pubKeys = append(pubKeys, pubKey) + + mocks.MockStakingKeeper.EXPECT(). + GetValidatorByConsAddr(ctx, consAddr).Return(validator, true).AnyTimes() - valUpdate := abci.ValidatorUpdate{PubKey: consumerTMPublicKey, Power: powers[i]} - valUpdates = append(valUpdates, valUpdate) + if i != 1 { + mocks.MockStakingKeeper.EXPECT(). + GetLastValidatorPower(ctx, providerValidatorAddr).Return(int64(i + 1)).AnyTimes() + } else { + mocks.MockStakingKeeper.EXPECT(). + GetLastValidatorPower(ctx, providerValidatorAddr).Return(int64(0)).AnyTimes() + } } - return -} -func TestComputePartialValidatorUpdateSet(t *testing.T) { - providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() + // first 6 validators to opt in + var toBeOptedIn []types.ProviderConsAddress - type TestValidator struct { - validator stakingtypes.Validator - power int64 - expectedValUpdate abci.ValidatorUpdate - } + providerKeeper.SetOptedIn(ctx, chainID, providerAddresses[0], 1, uint64(1)) - chainID := "chainID" + // this validator won't be added because it has a power of 0 .. .see above mock .. not here + providerKeeper.SetOptedIn(ctx, chainID, providerAddresses[1], 1, uint64(3)) - // create 10 validator updates - //var updates []abci.ValidatorUpdate - powers := []int64{1, 2, 3, 4, 5, 6} - vals, updates := createValidators( - []stakingtypes.BondStatus{stakingtypes.Bonded, stakingtypes.Unbonding, stakingtypes.Bonded, stakingtypes.Unbonded, - stakingtypes.Unbonded, stakingtypes.Bonded}, powers) - - addr0, _ := vals[0].GetConsAddr() - providerKeeper.SetOptedIn(ctx, chainID, types.NewProviderConsAddress(addr0), uint64(1)) - addr1, _ := vals[1].GetConsAddr() - providerKeeper.SetOptedIn(ctx, chainID, types.NewProviderConsAddress(addr1), uint64(2)) - addr2, _ := vals[2].GetConsAddr() - providerKeeper.SetOptedIn(ctx, chainID, types.NewProviderConsAddress(addr2), uint64(3)) - addr3, _ := vals[3].GetConsAddr() - providerKeeper.SetOptedIn(ctx, chainID, types.NewProviderConsAddress(addr3), uint64(4)) - - // set to be opted out - providerKeeper.SetToBeOptedOut(ctx, chainID, types.NewProviderConsAddress(addr2)) - providerKeeper.SetToBeOptedOut(ctx, chainID, types.NewProviderConsAddress(addr3)) - - // make all the validators at once ... kane to stakingUpdates based on the publickeys of those validators and then get the result - addr4, _ := vals[4].GetConsAddr() - providerKeeper.SetToBeOptedIn(ctx, chainID, types.NewProviderConsAddress(addr4)) - addr5, _ := vals[5].GetConsAddr() - providerKeeper.SetToBeOptedIn(ctx, chainID, types.NewProviderConsAddress(addr5)) - - for i, val := range vals { - consAddr, _ := val.GetConsAddr() - mocks.MockStakingKeeper.EXPECT(). - GetValidatorByConsAddr(ctx, consAddr).Return(val, true).AnyTimes() - mocks.MockStakingKeeper.EXPECT(). - GetLastValidatorPower(ctx, val.GetOperator()).Return(powers[i]).AnyTimes() + for i := 2; i < 6; i++ { + providerKeeper.SetToBeOptedIn(ctx, chainID, providerAddresses[i]) + toBeOptedIn = append(toBeOptedIn, providerAddresses[i]) } - actualValUpdates := providerKeeper.ComputePartialSetValUpdates(ctx, chainID, []abci.ValidatorUpdate{}) + var optedOut []types.ProviderConsAddress + for i := 6; i < 10; i++ { + providerKeeper.SetToBeOptedOut(ctx, chainID, providerAddresses[i]) + optedOut = append(optedOut, providerAddresses[i]) + } - fmt.Println(updates) - fmt.Println(actualValUpdates) - //require.Equal(t, expectedValUpdates, actualValUpdates) -} + require.NotEmpty(t, providerKeeper.GetToBeOptedIn(ctx, chainID)) + require.NotEmpty(t, providerKeeper.GetToBeOptedOut(ctx, chainID)) + + providerKeeper.ResetOptedInSet(ctx, chainID) -func TestResetPartialSet(t *testing.T) { - // s + require.Empty(t, providerKeeper.GetToBeOptedIn(ctx, chainID)) + require.Empty(t, providerKeeper.GetToBeOptedOut(ctx, chainID)) + + actualOptedInValidators := providerKeeper.GetOptedIn(ctx, chainID) + expectedOptedInValidators := []keeper.OptedInValidator{ + {ProviderAddr: providerAddresses[0], BlockHeight: 1, Power: 1}, + {ProviderAddr: providerAddresses[2], BlockHeight: uint64(ctx.BlockHeight()), Power: 3}, + {ProviderAddr: providerAddresses[3], BlockHeight: uint64(ctx.BlockHeight()), Power: 4}, + {ProviderAddr: providerAddresses[4], BlockHeight: uint64(ctx.BlockHeight()), Power: 5}, + {ProviderAddr: providerAddresses[5], BlockHeight: uint64(ctx.BlockHeight()), Power: 6}, + } + + sort.Slice(expectedOptedInValidators, func(i int, j int) bool { + return strings.Compare(expectedOptedInValidators[i].ProviderAddr.String(), expectedOptedInValidators[j].ProviderAddr.String()) < 0 + }) + + sort.Slice(actualOptedInValidators, func(i int, j int) bool { + return strings.Compare(actualOptedInValidators[i].ProviderAddr.String(), actualOptedInValidators[j].ProviderAddr.String()) < 0 + }) + require.Equal(t, expectedOptedInValidators, actualOptedInValidators) } diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go index c391d72ddd..9c129043bb 100644 --- a/x/ccv/provider/keeper/proposal.go +++ b/x/ccv/provider/keeper/proposal.go @@ -16,7 +16,6 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - abci "github.com/cometbft/cometbft/abci/types" tmtypes "github.com/cometbft/cometbft/types" "github.com/cosmos/interchain-security/v4/x/ccv/provider/types" @@ -256,7 +255,9 @@ func (k Keeper) MakeConsumerGenesis( }) // at this initial state, no validator is yet opted in ... but a validator is to be opted in - initialUpdates := k.ComputePartialSetValUpdates(ctx, chainID, []abci.ValidatorUpdate{}) + initialUpdates := k.ComputePartialSetValidatorUpdates(ctx, + k.GetNextOptedInValidators(ctx, chainID), + k.GetToBeOptedOut(ctx, chainID)) // //initialUpdates := []abci.ValidatorUpdate{} @@ -286,7 +287,7 @@ func (k Keeper) MakeConsumerGenesis( initialUpdatesWithConsumerKeys := k.MustApplyKeyAssignmentToValUpdates(ctx, chainID, initialUpdates, func(address types.ProviderConsAddress) bool { return k.IsToBeOptedIn(ctx, chainID, address) }) - k.ResetPartialSet(ctx, chainID) + k.ResetOptedInSet(ctx, chainID) // Get a hash of the consumer validator set from the update with applied consumer assigned keys updatesAsValSet, err := tmtypes.PB2TM.ValidatorUpdates(initialUpdatesWithConsumerKeys) diff --git a/x/ccv/provider/keeper/relay.go b/x/ccv/provider/keeper/relay.go index f090d367fe..32e6d3d312 100644 --- a/x/ccv/provider/keeper/relay.go +++ b/x/ccv/provider/keeper/relay.go @@ -2,6 +2,7 @@ package keeper import ( "fmt" + abci "github.com/cometbft/cometbft/abci/types" "strconv" clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" @@ -212,22 +213,32 @@ func (k Keeper) SendVSCPacketsToChain(ctx sdk.Context, chainID, channelID string // QueueVSCPackets queues latest validator updates for every registered consumer chain func (k Keeper) QueueVSCPackets(ctx sdk.Context) { valUpdateID := k.GetValidatorSetUpdateId(ctx) // current valset update ID - // Get the validator updates from the staking module. - // Note: GetValidatorUpdates panics if the updates provided by the x/staking module - // of cosmos-sdk is invalid. - valUpdates := k.stakingKeeper.GetValidatorUpdates(ctx) for _, chain := range k.GetAllConsumerChains(ctx) { - partialSetUpdates := k.ComputePartialSetValUpdates(ctx, chain.ChainId, valUpdates) + var valUpdates []abci.ValidatorUpdate + if k.IsTopN(ctx, chain.ChainId) { + valUpdates = k.ComputePartialSetValidatorUpdates(ctx, + k.GetNextOptedInValidators(ctx, chain.ChainId), + k.GetToBeOptedOut(ctx, chain.ChainId)) + } else { + valUpdates = k.stakingKeeper.GetValidatorUpdates(ctx) + } - // FIXME: staking updtes contain ... ARE only for active set ... - // Apply the key assignment to the validator updates. - valUpdates := k.MustApplyKeyAssignmentToValUpdates(ctx, chain.ChainId, partialSetUpdates, - func(address providertypes.ProviderConsAddress) bool { + var considerKeyReplacement func(address providertypes.ProviderConsAddress) bool + if k.IsTopN(ctx, chain.ChainId) { + considerKeyReplacement = func(address providertypes.ProviderConsAddress) bool { return k.IsOptedIn(ctx, chain.ChainId, address) - }) + } + } else { + considerKeyReplacement = func(address providertypes.ProviderConsAddress) bool { + return true + } + } + + // Apply the key assignment to the validator updates. + valUpdates = k.MustApplyKeyAssignmentToValUpdates(ctx, chain.ChainId, valUpdates, considerKeyReplacement) - k.ResetPartialSet(ctx, chain.ChainId) + k.ResetOptedInSet(ctx, chain.ChainId) // check whether there are changes in the validator set; // note that this also entails unbonding operations diff --git a/x/ccv/provider/types/errors.go b/x/ccv/provider/types/errors.go index b1d55a0ea1..6c19a7b396 100644 --- a/x/ccv/provider/types/errors.go +++ b/x/ccv/provider/types/errors.go @@ -24,5 +24,4 @@ var ( ErrInvalidConsumerClient = errorsmod.Register(ModuleName, 16, "ccv channel is not built on correct client") ErrDuplicateConsumerChain = errorsmod.Register(ModuleName, 17, "consumer chain already exists") ErrConsumerChainNotFound = errorsmod.Register(ModuleName, 18, "consumer chain not found") - ErrValidatorNotBonded = errorsmod.Register(ModuleName, 19, "validator not bonded") )