Skip to content

Commit

Permalink
fix!: several permissionless changes (#2189)
Browse files Browse the repository at this point in the history
* minor fixes in tests

* replace IsConsumerProposedOrRegistered with IsConsumerActive

* remove param from createStakingValidator

* handle error in GetTopN

* remove aux methods for getting PowerShapingParameters

* remove default PowerShapingParameters

* fix tests

* handle error messages
  • Loading branch information
mpoke committed Aug 29, 2024
1 parent 29b1bf4 commit 1135770
Show file tree
Hide file tree
Showing 24 changed files with 396 additions and 466 deletions.
7 changes: 4 additions & 3 deletions tests/integration/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,9 @@ func (suite *CCVTestSuite) SetupTest() {
suite.registerPacketSniffer(bundle.Chain)

// check that TopN is correctly set for the consumer
topN := providerKeeper.GetTopN(suite.providerCtx(), bundle.ConsumerId)
suite.Require().Equal(bundle.TopN, topN)
powerShapingParameters, err := providerKeeper.GetConsumerPowerShapingParameters(suite.providerCtx(), bundle.ConsumerId)
suite.Require().NoError(err)
suite.Require().Equal(bundle.TopN, powerShapingParameters.Top_N)
}

// initialize each consumer chain with it's corresponding genesis state
Expand Down Expand Up @@ -283,7 +284,7 @@ func initConsumerChain(
err = bundle.Path.EndpointA.UpdateClient()
s.Require().NoError(err)

if consumerId == "0" {
if consumerId == icstestingutils.FirstConsumerID {
// Support tests that were written before multiple consumers were supported.
firstBundle := s.getFirstBundle()
s.consumerApp = firstBundle.App
Expand Down
2 changes: 1 addition & 1 deletion testutil/ibc_testing/generic_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func AddConsumer[Tp testutil.ProviderApp, Tc testutil.ConsumerApp](
providerKeeper.SetConsumerInitializationParameters(providerChain.GetContext(), consumerId, initializationParameters)
providerKeeper.SetConsumerPowerShapingParameters(providerChain.GetContext(), consumerId, powerShapingParameters)
providerKeeper.SetConsumerPhase(providerChain.GetContext(), consumerId, providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED)
providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(providerChain.GetContext(), consumerId, coordinator.CurrentTime)
providerKeeper.AppendConsumerToBeLaunched(providerChain.GetContext(), consumerId, coordinator.CurrentTime)

// opt-in all validators
lastVals, err := providerApp.GetProviderKeeper().GetLastBondedValidators(providerChain.GetContext())
Expand Down
4 changes: 2 additions & 2 deletions testutil/keeper/unit_test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,8 @@ func TestProviderStateIsCleanedAfterConsumerChainIsStopped(t *testing.T, ctx sdk
require.Empty(t, acks)

// in case the chain was successfully stopped, it should not contain a Top N associated to it
topN := providerKeeper.GetTopN(ctx, consumerId)
require.Zero(t, topN)
_, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerId)
require.Error(t, err)

// test key assignment state is cleaned
require.Empty(t, providerKeeper.GetAllValidatorConsumerPubKeys(ctx, &consumerId))
Expand Down
6 changes: 3 additions & 3 deletions x/ccv/provider/keeper/distribution.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,10 +335,10 @@ func (k Keeper) IdentifyConsumerIdFromIBCPacket(ctx sdk.Context, packet channelt
// HandleSetConsumerCommissionRate sets a per-consumer chain commission rate for the given provider address
// on the condition that the given consumer chain exists.
func (k Keeper) HandleSetConsumerCommissionRate(ctx sdk.Context, consumerId string, providerAddr types.ProviderConsAddress, commissionRate math.LegacyDec) error {
// check that the consumer chain exists
if !k.IsConsumerProposedOrRegistered(ctx, consumerId) {
// check that the consumer chain is active -- registered, initialized, or launched
if !k.IsConsumerActive(ctx, consumerId) {
return errorsmod.Wrapf(
types.ErrUnknownConsumerId,
types.ErrInvalidPhase,
"unknown consumer chain, with id: %s", consumerId)
}

Expand Down
42 changes: 25 additions & 17 deletions x/ccv/provider/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,10 @@ func (k Keeper) GetConsumerChain(ctx sdk.Context, consumerId string) (types.Chai
}

clientID, _ := k.GetConsumerClientId(ctx, consumerId)
topN := k.GetTopN(ctx, consumerId)
powerShapingParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId)
if err != nil {
return types.Chain{}, fmt.Errorf("cannot find power shaping parameters for consumer (%s): %s", consumerId, err.Error())
}

// Get the minimal power in the top N for the consumer chain
minPowerInTopN, found := k.GetMinimumPowerInTopN(ctx, consumerId)
Expand Down Expand Up @@ -141,22 +144,19 @@ func (k Keeper) GetConsumerChain(ctx sdk.Context, consumerId string) (types.Chai
return types.Chain{}, fmt.Errorf("cannot get metadata for consumer (%s): %w", consumerId, err)
}

allowInactiveVals := k.AllowsInactiveValidators(ctx, consumerId)
minStake := k.GetMinStake(ctx, consumerId)

return types.Chain{
ChainId: chainID,
ClientId: clientID,
Top_N: topN,
Top_N: powerShapingParameters.Top_N,
MinPowerInTop_N: minPowerInTopN,
ValidatorSetCap: k.GetValidatorSetCap(ctx, consumerId),
ValidatorsPowerCap: k.GetValidatorsPowerCap(ctx, consumerId),
ValidatorSetCap: powerShapingParameters.ValidatorSetCap,
ValidatorsPowerCap: powerShapingParameters.ValidatorsPowerCap,
Allowlist: strAllowlist,
Denylist: strDenylist,
Phase: phase,
Metadata: metadata,
AllowInactiveVals: allowInactiveVals,
MinStake: minStake,
AllowInactiveVals: powerShapingParameters.AllowInactiveVals,
MinStake: powerShapingParameters.MinStake,
}, nil
}

Expand Down Expand Up @@ -314,7 +314,7 @@ func (k Keeper) QueryConsumerChainOptedInValidators(goCtx context.Context, req *
optedInVals := []string{}
ctx := sdk.UnwrapSDKContext(goCtx)

if !k.IsConsumerProposedOrRegistered(ctx, consumerId) {
if !k.IsConsumerActive(ctx, consumerId) {
return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("unknown consumer chain: %s", consumerId))
}

Expand Down Expand Up @@ -366,19 +366,23 @@ func (k Keeper) QueryConsumerValidators(goCtx context.Context, req *types.QueryC
}
minPower := int64(0)
// for TopN chains, compute the minPower that will be automatically opted in
if topN := k.GetTopN(ctx, consumerId); topN > 0 {
powerShapingParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId)
if err != nil {
return nil, status.Error(codes.Internal, fmt.Sprintf("failed to get power shaping params: %s", err))
}
if powerShapingParameters.Top_N > 0 {
activeValidators, err := k.GetLastProviderConsensusActiveValidators(ctx)
if err != nil {
return nil, status.Error(codes.Internal, fmt.Sprintf("failed to get active validators: %s", err))
}

minPower, err = k.ComputeMinPowerInTopN(ctx, activeValidators, topN)
minPower, err = k.ComputeMinPowerInTopN(ctx, activeValidators, powerShapingParameters.Top_N)
if err != nil {
return nil, status.Error(codes.Internal, fmt.Sprintf("failed to compute min power to opt in for chain %s: %s", consumerId, err))
}
}

consumerValSet = k.ComputeNextValidators(ctx, consumerId, bondedValidators, minPower)
consumerValSet = k.ComputeNextValidators(ctx, consumerId, bondedValidators, powerShapingParameters, minPower)

// sort the address of the validators by ascending lexical order as they were persisted to the store
sort.Slice(consumerValSet, func(i, j int) bool {
Expand Down Expand Up @@ -485,10 +489,14 @@ func (k Keeper) hasToValidate(

minPowerToOptIn := int64(0)
// If the consumer is TopN compute the minimum power
if topN := k.GetTopN(ctx, consumerId); topN > 0 {
powerShapingParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId)
if err != nil {
return false, err
}
if powerShapingParameters.Top_N > 0 {
// compute the minimum power to opt-in since the one in the state is stale
// Note that the effective min power will be computed at the end of the epoch
minPowerToOptIn, err = k.ComputeMinPowerInTopN(ctx, activeValidators, topN)
minPowerToOptIn, err = k.ComputeMinPowerInTopN(ctx, activeValidators, powerShapingParameters.Top_N)
if err != nil {
return false, err
}
Expand All @@ -500,7 +508,7 @@ func (k Keeper) hasToValidate(
if err != nil {
return false, err
}
nextValidators := k.ComputeNextValidators(ctx, consumerId, lastVals, minPowerToOptIn)
nextValidators := k.ComputeNextValidators(ctx, consumerId, lastVals, powerShapingParameters, minPowerToOptIn)
for _, v := range nextValidators {
consAddr := sdk.ConsAddress(v.ProviderConsAddr)
if provAddr.ToSdkConsAddr().Equals(consAddr) {
Expand Down Expand Up @@ -532,7 +540,7 @@ func (k Keeper) QueryValidatorConsumerCommissionRate(goCtx context.Context, req

ctx := sdk.UnwrapSDKContext(goCtx)

if !k.IsConsumerProposedOrRegistered(ctx, consumerId) {
if !k.IsConsumerActive(ctx, consumerId) {
return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("unknown consumer chain: %s", consumerId))
}

Expand Down
46 changes: 28 additions & 18 deletions x/ccv/provider/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ func TestQueryAllPairsValConAddrByConsumerChainID(t *testing.T) {
require.Error(t, err)

// Request with invalid consumer id
response, err := pk.QueryAllPairsValConAddrByConsumerChainID(ctx, &types.QueryAllPairsValConAddrByConsumerChainIDRequest{ConsumerId: "invalidConsumerId"})
_, err = pk.QueryAllPairsValConAddrByConsumerChainID(ctx, &types.QueryAllPairsValConAddrByConsumerChainIDRequest{ConsumerId: "invalidConsumerId"})
require.Error(t, err)

// Request is valid
response, err = pk.QueryAllPairsValConAddrByConsumerChainID(ctx, &types.QueryAllPairsValConAddrByConsumerChainIDRequest{ConsumerId: consumerId})
response, err := pk.QueryAllPairsValConAddrByConsumerChainID(ctx, &types.QueryAllPairsValConAddrByConsumerChainIDRequest{ConsumerId: consumerId})
require.NoError(t, err)

expectedResult := types.PairValConAddrProviderAndConsumer{
Expand Down Expand Up @@ -117,14 +117,18 @@ func TestQueryConsumerValidators(t *testing.T) {
// set the consumer to the "registered" phase
pk.SetConsumerPhase(ctx, consumerId, types.ConsumerPhase_CONSUMER_PHASE_REGISTERED)

// set power shaping params
err = pk.SetConsumerPowerShapingParameters(ctx, consumerId, types.PowerShapingParameters{})
require.NoError(t, err)

// expect empty valset
testkeeper.SetupMocksForLastBondedValidatorsExpectation(mocks.MockStakingKeeper, 0, []stakingtypes.Validator{}, 1) // -1 to allow the calls "AnyTimes"
res, err := pk.QueryConsumerValidators(ctx, &req)
require.NoError(t, err)
require.Len(t, res.Validators, 0)

// create bonded validators
val1 := createStakingValidator(ctx, mocks, 1, 1, 1)
val1 := createStakingValidator(ctx, mocks, 1, 1)
pk1, _ := val1.CmtConsPublicKey()
valConsAddr1, _ := val1.GetConsAddr()
providerAddr1 := types.NewProviderConsAddress(valConsAddr1)
Expand All @@ -133,7 +137,7 @@ func TestQueryConsumerValidators(t *testing.T) {
val1.Description = stakingtypes.Description{Moniker: "ConsumerValidator1"}
val1.Commission.Rate = math.LegacyMustNewDecFromStr("0.123")

val2 := createStakingValidator(ctx, mocks, 1, 2, 2)
val2 := createStakingValidator(ctx, mocks, 2, 2)
pk2, _ := val2.CmtConsPublicKey()
valConsAddr2, _ := val2.GetConsAddr()
providerAddr2 := types.NewProviderConsAddress(valConsAddr2)
Expand All @@ -142,7 +146,7 @@ func TestQueryConsumerValidators(t *testing.T) {
val2.Description = stakingtypes.Description{Moniker: "ConsumerValidator2"}
val2.Commission.Rate = math.LegacyMustNewDecFromStr("0.456")

val3 := createStakingValidator(ctx, mocks, 1, 3, 3)
val3 := createStakingValidator(ctx, mocks, 3, 3)
pk3, _ := val3.CmtConsPublicKey()
valConsAddr3, _ := val3.GetConsAddr()
providerAddr3 := types.NewProviderConsAddress(valConsAddr3)
Expand Down Expand Up @@ -289,7 +293,7 @@ func TestQueryConsumerChainsValidatorHasToValidate(t *testing.T) {
pk, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
defer ctrl.Finish()

val := createStakingValidator(ctx, mocks, 1, 1, 1)
val := createStakingValidator(ctx, mocks, 1, 1)
valConsAddr, _ := val.GetConsAddr()
providerAddr := types.NewProviderConsAddress(valConsAddr)
mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(ctx, valConsAddr).Return(val, nil).AnyTimes()
Expand All @@ -300,13 +304,15 @@ func TestQueryConsumerChainsValidatorHasToValidate(t *testing.T) {
}

// set up some consumer chains
consumerChains := []string{"chain1", "chain2", "chain3", "chain4"}
for _, cc := range consumerChains {
pk.SetConsumerClientId(ctx, cc, "clientID")
consumerIDs := []string{"1", "23", "456", "6789"}
for _, cID := range consumerIDs {
pk.SetConsumerClientId(ctx, cID, "clientID")
err := pk.SetConsumerPowerShapingParameters(ctx, cID, types.PowerShapingParameters{})
require.NoError(t, err)
}

// set `providerAddr` as a consumer validator on "chain1"
pk.SetConsumerValidator(ctx, "chain1", types.ConsensusValidator{
// set `providerAddr` as a consumer validator on first consumer chain
pk.SetConsumerValidator(ctx, consumerIDs[0], types.ConsensusValidator{
ProviderConsAddr: providerAddr.ToSdkConsAddr(),
Power: 1,
PublicKey: &crypto.PublicKey{
Expand All @@ -316,17 +322,18 @@ func TestQueryConsumerChainsValidatorHasToValidate(t *testing.T) {
},
})

// set `providerAddr` as an opted-in validator on "chain3"
pk.SetOptedIn(ctx, "chain3", providerAddr)
// set `providerAddr` as an opted-in validator on third consumer chain
pk.SetOptedIn(ctx, consumerIDs[2], providerAddr)

// set max provider consensus vals to include all validators
params := pk.GetParams(ctx)
params.MaxProviderConsensusValidators = 3
pk.SetParams(ctx, params)

// `providerAddr` has to validate "chain1" because it is a consumer validator in this chain, as well as "chain3"
// because it opted in, in "chain3" and `providerAddr` belongs to the bonded validators
expectedChains := []string{"chain1", "chain3"}
// `providerAddr` has to validate
// - first consumer because it is a consumer validator in this chain,
// - third consumer because it opted in
expectedChains := []string{consumerIDs[0], consumerIDs[2]}

res, err := pk.QueryConsumerChainsValidatorHasToValidate(ctx, &req)
require.NoError(t, err)
Expand Down Expand Up @@ -606,7 +613,10 @@ func TestQueryConsumerChains(t *testing.T) {
Metadata: types.ConsumerMetadata{Name: chainID},
}
pk.SetConsumerPhase(ctx, cID, c.Phase)
pk.SetConsumerMetadata(ctx, cID, c.Metadata)
err := pk.SetConsumerMetadata(ctx, cID, c.Metadata)
require.NoError(t, err)
err = pk.SetConsumerPowerShapingParameters(ctx, cID, types.PowerShapingParameters{})
require.NoError(t, err)
pk.SetConsumerChainId(ctx, cID, chainID)

consumerIds[i] = cID
Expand Down Expand Up @@ -644,7 +654,7 @@ func TestQueryConsumerChains(t *testing.T) {
name: "expect initialized consumers when phase is set to Initialized",
setup: func(ctx sdk.Context, pk keeper.Keeper) {
consumers[1].Phase = types.ConsumerPhase_CONSUMER_PHASE_INITIALIZED
err := pk.AppendConsumerToBeLaunchedOnSpawnTime(ctx, consumerIds[1], time.Now())
err := pk.AppendConsumerToBeLaunched(ctx, consumerIds[1], time.Now())
require.NoError(t, err)
pk.SetConsumerPhase(ctx, consumerIds[1], types.ConsumerPhase_CONSUMER_PHASE_INITIALIZED)
},
Expand Down
78 changes: 1 addition & 77 deletions x/ccv/provider/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -694,10 +694,7 @@ func (k Keeper) GetAllActiveConsumerIds(ctx sdk.Context) []string {
consumerIds := []string{}
for i := uint64(0); i < latestConsumerId; i++ {
consumerId := fmt.Sprintf("%d", i)
phase := k.GetConsumerPhase(ctx, consumerId)
if phase != types.ConsumerPhase_CONSUMER_PHASE_REGISTERED &&
phase != types.ConsumerPhase_CONSUMER_PHASE_INITIALIZED &&
phase != types.ConsumerPhase_CONSUMER_PHASE_LAUNCHED {
if !k.IsConsumerActive(ctx, consumerId) {
continue
}
consumerIds = append(consumerIds, consumerId)
Expand All @@ -706,29 +703,6 @@ func (k Keeper) GetAllActiveConsumerIds(ctx sdk.Context) []string {
return consumerIds
}

// GetTopN returns N if chain `consumerId` has a top N associated, and 0 otherwise.
func (k Keeper) GetTopN(
ctx sdk.Context,
consumerId string,
) uint32 {
powerShapingParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId)
if err != nil {
k.Logger(ctx).Error("could not retrieve power shaping parameters", "error", err)
}

return powerShapingParameters.Top_N
}

// IsTopN returns true if chain with `consumerId` is a Top-N chain (i.e., enforces at least one validator to validate chain `consumerId`)
func (k Keeper) IsTopN(ctx sdk.Context, consumerId string) bool {
return k.GetTopN(ctx, consumerId) > 0
}

// IsOptIn returns true if chain with `consumerId` is an Opt-In chain (i.e., no validator is forced to validate chain `consumerId`)
func (k Keeper) IsOptIn(ctx sdk.Context, consumerId string) bool {
return k.GetTopN(ctx, consumerId) == 0
}

func (k Keeper) SetOptedIn(
ctx sdk.Context,
consumerId string,
Expand Down Expand Up @@ -865,30 +839,6 @@ func (k Keeper) DeleteConsumerCommissionRate(
store.Delete(types.ConsumerCommissionRateKey(consumerId, providerAddr))
}

// GetValidatorsPowerCap returns the associated power cap of chain with `consumerId` and 0 if no power cap association is found
func (k Keeper) GetValidatorsPowerCap(
ctx sdk.Context,
consumerId string,
) uint32 {
powerShapingParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId)
if err != nil {
k.Logger(ctx).Error("could not retrieve power shaping parameters", "error", err)
}
return powerShapingParameters.ValidatorsPowerCap
}

// GetValidatorSetCap returns the associated validator set cap of chain with `consumerId` and 0 if no set cap association is found
func (k Keeper) GetValidatorSetCap(
ctx sdk.Context,
consumerId string,
) uint32 {
powerShapingParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId)
if err != nil {
k.Logger(ctx).Error("could not retrieve power shaping parameters", "error", err)
}
return powerShapingParameters.ValidatorSetCap
}

// SetAllowlist allowlists validator with `providerAddr` address on chain `consumerId`
func (k Keeper) SetAllowlist(
ctx sdk.Context,
Expand Down Expand Up @@ -1054,32 +1004,6 @@ func (k Keeper) DeleteMinimumPowerInTopN(
store.Delete(types.MinimumPowerInTopNKey(consumerId))
}

// GetMinStake returns the minimum stake required for a validator to validate
// a given consumer chain. Returns 0 if min stake is not found for this consumer id.
func (k Keeper) GetMinStake(
ctx sdk.Context,
consumerId string,
) uint64 {
powerShapingParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId)
if err != nil {
k.Logger(ctx).Error("could not retrieve power shaping parameters", "error", err)
}
return powerShapingParameters.MinStake
}

// AllowsInactiveValidators returns whether inactive validators are allowed to validate
// a given consumer chain. Returns false if flag on inactive validators is not found for this consumer id.
func (k Keeper) AllowsInactiveValidators(
ctx sdk.Context,
consumerId string,
) bool {
powerShapingParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId)
if err != nil {
k.Logger(ctx).Error("could not retrieve power shaping parameters", "error", err)
}
return powerShapingParameters.AllowInactiveVals
}

func (k Keeper) UnbondingCanComplete(ctx sdk.Context, id uint64) error {
return k.stakingKeeper.UnbondingCanComplete(ctx, id)
}
Expand Down
Loading

0 comments on commit 1135770

Please sign in to comment.