feat: give the application control to validate the app version

[cmwaters]

Description

CometBFT currently validates the app version before ProcessProposal is sent. This means coordination needs to happen in the prior block. If coordination breaks down, node operators don't know with which app version to validate the proposed block. Far more robust is if the app version is passed in ProcessProposal. This means that if coordination breaks down to upgrade to v2 we can still continue to approve and commit v1 blocks.

Note: It would be even better if instead of EndBlock, the app version of the block could be set in PrepareProposal but I'm not sure if that large of a change is necessary.

I'm still thinking about whether AppVersion should be removed entirely from tendermint state.

---

PR checklist

• Tests written/updated
• Changelog entry added in .changelog (we use
  unclog to manage our changelog)
• Updated relevant documentation (docs/ or spec/) and code comments

[evan-forbes]

If coordination breaks down, node operators don't know with which app version to validate the proposed block.

do you mind elaborating on when coordination would break down? my understanding of this was since we're bumping the version in endblock we'd never call ProcessProposal on a proposal of an unexpected height

Far more robust is if the app version is passed in ProcessProposal

afaiu we're doing this in v0.34.x since we're passing the entire header, do you know why did we stop doing this in upstream?

[cmwaters]

do you mind elaborating on when coordination would break down? my understanding of this was since we're bumping the version in endblock we'd never call ProcessProposal on a proposal of an unexpected height

In the first phase where we use a config file, if two users set different values for when to upgrade then the network would halt. user A would keep proposing a v2 block and user B would reject it and user B would propose a v1 block and user A would reject it. If we introduce this phase then user A proposing a v2 block would still get rejected but user B proposing a v1 block would still be accepted thus the network would avoid halting.

do you know why did we stop doing this in upstream?

I think the idea was to only give the application the information that is relevant. For example, there's no need for the application to know the evidence hash.

[evan-forbes]

In the first phase where we use a config file, if two users set different values for when to upgrade then the network would halt. ... If we introduce this phase then user A proposing a v2 block would still get rejected but user B proposing a v1 block would still be accepted thus the network would avoid halting.

shouldn't the network halt if nodes do not agree on the same version? That was at least my understanding of what was desired. Is this just a stop gap for signalling?

so to clarify, this is proposing that we shouldn't increment the app verison during end block at the configured height, we're only incrementing it once 2/3's prevotes on a proposal with the next version? or we at least have to make sure that we're never using the app's local state to determin the version (like in app.Version()), we only use what's in the header and then add a check in process proposal? Do we decrement the version if we don't precommit on that height?

[cmwaters]

Closing this for now as this isn't the path we want to take going forward with managing version changes