Skip to content

Commit

Permalink
feat!: Rewire dependencies on the staking module (#2056)
Browse files Browse the repository at this point in the history
* Change wiring for mint and gov to use ProviderKeeper instead of StakingKeeper

* Add test for IterateBondedValidatorsByPower

* Rewire GovKeeper

* Fix docstrings

* Test other modified functions

* Start writing some testing scenarios

* Add TotalBondedTokens to expected staking keeper interface
  • Loading branch information
p-offtermatt authored Jul 18, 2024
1 parent cb44240 commit 4caa577
Show file tree
Hide file tree
Showing 7 changed files with 384 additions and 23 deletions.
52 changes: 29 additions & 23 deletions app/provider/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,15 +373,6 @@ func New(
authcodec.NewBech32Codec(sdk.GetConfig().GetBech32ValidatorAddrPrefix()),
authcodec.NewBech32Codec(sdk.GetConfig().GetBech32ConsensusAddrPrefix()),
)
app.MintKeeper = mintkeeper.NewKeeper(
appCodec,
runtime.NewKVStoreService(keys[minttypes.StoreKey]),
app.StakingKeeper,
app.AccountKeeper,
app.BankKeeper,
authtypes.FeeCollectorName,
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)
app.DistrKeeper = distrkeeper.NewKeeper(
appCodec,
runtime.NewKVStoreService(keys[distrtypes.StoreKey]),
Expand Down Expand Up @@ -456,19 +447,6 @@ func New(
runtime.ProvideCometInfoService(),
)

govConfig := govtypes.DefaultConfig()
app.GovKeeper = govkeeper.NewKeeper(
appCodec,
runtime.NewKVStoreService(keys[govtypes.StoreKey]),
app.AccountKeeper,
app.BankKeeper,
app.StakingKeeper,
app.DistrKeeper,
app.MsgServiceRouter(),
govConfig,
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)

app.ProviderKeeper = ibcproviderkeeper.NewKeeper(
appCodec,
keys[providertypes.StoreKey],
Expand All @@ -483,13 +461,41 @@ func New(
app.AccountKeeper,
app.DistrKeeper,
app.BankKeeper,
*app.GovKeeper,
govkeeper.Keeper{}, // will be set after the GovKeeper is created
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
authcodec.NewBech32Codec(sdk.GetConfig().GetBech32ValidatorAddrPrefix()),
authcodec.NewBech32Codec(sdk.GetConfig().GetBech32ConsensusAddrPrefix()),
authtypes.FeeCollectorName,
)

govConfig := govtypes.DefaultConfig()
app.GovKeeper = govkeeper.NewKeeper(
appCodec,
runtime.NewKVStoreService(keys[govtypes.StoreKey]),
app.AccountKeeper,
app.BankKeeper,
app.ProviderKeeper,
app.DistrKeeper,
app.MsgServiceRouter(),
govConfig,
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)

// set the GovKeeper in the ProviderKeeper
app.ProviderKeeper.SetGovKeeper(*app.GovKeeper)

app.MintKeeper = mintkeeper.NewKeeper(
appCodec,
runtime.NewKVStoreService(keys[minttypes.StoreKey]),
// use the ProviderKeeper as StakingKeeper for mint
// because minting should be based on the consensus-active validators
app.ProviderKeeper,
app.AccountKeeper,
app.BankKeeper,
authtypes.FeeCollectorName,
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)

// gov router must be set after the provider keeper is created
// otherwise the provider keeper will not be able to handle proposals (will be nil)
govRouter := govv1beta1.NewRouter()
Expand Down
40 changes: 40 additions & 0 deletions docs/docs/adrs/adr-017-allowing-inactive-validators.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,46 @@ disables the main feature described in this ADR.

Additional risk mitigations are to increase the active set size slowly, and to monitor the effects on the network closely. For the first iteration, we propose to increase the active set size to 200 validators (while keeping the consensus validators to 180), thus letting the 20 validators with the most stake outside of the active set validate on consumer chains.

## Testing Scenarios

In the following,
- bonded validators refers to all validators that have bonded stake,
- active validators refers to the validators that take part in consensus,
- inactive validators refers to bonded validators that are not active validators.

### Scenario 1: Inactive validators should not be considered by governance

Inactive validators should not be considered for the purpose of governance.
In particular, the governance module should not allow inactive validators to vote on proposals,
and the quorum depends only on the stake bonded by active validators.

This can be tested by creating a governance proposal, then trying to vote on it with inactive validators.
The proposal should not pass.
Afterwards, we create another proposal and vote on it with active validators, too.
Then, the proposal should pass.

### Scenario 2: Inactive validators should not get rewards from the provider chain

Inactive validators should not get rewards from the provider chain.

This can be tested by starting a provider chain with inactive validators and checking the rewards of inactive validators.

### Scenario 3: Inactive validators should get rewards from consumer chains

An inactive validator that is validating on a consumer chain should receive rewards in the consumer chain token.

### Scenario 4: Inactive validators should not get slashed/jailed for downtime on the provider chain

This can be tested by having an inactive validator go offline on the provider chain for long enough to accrue downtime.
The validator should be neither slashed nor jailed for downtime.

### Scenario 5: Inactive validators *should* get jailed for downtime on the provider chain

This can be tested by having an inactive validator go offline on a consumer chain for long enough to accrue downtime.
The consumer chain should send a SlashPacket to the provider chain, which should jail the validator.

* **Mint**:

## Consequences

### Positive
Expand Down
88 changes: 88 additions & 0 deletions testutil/keeper/mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions x/ccv/provider/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ func (k Keeper) mustValidateFields() {
// ccv.PanicIfZeroOrNil(k.govKeeper, "govKeeper") // 17
}

func (k *Keeper) SetGovKeeper(govKeeper govkeeper.Keeper) {
k.govKeeper = govKeeper
}

// Logger returns a module-specific logger.
func (k Keeper) Logger(ctx context.Context) log.Logger {
sdkCtx := sdk.UnwrapSDKContext(ctx)
Expand Down
80 changes: 80 additions & 0 deletions x/ccv/provider/keeper/staking_keeper_interface.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package keeper

import (
"context"

"cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)

// IterateBondedValidatorsByPower iterates over the consensus-active validators by power.
// The same as IterateBondedValidatorsByPower in the StakingKeeper,
// but only returns the first MaxProviderConsensusValidators validators.
// This is used to implement the interface of the staking keeper to interface with
// modules that need to reference the consensus-active validators.
func (k Keeper) IterateBondedValidatorsByPower(ctx context.Context, fn func(index int64, validator stakingtypes.ValidatorI) (stop bool)) error {
maxProviderConsensusVals := k.GetMaxProviderConsensusValidators(sdk.UnwrapSDKContext(ctx))
counter := int64(0)
return k.stakingKeeper.IterateBondedValidatorsByPower(ctx, func(index int64, validator stakingtypes.ValidatorI) (stop bool) {
if counter >= maxProviderConsensusVals {
return true
}
counter++
return fn(index, validator)
})
}

// TotalBondedTokens gets the amount of tokens of the consensus-active validators.
// The same as TotalBondedTokens in the StakingKeeper, but only counts bonded tokens
// of the first MaxProviderConsensusValidators bonded validators.
// This is used to implement the interface of the staking keeper to interface with
// modules that need to reference the consensus-active validators.
func (k Keeper) TotalBondedTokens(ctx context.Context) (math.Int, error) {
// iterate through the bonded validators
totalBondedTokens := math.ZeroInt()

k.IterateBondedValidatorsByPower(ctx, func(_ int64, validator stakingtypes.ValidatorI) (stop bool) {
tokens := validator.GetTokens()
totalBondedTokens = totalBondedTokens.Add(tokens)
return false
})

return totalBondedTokens, nil
}

// IterateDelegations is the same as IterateDelegations in the StakingKeeper.
// Necessary to implement the interface of the staking keeper to interface with
// other modules.
func (k Keeper) IterateDelegations(ctx context.Context, delegator sdk.AccAddress, fn func(index int64, delegation stakingtypes.DelegationI) (stop bool)) error {
return k.stakingKeeper.IterateDelegations(ctx, delegator, fn)
}

// StakingTokenSupply is the same as StakingTotalSupply in the StakingKeeper.
// Necessary to implement the interface of the staking keeper to interface with
// other modules.
func (k Keeper) StakingTokenSupply(ctx context.Context) (math.Int, error) {
return k.stakingKeeper.StakingTokenSupply(ctx)
}

// BondedRatio gets the ratio of tokens staked to validators active in the consensus
// to the total supply of tokens.
// Same as BondedRatio in the StakingKeeper, but only counts
// tokens of the first MaxProviderConsensusValidators bonded validators.
func (k Keeper) BondedRatio(ctx context.Context) (math.LegacyDec, error) {
totalSupply, err := k.StakingTokenSupply(ctx)
if err != nil {
return math.LegacyZeroDec(), err
}

bondedTokens, err := k.TotalBondedTokens(ctx)
if err != nil {
return math.LegacyZeroDec(), err
}

if !totalSupply.IsPositive() {
return math.LegacyZeroDec(), nil
}

return math.LegacyNewDecFromInt(bondedTokens).QuoInt(totalSupply), nil
}
Loading

0 comments on commit 4caa577

Please sign in to comment.