diff --git a/internal/consensus/pbts_test.go b/internal/consensus/pbts_test.go index 0d3709da9..f15265eb5 100644 --- a/internal/consensus/pbts_test.go +++ b/internal/consensus/pbts_test.go @@ -31,6 +31,9 @@ type pbtsTestHarness struct { // configuration options set by the user of the test harness. pbtsTestConfiguration + // The timestamp of the first block produced by the network. + height1Time time.Time + // The Tendermint consensus state machine being run during // a run of the pbtsTestHarness. observedState *State @@ -71,19 +74,19 @@ type pbtsTestConfiguration struct { // The setting to use for the TimeoutPropose configuration parameter. timeoutPropose time.Duration - // The timestamp of the first block produced by the network. + // The genesis time genesisTime time.Time - // The time at which the proposal at height 2 should be delivered. - height2ProposalDeliverTime time.Time + // The times offset from height 1 block time of the block proposed at height 2. + height2ProposedBlockOffset time.Duration - // The timestamp of the block proposed at height 2. - height2ProposedBlockTime time.Time + // The time offset from height 1 block time at which the proposal at height 2 should be delivered. + height2ProposalTimeDeliveryOffset time.Duration - // The timestamp of the block proposed at height 4. - // At height 4, the proposed block time and the deliver time are the same so + // The time offset from height 1 block time of the block proposed at height 4. + // At height 4, the proposed block and the deliver offsets are the same so // that timely-ness does not affect height 4. - height4ProposedBlockTime time.Time + height4ProposedBlockOffset time.Duration } func newPBTSTestHarness(ctx context.Context, t *testing.T, tc pbtsTestConfiguration) pbtsTestHarness { @@ -91,14 +94,19 @@ func newPBTSTestHarness(ctx context.Context, t *testing.T, tc pbtsTestConfigurat const validators = 4 cfg := configSetup(t) clock := new(tmtimemocks.Source) - if tc.height4ProposedBlockTime.IsZero() { - // Set a default height4ProposedBlockTime. + if tc.genesisTime.IsZero() { + tc.genesisTime = time.Now() + } + + if tc.height4ProposedBlockOffset == 0 { + + // Set a default height4ProposedBlockOffset. // Use a proposed block time that is greater than the time that the // block at height 2 was delivered. Height 3 is not relevant for testing // and always occurs blockTimeIota before height 4. If not otherwise specified, // height 4 therefore occurs 2*blockTimeIota after height 2. - tc.height4ProposedBlockTime = tc.height2ProposalDeliverTime.Add(2 * blockTimeIota) + tc.height4ProposedBlockOffset = tc.height2ProposalTimeDeliveryOffset + 2*blockTimeIota } cfg.Consensus.TimeoutPropose = tc.timeoutPropose consensusParams := types.DefaultConsensusParams() @@ -143,8 +151,8 @@ func newPBTSTestHarness(ctx context.Context, t *testing.T, tc pbtsTestConfigurat } } -func (p *pbtsTestHarness) observedValidatorProposerHeight(previousBlockTime time.Time) heightResult { - p.validatorClock.On("Now").Return(p.height2ProposedBlockTime).Times(6) +func (p *pbtsTestHarness) observedValidatorProposerHeight(previousBlockTime time.Time) (heightResult, time.Time) { + p.validatorClock.On("Now").Return(p.genesisTime.Add(p.height2ProposedBlockOffset)).Times(6) ensureNewRound(p.t, p.roundCh, p.currentHeight, p.currentRound) @@ -167,26 +175,33 @@ func (p *pbtsTestHarness) observedValidatorProposerHeight(previousBlockTime time p.currentHeight++ incrementHeight(p.otherValidators...) - return res + return res, rs.ProposalBlock.Time } func (p *pbtsTestHarness) height2() heightResult { signer := p.otherValidators[0].PrivValidator - height3BlockTime := p.height2ProposedBlockTime.Add(-blockTimeIota) - return p.nextHeight(signer, p.height2ProposalDeliverTime, p.height2ProposedBlockTime, height3BlockTime) + return p.nextHeight(signer, + p.height1Time.Add(p.height2ProposalTimeDeliveryOffset), + p.height1Time.Add(p.height2ProposedBlockOffset), + p.height1Time.Add(p.height2ProposedBlockOffset+10*blockTimeIota)) } func (p *pbtsTestHarness) intermediateHeights() { signer := p.otherValidators[1].PrivValidator - blockTimeHeight3 := p.height4ProposedBlockTime.Add(-blockTimeIota) - p.nextHeight(signer, blockTimeHeight3, blockTimeHeight3, p.height4ProposedBlockTime) + p.nextHeight(signer, + p.height1Time.Add(p.height2ProposedBlockOffset+10*blockTimeIota), + p.height1Time.Add(p.height2ProposedBlockOffset+10*blockTimeIota), + p.height1Time.Add(p.height4ProposedBlockOffset)) signer = p.otherValidators[2].PrivValidator - p.nextHeight(signer, p.height4ProposedBlockTime, p.height4ProposedBlockTime, time.Now()) + p.nextHeight(signer, + p.height1Time.Add(p.height4ProposedBlockOffset), + p.height1Time.Add(p.height4ProposedBlockOffset), + time.Now()) } -func (p *pbtsTestHarness) height5() heightResult { - return p.observedValidatorProposerHeight(p.height4ProposedBlockTime) +func (p *pbtsTestHarness) height5() (heightResult, time.Time) { + return p.observedValidatorProposerHeight(p.height1Time.Add(p.height4ProposedBlockOffset)) } // nolint: lll @@ -296,10 +311,11 @@ type timestampedEvent struct { func (p *pbtsTestHarness) run() resultSet { startTestRound(p.ctx, p.observedState, p.currentHeight, p.currentRound) - r1 := p.observedValidatorProposerHeight(p.genesisTime) + r1, proposalBlockTime := p.observedValidatorProposerHeight(p.genesisTime) + p.height1Time = proposalBlockTime r2 := p.height2() p.intermediateHeights() - r5 := p.height5() + r5, _ := p.height5() return resultSet{ genesisHeight: r1, height2: r2, @@ -337,10 +353,11 @@ func TestProposerWaitsForGenesisTime(t *testing.T) { Precision: 10 * time.Millisecond, MessageDelay: 10 * time.Millisecond, }, - timeoutPropose: 10 * time.Millisecond, - genesisTime: initialTime, - height2ProposalDeliverTime: initialTime.Add(10 * time.Millisecond), - height2ProposedBlockTime: initialTime.Add(10 * time.Millisecond), + timeoutPropose: 10 * time.Millisecond, + genesisTime: initialTime, + height2ProposalTimeDeliveryOffset: 10 * time.Millisecond, + height2ProposedBlockOffset: 10 * time.Millisecond, + height4ProposedBlockOffset: 30 * time.Millisecond, } pbtsTest := newPBTSTestHarness(ctx, t, cfg) @@ -365,11 +382,11 @@ func TestProposerWaitsForPreviousBlock(t *testing.T) { Precision: 100 * time.Millisecond, MessageDelay: 500 * time.Millisecond, }, - timeoutPropose: 50 * time.Millisecond, - genesisTime: initialTime, - height2ProposalDeliverTime: initialTime.Add(150 * time.Millisecond), - height2ProposedBlockTime: initialTime.Add(100 * time.Millisecond), - height4ProposedBlockTime: initialTime.Add(800 * time.Millisecond), + timeoutPropose: 50 * time.Millisecond, + genesisTime: initialTime, + height2ProposalTimeDeliveryOffset: 150 * time.Millisecond, + height2ProposedBlockOffset: 100 * time.Millisecond, + height4ProposedBlockOffset: 800 * time.Millisecond, } pbtsTest := newPBTSTestHarness(ctx, t, cfg) @@ -378,7 +395,7 @@ func TestProposerWaitsForPreviousBlock(t *testing.T) { // the observed validator is the proposer at height 5. // ensure that the observed validator did not propose a block until after // the time configured for height 4. - assert.True(t, results.height5.proposalIssuedAt.After(cfg.height4ProposedBlockTime)) + assert.True(t, results.height5.proposalIssuedAt.After(pbtsTest.height1Time.Add(cfg.height4ProposedBlockOffset))) // Ensure that the validator issued a prevote for a non-nil block. assert.NotNil(t, results.height5.prevote.BlockID.Hash) @@ -434,10 +451,10 @@ func TestTimelyProposal(t *testing.T) { 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), + timeoutPropose: 40 * time.Millisecond, + genesisTime: initialTime, + height2ProposedBlockOffset: 15 * time.Millisecond, + height2ProposalTimeDeliveryOffset: 30 * time.Millisecond, } pbtsTest := newPBTSTestHarness(ctx, t, cfg) @@ -449,18 +466,15 @@ func TestTooFarInThePastProposal(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - initialTime := time.Now() - // localtime > proposedBlockTime + MsgDelay + Precision cfg := pbtsTestConfiguration{ synchronyParams: types.SynchronyParams{ Precision: 1 * time.Millisecond, MessageDelay: 10 * time.Millisecond, }, - timeoutPropose: 50 * time.Millisecond, - genesisTime: initialTime, - height2ProposedBlockTime: initialTime.Add(10 * time.Millisecond), - height2ProposalDeliverTime: initialTime.Add(21 * time.Millisecond), + timeoutPropose: 50 * time.Millisecond, + height2ProposedBlockOffset: 15 * time.Millisecond, + height2ProposalTimeDeliveryOffset: 27 * time.Millisecond, } pbtsTest := newPBTSTestHarness(ctx, t, cfg) @@ -474,19 +488,16 @@ func TestTooFarInTheFutureProposal(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - initialTime := time.Now() - // localtime < proposedBlockTime - Precision cfg := pbtsTestConfiguration{ synchronyParams: types.SynchronyParams{ 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), - height4ProposedBlockTime: initialTime.Add(150 * time.Millisecond), + timeoutPropose: 50 * time.Millisecond, + height2ProposedBlockOffset: 100 * time.Millisecond, + height2ProposalTimeDeliveryOffset: 10 * time.Millisecond, + height4ProposedBlockOffset: 150 * time.Millisecond, } pbtsTest := newPBTSTestHarness(ctx, t, cfg) diff --git a/internal/consensus/state.go b/internal/consensus/state.go index ef2a83bc4..c60df5455 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -1319,7 +1319,7 @@ func (cs *State) proposalIsTimely() bool { MessageDelay: cs.state.ConsensusParams.Synchrony.MessageDelay, } - return cs.Proposal.IsTimely(cs.ProposalReceiveTime, sp, cs.state.InitialHeight) + return cs.Proposal.IsTimely(cs.ProposalReceiveTime, sp) } func (cs *State) defaultDoPrevote(height int64, round int32) { @@ -1425,7 +1425,7 @@ func (cs *State) defaultDoPrevote(height int64, round int32) { } } - logger.Debug("prevote step: ProposalBlock is valid but was not our locked block or" + + 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(tmproto.PrevoteType, nil, types.PartSetHeader{}) } diff --git a/internal/consensus/state_test.go b/internal/consensus/state_test.go index a23f6cc1e..001309ce7 100644 --- a/internal/consensus/state_test.go +++ b/internal/consensus/state_test.go @@ -1611,7 +1611,7 @@ func TestStateLock_POLSafety2(t *testing.T) { ensureNewProposal(t, proposalCh, height, round) ensurePrevote(t, voteCh, height, round) - validatePrevote(ctx, t, cs1, round, vss[0], propBlockID1.Hash) + validatePrevote(ctx, t, cs1, round, vss[0], nil) } diff --git a/internal/state/state.go b/internal/state/state.go index fcbacdfcc..0b8989552 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -8,6 +8,7 @@ import ( "time" "github.com/gogo/protobuf/proto" + tmtime "github.com/tendermint/tendermint/libs/time" tmstate "github.com/tendermint/tendermint/proto/tendermint/state" tmversion "github.com/tendermint/tendermint/proto/tendermint/version" @@ -263,18 +264,10 @@ func (state State) MakeBlock( // Build base block with block data. block := types.MakeBlock(height, txs, commit, evidence) - // Set time. - var timestamp time.Time - if height == state.InitialHeight { - timestamp = state.LastBlockTime // genesis time - } else { - timestamp = time.Now() - } - // Fill rest of header with state data. block.Header.Populate( state.Version.Consensus, state.ChainID, - timestamp, state.LastBlockID, + tmtime.Now(), state.LastBlockID, state.Validators.Hash(), state.NextValidators.Hash(), state.ConsensusParams.HashConsensusParams(), state.AppHash, state.LastResultsHash, proposerAddress, diff --git a/internal/state/validation.go b/internal/state/validation.go index 8bec8f119..900b7b787 100644 --- a/internal/state/validation.go +++ b/internal/state/validation.go @@ -117,8 +117,8 @@ func validateBlock(state State, block *types.Block) error { case block.Height == state.InitialHeight: genesisTime := state.LastBlockTime - if !block.Time.Equal(genesisTime) { - return fmt.Errorf("block time %v is not equal to genesis time %v", + if block.Time.Before(genesisTime) { + return fmt.Errorf("block time %v is before genesis time %v", block.Time, genesisTime, ) diff --git a/rpc/client/evidence_test.go b/rpc/client/evidence_test.go index ae4e29f52..9187ddc1a 100644 --- a/rpc/client/evidence_test.go +++ b/rpc/client/evidence_test.go @@ -15,15 +15,11 @@ import ( "github.com/tendermint/tendermint/types" ) -// For some reason the empty node used in tests has a time of -// 2018-10-10 08:20:13.695936996 +0000 UTC -// this is because the test genesis time is set here -// so in order to validate evidence we need evidence to be the same time -var defaultTestTime = time.Date(2018, 10, 10, 8, 20, 13, 695936996, time.UTC) - func newEvidence(t *testing.T, val *privval.FilePV, vote *types.Vote, vote2 *types.Vote, - chainID string) *types.DuplicateVoteEvidence { + chainID string, + timestamp time.Time, +) *types.DuplicateVoteEvidence { t.Helper() var err error @@ -39,7 +35,7 @@ func newEvidence(t *testing.T, val *privval.FilePV, validator := types.NewValidator(val.Key.PubKey, 10) valSet := types.NewValidatorSet([]*types.Validator{validator}) - ev, err := types.NewDuplicateVoteEvidence(vote, vote2, defaultTestTime, valSet) + ev, err := types.NewDuplicateVoteEvidence(vote, vote2, timestamp, valSet) require.NoError(t, err) return ev } @@ -48,6 +44,7 @@ func makeEvidences( t *testing.T, val *privval.FilePV, chainID string, + timestamp time.Time, ) (correct *types.DuplicateVoteEvidence, fakes []*types.DuplicateVoteEvidence) { vote := types.Vote{ ValidatorAddress: val.Key.Address, @@ -55,7 +52,7 @@ func makeEvidences( Height: 1, Round: 0, Type: tmproto.PrevoteType, - Timestamp: defaultTestTime, + Timestamp: timestamp, BlockID: types.BlockID{ Hash: tmhash.Sum(tmrand.Bytes(tmhash.Size)), PartSetHeader: types.PartSetHeader{ @@ -67,7 +64,7 @@ func makeEvidences( vote2 := vote vote2.BlockID.Hash = tmhash.Sum([]byte("blockhash2")) - correct = newEvidence(t, val, &vote, &vote2, chainID) + correct = newEvidence(t, val, &vote, &vote2, chainID, timestamp) fakes = make([]*types.DuplicateVoteEvidence, 0) @@ -75,34 +72,34 @@ func makeEvidences( { v := vote2 v.ValidatorAddress = []byte("some_address") - fakes = append(fakes, newEvidence(t, val, &vote, &v, chainID)) + fakes = append(fakes, newEvidence(t, val, &vote, &v, chainID, timestamp)) } // different height { v := vote2 v.Height = vote.Height + 1 - fakes = append(fakes, newEvidence(t, val, &vote, &v, chainID)) + fakes = append(fakes, newEvidence(t, val, &vote, &v, chainID, timestamp)) } // different round { v := vote2 v.Round = vote.Round + 1 - fakes = append(fakes, newEvidence(t, val, &vote, &v, chainID)) + fakes = append(fakes, newEvidence(t, val, &vote, &v, chainID, timestamp)) } // different type { v := vote2 v.Type = tmproto.PrecommitType - fakes = append(fakes, newEvidence(t, val, &vote, &v, chainID)) + fakes = append(fakes, newEvidence(t, val, &vote, &v, chainID, timestamp)) } // exactly same vote { v := vote - fakes = append(fakes, newEvidence(t, val, &vote, &v, chainID)) + fakes = append(fakes, newEvidence(t, val, &vote, &v, chainID, timestamp)) } return correct, fakes diff --git a/rpc/client/rpc_test.go b/rpc/client/rpc_test.go index 12c13d686..a94af8d72 100644 --- a/rpc/client/rpc_test.go +++ b/rpc/client/rpc_test.go @@ -516,10 +516,12 @@ func TestClientMethodCalls(t *testing.T) { t.Run("BraodcastDuplicateVote", func(t *testing.T) { chainID := conf.ChainID() - correct, fakes := makeEvidences(t, pv, chainID) - // make sure that the node has produced enough blocks waitForBlock(ctx, t, c, 2) + evidenceHeight := int64(1) + block, _ := c.Block(ctx, &evidenceHeight) + ts := block.Block.Time + correct, fakes := makeEvidences(t, pv, chainID, ts) result, err := c.BroadcastEvidence(ctx, correct) require.NoError(t, err, "BroadcastEvidence(%s) failed", correct) diff --git a/types/proposal.go b/types/proposal.go index f9b78a222..56eb79287 100644 --- a/types/proposal.go +++ b/types/proposal.go @@ -87,20 +87,17 @@ func (p *Proposal) ValidateBasic() error { // localtime >= proposedBlockTime - Precision // localtime <= proposedBlockTime + MsgDelay + Precision // -// Note: If the proposal is for the `initialHeight` the second inequality is not checked. This is because -// the timestamp in this case is set to the preconfigured genesis time. // For more information on the meaning of 'timely', see the proposer-based timestamp specification: // https://github.com/tendermint/spec/tree/master/spec/consensus/proposer-based-timestamp -func (p *Proposal) IsTimely(recvTime time.Time, sp SynchronyParams, initialHeight int64) bool { +func (p *Proposal) IsTimely(recvTime time.Time, sp SynchronyParams) bool { // lhs is `proposedBlockTime - Precision` in the first inequality lhs := p.Timestamp.Add(-sp.Precision) // rhs is `proposedBlockTime + MsgDelay + Precision` in the second inequality rhs := p.Timestamp.Add(sp.MessageDelay).Add(sp.Precision) - if recvTime.Before(lhs) || (p.Height != initialHeight && recvTime.After(rhs)) { + if recvTime.Before(lhs) || recvTime.After(rhs) { return false } - return true } diff --git a/types/proposal_test.go b/types/proposal_test.go index fc8a6570b..d7791724f 100644 --- a/types/proposal_test.go +++ b/types/proposal_test.go @@ -198,7 +198,6 @@ func TestIsTimely(t *testing.T) { require.NoError(t, err) testCases := []struct { name string - genesisHeight int64 proposalHeight int64 proposalTime time.Time recvTime time.Time @@ -211,7 +210,6 @@ func TestIsTimely(t *testing.T) { // Checking that the following inequality evaluates to true: // 0 - 2 <= 1 <= 0 + 1 + 2 name: "basic timely", - genesisHeight: 1, proposalHeight: 2, proposalTime: genesisTime, recvTime: genesisTime.Add(1 * time.Nanosecond), @@ -223,7 +221,6 @@ func TestIsTimely(t *testing.T) { // Checking that the following inequality evaluates to false: // 0 - 2 <= 4 <= 0 + 1 + 2 name: "local time too large", - genesisHeight: 1, proposalHeight: 2, proposalTime: genesisTime, recvTime: genesisTime.Add(4 * time.Nanosecond), @@ -235,7 +232,6 @@ func TestIsTimely(t *testing.T) { // Checking that the following inequality evaluates to false: // 4 - 2 <= 0 <= 4 + 2 + 1 name: "proposal time too large", - genesisHeight: 1, proposalHeight: 2, proposalTime: genesisTime.Add(4 * time.Nanosecond), recvTime: genesisTime, @@ -243,32 +239,6 @@ func TestIsTimely(t *testing.T) { msgDelay: time.Nanosecond, expectTimely: false, }, - { - // Checking that the following inequality evaluates to true: - // 0 - 2 <= 4 - // and the following check is skipped - // 4 <= 0 + 1 + 2 - name: "local time too large but proposal is for genesis", - genesisHeight: 1, - proposalHeight: 1, - proposalTime: genesisTime, - recvTime: genesisTime.Add(4 * time.Nanosecond), - precision: time.Nanosecond * 2, - msgDelay: time.Nanosecond, - expectTimely: true, - }, - { - // Checking that the following inequality evaluates to false: - // 4 - 2 <= 0 - name: "proposal time too large for genesis block proposal", - genesisHeight: 1, - proposalHeight: 1, - proposalTime: genesisTime.Add(4 * time.Nanosecond), - recvTime: genesisTime, - precision: time.Nanosecond * 2, - msgDelay: time.Nanosecond, - expectTimely: false, - }, } for _, testCase := range testCases { @@ -283,7 +253,7 @@ func TestIsTimely(t *testing.T) { MessageDelay: testCase.msgDelay, } - ti := p.IsTimely(testCase.recvTime, sp, testCase.genesisHeight) + ti := p.IsTimely(testCase.recvTime, sp) assert.Equal(t, testCase.expectTimely, ti) }) }