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

Onchain upgrade to consumer chain from sovereign chain #697

Closed
wants to merge 13 commits into from
Closed

Onchain upgrade to consumer chain from sovereign chain #697

Show file tree
Hide file tree
Changes from all 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
38 changes: 31 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 := consumer.NewAppModule(app.ConsumerKeeper)
consumerModule := consumer.NewAppModule(app.ConsumerKeeper, app.StakingKeeper)

app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec,
Expand Down Expand Up @@ -483,7 +486,7 @@ func New(
mint.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,8 @@ func New(
distrtypes.ModuleName,
slashingtypes.ModuleName,
evidencetypes.ModuleName,
consumertypes.ModuleName, // Note: consumer beginblocker before staking module
Copy link
Contributor

Choose a reason for hiding this comment

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

I think changes like this would need to be upstreamed to all consumer chains that pattern match this file. No action item for the Stride team, just commenting for awareness on our team

Copy link
Contributor

Choose a reason for hiding this comment

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

@jstr1121 Why is this order necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

Beginblocker order shouldn't matter AFAIK

stakingtypes.ModuleName,
stakingtypes.ModuleName,
authtypes.ModuleName,
banktypes.ModuleName,
Expand All @@ -517,11 +522,11 @@ func New(
vestingtypes.ModuleName,
ibctransfertypes.ModuleName,
ibchost.ModuleName,
consumertypes.ModuleName,
)
app.MM.SetOrderEndBlockers(
crisistypes.ModuleName,
govtypes.ModuleName,
consumertypes.ModuleName, // Note: consumer endblocker before staking module
Copy link
Contributor

Choose a reason for hiding this comment

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

@jstr1121 Why is this order necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

With 1269163, the order doesn't matter AFAIK

stakingtypes.ModuleName,
capabilitytypes.ModuleName,
authtypes.ModuleName,
Expand All @@ -537,7 +542,6 @@ func New(
vestingtypes.ModuleName,
ibctransfertypes.ModuleName,
ibchost.ModuleName,
consumertypes.ModuleName,
)

// NOTE: The genutils module must occur after staking so that pools are
Expand All @@ -550,6 +554,7 @@ func New(
capabilitytypes.ModuleName,
authtypes.ModuleName,
banktypes.ModuleName,
consumertypes.ModuleName, // Note: consumer initiation before staking module
Copy link
Contributor

Choose a reason for hiding this comment

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

@jstr1121 Why is this order necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, InitGenesis shouldn't matter unless I'm missing something

distrtypes.ModuleName,
stakingtypes.ModuleName,
slashingtypes.ModuleName,
Expand All @@ -564,7 +569,6 @@ func New(
vestingtypes.ModuleName,
ibchost.ModuleName,
ibctransfertypes.ModuleName,
consumertypes.ModuleName,
)

app.MM.RegisterInvariants(&app.CrisisKeeper)
Expand All @@ -584,7 +588,7 @@ func New(
feegrantmodule.NewAppModule(appCodec, app.AccountKeeper, app.BankKeeper, app.FeeGrantKeeper, app.interfaceRegistry),
ccvgov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper, IsProposalWhitelisted),
mint.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),
mpoke marked this conversation as resolved.
Show resolved Hide resolved
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 +629,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 +638,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 = consumertypes.GenesisState{}
appCodec.MustUnmarshalJSON(appState[consumertypes.ModuleName], &consumerGenesis)

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

Comment on lines +641 to +657
Copy link
Contributor

Choose a reason for hiding this comment

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

This is needed only because the provider doesn't create a correct init genesis for the consumer CCV module with preCCV set to true. See the spec for more details. @smarshall-spitzbart please open an issue to address this.

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

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

if IsSkipHeight(upgradeInfo.Height) {
		return
	}

Comment on lines +670 to 673
Copy link
Contributor

Choose a reason for hiding this comment

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

@jstr1121 What are these changes for?

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

Choose a reason for hiding this comment

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

See

interchain-security/x/ccv/consumer/keeper/keeper.go

Line 85 in afd1b2b

func (k Keeper) mustValidateFields() {
on this keeper being nil. Maybe we can add a flag likeisDemocConsumer to make things very clear

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use a comment here


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