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.

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: allow endblocker to update app version #321

Merged
merged 1 commit into from
May 19, 2023

Conversation

cmwaters
Copy link

@cmwaters cmwaters commented May 19, 2023

Currently, I see know what that an application can use the end blocker to change the app version. All app version changes happen during a coordinated restart which means only the Info abci call is used to update the app version. This change means that the consensus param returned in EndBlock is always the applications app version. This means a module need only use SetProtocolVersion to enact a app version change in the following block

@@ -465,6 +465,8 @@ func (app *BaseApp) GetConsensusParams(ctx sdk.Context) *abci.ConsensusParams {
cp.Validator = &vp
}

cp.Version = &tmproto.VersionParams{AppVersion: app.appVersion}
Copy link
Member

Choose a reason for hiding this comment

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

nice, so now we should just be able to call app.SetVersion instead of wrapping endblock, I can dig it

@evan-forbes
Copy link
Member

we should try to upstream this (later)

@cmwaters
Copy link
Author

I need to dig in to how the SDK do it now because they've made some heavy modifications in that area since v0.46

@codecov-commenter
Copy link

Codecov Report

Merging #321 (787ece9) into release/v0.46.x-celestia (06c2fc2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@                    Coverage Diff                    @@
##           release/v0.46.x-celestia     #321   +/-   ##
=========================================================
  Coverage                     65.54%   65.55%           
=========================================================
  Files                           666      666           
  Lines                         70983    71018   +35     
=========================================================
+ Hits                          46529    46553   +24     
- Misses                        21876    21882    +6     
- Partials                       2578     2583    +5     
Impacted Files Coverage Δ
baseapp/baseapp.go 78.71% <100.00%> (+0.09%) ⬆️

... and 5 files with indirect coverage changes

@cmwaters cmwaters merged commit 57a798a into release/v0.46.x-celestia May 19, 2023
@cmwaters cmwaters deleted the cal/app-version branch May 19, 2023 13:09
@cmwaters
Copy link
Author

Ref: cosmos#16243

@cmwaters
Copy link
Author

It looks like v0.47 onwards will support altering the AppVersion through Endblock / FinalizeBlock via the ParamStore interface in baseapp. We will need to remember to make that adjustment when upgrading.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants