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

[Feature]: pass protocol version to cometbft #16455

Closed
yihuang opened this issue Jun 8, 2023 · 5 comments
Closed

[Feature]: pass protocol version to cometbft #16455

yihuang opened this issue Jun 8, 2023 · 5 comments
Assignees
Labels

Comments

@yihuang
Copy link
Collaborator

yihuang commented Jun 8, 2023

Summary

x/upgrade maintains an increasing protocol version number, it's set to BaseApp.appVersion, but it's not passed to cometbft, so it's never shown in http://127.0.0.1:26657/abci_info, and not included in block header.

Problem Definition

Proposal

  • Initialize BaseApp.appVersion from x/upgrade's current state.
  • Pass to cometbft through block delivery response when it's changed.
@alexanderbez
Copy link
Contributor

cc @cmwaters

@tac0turtle
Copy link
Member

callum has a pr to move app version out of upgrade module into consensus module, i believe this way it will get the data and pass it to comet when its required

@lucaslopezf
Copy link
Contributor

Hi @yihuang !

This issue has been resolved in the following PR: cosmos/cosmos-sdk#16244.

The protocol version is updated during an upgrade process. This is handled in the x/upgrade module, specifically in the ApplyUpgrade method of the Keeper: ApplyUpgrade

When an upgrade is applied, the protocol version is incremented and stored. This ensures that the application is aware of the new protocol version after an upgrade.
The updated protocol version is then notified in the EndBlock, as the PR shows, and the protocol version is also displayed in the abci_info endpoint (this is achieved by retrieving the protocol version in the Info method of BaseApp: abciInfo )

To validate this implementation, the following steps were performed:

  1. Start a Cosmos node
  2. Perform an Upgrade
  3. Query abci_info endpoint

The result of querying the abci_info endpoint after severals upgrades is shown in the following image:
image

Please confirm if this addresses your concerns and if the issue can be closed, thanks!

@educlerici-zondax
Copy link
Contributor

hi @yihuang were you able to take a look at lucas's answer?

@julienrbrt
Copy link
Member

Hey, this has been implemented in baseapp indeed. I'll open another issue for v2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 🥳 Done
Development

No branches or pull requests

6 participants