-
Notifications
You must be signed in to change notification settings - Fork 33
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!: add app version to param store #347
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,7 +53,6 @@ type BaseApp struct { // nolint: maligned | |
postHandler sdk.AnteHandler // post handler, optional, e.g. for tips | ||
|
||
appStore | ||
baseappVersions | ||
peerFilters | ||
snapshotData | ||
abciData | ||
|
@@ -98,6 +97,13 @@ type BaseApp struct { // nolint: maligned | |
// ResponseCommit.RetainHeight. | ||
minRetainBlocks uint64 | ||
|
||
// 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 | ||
|
||
// recovery handler for app.runTx method | ||
runTxRecoveryMiddleware recoveryMiddleware | ||
|
||
|
@@ -145,15 +151,6 @@ type abciData struct { | |
voteInfos []abci.VoteInfo | ||
} | ||
|
||
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 | ||
} | ||
|
||
// should really get handled in some db struct | ||
// which then has a sub-item, persistence fields | ||
type snapshotData struct { | ||
|
@@ -482,7 +479,12 @@ func (app *BaseApp) GetConsensusParams(ctx sdk.Context) *abci.ConsensusParams { | |
cp.Validator = &vp | ||
} | ||
|
||
cp.Version = &tmproto.VersionParams{AppVersion: app.appVersion} | ||
if app.paramStore.Has(ctx, ParamStoreKeyVersionParams) { | ||
var vp tmproto.VersionParams | ||
|
||
app.paramStore.Get(ctx, ParamStoreKeyVersionParams, &vp) | ||
cp.Version = &vp | ||
} | ||
|
||
return cp | ||
} | ||
|
@@ -507,8 +509,7 @@ func (app *BaseApp) StoreConsensusParams(ctx sdk.Context, cp *abci.ConsensusPara | |
app.paramStore.Set(ctx, ParamStoreKeyBlockParams, cp.Block) | ||
app.paramStore.Set(ctx, ParamStoreKeyEvidenceParams, cp.Evidence) | ||
app.paramStore.Set(ctx, ParamStoreKeyValidatorParams, cp.Validator) | ||
// 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) | ||
Comment on lines
-510
to
+512
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
} | ||
|
||
// getMaximumBlockGas gets the maximum gas from the consensus params. It panics | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ var ( | |
ParamStoreKeyBlockParams = []byte("BlockParams") | ||
ParamStoreKeyEvidenceParams = []byte("EvidenceParams") | ||
ParamStoreKeyValidatorParams = []byte("ValidatorParams") | ||
ParamStoreKeyVersionParams = []byte("VersionParams") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes we should. Good catch. Are the other params here protected by the paramsfilter |
||
) | ||
|
||
// ParamStore defines the interface the parameter store used by the BaseApp must | ||
|
@@ -84,3 +85,18 @@ func ValidateValidatorParams(i interface{}) error { | |
|
||
return nil | ||
} | ||
|
||
// ValidateVersionParams defines a stateless validation on VersionParams. This | ||
// function is called whenever the parameters are updated or stored. | ||
func ValidateVersionParams(i interface{}) error { | ||
v, ok := i.(tmproto.VersionParams) | ||
if !ok { | ||
return fmt.Errorf("invalid parameter type: %T", i) | ||
} | ||
|
||
if v.AppVersion == 0 { | ||
return errors.New("application version must not be zero") | ||
} | ||
|
||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,9 @@ | ||
package exported | ||
|
||
import sdk "github.com/cosmos/cosmos-sdk/types" | ||
|
||
// ProtocolVersionSetter defines the interface fulfilled by BaseApp | ||
// which allows setting it's appVersion field. | ||
type ProtocolVersionSetter interface { | ||
SetProtocolVersion(uint64) | ||
SetAppVersion(sdk.Context, uint64) | ||
} |
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.
FYI: this is removed in v0.47 and v0.50