From 2ff233d944df0ac95ed0d0ce74f8f0c834fb97f4 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] 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 | 9 +-- internal/consensus/state.go | 17 +++- internal/consensus/state_test.go | 112 +++++++++++++++++++++++++-- types/proposal.go | 4 +- types/proposal_test.go | 9 ++- 8 files changed, 135 insertions(+), 22 deletions(-) diff --git a/internal/consensus/byzantine_test.go b/internal/consensus/byzantine_test.go index 31ecdaf34..060286798 100644 --- a/internal/consensus/byzantine_test.go +++ b/internal/consensus/byzantine_test.go @@ -216,7 +216,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 4cf80cf15..3407010fe 100644 --- a/internal/consensus/common_test.go +++ b/internal/consensus/common_test.go @@ -245,7 +245,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() require.NoError(t, vs.SignProposal(ctx, chainID, p)) diff --git a/internal/consensus/pbts_test.go b/internal/consensus/pbts_test.go index 69b04e265..ea11a09c6 100644 --- a/internal/consensus/pbts_test.go +++ b/internal/consensus/pbts_test.go @@ -205,7 +205,7 @@ func (p *pbtsTestHarness) nextHeight(proposer types.PrivValidator, deliverTime, ps, err := b.MakePartSet(types.BlockPartSizeBytes) require.NoError(p.t, err) 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 89f03c2f7..4a8335515 100644 --- a/internal/consensus/replay_test.go +++ b/internal/consensus/replay_test.go @@ -387,7 +387,7 @@ func setupSimulator(ctx context.Context, t *testing.T) *simulatorTestSuite { require.NoError(t, err) 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) @@ -421,7 +421,7 @@ func setupSimulator(ctx context.Context, t *testing.T) *simulatorTestSuite { require.NoError(t, err) 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) @@ -484,7 +484,7 @@ func setupSimulator(ctx context.Context, t *testing.T) *simulatorTestSuite { selfIndex := valIndexFn(0) require.NotEqual(t, -1, selfIndex) - 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) @@ -555,8 +555,7 @@ func setupSimulator(ctx context.Context, t *testing.T) *simulatorTestSuite { selfIndex = valIndexFn(0) require.NotEqual(t, -1, selfIndex) - - 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 daceb42eb..f6ea2ba47 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -1247,7 +1247,7 @@ func (cs *State) defaultDecideProposal(ctx context.Context, height int64, round // 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 @@ -1369,6 +1369,12 @@ func (cs *State) defaultDoPrevote(ctx context.Context, height int64, round int32 return } + if !cs.Proposal.Timestamp.Equal(cs.ProposalBlock.Header.Time) { + logger.Debug("proposal timestamp not equal, prevoting nil") + cs.signAddVote(ctx, tmproto.PrevoteType, nil, types.PartSetHeader{}) + return + } + // Validate proposal block err := cs.blockExec.ValidateBlock(cs.state, cs.ProposalBlock) if err != nil { @@ -1393,6 +1399,7 @@ func (cs *State) defaultDoPrevote(ctx context.Context, 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(ctx, tmproto.PrevoteType, cs.ProposalBlock.Hash(), cs.ProposalBlockParts.Header()) return @@ -1528,9 +1535,15 @@ func (cs *State) enterPrecommit(ctx context.Context, height int64, round int32) cs.signAddVote(ctx, 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(ctx, 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 771cf2b98..358d5f028 100644 --- a/internal/consensus/state_test.go +++ b/internal/consensus/state_test.go @@ -238,7 +238,7 @@ func TestStateBadProposal(t *testing.T) { propBlockParts, err := propBlock.MakePartSet(partSize) require.NoError(t, err) 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) @@ -299,7 +299,7 @@ func TestStateOversizedBlock(t *testing.T) { propBlockParts, err := propBlock.MakePartSet(partSize) require.NoError(t, err) 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) @@ -843,7 +843,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) @@ -1208,7 +1208,7 @@ func TestStateLock_POLDoesNotUnlock(t *testing.T) { // new block if a proposal was not seen for that block. func TestStateLock_MissingProposalWhenPOLSeenDoesNotUpdateLock(t *testing.T) { config := configSetup(t) - logger := log.TestingLogger() + logger := log.NewTestingLogger(t) ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -1573,7 +1573,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) @@ -1714,7 +1714,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) @@ -2636,6 +2636,106 @@ 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 := makeState(ctx, t, config, logger, 4) + 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, _, err := cs1.createProposalBlock() + require.NoError(t, err) + round++ + incrementRound(vss[1:]...) + + propBlockParts, err := propBlock.MakePartSet(types.BlockPartSizeBytes) + require.NoError(t, err) + 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(ctx, proposal, propBlock, propBlockParts, "some peer")) + + startTestRound(ctx, cs1, height, round) + ensureProposal(t, proposalCh, height, round, blockID) + + signAddVotes(ctx, t, 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 := makeState(ctx, t, config, logger, 4) + 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, _, err := cs1.createProposalBlock() + require.NoError(t, err) + round++ + incrementRound(vss[1:]...) + + propBlockParts, err := propBlock.MakePartSet(types.BlockPartSizeBytes) + require.NoError(t, err) + 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(ctx, proposal, propBlock, propBlockParts, "some peer")) + + startTestRound(ctx, cs1, height, round) + ensureProposal(t, proposalCh, height, round, blockID) + + signAddVotes(ctx, t, 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 76983b2a2..8daccdd82 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 7ab6535ba..136e8a8e3 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" ) @@ -61,7 +62,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) @@ -173,7 +174,7 @@ func TestProposalValidateBasic(t *testing.T) { prop := NewProposal( 4, 2, 2, - blockID) + blockID, tmtime.Now()) p := prop.ToProto() err := privVal.SignProposal(ctx, "test_chain_id", p) prop.Signature = p.Signature @@ -185,9 +186,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