Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Fix has-to-validate query #1823

Merged
merged 6 commits into from
Apr 30, 2024
Merged

fix: Fix has-to-validate query #1823

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bf3a50e
Fix has-to-validate query
p-offtermatt Apr 26, 2024
33e4439
Merge branch 'feat/partial-set-security' into ph/fix-has-to-validate
p-offtermatt Apr 26, 2024
de6189a
Flip comparison sign for checking minPower
p-offtermatt Apr 26, 2024
37c8f8d
Regenerate traces
p-offtermatt Apr 26, 2024
062a574
Remove unnecessary print
p-offtermatt Apr 29, 2024
3ae1561
Address comments
p-offtermatt Apr 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions tests/e2e/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
ConsumerPendingPacketQueueSize *uint // Only relevant to consumer chains
RegisteredConsumerRewardDenoms *[]string
ClientsFrozenHeights *map[string]clienttypes.Height
HasToValidate *map[ValidatorID][]ChainID // only relevant to provider chain
}

type Proposal interface {
Expand Down Expand Up @@ -180,6 +181,14 @@
chainState.ClientsFrozenHeights = &chainClientsFrozenHeights
}

if modelState.HasToValidate != nil {
hasToValidate := map[ValidatorID][]ChainID{}
for validatorId := range *modelState.HasToValidate {
hasToValidate[validatorId] = tr.getHasToValidate(validatorId)
}
Dismissed Show dismissed Hide dismissed
chainState.HasToValidate = &hasToValidate
}

if modelState.ConsumerPendingPacketQueueSize != nil {
pendingPacketQueueSize := tr.getPendingPacketQueueSize(chain)
chainState.ConsumerPendingPacketQueueSize = &pendingPacketQueueSize
Expand Down Expand Up @@ -833,6 +842,30 @@
return clienttypes.Height{RevisionHeight: uint64(revHeight), RevisionNumber: uint64(revNumber)}
}

func (tr TestConfig) getHasToValidate(
validatorId ValidatorID,
) []ChainID {
//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
bz, err := exec.Command("docker", "exec", tr.containerConfig.InstanceName, tr.chainConfigs[ChainID("provi")].BinaryName,
"query", "provider", "has-to-validate",
tr.validatorConfigs[validatorId].ValconsAddress,
`--node`, tr.getQueryNode(ChainID("provi")),
`-o`, `json`,
).CombinedOutput()
if err != nil {
log.Fatal(err, "\n", string(bz))
}
log.Println("has-to-validate response: ", string(bz))
p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved

arr := gjson.Get(string(bz), "consumer_chain_ids").Array()
chains := []ChainID{}
for _, c := range arr {
chains = append(chains, ChainID(c.String()))
}

return chains
}

func (tc TestConfig) getTrustedHeight(
chain ChainID,
clientID string,
Expand Down
134 changes: 130 additions & 4 deletions tests/e2e/steps_partial_set_security.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ func stepsOptInChain() []Step {
Status: "PROPOSAL_STATUS_VOTING_PERIOD",
},
},
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {},
ValidatorID("bob"): {},
ValidatorID("carol"): {},
},
},
},
},
Expand All @@ -56,14 +61,30 @@ func stepsOptInChain() []Step {
Chain: ChainID("consu"),
Validator: ValidatorID("alice"),
},
State: State{},
State: State{
ChainID("provi"): ChainState{
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {}, // chain is not running yet
ValidatorID("bob"): {},
ValidatorID("carol"): {},
},
},
},
},
{
Action: OptInAction{
Chain: ChainID("consu"),
Validator: ValidatorID("bob"),
},
State: State{},
State: State{
ChainID("provi"): ChainState{
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {},
ValidatorID("bob"): {},
ValidatorID("carol"): {},
},
},
},
},
{
Action: VoteGovProposalAction{
Expand Down Expand Up @@ -149,6 +170,13 @@ func stepsOptInChain() []Step {
ValidatorID("carol"): 0,
},
},
ChainID("provi"): ChainState{
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"},
ValidatorID("bob"): {"consu"},
ValidatorID("carol"): {"consu"},
},
},
},
},
{
Expand Down Expand Up @@ -179,6 +207,13 @@ func stepsOptInChain() []Step {
ValidatorID("carol"): 300,
},
},
ChainID("provi"): ChainState{
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"},
ValidatorID("bob"): {"consu"},
ValidatorID("carol"): {"consu"},
},
},
},
},
{
Expand Down Expand Up @@ -213,6 +248,13 @@ func stepsOptInChain() []Step {
ValidatorID("carol"): 300,
},
},
ChainID("provi"): ChainState{
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"},
ValidatorID("bob"): {},
ValidatorID("carol"): {"consu"},
},
},
},
},
{
Expand All @@ -229,6 +271,11 @@ func stepsOptInChain() []Step {
ValidatorID("bob"): 0,
ValidatorID("carol"): 300,
},
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"},
ValidatorID("bob"): {"consu"}, // but has to validate is true because bob opted in on the provider
ValidatorID("carol"): {"consu"},
},
},
},
},
Expand Down Expand Up @@ -271,6 +318,11 @@ func stepsOptInChain() []Step {
ValidatorID("bob"): 200,
ValidatorID("carol"): 300,
},
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"},
ValidatorID("bob"): {"consu"},
ValidatorID("carol"): {"consu"},
},
},
},
},
Expand All @@ -290,6 +342,11 @@ func stepsOptInChain() []Step {
ValidatorID("bob"): 200,
ValidatorID("carol"): 300,
},
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"}, // but alice still is in the consumer valset so has to validate
ValidatorID("bob"): {"consu"},
ValidatorID("carol"): {"consu"},
},
},
},
},
Expand All @@ -309,6 +366,11 @@ func stepsOptInChain() []Step {
ValidatorID("bob"): 200,
ValidatorID("carol"): 300,
},
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"},
ValidatorID("bob"): {"consu"},
ValidatorID("carol"): {"consu"},
},
},
ChainID("consu"): ChainState{
ValPowers: &map[ValidatorID]uint{
Expand All @@ -334,6 +396,11 @@ func stepsOptInChain() []Step {
ValidatorID("bob"): 200,
ValidatorID("carol"): 300,
},
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"},
ValidatorID("bob"): {"consu"},
ValidatorID("carol"): {"consu"},
},
},
// still 0 power on the consumer
ChainID("consu"): ChainState{
Expand Down Expand Up @@ -385,6 +452,11 @@ func stepsOptInChain() []Step {
ValidatorID("bob"): 200,
ValidatorID("carol"): 300,
},
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"},
ValidatorID("bob"): {"consu"},
ValidatorID("carol"): {"consu"},
},
},
},
},
Expand All @@ -404,6 +476,11 @@ func stepsOptInChain() []Step {
ValidatorID("bob"): 200,
ValidatorID("carol"): 300,
},
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"},
ValidatorID("bob"): {"consu"},
ValidatorID("carol"): {"consu"},
},
},
},
},
Expand All @@ -430,6 +507,11 @@ func stepsOptInChain() []Step {
ValidatorID("bob"): 200,
ValidatorID("carol"): 300,
},
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"},
ValidatorID("bob"): {"consu"},
ValidatorID("carol"): {"consu"},
},
},
},
},
Expand Down Expand Up @@ -582,6 +664,13 @@ func stepsTopNChain() []Step {
ValidatorID("carol"): 500,
},
},
ChainID("provi"): ChainState{
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"},
ValidatorID("bob"): {"consu"},
ValidatorID("carol"): {"consu"},
},
},
},
},
{
Expand Down Expand Up @@ -614,7 +703,15 @@ func stepsTopNChain() []Step {
Chain: ChainID("consu"),
Validator: ValidatorID("bob"),
},
State: State{},
State: State{
ChainID("provi"): ChainState{
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"},
ValidatorID("bob"): {"consu"},
ValidatorID("carol"): {"consu"},
},
},
},
},
{
// opting out "bob" or "carol" does not work because they belong to the Top N validators
Expand All @@ -632,6 +729,13 @@ func stepsTopNChain() []Step {
ValidatorID("carol"): 500,
},
},
ChainID("provi"): ChainState{
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"},
ValidatorID("bob"): {"consu"},
ValidatorID("carol"): {"consu"},
},
},
},
},
{
Expand All @@ -657,6 +761,13 @@ func stepsTopNChain() []Step {
ValidatorID("carol"): 500,
},
},
ChainID("provi"): ChainState{
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {}, // alice has opted out and the epoch is over, so definitely doesnt have to validate anymore
p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved
ValidatorID("bob"): {"consu"},
ValidatorID("carol"): {"consu"},
},
},
},
},
// opt alice back in
Expand All @@ -665,7 +776,15 @@ func stepsTopNChain() []Step {
Chain: ChainID("consu"),
Validator: ValidatorID("alice"),
},
State: State{},
State: State{
ChainID("provi"): ChainState{
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"}, // alice has to validate again
ValidatorID("bob"): {"consu"},
ValidatorID("carol"): {"consu"},
},
},
},
},
{
Action: RelayPacketsAction{
Expand All @@ -683,6 +802,13 @@ func stepsTopNChain() []Step {
ValidatorID("carol"): 500,
},
},
ChainID("provi"): ChainState{
HasToValidate: &map[ValidatorID][]ChainID{
ValidatorID("alice"): {"consu"},
ValidatorID("bob"): {"consu"},
ValidatorID("carol"): {"consu"},
},
},
},
},
{
Expand Down
28 changes: 9 additions & 19 deletions x/ccv/provider/keeper/grpc_query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,31 +270,21 @@ func (k Keeper) QueryConsumerChainsValidatorHasToValidate(goCtx context.Context,

ctx := sdk.UnwrapSDKContext(goCtx)

provAddr := types.NewProviderConsAddress(consAddr)

// get all the consumer chains for which the validator is either already
// opted-in, currently a consumer validator or if its voting power is within the TopN validators
consumersToValidate := []string{}
for _, consumer := range k.GetAllConsumerChains(ctx) {
chainID := consumer.ChainId
provAddr := types.NewProviderConsAddress(consAddr)
if !k.IsOptedIn(ctx, chainID, provAddr) && !k.IsConsumerValidator(ctx, chainID, provAddr) {
// check that the validator voting power isn't in the TopN
if topN, found := k.GetTopN(ctx, chainID); found && topN > 0 {
p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved
val, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, consAddr)
if !found {
return nil, status.Error(codes.InvalidArgument, "invalid provider address")
}
power := k.stakingKeeper.GetLastValidatorPower(ctx, val.GetOperator())
minPowerToOptIn := k.ComputeMinPowerToOptIn(ctx, chainID, k.stakingKeeper.GetLastValidators(ctx), topN)

// Check if the validator's voting power is smaller
// than the minimum and hence not automatically opted in
if power < minPowerToOptIn {
continue
}
}
}

consumersToValidate = append(consumersToValidate, chainID)
hasToValidate, err := k.HasToValidate(ctx, provAddr, chainID)
if err != nil {
return nil, err
}
if hasToValidate {
consumersToValidate = append(consumersToValidate, chainID)
}
}

return &types.QueryConsumerChainsValidatorHasToValidateResponse{
Expand Down
Loading
Loading