validate reactor messages (#2711)

* validate reactor messages

Refs #2683

* validate blockchain messages

Refs #2683

* validate evidence messages

Refs #2683

* todo

* check ProposalPOL and signature sizes

* add a changelog entry

* check addr is valid when we add it to the addrbook

* validate incoming netAddr (not just nil check!)

* fixes after Bucky's review

* check timestamps

* beef up block#ValidateBasic

* move some checks into bcBlockResponseMessage

* update Gopkg.lock

Fix

```
grouped write of manifest, lock and vendor: failed to export github.com/tendermint/go-amino: fatal: failed to unpack tree object 6dcc6ddc143e116455c94b25c1004c99e0d0ca12
```

by running `dep ensure -update`

* bump year since now we check it

* generate test/p2p/data on the fly using tendermint testnet

* allow sync chains older than 1 year

* use full path when creating a testnet

* move testnet gen to test/docker/Dockerfile

* relax LastCommitRound check

Refs #2737

* fix conflicts after merge

* add small comment

* some ValidateBasic updates

* fixes

* AppHash length is not fixed

CHANGELOG_PENDING.md

@@ -92,6 +92,10 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
   github.com/tendermint/crypto
 - [tools] [\#2238](https://github.com/tendermint/tendermint/issues/2238) Binary dependencies are now locked to a specific git commit
 - [libs/log] [\#2706](https://github.com/tendermint/tendermint/issues/2706) Add year to log format
+- [consensus] [\#2683] validate all incoming messages
+- [evidence] [\#2683] validate all incoming messages
+- [blockchain] [\#2683] validate all incoming messages
+- [p2p/pex] [\#2683] validate pexAddrsMessage addresses
 
 ### BUG FIXES:
 - [autofile] [\#2428](https://github.com/tendermint/tendermint/issues/2428) Group.RotateFile need call Flush() before rename (@goolAdapter)

blockchain/reactor.go

@@ -1,6 +1,7 @@
 package blockchain
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 	"time"
@@ -180,6 +181,12 @@ func (bcR *BlockchainReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte)
 		return
 	}
 
+	if err = msg.ValidateBasic(); err != nil {
+		bcR.Logger.Error("Peer sent us invalid msg", "peer", src, "msg", msg, "err", err)
+		bcR.Switch.StopPeerForError(src, err)
+		return
+	}
+
 	bcR.Logger.Debug("Receive", "src", src, "chID", chID, "msg", msg)
 
 	switch msg := msg.(type) {
@@ -188,7 +195,6 @@ func (bcR *BlockchainReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte)
 			// Unfortunately not queued since the queue is full.
 		}
 	case *bcBlockResponseMessage:
-		// Got a block.
 		bcR.pool.AddBlock(src.ID(), msg.Block, len(msgBytes))
 	case *bcStatusRequestMessage:
 		// Send peer our state.
@@ -352,7 +358,9 @@ func (bcR *BlockchainReactor) BroadcastStatusRequest() error {
 // Messages
 
 // BlockchainMessage is a generic message for this reactor.
-type BlockchainMessage interface{}
+type BlockchainMessage interface {
+	ValidateBasic() error
+}
 
 func RegisterBlockchainMessages(cdc *amino.Codec) {
 	cdc.RegisterInterface((*BlockchainMessage)(nil), nil)
@@ -377,6 +385,14 @@ type bcBlockRequestMessage struct {
 	Height int64
 }
 
+// ValidateBasic performs basic validation.
+func (m *bcBlockRequestMessage) ValidateBasic() error {
+	if m.Height < 0 {
+		return errors.New("Negative Height")
+	}
+	return nil
+}
+
 func (m *bcBlockRequestMessage) String() string {
 	return fmt.Sprintf("[bcBlockRequestMessage %v]", m.Height)
 }
@@ -385,6 +401,14 @@ type bcNoBlockResponseMessage struct {
 	Height int64
 }
 
+// ValidateBasic performs basic validation.
+func (m *bcNoBlockResponseMessage) ValidateBasic() error {
+	if m.Height < 0 {
+		return errors.New("Negative Height")
+	}
+	return nil
+}
+
 func (brm *bcNoBlockResponseMessage) String() string {
 	return fmt.Sprintf("[bcNoBlockResponseMessage %d]", brm.Height)
 }
@@ -395,6 +419,15 @@ type bcBlockResponseMessage struct {
 	Block *types.Block
 }
 
+// ValidateBasic performs basic validation.
+func (m *bcBlockResponseMessage) ValidateBasic() error {
+	if err := m.Block.ValidateBasic(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func (m *bcBlockResponseMessage) String() string {
 	return fmt.Sprintf("[bcBlockResponseMessage %v]", m.Block.Height)
 }
@@ -405,6 +438,14 @@ type bcStatusRequestMessage struct {
 	Height int64
 }
 
+// ValidateBasic performs basic validation.
+func (m *bcStatusRequestMessage) ValidateBasic() error {
+	if m.Height < 0 {
+		return errors.New("Negative Height")
+	}
+	return nil
+}
+
 func (m *bcStatusRequestMessage) String() string {
 	return fmt.Sprintf("[bcStatusRequestMessage %v]", m.Height)
 }
@@ -415,6 +456,14 @@ type bcStatusResponseMessage struct {
 	Height int64
 }
 
+// ValidateBasic performs basic validation.
+func (m *bcStatusResponseMessage) ValidateBasic() error {
+	if m.Height < 0 {
+		return errors.New("Negative Height")
+	}
+	return nil
+}
+
 func (m *bcStatusResponseMessage) String() string {
 	return fmt.Sprintf("[bcStatusResponseMessage %v]", m.Height)
 }

config/toml.go

@@ -342,7 +342,7 @@ func ResetTestRoot(testName string) *Config {
 }
 
 var testGenesis = `{
-  "genesis_time": "2017-10-10T08:20:13.695936996Z",
+  "genesis_time": "2018-10-10T08:20:13.695936996Z",
   "chain_id": "tendermint_test",
   "validators": [
     {

consensus/common_test.go

@@ -615,8 +615,6 @@ func randGenesisDoc(numValidators int, randPower bool, minPower int64) (*types.G
 func randGenesisState(numValidators int, randPower bool, minPower int64) (sm.State, []types.PrivValidator) {
 	genDoc, privValidators := randGenesisDoc(numValidators, randPower, minPower)
 	s0, _ := sm.MakeGenesisState(genDoc)
-	db := dbm.NewMemDB() // remove this ?
-	sm.SaveState(db, s0)
 	return s0, privValidators
 }
 

consensus/mempool_test.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/tendermint/tendermint/abci/example/code"
 	abci "github.com/tendermint/tendermint/abci/types"
-
 	"github.com/tendermint/tendermint/types"
 )
 

consensus/reactor.go

@@ -8,8 +8,7 @@ import (
 
 	"github.com/pkg/errors"
 
-	"github.com/tendermint/go-amino"
+	amino "github.com/tendermint/go-amino"
-
 	cstypes "github.com/tendermint/tendermint/consensus/types"
 	cmn "github.com/tendermint/tendermint/libs/common"
 	tmevents "github.com/tendermint/tendermint/libs/events"
@@ -205,6 +204,13 @@ func (conR *ConsensusReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte)
 		conR.Switch.StopPeerForError(src, err)
 		return
 	}
+
+	if err = msg.ValidateBasic(); err != nil {
+		conR.Logger.Error("Peer sent us invalid msg", "peer", src, "msg", msg, "err", err)
+		conR.Switch.StopPeerForError(src, err)
+		return
+	}
+
 	conR.Logger.Debug("Receive", "src", src, "chId", chID, "msg", msg)
 
 	// Get peer states
@@ -242,8 +248,7 @@ func (conR *ConsensusReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte)
 			case types.PrecommitType:
 				ourVotes = votes.Precommits(msg.Round).BitArrayByBlockID(msg.BlockID)
 			default:
-				conR.Logger.Error("Bad VoteSetBitsMessage field Type")
+				panic("Bad VoteSetBitsMessage field Type. Forgot to add a check in ValidateBasic?")
-				return
 			}
 			src.TrySend(VoteSetBitsChannel, cdc.MustMarshalBinaryBare(&VoteSetBitsMessage{
 				Height:  msg.Height,
@@ -322,8 +327,7 @@ func (conR *ConsensusReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte)
 				case types.PrecommitType:
 					ourVotes = votes.Precommits(msg.Round).BitArrayByBlockID(msg.BlockID)
 				default:
-					conR.Logger.Error("Bad VoteSetBitsMessage field Type")
+					panic("Bad VoteSetBitsMessage field Type. Forgot to add a check in ValidateBasic?")
-					return
 				}
 				ps.ApplyVoteSetBitsMessage(msg, ourVotes)
 			} else {
@@ -440,9 +444,9 @@ func (conR *ConsensusReactor) broadcastHasVoteMessage(vote *types.Vote) {
 
 func makeRoundStepMessage(rs *cstypes.RoundState) (nrsMsg *NewRoundStepMessage) {
 	nrsMsg = &NewRoundStepMessage{
-		Height:                rs.Height,
+		Height: rs.Height,
-		Round:                 rs.Round,
+		Round:  rs.Round,
-		Step:                  rs.Step,
+		Step:   rs.Step,
 		SecondsSinceStartTime: int(time.Since(rs.StartTime).Seconds()),
 		LastCommitRound:       rs.LastCommit.Round(),
 	}
@@ -1349,7 +1353,9 @@ func (ps *PeerState) StringIndented(indent string) string {
 // Messages
 
 // ConsensusMessage is a message that can be sent and received on the ConsensusReactor
-type ConsensusMessage interface{}
+type ConsensusMessage interface {
+	ValidateBasic() error
+}
 
 func RegisterConsensusMessages(cdc *amino.Codec) {
 	cdc.RegisterInterface((*ConsensusMessage)(nil), nil)
@@ -1385,6 +1391,27 @@ type NewRoundStepMessage struct {
 	LastCommitRound       int
 }
 
+// ValidateBasic performs basic validation.
+func (m *NewRoundStepMessage) ValidateBasic() error {
+	if m.Height < 0 {
+		return errors.New("Negative Height")
+	}
+	if m.Round < 0 {
+		return errors.New("Negative Round")
+	}
+	if !m.Step.IsValid() {
+		return errors.New("Invalid Step")
+	}
+
+	// NOTE: SecondsSinceStartTime may be negative
+
+	if (m.Height == 1 && m.LastCommitRound != -1) ||
+		(m.Height > 1 && m.LastCommitRound < -1) { // TODO: #2737 LastCommitRound should always be >= 0 for heights > 1
+		return errors.New("Invalid LastCommitRound (for 1st block: -1, for others: >= 0)")
+	}
+	return nil
+}
+
 // String returns a string representation.
 func (m *NewRoundStepMessage) String() string {
 	return fmt.Sprintf("[NewRoundStep H:%v R:%v S:%v LCR:%v]",
@@ -1404,6 +1431,25 @@ type NewValidBlockMessage struct {
 	IsCommit         bool
 }
 
+// ValidateBasic performs basic validation.
+func (m *NewValidBlockMessage) ValidateBasic() error {
+	if m.Height < 0 {
+		return errors.New("Negative Height")
+	}
+	if m.Round < 0 {
+		return errors.New("Negative Round")
+	}
+	if err := m.BlockPartsHeader.ValidateBasic(); err != nil {
+		return fmt.Errorf("Wrong BlockPartsHeader: %v", err)
+	}
+	if m.BlockParts.Size() != m.BlockPartsHeader.Total {
+		return fmt.Errorf("BlockParts bit array size %d not equal to BlockPartsHeader.Total %d",
+			m.BlockParts.Size(),
+			m.BlockPartsHeader.Total)
+	}
+	return nil
+}
+
 // String returns a string representation.
 func (m *NewValidBlockMessage) String() string {
 	return fmt.Sprintf("[ValidBlockMessage H:%v R:%v BP:%v BA:%v IsCommit:%v]",
@@ -1417,6 +1463,11 @@ type ProposalMessage struct {
 	Proposal *types.Proposal
 }
 
+// ValidateBasic performs basic validation.
+func (m *ProposalMessage) ValidateBasic() error {
+	return m.Proposal.ValidateBasic()
+}
+
 // String returns a string representation.
 func (m *ProposalMessage) String() string {
 	return fmt.Sprintf("[Proposal %v]", m.Proposal)
@@ -1431,6 +1482,20 @@ type ProposalPOLMessage struct {
 	ProposalPOL      *cmn.BitArray
 }
 
+// ValidateBasic performs basic validation.
+func (m *ProposalPOLMessage) ValidateBasic() error {
+	if m.Height < 0 {
+		return errors.New("Negative Height")
+	}
+	if m.ProposalPOLRound < 0 {
+		return errors.New("Negative ProposalPOLRound")
+	}
+	if m.ProposalPOL.Size() == 0 {
+		return errors.New("Empty ProposalPOL bit array")
+	}
+	return nil
+}
+
 // String returns a string representation.
 func (m *ProposalPOLMessage) String() string {
 	return fmt.Sprintf("[ProposalPOL H:%v POLR:%v POL:%v]", m.Height, m.ProposalPOLRound, m.ProposalPOL)
@@ -1445,6 +1510,20 @@ type BlockPartMessage struct {
 	Part   *types.Part
 }
 
+// ValidateBasic performs basic validation.
+func (m *BlockPartMessage) ValidateBasic() error {
+	if m.Height < 0 {
+		return errors.New("Negative Height")
+	}
+	if m.Round < 0 {
+		return errors.New("Negative Round")
+	}
+	if err := m.Part.ValidateBasic(); err != nil {
+		return fmt.Errorf("Wrong Part: %v", err)
+	}
+	return nil
+}
+
 // String returns a string representation.
 func (m *BlockPartMessage) String() string {
 	return fmt.Sprintf("[BlockPart H:%v R:%v P:%v]", m.Height, m.Round, m.Part)
@@ -1457,6 +1536,11 @@ type VoteMessage struct {
 	Vote *types.Vote
 }
 
+// ValidateBasic performs basic validation.
+func (m *VoteMessage) ValidateBasic() error {
+	return m.Vote.ValidateBasic()
+}
+
 // String returns a string representation.
 func (m *VoteMessage) String() string {
 	return fmt.Sprintf("[Vote %v]", m.Vote)
@@ -1472,6 +1556,23 @@ type HasVoteMessage struct {
 	Index  int
 }
 
+// ValidateBasic performs basic validation.
+func (m *HasVoteMessage) ValidateBasic() error {
+	if m.Height < 0 {
+		return errors.New("Negative Height")
+	}
+	if m.Round < 0 {
+		return errors.New("Negative Round")
+	}
+	if !types.IsVoteTypeValid(m.Type) {
+		return errors.New("Invalid Type")
+	}
+	if m.Index < 0 {
+		return errors.New("Negative Index")
+	}
+	return nil
+}
+
 // String returns a string representation.
 func (m *HasVoteMessage) String() string {
 	return fmt.Sprintf("[HasVote VI:%v V:{%v/%02d/%v}]", m.Index, m.Height, m.Round, m.Type)
@@ -1487,6 +1588,23 @@ type VoteSetMaj23Message struct {
 	BlockID types.BlockID
 }
 
+// ValidateBasic performs basic validation.
+func (m *VoteSetMaj23Message) ValidateBasic() error {
+	if m.Height < 0 {
+		return errors.New("Negative Height")
+	}
+	if m.Round < 0 {
+		return errors.New("Negative Round")
+	}
+	if !types.IsVoteTypeValid(m.Type) {
+		return errors.New("Invalid Type")
+	}
+	if err := m.BlockID.ValidateBasic(); err != nil {
+		return fmt.Errorf("Wrong BlockID: %v", err)
+	}
+	return nil
+}
+
 // String returns a string representation.
 func (m *VoteSetMaj23Message) String() string {
 	return fmt.Sprintf("[VSM23 %v/%02d/%v %v]", m.Height, m.Round, m.Type, m.BlockID)
@@ -1503,6 +1621,24 @@ type VoteSetBitsMessage struct {
 	Votes   *cmn.BitArray
 }
 
+// ValidateBasic performs basic validation.
+func (m *VoteSetBitsMessage) ValidateBasic() error {
+	if m.Height < 0 {
+		return errors.New("Negative Height")
+	}
+	if m.Round < 0 {
+		return errors.New("Negative Round")
+	}
+	if !types.IsVoteTypeValid(m.Type) {
+		return errors.New("Invalid Type")
+	}
+	if err := m.BlockID.ValidateBasic(); err != nil {
+		return fmt.Errorf("Wrong BlockID: %v", err)
+	}
+	// NOTE: Votes.Size() can be zero if the node does not have any
+	return nil
+}
+
 // String returns a string representation.
 func (m *VoteSetBitsMessage) String() string {
 	return fmt.Sprintf("[VSB %v/%02d/%v %v %v]", m.Height, m.Round, m.Type, m.BlockID, m.Votes)
@@ -1515,6 +1651,11 @@ type ProposalHeartbeatMessage struct {
 	Heartbeat *types.Heartbeat
 }
 
+// ValidateBasic performs basic validation.
+func (m *ProposalHeartbeatMessage) ValidateBasic() error {
+	return m.Heartbeat.ValidateBasic()
+}
+
 // String returns a string representation.
 func (m *ProposalHeartbeatMessage) String() string {
 	return fmt.Sprintf("[HEARTBEAT %v]", m.Heartbeat)

consensus/replay.go

@@ -264,8 +264,12 @@ func (h *Handshaker) Handshake(proxyApp proxy.AppConns) error {
 
 // Replay all blocks since appBlockHeight and ensure the result matches the current state.
 // Returns the final AppHash or an error.
-func (h *Handshaker) ReplayBlocks(state sm.State, appHash []byte, appBlockHeight int64, proxyApp proxy.AppConns) ([]byte, error) {
+func (h *Handshaker) ReplayBlocks(
-
+	state sm.State,
+	appHash []byte,
+	appBlockHeight int64,
+	proxyApp proxy.AppConns,
+) ([]byte, error) {
 	storeBlockHeight := h.store.Height()
 	stateBlockHeight := state.LastBlockHeight
 	h.logger.Info("ABCI Replay Blocks", "appHeight", appBlockHeight, "storeHeight", storeBlockHeight, "stateHeight", stateBlockHeight)

consensus/types/round_state.go

@@ -26,8 +26,15 @@ const (
 	RoundStepPrecommitWait = RoundStepType(0x07) // Did receive any +2/3 precommits, start timeout
 	RoundStepCommit        = RoundStepType(0x08) // Entered commit state machine
 	// NOTE: RoundStepNewHeight acts as RoundStepCommitWait.
+
+	// NOTE: Update IsValid method if you change this!
 )
 
+// IsValid returns true if the step is valid, false if unknown/undefined.
+func (rs RoundStepType) IsValid() bool {
+	return uint8(rs) >= 0x01 && uint8(rs) <= 0x08
+}
+
 // String returns a string
 func (rs RoundStepType) String() string {
 	switch rs {

crypto/crypto.go

@@ -6,6 +6,7 @@ import (
 )
 
 const (
+	// AddressSize is the size of a pubkey address.
 	AddressSize = tmhash.TruncatedSize
 )
 

evidence/reactor.go

@@ -74,6 +74,13 @@ func (evR *EvidenceReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte) {
 		evR.Switch.StopPeerForError(src, err)
 		return
 	}
+
+	if err = msg.ValidateBasic(); err != nil {
+		evR.Logger.Error("Peer sent us invalid msg", "peer", src, "msg", msg, "err", err)
+		evR.Switch.StopPeerForError(src, err)
+		return
+	}
+
 	evR.Logger.Debug("Receive", "src", src, "chId", chID, "msg", msg)
 
 	switch msg := msg.(type) {
@@ -191,7 +198,9 @@ type PeerState interface {
 // Messages
 
 // EvidenceMessage is a message sent or received by the EvidenceReactor.
-type EvidenceMessage interface{}
+type EvidenceMessage interface {
+	ValidateBasic() error
+}
 
 func RegisterEvidenceMessages(cdc *amino.Codec) {
 	cdc.RegisterInterface((*EvidenceMessage)(nil), nil)
@@ -209,11 +218,21 @@ func decodeMsg(bz []byte) (msg EvidenceMessage, err error) {
 
 //-------------------------------------
 
-// EvidenceMessage contains a list of evidence.
+// EvidenceListMessage contains a list of evidence.
 type EvidenceListMessage struct {
 	Evidence []types.Evidence
 }
 
+// ValidateBasic performs basic validation.
+func (m *EvidenceListMessage) ValidateBasic() error {
+	for i, ev := range m.Evidence {
+		if err := ev.ValidateBasic(); err != nil {
+			return fmt.Errorf("Invalid evidence (#%d): %v", i, err)
+		}
+	}
+	return nil
+}
+
 // String returns a string representation of the EvidenceListMessage.
 func (m *EvidenceListMessage) String() string {
 	return fmt.Sprintf("[EvidenceListMessage %v]", m.Evidence)

p2p/pex/addrbook.go

@@ -648,6 +648,10 @@ func (a *addrBook) addAddress(addr, src *p2p.NetAddress) error {
 		return ErrAddrBookNonRoutable{addr}
 	}
 
+	if !addr.Valid() {
+		return ErrAddrBookInvalidAddr{addr}
+	}
+
 	// TODO: we should track ourAddrs by ID and by IP:PORT and refuse both.
 	if _, ok := a.ourAddrs[addr.String()]; ok {
 		return ErrAddrBookSelf{addr}

p2p/pex/errors.go

@@ -46,3 +46,11 @@ type ErrAddrBookNilAddr struct {
 func (err ErrAddrBookNilAddr) Error() string {
 	return fmt.Sprintf("Cannot add a nil address. Got (addr, src) = (%v, %v)", err.Addr, err.Src)
 }
+
+type ErrAddrBookInvalidAddr struct {
+	Addr *p2p.NetAddress
+}
+
+func (err ErrAddrBookInvalidAddr) Error() string {
+	return fmt.Sprintf("Cannot add invalid address %v", err.Addr)
+}

p2p/pex/pex_reactor.go

@@ -288,21 +288,37 @@ func (r *PEXReactor) RequestAddrs(p Peer) {
 func (r *PEXReactor) ReceiveAddrs(addrs []*p2p.NetAddress, src Peer) error {
 	id := string(src.ID())
 	if !r.requestsSent.Has(id) {
-		return cmn.NewError("Received unsolicited pexAddrsMessage")
+		return errors.New("Unsolicited pexAddrsMessage")
 	}
 	r.requestsSent.Delete(id)
 
 	srcAddr := src.NodeInfo().NetAddress()
 	for _, netAddr := range addrs {
-		// NOTE: GetSelection methods should never return nil addrs
+		// Validate netAddr. Disconnect from a peer if it sends us invalid data.
 		if netAddr == nil {
-			return cmn.NewError("received nil addr")
+			return errors.New("nil address in pexAddrsMessage")
+		}
+		// TODO: extract validating logic from NewNetAddressStringWithOptionalID
+		// and put it in netAddr#Valid (#2722)
+		na, err := p2p.NewNetAddressString(netAddr.String())
+		if err != nil {
+			return fmt.Errorf("%s address in pexAddrsMessage is invalid: %v",
+				netAddr.String(),
+				err,
+			)
 		}
 
-		err := r.book.AddAddress(netAddr, srcAddr)
+		// NOTE: we check netAddr validity and routability in book#AddAddress.
-		r.logErrAddrBook(err)
+		err = r.book.AddAddress(na, srcAddr)
+		if err != nil {
+			r.logErrAddrBook(err)
+			// XXX: should we be strict about incoming data and disconnect from a
+			// peer here too?
+			continue
+		}
 
-		// If this address came from a seed node, try to connect to it without waiting.
+		// If this address came from a seed node, try to connect to it without
+		// waiting.
 		for _, seedAddr := range r.seedAddrs {
 			if seedAddr.Equals(srcAddr) {
 				r.ensurePeers()

state/validation.go

@@ -21,22 +21,19 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error {
 
 	// Validate basic info.
 	if block.Version != state.Version.Consensus {
-		return fmt.Errorf(
+		return fmt.Errorf("Wrong Block.Header.Version. Expected %v, got %v",
-			"Wrong Block.Header.Version. Expected %v, got %v",
 			state.Version.Consensus,
 			block.Version,
 		)
 	}
 	if block.ChainID != state.ChainID {
-		return fmt.Errorf(
+		return fmt.Errorf("Wrong Block.Header.ChainID. Expected %v, got %v",
-			"Wrong Block.Header.ChainID. Expected %v, got %v",
 			state.ChainID,
 			block.ChainID,
 		)
 	}
 	if block.Height != state.LastBlockHeight+1 {
-		return fmt.Errorf(
+		return fmt.Errorf("Wrong Block.Header.Height. Expected %v, got %v",
-			"Wrong Block.Header.Height. Expected %v, got %v",
 			state.LastBlockHeight+1,
 			block.Height,
 		)
@@ -44,16 +41,15 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error {
 
 	// Validate prev block info.
 	if !block.LastBlockID.Equals(state.LastBlockID) {
-		return fmt.Errorf(
+		return fmt.Errorf("Wrong Block.Header.LastBlockID.  Expected %v, got %v",
-			"Wrong Block.Header.LastBlockID.  Expected %v, got %v",
 			state.LastBlockID,
 			block.LastBlockID,
 		)
 	}
+
 	newTxs := int64(len(block.Data.Txs))
 	if block.TotalTxs != state.LastBlockTotalTx+newTxs {
-		return fmt.Errorf(
+		return fmt.Errorf("Wrong Block.Header.TotalTxs. Expected %v, got %v",
-			"Wrong Block.Header.TotalTxs. Expected %v, got %v",
 			state.LastBlockTotalTx+newTxs,
 			block.TotalTxs,
 		)
@@ -61,46 +57,44 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error {
 
 	// Validate app info
 	if !bytes.Equal(block.AppHash, state.AppHash) {
-		return fmt.Errorf(
+		return fmt.Errorf("Wrong Block.Header.AppHash.  Expected %X, got %v",
-			"Wrong Block.Header.AppHash.  Expected %X, got %v",
 			state.AppHash,
 			block.AppHash,
 		)
 	}
 	if !bytes.Equal(block.ConsensusHash, state.ConsensusParams.Hash()) {
-		return fmt.Errorf(
+		return fmt.Errorf("Wrong Block.Header.ConsensusHash.  Expected %X, got %v",
-			"Wrong Block.Header.ConsensusHash.  Expected %X, got %v",
 			state.ConsensusParams.Hash(),
 			block.ConsensusHash,
 		)
 	}
 	if !bytes.Equal(block.LastResultsHash, state.LastResultsHash) {
-		return fmt.Errorf(
+		return fmt.Errorf("Wrong Block.Header.LastResultsHash.  Expected %X, got %v",
-			"Wrong Block.Header.LastResultsHash.  Expected %X, got %v",
 			state.LastResultsHash,
 			block.LastResultsHash,
 		)
 	}
 	if !bytes.Equal(block.ValidatorsHash, state.Validators.Hash()) {
-		return fmt.Errorf(
+		return fmt.Errorf("Wrong Block.Header.ValidatorsHash.  Expected %X, got %v",
-			"Wrong Block.Header.ValidatorsHash.  Expected %X, got %v",
 			state.Validators.Hash(),
 			block.ValidatorsHash,
 		)
 	}
 	if !bytes.Equal(block.NextValidatorsHash, state.NextValidators.Hash()) {
-		return fmt.Errorf("Wrong Block.Header.NextValidatorsHash.  Expected %X, got %v", state.NextValidators.Hash(), block.NextValidatorsHash)
+		return fmt.Errorf("Wrong Block.Header.NextValidatorsHash.  Expected %X, got %v",
+			state.NextValidators.Hash(),
+			block.NextValidatorsHash,
+		)
 	}
 
 	// Validate block LastCommit.
 	if block.Height == 1 {
 		if len(block.LastCommit.Precommits) != 0 {
-			return errors.New("Block at height 1 (first block) should have no LastCommit precommits")
+			return errors.New("Block at height 1 can't have LastCommit precommits")
 		}
 	} else {
 		if len(block.LastCommit.Precommits) != state.LastValidators.Size() {
-			return fmt.Errorf(
+			return fmt.Errorf("Invalid block commit size. Expected %v, got %v",
-				"Invalid block commit size. Expected %v, got %v",
 				state.LastValidators.Size(),
 				len(block.LastCommit.Precommits),
 			)
@@ -115,8 +109,7 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error {
 	// Validate block Time
 	if block.Height > 1 {
 		if !block.Time.After(state.LastBlockTime) {
-			return fmt.Errorf(
+			return fmt.Errorf("Block time %v not greater than last block time %v",
-				"Block time %v not greater than last block time %v",
 				block.Time,
 				state.LastBlockTime,
 			)
@@ -124,8 +117,7 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error {
 
 		medianTime := MedianTime(block.LastCommit, state.LastValidators)
 		if !block.Time.Equal(medianTime) {
-			return fmt.Errorf(
+			return fmt.Errorf("Invalid block time. Expected %v, got %v",
-				"Invalid block time. Expected %v, got %v",
 				medianTime,
 				block.Time,
 			)
@@ -133,8 +125,7 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error {
 	} else if block.Height == 1 {
 		genesisTime := state.LastBlockTime
 		if !block.Time.Equal(genesisTime) {
-			return fmt.Errorf(
+			return fmt.Errorf("Block time %v is not equal to genesis time %v",
-				"Block time %v is not equal to genesis time %v",
 				block.Time,
 				genesisTime,
 			)
@@ -160,8 +151,7 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error {
 	// a legit address and a known validator.
 	if len(block.ProposerAddress) != crypto.AddressSize ||
 		!state.Validators.HasAddress(block.ProposerAddress) {
-		return fmt.Errorf(
+		return fmt.Errorf("Block.Header.ProposerAddress, %X, is not a validator",
-			"Block.Header.ProposerAddress, %X, is not a validator",
 			block.ProposerAddress,
 		)
 	}

test/docker/Dockerfile

@@ -14,6 +14,7 @@ ENV GOBIN $GOPATH/bin
 WORKDIR $REPO
 
 # Copy in the code
+# TODO: rewrite to only copy Makefile & other files?
 COPY . $REPO
 
 # Install the vendored dependencies
@@ -21,16 +22,18 @@ COPY . $REPO
 RUN make get_tools
 RUN make get_vendor_deps
 
-# Now copy in the code
-# NOTE: this will overwrite whatever is in vendor/
-COPY . $REPO
-
 # install ABCI CLI
 RUN make install_abci
 
 # install Tendermint
 RUN make install
 
+RUN tendermint testnet --node-dir-prefix="mach" --v=4 --populate-persistent-peers=false --o=$REPO/test/p2p/data
+
+# Now copy in the code
+# NOTE: this will overwrite whatever is in vendor/
+COPY . $REPO
+
 # expose the volume for debugging
 VOLUME $REPO
 

test/p2p/README.md

@@ -37,7 +37,7 @@ for i in $(seq 1 4); do
 	  --ip="172.57.0.$((100 + $i))" \
 	  --name local_testnet_$i \
 	  --entrypoint tendermint \
-	  -e TMHOME=/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$i/core \
+	  -e TMHOME=/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$((i-1)) \
 	  tendermint_tester node --p2p.persistent_peers 172.57.0.101:26656,172.57.0.102:26656,172.57.0.103:26656,172.57.0.104:26656 --proxy_app=kvstore
 done
 ```
@@ -49,6 +49,3 @@ We can confirm they are making blocks by checking the `/status` message using `c
 ```
 curl 172.57.0.101:26657/status | jq .
 ```
-
-
-

test/p2p/data/mach1/core/config/genesis.json

@@ -1,39 +0,0 @@
-{
-  "genesis_time": "2016-06-24T20:01:19.322Z",
-  "chain_id": "chain-9ujDWI",
-  "validators": [
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "vokz3/FgDAJuNHGPF4Wkzeq5DDVpizlOOLaUeukd4RY="
-      },
-      "power": "1",
-      "name": "mach1"
-    },
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "bcU0RlMjEmWH0qKpO1nWibcXBzsd6WiiWm7xPVlTGK0="
-      },
-      "power": "1",
-      "name": "mach2"
-    },
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "rmesaX0TWqC0YB6lfqqz/r9Lqk8inEWlmMKYWxL80aE="
-      },
-      "power": "1",
-      "name": "mach3"
-    },
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "nryPWM7UtG3NWrirpZHdJTzXy1A3Jz/aMrwLZGHE79k="
-      },
-      "power": "1",
-      "name": "mach4"
-    }
-  ],
-  "app_hash": ""
-}

test/p2p/data/mach1/core/config/node_key.json

@@ -1,6 +0,0 @@
-{
-  "priv_key": {
-    "type": "tendermint/PrivKeyEd25519",
-    "value": "BpYtFp8xSrudBa5aBLRuSPD72PGDAUm0dJORDL3Kd5YJbluUzRefVFrjwoHZv1yeDj2P9xkEi2L3hJCUz/qFkQ=="
-  }
-}

test/p2p/data/mach1/core/config/priv_validator.json

@@ -1,14 +0,0 @@
-{
-  "address": "AE47BBD4B3ACD80BFE17F6E0C66C5B8492A81AE4",
-  "pub_key": {
-    "type": "tendermint/PubKeyEd25519",
-    "value": "vokz3/FgDAJuNHGPF4Wkzeq5DDVpizlOOLaUeukd4RY="
-  },
-  "last_height": "0",
-  "last_round": "0",
-  "last_step": 0,
-  "priv_key": {
-    "type": "tendermint/PrivKeyEd25519",
-    "value": "VHqgfHqM4WxcsqQMbCbRWwoylgQQqfHqblC2NvGrOJq+iTPf8WAMAm40cY8XhaTN6rkMNWmLOU44tpR66R3hFg=="
-  }
-}

test/p2p/data/mach2/core/config/genesis.json

@@ -1,39 +0,0 @@
-{
-  "genesis_time": "2016-06-24T20:01:19.322Z",
-  "chain_id": "chain-9ujDWI",
-  "validators": [
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "vokz3/FgDAJuNHGPF4Wkzeq5DDVpizlOOLaUeukd4RY="
-      },
-      "power": "1",
-      "name": "mach1"
-    },
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "bcU0RlMjEmWH0qKpO1nWibcXBzsd6WiiWm7xPVlTGK0="
-      },
-      "power": "1",
-      "name": "mach2"
-    },
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "rmesaX0TWqC0YB6lfqqz/r9Lqk8inEWlmMKYWxL80aE="
-      },
-      "power": "1",
-      "name": "mach3"
-    },
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "nryPWM7UtG3NWrirpZHdJTzXy1A3Jz/aMrwLZGHE79k="
-      },
-      "power": "1",
-      "name": "mach4"
-    }
-  ],
-  "app_hash": ""
-}

test/p2p/data/mach2/core/config/node_key.json

@@ -1,6 +0,0 @@
-{
-  "priv_key": {
-    "type": "tendermint/PrivKeyEd25519",
-    "value": "uM6LDVE4wQIIUmq9rc6RxzX8zEGG4G4Jcuw15klzQopF68YfJM4bkbPSavurEcJ4nvBMusKBg2GcARFrZqnFKA=="
-  }
-}

test/p2p/data/mach2/core/config/priv_validator.json

@@ -1,14 +0,0 @@
-{
-  "address": "5D61EE46CCE91F579086522D7FD8CEC3F854E946",
-  "pub_key": {
-    "type": "tendermint/PubKeyEd25519",
-    "value": "bcU0RlMjEmWH0qKpO1nWibcXBzsd6WiiWm7xPVlTGK0="
-  },
-  "last_height": "0",
-  "last_round": "0",
-  "last_step": 0,
-  "priv_key": {
-    "type": "tendermint/PrivKeyEd25519",
-    "value": "0EeInmBQL8MSnQq38zSxg47Z7R7Nmcu5a3GtWr9agUNtxTRGUyMSZYfSoqk7WdaJtxcHOx3paKJabvE9WVMYrQ=="
-  }
-}

test/p2p/data/mach3/core/config/genesis.json

@@ -1,39 +0,0 @@
-{
-  "genesis_time": "2016-06-24T20:01:19.322Z",
-  "chain_id": "chain-9ujDWI",
-  "validators": [
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "vokz3/FgDAJuNHGPF4Wkzeq5DDVpizlOOLaUeukd4RY="
-      },
-      "power": "1",
-      "name": "mach1"
-    },
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "bcU0RlMjEmWH0qKpO1nWibcXBzsd6WiiWm7xPVlTGK0="
-      },
-      "power": "1",
-      "name": "mach2"
-    },
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "rmesaX0TWqC0YB6lfqqz/r9Lqk8inEWlmMKYWxL80aE="
-      },
-      "power": "1",
-      "name": "mach3"
-    },
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "nryPWM7UtG3NWrirpZHdJTzXy1A3Jz/aMrwLZGHE79k="
-      },
-      "power": "1",
-      "name": "mach4"
-    }
-  ],
-  "app_hash": ""
-}

test/p2p/data/mach3/core/config/node_key.json

@@ -1,6 +0,0 @@
-{
-  "priv_key": {
-    "type": "tendermint/PrivKeyEd25519",
-    "value": "kT3orG0YkipT9rAZbvAjtGk/7Pu1ZeCE8LSUF2jz2uiSs1rdlUVi/gccRlvCRLKvrtSicOyEkmk0FHPOGS3mgg=="
-  }
-}

test/p2p/data/mach3/core/config/priv_validator.json

@@ -1,14 +0,0 @@
-{
-  "address": "705F9DA2CC7D7AF5F4519455ED99622E40E439A1",
-  "pub_key": {
-    "type": "tendermint/PubKeyEd25519",
-    "value": "rmesaX0TWqC0YB6lfqqz/r9Lqk8inEWlmMKYWxL80aE="
-  },
-  "last_height": "0",
-  "last_round": "0",
-  "last_step": 0,
-  "priv_key": {
-    "type": "tendermint/PrivKeyEd25519",
-    "value": "waTkfzSfxfVW9Kmie6d2uUQkwxK6ps9u5EuGc0jXw/KuZ6xpfRNaoLRgHqV+qrP+v0uqTyKcRaWYwphbEvzRoQ=="
-  }
-}

test/p2p/data/mach4/core/config/genesis.json

@@ -1,39 +0,0 @@
-{
-  "genesis_time": "2016-06-24T20:01:19.322Z",
-  "chain_id": "chain-9ujDWI",
-  "validators": [
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "vokz3/FgDAJuNHGPF4Wkzeq5DDVpizlOOLaUeukd4RY="
-      },
-      "power": "1",
-      "name": "mach1"
-    },
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "bcU0RlMjEmWH0qKpO1nWibcXBzsd6WiiWm7xPVlTGK0="
-      },
-      "power": "1",
-      "name": "mach2"
-    },
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "rmesaX0TWqC0YB6lfqqz/r9Lqk8inEWlmMKYWxL80aE="
-      },
-      "power": "1",
-      "name": "mach3"
-    },
-    {
-      "pub_key": {
-        "type": "tendermint/PubKeyEd25519",
-        "value": "nryPWM7UtG3NWrirpZHdJTzXy1A3Jz/aMrwLZGHE79k="
-      },
-      "power": "1",
-      "name": "mach4"
-    }
-  ],
-  "app_hash": ""
-}

test/p2p/data/mach4/core/config/node_key.json

@@ -1,6 +0,0 @@
-{
-  "priv_key": {
-    "type": "tendermint/PrivKeyEd25519",
-    "value": "QIIm8/QEEawiJi3Zozv+J9b+1CufCEkGs3lxGMlRy4L4FVIXCoXJTwYIrotZtwoMqLYEqQV1hbKKJmFA3GFelw=="
-  }
-}

test/p2p/data/mach4/core/config/priv_validator.json

@@ -1,14 +0,0 @@
-{
-  "address": "D1054266EC9EEA511ED9A76DEFD520BBE1B5E850",
-  "pub_key": {
-    "type": "tendermint/PubKeyEd25519",
-    "value": "nryPWM7UtG3NWrirpZHdJTzXy1A3Jz/aMrwLZGHE79k="
-  },
-  "last_height": "0",
-  "last_round": "0",
-  "last_step": 0,
-  "priv_key": {
-    "type": "tendermint/PrivKeyEd25519",
-    "value": "xMw+0o8CDC29qYvNvwjDztNwRw508l6TjV0pXo49KwyevI9YztS0bc1auKulkd0lPNfLUDcnP9oyvAtkYcTv2Q=="
-  }
-}

test/p2p/ip_plus_id.sh

@@ -3,5 +3,5 @@ set -eu
 
 ID=$1
 DOCKER_IMAGE=$2
-NODEID="$(docker run --rm -e TMHOME=/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$ID/core $DOCKER_IMAGE tendermint show_node_id)"
+NODEID="$(docker run --rm -e TMHOME=/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$((ID-1)) $DOCKER_IMAGE tendermint show_node_id)"
 echo "$NODEID@172.57.0.$((100+$ID))"

test/p2p/peer.sh

@@ -15,13 +15,15 @@ echo "starting tendermint peer ID=$ID"
 # NOTE: $NODE_FLAGS should be unescaped (no quotes). otherwise it will be
 # treated as one flag.
 
+# test/p2p/data/mach$((ID-1)) data is generated in test/docker/Dockerfile using
+# the tendermint testnet command.
 if [[ "$ID" == "x" ]]; then # Set "x" to "1" to print to console.
 	docker run \
 		--net="$NETWORK_NAME" \
 		--ip=$(test/p2p/ip.sh "$ID") \
 		--name "local_testnet_$ID" \
 		--entrypoint tendermint \
-		-e TMHOME="/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$ID/core" \
+		-e TMHOME="/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$((ID-1))" \
 		-e GOMAXPROCS=1 \
 		--log-driver=syslog \
 		--log-opt syslog-address=udp://127.0.0.1:5514 \
@@ -34,7 +36,7 @@ else
 		--ip=$(test/p2p/ip.sh "$ID") \
 		--name "local_testnet_$ID" \
 		--entrypoint tendermint \
-		-e TMHOME="/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$ID/core" \
+		-e TMHOME="/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$((ID-1))" \
 		-e GOMAXPROCS=1 \
 		--log-driver=syslog \
 		--log-opt syslog-address=udp://127.0.0.1:5514 \

test/p2p/pex/test_addrbook.sh

@@ -18,7 +18,7 @@ echo "1. restart peer $ID"
 docker stop "local_testnet_$ID"
 echo "stopped local_testnet_$ID"
 # preserve addrbook.json
-docker cp "local_testnet_$ID:/go/src/github.com/tendermint/tendermint/test/p2p/data/mach1/core/config/addrbook.json" "/tmp/addrbook.json"
+docker cp "local_testnet_$ID:/go/src/github.com/tendermint/tendermint/test/p2p/data/mach0/config/addrbook.json" "/tmp/addrbook.json"
 set +e #CIRCLE
 docker rm -vf "local_testnet_$ID"
 set -e
@@ -32,11 +32,11 @@ bash test/p2p/client.sh "$DOCKER_IMAGE" "$NETWORK_NAME" "$CLIENT_NAME" "test/p2p
 
 # Now we know that the node is up.
 
-docker cp "/tmp/addrbook.json" "local_testnet_$ID:/go/src/github.com/tendermint/tendermint/test/p2p/data/mach1/core/config/addrbook.json"
+docker cp "/tmp/addrbook.json" "local_testnet_$ID:/go/src/github.com/tendermint/tendermint/test/p2p/data/mach0/config/addrbook.json"
 echo "with the following addrbook:"
 cat /tmp/addrbook.json
 # exec doesn't work on circle
-# docker exec "local_testnet_$ID" cat "/go/src/github.com/tendermint/tendermint/test/p2p/data/mach1/core/config/addrbook.json"
+# docker exec "local_testnet_$ID" cat "/go/src/github.com/tendermint/tendermint/test/p2p/data/mach0/config/addrbook.json"
 echo ""
 
 echo "----------------------------------------------------------------------"

types/block.go

@@ -2,12 +2,14 @@ package types
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
 	"strings"
 	"sync"
 	"time"
 
+	"github.com/pkg/errors"
+
+	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/crypto/merkle"
 	cmn "github.com/tendermint/tendermint/libs/common"
 	"github.com/tendermint/tendermint/version"
@@ -57,54 +59,117 @@ func MakeBlock(height int64, txs []Tx, lastCommit *Commit, evidence []Evidence)
 
 // ValidateBasic performs basic validation that doesn't involve state data.
 // It checks the internal consistency of the block.
+// Further validation is done using state#ValidateBlock.
 func (b *Block) ValidateBasic() error {
 	if b == nil {
-		return errors.New("Nil blocks are invalid")
+		return errors.New("nil block")
 	}
 	b.mtx.Lock()
 	defer b.mtx.Unlock()
 
-	if b.Height < 0 {
+	if len(b.ChainID) > MaxChainIDLen {
-		return fmt.Errorf(
+		return fmt.Errorf("ChainID is too long. Max is %d, got %d", MaxChainIDLen, len(b.ChainID))
-			"Negative Block.Header.Height: %v",
-			b.Height,
-		)
 	}
 
+	if b.Height < 0 {
+		return errors.New("Negative Header.Height")
+	} else if b.Height == 0 {
+		return errors.New("Zero Header.Height")
+	}
+
+	// NOTE: Timestamp validation is subtle and handled elsewhere.
+
 	newTxs := int64(len(b.Data.Txs))
 	if b.NumTxs != newTxs {
-		return fmt.Errorf(
+		return fmt.Errorf("Wrong Header.NumTxs. Expected %v, got %v",
-			"Wrong Block.Header.NumTxs. Expected %v, got %v",
 			newTxs,
 			b.NumTxs,
 		)
 	}
+
+	// TODO: fix tests so we can do this
+	/*if b.TotalTxs < b.NumTxs {
+		return fmt.Errorf("Header.TotalTxs (%d) is less than Header.NumTxs (%d)", b.TotalTxs, b.NumTxs)
+	}*/
+	if b.TotalTxs < 0 {
+		return errors.New("Negative Header.TotalTxs")
+	}
+
+	if err := b.LastBlockID.ValidateBasic(); err != nil {
+		return fmt.Errorf("Wrong Header.LastBlockID: %v", err)
+	}
+
+	// Validate the last commit and its hash.
+	if b.Header.Height > 1 {
+		if b.LastCommit == nil {
+			return errors.New("nil LastCommit")
+		}
+		if err := b.LastCommit.ValidateBasic(); err != nil {
+			return fmt.Errorf("Wrong LastCommit")
+		}
+	}
+	if err := ValidateHash(b.LastCommitHash); err != nil {
+		return fmt.Errorf("Wrong Header.LastCommitHash: %v", err)
+	}
 	if !bytes.Equal(b.LastCommitHash, b.LastCommit.Hash()) {
-		return fmt.Errorf(
+		return fmt.Errorf("Wrong Header.LastCommitHash. Expected %v, got %v",
-			"Wrong Block.Header.LastCommitHash.  Expected %v, got %v",
-			b.LastCommitHash,
 			b.LastCommit.Hash(),
+			b.LastCommitHash,
 		)
 	}
-	if b.Header.Height != 1 {
+
-		if err := b.LastCommit.ValidateBasic(); err != nil {
+	// Validate the hash of the transactions.
-			return err
+	// NOTE: b.Data.Txs may be nil, but b.Data.Hash()
-		}
+	// still works fine
+	if err := ValidateHash(b.DataHash); err != nil {
+		return fmt.Errorf("Wrong Header.DataHash: %v", err)
 	}
 	if !bytes.Equal(b.DataHash, b.Data.Hash()) {
 		return fmt.Errorf(
-			"Wrong Block.Header.DataHash.  Expected %v, got %v",
+			"Wrong Header.DataHash. Expected %v, got %v",
-			b.DataHash,
 			b.Data.Hash(),
+			b.DataHash,
 		)
 	}
+
+	// Basic validation of hashes related to application data.
+	// Will validate fully against state in state#ValidateBlock.
+	if err := ValidateHash(b.ValidatorsHash); err != nil {
+		return fmt.Errorf("Wrong Header.ValidatorsHash: %v", err)
+	}
+	if err := ValidateHash(b.NextValidatorsHash); err != nil {
+		return fmt.Errorf("Wrong Header.NextValidatorsHash: %v", err)
+	}
+	if err := ValidateHash(b.ConsensusHash); err != nil {
+		return fmt.Errorf("Wrong Header.ConsensusHash: %v", err)
+	}
+	// NOTE: AppHash is arbitrary length
+	if err := ValidateHash(b.LastResultsHash); err != nil {
+		return fmt.Errorf("Wrong Header.LastResultsHash: %v", err)
+	}
+
+	// Validate evidence and its hash.
+	if err := ValidateHash(b.EvidenceHash); err != nil {
+		return fmt.Errorf("Wrong Header.EvidenceHash: %v", err)
+	}
+	// NOTE: b.Evidence.Evidence may be nil, but we're just looping.
+	for i, ev := range b.Evidence.Evidence {
+		if err := ev.ValidateBasic(); err != nil {
+			return fmt.Errorf("Invalid evidence (#%d): %v", i, err)
+		}
+	}
 	if !bytes.Equal(b.EvidenceHash, b.Evidence.Hash()) {
-		return fmt.Errorf(
+		return fmt.Errorf("Wrong Header.EvidenceHash. Expected %v, got %v",
-			"Wrong Block.Header.EvidenceHash.  Expected %v, got %v",
 			b.EvidenceHash,
 			b.Evidence.Hash(),
 		)
 	}
+
+	if len(b.ProposerAddress) != crypto.AddressSize {
+		return fmt.Errorf("Expected len(Header.ProposerAddress) to be %d, got %d",
+			crypto.AddressSize, len(b.ProposerAddress))
+	}
+
 	return nil
 }
 
@@ -719,6 +784,18 @@ func (blockID BlockID) Key() string {
 	return string(blockID.Hash) + string(bz)
 }
 
+// ValidateBasic performs basic validation.
+func (blockID BlockID) ValidateBasic() error {
+	// Hash can be empty in case of POLBlockID in Proposal.
+	if err := ValidateHash(blockID.Hash); err != nil {
+		return fmt.Errorf("Wrong Hash")
+	}
+	if err := blockID.PartsHeader.ValidateBasic(); err != nil {
+		return fmt.Errorf("Wrong PartsHeader: %v", err)
+	}
+	return nil
+}
+
 // String returns a human readable string representation of the BlockID
 func (blockID BlockID) String() string {
 	return fmt.Sprintf(`%v:%v`, blockID.Hash, blockID.PartsHeader)

types/block_test.go

@@ -80,11 +80,13 @@ func TestBlockValidateBasic(t *testing.T) {
 			blk.EvidenceHash = []byte("something else")
 		}, true},
 	}
-	for _, tc := range testCases {
+	for i, tc := range testCases {
 		t.Run(tc.testName, func(t *testing.T) {
 			block := MakeBlock(h, txs, commit, evList)
+			block.ProposerAddress = valSet.GetProposer().Address
 			tc.malleateBlock(block)
-			assert.Equal(t, tc.expErr, block.ValidateBasic() != nil, "ValidateBasic had an unexpected result")
+			err = block.ValidateBasic()
+			assert.Equal(t, tc.expErr, err != nil, "#%d: %v", i, err)
 		})
 	}
 }

types/evidence.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 
+	"github.com/pkg/errors"
 	"github.com/tendermint/tendermint/crypto/tmhash"
 
 	amino "github.com/tendermint/go-amino"
@@ -60,6 +61,7 @@ type Evidence interface {
 	Verify(chainID string, pubKey crypto.PubKey) error // verify the evidence
 	Equal(Evidence) bool                               // check equality of evidence
 
+	ValidateBasic() error
 	String() string
 }
 
@@ -172,6 +174,23 @@ func (dve *DuplicateVoteEvidence) Equal(ev Evidence) bool {
 	return bytes.Equal(dveHash, evHash)
 }
 
+// ValidateBasic performs basic validation.
+func (dve *DuplicateVoteEvidence) ValidateBasic() error {
+	if len(dve.PubKey.Bytes()) == 0 {
+		return errors.New("Empty PubKey")
+	}
+	if dve.VoteA == nil || dve.VoteB == nil {
+		return fmt.Errorf("One or both of the votes are empty %v, %v", dve.VoteA, dve.VoteB)
+	}
+	if err := dve.VoteA.ValidateBasic(); err != nil {
+		return fmt.Errorf("Invalid VoteA: %v", err)
+	}
+	if err := dve.VoteB.ValidateBasic(); err != nil {
+		return fmt.Errorf("Invalid VoteB: %v", err)
+	}
+	return nil
+}
+
 //-----------------------------------------------------------------
 
 // UNSTABLE
@@ -201,6 +220,7 @@ func (e MockGoodEvidence) Equal(ev Evidence) bool {
 	return e.Height_ == e2.Height_ &&
 		bytes.Equal(e.Address_, e2.Address_)
 }
+func (e MockGoodEvidence) ValidateBasic() error { return nil }
 func (e MockGoodEvidence) String() string {
 	return fmt.Sprintf("GoodEvidence: %d/%s", e.Height_, e.Address_)
 }
@@ -218,6 +238,7 @@ func (e MockBadEvidence) Equal(ev Evidence) bool {
 	return e.Height_ == e2.Height_ &&
 		bytes.Equal(e.Address_, e2.Address_)
 }
+func (e MockBadEvidence) ValidateBasic() error { return nil }
 func (e MockBadEvidence) String() string {
 	return fmt.Sprintf("BadEvidence: %d/%s", e.Height_, e.Address_)
 }

types/heartbeat.go

@@ -3,6 +3,8 @@ package types
 import (
 	"fmt"
 
+	"github.com/pkg/errors"
+	"github.com/tendermint/tendermint/crypto"
 	cmn "github.com/tendermint/tendermint/libs/common"
 )
 
@@ -50,3 +52,32 @@ func (heartbeat *Heartbeat) String() string {
 		heartbeat.Height, heartbeat.Round, heartbeat.Sequence,
 		fmt.Sprintf("/%X.../", cmn.Fingerprint(heartbeat.Signature[:])))
 }
+
+// ValidateBasic performs basic validation.
+func (heartbeat *Heartbeat) ValidateBasic() error {
+	if len(heartbeat.ValidatorAddress) != crypto.AddressSize {
+		return fmt.Errorf("Expected ValidatorAddress size to be %d bytes, got %d bytes",
+			crypto.AddressSize,
+			len(heartbeat.ValidatorAddress),
+		)
+	}
+	if heartbeat.ValidatorIndex < 0 {
+		return errors.New("Negative ValidatorIndex")
+	}
+	if heartbeat.Height < 0 {
+		return errors.New("Negative Height")
+	}
+	if heartbeat.Round < 0 {
+		return errors.New("Negative Round")
+	}
+	if heartbeat.Sequence < 0 {
+		return errors.New("Negative Sequence")
+	}
+	if len(heartbeat.Signature) == 0 {
+		return errors.New("Signature is missing")
+	}
+	if len(heartbeat.Signature) > MaxSignatureSize {
+		return fmt.Errorf("Signature is too big (max: %d)", MaxSignatureSize)
+	}
+	return nil
+}

types/part_set.go

@@ -2,11 +2,12 @@ package types
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
 	"io"
 	"sync"
 
+	"github.com/pkg/errors"
+
 	"github.com/tendermint/tendermint/crypto/merkle"
 	"github.com/tendermint/tendermint/crypto/tmhash"
 	cmn "github.com/tendermint/tendermint/libs/common"
@@ -36,6 +37,17 @@ func (part *Part) Hash() []byte {
 	return part.hash
 }
 
+// ValidateBasic performs basic validation.
+func (part *Part) ValidateBasic() error {
+	if part.Index < 0 {
+		return errors.New("Negative Index")
+	}
+	if len(part.Bytes) > BlockPartSizeBytes {
+		return fmt.Errorf("Too big (max: %d)", BlockPartSizeBytes)
+	}
+	return nil
+}
+
 func (part *Part) String() string {
 	return part.StringIndented("")
 }
@@ -70,6 +82,18 @@ func (psh PartSetHeader) Equals(other PartSetHeader) bool {
 	return psh.Total == other.Total && bytes.Equal(psh.Hash, other.Hash)
 }
 
+// ValidateBasic performs basic validation.
+func (psh PartSetHeader) ValidateBasic() error {
+	if psh.Total < 0 {
+		return errors.New("Negative Total")
+	}
+	// Hash can be empty in case of POLBlockID.PartsHeader in Proposal.
+	if err := ValidateHash(psh.Hash); err != nil {
+		return errors.Wrap(err, "Wrong Hash")
+	}
+	return nil
+}
+
 //-------------------------------------
 
 type PartSet struct {

types/proposal.go

@@ -43,6 +43,35 @@ func NewProposal(height int64, round int, polRound int, blockID BlockID) *Propos
 	}
 }
 
+// ValidateBasic performs basic validation.
+func (p *Proposal) ValidateBasic() error {
+	if p.Type != ProposalType {
+		return errors.New("Invalid Type")
+	}
+	if p.Height < 0 {
+		return errors.New("Negative Height")
+	}
+	if p.Round < 0 {
+		return errors.New("Negative Round")
+	}
+	if p.POLRound < -1 {
+		return errors.New("Negative POLRound (exception: -1)")
+	}
+	if err := p.BlockID.ValidateBasic(); err != nil {
+		return fmt.Errorf("Wrong BlockID: %v", err)
+	}
+
+	// NOTE: Timestamp validation is subtle and handled elsewhere.
+
+	if len(p.Signature) == 0 {
+		return errors.New("Signature is missing")
+	}
+	if len(p.Signature) > MaxSignatureSize {
+		return fmt.Errorf("Signature is too big (max: %d)", MaxSignatureSize)
+	}
+	return nil
+}
+
 // String returns a string representation of the Proposal.
 func (p *Proposal) String() string {
 	return fmt.Sprintf("Proposal{%v/%v (%v, %v) %X @ %s}",

types/signable.go

@@ -1,5 +1,17 @@
 package types
 
+import (
+	"github.com/tendermint/tendermint/crypto/ed25519"
+	cmn "github.com/tendermint/tendermint/libs/common"
+)
+
+var (
+	// MaxSignatureSize is a maximum allowed signature size for the Heartbeat,
+	// Proposal and Vote.
+	// XXX: secp256k1 does not have Size nor MaxSize defined.
+	MaxSignatureSize = cmn.MaxInt(ed25519.SignatureSize, 64)
+)
+
 // Signable is an interface for all signable things.
 // It typically removes signatures before serializing.
 // SignBytes returns the bytes to be signed

types/signed_msg_type.go

@@ -15,11 +15,10 @@ const (
 	HeartbeatType SignedMsgType = 0x30
 )
 
-func IsVoteTypeValid(type_ SignedMsgType) bool {
+// IsVoteTypeValid returns true if t is a valid vote type.
-	switch type_ {
+func IsVoteTypeValid(t SignedMsgType) bool {
-	case PrevoteType:
+	switch t {
-		return true
+	case PrevoteType, PrecommitType:
-	case PrecommitType:
 		return true
 	default:
 		return false

types/validation.go

@@ -0,0 +1,40 @@
+package types
+
+import (
+	"fmt"
+	"time"
+
+	"github.com/tendermint/tendermint/crypto/tmhash"
+	tmtime "github.com/tendermint/tendermint/types/time"
+)
+
+// ValidateTime does a basic time validation ensuring time does not drift too
+// much: +/- one year.
+// TODO: reduce this to eg 1 day
+// NOTE: DO NOT USE in ValidateBasic methods in this package. This function
+// can only be used for real time validation, like on proposals and votes
+// in the consensus. If consensus is stuck, and rounds increase for more than a day,
+// having only a 1-day band here could break things...
+// Can't use for validating blocks because we may be syncing years worth of history.
+func ValidateTime(t time.Time) error {
+	var (
+		now     = tmtime.Now()
+		oneYear = 8766 * time.Hour
+	)
+	if t.Before(now.Add(-oneYear)) || t.After(now.Add(oneYear)) {
+		return fmt.Errorf("Time drifted too much. Expected: -1 < %v < 1 year", now)
+	}
+	return nil
+}
+
+// ValidateHash returns an error if the hash is not empty, but its
+// size != tmhash.Size.
+func ValidateHash(h []byte) error {
+	if len(h) > 0 && len(h) != tmhash.Size {
+		return fmt.Errorf("Expected size to be %d bytes, got %d bytes",
+			tmhash.Size,
+			len(h),
+		)
+	}
+	return nil
+}

types/vote.go

@@ -46,7 +46,8 @@ func NewConflictingVoteError(val *Validator, voteA, voteB *Vote) *ErrVoteConflic
 // Address is hex bytes.
 type Address = crypto.Address
 
-// Represents a prevote, precommit, or commit vote from validators for consensus.
+// Vote represents a prevote, precommit, or commit vote from validators for
+// consensus.
 type Vote struct {
 	Type             SignedMsgType `json:"type"`
 	Height           int64         `json:"height"`
@@ -108,3 +109,38 @@ func (vote *Vote) Verify(chainID string, pubKey crypto.PubKey) error {
 	}
 	return nil
 }
+
+// ValidateBasic performs basic validation.
+func (vote *Vote) ValidateBasic() error {
+	if !IsVoteTypeValid(vote.Type) {
+		return errors.New("Invalid Type")
+	}
+	if vote.Height < 0 {
+		return errors.New("Negative Height")
+	}
+	if vote.Round < 0 {
+		return errors.New("Negative Round")
+	}
+
+	// NOTE: Timestamp validation is subtle and handled elsewhere.
+
+	if err := vote.BlockID.ValidateBasic(); err != nil {
+		return fmt.Errorf("Wrong BlockID: %v", err)
+	}
+	if len(vote.ValidatorAddress) != crypto.AddressSize {
+		return fmt.Errorf("Expected ValidatorAddress size to be %d bytes, got %d bytes",
+			crypto.AddressSize,
+			len(vote.ValidatorAddress),
+		)
+	}
+	if vote.ValidatorIndex < 0 {
+		return errors.New("Negative ValidatorIndex")
+	}
+	if len(vote.Signature) == 0 {
+		return errors.New("Signature is missing")
+	}
+	if len(vote.Signature) > MaxSignatureSize {
+		return fmt.Errorf("Signature is too big (max: %d)", MaxSignatureSize)
+	}
+	return nil
+}