From b297efb5321d3d7af9e29acac267806b89130a8a Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Sun, 18 Mar 2018 23:05:04 +0100 Subject: [PATCH 1/2] consensus: return from go-routine in test --- consensus/mempool_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/consensus/mempool_test.go b/consensus/mempool_test.go index d1714a741..d283ff4ba 100644 --- a/consensus/mempool_test.go +++ b/consensus/mempool_test.go @@ -152,6 +152,7 @@ func TestMempoolRmBadTx(t *testing.T) { txs := cs.mempool.Reap(1) if len(txs) == 0 { emptyMempoolCh <- struct{}{} + return } time.Sleep(10 * time.Millisecond) } From ab7dea4f200c4f3733b7aa758557f6b91ad28505 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Sun, 18 Mar 2018 23:07:23 +0100 Subject: [PATCH 2/2] consensus: return from errors sooner in addVote --- consensus/state.go | 192 +++++++++++++++++++++++---------------------- 1 file changed, 98 insertions(+), 94 deletions(-) diff --git a/consensus/state.go b/consensus/state.go index 30bd56f10..c7e5e56ac 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -1355,111 +1355,115 @@ func (cs *ConsensusState) addVote(vote *types.Vote, peerID p2p.ID) (added bool, return added, ErrVoteHeightMismatch } added, err = cs.LastCommit.AddVote(vote) - if added { - cs.Logger.Info(cmn.Fmt("Added to lastPrecommits: %v", cs.LastCommit.StringShort())) - cs.eventBus.PublishEventVote(types.EventDataVote{vote}) - - // if we can skip timeoutCommit and have all the votes now, - if cs.config.SkipTimeoutCommit && cs.LastCommit.HasAll() { - // go straight to new round (skip timeout commit) - // cs.scheduleTimeout(time.Duration(0), cs.Height, 0, cstypes.RoundStepNewHeight) - cs.enterNewRound(cs.Height, 0) - } + if !added { + return added, err + } + + cs.Logger.Info(cmn.Fmt("Added to lastPrecommits: %v", cs.LastCommit.StringShort())) + cs.eventBus.PublishEventVote(types.EventDataVote{vote}) + + // if we can skip timeoutCommit and have all the votes now, + if cs.config.SkipTimeoutCommit && cs.LastCommit.HasAll() { + // go straight to new round (skip timeout commit) + // cs.scheduleTimeout(time.Duration(0), cs.Height, 0, cstypes.RoundStepNewHeight) + cs.enterNewRound(cs.Height, 0) } return } - // A prevote/precommit for this height? - if vote.Height == cs.Height { - height := cs.Height - added, err = cs.Votes.AddVote(vote, peerID) - if added { - cs.eventBus.PublishEventVote(types.EventDataVote{vote}) - - switch vote.Type { - case types.VoteTypePrevote: - prevotes := cs.Votes.Prevotes(vote.Round) - cs.Logger.Info("Added to prevote", "vote", vote, "prevotes", prevotes.StringShort()) - blockID, ok := prevotes.TwoThirdsMajority() - // First, unlock if prevotes is a valid POL. - // >> lockRound < POLRound <= unlockOrChangeLockRound (see spec) - // NOTE: If (lockRound < POLRound) but !(POLRound <= unlockOrChangeLockRound), - // we'll still enterNewRound(H,vote.R) and enterPrecommit(H,vote.R) to process it - // there. - if (cs.LockedBlock != nil) && (cs.LockedRound < vote.Round) && (vote.Round <= cs.Round) { - if ok && !cs.LockedBlock.HashesTo(blockID.Hash) { - cs.Logger.Info("Unlocking because of POL.", "lockedRound", cs.LockedRound, "POLRound", vote.Round) - cs.LockedRound = 0 - cs.LockedBlock = nil - cs.LockedBlockParts = nil - cs.eventBus.PublishEventUnlock(cs.RoundStateEvent()) - } - } - // Update ValidBlock - if ok && !blockID.IsZero() && !cs.ValidBlock.HashesTo(blockID.Hash) && vote.Round > cs.ValidRound { - // update valid value - if cs.ProposalBlock.HashesTo(blockID.Hash) { - cs.ValidRound = vote.Round - cs.ValidBlock = cs.ProposalBlock - cs.ValidBlockParts = cs.ProposalBlockParts - } - //TODO: We might want to update ValidBlock also in case we don't have that block yet, - // and obtain the required block using gossiping - } + // Height mismatch is ignored. + // Not necessarily a bad peer, but not favourable behaviour. + if vote.Height != cs.Height { + err = ErrVoteHeightMismatch + cs.Logger.Info("Vote ignored and not added", "voteHeight", vote.Height, "csHeight", cs.Height, "err", err) + return + } - if cs.Round <= vote.Round && prevotes.HasTwoThirdsAny() { - // Round-skip over to PrevoteWait or goto Precommit. - cs.enterNewRound(height, vote.Round) // if the vote is ahead of us - if prevotes.HasTwoThirdsMajority() { - cs.enterPrecommit(height, vote.Round) - } else { - cs.enterPrevote(height, vote.Round) // if the vote is ahead of us - cs.enterPrevoteWait(height, vote.Round) - } - } else if cs.Proposal != nil && 0 <= cs.Proposal.POLRound && cs.Proposal.POLRound == vote.Round { - // If the proposal is now complete, enter prevote of cs.Round. - if cs.isProposalComplete() { - cs.enterPrevote(height, cs.Round) - } - } - case types.VoteTypePrecommit: - precommits := cs.Votes.Precommits(vote.Round) - cs.Logger.Info("Added to precommit", "vote", vote, "precommits", precommits.StringShort()) - blockID, ok := precommits.TwoThirdsMajority() - if ok { - if len(blockID.Hash) == 0 { - cs.enterNewRound(height, vote.Round+1) - } else { - cs.enterNewRound(height, vote.Round) - cs.enterPrecommit(height, vote.Round) - cs.enterCommit(height, vote.Round) - - if cs.config.SkipTimeoutCommit && precommits.HasAll() { - // if we have all the votes now, - // go straight to new round (skip timeout commit) - // cs.scheduleTimeout(time.Duration(0), cs.Height, 0, cstypes.RoundStepNewHeight) - cs.enterNewRound(cs.Height, 0) - } - - } - } else if cs.Round <= vote.Round && precommits.HasTwoThirdsAny() { - cs.enterNewRound(height, vote.Round) - cs.enterPrecommit(height, vote.Round) - cs.enterPrecommitWait(height, vote.Round) + height := cs.Height + added, err = cs.Votes.AddVote(vote, peerID) + if !added { + // Either duplicate, or error upon cs.Votes.AddByIndex() + return + } + + cs.eventBus.PublishEventVote(types.EventDataVote{vote}) + + switch vote.Type { + case types.VoteTypePrevote: + prevotes := cs.Votes.Prevotes(vote.Round) + cs.Logger.Info("Added to prevote", "vote", vote, "prevotes", prevotes.StringShort()) + blockID, ok := prevotes.TwoThirdsMajority() + // First, unlock if prevotes is a valid POL. + // >> lockRound < POLRound <= unlockOrChangeLockRound (see spec) + // NOTE: If (lockRound < POLRound) but !(POLRound <= unlockOrChangeLockRound), + // we'll still enterNewRound(H,vote.R) and enterPrecommit(H,vote.R) to process it + // there. + if (cs.LockedBlock != nil) && (cs.LockedRound < vote.Round) && (vote.Round <= cs.Round) { + if ok && !cs.LockedBlock.HashesTo(blockID.Hash) { + cs.Logger.Info("Unlocking because of POL.", "lockedRound", cs.LockedRound, "POLRound", vote.Round) + cs.LockedRound = 0 + cs.LockedBlock = nil + cs.LockedBlockParts = nil + cs.eventBus.PublishEventUnlock(cs.RoundStateEvent()) + } + } + // Update ValidBlock + if ok && !blockID.IsZero() && !cs.ValidBlock.HashesTo(blockID.Hash) && vote.Round > cs.ValidRound { + // update valid value + if cs.ProposalBlock.HashesTo(blockID.Hash) { + cs.ValidRound = vote.Round + cs.ValidBlock = cs.ProposalBlock + cs.ValidBlockParts = cs.ProposalBlockParts + } + //TODO: We might want to update ValidBlock also in case we don't have that block yet, + // and obtain the required block using gossiping + } + + if cs.Round <= vote.Round && prevotes.HasTwoThirdsAny() { + // Round-skip over to PrevoteWait or goto Precommit. + cs.enterNewRound(height, vote.Round) // if the vote is ahead of us + if prevotes.HasTwoThirdsMajority() { + cs.enterPrecommit(height, vote.Round) + } else { + cs.enterPrevote(height, vote.Round) // if the vote is ahead of us + cs.enterPrevoteWait(height, vote.Round) + } + } else if cs.Proposal != nil && 0 <= cs.Proposal.POLRound && cs.Proposal.POLRound == vote.Round { + // If the proposal is now complete, enter prevote of cs.Round. + if cs.isProposalComplete() { + cs.enterPrevote(height, cs.Round) + } + } + case types.VoteTypePrecommit: + precommits := cs.Votes.Precommits(vote.Round) + cs.Logger.Info("Added to precommit", "vote", vote, "precommits", precommits.StringShort()) + blockID, ok := precommits.TwoThirdsMajority() + if ok { + if len(blockID.Hash) == 0 { + cs.enterNewRound(height, vote.Round+1) + } else { + cs.enterNewRound(height, vote.Round) + cs.enterPrecommit(height, vote.Round) + cs.enterCommit(height, vote.Round) + + if cs.config.SkipTimeoutCommit && precommits.HasAll() { + // if we have all the votes now, + // go straight to new round (skip timeout commit) + // cs.scheduleTimeout(time.Duration(0), cs.Height, 0, cstypes.RoundStepNewHeight) + cs.enterNewRound(cs.Height, 0) } - default: - cmn.PanicSanity(cmn.Fmt("Unexpected vote type %X", vote.Type)) // Should not happen. + } + } else if cs.Round <= vote.Round && precommits.HasTwoThirdsAny() { + cs.enterNewRound(height, vote.Round) + cs.enterPrecommit(height, vote.Round) + cs.enterPrecommitWait(height, vote.Round) } - // Either duplicate, or error upon cs.Votes.AddByIndex() - return - } else { - err = ErrVoteHeightMismatch + default: + panic(cmn.Fmt("Unexpected vote type %X", vote.Type)) // go-wire should prevent this. } - // Height mismatch, bad peer? - cs.Logger.Info("Vote ignored and not added", "voteHeight", vote.Height, "csHeight", cs.Height, "err", err) return }