From 08b4fb6fbd0db23d180de488b1903de7c30019cb Mon Sep 17 00:00:00 2001 From: Teddy Ding Date: Wed, 6 Sep 2023 10:18:49 -0400 Subject: [PATCH] backport: #1203 and #1231 (#21) --- consensus/state.go | 189 +++++++++++--------- consensus/state_test.go | 385 +++++++++++++++++++++++++++++++++++----- types/vote_set_test.go | 20 +-- 3 files changed, 460 insertions(+), 134 deletions(-) diff --git a/consensus/state.go b/consensus/state.go index 7ba9113be..da1205b82 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -1244,8 +1244,11 @@ func (cs *State) createProposalBlock() (*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) @@ -1275,17 +1278,9 @@ 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. - // TODO(CORE-434): Incorporate fix from Proposer-based Timestamp. - 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()) - 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") + logger.Debug("prevote step: ProposalBlock is nil; prevoting nil") cs.signAddVote(cmtproto.PrevoteType, nil, types.PartSetHeader{}) return } @@ -1300,90 +1295,124 @@ func (cs *State) defaultDoPrevote(height int64, round int32) { return } - /// The following implements line 28 - 33 of algorithm: - // - // upon ⟨PROPOSAL, h_p, round_p, v, vr⟩ from proposer(h_p, round_p) - // AND 2f + 1 ⟨PREVOTE, h_p, vr, id(v)⟩ - // while step_p = propose ∧ (vr ≥ 0 ∧ vr < round_p) do { - // if valid(v) ∧ (lockedRound_p ≤ vr ∨ lockedValue_p = v) { - // broadcast ⟨PREVOTE, h_p, round_p, id(v)⟩ - // } else { - // broadcast ⟨PREVOTE, h_p, round_p, nil⟩ - // } - // step_p ← prevote - // } - // - // Determine if the proposed block has a sane, non-nil proof-of-lock. - if cs.Proposal.POLRound >= 0 && cs.Proposal.POLRound < cs.Round { - // Validate the proof-of-lock using known prevotes. - blockID, ok := cs.Votes.Prevotes(cs.Proposal.POLRound).TwoThirdsMajority() - if ok && cs.ProposalBlock.HashesTo(blockID.Hash) { - // Validate the proof-of-lock round is at least as new as the possible locked round. - // (vr >= 0, vr > round_p, 2f+1 prevotes at round vr, lockedRound_p <= vr) execute 30. - // Note we skipped the `valid(v)`` check, since at POLRound we've witnessed 2/3+ prevotes for `v`. - // This means 1/3+ honest validators have accepted the block. - if cs.Proposal.POLRound >= cs.LockedRound { - logger.Debug("prevote step: ProposalBlock POLRound >= LockedRound; prevoting the block") + /* + 22: upon from proposer(h_p, round_p) while step_p = propose do + 23: if valid(v) && (lockedRound_p = −1 || lockedValue_p = v) then + 24: broadcast + 25: else + 26: broadcast + + Here, cs.Proposal.POLRound corresponds to the -1 in the rule of the pseude-code (line 22). + This means that the proposer is producing a new proposal that has not previously + seen a 2/3 majority by the network. + + If the application deems the proposal as valid AND we're not locked on a + block OR the proposal matches our locked block (line 23), we prevote the + proposal (line 24). + + Otherwise, we have already locked on a value that is different from the + proposed value, so we prevote nil (line 26). + + Note that there are two cases on which we know that the proposal is + application-valid, that is, it was validated by the application at least + by one correct node in a previous step: + - when the proposal matches our non-nil valid block AND we're not locked on a block, and + - when the proposal matches our non-nil locked block. + In these cases we do not need to query the application to validate the + proposal. + */ + if cs.Proposal.POLRound == -1 { + if cs.LockedRound == -1 { + if cs.ValidRound != -1 && cs.ProposalBlock.HashesTo(cs.ValidBlock.Hash()) { + logger.Debug("prevote step: ProposalBlock matches our valid block; prevoting the proposal") cs.signAddVote(cmtproto.PrevoteType, cs.ProposalBlock.Hash(), cs.ProposalBlockParts.Header()) return } - // TODO(CORE-434): The following implements the canonical Tendermint algorithm, but the condition - // is never true due to locked block logic (line 1275) above, a known bug in current implementation. - // Uncomment the following when locked block logic is fixed. + // We request the Application, via a `ProcessProposal` ABCI call, to + // confirm that the block is valid. If the application does not + // accept the block, consensus prevotes nil. // - // Validate the proposed block is equal to our locked block. - // (vr >= 0, vr > round_p, 2f+1 prevotes at round vr, lockedRound_p <= vr) execute 30. - // if cs.ProposalBlock.HashesTo(cs.LockedBlock.Hash()) { - // logger.Debug("prevote step: ProposalBlock matches our locked block; prevoting the block") - // cs.signAddVote(cmtproto.PrevoteType, cs.ProposalBlock.Hash(), cs.ProposalBlockParts.Header()) - // return - // } - - // Proof-of-lock is before our locked round. - // (else case on line 31) execute line 32. - logger.Debug("prevote step: ProposalBlock POLRound < LockedRound; prevoting nil") - cs.signAddVote(cmtproto.PrevoteType, nil, types.PartSetHeader{}) + // WARNING: misuse of block rejection by the Application can seriously compromise + // the liveness properties of consensus. + // Please see `PrepareProosal`-`ProcessProposal` coherence and determinism properties + // in the ABCI++ specification. + isAppValid, err := cs.blockExec.ProcessProposal(cs.ProposalBlock, cs.state) + if err != nil { + panic(fmt.Sprintf( + "state machine returned an error (%v) when calling ProcessProposal", err, + )) + } + cs.metrics.MarkProposalProcessed(isAppValid) + + if !isAppValid { + logger.Error("prevote step: state machine rejected a proposed block; this should not happen:"+ + "the proposer may be misbehaving; prevoting nil", "err", err) + cs.signAddVote(cmtproto.PrevoteType, nil, types.PartSetHeader{}) + return + } + + 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()) + 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()) return } - // Could not validate proof-of-lock +2/3 majority. Prevote nil even if the block is valid. - logger.Debug("prevote step: ProposalBlock proof-of-lock not validated; prevoting nil") + 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{}) return } /* - Before prevoting on the block received from the proposer for the current round and height, - we request the Application, via `ProcessProposal` ABCI call, to confirm that the block is - valid. If the Application does not accept the block, consensus prevotes `nil`. - - WARNING: misuse of block rejection by the Application can seriously compromise - the liveness properties of consensus. - Please see `PrepareProosal`-`ProcessProposal` coherence and determinism properties - in the ABCI++ specification. + 28: upon from proposer(h_p, round_p) AND 2f + 1 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 + 31: else + 32: broadcast + + This rule is a bit confusing but breaks down as follows: + + First note that 'valid(v)' in line 29 states that we should request the + application to validate the proposal. We know that the proposal was + prevoted by a +2/3 majority, so it must have been prevoted and validated + at least by one correct node. Therefore it must be valid and in the + following cases we don't need to query the application again. + + 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 for 'v' in round 'v_r' (line 28), + then we will issue a prevote for 'v' in this round (line 30) if 'v' either matches our locked value OR + 'v_r' is a round greater than or equal to our current locked round (line 29). + Otherwise we prevote nil (line 32). + + Note that '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'. */ - isAppValid, err := cs.blockExec.ProcessProposal(cs.ProposalBlock, cs.state) - if err != nil { - panic(fmt.Sprintf( - "state machine returned an error (%v) when calling ProcessProposal", err, - )) - } - cs.metrics.MarkProposalProcessed(isAppValid) - - // Vote nil if the Application rejected the block - if !isAppValid { - logger.Error("prevote step: state machine rejected a proposed block; this should not happen:"+ - "the proposer may be misbehaving; prevoting nil", "err", err) - cs.signAddVote(cmtproto.PrevoteType, nil, types.PartSetHeader{}) - return + 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()) + 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()) + 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()) + 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{}) } // Enter: any +2/3 prevotes at next round. diff --git a/consensus/state_test.go b/consensus/state_test.go index f061c862a..01bfdbece 100644 --- a/consensus/state_test.go +++ b/consensus/state_test.go @@ -479,8 +479,8 @@ func TestStateLockNoPOL(t *testing.T) { signAddVotes(cs1, cmtproto.PrevoteType, theBlockHash, thePartSetHeader, vs2) ensurePrevote(voteCh, height, round) // prevote - ensurePrecommit(voteCh, height, round) // precommit // the proposed block should now be locked and our precommit added + ensurePrecommit(voteCh, height, round) // precommit validatePrecommit(t, cs1, round, round, vss[0], theBlockHash, theBlockHash) // we should now be stuck in limbo forever, waiting for more precommits @@ -513,10 +513,9 @@ func TestStateLockNoPOL(t *testing.T) { require.Nil(t, rs.ProposalBlock, "Expected proposal block to be nil") - // wait to finish prevote + // we should have prevoted nil since we did not see a proposal in the round. ensurePrevote(voteCh, height, round) - // we should have prevoted our locked block - validatePrevote(t, cs1, round, vss[0], rs.LockedBlock.Hash()) + validatePrevote(t, cs1, round, vss[0], nil) // add a conflicting prevote from the other validator bps, err := rs.LockedBlock.MakePartSet(partSize) @@ -529,9 +528,9 @@ func TestStateLockNoPOL(t *testing.T) { // and then prevote wait, which should timeout. then wait for precommit ensureNewTimeout(timeoutWaitCh, height, round, cs1.config.Prevote(round).Nanoseconds()) - ensurePrecommit(voteCh, height, round) // precommit - // the proposed block should still be locked and our precommit added - // we should precommit nil and be locked on the proposal + // the proposed block should still be locked block. + // we should precommit nil and be locked on the proposal. + ensurePrecommit(voteCh, height, round) validatePrecommit(t, cs1, round, 0, vss[0], nil, theBlockHash) // add conflicting precommit from vs2 @@ -614,9 +613,10 @@ func TestStateLockNoPOL(t *testing.T) { } ensureNewProposal(proposalCh, height, round) - ensurePrevote(voteCh, height, round) // prevote - // prevote for locked block (not proposal) - validatePrevote(t, cs1, 3, vss[0], cs1.LockedBlock.Hash()) + + // prevote for nil since we did not see a proposal for our locked block in the round. + ensurePrevote(voteCh, height, round) + validatePrevote(t, cs1, 3, vss[0], nil) // prevote for proposed block bps4, err := propBlock.MakePartSet(partSize) @@ -724,11 +724,9 @@ func TestStateLockPOLUpdateLock(t *testing.T) { // ensure that the validator receives the proposal. ensureNewProposal(proposalCh, height, round) - // Prevote our locked block. - // TODO: Ensure we prevote for the proposal if it is valid and from a round greater than - // the valid round: https://github.com/tendermint/tendermint/issues/6850. + // Prevote our nil since the proposal does not match our locked block. ensurePrevote(voteCh, height, round) - validatePrevote(t, cs1, round, vss[0], theBlockHash) + validatePrevote(t, cs1, round, vss[0], nil) // Add prevotes from the remainder of the validators for the new locked block. signAddVotes(cs1, cmtproto.PrevoteType, propBlockR1Hash, propBlockR1Parts.Header(), vs2, vs3, vs4) @@ -743,6 +741,176 @@ func TestStateLockPOLUpdateLock(t *testing.T) { validatePrecommit(t, cs1, round, round, vss[0], propBlockR1Hash, propBlockR1Hash) } +// TestStateLockPrevoteNilWhenLockedAndMissProposal tests that a validator prevotes nil +// if it is locked on a block and misses the proposal in a round. +func TestStateLockPrevoteNilWhenLockedAndMissProposal(t *testing.T) { + cs1, vss := randState(4) + vs2, vs3, vs4 := vss[1], vss[2], vss[3] + height, round := cs1.Height, cs1.Round + + timeoutWaitCh := subscribe(cs1.eventBus, types.EventQueryTimeoutWait) + proposalCh := subscribe(cs1.eventBus, types.EventQueryCompleteProposal) + pv1, err := cs1.privValidator.GetPubKey() + require.NoError(t, err) + addr := pv1.Address() + voteCh := subscribeToVoter(cs1, addr) + lockCh := subscribe(cs1.eventBus, types.EventQueryLock) + newRoundCh := subscribe(cs1.eventBus, types.EventQueryNewRound) + + /* + Round 0: + cs1 creates a proposal for block B. + Send a prevote for B from each of the validators to cs1. + Send a precommit for nil from all of the validators to cs1. + This ensures that cs1 will lock on B in this round but not precommit it. + */ + t.Log("### Starting Round 0") + + startTestRound(cs1, height, round) + + ensureNewRound(newRoundCh, height, round) + ensureNewProposal(proposalCh, height, round) + rs := cs1.GetRoundState() + theBlockHash := rs.ProposalBlock.Hash() + theBlockParts := rs.ProposalBlockParts + + ensurePrevote(voteCh, height, round) + + signAddVotes(cs1, cmtproto.PrevoteType, theBlockHash, theBlockParts.Header(), vs2, vs3, vs4) + + // check that the validator generates a Lock event. + ensureLock(lockCh, height, round) + + // the proposed block should now be locked and our precommit added. + ensurePrecommit(voteCh, height, round) + validatePrecommit(t, cs1, round, round, vss[0], theBlockHash, theBlockHash) + + // add precommits from the rest of the validators. + signAddVotes(cs1, cmtproto.PrecommitType, nil, types.PartSetHeader{}, vs2, vs3, vs4) + + // timeout to new round. + ensureNewTimeout(timeoutWaitCh, height, round, cs1.config.Precommit(round).Nanoseconds()) + + /* + Round 1: + Send a prevote for nil from each of the validators to cs1. + Send a precommit for nil from all of the validtors to cs1. + Check that cs1 prevotes nil instead of its locked block, but ensure + that it maintains its locked block. + */ + t.Log("### Starting Round 1") + incrementRound(vs2, vs3, vs4) + round++ + + ensureNewRound(newRoundCh, height, round) + + // Prevote our nil. + ensurePrevote(voteCh, height, round) + validatePrevote(t, cs1, round, vss[0], nil) + + // Add prevotes from the remainder of the validators nil. + signAddVotes(cs1, cmtproto.PrevoteType, nil, types.PartSetHeader{}, vs2, vs3, vs4) + ensurePrecommit(voteCh, height, round) + // We should now be locked on the same block but with an updated locked round. + validatePrecommit(t, cs1, round, 0, vss[0], nil, theBlockHash) +} + +// TestStateLock_PrevoteNilWhenLockedAndMissProposal tests that a validator prevotes nil +// if it is locked on a block and misses the proposal in a round. +func TestStateLockPrevoteNilWhenLockedAndDifferentProposal(t *testing.T) { + _, cancel := context.WithCancel(context.Background()) + defer cancel() + + /* + All of the assertions in this test occur on the `cs1` validator. + The test sends signed votes from the other validators to cs1 and + cs1's state is then examined to verify that it now matches the expected + state. + */ + + cs1, vss := randState(4) + vs2, vs3, vs4 := vss[1], vss[2], vss[3] + height, round := cs1.Height, cs1.Round + + timeoutWaitCh := subscribe(cs1.eventBus, types.EventQueryTimeoutWait) + proposalCh := subscribe(cs1.eventBus, types.EventQueryCompleteProposal) + pv1, err := cs1.privValidator.GetPubKey() + require.NoError(t, err) + addr := pv1.Address() + voteCh := subscribeToVoter(cs1, addr) + lockCh := subscribe(cs1.eventBus, types.EventQueryLock) + newRoundCh := subscribe(cs1.eventBus, types.EventQueryNewRound) + + /* + Round 0: + cs1 creates a proposal for block B. + Send a prevote for B from each of the validators to cs1. + Send a precommit for nil from all of the validators to cs1. + This ensures that cs1 will lock on B in this round but not precommit it. + */ + t.Log("### Starting Round 0") + startTestRound(cs1, height, round) + + ensureNewRound(newRoundCh, height, round) + ensureNewProposal(proposalCh, height, round) + rs := cs1.GetRoundState() + theBlockHash := rs.ProposalBlock.Hash() + theBlockParts := rs.ProposalBlockParts + + ensurePrevote(voteCh, height, round) + + signAddVotes(cs1, cmtproto.PrevoteType, theBlockHash, theBlockParts.Header(), vs2, vs3, vs4) + + // check that the validator generates a Lock event. + ensureLock(lockCh, height, round) + + // the proposed block should now be locked and our precommit added. + ensurePrecommit(voteCh, height, round) + validatePrecommit(t, cs1, round, round, vss[0], theBlockHash, theBlockHash) + + // add precommits from the rest of the validators. + signAddVotes(cs1, cmtproto.PrecommitType, nil, types.PartSetHeader{}, vs2, vs3, vs4) + + // timeout to new round. + ensureNewTimeout(timeoutWaitCh, height, round, cs1.config.Precommit(round).Nanoseconds()) + + /* + Round 1: + Create a proposal for a new block. + Send a prevote for nil from each of the validators to cs1. + Send a precommit for nil from all of the validtors to cs1. + Check that cs1 prevotes nil instead of its locked block, but ensure + that it maintains its locked block. + */ + t.Log("### Starting Round 1") + incrementRound(vs2, vs3, vs4) + round++ + cs2 := newState(cs1.state, vs2, kvstore.NewApplication()) + propR1, propBlockR1 := decideProposal(t, cs2, vs2, vs2.Height, vs2.Round) + propBlockR1Parts, err := propBlockR1.MakePartSet(types.BlockPartSizeBytes) + require.NoError(t, err) + + propBlockR1Hash := propBlockR1.Hash() + require.NotEqual(t, propBlockR1Hash, theBlockHash) + if err := cs1.SetProposalAndBlock(propR1, propBlockR1, propBlockR1Parts, "some peer"); err != nil { + t.Fatal(err) + } + + ensureNewRound(newRoundCh, height, round) + ensureNewProposal(proposalCh, height, round) + + // Prevote our nil. + ensurePrevote(voteCh, height, round) + validatePrevote(t, cs1, round, vss[0], nil) + + // Add prevotes from the remainder of the validators for nil. + signAddVotes(cs1, cmtproto.PrevoteType, nil, types.PartSetHeader{}, vs2, vs3, vs4) + + // We should now be locked on the same block but prevote nil. + ensurePrecommit(voteCh, height, round) + validatePrecommit(t, cs1, round, 0, vss[0], nil, theBlockHash) +} + // TestStateLockPOLDoesNotUnlock tests that a validator maintains its locked block // despite receiving +2/3 nil prevotes and nil precommits from other validators. // Tendermint used to 'unlock' its locked block when greater than 2/3 prevotes @@ -751,7 +919,6 @@ func TestStateLockPOLUpdateLock(t *testing.T) { func TestStateLockPOLDoesNotUnlock(t *testing.T) { _, cancel := context.WithCancel(context.Background()) defer cancel() - /* All of the assertions in this test occur on the `cs1` validator. The test sends signed votes from the other validators to cs1 and @@ -824,9 +991,12 @@ func TestStateLockPOLDoesNotUnlock(t *testing.T) { t.Log("#### ONTO ROUND 1") round++ incrementRound(vs2, vs3, vs4) - prop, propBlock := decideProposal(t, cs1, vs2, vs2.Height, vs2.Round) + cs2 := newState(cs1.state, vs2, kvstore.NewApplication()) + prop, propBlock := decideProposal(t, cs2, vs2, vs2.Height, vs2.Round) propBlockParts, err := propBlock.MakePartSet(types.BlockPartSizeBytes) require.NoError(t, err) + + require.NotEqual(t, propBlock.Hash(), theBlockHash) if err := cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, ""); err != nil { t.Fatal(err) } @@ -835,12 +1005,10 @@ func TestStateLockPOLDoesNotUnlock(t *testing.T) { ensureNewProposal(proposalCh, height, round) - // prevote for the locked block. We do not currently prevote for the - // proposal. - // TODO: do not prevote the locked block if it does not match the proposal. - // (https://github.com/tendermint/tendermint/issues/6850) + // Prevote for nil since the proposed block does not match our locked block. ensurePrevote(voteCh, height, round) - validatePrevote(t, cs1, round, vss[0], theBlockHash) + validatePrevote(t, cs1, round, vss[0], nil) + // add >2/3 prevotes for nil from all other validators signAddVotes(cs1, cmtproto.PrevoteType, nil, types.PartSetHeader{}, vs2, vs3, vs4) @@ -851,7 +1019,6 @@ func TestStateLockPOLDoesNotUnlock(t *testing.T) { signAddVotes(cs1, cmtproto.PrecommitType, nil, types.PartSetHeader{}, vs2, vs3, vs4) ensureNewTimeout(timeoutWaitCh, height, round, cs1.config.Precommit(round).Nanoseconds()) - /* Round 2: The validator cs1 saw >2/3 precommits for nil in the previous round. @@ -861,9 +1028,11 @@ func TestStateLockPOLDoesNotUnlock(t *testing.T) { t.Log("#### ONTO ROUND 2") round++ incrementRound(vs2, vs3, vs4) - prop, propBlock = decideProposal(t, cs1, vs3, vs3.Height, vs3.Round) + cs3 := newState(cs1.state, vs2, kvstore.NewApplication()) + prop, propBlock = decideProposal(t, cs3, vs3, vs3.Height, vs3.Round) propBlockParts, err = propBlock.MakePartSet(types.BlockPartSizeBytes) require.NoError(t, err) + if err := cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, ""); err != nil { t.Fatal(err) } @@ -872,8 +1041,9 @@ func TestStateLockPOLDoesNotUnlock(t *testing.T) { ensureNewProposal(proposalCh, height, round) + // Prevote for nil since the proposal does not match our locked block. ensurePrevote(voteCh, height, round) - validatePrevote(t, cs1, round, vss[0], theBlockHash) + validatePrevote(t, cs1, round, vss[0], nil) signAddVotes(cs1, cmtproto.PrevoteType, nil, types.PartSetHeader{}, vs2, vs3, vs4) @@ -957,9 +1127,9 @@ func TestStateLockMissingProposalWhenPOLSeenDoesNotUpdateLock(t *testing.T) { ensureNewRound(newRoundCh, height, round) - // go to prevote, node should prevote for locked block (not the new proposal) - this is relocking + // prevote for nil since the proposal was not seen. ensurePrevote(voteCh, height, round) - validatePrevote(t, cs1, round, vss[0], firstBlockHash) + validatePrevote(t, cs1, round, vss[0], nil) // now lets add prevotes from everyone else for the new block signAddVotes(cs1, cmtproto.PrevoteType, secondBlockHash, secondBlockParts.Header(), vs2, vs3, vs4) @@ -1087,15 +1257,14 @@ func TestStateLockPOLSafety1(t *testing.T) { ensureNewTimeout(timeoutWaitCh, height, round, cs1.config.Precommit(round).Nanoseconds()) t.Log("### ONTO ROUND 1") - - prop, propBlock := decideProposal(t, cs1, vs2, vs2.Height, vs2.Round+1) + incrementRound(vs2, vs3, vs4) + round++ // moving to the next round + cs2 := newState(cs1.state, vs2, kvstore.NewApplication()) + prop, propBlock := decideProposal(t, cs2, vs2, vs2.Height, vs2.Round) propBlockHash := propBlock.Hash() propBlockParts, err := propBlock.MakePartSet(partSize) require.NoError(t, err) - incrementRound(vs2, vs3, vs4) - - round++ // moving to the next round ensureNewRound(newRoundCh, height, round) //XXX: this isnt guaranteed to get there before the timeoutPropose ... @@ -1112,9 +1281,8 @@ func TestStateLockPOLSafety1(t *testing.T) { rs = cs1.GetRoundState() if rs.LockedBlock != nil { - panic("we should not be locked!") + t.Fatalf("was not expected to be locked on a block") } - t.Logf("new prop hash %v", fmt.Sprintf("%X", propBlockHash)) // go to prevote, prevote for proposal block ensurePrevote(voteCh, height, round) @@ -1146,8 +1314,8 @@ func TestStateLockPOLSafety1(t *testing.T) { // finish prevote ensurePrevote(voteCh, height, round) - // we should prevote what we're locked on - validatePrevote(t, cs1, round, vss[0], propBlockHash) + // we should prevote for nil + validatePrevote(t, cs1, round, vss[0], nil) newStepCh := subscribe(cs1.eventBus, types.EventQueryNewRoundStep) @@ -1259,6 +1427,141 @@ func TestStateLockPOLSafety2(t *testing.T) { } +// TestStatePrevotePOLFromPreviousRound tests that a validator will prevote +// for a block if it is locked on a different block but saw a POL for the block +// it is not locked on in a previous round. +func TestStatePrevotePOLFromPreviousRound(t *testing.T) { + _, cancel := context.WithCancel(context.Background()) + defer cancel() + + cs1, vss := randState(4) + vs2, vs3, vs4 := vss[1], vss[2], vss[3] + height, round := cs1.Height, cs1.Round + + partSize := types.BlockPartSizeBytes + + timeoutWaitCh := subscribe(cs1.eventBus, types.EventQueryTimeoutWait) + proposalCh := subscribe(cs1.eventBus, types.EventQueryCompleteProposal) + pv1, err := cs1.privValidator.GetPubKey() + require.NoError(t, err) + addr := pv1.Address() + voteCh := subscribeToVoter(cs1, addr) + lockCh := subscribe(cs1.eventBus, types.EventQueryLock) + newRoundCh := subscribe(cs1.eventBus, types.EventQueryNewRound) + + /* + Round 0: + cs1 creates a proposal for block B. + Send a prevote for B from each of the validators to cs1. + Send a precommit for nil from all of the validators to cs1. + This ensures that cs1 will lock on B in this round but not precommit it. + */ + t.Log("### Starting Round 0") + + startTestRound(cs1, height, round) + + ensureNewRound(newRoundCh, height, round) + ensureNewProposal(proposalCh, height, round) + rs := cs1.GetRoundState() + theBlockHash := rs.ProposalBlock.Hash() + theBlockParts := rs.ProposalBlockParts.Header() + + ensurePrevote(voteCh, height, round) + + signAddVotes(cs1, cmtproto.PrevoteType, theBlockHash, theBlockParts, vs2, vs3, vs4) + + // check that the validator generates a Lock event. + ensureLock(lockCh, height, round) + + // the proposed block should now be locked and our precommit added. + ensurePrecommit(voteCh, height, round) + validatePrecommit(t, cs1, round, round, vss[0], theBlockHash, theBlockHash) + + // add precommits from the rest of the validators. + signAddVotes(cs1, cmtproto.PrecommitType, nil, types.PartSetHeader{}, vs2, vs3, vs4) + + // timeout to new round. + ensureNewTimeout(timeoutWaitCh, height, round, cs1.config.Precommit(round).Nanoseconds()) + + /* + Round 1: + Create a block, D but do not send a proposal for it to cs1. + Send a prevote for D from each of the validators to cs1 so that cs1 sees a POL. + Send a precommit for nil from all of the validtors to cs1. + cs1 has now seen greater than 2/3 of the voting power prevote D in this round + but cs1 did not see the proposal for D in this round so it will not prevote or precommit it. + */ + t.Log("### Starting Round 1") + incrementRound(vs2, vs3, vs4) + round++ + // Generate a new proposal block. + cs2 := newState(cs1.state, vs2, kvstore.NewApplication()) + cs2.ValidRound = 1 + propR1, propBlockR1 := decideProposal(t, cs2, vs2, vs2.Height, round) + t.Log(propR1.POLRound) + propBlockR1Parts, err := propBlockR1.MakePartSet(partSize) + require.NoError(t, err) + + propBlockR1Hash := propBlockR1.Hash() + require.NotEqual(t, propBlockR1Hash, theBlockHash) + + ensureNewRound(newRoundCh, height, round) + + signAddVotes(cs1, cmtproto.PrevoteType, propBlockR1Hash, propBlockR1Parts.Header(), vs2, vs3, vs4) + + ensurePrevote(voteCh, height, round) + validatePrevote(t, cs1, round, vss[0], nil) + + signAddVotes(cs1, cmtproto.PrecommitType, nil, types.PartSetHeader{}, vs2, vs3, vs4) + + ensurePrecommit(voteCh, height, round) + + // timeout to new round. + ensureNewTimeout(timeoutWaitCh, height, round, cs1.config.Precommit(round).Nanoseconds()) + + /* + Round 2: + Create a new proposal for D, the same block from Round 1. + cs1 already saw greater than 2/3 of the voting power on the network vote for + D in a previous round, so it should prevote D once it receives a proposal for it. + cs1 does not need to receive prevotes from other validators before the proposal + in this round. It will still prevote the block. + Send cs1 prevotes for nil and check that it still prevotes its locked block + and not the block that it prevoted. + */ + t.Log("### Starting Round 2") + incrementRound(vs2, vs3, vs4) + round++ + propBlockID := types.BlockID{Hash: propBlockR1Hash, PartSetHeader: propBlockR1Parts.Header()} + propR2 := types.NewProposal(height, round, 1, propBlockID) + p := propR2.ToProto() + if err := vs3.SignProposal(cs1.state.ChainID, p); err != nil { + t.Fatalf("error signing proposal: %s", err) + } + propR2.Signature = p.Signature + + // cs1 receives a proposal for D, the block that received a POL in round 1. + if err := cs1.SetProposalAndBlock(propR2, propBlockR1, propBlockR1Parts, ""); err != nil { + t.Fatal(err) + } + + ensureNewRound(newRoundCh, height, round) + + ensureNewProposal(proposalCh, height, round) + + // We should now prevote this block, despite being locked on the block from + // round 0. + ensurePrevote(voteCh, height, round) + validatePrevote(t, cs1, round, vss[0], propBlockR1Hash) + + signAddVotes(cs1, cmtproto.PrevoteType, nil, types.PartSetHeader{}, vs2, vs3, vs4) + + // cs1 did not receive a POL within this round, so it should remain locked + // on the block from round 0. + ensurePrecommit(voteCh, height, round) + validatePrecommit(t, cs1, round, 0, vss[0], nil, theBlockHash) +} + // 4 vals. // polka P0 at R0 for B0. We lock B0 on P0 at R0. @@ -1315,10 +1618,9 @@ func TestProposeValidBlock(t *testing.T) { // timeout of propose ensureNewTimeout(timeoutProposeCh, height, round, cs1.config.Propose(round).Nanoseconds()) - // We did not see a valid proposal within this round, so prevote our locked block. - // TODO: prevote nil + // We did not see a valid proposal within this round, so prevote nil. ensurePrevote(voteCh, height, round) - validatePrevote(t, cs1, round, vss[0], propBlockHash) + validatePrevote(t, cs1, round, vss[0], nil) signAddVotes(cs1, cmtproto.PrecommitType, nil, types.PartSetHeader{}, vs2, vs3, vs4) @@ -1985,17 +2287,16 @@ func TestStateHalt1(t *testing.T) { round++ // moving to the next round ensureNewRound(newRoundCh, height, round) - rs = cs1.GetRoundState() t.Log("### ONTO ROUND 1") /*Round2 - // we timeout and prevote our lock + // we timeout and prevote // a polka happened but we didn't see it! */ - // go to prevote, prevote for locked block + // prevote for nil since we did not receive a proposal in this round. ensurePrevote(voteCh, height, round) - validatePrevote(t, cs1, round, vss[0], rs.LockedBlock.Hash()) + validatePrevote(t, cs1, round, vss[0], nil) // now we receive the precommit from the previous round addVotes(cs1, precommit4) diff --git a/types/vote_set_test.go b/types/vote_set_test.go index edb47d228..e50e18c2f 100644 --- a/types/vote_set_test.go +++ b/types/vote_set_test.go @@ -166,7 +166,7 @@ func TestVoteSet_2_3Majority(t *testing.T) { _, err = signAddVote(privValidators[7], vote, voteSet) require.NoError(t, err) blockID, ok = voteSet.TwoThirdsMajority() - assert.True(t, ok || blockID.IsNil(), "there should be 2/3 majority for nil") + assert.True(t, ok && blockID.IsNil(), "there should be 2/3 majority for nil") } } @@ -188,7 +188,7 @@ func TestVoteSet_2_3MajorityRedux(t *testing.T) { BlockID: BlockID{blockHash, blockPartSetHeader}, } - // 66 out of 100 voted for nil. + // 66 out of 100 voted for blockHash. for i := int32(0); i < 66; i++ { pubKey, err := privValidators[i].GetPubKey() require.NoError(t, err) @@ -386,16 +386,12 @@ func TestVoteSet_Conflicts(t *testing.T) { } // check - if !voteSet.HasTwoThirdsMajority() { - t.Errorf("we should have 2/3 majority for blockHash1") - } - blockIDMaj23, _ := voteSet.TwoThirdsMajority() - if !bytes.Equal(blockIDMaj23.Hash, blockHash1) { - t.Errorf("got the wrong 2/3 majority blockhash") - } - if !voteSet.HasTwoThirdsAny() { - t.Errorf("we should have 2/3 if any votes") - } + require.True(t, voteSet.HasTwoThirdsMajority(), "we should have 2/3 majority for blockHash1") + + blockIDMaj23, ok := voteSet.TwoThirdsMajority() + require.True(t, ok) + require.True(t, bytes.Equal(blockIDMaj23.Hash, blockHash1), "got the wrong 2/3 majority blockhash") + require.True(t, voteSet.HasTwoThirdsAny(), "we should have 2/3 if any votes") } func TestVoteSet_MakeCommit(t *testing.T) {