feat!: add app version to param store #347
Conversation
cmwaters
changed the title
| type baseappVersions struct { | ||
| // application's version string | ||
| version string | ||
|
|
||
| // application's protocol version that increments on every upgrade | ||
| // if BaseApp is passed to the upgrade keeper's NewKeeper method. | ||
| appVersion uint64 | ||
| } |
There was a problem hiding this comment.
FYI: this is removed in v0.47 and v0.50
| // We're explicitly not storing the Tendermint app_version in the param store. It's | ||
| // stored instead in the x/upgrade store, with its own bump logic. | ||
| app.paramStore.Set(ctx, ParamStoreKeyVersionParams, cp.Version) |
There was a problem hiding this comment.
We don't store it in upgrade because we have a different module hence we should store it here. In v0.50.x, the SDK also begins to store the app version in the ParamStore of BaseApp
| @@ -18,6 +18,7 @@ var ( | |||
| ParamStoreKeyBlockParams = []byte("BlockParams") | |||
| ParamStoreKeyEvidenceParams = []byte("EvidenceParams") | |||
| ParamStoreKeyValidatorParams = []byte("ValidatorParams") | |||
| ParamStoreKeyVersionParams = []byte("VersionParams") | |||
There was a problem hiding this comment.
I forget, are these accessible by the gov handler? are we going to have to add this to the params filter? If so we should create an issue ofc
There was a problem hiding this comment.
Yes we should. Good catch. Are the other params here protected by the paramsfilter
| if ctx.BlockHeader().Version.App >= 2 { | ||
| vp := &tmproto.VersionParams{AppVersion: v} | ||
| app.paramStore.Set(ctx, ParamStoreKeyVersionParams, vp) | ||
| } |
There was a problem hiding this comment.
when does SetAppVersion get called when version.App == 1?
|
|
Description
This PR adds app version to the
ParamStoremeaning that app version is no longer just in memory but is also persisted. We need this for example when a node restarts as Tendermint will callInfoand it must remember the correct app version. It's also important for ensuring that state sync works beyond v1Author 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.mdReviewers 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