Skip to content

consensus: correctly save reference to previous height precommits (backport #9760)#9776

Merged
williambanfield merged 2 commits intov0.34.xfrom
mergify/bp/v0.34.x/pr-9760
Nov 28, 2022
Merged

consensus: correctly save reference to previous height precommits (backport #9760)#9776
williambanfield merged 2 commits intov0.34.xfrom
mergify/bp/v0.34.x/pr-9760

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Nov 28, 2022

This is an automatic backport of pull request #9760 done by Mergify.
Cherry-pick of da204d3 has failed:

On branch mergify/bp/v0.34.x/pr-9760
Your branch is up to date with 'origin/v0.34.x'.

You are currently cherry-picking commit da204d371.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   consensus/reactor.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   CHANGELOG_PENDING.md

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally


Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

)

This change reduces the number of Precommit messages sent to peers by 50%.

During the `ApplyNewRoundStepMessage`, we update the known state of the peer sending us the message.

We set the value of `ps.PRS.Precommits` to nil in this method if the peer is entering a new height or round.
https://github.com/tendermint/tendermint/blob/34ca3fb4741cdb205a4714d345218a0d5e9fefd0/consensus/reactor.go#L1368

We then assign `ps.PRS.LastCommit = ps.PRS.Precommits` if the peer is entering a new height only - this does not happen during just a round change. This therefore results in `ps.PRS.LastCommit` having the value `nil`.

When the `LastCommit` bit field is seen as `nil` in the reactor, an empty bit field is initialized.
https://github.com/tendermint/tendermint/blob/34ca3fb4741cdb205a4714d345218a0d5e9fefd0/consensus/reactor.go#L1273

The code responsible for gossiping votes from the previous height uses this `LastCommit` value and, seeing an empty `LastCommit` will resend every `Precommit` from the previous height since it lost the information it previously had detailing which precommits from the previous height the peer had.

This can be seen in the code responsible for gossiping precommits from the previous height:
https://github.com/tendermint/tendermint/blob/34ca3fb4741cdb205a4714d345218a0d5e9fefd0/consensus/reactor.go#L773

Where this code grabs the, previously `nil`, `LastCommit` bit field:
https://github.com/tendermint/tendermint/blob/34ca3fb4741cdb205a4714d345218a0d5e9fefd0/consensus/reactor.go#L1204-L1212

---

#### PR checklist

- [ ] Tests written/updated, or no tests needed
- [ ] `CHANGELOG_PENDING.md` updated, or no changelog entry needed
- [ ] Updated relevant documentation (`docs/`) and code comments, or no
      documentation updates needed

(cherry picked from commit da204d3)

# Conflicts:
#	CHANGELOG_PENDING.md
@mergify mergify bot requested a review from ebuchman as a code owner November 28, 2022 20:13
@mergify mergify bot requested a review from a team November 28, 2022 20:13
@mergify mergify bot requested review from adizere and lasarojc as code owners November 28, 2022 20:13
@mergify mergify bot added the conflicts label Nov 28, 2022
@williambanfield williambanfield merged commit 409d94a into v0.34.x Nov 28, 2022
@williambanfield williambanfield deleted the mergify/bp/v0.34.x/pr-9760 branch November 28, 2022 20:49
thanethomson added a commit to informalsystems/tendermint that referenced this pull request Feb 3, 2023
* Revert "Fixed ordering of match.events in light client RPC (tendermint#9877)"

* Revert "state/kvindexer: associate event attributes with events (tendermint#9759)"

* Revert "consensus: correctly save reference to previous height precommits (backport tendermint#9760) (tendermint#9776)"

* add peer gossip sleep (tendermint#241) (tendermint#245)

* add peer gossip sleep

* add changelog

* Update .changelog/unreleased/bug-fixes/4-busy-loop-send-block-part.md

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* Update .changelog/unreleased/bug-fixes/4-busy-loop-send-block-part.md

---------

Co-authored-by: jhernandezb <contact@jhernandez.me>
Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>
(cherry picked from commit 5954e75)

Co-authored-by: Sergio Mena <sergio@informal.systems>

* Build changelog for v0.34.25 release

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Bump version

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Thane Thomson <connect@thanethomson.com>
@moul moul mentioned this pull request Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant