diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index faaf1756..66f2fdc1 100644 --- a/CHANGELOG_PENDING.md +++ b/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) diff --git a/state/validation.go b/state/validation.go index ccfe1ef1..9d8ef97a 100644 --- a/state/validation.go +++ b/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) } } diff --git a/state/validation_test.go b/state/validation_test.go index ba76a72b..e5f45166 100644 --- a/state/validation_test.go +++ b/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) { - state, _ := state(1, 1) +// TODO(#2589): +// - 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 - block := makeBlock(state, 1) + // A good block passes. + block := makeBlock(state, height) err := blockExec.ValidateBlock(state, block) require.NoError(t, err) - // wrong chain fails - block = makeBlock(state, 1) - block.ChainID = "not-the-real-one" - err = blockExec.ValidateBlock(state, block) - require.Error(t, err) + wrongHash := tmhash.Sum([]byte("this hash is wrong")) - // wrong height fails - block = makeBlock(state, 1) - block.Height += 10 - err = blockExec.ValidateBlock(state, block) - require.Error(t, err) + // Manipulation of any header field causes failure. + testCases := []struct { + 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 - // wrong total tx fails - block = makeBlock(state, 1) - block.TotalTxs += 10 - err = blockExec.ValidateBlock(state, block) - require.Error(t, err) + {"LastBlockID wrong", func(block *types.Block) { block.LastBlockID.PartsHeader.Total += 10 }}, + {"LastCommitHash wrong", func(block *types.Block) { block.LastCommitHash = wrongHash }}, + {"DataHash wrong", func(block *types.Block) { block.DataHash = wrongHash }}, - // wrong blockid fails - block = makeBlock(state, 1) - block.LastBlockID.PartsHeader.Total += 10 - err = blockExec.ValidateBlock(state, block) - require.Error(t, err) + {"ValidatorsHash wrong", func(block *types.Block) { block.ValidatorsHash = wrongHash }}, + {"NextValidatorsHash wrong", func(block *types.Block) { block.NextValidatorsHash = wrongHash }}, + {"ConsensusHash wrong", func(block *types.Block) { block.ConsensusHash = wrongHash }}, + {"AppHash wrong", func(block *types.Block) { block.AppHash = wrongHash }}, + {"LastResultsHash wrong", func(block *types.Block) { block.LastResultsHash = wrongHash }}, - // wrong app hash fails - block = makeBlock(state, 1) - block.AppHash = []byte("wrong app hash") - err = blockExec.ValidateBlock(state, block) - require.Error(t, err) + {"EvidenceHash wrong", func(block *types.Block) { block.EvidenceHash = wrongHash }}, + {"Proposer wrong", func(block *types.Block) { block.ProposerAddress = ed25519.GenPrivKey().PubKey().Address() }}, + {"Proposer invalid", func(block *types.Block) { block.ProposerAddress = []byte("wrong size") }}, + } - // wrong consensus hash fails - block = makeBlock(state, 1) - block.ConsensusHash = []byte("wrong consensus hash") - err = blockExec.ValidateBlock(state, block) - require.Error(t, err) - - // wrong results hash fails - block = makeBlock(state, 1) - block.LastResultsHash = []byte("wrong results hash") - err = blockExec.ValidateBlock(state, block) - require.Error(t, err) - - // wrong validators hash fails - block = makeBlock(state, 1) - block.ValidatorsHash = []byte("wrong validators hash") - err = blockExec.ValidateBlock(state, block) - require.Error(t, err) - - // wrong proposer address - block = makeBlock(state, 1) - block.ProposerAddress = ed25519.GenPrivKey().PubKey().Address() - err = blockExec.ValidateBlock(state, block) - require.Error(t, err) - block.ProposerAddress = []byte("wrong size") - err = blockExec.ValidateBlock(state, block) - require.Error(t, err) + for _, tc := range testCases { + block := makeBlock(state, height) + tc.malleateBlock(block) + err := blockExec.ValidateBlock(state, block) + require.Error(t, err, tc.name) + } +} + +/* + TODO(#2589): + - test Block.Data.Hash() == Block.DataHash + - test len(Block.Data.Txs) == Block.NumTxs +*/ +func TestValidateBlockData(t *testing.T) { +} + +/* + TODO(#2589): + - test len(block.LastCommit.Precommits) == state.LastValidators.Size() + - test state.LastValidators.VerifyCommit +*/ +func TestValidateBlockCommit(t *testing.T) { +} + +/* + 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) { } diff --git a/types/evidence.go b/types/evidence.go index 241e0939..916dd094 100644 --- a/types/evidence.go +++ b/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