From f95abc275ce67d4d0c4ae96badf4357d63704ad0 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Tue, 21 Dec 2021 18:15:54 +0100 Subject: [PATCH] Add more timely tests and check votes --- internal/consensus/pbts_test.go | 71 +++++++++++++++++++++++++++++++++ internal/consensus/state.go | 45 ++++++++++++++++----- 2 files changed, 106 insertions(+), 10 deletions(-) diff --git a/internal/consensus/pbts_test.go b/internal/consensus/pbts_test.go index a8c3e007e..761cbf18d 100644 --- a/internal/consensus/pbts_test.go +++ b/internal/consensus/pbts_test.go @@ -422,3 +422,74 @@ func TestProposerWaitTime(t *testing.T) { }) } } + +func TestTimelyProposal(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + initialTime := time.Now() + + cfg := pbtsTestConfiguration{ + timingParams: types.TimingParams{ + Precision: 10 * time.Millisecond, + MessageDelay: 140 * time.Millisecond, + }, + timeoutPropose: 40 * time.Millisecond, + genesisTime: initialTime, + height2ProposedBlockTime: initialTime.Add(10 * time.Millisecond), + height2ProposalDeliverTime: initialTime.Add(30 * time.Millisecond), + } + + pbtsTest := newPBTSTestHarness(ctx, t, cfg) + results := pbtsTest.run() + assert.True(t, results.height2.prevote.BlockID.Hash != nil) +} + +func TestTooFarInThePastProposal(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + initialTime := time.Now() + + // localtime > proposedBlockTime + MsgDelay + Precision + cfg := pbtsTestConfiguration{ + timingParams: types.TimingParams{ + Precision: 1 * time.Millisecond, + MessageDelay: 10 * time.Millisecond, + }, + timeoutPropose: 50 * time.Millisecond, + genesisTime: initialTime, + height2ProposedBlockTime: initialTime.Add(10 * time.Millisecond), + height2ProposalDeliverTime: initialTime.Add(100 * time.Millisecond), + } + + pbtsTest := newPBTSTestHarness(ctx, t, cfg) + results := pbtsTest.run() + time.Sleep(1 * time.Second) + + assert.True(t, results.height2.prevote.BlockID.Hash == nil) +} + +func TestTooFarInTheFutureProposal(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + initialTime := time.Now() + + // localtime < proposedBlockTime - Precision + cfg := pbtsTestConfiguration{ + timingParams: types.TimingParams{ + Precision: 1 * time.Millisecond, + MessageDelay: 10 * time.Millisecond, + }, + timeoutPropose: 50 * time.Millisecond, + genesisTime: initialTime, + height2ProposedBlockTime: initialTime.Add(100 * time.Millisecond), + height2ProposalDeliverTime: initialTime.Add(10 * time.Millisecond), + } + + pbtsTest := newPBTSTestHarness(ctx, t, cfg) + results := pbtsTest.run() + + assert.True(t, results.height2.prevote.BlockID.Hash == nil) +} diff --git a/internal/consensus/state.go b/internal/consensus/state.go index f3d60b709..929b27328 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -1312,6 +1312,18 @@ func (cs *State) enterPrevote(height int64, round int32) { // (so we have more time to try and collect +2/3 prevotes for a single block) } +func (cs *State) proposalIsTimely(proposal *types.Proposal) bool { + tp := types.TimingParams{ + Precision: cs.state.ConsensusParams.Timing.Precision, + MessageDelay: cs.state.ConsensusParams.Timing.MessageDelay, + } + + if !proposal.IsTimely(tmtime.DefaultSource{}, tp, cs.state.InitialHeight) { + return false + } + return true +} + func (cs *State) defaultDoPrevote(height int64, round int32) { logger := cs.Logger.With("height", height, "round", round) @@ -1322,6 +1334,12 @@ func (cs *State) defaultDoPrevote(height int64, round int32) { return } + if cs.Proposal == nil { + logger.Debug("prevote step; did not receive proposal, prevoting nil") + cs.signAddVote(tmproto.PrevoteType, nil, types.PartSetHeader{}) + return + } + if !cs.Proposal.Timestamp.Equal(cs.ProposalBlock.Header.Time) { logger.Debug("proposal timestamp not equal, prevoting nil") cs.signAddVote(tmproto.PrevoteType, nil, types.PartSetHeader{}) @@ -1352,7 +1370,11 @@ func (cs *State) defaultDoPrevote(height int64, round int32) { */ if cs.Proposal.POLRound == -1 { if cs.LockedRound == -1 { - // TODO(@wbanfield) add check for timely here as well + if !cs.proposalIsTimely(cs.Proposal) { + logger.Debug("prevote step: ProposalBlock is not timely; prevoting nil") + cs.signAddVote(tmproto.PrevoteType, nil, types.PartSetHeader{}) + return + } logger.Debug("prevote step: ProposalBlock is valid and there is no locked block; prevoting the proposal") cs.signAddVote(tmproto.PrevoteType, cs.ProposalBlock.Hash(), cs.ProposalBlockParts.Header()) return @@ -1490,6 +1512,13 @@ func (cs *State) enterPrecommit(height int64, round int32) { } // At this point, +2/3 prevoted for a particular block. + // If we never received a proposal for this block, we must precommit nil + if cs.Proposal == nil { + logger.Debug("precommit step; did not receive proposal, precommitting nil") + cs.signAddVote(tmproto.PrecommitType, nil, types.PartSetHeader{}) + return + } + // If the proposal time does not match the block time, precommit nil. if !cs.Proposal.Timestamp.Equal(cs.ProposalBlock.Header.Time) { logger.Debug("proposal timestamp not equal, precommitting nil") @@ -1882,7 +1911,7 @@ func (cs *State) RecordMetrics(height int64, block *types.Block) { func (cs *State) defaultSetProposal(proposal *types.Proposal) error { // Already have one // TODO: possibly catch double proposals - if cs.Proposal != nil { + if cs.Proposal != nil || proposal == nil { return nil } @@ -1897,14 +1926,10 @@ func (cs *State) defaultSetProposal(proposal *types.Proposal) error { return ErrInvalidProposalPOLRound } - // Verify timely - tp := types.TimingParams{ - Precision: cs.state.ConsensusParams.Timing.Precision, - MessageDelay: cs.state.ConsensusParams.Timing.MessageDelay, - } - if proposal.POLRound == -1 && !proposal.IsTimely(tmtime.DefaultSource{}, tp, cs.state.InitialHeight) { - return ErrInvalidProposalNotTimely - } + // Verify that the proposal is not too far in the past or future + //if !cs.proposalIsTimely(proposal) { + // return ErrInvalidProposalNotTimely + //} p := proposal.ToProto() // Verify signature