-
Notifications
You must be signed in to change notification settings - Fork 229
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
chore(cosmos): update max block size #8484
Conversation
golang/cosmos/app/app.go
Outdated
app.UpgradeKeeper.SetModuleVersionMap(ctx, app.mm.GetVersionMap()) | ||
res := app.mm.InitGenesis(ctx, app.appCodec, genesisState) | ||
|
||
// Update tendermint consensus params with any changes made | ||
res.ConsensusParams = app.BaseApp.GetConsensusParams(ctx) |
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.
Hmm, this seems wrong - the BaseApp
state should follow from the genesis, not vice-versa. Try doing the validation on res.ConsensusParams
here, as suggested above, which ensures that the genesis file had an acceptable value, and then BaseApp
should be initialized to that acceptable value.
The default value probably isn't what you want to update. There's a |
After discussion, will update the default in tendermint/CometBFT and do a straight param change in the upgrade handler |
63fbbef
to
d4af8cc
Compare
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.
@JimLarson PTAL. This is now layered on top of #8409 and temporarily using a commit based version of CometBFT until agoric-labs/cometbft#5 is merged and tagged.
d4af8cc
to
6c5e844
Compare
@JimLarson PTAL. #8409 is no longer a dependency, and we're instead targetting our tendermint fork instead. |
6c5e844
to
337ede0
Compare
337ede0
to
5fb6334
Compare
chore(cosmos): update max block size
chore(cosmos): update max block size
chore(cosmos): update max block size
fixes: #8483
Description
Update the
BlockParams.MaxBytes
value during upgrade-12 to reflect the new lower default adopted in agoric-labs/tendermint#36This change does not change the max allowed size the param can have, and the param remains fully configurable by governance.
Security Considerations
Mitigation for a public Tendermint/CometBFT advisory, for which the Agoric chain is not impacted beyond slow blocks, something that can already happen safely on the Agoric chain.
Scaling Considerations
None
Documentation Considerations
None
Testing Considerations
Docker upgrade test added to verify the param has been changed after upgrade
Upgrade Considerations
Chain software upgrade required.