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!: Allow inactive provider chain validators to validate on consumer chains #2079

Merged
merged 54 commits into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
b9928d4
refactor!: Refactor the validator set storage and add provider consen…
p-offtermatt Jul 2, 2024
b28b139
feat!: Introduce the MaxProviderConsensusValidators param (#1992)
p-offtermatt Jul 2, 2024
e71129e
feat!: Wire the provider module to return ValidatorUpdates, instead o…
p-offtermatt Jul 15, 2024
9788053
test: enable the simulator for the provider module (#2005)
p-offtermatt Jul 15, 2024
3db7f3d
feat!: Let consumer chains choose a minimum stake and validator rank …
p-offtermatt Jul 17, 2024
934550f
feat!: Rewire dependencies on the staking module (#2056)
p-offtermatt Jul 18, 2024
1b40c17
feat: Calculate Top N based on active validators only (#2070)
p-offtermatt Jul 22, 2024
0f4651b
Delete hanging changelog entry
p-offtermatt Jul 24, 2024
e5f155d
Address comments
p-offtermatt Jul 24, 2024
b97ab26
Address more comments
p-offtermatt Jul 24, 2024
4bfc1df
Add migration for param
p-offtermatt Jul 24, 2024
8a67a8a
Fix allow inactive validators param test
p-offtermatt Jul 24, 2024
5ceaffc
Fix tests
p-offtermatt Jul 24, 2024
a859f7a
Merge branch 'main' into ph/inactive-vals-v50-2
p-offtermatt Jul 24, 2024
92904e9
Add LastProviderConsensusValidatorKey to fully defined keys
p-offtermatt Jul 24, 2024
9ab14ee
Fix key for validator set updates
p-offtermatt Jul 24, 2024
8024804
Add info about genesis/endblock ordering
p-offtermatt Jul 24, 2024
264b642
Add unit test for ProviderValidatorUpdates
p-offtermatt Jul 25, 2024
22bfde8
Add example to proto definition of max_rank
p-offtermatt Jul 25, 2024
42c05b2
Remove max rank
p-offtermatt Jul 26, 2024
ff2f074
Merge branch 'main' into ph/inactive-vals-v50-2
p-offtermatt Jul 26, 2024
60bf261
Remove references to max rank
p-offtermatt Jul 26, 2024
7fbb134
Start adding an extension to the simulator
p-offtermatt Jul 18, 2024
da4dd8e
Make invariant fail early when param is 0
p-offtermatt Jul 18, 2024
f4ce5b1
Reorder InitGenesis to put Crisis last
p-offtermatt Jul 26, 2024
75bb7a9
Remove canary
p-offtermatt Jul 26, 2024
af528c7
Swap equals for not equals
p-offtermatt Jul 26, 2024
74b4b94
Disable invariant check when max validators != max provider consensus…
p-offtermatt Jul 26, 2024
9b9a945
Make the simulator use a random seed
p-offtermatt Jul 26, 2024
a73ef5a
Remove TODO
p-offtermatt Jul 26, 2024
0726e63
Remove decoder
p-offtermatt Jul 26, 2024
837d111
Run go mod tidy
p-offtermatt Jul 29, 2024
0c2b427
Add migration in UPGRADING.md
p-offtermatt Jul 29, 2024
8c06477
Merge branch 'main' into ph/inactive-vals-v50-2
p-offtermatt Aug 1, 2024
4efaa98
Fix tests
p-offtermatt Aug 1, 2024
a35e39b
Put random seed generation into golang code
p-offtermatt Aug 2, 2024
4a01439
Rename simulation jobs
p-offtermatt Aug 2, 2024
fb6d09f
Update UPGRADING.md
p-offtermatt Aug 2, 2024
5cb6376
Update UPGRADING.md
p-offtermatt Aug 2, 2024
1860933
Update x/ccv/provider/keeper/genesis.go
p-offtermatt Aug 2, 2024
266e9da
Mention simulation tests in testing.md
p-offtermatt Aug 2, 2024
7c96789
Address some comments
p-offtermatt Aug 2, 2024
24823a6
Remake protos
p-offtermatt Aug 2, 2024
a7b1140
Panic when LastActiveBondedValidators fails
p-offtermatt Aug 2, 2024
aeb2e96
Address some comments
p-offtermatt Aug 2, 2024
2c70e3b
Address comments
p-offtermatt Aug 2, 2024
ed7e12d
Reorder tests
p-offtermatt Aug 2, 2024
484cb7d
Adjust stake_multiplier to stakeMultiplier
p-offtermatt Aug 2, 2024
930a482
Address comments
p-offtermatt Aug 5, 2024
20345bd
Add error logging
p-offtermatt Aug 5, 2024
46dbb4e
Fix reference to bank blocked addrs in simulation
p-offtermatt Aug 5, 2024
c3b36e6
Change hasToValidate to only take into account active validators
p-offtermatt Aug 5, 2024
bf35f2c
Update docs/docs/adrs/adr-017-allowing-inactive-validators.md
p-offtermatt Aug 5, 2024
f3b7995
Clarify: Slash happens on provider
p-offtermatt Aug 5, 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Add minimum stake and maximum validator rank requirements to let consumer chains
determine requirements for validators that validate their chain. ([\#2035](https://github.com/cosmos/interchain-security/pull/2035))
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Add the `allow_inactive_vals` parameter for consumer chains to choose whether inactive validators can validate their chain ([\#2066](https://github.com/cosmos/interchain-security/pull/2066))
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Add minimum stake and maximum validator rank requirements to let consumer chains
determine requirements for validators that validate their chain. ([\#2035](https://github.com/cosmos/interchain-security/pull/2035))
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Add the `allow_inactive_vals` parameter for consumer chains to choose whether inactive validators can validate their chain ([\#2066](https://github.com/cosmos/interchain-security/pull/2066))
17 changes: 17 additions & 0 deletions .github/workflows/nightly-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,23 @@ jobs:
- name: E2E active set changes
run: go run ./tests/e2e/... --tc active-set-changes

inactive-provider-validators-on-consumer-test:
runs-on: ubuntu-latest
timeout-minutes: 20
steps:
- uses: actions/setup-go@v5
with:
go-version: "1.22"
- uses: actions/checkout@v4
- name: Checkout LFS objects
run: git lfs checkout
- name: Setup Go
uses: actions/setup-go@v5
with:
go-version: "1.22" # The Go version to download (if necessary) and use.
- name: E2E inactive provider validators on consumer
run: go run ./tests/e2e/... --tc inactive-provider-validators-on-consumer

nightly-test-fail:
needs:
- happy-path-test
Expand Down
44 changes: 44 additions & 0 deletions .github/workflows/simulation.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
name: Simulation
on:
workflow_call:
pull_request:
merge_group:
push:
branches:
- main
- release/v*
- feat/*

permissions:
contents: read

concurrency:
group: ci-${{ github.ref }}-tests
cancel-in-progress: true

jobs:
simulation:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: "1.22"
check-latest: true
cache: true
cache-dependency-path: go.sum
- uses: technote-space/[email protected]
id: git_diff
with:
PATTERNS: |
**/*.go
go.mod
go.sum
**/go.mod
**/go.sum
**/Makefile
Makefile
- name: simulation test
if: env.GIT_DIFF
run: |
make sim-full
7 changes: 7 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,13 @@ verify-models:
../run_invariants.sh


###############################################################################
### Simulation tests ###

# Run a full simulation test
sim-full:
p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved
cd app/provider;\
go test -mod=readonly -run=^TestFullAppSimulation$ -Enabled=true -NumBlocks=500 -BlockSize=200 -Commit=true -timeout 24h github.com/cosmos/interchain-security/v5/app/provider -v

###############################################################################
### Linting ###
Expand Down
103 changes: 63 additions & 40 deletions app/provider/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ import (
"os"
"path/filepath"

dbm "github.com/cosmos/cosmos-db"
"github.com/cosmos/gogoproto/proto"
"github.com/cosmos/ibc-go/modules/capability"
capabilitykeeper "github.com/cosmos/ibc-go/modules/capability/keeper"
capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
"github.com/cosmos/ibc-go/v8/modules/apps/transfer"
ibctransferkeeper "github.com/cosmos/ibc-go/v8/modules/apps/transfer/keeper"
ibctransfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
Expand All @@ -26,7 +30,7 @@ import (
reflectionv1 "cosmossdk.io/api/cosmos/reflection/v1"
"cosmossdk.io/client/v2/autocli"
"cosmossdk.io/core/appmodule"

"cosmossdk.io/log"
storetypes "cosmossdk.io/store/types"
"cosmossdk.io/x/evidence"
evidencekeeper "cosmossdk.io/x/evidence/keeper"
Expand All @@ -35,6 +39,7 @@ import (
"cosmossdk.io/x/upgrade"
upgradekeeper "cosmossdk.io/x/upgrade/keeper"
upgradetypes "cosmossdk.io/x/upgrade/types"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
Expand All @@ -50,16 +55,20 @@ import (
"github.com/cosmos/cosmos-sdk/server/config"
servertypes "github.com/cosmos/cosmos-sdk/server/types"
"github.com/cosmos/cosmos-sdk/std"
"github.com/cosmos/cosmos-sdk/testutil/testdata/testpb"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
"github.com/cosmos/cosmos-sdk/types/msgservice"
sigtypes "github.com/cosmos/cosmos-sdk/types/tx/signing"
"github.com/cosmos/cosmos-sdk/version"
"github.com/cosmos/cosmos-sdk/x/auth"
"github.com/cosmos/cosmos-sdk/x/auth/ante"
authcodec "github.com/cosmos/cosmos-sdk/x/auth/codec"
authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper"
"github.com/cosmos/cosmos-sdk/x/auth/posthandler"
authsims "github.com/cosmos/cosmos-sdk/x/auth/simulation"
authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
txmodule "github.com/cosmos/cosmos-sdk/x/auth/tx/config"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
"github.com/cosmos/cosmos-sdk/x/auth/vesting"
vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types"
Expand Down Expand Up @@ -93,29 +102,21 @@ import (
"github.com/cosmos/cosmos-sdk/x/slashing"
slashingkeeper "github.com/cosmos/cosmos-sdk/x/slashing/keeper"
slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types"
"github.com/cosmos/cosmos-sdk/x/staking"
stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
"github.com/cosmos/ibc-go/modules/capability"
capabilitykeeper "github.com/cosmos/ibc-go/modules/capability/keeper"
capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"

"cosmossdk.io/log"
abci "github.com/cometbft/cometbft/abci/types"
tmjson "github.com/cometbft/cometbft/libs/json"
tmos "github.com/cometbft/cometbft/libs/os"
dbm "github.com/cosmos/cosmos-db"

appencoding "github.com/cosmos/interchain-security/v5/app/encoding"
testutil "github.com/cosmos/interchain-security/v5/testutil/integration"
no_valupdates_genutil "github.com/cosmos/interchain-security/v5/x/ccv/no_valupdates_genutil"
no_valupdates_staking "github.com/cosmos/interchain-security/v5/x/ccv/no_valupdates_staking"
ibcprovider "github.com/cosmos/interchain-security/v5/x/ccv/provider"
ibcproviderclient "github.com/cosmos/interchain-security/v5/x/ccv/provider/client"
ibcproviderkeeper "github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper"
providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types"

"github.com/cosmos/cosmos-sdk/testutil/testdata/testpb"
sigtypes "github.com/cosmos/cosmos-sdk/types/tx/signing"
txmodule "github.com/cosmos/cosmos-sdk/x/auth/tx/config"
)

const (
Expand Down Expand Up @@ -152,7 +153,7 @@ var (
mint.AppModuleBasic{},
slashing.AppModuleBasic{},
distr.AppModuleBasic{},
staking.AppModuleBasic{},
no_valupdates_staking.AppModuleBasic{},
upgrade.AppModuleBasic{},
evidence.AppModuleBasic{},

Expand Down Expand Up @@ -352,9 +353,7 @@ func New(
// 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 := app.ModuleAccountAddrs()
delete(bankBlockedAddrs, authtypes.NewModuleAddress(
providertypes.ConsumerRewardsPool).String())
bankBlockedAddrs := BankBlockedAddrs(app)
p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved

app.BankKeeper = bankkeeper.NewBaseKeeper(
appCodec,
Expand All @@ -374,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(),
)
Copy link
Contributor

@insumity insumity Aug 5, 2024

Choose a reason for hiding this comment

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

DistrKeeper should use the staking keeper or the provider one?
Also, the same question for the slashing, evidence, and IBC keeper.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, yes. Thanks. Is there a problem with using the provider keeper and hence the consensus-active validator set even though they're fine using the staking keeper? Is there a potentiality where distr, slashing, evidence, ... might need to use 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.

It just means our provider keeper would need to implement the interface that these keepers expect from the staking keeper, too, so it just increases the surface of the provider keeper for no benefit

app.DistrKeeper = distrkeeper.NewKeeper(
appCodec,
runtime.NewKVStoreService(keys[distrtypes.StoreKey]),
Expand Down Expand Up @@ -457,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 @@ -484,13 +461,43 @@ 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,
// use the ProviderKeeper as StakingKeeper for gov
// because governance should be based on the consensus-active validators
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
p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved
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 Expand Up @@ -536,7 +543,7 @@ func New(
// NOTE: Any module instantiated in the module manager that is later modified
// must be passed by reference here.
app.MM = module.NewManager(
genutil.NewAppModule(
no_valupdates_genutil.NewAppModule(
app.AccountKeeper,
app.StakingKeeper,
app,
Expand All @@ -552,7 +559,7 @@ func New(
mint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper, nil, app.GetSubspace(minttypes.ModuleName)),
slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper, app.GetSubspace(slashingtypes.ModuleName), app.interfaceRegistry),
distr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper, app.GetSubspace(distrtypes.ModuleName)),
staking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(stakingtypes.ModuleName)),
no_valupdates_staking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(stakingtypes.ModuleName)),
upgrade.NewAppModule(&app.UpgradeKeeper, app.AccountKeeper.AddressCodec()),
evidence.NewAppModule(app.EvidenceKeeper),

Expand Down Expand Up @@ -678,6 +685,15 @@ func New(
panic(err)
}

// create the simulation manager and define the order of the modules for deterministic simulations
overrideModules := map[string]module.AppModuleSimulation{
authtypes.ModuleName: auth.NewAppModule(app.appCodec, app.AccountKeeper, authsims.RandomGenesisAccounts, app.GetSubspace(authtypes.ModuleName)),
}
app.sm = module.NewSimulationManagerFromAppModules(app.MM.Modules, overrideModules)

// register the store decoders for simulation tests
app.sm.RegisterStoreDecoders()

// Note this upgrade handler is just an example and may not be exactly what you need to implement.
// See https://docs.cosmos.network/v0.45/building-modules/upgrade.html
app.UpgradeKeeper.SetUpgradeHandler(
Expand Down Expand Up @@ -778,6 +794,13 @@ func New(
return app
}

func BankBlockedAddrs(app *App) map[string]bool {
bankBlockedAddrs := app.ModuleAccountAddrs()
delete(bankBlockedAddrs, authtypes.NewModuleAddress(
providertypes.ConsumerRewardsPool).String())
return bankBlockedAddrs
}

// Name returns the name of the App
func (app *App) Name() string { return app.BaseApp.Name() }

Expand Down
Loading