From 4674e772339e482a7daa21907eb0220f34ec8d3f Mon Sep 17 00:00:00 2001 From: mpoke Date: Tue, 3 Sep 2024 11:22:38 +0200 Subject: [PATCH 1/4] update allowlist and denylist power shaping update --- x/ccv/provider/keeper/msg_server.go | 5 --- x/ccv/provider/keeper/permissionless.go | 44 +++++++++++++++++++++---- x/ccv/types/errors.go | 2 ++ 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/x/ccv/provider/keeper/msg_server.go b/x/ccv/provider/keeper/msg_server.go index 1764796c57..83664e369e 100644 --- a/x/ccv/provider/keeper/msg_server.go +++ b/x/ccv/provider/keeper/msg_server.go @@ -319,8 +319,6 @@ func (k msgServer) CreateConsumer(goCtx context.Context, msg *types.MsgCreateCon return &resp, errorsmod.Wrap(types.ErrCannotCreateTopNChain, "cannot create a Top N chain using the `MsgCreateConsumer` message; use `MsgUpdateConsumer` instead") } - - // TODO (PERMISSIONLESS) UpdateAllowlist & UpdateDenylist } if err := k.Keeper.SetConsumerPowerShapingParameters(ctx, consumerId, powerShapingParameters); err != nil { return &resp, errorsmod.Wrapf(types.ErrInvalidPowerShapingParameters, @@ -418,9 +416,6 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon return &resp, errorsmod.Wrapf(types.ErrInvalidPowerShapingParameters, "cannot set power shaping parameters") } - - k.Keeper.UpdateAllowlist(ctx, consumerId, msg.PowerShapingParameters.Allowlist) - k.Keeper.UpdateDenylist(ctx, consumerId, msg.PowerShapingParameters.Denylist) err = k.Keeper.UpdateMinimumPowerInTopN(ctx, consumerId, oldTopN, msg.PowerShapingParameters.Top_N) if err != nil { return &resp, errorsmod.Wrapf(types.ErrCannotUpdateMinimumPowerInTopN, diff --git a/x/ccv/provider/keeper/permissionless.go b/x/ccv/provider/keeper/permissionless.go index 695db64463..8d2335e6ac 100644 --- a/x/ccv/provider/keeper/permissionless.go +++ b/x/ccv/provider/keeper/permissionless.go @@ -2,11 +2,14 @@ package keeper import ( "encoding/binary" + "errors" "fmt" "strconv" + errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" + ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" ) // setConsumerId sets the provided consumerId @@ -148,30 +151,57 @@ func (k Keeper) GetConsumerPowerShapingParameters(ctx sdk.Context, consumerId st store := ctx.KVStore(k.storeKey) bz := store.Get(types.ConsumerIdToPowerShapingParametersKey(consumerId)) if bz == nil { - return types.PowerShapingParameters{}, fmt.Errorf("failed to retrieve power-shaping parameters for consumer id (%s)", consumerId) + return types.PowerShapingParameters{}, errorsmod.Wrapf(ccvtypes.ErrStoreKeyNotFound, + "GetConsumerPowerShapingParameters, consumerId(%s)", consumerId) } var record types.PowerShapingParameters if err := record.Unmarshal(bz); err != nil { - return types.PowerShapingParameters{}, fmt.Errorf("failed to unmarshal power-shaping parameters for consumer id (%s): %w", consumerId, err) + return types.PowerShapingParameters{}, errorsmod.Wrapf(ccvtypes.ErrStoreUnmarshal, + "GetConsumerPowerShapingParameters, consumerId(%s): %s", consumerId, err.Error()) } return record, nil } -// SetConsumerPowerShapingParameters sets the power-shaping parameters associated with this consumer id +// SetConsumerPowerShapingParameters sets the power-shaping parameters associated with this consumer id. +// Note that it also updates the allowlist and denylist indexes if they are different func (k Keeper) SetConsumerPowerShapingParameters(ctx sdk.Context, consumerId string, parameters types.PowerShapingParameters) error { store := ctx.KVStore(k.storeKey) bz, err := parameters.Marshal() if err != nil { return fmt.Errorf("failed to marshal power-shaping parameters (%+v) for consumer id (%s): %w", parameters, consumerId, err) } + + // get old power shaping params + oldParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId) + // ignore ErrStoreKeyNotFound errors as this might be the first time the power shaping params are set + if errors.Is(err, ccvtypes.ErrStoreUnmarshal) { + return fmt.Errorf("cannot get consumer previous power shaping parameters: %w", err) + } + store.Set(types.ConsumerIdToPowerShapingParametersKey(consumerId), bz) + + // update allowlist and denylist indexes if needed + if !equalStringSlices(oldParameters.Allowlist, parameters.Allowlist) { + k.UpdateAllowlist(ctx, consumerId, parameters.Allowlist) + } + if !equalStringSlices(oldParameters.Denylist, parameters.Denylist) { + k.UpdateDenylist(ctx, consumerId, parameters.Denylist) + } + return nil } -// DeleteConsumerPowerShapingParameters deletes the power-shaping parameters associated with this consumer id -func (k Keeper) DeleteConsumerPowerShapingParameters(ctx sdk.Context, consumerId string) { - store := ctx.KVStore(k.storeKey) - store.Delete(types.ConsumerIdToPowerShapingParametersKey(consumerId)) +// equalStringSlices returns true if two string slices are equal +func equalStringSlices(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i, v := range a { + if v != b[i] { + return false + } + } + return true } // GetConsumerPhase returns the phase associated with this consumer id diff --git a/x/ccv/types/errors.go b/x/ccv/types/errors.go index e492984e48..995d9905e8 100644 --- a/x/ccv/types/errors.go +++ b/x/ccv/types/errors.go @@ -22,4 +22,6 @@ var ( ErrDuplicateConsumerChain = errorsmod.Register(ModuleName, 14, "consumer chain already exists") ErrConsumerChainNotFound = errorsmod.Register(ModuleName, 15, "consumer chain not found") ErrInvalidDoubleVotingEvidence = errorsmod.Register(ModuleName, 16, "invalid consumer double voting evidence") + ErrStoreKeyNotFound = errorsmod.Register(ModuleName, 17, "store key not found") + ErrStoreUnmarshal = errorsmod.Register(ModuleName, 18, "cannot unmarshal value from store") ) From c226518e51b9c1d7a2e83a562b4ef643cbc5752f Mon Sep 17 00:00:00 2001 From: mpoke Date: Tue, 3 Sep 2024 11:22:50 +0200 Subject: [PATCH 2/4] fix tests --- .../integration/partial_set_security_test.go | 14 ++-- tests/mbt/driver/setup.go | 3 +- testutil/keeper/unit_test_helpers.go | 11 ++-- .../keeper/consumer_lifecycle_test.go | 20 ++++-- x/ccv/provider/keeper/grpc_query_test.go | 15 +++-- x/ccv/provider/keeper/keeper_test.go | 14 ++-- .../keeper/partial_set_security_test.go | 27 +++++--- x/ccv/provider/keeper/permissionless_test.go | 65 ++++++++++++++----- x/ccv/provider/keeper/relay_test.go | 6 +- 9 files changed, 123 insertions(+), 52 deletions(-) diff --git a/tests/integration/partial_set_security_test.go b/tests/integration/partial_set_security_test.go index e99ce6f44a..db628aaccd 100644 --- a/tests/integration/partial_set_security_test.go +++ b/tests/integration/partial_set_security_test.go @@ -1,11 +1,12 @@ package integration import ( - "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" "slices" "sort" "testing" + "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" + "cosmossdk.io/math" ccv "github.com/cosmos/interchain-security/v5/x/ccv/types" "github.com/stretchr/testify/require" @@ -152,9 +153,14 @@ func TestMinStake(t *testing.T) { // adjust parameters // set the minStake according to the test case - providerKeeper.SetConsumerPowerShapingParameters(s.providerChain.GetContext(), s.getFirstBundle().ConsumerId, types.PowerShapingParameters{ - MinStake: tc.minStake, - }) + err = providerKeeper.SetConsumerPowerShapingParameters( + s.providerChain.GetContext(), + s.getFirstBundle().ConsumerId, + types.PowerShapingParameters{ + MinStake: tc.minStake, + }, + ) + s.Require().NoError(err) // delegate and undelegate to trigger a vscupdate diff --git a/tests/mbt/driver/setup.go b/tests/mbt/driver/setup.go index adaa444167..97102960d6 100644 --- a/tests/mbt/driver/setup.go +++ b/tests/mbt/driver/setup.go @@ -389,9 +389,10 @@ func (s *Driver) ConfigureNewPath(consumerChain, providerChain *ibctesting.TestC require.NoError(s.t, err, "Error setting consumer genesis on provider for chain %v", consumerChain.ChainID) // set the top N percentage to 100 to simulate a full consumer - s.providerKeeper().SetConsumerPowerShapingParameters(providerChain.GetContext(), consumerChain.ChainID, types.PowerShapingParameters{ + err = s.providerKeeper().SetConsumerPowerShapingParameters(providerChain.GetContext(), consumerChain.ChainID, types.PowerShapingParameters{ Top_N: 100, }) + require.NoError(s.t, err, "Error setting consumer top N for chain %v", consumerChain.ChainID) // Client ID is set in InitGenesis and we treat it as a black box. So // must query it to use it with the endpoint. diff --git a/testutil/keeper/unit_test_helpers.go b/testutil/keeper/unit_test_helpers.go index 6249227e00..1572937607 100644 --- a/testutil/keeper/unit_test_helpers.go +++ b/testutil/keeper/unit_test_helpers.go @@ -232,12 +232,15 @@ func SetupForStoppingConsumerChain(t *testing.T, ctx sdk.Context, gomock.InOrder(expectations...) providerKeeper.SetConsumerChainId(ctx, consumerId, "chainID") - providerKeeper.SetConsumerMetadata(ctx, consumerId, GetTestConsumerMetadata()) - providerKeeper.SetConsumerInitializationParameters(ctx, consumerId, GetTestInitializationParameters()) - providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, GetTestPowerShapingParameters()) + err := providerKeeper.SetConsumerMetadata(ctx, consumerId, GetTestConsumerMetadata()) + require.NoError(t, err) + err = providerKeeper.SetConsumerInitializationParameters(ctx, consumerId, GetTestInitializationParameters()) + require.NoError(t, err) + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, GetTestPowerShapingParameters()) + require.NoError(t, err) providerKeeper.SetConsumerPhase(ctx, consumerId, providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) - err := providerKeeper.CreateConsumerClient(ctx, consumerId) + err = providerKeeper.CreateConsumerClient(ctx, consumerId) require.NoError(t, err) // set the mapping consumer ID <> client ID for the consumer chain providerKeeper.SetConsumerClientId(ctx, consumerId, "clientID") diff --git a/x/ccv/provider/keeper/consumer_lifecycle_test.go b/x/ccv/provider/keeper/consumer_lifecycle_test.go index cf5aaa873e..7fc238570c 100644 --- a/x/ccv/provider/keeper/consumer_lifecycle_test.go +++ b/x/ccv/provider/keeper/consumer_lifecycle_test.go @@ -398,11 +398,14 @@ func TestCreateConsumerClient(t *testing.T) { // Call method with same arbitrary values as defined above in mock expectations. providerKeeper.SetConsumerChainId(ctx, "0", "chainID") - providerKeeper.SetConsumerMetadata(ctx, "0", testkeeper.GetTestConsumerMetadata()) - providerKeeper.SetConsumerInitializationParameters(ctx, "0", testkeeper.GetTestInitializationParameters()) - providerKeeper.SetConsumerPowerShapingParameters(ctx, "0", testkeeper.GetTestPowerShapingParameters()) - err := providerKeeper.CreateConsumerClient(ctx, "0") + err := providerKeeper.SetConsumerMetadata(ctx, "0", testkeeper.GetTestConsumerMetadata()) + require.NoError(t, err) + err = providerKeeper.SetConsumerInitializationParameters(ctx, "0", testkeeper.GetTestInitializationParameters()) + require.NoError(t, err) + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, "0", testkeeper.GetTestPowerShapingParameters()) + require.NoError(t, err) + err = providerKeeper.CreateConsumerClient(ctx, "0") if tc.expClientCreated { require.NoError(t, err) testCreatedConsumerClient(t, ctx, providerKeeper, "0", "clientID") @@ -526,9 +529,12 @@ func TestMakeConsumerGenesis(t *testing.T) { UnbondingPeriod: unbondingPeriod, } providerKeeper.SetConsumerChainId(ctx, "0", "testchain1") - providerKeeper.SetConsumerMetadata(ctx, "0", consumerMetadata) - providerKeeper.SetConsumerInitializationParameters(ctx, "0", initializationParameters) - providerKeeper.SetConsumerPowerShapingParameters(ctx, "0", providertypes.PowerShapingParameters{}) + err := providerKeeper.SetConsumerMetadata(ctx, "0", consumerMetadata) + require.NoError(t, err) + err = providerKeeper.SetConsumerInitializationParameters(ctx, "0", initializationParameters) + require.NoError(t, err) + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, "0", providertypes.PowerShapingParameters{}) + require.NoError(t, err) actualGenesis, _, err := providerKeeper.MakeConsumerGenesis(ctx, "0") require.NoError(t, err) diff --git a/x/ccv/provider/keeper/grpc_query_test.go b/x/ccv/provider/keeper/grpc_query_test.go index c540c4980c..9d91bd25f7 100644 --- a/x/ccv/provider/keeper/grpc_query_test.go +++ b/x/ccv/provider/keeper/grpc_query_test.go @@ -184,7 +184,8 @@ func TestQueryConsumerValidators(t *testing.T) { require.Equal(t, res.Validators[0].ProviderAddress, providerAddr1.String()) // update consumer TopN param - pk.SetConsumerPowerShapingParameters(ctx, consumerId, types.PowerShapingParameters{Top_N: 50}) + err = pk.SetConsumerPowerShapingParameters(ctx, consumerId, types.PowerShapingParameters{Top_N: 50}) + require.NoError(t, err) // expect both opted-in and topN validator expRes := types.QueryConsumerValidatorsResponse{ @@ -445,13 +446,14 @@ func TestGetConsumerChain(t *testing.T) { clientID := fmt.Sprintf("client-%d", len(consumerID)-i) topN := topNs[i] pk.SetConsumerClientId(ctx, consumerID, clientID) - pk.SetConsumerPowerShapingParameters(ctx, consumerID, types.PowerShapingParameters{ + err := pk.SetConsumerPowerShapingParameters(ctx, consumerID, types.PowerShapingParameters{ Top_N: topN, ValidatorSetCap: validatorSetCaps[i], ValidatorsPowerCap: validatorPowerCaps[i], MinStake: minStakes[i].Uint64(), AllowInactiveVals: allowInactiveVals[i], }) + require.NoError(t, err) pk.SetMinimumPowerInTopN(ctx, consumerID, expectedMinPowerInTopNs[i]) for _, addr := range allowlists[i] { pk.SetAllowlist(ctx, consumerID, addr) @@ -532,7 +534,8 @@ func TestQueryConsumerChain(t *testing.T) { _, err = providerKeeper.QueryConsumerChain(ctx, &req) require.Error(t, err) - providerKeeper.SetConsumerMetadata(ctx, consumerId, types.ConsumerMetadata{Name: chainId}) + err = providerKeeper.SetConsumerMetadata(ctx, consumerId, types.ConsumerMetadata{Name: chainId}) + require.NoError(t, err) expRes := types.QueryConsumerChainResponse{ ChainId: chainId, @@ -548,17 +551,19 @@ func TestQueryConsumerChain(t *testing.T) { require.NoError(t, err) require.Equal(t, &expRes, res) - providerKeeper.SetConsumerInitializationParameters( + err = providerKeeper.SetConsumerInitializationParameters( ctx, consumerId, types.ConsumerInitializationParameters{SpawnTime: ctx.BlockTime()}, ) + require.NoError(t, err) - providerKeeper.SetConsumerPowerShapingParameters( + err = providerKeeper.SetConsumerPowerShapingParameters( ctx, consumerId, types.PowerShapingParameters{Top_N: uint32(50)}, ) + require.NoError(t, err) expRes.InitParams = &types.ConsumerInitializationParameters{SpawnTime: ctx.BlockTime()} expRes.PowerShapingParams = &types.PowerShapingParameters{Top_N: uint32(50)} diff --git a/x/ccv/provider/keeper/keeper_test.go b/x/ccv/provider/keeper/keeper_test.go index 5431c95390..c19502c0da 100644 --- a/x/ccv/provider/keeper/keeper_test.go +++ b/x/ccv/provider/keeper/keeper_test.go @@ -583,11 +583,12 @@ func TestUpdateMinimumPowerInTopN(t *testing.T) { consumerId := "0" // test case where Top N is 0 in which case there's no minimum power in top N - providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ + err := providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ Top_N: 0, }) + require.NoError(t, err) - err := providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 0, 0) + err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 0, 0) require.NoError(t, err) _, found := providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) require.False(t, found) @@ -607,9 +608,10 @@ func TestUpdateMinimumPowerInTopN(t *testing.T) { providerKeeper.SetParams(ctx, params) // when top N is 50, the minimum power is 30 (because top validator has to validate) - providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ Top_N: 50, }) + require.NoError(t, err) err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 0, 50) require.NoError(t, err) minimumPowerInTopN, found := providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) @@ -617,9 +619,10 @@ func TestUpdateMinimumPowerInTopN(t *testing.T) { require.Equal(t, int64(30), minimumPowerInTopN) // when top N is 51, the minimum power is 20 (because top 2 validators have to validate) - providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ Top_N: 51, }) + require.NoError(t, err) err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 50, 51) require.NoError(t, err) minimumPowerInTopN, found = providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) @@ -627,9 +630,10 @@ func TestUpdateMinimumPowerInTopN(t *testing.T) { require.Equal(t, int64(20), minimumPowerInTopN) // when top N is 100, the minimum power is 10 (that of the validator with the lowest power) - providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ Top_N: 100, }) + require.NoError(t, err) err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 51, 100) require.NoError(t, err) minimumPowerInTopN, found = providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) diff --git a/x/ccv/provider/keeper/partial_set_security_test.go b/x/ccv/provider/keeper/partial_set_security_test.go index f760599907..b29ff94a47 100644 --- a/x/ccv/provider/keeper/partial_set_security_test.go +++ b/x/ccv/provider/keeper/partial_set_security_test.go @@ -409,7 +409,8 @@ func TestCanValidateChain(t *testing.T) { // with TopN chains, the validator can be considered, mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(gomock.Any(), providerAddr.Address).Return(validator, nil).Times(2) - providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerID, types.PowerShapingParameters{Top_N: 50}) + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerID, types.PowerShapingParameters{Top_N: 50}) + require.NoError(t, err) powerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerID) require.NoError(t, err) // validator's power is LT the min power @@ -422,7 +423,8 @@ func TestCanValidateChain(t *testing.T) { require.True(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, powerShapingParameters.Top_N, 2)) // With OptIn chains, validator can validate only if it has already opted-in - providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerID, types.PowerShapingParameters{Top_N: 0}) + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerID, types.PowerShapingParameters{Top_N: 0}) + require.NoError(t, err) powerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerID) require.NoError(t, err) require.True(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, powerShapingParameters.Top_N, 2)) @@ -471,41 +473,46 @@ func TestCapValidatorSet(t *testing.T) { consumerValidators := providerKeeper.CapValidatorSet(ctx, powerShapingParameters, validators) require.Equal(t, validators, consumerValidators) - providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", types.PowerShapingParameters{ + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", types.PowerShapingParameters{ ValidatorSetCap: 0, }) + require.NoError(t, err) powerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, "consumerId") require.NoError(t, err) consumerValidators = providerKeeper.CapValidatorSet(ctx, powerShapingParameters, validators) require.Equal(t, validators, consumerValidators) - providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", types.PowerShapingParameters{ + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", types.PowerShapingParameters{ ValidatorSetCap: 100, }) + require.NoError(t, err) powerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, "consumerId") require.NoError(t, err) consumerValidators = providerKeeper.CapValidatorSet(ctx, powerShapingParameters, validators) require.Equal(t, validators, consumerValidators) - providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", types.PowerShapingParameters{ + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", types.PowerShapingParameters{ ValidatorSetCap: 1, }) + require.NoError(t, err) powerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, "consumerId") require.NoError(t, err) consumerValidators = providerKeeper.CapValidatorSet(ctx, powerShapingParameters, validators) require.Equal(t, []types.ConsensusValidator{validatorC}, consumerValidators) - providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", types.PowerShapingParameters{ + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", types.PowerShapingParameters{ ValidatorSetCap: 2, }) + require.NoError(t, err) powerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, "consumerId") require.NoError(t, err) consumerValidators = providerKeeper.CapValidatorSet(ctx, powerShapingParameters, validators) require.Equal(t, []types.ConsensusValidator{validatorC, validatorB}, consumerValidators) - providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", types.PowerShapingParameters{ + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", types.PowerShapingParameters{ ValidatorSetCap: 3, }) + require.NoError(t, err) powerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, "consumerId") require.NoError(t, err) consumerValidators = providerKeeper.CapValidatorSet(ctx, powerShapingParameters, validators) @@ -563,9 +570,10 @@ func TestCapValidatorsPower(t *testing.T) { sortValidators(cappedValidators) require.Equal(t, validators, cappedValidators) - providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", types.PowerShapingParameters{ + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", types.PowerShapingParameters{ ValidatorsPowerCap: 33, }) + require.NoError(t, err) powerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, "consumerId") require.NoError(t, err) cappedValidators = providerKeeper.CapValidatorsPower(ctx, powerShapingParameters.ValidatorsPowerCap, validators) @@ -858,10 +866,11 @@ func TestIfInactiveValsDisallowedProperty(t *testing.T) { // Set up the parameters in the provider keeper // do not allow inactive validators - providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", types.PowerShapingParameters{ + err := providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", types.PowerShapingParameters{ MinStake: minStake, AllowInactiveVals: false, }) + require.NoError(t, err) params := providerKeeper.GetParams(ctx) params.MaxProviderConsensusValidators = int64(maxProviderConsensusVals) providerKeeper.SetParams(ctx, params) diff --git a/x/ccv/provider/keeper/permissionless_test.go b/x/ccv/provider/keeper/permissionless_test.go index 20a929f595..b41700fd38 100644 --- a/x/ccv/provider/keeper/permissionless_test.go +++ b/x/ccv/provider/keeper/permissionless_test.go @@ -1,12 +1,17 @@ package keeper_test import ( + "bytes" + "errors" + "sort" "testing" "time" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" testkeeper "github.com/cosmos/interchain-security/v5/testutil/keeper" providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" + ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" "github.com/stretchr/testify/require" ) @@ -179,47 +184,77 @@ func TestConsumerInitializationParameters(t *testing.T) { require.Equal(t, providertypes.ConsumerInitializationParameters{}, actualInitializationParameters) } -// TestConsumerPowerShapingParameters tests the getter, setter, and deletion of the consumer id to power-shaping parameters methods +// TestConsumerPowerShapingParameters tests the getter and setter of the consumer id to power-shaping parameters methods func TestConsumerPowerShapingParameters(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() - _, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, "consumerId") + consumerId := "consumerId" + consAddrs := []string{ + "cosmosvalcons1kswr5sq599365kcjmhgufevfps9njf43e4lwdk", + "cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6", + "cosmosvalcons1muys5jyqk4xd27e208nym85kn0t4zjcfeu63fe", + "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39", + "cosmosvalcons1qmq08eruchr5sf5s3rwz7djpr5a25f7xw4mceq", + "cosmosvalcons1uuec3cjxajv5te08p220usrjhkfhg9wyvqn0tm", + } + providerConsAddr := []providertypes.ProviderConsAddress{} + for _, addr := range consAddrs { + ca, _ := sdk.ConsAddressFromBech32(addr) + providerConsAddr = append(providerConsAddr, providertypes.NewProviderConsAddress(ca)) + } + sortProviderConsAddr := func(consAddrs []providertypes.ProviderConsAddress) { + sort.Slice(consAddrs, func(i, j int) bool { + return bytes.Compare(consAddrs[i].Address, consAddrs[j].Address) < 0 + }) + } + + _, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerId) require.Error(t, err) + require.True(t, errors.Is(err, ccvtypes.ErrStoreKeyNotFound)) expectedPowerShapingParameters := providertypes.PowerShapingParameters{ Top_N: 10, ValidatorsPowerCap: 34, ValidatorSetCap: 10, - Allowlist: []string{"allowlist1", "allowlist2"}, - Denylist: []string{"denylist1", "denylist2"}, + Allowlist: []string{consAddrs[0], consAddrs[1]}, + Denylist: []string{consAddrs[2], consAddrs[3]}, MinStake: 234, AllowInactiveVals: true, } - providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", expectedPowerShapingParameters) - actualPowerShapingParameters, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, "consumerId") + expectedAllowlist := []providertypes.ProviderConsAddress{providerConsAddr[0], providerConsAddr[1]} + sortProviderConsAddr(expectedAllowlist) + expectedDenylist := []providertypes.ProviderConsAddress{providerConsAddr[2], providerConsAddr[3]} + sortProviderConsAddr(expectedDenylist) + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, expectedPowerShapingParameters) + require.NoError(t, err) + actualPowerShapingParameters, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerId) require.NoError(t, err) require.Equal(t, expectedPowerShapingParameters, actualPowerShapingParameters) + require.Equal(t, expectedAllowlist, providerKeeper.GetAllowList(ctx, consumerId)) + require.Equal(t, expectedDenylist, providerKeeper.GetDenyList(ctx, consumerId)) // assert that overwriting the current initialization record works expectedPowerShapingParameters = providertypes.PowerShapingParameters{ Top_N: 12, ValidatorsPowerCap: 67, ValidatorSetCap: 20, - Allowlist: []string{"allowlist3", "allowlist4"}, - Denylist: []string{"denylist3", "denylist4"}, + Allowlist: []string{consAddrs[4], consAddrs[5]}, + Denylist: []string{consAddrs[2], consAddrs[3]}, MinStake: 567, AllowInactiveVals: false, } - providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", expectedPowerShapingParameters) - actualPowerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, "consumerId") + expectedAllowlist = []providertypes.ProviderConsAddress{providerConsAddr[4], providerConsAddr[5]} + sortProviderConsAddr(expectedAllowlist) + expectedDenylist = []providertypes.ProviderConsAddress{providerConsAddr[2], providerConsAddr[3]} + sortProviderConsAddr(expectedDenylist) + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, expectedPowerShapingParameters) + require.NoError(t, err) + actualPowerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerId) require.NoError(t, err) require.Equal(t, expectedPowerShapingParameters, actualPowerShapingParameters) - - providerKeeper.DeleteConsumerPowerShapingParameters(ctx, "consumerId") - actualPowerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, "consumerId") - require.Error(t, err) - require.Equal(t, providertypes.PowerShapingParameters{}, actualPowerShapingParameters) + require.Equal(t, expectedAllowlist, providerKeeper.GetAllowList(ctx, consumerId)) + require.Equal(t, expectedDenylist, providerKeeper.GetDenyList(ctx, consumerId)) } // TestConsumerPhase tests the getter, setter, and deletion of the consumer id to phase methods diff --git a/x/ccv/provider/keeper/relay_test.go b/x/ccv/provider/keeper/relay_test.go index acdceb5ea2..e45fedf35a 100644 --- a/x/ccv/provider/keeper/relay_test.go +++ b/x/ccv/provider/keeper/relay_test.go @@ -588,9 +588,10 @@ func TestEndBlockVSU(t *testing.T) { chainID := "consumerId" - providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", providertypes.PowerShapingParameters{ + err := providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", providertypes.PowerShapingParameters{ Top_N: 100, }) + require.NoError(t, err) // 10 blocks constitute an epoch params := providertypes.DefaultParams() @@ -741,10 +742,11 @@ func TestQueueVSCPacketsWithPowerCapping(t *testing.T) { // add a consumer chain providerKeeper.SetConsumerClientId(ctx, "consumerId", "clientID") - providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", providertypes.PowerShapingParameters{ + err := providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", providertypes.PowerShapingParameters{ Top_N: 50, // would opt in E ValidatorsPowerCap: 40, // set a power-capping of 40% }) + require.NoError(t, err) // opt in all validators providerKeeper.SetOptedIn(ctx, "consumerId", providertypes.NewProviderConsAddress(valAConsAddr)) From 0777f8728fd1e85c78eb61895fe2dca604864ccb Mon Sep 17 00:00:00 2001 From: mpoke Date: Tue, 3 Sep 2024 11:33:24 +0200 Subject: [PATCH 3/4] refactor: move power shaping params to power_shaping.go --- x/ccv/provider/keeper/keeper.go | 215 ------------- x/ccv/provider/keeper/keeper_test.go | 227 -------------- x/ccv/provider/keeper/permissionless.go | 61 ---- x/ccv/provider/keeper/permissionless_test.go | 78 ----- x/ccv/provider/keeper/power_shaping.go | 286 +++++++++++++++++ x/ccv/provider/keeper/power_shaping_test.go | 313 +++++++++++++++++++ 6 files changed, 599 insertions(+), 581 deletions(-) create mode 100644 x/ccv/provider/keeper/power_shaping.go create mode 100644 x/ccv/provider/keeper/power_shaping_test.go diff --git a/x/ccv/provider/keeper/keeper.go b/x/ccv/provider/keeper/keeper.go index 0fc911c8dc..b4c037c0c3 100644 --- a/x/ccv/provider/keeper/keeper.go +++ b/x/ccv/provider/keeper/keeper.go @@ -867,221 +867,6 @@ func (k Keeper) DeleteConsumerCommissionRate( store.Delete(types.ConsumerCommissionRateKey(consumerId, providerAddr)) } -// SetAllowlist allowlists validator with `providerAddr` address on chain `consumerId` -func (k Keeper) SetAllowlist( - ctx sdk.Context, - consumerId string, - providerAddr types.ProviderConsAddress, -) { - store := ctx.KVStore(k.storeKey) - store.Set(types.AllowlistKey(consumerId, providerAddr), []byte{}) -} - -// GetAllowList returns all allowlisted validators -func (k Keeper) GetAllowList( - ctx sdk.Context, - consumerId string, -) (providerConsAddresses []types.ProviderConsAddress) { - store := ctx.KVStore(k.storeKey) - key := types.StringIdWithLenKey(types.AllowlistKeyPrefix(), consumerId) - iterator := storetypes.KVStorePrefixIterator(store, key) - defer iterator.Close() - - for ; iterator.Valid(); iterator.Next() { - providerConsAddresses = append(providerConsAddresses, types.NewProviderConsAddress(iterator.Key()[len(key):])) - } - - return providerConsAddresses -} - -// IsAllowlisted returns `true` if validator with `providerAddr` has been allowlisted on chain `consumerId` -func (k Keeper) IsAllowlisted( - ctx sdk.Context, - consumerId string, - providerAddr types.ProviderConsAddress, -) bool { - store := ctx.KVStore(k.storeKey) - bz := store.Get(types.AllowlistKey(consumerId, providerAddr)) - return bz != nil -} - -// DeleteAllowlist deletes all allowlisted validators -func (k Keeper) DeleteAllowlist(ctx sdk.Context, consumerId string) { - store := ctx.KVStore(k.storeKey) - iterator := storetypes.KVStorePrefixIterator(store, types.StringIdWithLenKey(types.AllowlistKeyPrefix(), consumerId)) - defer iterator.Close() - - keysToDel := [][]byte{} - for ; iterator.Valid(); iterator.Next() { - keysToDel = append(keysToDel, iterator.Key()) - } - - for _, key := range keysToDel { - store.Delete(key) - } -} - -// IsAllowlistEmpty returns `true` if no validator is allowlisted on chain `consumerId` -func (k Keeper) IsAllowlistEmpty(ctx sdk.Context, consumerId string) bool { - store := ctx.KVStore(k.storeKey) - iterator := storetypes.KVStorePrefixIterator(store, types.StringIdWithLenKey(types.AllowlistKeyPrefix(), consumerId)) - defer iterator.Close() - - return !iterator.Valid() -} - -// UpdateAllowlist populates the allowlist store for the consumer chain with this consumer id -func (k Keeper) UpdateAllowlist(ctx sdk.Context, consumerId string, allowlist []string) { - k.DeleteAllowlist(ctx, consumerId) - for _, address := range allowlist { - consAddr, err := sdk.ConsAddressFromBech32(address) - if err != nil { - continue - } - - k.SetAllowlist(ctx, consumerId, types.NewProviderConsAddress(consAddr)) - } -} - -// SetDenylist denylists validator with `providerAddr` address on chain `consumerId` -func (k Keeper) SetDenylist( - ctx sdk.Context, - consumerId string, - providerAddr types.ProviderConsAddress, -) { - store := ctx.KVStore(k.storeKey) - store.Set(types.DenylistKey(consumerId, providerAddr), []byte{}) -} - -// GetDenyList returns all denylisted validators -func (k Keeper) GetDenyList( - ctx sdk.Context, - consumerId string, -) (providerConsAddresses []types.ProviderConsAddress) { - store := ctx.KVStore(k.storeKey) - key := types.StringIdWithLenKey(types.DenylistKeyPrefix(), consumerId) - iterator := storetypes.KVStorePrefixIterator(store, key) - defer iterator.Close() - - for ; iterator.Valid(); iterator.Next() { - providerConsAddresses = append(providerConsAddresses, types.NewProviderConsAddress(iterator.Key()[len(key):])) - } - - return providerConsAddresses -} - -// IsDenylisted returns `true` if validator with `providerAddr` has been denylisted on chain `consumerId` -func (k Keeper) IsDenylisted( - ctx sdk.Context, - consumerId string, - providerAddr types.ProviderConsAddress, -) bool { - store := ctx.KVStore(k.storeKey) - bz := store.Get(types.DenylistKey(consumerId, providerAddr)) - return bz != nil -} - -// DeleteDenylist deletes all denylisted validators -func (k Keeper) DeleteDenylist(ctx sdk.Context, consumerId string) { - store := ctx.KVStore(k.storeKey) - iterator := storetypes.KVStorePrefixIterator(store, types.StringIdWithLenKey(types.DenylistKeyPrefix(), consumerId)) - defer iterator.Close() - - keysToDel := [][]byte{} - for ; iterator.Valid(); iterator.Next() { - keysToDel = append(keysToDel, iterator.Key()) - } - - for _, key := range keysToDel { - store.Delete(key) - } -} - -// IsDenylistEmpty returns `true` if no validator is denylisted on chain `consumerId` -func (k Keeper) IsDenylistEmpty(ctx sdk.Context, consumerId string) bool { - store := ctx.KVStore(k.storeKey) - iterator := storetypes.KVStorePrefixIterator(store, types.StringIdWithLenKey(types.DenylistKeyPrefix(), consumerId)) - defer iterator.Close() - - return !iterator.Valid() -} - -// UpdateDenylist populates the denylist store for the consumer chain with this consumer id -func (k Keeper) UpdateDenylist(ctx sdk.Context, consumerId string, denylist []string) { - k.DeleteDenylist(ctx, consumerId) - for _, address := range denylist { - consAddr, err := sdk.ConsAddressFromBech32(address) - if err != nil { - continue - } - - k.SetDenylist(ctx, consumerId, types.NewProviderConsAddress(consAddr)) - } -} - -// SetMinimumPowerInTopN sets the minimum power required for a validator to be in the top N -// for a given consumer chain. -func (k Keeper) SetMinimumPowerInTopN( - ctx sdk.Context, - consumerId string, - power int64, -) { - store := ctx.KVStore(k.storeKey) - - buf := make([]byte, 8) - binary.BigEndian.PutUint64(buf, uint64(power)) - - store.Set(types.MinimumPowerInTopNKey(consumerId), buf) -} - -// GetMinimumPowerInTopN returns the minimum power required for a validator to be in the top N -// for a given consumer chain. -func (k Keeper) GetMinimumPowerInTopN( - ctx sdk.Context, - consumerId string, -) (int64, bool) { - store := ctx.KVStore(k.storeKey) - buf := store.Get(types.MinimumPowerInTopNKey(consumerId)) - if buf == nil { - return 0, false - } - return int64(binary.BigEndian.Uint64(buf)), true -} - -// DeleteMinimumPowerInTopN removes the minimum power required for a validator to be in the top N -// for a given consumer chain. -func (k Keeper) DeleteMinimumPowerInTopN( - ctx sdk.Context, - consumerId string, -) { - store := ctx.KVStore(k.storeKey) - store.Delete(types.MinimumPowerInTopNKey(consumerId)) -} - -// UpdateMinimumPowerInTopN populates the minimum power in Top N for the consumer chain with this consumer id -func (k Keeper) UpdateMinimumPowerInTopN(ctx sdk.Context, consumerId string, oldTopN uint32, newTopN uint32) error { - // if the top N changes, we need to update the new minimum power in top N - if newTopN != oldTopN { - if newTopN > 0 { - // if the chain receives a non-zero top N value, store the minimum power in the top N - bondedValidators, err := k.GetLastProviderConsensusActiveValidators(ctx) - if err != nil { - return err - } - minPower, err := k.ComputeMinPowerInTopN(ctx, bondedValidators, newTopN) - if err != nil { - return err - } - k.SetMinimumPowerInTopN(ctx, consumerId, minPower) - } else { - // if the chain receives a zero top N value, we delete the min power - k.DeleteMinimumPowerInTopN(ctx, consumerId) - } - } - - return nil -} - func (k Keeper) UnbondingCanComplete(ctx sdk.Context, id uint64) error { return k.stakingKeeper.UnbondingCanComplete(ctx, id) } diff --git a/x/ccv/provider/keeper/keeper_test.go b/x/ccv/provider/keeper/keeper_test.go index c19502c0da..0a5fe59c97 100644 --- a/x/ccv/provider/keeper/keeper_test.go +++ b/x/ccv/provider/keeper/keeper_test.go @@ -8,12 +8,9 @@ import ( "cosmossdk.io/math" ibctesting "github.com/cosmos/ibc-go/v8/testing" - "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" - sdk "github.com/cosmos/cosmos-sdk/types" - stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" abci "github.com/cometbft/cometbft/abci/types" tmprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" @@ -372,165 +369,6 @@ func TestConsumerCommissionRate(t *testing.T) { require.False(t, found) } -// TestAllowlist tests the `SetAllowlist`, `IsAllowlisted`, `DeleteAllowlist`, and `IsAllowlistEmpty` methods -func TestAllowlist(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - chainID := "consumerId" - - // no validator was allowlisted and hence the allowlist is empty - require.True(t, providerKeeper.IsAllowlistEmpty(ctx, chainID)) - - providerAddr1 := providertypes.NewProviderConsAddress([]byte("providerAddr1")) - providerKeeper.SetAllowlist(ctx, chainID, providerAddr1) - require.True(t, providerKeeper.IsAllowlisted(ctx, chainID, providerAddr1)) - - // allowlist is not empty anymore - require.False(t, providerKeeper.IsAllowlistEmpty(ctx, chainID)) - - providerAddr2 := providertypes.NewProviderConsAddress([]byte("providerAddr2")) - providerKeeper.SetAllowlist(ctx, chainID, providerAddr2) - require.True(t, providerKeeper.IsAllowlisted(ctx, chainID, providerAddr2)) - require.False(t, providerKeeper.IsAllowlistEmpty(ctx, chainID)) - - providerKeeper.DeleteAllowlist(ctx, chainID) - require.False(t, providerKeeper.IsAllowlisted(ctx, chainID, providerAddr1)) - require.False(t, providerKeeper.IsAllowlisted(ctx, chainID, providerAddr2)) - require.True(t, providerKeeper.IsAllowlistEmpty(ctx, chainID)) -} - -func TestUpdateAllowlist(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - consumerId := "0" - - providerConsAddr1 := "cosmosvalcons1qmq08eruchr5sf5s3rwz7djpr5a25f7xw4mceq" - consAddr1, _ := sdk.ConsAddressFromBech32(providerConsAddr1) - providerConsAddr2 := "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39" - consAddr2, _ := sdk.ConsAddressFromBech32(providerConsAddr2) - - providerKeeper.UpdateAllowlist(ctx, consumerId, []string{providerConsAddr1, providerConsAddr2}) - - expectedAllowlist := []providertypes.ProviderConsAddress{ - providertypes.NewProviderConsAddress(consAddr1), - providertypes.NewProviderConsAddress(consAddr2)} - require.Equal(t, expectedAllowlist, providerKeeper.GetAllowList(ctx, consumerId)) -} - -// TestDenylist tests the `SetDenylist`, `IsDenylisted`, `DeleteDenylist`, and `IsDenylistEmpty` methods -func TestDenylist(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - chainID := "consumerId" - - // no validator was denylisted and hence the denylist is empty - require.True(t, providerKeeper.IsDenylistEmpty(ctx, chainID)) - - providerAddr1 := providertypes.NewProviderConsAddress([]byte("providerAddr1")) - providerKeeper.SetDenylist(ctx, chainID, providerAddr1) - require.True(t, providerKeeper.IsDenylisted(ctx, chainID, providerAddr1)) - - // denylist is not empty anymore - require.False(t, providerKeeper.IsDenylistEmpty(ctx, chainID)) - - providerAddr2 := providertypes.NewProviderConsAddress([]byte("providerAddr2")) - providerKeeper.SetDenylist(ctx, chainID, providerAddr2) - require.True(t, providerKeeper.IsDenylisted(ctx, chainID, providerAddr2)) - require.False(t, providerKeeper.IsDenylistEmpty(ctx, chainID)) - - providerKeeper.DeleteDenylist(ctx, chainID) - require.False(t, providerKeeper.IsDenylisted(ctx, chainID, providerAddr1)) - require.False(t, providerKeeper.IsDenylisted(ctx, chainID, providerAddr2)) - require.True(t, providerKeeper.IsDenylistEmpty(ctx, chainID)) -} - -func TestUpdateDenylist(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - consumerId := "0" - - providerConsAddr1 := "cosmosvalcons1qmq08eruchr5sf5s3rwz7djpr5a25f7xw4mceq" - consAddr1, _ := sdk.ConsAddressFromBech32(providerConsAddr1) - providerConsAddr2 := "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39" - consAddr2, _ := sdk.ConsAddressFromBech32(providerConsAddr2) - - providerKeeper.UpdateDenylist(ctx, consumerId, []string{providerConsAddr1, providerConsAddr2}) - - expectedDenylist := []providertypes.ProviderConsAddress{ - providertypes.NewProviderConsAddress(consAddr1), - providertypes.NewProviderConsAddress(consAddr2)} - require.Equal(t, expectedDenylist, providerKeeper.GetDenyList(ctx, consumerId)) -} - -// Tests setting, getting and deleting parameters that are stored per-consumer chain. -// The tests cover the following parameters: -// - MinimumPowerInTopN -func TestKeeperConsumerParams(t *testing.T) { - k, ctx, _, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - - tests := []struct { - name string - settingFunc func(sdk.Context, string, int64) - getFunc func(sdk.Context, string) int64 - deleteFunc func(sdk.Context, string) - initialValue int64 - updatedValue int64 - }{ - { - name: "Minimum Power In Top N", - settingFunc: func(ctx sdk.Context, id string, val int64) { k.SetMinimumPowerInTopN(ctx, id, val) }, - getFunc: func(ctx sdk.Context, id string) int64 { - minimumPowerInTopN, _ := k.GetMinimumPowerInTopN(ctx, id) - return minimumPowerInTopN - }, - deleteFunc: func(ctx sdk.Context, id string) { k.DeleteMinimumPowerInTopN(ctx, id) }, - initialValue: 1000, - updatedValue: 2000, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - chainID := "consumerId" - // Set initial value - tt.settingFunc(ctx, chainID, int64(tt.initialValue)) - - // Retrieve and check initial value - actualValue := tt.getFunc(ctx, chainID) - require.EqualValues(t, tt.initialValue, actualValue) - - // Update value - tt.settingFunc(ctx, chainID, int64(tt.updatedValue)) - - // Retrieve and check updated value - newActualValue := tt.getFunc(ctx, chainID) - require.EqualValues(t, tt.updatedValue, newActualValue) - - // Check non-existent consumer id - res := tt.getFunc(ctx, "not the consumerId") - require.Zero(t, res) - - // Delete value - tt.deleteFunc(ctx, chainID) - - // Check that value was deleted - res = tt.getFunc(ctx, chainID) - require.Zero(t, res) - - // Try deleting again - tt.deleteFunc(ctx, chainID) - - // Check that the value is still deleted - res = tt.getFunc(ctx, chainID) - require.Zero(t, res) - }) - } -} - // TestConsumerClientId tests the getter, setter, and deletion of the client id <> consumer id mappings func TestConsumerClientId(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) @@ -575,68 +413,3 @@ func TestConsumerClientId(t *testing.T) { _, found = providerKeeper.GetClientIdToConsumerId(ctx, clientIds[1]) require.False(t, found) } - -func TestUpdateMinimumPowerInTopN(t *testing.T) { - providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - consumerId := "0" - - // test case where Top N is 0 in which case there's no minimum power in top N - err := providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ - Top_N: 0, - }) - require.NoError(t, err) - - err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 0, 0) - require.NoError(t, err) - _, found := providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) - require.False(t, found) - - // test cases where Top N > 0 and for this we mock some validators - powers := []int64{10, 20, 30} - validators := []stakingtypes.Validator{ - createStakingValidator(ctx, mocks, powers[0], 1), // this validator has ~16 of the total voting power - createStakingValidator(ctx, mocks, powers[1], 2), // this validator has ~33% of the total voting gpower - createStakingValidator(ctx, mocks, powers[2], 3), // this validator has 50% of the total voting power - } - mocks.MockStakingKeeper.EXPECT().GetBondedValidatorsByPower(gomock.Any()).Return(validators, nil).AnyTimes() - - maxProviderConsensusValidators := int64(3) - params := providerKeeper.GetParams(ctx) - params.MaxProviderConsensusValidators = maxProviderConsensusValidators - providerKeeper.SetParams(ctx, params) - - // when top N is 50, the minimum power is 30 (because top validator has to validate) - err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ - Top_N: 50, - }) - require.NoError(t, err) - err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 0, 50) - require.NoError(t, err) - minimumPowerInTopN, found := providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) - require.True(t, found) - require.Equal(t, int64(30), minimumPowerInTopN) - - // when top N is 51, the minimum power is 20 (because top 2 validators have to validate) - err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ - Top_N: 51, - }) - require.NoError(t, err) - err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 50, 51) - require.NoError(t, err) - minimumPowerInTopN, found = providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) - require.True(t, found) - require.Equal(t, int64(20), minimumPowerInTopN) - - // when top N is 100, the minimum power is 10 (that of the validator with the lowest power) - err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ - Top_N: 100, - }) - require.NoError(t, err) - err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 51, 100) - require.NoError(t, err) - minimumPowerInTopN, found = providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) - require.True(t, found) - require.Equal(t, int64(10), minimumPowerInTopN) -} diff --git a/x/ccv/provider/keeper/permissionless.go b/x/ccv/provider/keeper/permissionless.go index 8d2335e6ac..ecedda108d 100644 --- a/x/ccv/provider/keeper/permissionless.go +++ b/x/ccv/provider/keeper/permissionless.go @@ -2,14 +2,11 @@ package keeper import ( "encoding/binary" - "errors" "fmt" "strconv" - errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" - ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" ) // setConsumerId sets the provided consumerId @@ -146,64 +143,6 @@ func (k Keeper) DeleteConsumerInitializationParameters(ctx sdk.Context, consumer store.Delete(types.ConsumerIdToInitializationParametersKey(consumerId)) } -// GetConsumerPowerShapingParameters returns the power-shaping parameters associated with this consumer id -func (k Keeper) GetConsumerPowerShapingParameters(ctx sdk.Context, consumerId string) (types.PowerShapingParameters, error) { - store := ctx.KVStore(k.storeKey) - bz := store.Get(types.ConsumerIdToPowerShapingParametersKey(consumerId)) - if bz == nil { - return types.PowerShapingParameters{}, errorsmod.Wrapf(ccvtypes.ErrStoreKeyNotFound, - "GetConsumerPowerShapingParameters, consumerId(%s)", consumerId) - } - var record types.PowerShapingParameters - if err := record.Unmarshal(bz); err != nil { - return types.PowerShapingParameters{}, errorsmod.Wrapf(ccvtypes.ErrStoreUnmarshal, - "GetConsumerPowerShapingParameters, consumerId(%s): %s", consumerId, err.Error()) - } - return record, nil -} - -// SetConsumerPowerShapingParameters sets the power-shaping parameters associated with this consumer id. -// Note that it also updates the allowlist and denylist indexes if they are different -func (k Keeper) SetConsumerPowerShapingParameters(ctx sdk.Context, consumerId string, parameters types.PowerShapingParameters) error { - store := ctx.KVStore(k.storeKey) - bz, err := parameters.Marshal() - if err != nil { - return fmt.Errorf("failed to marshal power-shaping parameters (%+v) for consumer id (%s): %w", parameters, consumerId, err) - } - - // get old power shaping params - oldParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId) - // ignore ErrStoreKeyNotFound errors as this might be the first time the power shaping params are set - if errors.Is(err, ccvtypes.ErrStoreUnmarshal) { - return fmt.Errorf("cannot get consumer previous power shaping parameters: %w", err) - } - - store.Set(types.ConsumerIdToPowerShapingParametersKey(consumerId), bz) - - // update allowlist and denylist indexes if needed - if !equalStringSlices(oldParameters.Allowlist, parameters.Allowlist) { - k.UpdateAllowlist(ctx, consumerId, parameters.Allowlist) - } - if !equalStringSlices(oldParameters.Denylist, parameters.Denylist) { - k.UpdateDenylist(ctx, consumerId, parameters.Denylist) - } - - return nil -} - -// equalStringSlices returns true if two string slices are equal -func equalStringSlices(a, b []string) bool { - if len(a) != len(b) { - return false - } - for i, v := range a { - if v != b[i] { - return false - } - } - return true -} - // GetConsumerPhase returns the phase associated with this consumer id func (k Keeper) GetConsumerPhase(ctx sdk.Context, consumerId string) types.ConsumerPhase { store := ctx.KVStore(k.storeKey) diff --git a/x/ccv/provider/keeper/permissionless_test.go b/x/ccv/provider/keeper/permissionless_test.go index b41700fd38..049b5d92c1 100644 --- a/x/ccv/provider/keeper/permissionless_test.go +++ b/x/ccv/provider/keeper/permissionless_test.go @@ -1,17 +1,12 @@ package keeper_test import ( - "bytes" - "errors" - "sort" "testing" "time" - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" testkeeper "github.com/cosmos/interchain-security/v5/testutil/keeper" providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" - ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" "github.com/stretchr/testify/require" ) @@ -184,79 +179,6 @@ func TestConsumerInitializationParameters(t *testing.T) { require.Equal(t, providertypes.ConsumerInitializationParameters{}, actualInitializationParameters) } -// TestConsumerPowerShapingParameters tests the getter and setter of the consumer id to power-shaping parameters methods -func TestConsumerPowerShapingParameters(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - consumerId := "consumerId" - consAddrs := []string{ - "cosmosvalcons1kswr5sq599365kcjmhgufevfps9njf43e4lwdk", - "cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6", - "cosmosvalcons1muys5jyqk4xd27e208nym85kn0t4zjcfeu63fe", - "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39", - "cosmosvalcons1qmq08eruchr5sf5s3rwz7djpr5a25f7xw4mceq", - "cosmosvalcons1uuec3cjxajv5te08p220usrjhkfhg9wyvqn0tm", - } - providerConsAddr := []providertypes.ProviderConsAddress{} - for _, addr := range consAddrs { - ca, _ := sdk.ConsAddressFromBech32(addr) - providerConsAddr = append(providerConsAddr, providertypes.NewProviderConsAddress(ca)) - } - sortProviderConsAddr := func(consAddrs []providertypes.ProviderConsAddress) { - sort.Slice(consAddrs, func(i, j int) bool { - return bytes.Compare(consAddrs[i].Address, consAddrs[j].Address) < 0 - }) - } - - _, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerId) - require.Error(t, err) - require.True(t, errors.Is(err, ccvtypes.ErrStoreKeyNotFound)) - - expectedPowerShapingParameters := providertypes.PowerShapingParameters{ - Top_N: 10, - ValidatorsPowerCap: 34, - ValidatorSetCap: 10, - Allowlist: []string{consAddrs[0], consAddrs[1]}, - Denylist: []string{consAddrs[2], consAddrs[3]}, - MinStake: 234, - AllowInactiveVals: true, - } - expectedAllowlist := []providertypes.ProviderConsAddress{providerConsAddr[0], providerConsAddr[1]} - sortProviderConsAddr(expectedAllowlist) - expectedDenylist := []providertypes.ProviderConsAddress{providerConsAddr[2], providerConsAddr[3]} - sortProviderConsAddr(expectedDenylist) - err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, expectedPowerShapingParameters) - require.NoError(t, err) - actualPowerShapingParameters, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerId) - require.NoError(t, err) - require.Equal(t, expectedPowerShapingParameters, actualPowerShapingParameters) - require.Equal(t, expectedAllowlist, providerKeeper.GetAllowList(ctx, consumerId)) - require.Equal(t, expectedDenylist, providerKeeper.GetDenyList(ctx, consumerId)) - - // assert that overwriting the current initialization record works - expectedPowerShapingParameters = providertypes.PowerShapingParameters{ - Top_N: 12, - ValidatorsPowerCap: 67, - ValidatorSetCap: 20, - Allowlist: []string{consAddrs[4], consAddrs[5]}, - Denylist: []string{consAddrs[2], consAddrs[3]}, - MinStake: 567, - AllowInactiveVals: false, - } - expectedAllowlist = []providertypes.ProviderConsAddress{providerConsAddr[4], providerConsAddr[5]} - sortProviderConsAddr(expectedAllowlist) - expectedDenylist = []providertypes.ProviderConsAddress{providerConsAddr[2], providerConsAddr[3]} - sortProviderConsAddr(expectedDenylist) - err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, expectedPowerShapingParameters) - require.NoError(t, err) - actualPowerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerId) - require.NoError(t, err) - require.Equal(t, expectedPowerShapingParameters, actualPowerShapingParameters) - require.Equal(t, expectedAllowlist, providerKeeper.GetAllowList(ctx, consumerId)) - require.Equal(t, expectedDenylist, providerKeeper.GetDenyList(ctx, consumerId)) -} - // TestConsumerPhase tests the getter, setter, and deletion of the consumer id to phase methods func TestConsumerPhase(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) diff --git a/x/ccv/provider/keeper/power_shaping.go b/x/ccv/provider/keeper/power_shaping.go new file mode 100644 index 0000000000..572564229f --- /dev/null +++ b/x/ccv/provider/keeper/power_shaping.go @@ -0,0 +1,286 @@ +package keeper + +import ( + "encoding/binary" + "errors" + "fmt" + + errorsmod "cosmossdk.io/errors" + storetypes "cosmossdk.io/store/types" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" + ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" +) + +// GetConsumerPowerShapingParameters returns the power-shaping parameters associated with this consumer id +func (k Keeper) GetConsumerPowerShapingParameters(ctx sdk.Context, consumerId string) (types.PowerShapingParameters, error) { + store := ctx.KVStore(k.storeKey) + bz := store.Get(types.ConsumerIdToPowerShapingParametersKey(consumerId)) + if bz == nil { + return types.PowerShapingParameters{}, errorsmod.Wrapf(ccvtypes.ErrStoreKeyNotFound, + "GetConsumerPowerShapingParameters, consumerId(%s)", consumerId) + } + var record types.PowerShapingParameters + if err := record.Unmarshal(bz); err != nil { + return types.PowerShapingParameters{}, errorsmod.Wrapf(ccvtypes.ErrStoreUnmarshal, + "GetConsumerPowerShapingParameters, consumerId(%s): %s", consumerId, err.Error()) + } + return record, nil +} + +// SetConsumerPowerShapingParameters sets the power-shaping parameters associated with this consumer id. +// Note that it also updates the allowlist and denylist indexes if they are different +func (k Keeper) SetConsumerPowerShapingParameters(ctx sdk.Context, consumerId string, parameters types.PowerShapingParameters) error { + store := ctx.KVStore(k.storeKey) + bz, err := parameters.Marshal() + if err != nil { + return fmt.Errorf("failed to marshal power-shaping parameters (%+v) for consumer id (%s): %w", parameters, consumerId, err) + } + + // get old power shaping params + oldParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId) + // ignore ErrStoreKeyNotFound errors as this might be the first time the power shaping params are set + if errors.Is(err, ccvtypes.ErrStoreUnmarshal) { + return fmt.Errorf("cannot get consumer previous power shaping parameters: %w", err) + } + + store.Set(types.ConsumerIdToPowerShapingParametersKey(consumerId), bz) + + // update allowlist and denylist indexes if needed + if !equalStringSlices(oldParameters.Allowlist, parameters.Allowlist) { + k.UpdateAllowlist(ctx, consumerId, parameters.Allowlist) + } + if !equalStringSlices(oldParameters.Denylist, parameters.Denylist) { + k.UpdateDenylist(ctx, consumerId, parameters.Denylist) + } + + return nil +} + +// equalStringSlices returns true if two string slices are equal +func equalStringSlices(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i, v := range a { + if v != b[i] { + return false + } + } + return true +} + +// SetAllowlist allowlists validator with `providerAddr` address on chain `consumerId` +func (k Keeper) SetAllowlist( + ctx sdk.Context, + consumerId string, + providerAddr types.ProviderConsAddress, +) { + store := ctx.KVStore(k.storeKey) + store.Set(types.AllowlistKey(consumerId, providerAddr), []byte{}) +} + +// GetAllowList returns all allowlisted validators +func (k Keeper) GetAllowList( + ctx sdk.Context, + consumerId string, +) (providerConsAddresses []types.ProviderConsAddress) { + store := ctx.KVStore(k.storeKey) + key := types.StringIdWithLenKey(types.AllowlistKeyPrefix(), consumerId) + iterator := storetypes.KVStorePrefixIterator(store, key) + defer iterator.Close() + + for ; iterator.Valid(); iterator.Next() { + providerConsAddresses = append(providerConsAddresses, types.NewProviderConsAddress(iterator.Key()[len(key):])) + } + + return providerConsAddresses +} + +// IsAllowlisted returns `true` if validator with `providerAddr` has been allowlisted on chain `consumerId` +func (k Keeper) IsAllowlisted( + ctx sdk.Context, + consumerId string, + providerAddr types.ProviderConsAddress, +) bool { + store := ctx.KVStore(k.storeKey) + bz := store.Get(types.AllowlistKey(consumerId, providerAddr)) + return bz != nil +} + +// DeleteAllowlist deletes all allowlisted validators +func (k Keeper) DeleteAllowlist(ctx sdk.Context, consumerId string) { + store := ctx.KVStore(k.storeKey) + iterator := storetypes.KVStorePrefixIterator(store, types.StringIdWithLenKey(types.AllowlistKeyPrefix(), consumerId)) + defer iterator.Close() + + keysToDel := [][]byte{} + for ; iterator.Valid(); iterator.Next() { + keysToDel = append(keysToDel, iterator.Key()) + } + + for _, key := range keysToDel { + store.Delete(key) + } +} + +// IsAllowlistEmpty returns `true` if no validator is allowlisted on chain `consumerId` +func (k Keeper) IsAllowlistEmpty(ctx sdk.Context, consumerId string) bool { + store := ctx.KVStore(k.storeKey) + iterator := storetypes.KVStorePrefixIterator(store, types.StringIdWithLenKey(types.AllowlistKeyPrefix(), consumerId)) + defer iterator.Close() + + return !iterator.Valid() +} + +// UpdateAllowlist populates the allowlist store for the consumer chain with this consumer id +func (k Keeper) UpdateAllowlist(ctx sdk.Context, consumerId string, allowlist []string) { + k.DeleteAllowlist(ctx, consumerId) + for _, address := range allowlist { + consAddr, err := sdk.ConsAddressFromBech32(address) + if err != nil { + continue + } + + k.SetAllowlist(ctx, consumerId, types.NewProviderConsAddress(consAddr)) + } +} + +// SetDenylist denylists validator with `providerAddr` address on chain `consumerId` +func (k Keeper) SetDenylist( + ctx sdk.Context, + consumerId string, + providerAddr types.ProviderConsAddress, +) { + store := ctx.KVStore(k.storeKey) + store.Set(types.DenylistKey(consumerId, providerAddr), []byte{}) +} + +// GetDenyList returns all denylisted validators +func (k Keeper) GetDenyList( + ctx sdk.Context, + consumerId string, +) (providerConsAddresses []types.ProviderConsAddress) { + store := ctx.KVStore(k.storeKey) + key := types.StringIdWithLenKey(types.DenylistKeyPrefix(), consumerId) + iterator := storetypes.KVStorePrefixIterator(store, key) + defer iterator.Close() + + for ; iterator.Valid(); iterator.Next() { + providerConsAddresses = append(providerConsAddresses, types.NewProviderConsAddress(iterator.Key()[len(key):])) + } + + return providerConsAddresses +} + +// IsDenylisted returns `true` if validator with `providerAddr` has been denylisted on chain `consumerId` +func (k Keeper) IsDenylisted( + ctx sdk.Context, + consumerId string, + providerAddr types.ProviderConsAddress, +) bool { + store := ctx.KVStore(k.storeKey) + bz := store.Get(types.DenylistKey(consumerId, providerAddr)) + return bz != nil +} + +// DeleteDenylist deletes all denylisted validators +func (k Keeper) DeleteDenylist(ctx sdk.Context, consumerId string) { + store := ctx.KVStore(k.storeKey) + iterator := storetypes.KVStorePrefixIterator(store, types.StringIdWithLenKey(types.DenylistKeyPrefix(), consumerId)) + defer iterator.Close() + + keysToDel := [][]byte{} + for ; iterator.Valid(); iterator.Next() { + keysToDel = append(keysToDel, iterator.Key()) + } + + for _, key := range keysToDel { + store.Delete(key) + } +} + +// IsDenylistEmpty returns `true` if no validator is denylisted on chain `consumerId` +func (k Keeper) IsDenylistEmpty(ctx sdk.Context, consumerId string) bool { + store := ctx.KVStore(k.storeKey) + iterator := storetypes.KVStorePrefixIterator(store, types.StringIdWithLenKey(types.DenylistKeyPrefix(), consumerId)) + defer iterator.Close() + + return !iterator.Valid() +} + +// UpdateDenylist populates the denylist store for the consumer chain with this consumer id +func (k Keeper) UpdateDenylist(ctx sdk.Context, consumerId string, denylist []string) { + k.DeleteDenylist(ctx, consumerId) + for _, address := range denylist { + consAddr, err := sdk.ConsAddressFromBech32(address) + if err != nil { + continue + } + + k.SetDenylist(ctx, consumerId, types.NewProviderConsAddress(consAddr)) + } +} + +// SetMinimumPowerInTopN sets the minimum power required for a validator to be in the top N +// for a given consumer chain. +func (k Keeper) SetMinimumPowerInTopN( + ctx sdk.Context, + consumerId string, + power int64, +) { + store := ctx.KVStore(k.storeKey) + + buf := make([]byte, 8) + binary.BigEndian.PutUint64(buf, uint64(power)) + + store.Set(types.MinimumPowerInTopNKey(consumerId), buf) +} + +// GetMinimumPowerInTopN returns the minimum power required for a validator to be in the top N +// for a given consumer chain. +func (k Keeper) GetMinimumPowerInTopN( + ctx sdk.Context, + consumerId string, +) (int64, bool) { + store := ctx.KVStore(k.storeKey) + buf := store.Get(types.MinimumPowerInTopNKey(consumerId)) + if buf == nil { + return 0, false + } + return int64(binary.BigEndian.Uint64(buf)), true +} + +// DeleteMinimumPowerInTopN removes the minimum power required for a validator to be in the top N +// for a given consumer chain. +func (k Keeper) DeleteMinimumPowerInTopN( + ctx sdk.Context, + consumerId string, +) { + store := ctx.KVStore(k.storeKey) + store.Delete(types.MinimumPowerInTopNKey(consumerId)) +} + +// UpdateMinimumPowerInTopN populates the minimum power in Top N for the consumer chain with this consumer id +func (k Keeper) UpdateMinimumPowerInTopN(ctx sdk.Context, consumerId string, oldTopN uint32, newTopN uint32) error { + // if the top N changes, we need to update the new minimum power in top N + if newTopN != oldTopN { + if newTopN > 0 { + // if the chain receives a non-zero top N value, store the minimum power in the top N + bondedValidators, err := k.GetLastProviderConsensusActiveValidators(ctx) + if err != nil { + return err + } + minPower, err := k.ComputeMinPowerInTopN(ctx, bondedValidators, newTopN) + if err != nil { + return err + } + k.SetMinimumPowerInTopN(ctx, consumerId, minPower) + } else { + // if the chain receives a zero top N value, we delete the min power + k.DeleteMinimumPowerInTopN(ctx, consumerId) + } + } + + return nil +} diff --git a/x/ccv/provider/keeper/power_shaping_test.go b/x/ccv/provider/keeper/power_shaping_test.go new file mode 100644 index 0000000000..bcf49373c6 --- /dev/null +++ b/x/ccv/provider/keeper/power_shaping_test.go @@ -0,0 +1,313 @@ +package keeper_test + +import ( + "bytes" + "errors" + "sort" + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + testkeeper "github.com/cosmos/interchain-security/v5/testutil/keeper" + providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" + ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" +) + +// TestConsumerPowerShapingParameters tests the getter and setter of the consumer id to power-shaping parameters methods +func TestConsumerPowerShapingParameters(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + consumerId := "consumerId" + consAddrs := []string{ + "cosmosvalcons1kswr5sq599365kcjmhgufevfps9njf43e4lwdk", + "cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6", + "cosmosvalcons1muys5jyqk4xd27e208nym85kn0t4zjcfeu63fe", + "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39", + "cosmosvalcons1qmq08eruchr5sf5s3rwz7djpr5a25f7xw4mceq", + "cosmosvalcons1uuec3cjxajv5te08p220usrjhkfhg9wyvqn0tm", + } + providerConsAddr := []providertypes.ProviderConsAddress{} + for _, addr := range consAddrs { + ca, _ := sdk.ConsAddressFromBech32(addr) + providerConsAddr = append(providerConsAddr, providertypes.NewProviderConsAddress(ca)) + } + sortProviderConsAddr := func(consAddrs []providertypes.ProviderConsAddress) { + sort.Slice(consAddrs, func(i, j int) bool { + return bytes.Compare(consAddrs[i].Address, consAddrs[j].Address) < 0 + }) + } + + _, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerId) + require.Error(t, err) + require.True(t, errors.Is(err, ccvtypes.ErrStoreKeyNotFound)) + + expectedPowerShapingParameters := providertypes.PowerShapingParameters{ + Top_N: 10, + ValidatorsPowerCap: 34, + ValidatorSetCap: 10, + Allowlist: []string{consAddrs[0], consAddrs[1]}, + Denylist: []string{consAddrs[2], consAddrs[3]}, + MinStake: 234, + AllowInactiveVals: true, + } + expectedAllowlist := []providertypes.ProviderConsAddress{providerConsAddr[0], providerConsAddr[1]} + sortProviderConsAddr(expectedAllowlist) + expectedDenylist := []providertypes.ProviderConsAddress{providerConsAddr[2], providerConsAddr[3]} + sortProviderConsAddr(expectedDenylist) + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, expectedPowerShapingParameters) + require.NoError(t, err) + actualPowerShapingParameters, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerId) + require.NoError(t, err) + require.Equal(t, expectedPowerShapingParameters, actualPowerShapingParameters) + require.Equal(t, expectedAllowlist, providerKeeper.GetAllowList(ctx, consumerId)) + require.Equal(t, expectedDenylist, providerKeeper.GetDenyList(ctx, consumerId)) + + // assert that overwriting the current initialization record works + expectedPowerShapingParameters = providertypes.PowerShapingParameters{ + Top_N: 12, + ValidatorsPowerCap: 67, + ValidatorSetCap: 20, + Allowlist: []string{consAddrs[4], consAddrs[5]}, + Denylist: []string{consAddrs[2], consAddrs[3]}, + MinStake: 567, + AllowInactiveVals: false, + } + expectedAllowlist = []providertypes.ProviderConsAddress{providerConsAddr[4], providerConsAddr[5]} + sortProviderConsAddr(expectedAllowlist) + expectedDenylist = []providertypes.ProviderConsAddress{providerConsAddr[2], providerConsAddr[3]} + sortProviderConsAddr(expectedDenylist) + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, expectedPowerShapingParameters) + require.NoError(t, err) + actualPowerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerId) + require.NoError(t, err) + require.Equal(t, expectedPowerShapingParameters, actualPowerShapingParameters) + require.Equal(t, expectedAllowlist, providerKeeper.GetAllowList(ctx, consumerId)) + require.Equal(t, expectedDenylist, providerKeeper.GetDenyList(ctx, consumerId)) +} + +// TestAllowlist tests the `SetAllowlist`, `IsAllowlisted`, `DeleteAllowlist`, and `IsAllowlistEmpty` methods +func TestAllowlist(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + chainID := "consumerId" + + // no validator was allowlisted and hence the allowlist is empty + require.True(t, providerKeeper.IsAllowlistEmpty(ctx, chainID)) + + providerAddr1 := providertypes.NewProviderConsAddress([]byte("providerAddr1")) + providerKeeper.SetAllowlist(ctx, chainID, providerAddr1) + require.True(t, providerKeeper.IsAllowlisted(ctx, chainID, providerAddr1)) + + // allowlist is not empty anymore + require.False(t, providerKeeper.IsAllowlistEmpty(ctx, chainID)) + + providerAddr2 := providertypes.NewProviderConsAddress([]byte("providerAddr2")) + providerKeeper.SetAllowlist(ctx, chainID, providerAddr2) + require.True(t, providerKeeper.IsAllowlisted(ctx, chainID, providerAddr2)) + require.False(t, providerKeeper.IsAllowlistEmpty(ctx, chainID)) + + providerKeeper.DeleteAllowlist(ctx, chainID) + require.False(t, providerKeeper.IsAllowlisted(ctx, chainID, providerAddr1)) + require.False(t, providerKeeper.IsAllowlisted(ctx, chainID, providerAddr2)) + require.True(t, providerKeeper.IsAllowlistEmpty(ctx, chainID)) +} + +func TestUpdateAllowlist(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + consumerId := "0" + + providerConsAddr1 := "cosmosvalcons1qmq08eruchr5sf5s3rwz7djpr5a25f7xw4mceq" + consAddr1, _ := sdk.ConsAddressFromBech32(providerConsAddr1) + providerConsAddr2 := "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39" + consAddr2, _ := sdk.ConsAddressFromBech32(providerConsAddr2) + + providerKeeper.UpdateAllowlist(ctx, consumerId, []string{providerConsAddr1, providerConsAddr2}) + + expectedAllowlist := []providertypes.ProviderConsAddress{ + providertypes.NewProviderConsAddress(consAddr1), + providertypes.NewProviderConsAddress(consAddr2)} + require.Equal(t, expectedAllowlist, providerKeeper.GetAllowList(ctx, consumerId)) +} + +// TestDenylist tests the `SetDenylist`, `IsDenylisted`, `DeleteDenylist`, and `IsDenylistEmpty` methods +func TestDenylist(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + chainID := "consumerId" + + // no validator was denylisted and hence the denylist is empty + require.True(t, providerKeeper.IsDenylistEmpty(ctx, chainID)) + + providerAddr1 := providertypes.NewProviderConsAddress([]byte("providerAddr1")) + providerKeeper.SetDenylist(ctx, chainID, providerAddr1) + require.True(t, providerKeeper.IsDenylisted(ctx, chainID, providerAddr1)) + + // denylist is not empty anymore + require.False(t, providerKeeper.IsDenylistEmpty(ctx, chainID)) + + providerAddr2 := providertypes.NewProviderConsAddress([]byte("providerAddr2")) + providerKeeper.SetDenylist(ctx, chainID, providerAddr2) + require.True(t, providerKeeper.IsDenylisted(ctx, chainID, providerAddr2)) + require.False(t, providerKeeper.IsDenylistEmpty(ctx, chainID)) + + providerKeeper.DeleteDenylist(ctx, chainID) + require.False(t, providerKeeper.IsDenylisted(ctx, chainID, providerAddr1)) + require.False(t, providerKeeper.IsDenylisted(ctx, chainID, providerAddr2)) + require.True(t, providerKeeper.IsDenylistEmpty(ctx, chainID)) +} + +func TestUpdateDenylist(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + consumerId := "0" + + providerConsAddr1 := "cosmosvalcons1qmq08eruchr5sf5s3rwz7djpr5a25f7xw4mceq" + consAddr1, _ := sdk.ConsAddressFromBech32(providerConsAddr1) + providerConsAddr2 := "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39" + consAddr2, _ := sdk.ConsAddressFromBech32(providerConsAddr2) + + providerKeeper.UpdateDenylist(ctx, consumerId, []string{providerConsAddr1, providerConsAddr2}) + + expectedDenylist := []providertypes.ProviderConsAddress{ + providertypes.NewProviderConsAddress(consAddr1), + providertypes.NewProviderConsAddress(consAddr2)} + require.Equal(t, expectedDenylist, providerKeeper.GetDenyList(ctx, consumerId)) +} + +// Tests setting, getting and deleting parameters that are stored per-consumer chain. +// The tests cover the following parameters: +// - MinimumPowerInTopN +func TestKeeperConsumerParams(t *testing.T) { + k, ctx, _, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + + tests := []struct { + name string + settingFunc func(sdk.Context, string, int64) + getFunc func(sdk.Context, string) int64 + deleteFunc func(sdk.Context, string) + initialValue int64 + updatedValue int64 + }{ + { + name: "Minimum Power In Top N", + settingFunc: func(ctx sdk.Context, id string, val int64) { k.SetMinimumPowerInTopN(ctx, id, val) }, + getFunc: func(ctx sdk.Context, id string) int64 { + minimumPowerInTopN, _ := k.GetMinimumPowerInTopN(ctx, id) + return minimumPowerInTopN + }, + deleteFunc: func(ctx sdk.Context, id string) { k.DeleteMinimumPowerInTopN(ctx, id) }, + initialValue: 1000, + updatedValue: 2000, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + chainID := "consumerId" + // Set initial value + tt.settingFunc(ctx, chainID, int64(tt.initialValue)) + + // Retrieve and check initial value + actualValue := tt.getFunc(ctx, chainID) + require.EqualValues(t, tt.initialValue, actualValue) + + // Update value + tt.settingFunc(ctx, chainID, int64(tt.updatedValue)) + + // Retrieve and check updated value + newActualValue := tt.getFunc(ctx, chainID) + require.EqualValues(t, tt.updatedValue, newActualValue) + + // Check non-existent consumer id + res := tt.getFunc(ctx, "not the consumerId") + require.Zero(t, res) + + // Delete value + tt.deleteFunc(ctx, chainID) + + // Check that value was deleted + res = tt.getFunc(ctx, chainID) + require.Zero(t, res) + + // Try deleting again + tt.deleteFunc(ctx, chainID) + + // Check that the value is still deleted + res = tt.getFunc(ctx, chainID) + require.Zero(t, res) + }) + } +} + +func TestUpdateMinimumPowerInTopN(t *testing.T) { + providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + consumerId := "0" + + // test case where Top N is 0 in which case there's no minimum power in top N + err := providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ + Top_N: 0, + }) + require.NoError(t, err) + + err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 0, 0) + require.NoError(t, err) + _, found := providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) + require.False(t, found) + + // test cases where Top N > 0 and for this we mock some validators + powers := []int64{10, 20, 30} + validators := []stakingtypes.Validator{ + createStakingValidator(ctx, mocks, powers[0], 1), // this validator has ~16 of the total voting power + createStakingValidator(ctx, mocks, powers[1], 2), // this validator has ~33% of the total voting gpower + createStakingValidator(ctx, mocks, powers[2], 3), // this validator has 50% of the total voting power + } + mocks.MockStakingKeeper.EXPECT().GetBondedValidatorsByPower(gomock.Any()).Return(validators, nil).AnyTimes() + + maxProviderConsensusValidators := int64(3) + params := providerKeeper.GetParams(ctx) + params.MaxProviderConsensusValidators = maxProviderConsensusValidators + providerKeeper.SetParams(ctx, params) + + // when top N is 50, the minimum power is 30 (because top validator has to validate) + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ + Top_N: 50, + }) + require.NoError(t, err) + err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 0, 50) + require.NoError(t, err) + minimumPowerInTopN, found := providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) + require.True(t, found) + require.Equal(t, int64(30), minimumPowerInTopN) + + // when top N is 51, the minimum power is 20 (because top 2 validators have to validate) + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ + Top_N: 51, + }) + require.NoError(t, err) + err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 50, 51) + require.NoError(t, err) + minimumPowerInTopN, found = providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) + require.True(t, found) + require.Equal(t, int64(20), minimumPowerInTopN) + + // when top N is 100, the minimum power is 10 (that of the validator with the lowest power) + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ + Top_N: 100, + }) + require.NoError(t, err) + err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 51, 100) + require.NoError(t, err) + minimumPowerInTopN, found = providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) + require.True(t, found) + require.Equal(t, int64(10), minimumPowerInTopN) +} From 21b34e5d719c1bab8d0b3f3024b87c5d8e3a4348 Mon Sep 17 00:00:00 2001 From: mpoke Date: Tue, 3 Sep 2024 12:14:19 +0200 Subject: [PATCH 4/4] rename record to parameters --- x/ccv/provider/keeper/power_shaping.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/ccv/provider/keeper/power_shaping.go b/x/ccv/provider/keeper/power_shaping.go index 572564229f..c54ca41e4e 100644 --- a/x/ccv/provider/keeper/power_shaping.go +++ b/x/ccv/provider/keeper/power_shaping.go @@ -20,12 +20,12 @@ func (k Keeper) GetConsumerPowerShapingParameters(ctx sdk.Context, consumerId st return types.PowerShapingParameters{}, errorsmod.Wrapf(ccvtypes.ErrStoreKeyNotFound, "GetConsumerPowerShapingParameters, consumerId(%s)", consumerId) } - var record types.PowerShapingParameters - if err := record.Unmarshal(bz); err != nil { + var parameters types.PowerShapingParameters + if err := parameters.Unmarshal(bz); err != nil { return types.PowerShapingParameters{}, errorsmod.Wrapf(ccvtypes.ErrStoreUnmarshal, "GetConsumerPowerShapingParameters, consumerId(%s): %s", consumerId, err.Error()) } - return record, nil + return parameters, nil } // SetConsumerPowerShapingParameters sets the power-shaping parameters associated with this consumer id.