From e4598b1de14846e60df1bc9f79a9d399f5ab212e Mon Sep 17 00:00:00 2001 From: William Banfield <4561443+williambanfield@users.noreply.github.com> Date: Thu, 9 Dec 2021 17:18:41 -0500 Subject: [PATCH 1/6] internal/consensus: remove proposal wait time (#7418) --- internal/consensus/pbts_test.go | 121 -------------------------------- internal/consensus/state.go | 34 +-------- 2 files changed, 1 insertion(+), 154 deletions(-) diff --git a/internal/consensus/pbts_test.go b/internal/consensus/pbts_test.go index c0c64a715..c050dd6e5 100644 --- a/internal/consensus/pbts_test.go +++ b/internal/consensus/pbts_test.go @@ -324,74 +324,6 @@ func (hr heightResult) isComplete() bool { return !hr.proposalIssuedAt.IsZero() && !hr.prevoteIssuedAt.IsZero() && hr.prevote != nil } -// TestReceiveProposalWaitsForPreviousBlockTime tests that a validator receiving -// a proposal waits until the previous block time passes before issuing a prevote. -// The test delivers the block to the validator after the configured `timeout-propose`, -// but before the proposer-based timestamp bound on block delivery and checks that -// the consensus algorithm correctly waits for the new block to be delivered -// and issues a prevote for it. -func TestReceiveProposalWaitsForPreviousBlockTime(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - initialTime := time.Now().Add(50 * time.Millisecond) - cfg := pbtsTestConfiguration{ - timingParams: types.TimingParams{ - Precision: 100 * time.Millisecond, - MessageDelay: 500 * time.Millisecond, - }, - timeoutPropose: 50 * time.Millisecond, - genesisTime: initialTime, - height2ProposalDeliverTime: initialTime.Add(450 * time.Millisecond), - height2ProposedBlockTime: initialTime.Add(350 * time.Millisecond), - } - - pbtsTest := newPBTSTestHarness(ctx, t, cfg) - results := pbtsTest.run() - - // Check that the validator waited until after the proposer-based timestamp - // waitingTime bound. - assert.True(t, results.height2.prevoteIssuedAt.After(cfg.height2ProposalDeliverTime)) - maxWaitingTime := cfg.genesisTime.Add(cfg.timingParams.Precision).Add(cfg.timingParams.MessageDelay) - assert.True(t, results.height2.prevoteIssuedAt.Before(maxWaitingTime)) - - // Check that the validator did not prevote for nil. - assert.NotNil(t, results.height2.prevote.BlockID.Hash) -} - -// TestReceiveProposalTimesOutOnSlowDelivery tests that a validator receiving -// a proposal times out and prevotes nil if the block is not delivered by the -// within the proposer-based timestamp algorithm's waitingTime bound. -// The test delivers the block to the validator after the previous block's time -// and after the proposer-based timestamp bound on block delivery. -// The test then checks that the validator correctly waited for the new block -// and prevoted nil after timing out. -func TestReceiveProposalTimesOutOnSlowDelivery(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - initialTime := time.Now() - cfg := pbtsTestConfiguration{ - timingParams: types.TimingParams{ - Precision: 100 * time.Millisecond, - MessageDelay: 500 * time.Millisecond, - }, - timeoutPropose: 50 * time.Millisecond, - genesisTime: initialTime, - height2ProposalDeliverTime: initialTime.Add(660 * time.Millisecond), - height2ProposedBlockTime: initialTime.Add(350 * time.Millisecond), - } - - pbtsTest := newPBTSTestHarness(ctx, t, cfg) - results := pbtsTest.run() - - // Check that the validator waited until after the proposer-based timestamp - // waitinTime bound. - maxWaitingTime := initialTime.Add(cfg.timingParams.Precision).Add(cfg.timingParams.MessageDelay) - assert.True(t, results.height2.prevoteIssuedAt.After(maxWaitingTime)) - - // Ensure that the validator issued a prevote for nil. - assert.Nil(t, results.height2.prevote.BlockID.Hash) -} - // TestProposerWaitsForGenesisTime tests that a proposer will not propose a block // until after the genesis time has passed. The test sets the genesis time in the // future and then ensures that the observed validator waits to propose a block. @@ -491,56 +423,3 @@ func TestProposerWaitTime(t *testing.T) { }) } } - -func TestProposalTimeout(t *testing.T) { - genesisTime, err := time.Parse(time.RFC3339, "2019-03-13T23:00:00Z") - require.NoError(t, err) - testCases := []struct { - name string - localTime time.Time - previousBlockTime time.Time - precision time.Duration - msgDelay time.Duration - expectedDuration time.Duration - }{ - { - name: "MsgDelay + Precision has not quite elapsed", - localTime: genesisTime.Add(525 * time.Millisecond), - previousBlockTime: genesisTime.Add(6 * time.Millisecond), - precision: time.Millisecond * 20, - msgDelay: time.Millisecond * 500, - expectedDuration: 1 * time.Millisecond, - }, - { - name: "MsgDelay + Precision equals current time", - localTime: genesisTime.Add(525 * time.Millisecond), - previousBlockTime: genesisTime.Add(5 * time.Millisecond), - precision: time.Millisecond * 20, - msgDelay: time.Millisecond * 500, - expectedDuration: 0, - }, - { - name: "MsgDelay + Precision has elapsed", - localTime: genesisTime.Add(725 * time.Millisecond), - previousBlockTime: genesisTime.Add(5 * time.Millisecond), - precision: time.Millisecond * 20, - msgDelay: time.Millisecond * 500, - expectedDuration: 0, - }, - } - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - - mockSource := new(tmtimemocks.Source) - mockSource.On("Now").Return(testCase.localTime) - - tp := types.TimingParams{ - Precision: testCase.precision, - MessageDelay: testCase.msgDelay, - } - - ti := proposalStepWaitingTime(mockSource, testCase.previousBlockTime, tp) - assert.Equal(t, testCase.expectedDuration, ti) - }) - } -} diff --git a/internal/consensus/state.go b/internal/consensus/state.go index 29e72d234..addcdec05 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -1130,11 +1130,8 @@ func (cs *State) enterPropose(height int64, round int32) { } }() - waitingTime := proposalStepWaitingTime(tmtime.DefaultSource{}, cs.state.LastBlockTime, cs.state.ConsensusParams.Timing) // nolint: lll - proposalTimeout := maxDuration(cs.config.Propose(round), waitingTime) - // If we don't get the proposal and all block parts quick enough, enterPrevote - cs.scheduleTimeout(proposalTimeout, height, round, cstypes.RoundStepPropose) + cs.scheduleTimeout(cs.config.Propose(round), height, round, cstypes.RoundStepPropose) // Nothing more to do if we're not a validator if cs.privValidator == nil { @@ -2444,32 +2441,3 @@ func proposerWaitTime(lt tmtime.Source, bt time.Time) time.Duration { } return 0 } - -// proposalStepWaitingTime is used along with the `timeout-propose` configuration -// parameter to determines how long a validator will wait for a block to be sent from a proposer. -// proposalStepWaitingTime ensures that the validator waits long enough for the proposer to -// deliver a block with a monotically increasing timestamp. -// -// To ensure that the validator waits long enough, it must wait until the previous -// block's timestamp. It also must account for the difference between its own clock and -// the proposer's clock, i.e. the 'Precision', and the amount of time for the message to be transmitted, -// i.e. the MsgDelay. -// -// The result of proposalStepWaitingTime is compared with the configured `timeout-propose` duration, -// and the validator waits for whichever duration is larger before advancing to the next step -// and prevoting nil. -func proposalStepWaitingTime(lt tmtime.Source, bt time.Time, tp types.TimingParams) time.Duration { - t := lt.Now() - wt := bt.Add(tp.Precision).Add(tp.MessageDelay) - if t.After(wt) { - return 0 - } - return wt.Sub(t) -} - -func maxDuration(d1, d2 time.Duration) time.Duration { - if d1 >= d2 { - return d1 - } - return d2 -} From 9cd4cfed6bd60327cb0d29d374fa79aef0e24903 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Tue, 14 Dec 2021 20:09:02 -0500 Subject: [PATCH 2/6] Remove block Id checks and enable tests --- internal/consensus/common_test.go | 3 --- internal/consensus/pbts_test.go | 2 -- 2 files changed, 5 deletions(-) diff --git a/internal/consensus/common_test.go b/internal/consensus/common_test.go index 550c60b2b..f978dfa13 100644 --- a/internal/consensus/common_test.go +++ b/internal/consensus/common_test.go @@ -700,9 +700,6 @@ func ensureProposalWithTimeout(t *testing.T, proposalCh <-chan tmpubsub.Message, if proposalEvent.Round != round { t.Fatalf("expected round %v, got %v", round, proposalEvent.Round) } - if !proposalEvent.BlockID.Equals(propID) { - t.Fatalf("Proposed block does not match expected block (%v != %v)", proposalEvent.BlockID, propID) - } } func ensurePrecommit(t *testing.T, voteCh <-chan tmpubsub.Message, height int64, round int32) { t.Helper() diff --git a/internal/consensus/pbts_test.go b/internal/consensus/pbts_test.go index ba3dbab70..00943433d 100644 --- a/internal/consensus/pbts_test.go +++ b/internal/consensus/pbts_test.go @@ -328,7 +328,6 @@ func (hr heightResult) isComplete() bool { // until after the genesis time has passed. The test sets the genesis time in the // future and then ensures that the observed validator waits to propose a block. func TestProposerWaitsForGenesisTime(t *testing.T) { - t.Skip() ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -359,7 +358,6 @@ func TestProposerWaitsForGenesisTime(t *testing.T) { // and then verifies that the observed validator waits until after the block time // of height 4 to propose a block at height 5. func TestProposerWaitsForPreviousBlock(t *testing.T) { - t.Skip() ctx, cancel := context.WithCancel(context.Background()) defer cancel() initialTime := time.Now().Add(time.Millisecond * 50) From 56a20056ec96947a6102917d43f8e1600e1b379c Mon Sep 17 00:00:00 2001 From: William Banfield <4561443+williambanfield@users.noreply.github.com> Date: Wed, 15 Dec 2021 14:46:55 -0500 Subject: [PATCH 3/6] internal/consensus: prevote nil if proposal timestamp does not match (#7391) This change updates the proposal logic to use the block's timestamp in the proposal message. It adds an additional piece of validation logic to the prevote step to check that the block's timestamp matches the proposal message's timestamp. --- internal/consensus/byzantine_test.go | 2 +- internal/consensus/common_test.go | 2 +- internal/consensus/pbts_test.go | 2 +- internal/consensus/replay_test.go | 8 +- internal/consensus/state.go | 17 ++++- internal/consensus/state_test.go | 108 +++++++++++++++++++++++++-- types/proposal.go | 4 +- types/proposal_test.go | 9 ++- 8 files changed, 132 insertions(+), 20 deletions(-) diff --git a/internal/consensus/byzantine_test.go b/internal/consensus/byzantine_test.go index 70555d440..e122c8420 100644 --- a/internal/consensus/byzantine_test.go +++ b/internal/consensus/byzantine_test.go @@ -214,7 +214,7 @@ func TestByzantinePrevoteEquivocation(t *testing.T) { // Make proposal propBlockID := types.BlockID{Hash: block.Hash(), PartSetHeader: blockParts.Header()} - proposal := types.NewProposal(height, round, lazyNodeState.ValidRound, propBlockID) + proposal := types.NewProposal(height, round, lazyNodeState.ValidRound, propBlockID, block.Header.Time) p := proposal.ToProto() if err := lazyNodeState.privValidator.SignProposal(ctx, lazyNodeState.state.ChainID, p); err == nil { proposal.Signature = p.Signature diff --git a/internal/consensus/common_test.go b/internal/consensus/common_test.go index 550c60b2b..dee25e33c 100644 --- a/internal/consensus/common_test.go +++ b/internal/consensus/common_test.go @@ -248,7 +248,7 @@ func decideProposal( // Make proposal polRound, propBlockID := validRound, types.BlockID{Hash: block.Hash(), PartSetHeader: blockParts.Header()} - proposal = types.NewProposal(height, round, polRound, propBlockID) + proposal = types.NewProposal(height, round, polRound, propBlockID, block.Header.Time) p := proposal.ToProto() if err := vs.SignProposal(ctx, chainID, p); err != nil { t.Fatalf("error signing proposal: %s", err) diff --git a/internal/consensus/pbts_test.go b/internal/consensus/pbts_test.go index c050dd6e5..aa29b4a3b 100644 --- a/internal/consensus/pbts_test.go +++ b/internal/consensus/pbts_test.go @@ -204,7 +204,7 @@ func (p *pbtsTestHarness) nextHeight(proposer types.PrivValidator, deliverTime, b.Header.ProposerAddress = k.Address() ps := b.MakePartSet(types.BlockPartSizeBytes) bid := types.BlockID{Hash: b.Hash(), PartSetHeader: ps.Header()} - prop := types.NewProposal(p.currentHeight, 0, -1, bid) + prop := types.NewProposal(p.currentHeight, 0, -1, bid, proposedTime) tp := prop.ToProto() if err := proposer.SignProposal(context.Background(), p.observedState.state.ChainID, tp); err != nil { diff --git a/internal/consensus/replay_test.go b/internal/consensus/replay_test.go index 69124df0d..7893a1f39 100644 --- a/internal/consensus/replay_test.go +++ b/internal/consensus/replay_test.go @@ -384,7 +384,7 @@ func setupSimulator(ctx context.Context, t *testing.T) *simulatorTestSuite { propBlockParts := propBlock.MakePartSet(partSize) blockID := types.BlockID{Hash: propBlock.Hash(), PartSetHeader: propBlockParts.Header()} - proposal := types.NewProposal(vss[1].Height, round, -1, blockID) + proposal := types.NewProposal(vss[1].Height, round, -1, blockID, propBlock.Header.Time) p := proposal.ToProto() if err := vss[1].SignProposal(ctx, cfg.ChainID(), p); err != nil { t.Fatal("failed to sign bad proposal", err) @@ -416,7 +416,7 @@ func setupSimulator(ctx context.Context, t *testing.T) *simulatorTestSuite { propBlockParts = propBlock.MakePartSet(partSize) blockID = types.BlockID{Hash: propBlock.Hash(), PartSetHeader: propBlockParts.Header()} - proposal = types.NewProposal(vss[2].Height, round, -1, blockID) + proposal = types.NewProposal(vss[2].Height, round, -1, blockID, propBlock.Header.Time) p = proposal.ToProto() if err := vss[2].SignProposal(ctx, cfg.ChainID(), p); err != nil { t.Fatal("failed to sign bad proposal", err) @@ -475,7 +475,7 @@ func setupSimulator(ctx context.Context, t *testing.T) *simulatorTestSuite { selfIndex := valIndexFn(0) - proposal = types.NewProposal(vss[3].Height, round, -1, blockID) + proposal = types.NewProposal(vss[3].Height, round, -1, blockID, propBlock.Header.Time) p = proposal.ToProto() if err := vss[3].SignProposal(ctx, cfg.ChainID(), p); err != nil { t.Fatal("failed to sign bad proposal", err) @@ -540,7 +540,7 @@ func setupSimulator(ctx context.Context, t *testing.T) *simulatorTestSuite { sort.Sort(ValidatorStubsByPower(newVss)) selfIndex = valIndexFn(0) - proposal = types.NewProposal(vss[1].Height, round, -1, blockID) + proposal = types.NewProposal(vss[1].Height, round, -1, blockID, propBlock.Header.Time) p = proposal.ToProto() if err := vss[1].SignProposal(ctx, cfg.ChainID(), p); err != nil { t.Fatal("failed to sign bad proposal", err) diff --git a/internal/consensus/state.go b/internal/consensus/state.go index addcdec05..1409a6f51 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -1199,7 +1199,7 @@ func (cs *State) defaultDecideProposal(height int64, round int32) { // Make proposal propBlockID := types.BlockID{Hash: block.Hash(), PartSetHeader: blockParts.Header()} - proposal := types.NewProposal(height, round, cs.ValidRound, propBlockID) + proposal := types.NewProposal(height, round, cs.ValidRound, propBlockID, block.Header.Time) p := proposal.ToProto() // wait the max amount we would wait for a proposal @@ -1321,6 +1321,12 @@ func (cs *State) defaultDoPrevote(height int64, round int32) { 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{}) + return + } + // Validate proposal block err := cs.blockExec.ValidateBlock(cs.state, cs.ProposalBlock) if err != nil { @@ -1345,6 +1351,7 @@ 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 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 @@ -1480,9 +1487,15 @@ func (cs *State) enterPrecommit(height int64, round int32) { cs.signAddVote(tmproto.PrecommitType, nil, types.PartSetHeader{}) return } - // At this point, +2/3 prevoted for a particular block. + // 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") + cs.signAddVote(tmproto.PrecommitType, nil, types.PartSetHeader{}) + return + } + // If we're already locked on that block, precommit it, and update the LockedRound if cs.LockedBlock.HashesTo(blockID.Hash) { logger.Debug("precommit step; +2/3 prevoted locked block; relocking") diff --git a/internal/consensus/state_test.go b/internal/consensus/state_test.go index b3e88748f..a23f6cc1e 100644 --- a/internal/consensus/state_test.go +++ b/internal/consensus/state_test.go @@ -246,7 +246,7 @@ func TestStateBadProposal(t *testing.T) { propBlock.AppHash = stateHash propBlockParts := propBlock.MakePartSet(partSize) blockID := types.BlockID{Hash: propBlock.Hash(), PartSetHeader: propBlockParts.Header()} - proposal := types.NewProposal(vs2.Height, round, -1, blockID) + proposal := types.NewProposal(vs2.Height, round, -1, blockID, propBlock.Header.Time) p := proposal.ToProto() if err := vs2.SignProposal(ctx, config.ChainID(), p); err != nil { t.Fatal("failed to sign bad proposal", err) @@ -306,7 +306,7 @@ func TestStateOversizedBlock(t *testing.T) { propBlockParts := propBlock.MakePartSet(partSize) blockID := types.BlockID{Hash: propBlock.Hash(), PartSetHeader: propBlockParts.Header()} - proposal := types.NewProposal(height, round, -1, blockID) + proposal := types.NewProposal(height, round, -1, blockID, propBlock.Header.Time) p := proposal.ToProto() if err := vs2.SignProposal(ctx, config.ChainID(), p); err != nil { t.Fatal("failed to sign bad proposal", err) @@ -856,7 +856,7 @@ func TestStateLock_POLRelock(t *testing.T) { t.Log("### Starting Round 1") incrementRound(vs2, vs3, vs4) round++ - propR1 := types.NewProposal(height, round, cs1.ValidRound, blockID) + propR1 := types.NewProposal(height, round, cs1.ValidRound, blockID, theBlock.Header.Time) p := propR1.ToProto() if err := vs2.SignProposal(ctx, cs1.state.ChainID, p); err != nil { t.Fatalf("error signing proposal: %s", err) @@ -1588,7 +1588,7 @@ func TestStateLock_POLSafety2(t *testing.T) { round++ // moving to the next round // in round 2 we see the polkad block from round 0 - newProp := types.NewProposal(height, round, 0, propBlockID0) + newProp := types.NewProposal(height, round, 0, propBlockID0, propBlock0.Header.Time) p := newProp.ToProto() if err := vs3.SignProposal(ctx, config.ChainID(), p); err != nil { t.Fatal(err) @@ -1730,7 +1730,7 @@ func TestState_PrevotePOLFromPreviousRound(t *testing.T) { t.Log("### Starting Round 2") incrementRound(vs2, vs3, vs4) round++ - propR2 := types.NewProposal(height, round, 1, r1BlockID) + propR2 := types.NewProposal(height, round, 1, r1BlockID, propBlockR1.Header.Time) p := propR2.ToProto() if err := vs3.SignProposal(ctx, cs1.state.ChainID, p); err != nil { t.Fatalf("error signing proposal: %s", err) @@ -2649,6 +2649,104 @@ func TestSignSameVoteTwice(t *testing.T) { require.Equal(t, vote, vote2) } +// TestStateTimestamp_ProposalNotMatch tests that a validator does not prevote a +// proposed block if the timestamp in the block does not matche the timestamp in the +// corresponding proposal message. +func TestStateTimestamp_ProposalNotMatch(t *testing.T) { + config := configSetup(t) + logger := log.TestingLogger() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + cs1, vss, err := makeState(ctx, config, logger, 4) + require.NoError(t, err) + height, round := cs1.Height, cs1.Round + vs2, vs3, vs4 := vss[1], vss[2], vss[3] + + proposalCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryCompleteProposal) + pv1, err := cs1.privValidator.GetPubKey(ctx) + require.NoError(t, err) + addr := pv1.Address() + voteCh := subscribeToVoter(ctx, t, cs1, addr) + + propBlock, _ := cs1.createProposalBlock() + round++ + incrementRound(vss[1:]...) + + propBlockParts := propBlock.MakePartSet(types.BlockPartSizeBytes) + blockID := types.BlockID{Hash: propBlock.Hash(), PartSetHeader: propBlockParts.Header()} + + // Create a proposal with a timestamp that does not match the timestamp of the block. + proposal := types.NewProposal(vs2.Height, round, -1, blockID, propBlock.Header.Time.Add(time.Millisecond)) + p := proposal.ToProto() + if err := vs2.SignProposal(ctx, config.ChainID(), p); err != nil { + t.Fatal("failed to sign bad proposal", err) + } + proposal.Signature = p.Signature + require.NoError(t, cs1.SetProposalAndBlock(proposal, propBlock, propBlockParts, "some peer")) + + startTestRound(ctx, cs1, height, round) + ensureProposal(t, proposalCh, height, round, blockID) + + signAddVotes(ctx, cs1, tmproto.PrevoteType, config.ChainID(), blockID, vs2, vs3, vs4) + + // ensure that the validator prevotes nil. + ensurePrevote(t, voteCh, height, round) + validatePrevote(ctx, t, cs1, round, vss[0], nil) + + ensurePrecommit(t, voteCh, height, round) + validatePrecommit(ctx, t, cs1, round, -1, vss[0], nil, nil) +} + +// TestStateTimestamp_ProposalMatch tests that a validator prevotes a +// proposed block if the timestamp in the block matches the timestamp in the +// corresponding proposal message. +func TestStateTimestamp_ProposalMatch(t *testing.T) { + config := configSetup(t) + logger := log.TestingLogger() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + cs1, vss, err := makeState(ctx, config, logger, 4) + require.NoError(t, err) + height, round := cs1.Height, cs1.Round + vs2, vs3, vs4 := vss[1], vss[2], vss[3] + + proposalCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryCompleteProposal) + pv1, err := cs1.privValidator.GetPubKey(ctx) + require.NoError(t, err) + addr := pv1.Address() + voteCh := subscribeToVoter(ctx, t, cs1, addr) + + propBlock, _ := cs1.createProposalBlock() + round++ + incrementRound(vss[1:]...) + + propBlockParts := propBlock.MakePartSet(types.BlockPartSizeBytes) + blockID := types.BlockID{Hash: propBlock.Hash(), PartSetHeader: propBlockParts.Header()} + + // Create a proposal with a timestamp that matches the timestamp of the block. + proposal := types.NewProposal(vs2.Height, round, -1, blockID, propBlock.Header.Time) + p := proposal.ToProto() + if err := vs2.SignProposal(ctx, config.ChainID(), p); err != nil { + t.Fatal("failed to sign bad proposal", err) + } + proposal.Signature = p.Signature + require.NoError(t, cs1.SetProposalAndBlock(proposal, propBlock, propBlockParts, "some peer")) + + startTestRound(ctx, cs1, height, round) + ensureProposal(t, proposalCh, height, round, blockID) + + signAddVotes(ctx, cs1, tmproto.PrevoteType, config.ChainID(), blockID, vs2, vs3, vs4) + + // ensure that the validator prevotes the block. + ensurePrevote(t, voteCh, height, round) + validatePrevote(ctx, t, cs1, round, vss[0], propBlock.Hash()) + + ensurePrecommit(t, voteCh, height, round) + validatePrecommit(ctx, t, cs1, round, 1, vss[0], propBlock.Hash(), propBlock.Hash()) +} + // subscribe subscribes test client to the given query and returns a channel with cap = 1. func subscribe( ctx context.Context, diff --git a/types/proposal.go b/types/proposal.go index 26df39a56..288abd6be 100644 --- a/types/proposal.go +++ b/types/proposal.go @@ -34,14 +34,14 @@ type Proposal struct { // NewProposal returns a new Proposal. // If there is no POLRound, polRound should be -1. -func NewProposal(height int64, round int32, polRound int32, blockID BlockID) *Proposal { +func NewProposal(height int64, round int32, polRound int32, blockID BlockID, ts time.Time) *Proposal { return &Proposal{ Type: tmproto.ProposalType, Height: height, Round: round, BlockID: blockID, POLRound: polRound, - Timestamp: tmtime.Now(), + Timestamp: tmtime.Canonical(ts), } } diff --git a/types/proposal_test.go b/types/proposal_test.go index 741396cc9..a7a6083f8 100644 --- a/types/proposal_test.go +++ b/types/proposal_test.go @@ -13,6 +13,7 @@ import ( "github.com/tendermint/tendermint/crypto/tmhash" "github.com/tendermint/tendermint/internal/libs/protoio" tmrand "github.com/tendermint/tendermint/libs/rand" + tmtime "github.com/tendermint/tendermint/libs/time" tmtimemocks "github.com/tendermint/tendermint/libs/time/mocks" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) @@ -63,7 +64,7 @@ func TestProposalVerifySignature(t *testing.T) { prop := NewProposal( 4, 2, 2, - BlockID{tmrand.Bytes(tmhash.Size), PartSetHeader{777, tmrand.Bytes(tmhash.Size)}}) + BlockID{tmrand.Bytes(tmhash.Size), PartSetHeader{777, tmrand.Bytes(tmhash.Size)}}, tmtime.Now()) p := prop.ToProto() signBytes := ProposalSignBytes("test_chain_id", p) @@ -154,7 +155,7 @@ func TestProposalValidateBasic(t *testing.T) { t.Run(tc.testName, func(t *testing.T) { prop := NewProposal( 4, 2, 2, - blockID) + blockID, tmtime.Now()) p := prop.ToProto() err := privVal.SignProposal(context.Background(), "test_chain_id", p) prop.Signature = p.Signature @@ -166,9 +167,9 @@ func TestProposalValidateBasic(t *testing.T) { } func TestProposalProtoBuf(t *testing.T) { - proposal := NewProposal(1, 2, 3, makeBlockID([]byte("hash"), 2, []byte("part_set_hash"))) + proposal := NewProposal(1, 2, 3, makeBlockID([]byte("hash"), 2, []byte("part_set_hash")), tmtime.Now()) proposal.Signature = []byte("sig") - proposal2 := NewProposal(1, 2, 3, BlockID{}) + proposal2 := NewProposal(1, 2, 3, BlockID{}, tmtime.Now()) testCases := []struct { msg string From 8c02f2c2e6bac3bf6ce50da8474abe577e0577a9 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Sat, 18 Dec 2021 03:01:26 +0100 Subject: [PATCH 4/6] William's suggestion to get the proposal from the proposer instead of generating it. --- internal/consensus/pbts_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/consensus/pbts_test.go b/internal/consensus/pbts_test.go index 0f1e97cf1..e630db989 100644 --- a/internal/consensus/pbts_test.go +++ b/internal/consensus/pbts_test.go @@ -147,11 +147,12 @@ func (p *pbtsTestHarness) observedValidatorProposerHeight(previousBlockTime time p.validatorClock.On("Now").Return(p.height2ProposedBlockTime).Times(6) ensureNewRound(p.t, p.roundCh, p.currentHeight, p.currentRound) - propBlock, partSet := p.observedState.createProposalBlock() - bid := types.BlockID{Hash: propBlock.Hash(), PartSetHeader: partSet.Header()} timeout := time.Until(previousBlockTime.Add(ensureTimeout)) - ensureProposalWithTimeout(p.t, p.ensureProposalCh, p.currentHeight, p.currentRound, bid, timeout) + ensureProposalWithTimeout(p.t, p.ensureProposalCh, p.currentHeight, p.currentRound, types.BlockID{}, timeout) + + rs := p.observedState.GetRoundState() + bid := types.BlockID{Hash: rs.ProposalBlock.Hash(), PartSetHeader: rs.ProposalBlockParts.Header()} ensurePrevote(p.t, p.ensureVoteCh, p.currentHeight, p.currentRound) signAddVotes(p.ctx, p.observedState, tmproto.PrevoteType, p.chainID, bid, p.otherValidators...) From c1753136219cd1b1265d37f192f4f51e73cc63d9 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Sat, 18 Dec 2021 03:19:58 +0100 Subject: [PATCH 5/6] Remove error check on service stop --- internal/consensus/pbts_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/consensus/pbts_test.go b/internal/consensus/pbts_test.go index e630db989..103318239 100644 --- a/internal/consensus/pbts_test.go +++ b/internal/consensus/pbts_test.go @@ -300,8 +300,7 @@ func (p *pbtsTestHarness) run() resultSet { r2 := p.height2() p.intermediateHeights() r5 := p.height5() - err := p.observedState.Stop() - require.NoError(p.t, err) + p.observedState.Stop() return resultSet{ genesisHeight: r1, height2: r2, From a587cfddbbcc64871d09c6ede5ca13db6ae76070 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Sat, 18 Dec 2021 03:26:30 +0100 Subject: [PATCH 6/6] Bring back block ID check in ensureProposalWithTimout --- internal/consensus/common_test.go | 8 ++++++-- internal/consensus/pbts_test.go | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/consensus/common_test.go b/internal/consensus/common_test.go index 37709a15d..3c0c72757 100644 --- a/internal/consensus/common_test.go +++ b/internal/consensus/common_test.go @@ -682,11 +682,11 @@ func ensureRelock(t *testing.T, relockCh <-chan tmpubsub.Message, height int64, } func ensureProposal(t *testing.T, proposalCh <-chan tmpubsub.Message, height int64, round int32, propID types.BlockID) { - ensureProposalWithTimeout(t, proposalCh, height, round, propID, ensureTimeout) + ensureProposalWithTimeout(t, proposalCh, height, round, &propID, ensureTimeout) } // nolint: lll -func ensureProposalWithTimeout(t *testing.T, proposalCh <-chan tmpubsub.Message, height int64, round int32, propID types.BlockID, timeout time.Duration) { +func ensureProposalWithTimeout(t *testing.T, proposalCh <-chan tmpubsub.Message, height int64, round int32, propID *types.BlockID, timeout time.Duration) { t.Helper() msg := ensureMessageBeforeTimeout(t, proposalCh, timeout) proposalEvent, ok := msg.Data().(types.EventDataCompleteProposal) @@ -700,7 +700,11 @@ func ensureProposalWithTimeout(t *testing.T, proposalCh <-chan tmpubsub.Message, if proposalEvent.Round != round { t.Fatalf("expected round %v, got %v", round, proposalEvent.Round) } + if propID != nil && !proposalEvent.BlockID.Equals(*propID) { + t.Fatalf("Proposed block does not match expected block (%v != %v)", proposalEvent.BlockID, *propID) + } } + func ensurePrecommit(t *testing.T, voteCh <-chan tmpubsub.Message, height int64, round int32) { t.Helper() ensureVote(t, voteCh, height, round, tmproto.PrecommitType) diff --git a/internal/consensus/pbts_test.go b/internal/consensus/pbts_test.go index 103318239..18d04fff0 100644 --- a/internal/consensus/pbts_test.go +++ b/internal/consensus/pbts_test.go @@ -149,7 +149,7 @@ func (p *pbtsTestHarness) observedValidatorProposerHeight(previousBlockTime time ensureNewRound(p.t, p.roundCh, p.currentHeight, p.currentRound) timeout := time.Until(previousBlockTime.Add(ensureTimeout)) - ensureProposalWithTimeout(p.t, p.ensureProposalCh, p.currentHeight, p.currentRound, types.BlockID{}, timeout) + ensureProposalWithTimeout(p.t, p.ensureProposalCh, p.currentHeight, p.currentRound, nil, timeout) rs := p.observedState.GetRoundState() bid := types.BlockID{Hash: rs.ProposalBlock.Hash(), PartSetHeader: rs.ProposalBlockParts.Header()} @@ -300,7 +300,7 @@ func (p *pbtsTestHarness) run() resultSet { r2 := p.height2() p.intermediateHeights() r5 := p.height5() - p.observedState.Stop() + _ = p.observedState.Stop() return resultSet{ genesisHeight: r1, height2: r2,