Skip to content

Commit

Permalink
Forward-port: update state to prevote nil when proposal block does …
Browse files Browse the repository at this point in the history
…not match locked block (cometbft#1203)

* consensus: update state to prevote nil when proposal block does not match locked block. (#6986)

* add failing test

* tweak comments in failing test

* failing test comment

* initial attempt at removing prevote locked block logic

* comment out broken function

* undo reset on prevotes

* fixing TestProposeValidBlock test

* update test for completed POL update

* comment updates

* further unlock testing

* update comments

* Update internal/consensus/state.go

* spacing nit

* comment cleanup

* nil check in addVote

* update unlock description

* update precommit on relock comment

* add ensure new timeout back

* rename IsZero to IsNil and replace uses of block len check with helper

* add testing.T to new assertions

* begin removing unlock condition

* fix TestStateProposerSelection2 to precommit for nil correctly

* remove erroneous sleep

* update TestStatePOL comment

* update relock test to be more clear

* add _ into test names

* rename slashing

* udpate no relock function to be cleaner

* do not relock on old proposal test cleanup

* con state name update

* remove all references to unlock

* update test comments to include new

* add relock test

* add ensureRelock to common_test

* remove all event unlock

* remove unlock checks

* no lint add space

* lint ++

* add test for nil prevote on different proposal

* fix prevote nil condition

* fix defaultDoPrevote

* state_test.go fixes to accomodate prevoting for nil

* add failing test for POL from previous round case

* update prevote logic to prevote POL from previous round

* state.go comment fixes

* update validatePrevotes to correctly look for nil

* update new test name and comment

* update POLFromPreviousRound test

* fixes post merge

* fix spacing

* make the linter happy

* change prevote log message

* update prevote nil debug line

* update enterPrevote comment

* lint

* Update internal/consensus/state.go

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>

* Update internal/consensus/state.go

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>

* add english description of alg rules

* Update internal/consensus/state.go

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>

* comment fixes from review

* fix comment

* fix comment

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>

* Fix UTs

* Addressed comments

* Add changelog

* Update consensus/state.go

Co-authored-by: Hernán Vanzetto <15466498+hvanz@users.noreply.github.com>

---------

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Hernán Vanzetto <15466498+hvanz@users.noreply.github.com>
  • Loading branch information
4 people authored and roy-dydx committed Apr 16, 2024
1 parent a503ffd commit f5ff487
Show file tree
Hide file tree
Showing 3 changed files with 434 additions and 74 deletions.
90 changes: 75 additions & 15 deletions consensus/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -1312,8 +1312,11 @@ func (cs *State) createProposalBlock(ctx context.Context) (*types.Block, error)

// Enter: `timeoutPropose` after entering Propose.
// Enter: proposal block and POL is ready.
// Prevote for LockedBlock if we're locked, or ProposalBlock if valid.
// Otherwise vote nil.
// If we received a valid proposal within this round and we are not locked on a block,
// we will prevote for block.
// Otherwise, if we receive a valid proposal that matches the block we are
// locked on or matches a block that received a POL in a round later than our
// locked round, prevote for the proposal, otherwise vote nil.
func (cs *State) enterPrevote(height int64, round int32) {
logger := cs.Logger.With("height", height, "round", round)

Expand Down Expand Up @@ -1343,14 +1346,7 @@ func (cs *State) enterPrevote(height int64, round int32) {
func (cs *State) defaultDoPrevote(height int64, round int32) {
logger := cs.Logger.With("height", height, "round", round)

// If a block is locked, prevote that.
if cs.LockedBlock != nil {
logger.Debug("prevote step; already locked on a block; prevoting locked block")
cs.signAddVote(cmtproto.PrevoteType, cs.LockedBlock.Hash(), cs.LockedBlockParts.Header(), nil)
return
}

// If ProposalBlock is nil, prevote nil.
// We did not receive a proposal within this round. (and thus executing this from a timeout)
if cs.ProposalBlock == nil {
logger.Debug("prevote step: ProposalBlock is nil")
cs.signAddVote(cmtproto.PrevoteType, nil, types.PartSetHeader{}, nil)
Expand Down Expand Up @@ -1393,11 +1389,75 @@ func (cs *State) defaultDoPrevote(height int64, round int32) {
return
}

// Prevote cs.ProposalBlock
// NOTE: the proposal signature is validated when it is received,
// and the proposal block parts are validated as they are received (against the merkle hash in the proposal)
logger.Debug("prevote step: ProposalBlock is valid")
cs.signAddVote(cmtproto.PrevoteType, cs.ProposalBlock.Hash(), cs.ProposalBlockParts.Header(), nil)
/*
22: upon <PROPOSAL, h_p, round_p, v, −1> from proposer(h_p, round_p) while step_p = propose do
23: if valid(v) && (lockedRound_p = −1 || lockedValue_p = v) then
24: broadcast <PREVOTE, h_p, round_p, id(v)>
25: else
26: broadcast <PREVOTE, h_p, round_p, nil>
Here, cs.Proposal.POLRound corresponds to the -1 in the above algorithm rule.
This means that the proposer is producing a new proposal that has not previously
seen a 2/3 majority by the network.
If we have already locked on a different value that is different from the proposed value,
we prevote nil since we are locked on a different value. Otherwise, if we're not locked on a block
or the proposal matches our locked block, we prevote the proposal.
*/
if cs.Proposal.POLRound == -1 {
if cs.LockedRound == -1 {
logger.Debug("prevote step: ProposalBlock is valid and there is no locked block; prevoting the proposal")
cs.signAddVote(cmtproto.PrevoteType, cs.ProposalBlock.Hash(), cs.ProposalBlockParts.Header(), nil)
return
}
if cs.ProposalBlock.HashesTo(cs.LockedBlock.Hash()) {
logger.Debug("prevote step: ProposalBlock is valid (POLRound is -1) and matches our locked block; prevoting the proposal")
cs.signAddVote(cmtproto.PrevoteType, cs.ProposalBlock.Hash(), cs.ProposalBlockParts.Header(), cs.LockedBlock)
return
}
logger.Debug("prevote step: ProposalBlock is valid (POLRound is -1), but doesn't match our locked block; prevoting nil")
cs.signAddVote(cmtproto.PrevoteType, nil, types.PartSetHeader{}, nil)
return
}

/*
28: upon <PROPOSAL, h_p, round_p, v, v_r> from proposer(h_p, round_p) AND 2f + 1 <PREVOTE, h_p, v_r, id(v)> while
step_p = propose && (v_r ≥ 0 && v_r < round_p) do
29: if valid(v) && (lockedRound_p ≤ v_r || lockedValue_p = v) then
30: broadcast <PREVOTE, h_p, round_p, id(v)>
31: else
32: broadcast <PREVOTE, h_p, round_p, nil>
This rule is a bit confusing but breaks down as follows:
If we see a proposal in the current round for value 'v' that lists its valid round as 'v_r'
AND this validator saw a 2/3 majority of the voting power prevote 'v' in round 'v_r', then we will
issue a prevote for 'v' in this round if 'v' is valid and either matches our locked value OR
'v_r' is a round greater than or equal to our current locked round.
'v_r' can be a round greater than to our current locked round if a 2/3 majority of
the network prevoted a value in round 'v_r' but we did not lock on it, possibly because we
missed the proposal in round 'v_r'.
*/
blockID, ok := cs.Votes.Prevotes(cs.Proposal.POLRound).TwoThirdsMajority()
ok = ok && !blockID.IsNil()
if ok && cs.ProposalBlock.HashesTo(blockID.Hash) && cs.Proposal.POLRound >= 0 && cs.Proposal.POLRound < cs.Round {
if cs.LockedRound <= cs.Proposal.POLRound {
logger.Debug("prevote step: ProposalBlock is valid and received a 2/3" +
"majority in a round later than the locked round; prevoting the proposal")
cs.signAddVote(cmtproto.PrevoteType, cs.ProposalBlock.Hash(), cs.ProposalBlockParts.Header(), cs.ProposalBlock)
return
}
if cs.ProposalBlock.HashesTo(cs.LockedBlock.Hash()) {
logger.Debug("prevote step: ProposalBlock is valid and matches our locked block; prevoting the proposal")
cs.signAddVote(cmtproto.PrevoteType, cs.ProposalBlock.Hash(), cs.ProposalBlockParts.Header(), cs.LockedBlock)
return
}
}

logger.Debug("prevote step: ProposalBlock is valid but was not our locked block or" +
"did not receive a more recent majority; prevoting nil")
cs.signAddVote(cmtproto.PrevoteType, nil, types.PartSetHeader{}, nil)
}

// Enter: any +2/3 prevotes at next round.
Expand Down
Loading

0 comments on commit f5ff487

Please sign in to comment.