Toggle VoteExtensions using a consensus parameter

[williambanfield]

As of v0.36 VoteExtensions are set to be required by Tendermint. This poses a challenge to validators wishing to upgrade to v0.36 without performing a hardfork upgrade.

Validators running previous versions of Tendermint that update to v0.36 will not be able to propose blocks during the first height if vote extensions are required. This is because the previous height was completed without them and so Tendermint will not be able to deliver vote extension data to the application. For more information on this problem, see RFC-009.

To allow a smooth transition from previous versions of Tendermint to v0.36, I propose creating a consensus parameter nominally called VoteExtensions.InitialHeight. The default value of this field will be 0. When set to 0, vote extensions will not be required in any capacity. This will allow new validators that come up without extension data to not error. Chains will then be able to set this to a specific height after successfully running and collecting vote extensions.

Why A Height?

Whatever mechanism for enabling this system needs to save the initial height the feature is enabled. If VoteExtensions are enabled at height H, then validators will attempt to use the data from height H-1 as the vote extension data. That data is not guaranteed to exist because the network wasn't validating the existence of the extension data.

If we us a height then we can ensure that the correct behavior is enforced. The height that the parameter is set to, H, will be the first height that votes will be validated for the presence of vote extension data. The following height, H+1, will be the first height that the proposal creation logic will require vote extension data.

Effect of the variable on consensus

When Set to A Value, H, Greater Than 0

• If height is < H, vote extension data not checked for in any logic.
• If height == H, vote extension data is required on incoming votes but not required to be delivered to the application.
• If Height > H, vote extension data is required on incoming votes and is required to be delivered to the application.

When Set to 0

• Vote extension data not checked for in any logic.

Should chains be disable vote extensions after beginning their use?

Vote extension data is a new feature and somewhat experimental. Applications that encounter issue with the technology may wish to disable it while further debugging and diagnosing the problems they encounter.

However, from the use cases that have been proposed thus far, vote extensions are a pivotal piece of applications. For applications taking advantage of threshold decryption and oracle data for pricing, this is a pretty central piece of their design and execution. If they disable the feature, they will also need to fundamentally alter their application. It therefore seems safe to make vote extensions required after being enabled.

[thanethomson]

This makes sense to me. I like the idea of specifying a height instead of using a boolean value - it definitely seems clearer and more robust.

[thanethomson]

Just so I understand fully, looking at the ConsensusParams struct, is your idea to make something like the following change?

type ConsensusParams struct {
    VoteExtensions VoteExtensionsParams
    // ... existing fields
}

type VoteExtensionsParams struct {
    InitialHeight int64
}

If so, do you anticipate that we'd potentially add more vote extensions-related consensus parameters later? If not I'd advocate for something a little more generic, like ABCIParams with one field so far: VoteExtensionsHeight.

[sergio-mena]

Thanks @williambanfield for this proposal. It seems sound to me.
If folks don't have further concerns on #8317 (I have addressed all outstanding comments), I propose the following as next steps:

1. RFC017: ABCI++ Vote Extension Propagation #8317 (RFC 017, v1) gets approved and merged
2. @williambanfield (or myself, if you're still not recovered) turns the description of this issue into additional paragrahps/sections of RFC017
3. The new version of RFC017 should explain how the different solutions explained should be adapted to remain correct, in particular (longer-term) solution 3. I could take this on
4. Implementation done on top of abci++: Propagate vote extensions (RFC 017) #8433 (or once abci++: Propagate vote extensions (RFC 017) #8433 is merged)

Steps 3) and 4) can be carried out in parallel.

Let me know what you think

[sergio-mena]

One last thought: Should Tendermint "automatically" set VoteExtensions.InitialHeight to 1 in the case we are starting the chain from scratch in v0.36 or later?
Maybe we can set a genesis param, with the same semantics as VoteExtensions.InitialHeight, but with default value set to 1. So chains migrating to v0.36 are not affected by the genesis param (so default value, taken from the consensus param, is 0). And chains starting in v0.36 will take the genesis param (default value: 1), unless the App explicitly provides another value (e.g., 0).

[creachadair]

One last thought: Should Tendermint "automatically" set VoteExtensions.InitialHeight to 1 in the case we are starting the chain from scratch in v0.36 or later?

We could certainly do that, but given some of the subtleties associated with using vote extensions my intuition is we might be better off making this an explicit opt-in even for new chains.

[sergio-mena]

We could certainly do that, but given some of the subtleties associated with using vote extensions my intuition is we might be better off making this an explicit opt-in even for new chains.

Both ways seem reasonable to me. No strong opinion on either at this time.

[williambanfield]

@thanethomson

Just so I understand fully, looking at the ConsensusParams struct, is your idea to make something like the following change...

Either name is probably fine. I can't predict if we'll have more vote extension parameters so it's hard to say. My only qualm with the ABCIParams name is that vote extensions are not strictly an ABCI feature. Accepting / rejecting empty vote extensions has impacts on consensus underneath the conceptual ABCI barrier.

[williambanfield]

@sergio-mena I'm happy to take on the steps outlined above, although I would strongly prefer to wait until the work is completed in those PRs. I find it confusing to review PRs that expand in scope and change while they are open since you need to simultaneously track what all of the changes the PR are trying to do and what the changes are that match that, usually with some cleanups sprinkled in. For me, that usually becomes too expansive to fit into my head, so I'd prefer to make the changes in serial.

[thanethomson]

My only qualm with the ABCIParams name is that vote extensions are not strictly an ABCI feature.

I was thinking more of it as ABCI(++)-related consensus params, since they'd be embedded in the ConsensusParams struct.

[thanethomson]

However, from the use cases that have been proposed thus far, vote extensions are a pivotal piece of applications. For applications taking advantage of threshold decryption and oracle data for pricing, this is a pretty central piece of their design and execution. If they disable the feature, they will also need to fundamentally alter their application. It therefore seems safe to make vote extensions required after being enabled.

So in this regard, what if application developers want to roll back enabling vote extensions, or make them optional within the application itself? I suppose as a fallback the application itself could just ignore and not generate vote extensions if it wants, and we'd continue to just sign empty vote extensions once they're enabled.

[sergio-mena]

So in this regard, what if application developers want to roll back enabling vote extensions, or make them optional within the application itself? I suppose as a fallback the application itself could just ignore and not generate vote extensions if it wants, and we'd continue to just sign empty vote extensions once they're enabled.

Yes, this the way I see it: once extensions are activated, the app can choose to ignore the feature altogether by always extending with empty byte arrays. And that's what it'll see at PrepareProposal time: empty extensions