Skip to content

Remove residual ADR-017 plumbing: MaxProviderConsensusValidators and ComputeNextValidators #60

Description

@tbruyelle

Context

VAAS runs "all active validators validate all consumers" — PSS (Partial Set Security, per-consumer opt-in/out) is disabled forever, and its remnant in ComputeNextValidators is a dead always-true filter predicate.

MaxProviderConsensusValidators is not PSS — it is the residual implementation of a different, already-removed ICS feature: inactive provider validators (ADR-017). DESIGN_RATIONALE.md explicitly lists it as removed (line 54: "inactive-validator participation (ADR-017)"; line 134: "Inactive provider validators participating in consumer security (ADR-017)"). The parameter and its plumbing were left behind, so the code contradicts the stated design — the Guiding Principles section (lines 23-25) still says the consumer set is "capped at MaxProviderConsensusValidators". The cap is also inert today: the default is 180 and nothing in this repo lowers staking MaxValidators below that, so the truncation never fires.

Decision: finish removing ADR-017 by deleting the leftover cap and its plumbing. The provider consensus set becomes the full staking bonded set, and the consumer set follows it. Because ComputeNextValidators's only remaining logic (sort → truncate → map-all) exists solely to serve the cap, removing the cap lets it go too.

The provider module is a fresh start (module.go: "Migrations are not registered", ConsensusVersion() == 1), so no upgrade/migration handler is needed.

Changes

Proto (regenerate with make proto-gen, needs Docker)

  • proto/vaas/provider/v1/provider.proto: delete int64 max_provider_consensus_validators = 5;. No reserved needed — fresh start, no deployed chain, so field-number reuse is not a concern. Regenerate x/vaas/provider/types/provider.pb.go.

Provider types — x/vaas/provider/types/params.go

  • Remove the DefaultMaxProviderConsensusValidators = 180 const.
  • Drop the maxProviderConsensusValidators int64 parameter from NewParams (and the struct field assignment), and the corresponding argument in DefaultParams().
  • Remove the ValidatePositiveInt64(p.MaxProviderConsensusValidators) check in Validate().

Provider keeper

  • keeper/params.go: delete GetMaxProviderConsensusValidators.
  • keeper/validator_set_update.go:
    • Delete ComputeNextValidators.
    • Delete GetLastProviderConsensusActiveValidators (no callers).
    • In ComputeConsumerNextValSet, replace the ComputeNextValidators call with a direct FilterValidators(ctx, consumerId, bondedValidators, <always-true predicate>). Keep FilterValidators unchanged — still used by key_assignment_test.go:790 with a real predicate.
    • Remove the now-unused sort import (its only use was in ComputeNextValidators).
  • keeper/relay.go ProviderValidatorUpdates: drop maxValidators := min(k.GetMaxProviderConsensusValidators(ctx), …) and iterate the full bondedValidators (GetBondedValidatorsByPower already bounds to staking MaxValidators). Update the doc comment.
  • keeper/genesis.go InitGenesisValUpdates: remove the truncation block (maxVals := …; valSet = valSet[:maxVals]); iterate the full valSet. Update the comment. Also fix the stale comment in module.go:124-125.
  • keeper/staking_keeper_interface.go: this file existed only to expose the capped staking view for the inactive-validators feature and is not wired into mint/distribution (the app injects app.StakingKeeper directly). Remove IterateBondedValidatorsByPower, TotalBondedTokens, BondedRatio, and the now-orphan StakingTokenSupply/IterateDelegations passthroughs. If the file ends up empty, delete it. No interface assertion depends on these (only QueryServer is asserted, in grpc_query.go).
  • keeper/invariants.go: remove MaxProviderConsensusValidatorsInvariant and StakingKeeperEquivalenceInvariant plus their RegisterInvariants registrations; keep FeePoolSharesConsistencyInvariant. Drop the now-unused stakingtypes import (strings stays — used by the fee-pool invariant).

Tests

  • types/params_test.go, types/genesis_test.go, keeper/params_test.go: drop the 180 arg from every NewParams(...) call and any assertion on the removed getter.
  • keeper/validator_set_update_test.go: remove TestComputeNextValidators (and any GetLastProviderConsensusActiveValidators test); trim MaxValidators mock expectations that existed only for them. Keep tests for DiffValidators, FilterValidators, ComputeConsumerNextValSet.
  • keeper/genesis_test.go: fix the comment at line 167 and confirm the InitGenesisValUpdates test passes with no truncation.

Docs

  • DESIGN_RATIONALE.md:23-25 (Guiding Principles): remove the "capped at MaxProviderConsensusValidators" clause — the consumer set is simply the full active provider set. The "Removed" sections (lines 50-59, 128-134) already correctly document ADR-017 as removed; leave them as-is.
  • docs/consumer-transition.md:54: drop the "provider's MaxProviderConsensusValidators cap" wording.
  • REWRITE_SUMMARY.md:84,196: update the ComputeNextValidators / "up to MaxProviderConsensusValidators" notes to reflect the full-set behavior.

Notes / risks

  • Behavioral change: a governance-set cap below MaxValidators (inactive validators) is no longer possible. This is the intended outcome (ADR-017 is already declared removed in DESIGN_RATIONALE.md).
  • Fresh module, no migration/upgrade handler needed.

Verification

  1. make proto-gen then git diff --stat on provider.pb.go (only field 5 gone).
  2. make build — no unused-import / signature errors.
  3. go test ./x/vaas/provider/... (targeted), then make test.
  4. make lint.
  5. Optional end-to-end confidence: make localnet-start, launch a consumer, confirm the consumer receives the full provider validator set at the epoch boundary and the diff/snapshot VSC path still works.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions