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

What should Tendermint do when a precommit message is received without Extension #8174

Closed
4 tasks
sergio-mena opened this issue Mar 21, 2022 · 5 comments
Closed
4 tasks
Assignees

Comments

@sergio-mena
Copy link
Contributor

sergio-mena commented Mar 21, 2022

Problem Definition

When a precommit messsge is received, VerifyVoteExtension is called even if the message does not have an extension or the extension has length 0, according to the current version of the ABCI++ spec

The question is whether is would make sense to skip the verification in the case of empty or non-existent extension. This probably depends on the use cases for vote extensions. Currently we don't see an obvious reason not skip the verification under such conditions.

Once a decision is reached, we need to adapt the implementation (should be quick). Also, the spec should be updated to clearly describe the approach described (probably in section "When does Tendermint use it")


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@sergio-mena
Copy link
Contributor Author

The discussion during yesterday's ABCI++ bi-weekly went around the following three cases:
a) The App does not extend the vote (nil-byte array)
b) The App returns a 0-length extension (0-length byte array)
c) The App returns at least 1 byte as extension (the normal case)

@williambanfield rightly pointed out that, according to the latest vote extension design, if we allow a) as a possibility, an intermediate node in the gossip network could always remove a vote extension from the vote message. Therefore we must exclude a) as a possibility.

The second decision taken, which was the initial goal of this discussion, is that VerifyVoteExtension will also be called in the case the extension received has 0 length.

In conclusion:

  • If the App does not have a vote extension to provide, it returns a 0-byte array in ResponseExtendVote
    • Then, Tendermint will construct and sign the CanonicalVoteExtension structure in memory, including the empty byte array.
    • The (possibly 0-length) vote extension, together with its signature MUST be part of the precommit message sent
  • If Tendermint receives a precommit message without a vote extension (which can be the empty byte array) AND its signature, it discards the message
  • If Tendermint receives a precommit message with the empty byte array as vote extension, together with its (valid) signature, it MUST call RequestVeryfyVoteExtension with the empty byte array.

Please feel free to correct any inaccuracy in the text above.

I will open a PR to address this in the spec.

sergio-mena added a commit that referenced this issue Mar 28, 2022
* Clarify 0-length vote extensions in the spec, according to #8174

* Update spec so that Tendermnit can propose more txs than the size limit in

* Addressed Manu's comment

* Reworded size limit following Manu's suggestion
@williambanfield
Copy link
Contributor

@sergio-mena is there anything left to do for this issue?

@sergio-mena
Copy link
Contributor Author

@sergio-mena is there anything left to do for this issue?

This should be closed by @thanethomson's PR #8141

@sergio-mena
Copy link
Contributor Author

@thanethomson please make sure we reject a precommit message that does not contain the vote extension AND its signature

@sergio-mena
Copy link
Contributor Author

Closed by #8141

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

No branches or pull requests

3 participants