-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
fix: app config and simapp (v1,v2) fixes #14209
Conversation
@@ -86,7 +85,6 @@ func (a *App) Load(loadLatest bool) error { | |||
|
|||
if len(a.config.InitGenesis) != 0 { | |||
a.ModuleManager.SetOrderInitGenesis(a.config.InitGenesis...) | |||
a.SetInitChainer(a.InitChainer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we need to set the module version map set for x/upgrade. But this should really be done somehow automatically with app config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
app_v2.go
already sets it, so this was overwritting it and not calling app.UpgradeKeeper.SetModuleVersionMap(ctx, app.ModuleManager.GetVersionMap())
.
Didn't run the integration tests locally, if you use runtime standalone it is indeed needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I'm saying is that app_v2.go shouldn't need to manually call it. Is there a clean way to make that happen? Doesn't need to be in this PR btw
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, let me check 👍🏾
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we need the store to be available, I have not found a clean way.
713f2de
to
7d53ceb
Compare
@@ -144,7 +144,8 @@ func AppStateRandomizedFn( | |||
accs []simtypes.Account, genesisTimestamp time.Time, appParams simtypes.AppParams, | |||
) (json.RawMessage, []simtypes.Account) { | |||
numAccs := int64(len(accs)) | |||
genesisState := NewDefaultGenesisState(cdc) | |||
// TODO - in case runtime.RegisterModules(...) is used, the genesis state of the module won't be reflected here | |||
genesisState := ModuleBasics.DefaultGenesis(cdc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure how to avoid this.
I will refactor upgrade app config in a follow during my audit. Reverting it from this PR. |
@@ -591,6 +608,11 @@ func (app *SimApp) TxConfig() client.TxConfig { | |||
return app.txConfig | |||
} | |||
|
|||
// DefaultGenesis returns a default genesis from the registered AppModuleBasic's. | |||
func (a *SimApp) DefaultGenesis() map[string]json.RawMessage { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we replace NewDefaultGenesisState
by this. In the case of appv2 it could lead to something different than ModuleBasics
.
[Cosmos SDK - Client V2] Kudos, SonarCloud Quality Gate passed! |
[Cosmos SDK - Tx] Kudos, SonarCloud Quality Gate passed! |
[Cosmos SDK Core] Kudos, SonarCloud Quality Gate passed! |
[Cosmos SDK - SimApp] Kudos, SonarCloud Quality Gate passed! |
[Cosmos SDK - Rosetta] Kudos, SonarCloud Quality Gate passed! |
[Cosmos SDK] Kudos, SonarCloud Quality Gate passed! |
(cherry picked from commit 4e2196f) # Conflicts: # tools/rosetta/go.mod # tools/rosetta/go.sum
Description
NewDefaultGenesisState
because it not take in account modules registered manually withruntime.RegisterModules
LoadStreamingServices
was using an empty list of keysFollow-up
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change