From 409d94abac64f4ae9ac468193b6cf08cd41e50d3 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 28 Nov 2022 15:49:33 -0500 Subject: [PATCH] consensus: correctly save reference to previous height precommits (backport #9760) (#9776) * consensus: correctly save reference to previous height precommits (#9760) 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 Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> Co-authored-by: William Banfield --- CHANGELOG_PENDING.md | 2 ++ consensus/reactor.go | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index c62099fdb..435c491c7 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -18,5 +18,7 @@ ### IMPROVEMENTS +- [consensus] \#9760 Save peer LastCommit correctly to achieve 50% reduction in gossiped precommits. (@williambanfield) + ### BUG FIXES diff --git a/consensus/reactor.go b/consensus/reactor.go index e052f197a..bcd61675a 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -1375,6 +1375,7 @@ func (ps *PeerState) ApplyNewRoundStepMessage(msg *NewRoundStepMessage) { psRound := ps.PRS.Round psCatchupCommitRound := ps.PRS.CatchupCommitRound psCatchupCommit := ps.PRS.CatchupCommit + lastPrecommits := ps.PRS.Precommits startTime := tmtime.Now().Add(-1 * time.Duration(msg.SecondsSinceStartTime) * time.Second) ps.PRS.Height = msg.Height @@ -1402,7 +1403,7 @@ func (ps *PeerState) ApplyNewRoundStepMessage(msg *NewRoundStepMessage) { // Shift Precommits to LastCommit. if psHeight+1 == msg.Height && psRound == msg.LastCommitRound { ps.PRS.LastCommitRound = msg.LastCommitRound - ps.PRS.LastCommit = ps.PRS.Precommits + ps.PRS.LastCommit = lastPrecommits } else { ps.PRS.LastCommitRound = msg.LastCommitRound ps.PRS.LastCommit = nil