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.

Sign up for GitHub

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

On-chain upgrade to consumer chain #1

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

On-chain upgrade to consumer chain #1

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
40f3fdd
on-chain upgrade to consumer chain wip
jstr1121 Dec 19, 2022
aa566c9
add preCCV store and use it on democracy staking
jstr1121 Dec 19, 2022
dc9768d
add TODOs and one more packet possibility
jstr1121 Dec 19, 2022
473e467
status update
jstr1121 Dec 28, 2022
c6803f1
Resolve hermes start issue for trusted validator set by changing revi…
jstr1121 Jan 5, 2023
15265ad
remove intermediary logs
jstr1121 Jan 6, 2023
2806d6f
remove further unused codebase
jstr1121 Jan 6, 2023
c3ae9a4
updates for endblocker test, existing test fixes, get last validators
jstr1121 Jan 18, 2023
67aa39b
update for slashing sovereign validators for the fault made before co…
jstr1121 Jan 19, 2023
09f7b61
resolve comments on github and slack communication
jstr1121 Jan 31, 2023
d7b5a7f
Merge branch 'main' of github.com:Stride-Labs/interchain-security int…
jstr1121 Feb 21, 2023
8aea5bf
update sovereign app to use v4 ibc from v3 & resolve consumer module …
jstr1121 Feb 21, 2023
62da0d2
Update app/sovereign/upgrades/v3/upgrades.go
jstr1121 Feb 21, 2023
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
37 changes: 30 additions & 7 deletions app/consumer-democracy/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"os"
"path/filepath"

genutiltypes "github.com/cosmos/cosmos-sdk/x/genutil/types"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/grpc/tmservice"
Expand Down Expand Up @@ -417,6 +419,7 @@ func New(
&app.IBCKeeper.PortKeeper,
app.IBCKeeper.ConnectionKeeper,
app.IBCKeeper.ClientKeeper,
app.StakingKeeper,
app.SlashingKeeper,
app.BankKeeper,
app.AccountKeeper,
Expand All @@ -436,7 +439,7 @@ func New(

// register slashing module StakingHooks to the consumer keeper
app.ConsumerKeeper = *app.ConsumerKeeper.SetHooks(app.SlashingKeeper.Hooks())
consumerModule := ibcconsumer.NewAppModule(app.ConsumerKeeper)
consumerModule := ibcconsumer.NewAppModule(app.ConsumerKeeper, app.StakingKeeper)
Copy link

Choose a reason for hiding this comment

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

Seems good, just curious on the context why new arguments are needed in this file and app/consumer/app.go

Copy link
Author

Choose a reason for hiding this comment

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

It is using staking keeper's previous validator set, to set voting power of the existing validators as zero. Probably, there should be a way to use previous validator set without staking keeper.

Copy link

Choose a reason for hiding this comment

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

Got it, I'll need to refresh myself about how the democracy consumer app.go works, and how its staking keeper is used


app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec,
Expand Down Expand Up @@ -483,7 +486,7 @@ func New(
ccvmint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper),
slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.ConsumerKeeper),
ccvdistr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper, authtypes.FeeCollectorName),
ccvstaking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper),
ccvstaking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper, app.ConsumerKeeper),
upgrade.NewAppModule(app.UpgradeKeeper),
evidence.NewAppModule(app.EvidenceKeeper),
params.NewAppModule(app.ParamsKeeper),
Expand All @@ -506,6 +509,7 @@ func New(
ccvdistrtypes.ModuleName,
slashingtypes.ModuleName,
evidencetypes.ModuleName,
ibcconsumertypes.ModuleName, // Note: consumer beginblocker before staking module
ccvstakingtypes.ModuleName,
authtypes.ModuleName,
banktypes.ModuleName,
Expand All @@ -517,11 +521,11 @@ func New(
vestingtypes.ModuleName,
ibctransfertypes.ModuleName,
ibchost.ModuleName,
ibcconsumertypes.ModuleName,
)
app.MM.SetOrderEndBlockers(
crisistypes.ModuleName,
ccvgovtypes.ModuleName,
ibcconsumertypes.ModuleName, // Note: consumer endblocker before staking module
ccvstakingtypes.ModuleName,
capabilitytypes.ModuleName,
authtypes.ModuleName,
Expand All @@ -537,7 +541,6 @@ func New(
vestingtypes.ModuleName,
ibctransfertypes.ModuleName,
ibchost.ModuleName,
ibcconsumertypes.ModuleName,
)

// NOTE: The genutils module must occur after staking so that pools are
Expand All @@ -550,6 +553,7 @@ func New(
capabilitytypes.ModuleName,
authtypes.ModuleName,
banktypes.ModuleName,
ibcconsumertypes.ModuleName, // Note: consumer initiation before staking module
ccvdistrtypes.ModuleName,
ccvstakingtypes.ModuleName,
slashingtypes.ModuleName,
Expand All @@ -564,7 +568,6 @@ func New(
vestingtypes.ModuleName,
ibchost.ModuleName,
ibctransfertypes.ModuleName,
ibcconsumertypes.ModuleName,
)

app.MM.RegisterInvariants(&app.CrisisKeeper)
Expand All @@ -584,7 +587,7 @@ func New(
feegrantmodule.NewAppModule(appCodec, app.AccountKeeper, app.BankKeeper, app.FeeGrantKeeper, app.interfaceRegistry),
ccvgov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper, IsProposalWhitelisted),
ccvmint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper),
ccvstaking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper),
ccvstaking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper, app.ConsumerKeeper),
ccvdistr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper, authtypes.FeeCollectorName),
slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper),
params.NewAppModule(app.ParamsKeeper),
Expand Down Expand Up @@ -625,6 +628,7 @@ func New(
app.UpgradeKeeper.SetUpgradeHandler(
upgradeName,
func(ctx sdk.Context, _ upgradetypes.Plan, _ module.VersionMap) (module.VersionMap, error) {

app.IBCKeeper.ConnectionKeeper.SetParams(ctx, ibcconnectiontypes.DefaultParams())

fromVM := make(map[string]uint64)
Expand All @@ -633,6 +637,23 @@ func New(
fromVM[moduleName] = eachModule.ConsensusVersion()
}

// TODO: should have a way to read from current node home
userHomeDir, err := os.UserHomeDir()
if err != nil {
stdlog.Println("Failed to get home dir %2", err)
}
nodeHome := userHomeDir + "/.sovereign/config/genesis.json"
appState, _, err := genutiltypes.GenesisStateFromGenFile(nodeHome)
if err != nil {
return fromVM, fmt.Errorf("failed to unmarshal genesis state: %w", err)
}

var consumerGenesis = ibcconsumertypes.GenesisState{}
appCodec.MustUnmarshalJSON(appState[ibcconsumertypes.ModuleName], &consumerGenesis)

consumerGenesis.PreCCV = true
app.ConsumerKeeper.InitGenesis(ctx, &consumerGenesis)
Copy link

Choose a reason for hiding this comment

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

How does this call to the consumer keeper's InitGenesis interact with the existing system for calling every keeper's InitGenesis? See InitChainer in this file, and the sdk module manager's InitGenesis method

Copy link
Author

Choose a reason for hiding this comment

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

InitGenesis is only called when it's doing hard fork or launching a new chain, on soft fork, to init, it should be on upgrade handler I believe.


ctx.Logger().Info("start to run module migrations...")

return app.MM.RunMigrations(ctx, app.configurator, fromVM)
Expand All @@ -645,7 +666,9 @@ func New(
}

if upgradeInfo.Name == upgradeName && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) {
storeUpgrades := store.StoreUpgrades{}
storeUpgrades := store.StoreUpgrades{
Added: []string{ibcconsumertypes.ModuleName},
}

// configure store loader that checks if version == upgradeHeight and applies store upgrades
app.SetStoreLoader(upgradetypes.UpgradeStoreLoader(upgradeInfo.Height, &storeUpgrades))
Expand Down
3 changes: 2 additions & 1 deletion app/consumer/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ func New(
&app.IBCKeeper.PortKeeper,
app.IBCKeeper.ConnectionKeeper,
app.IBCKeeper.ClientKeeper,
nil,
Copy link

@shaspitz shaspitz Jan 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Could we pass in staking keeper here in case we'd ever want to test this upgrade with a normal consumer? Maybe I just need more clarity on how this PR affects (or shouldn't affect) the normal consumer, and the democracy consumer

Copy link
Author

Choose a reason for hiding this comment

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

Staking keeper will only be used for democracy one, but could pass consumer keeper here since consumer keeper implement the interface of staking keeper. On consumer app, there's no staking keeper FYI.

Copy link
Author

Choose a reason for hiding this comment

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

The reason for not passing anything here is to show that it's not democracy app

Copy link
Author

Choose a reason for hiding this comment

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

For all the uses of stakingKeeper, it has checker for stakingKeeper == nil check FYI

app.SlashingKeeper,
app.BankKeeper,
app.AccountKeeper,
Expand All @@ -344,7 +345,7 @@ func New(

// register slashing module Slashing hooks to the consumer keeper
app.ConsumerKeeper = *app.ConsumerKeeper.SetHooks(app.SlashingKeeper.Hooks())
consumerModule := ibcconsumer.NewAppModule(app.ConsumerKeeper)
consumerModule := ibcconsumer.NewAppModule(app.ConsumerKeeper, nil)

app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec,
Expand Down
Loading