Skip to content

consensus: check that proposal is non-nil before voting#7480

Merged
williambanfield merged 2 commits intowb/proposer-based-timestampsfrom
wb/proposal-nil-check
Dec 22, 2021
Merged

consensus: check that proposal is non-nil before voting#7480
williambanfield merged 2 commits intowb/proposer-based-timestampsfrom
wb/proposal-nil-check

Conversation

@williambanfield
Copy link
Contributor

@williambanfield williambanfield commented Dec 20, 2021

Response to debugging with @ancazamfir. Proposal can be nil if not sent.

Copy link
Contributor

@ancazamfir ancazamfir left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for finding the issue and the fix @williambanfield!

@williambanfield williambanfield merged commit 9ade45d into wb/proposer-based-timestamps Dec 22, 2021
@williambanfield williambanfield deleted the wb/proposal-nil-check branch December 22, 2021 16:09
adizere pushed a commit to cometbft/cometbft that referenced this pull request Feb 7, 2024
…2221)

Addresses #2119.

ADR 112, describing the PBTS implementation,
[states](https://github.com/cometbft/cometbft/pull/2223/files#diff-81d55f6e52eabd6cce264a76fba015de46092b9ccda241de10cd265890ab8ad9R302):

    The precommit step will not require much modification.
Its proposal validation rules will change in the same ways that
validation will change in the prevote step with the exception of the
`timely` check: precommit validation will never check that the timestamp
is `timely`.

The changes proposed here are in line with this description.

Moreover, this PR in fact revert the changes to the precommit step
(`enterPrecommit` method) added by
tendermint/tendermint#7480 and
tendermint/tendermint#7391.

---

#### PR checklist

- [x] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
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.

2 participants