From e1538bf67ea5122b0add392e93bd2a3ea73dee69 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Fri, 12 Oct 2018 09:03:58 +0400 Subject: [PATCH] state: require block.Time of the fist block to be genesis time (#2594) * require block.Time of the fist block to be genesis time Refs #2587: ``` We only start validating block.Time when Height > 1, because there is no commit to compute the median timestamp from for the first block. This means a faulty proposer could make the first block with whatever time they want. Instead, we should require the timestamp of block 1 to match the genesis time. I discovered this while refactoring the ValidateBlock tests to be table-driven while working on tests for #2560. ``` * do not accept blocks with negative height * update changelog and spec * nanos precision for test genesis time * Fix failing test (#2607) --- CHANGELOG_PENDING.md | 1 + cmd/tendermint/commands/lite.go | 14 +++++++------- config/toml.go | 2 +- consensus/state_test.go | 7 ++++--- docs/spec/blockchain/blockchain.md | 9 +++++++++ p2p/netaddress.go | 1 + state/state.go | 5 +---- state/validation.go | 9 +++++++++ state/validation_test.go | 11 ++++++----- types/block.go | 7 +++++++ types/block_test.go | 3 ++- 11 files changed, 48 insertions(+), 21 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index dace33f1..f1268402 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -28,6 +28,7 @@ BREAKING CHANGES: * Blockchain Protocol * [types] \#2459 `Vote`/`Proposal`/`Heartbeat` use amino encoding instead of JSON in `SignBytes`. * [types] \#2512 Remove the pubkey field from the validator hash + * [state] \#2587 require block.Time of the fist block to be genesis time * P2P Protocol diff --git a/cmd/tendermint/commands/lite.go b/cmd/tendermint/commands/lite.go index bc51d7de..eb2817b6 100644 --- a/cmd/tendermint/commands/lite.go +++ b/cmd/tendermint/commands/lite.go @@ -26,12 +26,12 @@ just with added trust and running locally.`, } var ( - listenAddr string - nodeAddr string - chainID string - home string - maxOpenConnections int - cacheSize int + listenAddr string + nodeAddr string + chainID string + home string + maxOpenConnections int + cacheSize int ) func init() { @@ -39,7 +39,7 @@ func init() { LiteCmd.Flags().StringVar(&nodeAddr, "node", "tcp://localhost:26657", "Connect to a Tendermint node at this address") LiteCmd.Flags().StringVar(&chainID, "chain-id", "tendermint", "Specify the Tendermint chain ID") LiteCmd.Flags().StringVar(&home, "home-dir", ".tendermint-lite", "Specify the home directory") - LiteCmd.Flags().IntVar(&maxOpenConnections,"max-open-connections",900,"Maximum number of simultaneous connections (including WebSocket).") + LiteCmd.Flags().IntVar(&maxOpenConnections, "max-open-connections", 900, "Maximum number of simultaneous connections (including WebSocket).") LiteCmd.Flags().IntVar(&cacheSize, "cache-size", 10, "Specify the memory trust store cache size") } diff --git a/config/toml.go b/config/toml.go index ddfe5f05..62e5fa97 100644 --- a/config/toml.go +++ b/config/toml.go @@ -342,7 +342,7 @@ func ResetTestRoot(testName string) *Config { } var testGenesis = `{ - "genesis_time": "0001-01-01T00:00:00.000Z", + "genesis_time": "2017-10-10T08:20:13.695936996Z", "chain_id": "tendermint_test", "validators": [ { diff --git a/consensus/state_test.go b/consensus/state_test.go index 831f77f4..e7d4b4fa 100644 --- a/consensus/state_test.go +++ b/consensus/state_test.go @@ -455,8 +455,9 @@ func TestStateLockNoPOL(t *testing.T) { ensureNewTimeout(timeoutWaitCh, cs1.config.TimeoutPrecommit.Nanoseconds()) + cs2, _ := randConsensusState(2) // needed so generated block is different than locked block // before we time out into new round, set next proposal block - prop, propBlock := decideProposal(cs1, vs2, vs2.Height, vs2.Round+1) + prop, propBlock := decideProposal(cs2, vs2, vs2.Height, vs2.Round+1) if prop == nil || propBlock == nil { t.Fatal("Failed to create proposal block with vs2") } @@ -479,7 +480,7 @@ func TestStateLockNoPOL(t *testing.T) { ensureNewVote(voteCh) // prevote // prevote for locked block (not proposal) - validatePrevote(t, cs1, 0, vss[0], cs1.LockedBlock.Hash()) + validatePrevote(t, cs1, 3, vss[0], cs1.LockedBlock.Hash()) signAddVotes(cs1, types.VoteTypePrevote, propBlock.Hash(), propBlock.MakePartSet(partSize).Header(), vs2) ensureNewVote(voteCh) @@ -487,7 +488,7 @@ func TestStateLockNoPOL(t *testing.T) { ensureNewTimeout(timeoutWaitCh, cs1.config.TimeoutPrevote.Nanoseconds()) ensureNewVote(voteCh) - validatePrecommit(t, cs1, 2, 0, vss[0], nil, theBlockHash) // precommit nil but locked on proposal + validatePrecommit(t, cs1, 3, 0, vss[0], nil, theBlockHash) // precommit nil but locked on proposal signAddVotes(cs1, types.VoteTypePrecommit, propBlock.Hash(), propBlock.MakePartSet(partSize).Header(), vs2) // NOTE: conflicting precommits at same height ensureNewVote(voteCh) diff --git a/docs/spec/blockchain/blockchain.md b/docs/spec/blockchain/blockchain.md index bd0af70a..4a433b5d 100644 --- a/docs/spec/blockchain/blockchain.md +++ b/docs/spec/blockchain/blockchain.md @@ -230,6 +230,15 @@ It must equal the weighted median of the timestamps of the valid votes in the bl Note: the timestamp of a vote must be greater by at least one millisecond than that of the block being voted on. +The timestamp of the first block must be equal to the genesis time (since +there's no votes to compute the median). + +``` +if block.Header.Height == 1 { + block.Header.Timestamp == genesisTime +} +``` + See the section on [BFT time](../consensus/bft-time.md) for more details. ### NumTxs diff --git a/p2p/netaddress.go b/p2p/netaddress.go index f848b7a5..ec9a0ea7 100644 --- a/p2p/netaddress.go +++ b/p2p/netaddress.go @@ -14,6 +14,7 @@ import ( "time" "errors" + cmn "github.com/tendermint/tendermint/libs/common" ) diff --git a/state/state.go b/state/state.go index 1f60fd65..23c0d632 100644 --- a/state/state.go +++ b/state/state.go @@ -118,10 +118,7 @@ func (state State) MakeBlock( // Set time if height == 1 { - block.Time = tmtime.Now() - if block.Time.Before(state.LastBlockTime) { - block.Time = state.LastBlockTime // state.LastBlockTime for height == 1 is genesis time - } + block.Time = state.LastBlockTime // genesis time } else { block.Time = MedianTime(commit, state.LastValidators) } diff --git a/state/validation.go b/state/validation.go index 9d8ef97a..a308870e 100644 --- a/state/validation.go +++ b/state/validation.go @@ -123,6 +123,15 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error { block.Time, ) } + } else if block.Height == 1 { + genesisTime := state.LastBlockTime + if !block.Time.Equal(genesisTime) { + return fmt.Errorf( + "Block time %v is not equal to genesis time %v", + block.Time, + genesisTime, + ) + } } // Limit the amount of evidence diff --git a/state/validation_test.go b/state/validation_test.go index e5f45166..3c58c713 100644 --- a/state/validation_test.go +++ b/state/validation_test.go @@ -2,6 +2,7 @@ package state import ( "testing" + "time" "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/crypto/ed25519" @@ -32,11 +33,11 @@ func TestValidateBlockHeader(t *testing.T) { name string malleateBlock func(block *types.Block) }{ - {"ChainID wrong", func(block *types.Block) { block.ChainID = "not-the-real-one" }}, // wrong chain id - {"Height wrong", func(block *types.Block) { block.Height += 10 }}, // wrong height - // TODO(#2589) (#2587) : {"Time", func(block *types.Block) { block.Time.Add(-time.Second * 3600 * 24) }}, // wrong time - {"NumTxs wrong", func(block *types.Block) { block.NumTxs += 10 }}, // wrong num txs - {"TotalTxs wrong", func(block *types.Block) { block.TotalTxs += 10 }}, // wrong total txs + {"ChainID wrong", func(block *types.Block) { block.ChainID = "not-the-real-one" }}, + {"Height wrong", func(block *types.Block) { block.Height += 10 }}, + {"Time wrong", func(block *types.Block) { block.Time = block.Time.Add(-time.Second * 3600 * 24) }}, + {"NumTxs wrong", func(block *types.Block) { block.NumTxs += 10 }}, + {"TotalTxs wrong", func(block *types.Block) { block.TotalTxs += 10 }}, {"LastBlockID wrong", func(block *types.Block) { block.LastBlockID.PartsHeader.Total += 10 }}, {"LastCommitHash wrong", func(block *types.Block) { block.LastCommitHash = wrongHash }}, diff --git a/types/block.go b/types/block.go index 07a71ca8..bd9092f4 100644 --- a/types/block.go +++ b/types/block.go @@ -64,6 +64,13 @@ func (b *Block) ValidateBasic() error { b.mtx.Lock() defer b.mtx.Unlock() + if b.Height < 0 { + return fmt.Errorf( + "Negative Block.Header.Height: %v", + b.Height, + ) + } + newTxs := int64(len(b.Data.Txs)) if b.NumTxs != newTxs { return fmt.Errorf( diff --git a/types/block_test.go b/types/block_test.go index 887f35a1..962aa002 100644 --- a/types/block_test.go +++ b/types/block_test.go @@ -60,6 +60,7 @@ func TestBlockValidateBasic(t *testing.T) { }{ {"Make Block", func(blk *Block) {}, false}, {"Make Block w/ proposer Addr", func(blk *Block) { blk.ProposerAddress = valSet.GetProposer().Address }, false}, + {"Negative Height", func(blk *Block) { blk.Height = -1 }, true}, {"Increase NumTxs", func(blk *Block) { blk.NumTxs++ }, true}, {"Remove 1/2 the commits", func(blk *Block) { blk.LastCommit.Precommits = commit.Precommits[:commit.Size()/2] @@ -81,7 +82,7 @@ func TestBlockValidateBasic(t *testing.T) { t.Run(tc.testName, func(t *testing.T) { block := MakeBlock(h, txs, commit, evList) tc.malleateBlock(block) - assert.Equal(t, tc.expErr, block.ValidateBasic() != nil, "Validate Basic had an unexpected result") + assert.Equal(t, tc.expErr, block.ValidateBasic() != nil, "ValidateBasic had an unexpected result") }) } }