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.

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

feat!: Rewire dependencies on the staking module #2056

Merged
merged 7 commits into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
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.StakingKeeper,
p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved
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
73 changes: 73 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
81 changes: 81 additions & 0 deletions x/ccv/provider/keeper/staking_keeper_interface.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package keeper

import (
"context"

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

// 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 {
p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved
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)
})
}

// Gets the amount of bonded tokens, which is equal
// to the amount of tokens of the consensus-active validators.
// The same as TotalBondedTokens, but only counts
// tokens of 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) TotalBondedTokens(ctx context.Context) (math.Int, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at TotalBondedTokens it's not fully clear to me that bonded tokens does not include the tokens of non-consensus-active validators.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the docstring, does that help?
Or are you talking about the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are some references for how the original TotalBondedTokens function behaves:

It just gets the tokens in the bonded pool here https://github.com/cosmos/cosmos-sdk/blob/a32186608aab0bd436049377ddb34f90006fcbf7/x/staking/keeper/pool.go#L80

the bonded pool contains the tokens from bonded validators. e.g. see here that tokens are sent to the bonded pool when someone delegates to an active validator https://github.com/cosmos/cosmos-sdk/blob/a32186608aab0bd436049377ddb34f90006fcbf7/x/staking/keeper/delegation.go#L913,
and how tokens are transferred between bonded/not-bonded pool on valstatechanges https://github.com/cosmos/cosmos-sdk/blob/a32186608aab0bd436049377ddb34f90006fcbf7/x/staking/keeper/val_state_change.go#L251

// 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
}

// 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)
}

// 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)
}

// 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
}
96 changes: 96 additions & 0 deletions x/ccv/provider/keeper/staking_keeper_interface_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package keeper_test

import (
"sort"
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/staking/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
testkeeper "github.com/cosmos/interchain-security/v5/testutil/keeper"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/require"
)

// Tests that IterateBondedValidatorsByPower returns the correct validators,
p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved
// namely the ones with most power, and stops when the max number of validators is reached.
func TestIterateBondedValidatorsByPower(t *testing.T) {
tests := []struct {
name string
maxValidators int64
powers []int64
expectedPowers []int64
}{
{
name: "no validators",
maxValidators: 5,
powers: []int64{},
expectedPowers: []int64{},
},
{
name: "validators less than max",
maxValidators: 5,
powers: []int64{10, 20, 30},
expectedPowers: []int64{10, 20, 30}, // get all validators back
},
{
name: "validators more than max",
maxValidators: 2,
powers: []int64{10, 20, 30},
expectedPowers: []int64{30, 20}, // get the top 2 validators back
},
{
name: "validators equal to max",
maxValidators: 3,
powers: []int64{10, 20, 30},
expectedPowers: []int64{30, 20, 10}, // get all validators back
},
{
name: "validators with equal powers",
maxValidators: 3,
powers: []int64{10, 10, 10, 20, 20, 30, 30},
expectedPowers: []int64{30, 30, 20}, // get the top 3 validators back
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
defer ctrl.Finish()

counter := int64(0)
vals, _ := createStakingValidatorsAndMocks(ctx, mocks, tt.powers...)

params := providerKeeper.GetParams(ctx)
params.MaxProviderConsensusValidators = tt.maxValidators
providerKeeper.SetParams(ctx, params)

// sort vals by descending number of tokens
sort.Slice(
vals,
func(i, j int) bool {
return vals[i].GetTokens().Int64() > vals[j].GetTokens().Int64()
},
)

mocks.MockStakingKeeper.EXPECT().IterateBondedValidatorsByPower(gomock.Any(), gomock.Any()).DoAndReturn(
func(ctx sdk.Context, cb func(int64, stakingtypes.ValidatorI) bool) error {
for i, val := range vals {
if stop := cb(int64(i), val); stop {
break
}
}
return nil
})
actualValPowers := []int64{}
err := providerKeeper.IterateBondedValidatorsByPower(ctx, func(index int64, validator types.ValidatorI) (stop bool) {
counter++
actualValPowers = append(actualValPowers, validator.GetTokens().Int64())
return false
})
require.NoError(t, err)
// we don't check that the elements exactly match; just matching the powers is good enough
require.ElementsMatch(t, tt.expectedPowers, actualValPowers)
})
}
}
p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved
Loading
Loading