types: cap evidence in block validation (#2560)

* cap evidence in block validation

* state: use table-driven test for ValidateBlockHeader

* state: test evidence cap

* fixes from review

CHANGELOG_PENDING.md

@@ -49,6 +49,8 @@ timeoutPrecommit before starting next round
 - [evidence] \#2515 fix db iter leak (@goolAdapter)
 - [common/bit_array] Fixed a bug in the `Or` function
 - [common/bit_array] Fixed a bug in the `Sub` function (@bradyjoestar)
-- [common] \#2534 make bit array's PickRandom choose uniformly from true bits
+- [common] \#2534 Make bit array's PickRandom choose uniformly from true bits
+- [consensus] \#1637 Limit the amount of evidence that can be included in a
+  block
 - [p2p] \#2555 fix p2p switch FlushThrottle value (@goolAdapter)
 - [libs/event] \#2518 fix event concurrency flaw (@goolAdapter)

state/validation.go

@@ -125,13 +125,17 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error {
 		}
 	}
 
+	// Limit the amount of evidence
+	maxEvidenceBytes := types.MaxEvidenceBytesPerBlock(state.ConsensusParams.BlockSize.MaxBytes)
+	evidenceBytes := int64(len(block.Evidence.Evidence)) * types.MaxEvidenceBytes
+	if evidenceBytes > maxEvidenceBytes {
+		return types.NewErrEvidenceOverflow(maxEvidenceBytes, evidenceBytes)
+	}
+
 	// Validate all evidence.
-	// TODO: Each check requires loading an old validator set.
-	// We should cap the amount of evidence per block
-	// to prevent potential proposer DoS.
 	for _, ev := range block.Evidence.Evidence {
 		if err := VerifyEvidence(stateDB, state, ev); err != nil {
-			return types.NewEvidenceInvalidErr(ev, err)
+			return types.NewErrEvidenceInvalid(ev, err)
 		}
 	}
 

state/validation_test.go

@@ -5,74 +5,119 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"github.com/tendermint/tendermint/crypto/ed25519"
-	dbm "github.com/tendermint/tendermint/libs/db"
+	"github.com/tendermint/tendermint/crypto/tmhash"
 	"github.com/tendermint/tendermint/libs/log"
+	"github.com/tendermint/tendermint/types"
 )
 
-func TestValidateBlock(t *testing.T) {
+// TODO(#2589):
-	state, _ := state(1, 1)
+// - generalize this past the first height
+// - add txs and build up full State properly
+// - test block.Time (see #2587 - there are no conditions on time for the first height)
+func TestValidateBlockHeader(t *testing.T) {
+	var height int64 = 1 // TODO(#2589): generalize
+	state, stateDB := state(1, int(height))
 
-	blockExec := NewBlockExecutor(dbm.NewMemDB(), log.TestingLogger(), nil, nil, nil)
+	blockExec := NewBlockExecutor(stateDB, log.TestingLogger(), nil, nil, nil)
 
-	// proper block must pass
+	// A good block passes.
-	block := makeBlock(state, 1)
+	block := makeBlock(state, height)
 	err := blockExec.ValidateBlock(state, block)
 	require.NoError(t, err)
 
-	// wrong chain fails
+	wrongHash := tmhash.Sum([]byte("this hash is wrong"))
-	block = makeBlock(state, 1)
-	block.ChainID = "not-the-real-one"
-	err = blockExec.ValidateBlock(state, block)
-	require.Error(t, err)
 
-	// wrong height fails
+	// Manipulation of any header field causes failure.
-	block = makeBlock(state, 1)
+	testCases := []struct {
-	block.Height += 10
+		name          string
-	err = blockExec.ValidateBlock(state, block)
+		malleateBlock func(block *types.Block)
-	require.Error(t, err)
+	}{
+		{"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
 
-	// wrong total tx fails
+		{"LastBlockID wrong", func(block *types.Block) { block.LastBlockID.PartsHeader.Total += 10 }},
-	block = makeBlock(state, 1)
+		{"LastCommitHash wrong", func(block *types.Block) { block.LastCommitHash = wrongHash }},
-	block.TotalTxs += 10
+		{"DataHash wrong", func(block *types.Block) { block.DataHash = wrongHash }},
-	err = blockExec.ValidateBlock(state, block)
-	require.Error(t, err)
 
-	// wrong blockid fails
+		{"ValidatorsHash wrong", func(block *types.Block) { block.ValidatorsHash = wrongHash }},
-	block = makeBlock(state, 1)
+		{"NextValidatorsHash wrong", func(block *types.Block) { block.NextValidatorsHash = wrongHash }},
-	block.LastBlockID.PartsHeader.Total += 10
+		{"ConsensusHash wrong", func(block *types.Block) { block.ConsensusHash = wrongHash }},
-	err = blockExec.ValidateBlock(state, block)
+		{"AppHash wrong", func(block *types.Block) { block.AppHash = wrongHash }},
-	require.Error(t, err)
+		{"LastResultsHash wrong", func(block *types.Block) { block.LastResultsHash = wrongHash }},
 
-	// wrong app hash fails
+		{"EvidenceHash wrong", func(block *types.Block) { block.EvidenceHash = wrongHash }},
-	block = makeBlock(state, 1)
+		{"Proposer wrong", func(block *types.Block) { block.ProposerAddress = ed25519.GenPrivKey().PubKey().Address() }},
-	block.AppHash = []byte("wrong app hash")
+		{"Proposer invalid", func(block *types.Block) { block.ProposerAddress = []byte("wrong size") }},
-	err = blockExec.ValidateBlock(state, block)
+	}
-	require.Error(t, err)
 
-	// wrong consensus hash fails
+	for _, tc := range testCases {
-	block = makeBlock(state, 1)
+		block := makeBlock(state, height)
-	block.ConsensusHash = []byte("wrong consensus hash")
+		tc.malleateBlock(block)
-	err = blockExec.ValidateBlock(state, block)
+		err := blockExec.ValidateBlock(state, block)
-	require.Error(t, err)
+		require.Error(t, err, tc.name)
-
+	}
-	// wrong results hash fails
+}
-	block = makeBlock(state, 1)
+
-	block.LastResultsHash = []byte("wrong results hash")
+/*
-	err = blockExec.ValidateBlock(state, block)
+	TODO(#2589):
-	require.Error(t, err)
+	- test Block.Data.Hash() == Block.DataHash
-
+	- test len(Block.Data.Txs) == Block.NumTxs
-	// wrong validators hash fails
+*/
-	block = makeBlock(state, 1)
+func TestValidateBlockData(t *testing.T) {
-	block.ValidatorsHash = []byte("wrong validators hash")
+}
-	err = blockExec.ValidateBlock(state, block)
+
-	require.Error(t, err)
+/*
-
+	TODO(#2589):
-	// wrong proposer address
+	- test len(block.LastCommit.Precommits) == state.LastValidators.Size()
-	block = makeBlock(state, 1)
+	- test state.LastValidators.VerifyCommit
-	block.ProposerAddress = ed25519.GenPrivKey().PubKey().Address()
+*/
-	err = blockExec.ValidateBlock(state, block)
+func TestValidateBlockCommit(t *testing.T) {
-	require.Error(t, err)
+}
-	block.ProposerAddress = []byte("wrong size")
+
-	err = blockExec.ValidateBlock(state, block)
+/*
-	require.Error(t, err)
+	TODO(#2589):
+	- test good/bad evidence in block
+*/
+func TestValidateBlockEvidence(t *testing.T) {
+	var height int64 = 1 // TODO(#2589): generalize
+	state, stateDB := state(1, int(height))
+
+	blockExec := NewBlockExecutor(stateDB, log.TestingLogger(), nil, nil, nil)
+
+	// make some evidence
+	addr, _ := state.Validators.GetByIndex(0)
+	goodEvidence := types.NewMockGoodEvidence(height, 0, addr)
+
+	// A block with a couple pieces of evidence passes.
+	block := makeBlock(state, height)
+	block.Evidence.Evidence = []types.Evidence{goodEvidence, goodEvidence}
+	block.EvidenceHash = block.Evidence.Hash()
+	err := blockExec.ValidateBlock(state, block)
+	require.NoError(t, err)
+
+	// A block with too much evidence fails.
+	maxBlockSize := state.ConsensusParams.BlockSize.MaxBytes
+	maxEvidenceBytes := types.MaxEvidenceBytesPerBlock(maxBlockSize)
+	maxEvidence := maxEvidenceBytes / types.MaxEvidenceBytes
+	require.True(t, maxEvidence > 2)
+	for i := int64(0); i < maxEvidence; i++ {
+		block.Evidence.Evidence = append(block.Evidence.Evidence, goodEvidence)
+	}
+	block.EvidenceHash = block.Evidence.Hash()
+	err = blockExec.ValidateBlock(state, block)
+	require.Error(t, err)
+	_, ok := err.(*types.ErrEvidenceOverflow)
+	require.True(t, ok)
+}
+
+/*
+	TODO(#2589):
+	- test unmarshalling BlockParts that are too big into a Block that
+		(note this logic happens in the consensus, not in the validation here).
+	- test making blocks from the types.MaxXXX functions works/fails as expected
+*/
+func TestValidateBlockSize(t *testing.T) {
 }

types/evidence.go

@@ -21,7 +21,8 @@ type ErrEvidenceInvalid struct {
 	ErrorValue error
 }
 
-func NewEvidenceInvalidErr(ev Evidence, err error) *ErrEvidenceInvalid {
+// NewErrEvidenceInvalid returns a new EvidenceInvalid with the given err.
+func NewErrEvidenceInvalid(ev Evidence, err error) *ErrEvidenceInvalid {
 	return &ErrEvidenceInvalid{ev, err}
 }
 
@@ -30,6 +31,22 @@ func (err *ErrEvidenceInvalid) Error() string {
 	return fmt.Sprintf("Invalid evidence: %v. Evidence: %v", err.ErrorValue, err.Evidence)
 }
 
+// ErrEvidenceOverflow is for when there is too much evidence in a block.
+type ErrEvidenceOverflow struct {
+	MaxBytes int64
+	GotBytes int64
+}
+
+// NewErrEvidenceOverflow returns a new ErrEvidenceOverflow where got > max.
+func NewErrEvidenceOverflow(max, got int64) *ErrEvidenceOverflow {
+	return &ErrEvidenceOverflow{max, got}
+}
+
+// Error returns a string representation of the error.
+func (err *ErrEvidenceOverflow) Error() string {
+	return fmt.Sprintf("Too much evidence: Max %d bytes, got %d bytes", err.MaxBytes, err.GotBytes)
+}
+
 //-------------------------------------------
 
 // Evidence represents any provable malicious activity by a validator