Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
p-offtermatt committed Aug 5, 2024
1 parent 484cb7d commit 930a482
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 13 deletions.
11 changes: 6 additions & 5 deletions app/provider/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,10 +350,7 @@ func New(
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)

// Remove the ConsumerRewardsPool from the group of blocked recipient addresses in bank
// this is required for the provider chain to be able to receive tokens from
// the consumer chain
bankBlockedAddrs := BankBlockedAddrs(app)
bankBlockedAddrs := ComputeBankBlockedAddrs(app)

app.BankKeeper = bankkeeper.NewBaseKeeper(
appCodec,
Expand Down Expand Up @@ -802,7 +799,11 @@ func New(
return app
}

func BankBlockedAddrs(app *App) map[string]bool {
// Computes the addresses that should be blocked by the Bank module.
// We remove the ConsumerRewardsPool from the group of blocked recipient addresses in bank.
// This is required for the provider chain to be able to receive tokens from
// the consumer chain
func ComputeBankBlockedAddrs(app *App) map[string]bool {
bankBlockedAddrs := app.ModuleAccountAddrs()
delete(bankBlockedAddrs, authtypes.NewModuleAddress(
providertypes.ConsumerRewardsPool).String())
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/adrs/adr-017-allowing-inactive-validators.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ achieved without further changes to staking or reward distributions, because sim

The following changes to the state are required:

* Introduce the `MaxProviderConsensusValidators` parameter to the provider module, which is the number of validators that the provider module will send to consumer chains.
* Introduce the `MaxProviderConsensusValidators` parameter to the provider module, which is the number of validators that the provider module will send to the consensus engine.
* Store the provider consensus validator set in the provider module state under the `LastProviderConsensusValsPrefix` key. This is the last set of validators that the provider sent to the consensus engine. This is needed to compute the ValUpdates to send to the consensus engine (by diffing the current set with this last sent set).
* Increase the `MaxValidators` parameter of the staking module to the desired size of the potential validator
set of consumer chains.
Expand Down
2 changes: 1 addition & 1 deletion x/ccv/provider/keeper/legacy_proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func (k Keeper) HandleLegacyConsumerModificationProposal(ctx sdk.Context, p *typ
if p.Top_N != oldTopN {
if p.Top_N > 0 {
// if the chain receives a non-zero top N value, store the minimum power in the top N
activeValidators, err := k.GetLastActiveBondedValidators(ctx)
activeValidators, err := k.GetLastProviderConsensusActiveValidators(ctx)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion x/ccv/provider/keeper/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func (k Keeper) MakeConsumerGenesis(
// we do not want to base the power calculation for the top N
// on inactive validators, too, since the top N will be a percentage of the active set power
// otherwise, it could be that inactive validators are forced to validate
activeValidators, err := k.GetLastActiveBondedValidators(ctx)
activeValidators, err := k.GetLastProviderConsensusActiveValidators(ctx)
if err != nil {
return gen, nil, errorsmod.Wrapf(stakingtypes.ErrNoValidatorFound, "error getting last active bonded validators: %s", err)
}
Expand Down
5 changes: 1 addition & 4 deletions x/ccv/provider/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet) err
// the Validator Set Update sub-protocol
func (k Keeper) EndBlockVSU(ctx sdk.Context) ([]abci.ValidatorUpdate, error) {
// logic to update the provider consensus validator set.
// Important: must be called before the rest of EndBlockVSU, because
// we need to know the updated provider validator set
// to compute the minimum power in the top N
valUpdates := k.ProviderValidatorUpdates(ctx)

if k.BlocksUntilNextEpoch(ctx) == 0 {
Expand Down Expand Up @@ -205,7 +202,7 @@ func (k Keeper) QueueVSCPackets(ctx sdk.Context) {
if topN > 0 {
// in a Top-N chain, we automatically opt in all validators that belong to the top N
// of the active validators
activeValidators, err := k.GetLastActiveBondedValidators(ctx)
activeValidators, err := k.GetLastProviderConsensusActiveValidators(ctx)
if err != nil {
// something must be broken in the bonded validators, so we have to panic since there is no realistic way to proceed
panic(fmt.Errorf("failed to get active validators: %w", err))

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
Expand Down
4 changes: 3 additions & 1 deletion x/ccv/provider/keeper/validator_set_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,9 @@ func (k Keeper) GetLastBondedValidators(ctx sdk.Context) ([]stakingtypes.Validat
return ccv.GetLastBondedValidatorsUtil(ctx, k.stakingKeeper, k.Logger(ctx), maxVals)
}

func (k Keeper) GetLastActiveBondedValidators(ctx sdk.Context) ([]stakingtypes.Validator, error) {
// GetLastProviderConsensusActiveValidators returns the `MaxProviderConsensusValidators` many validators with the largest powers
// from the last bonded validators in the staking module.
func (k Keeper) GetLastProviderConsensusActiveValidators(ctx sdk.Context) ([]stakingtypes.Validator, error) {
maxVals := k.GetMaxProviderConsensusValidators(ctx)
return ccv.GetLastBondedValidatorsUtil(ctx, k.stakingKeeper, k.Logger(ctx), uint32(maxVals))
}

0 comments on commit 930a482

Please sign in to comment.