diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index d46f13a80..7d603b0c6 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -72,6 +72,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [mempool] [\#4759](https://github.com/tendermint/tendermint/pull/4759) Allow ReapX and CheckTx functions to run in parallel (@melekes) - [state] [\#4781](https://github.com/tendermint/tendermint/pull/4781) Export `InitStateVersion` for the initial state version (@erikgrinaker) - [p2p/conn] \#4795 Return err on `signChallenge()` instead of panic +- [evidence] [\#4839](https://github.com/tendermint/tendermint/pull/4839) Reject duplicate evidence from being proposed (@cmwaters) ### BUG FIXES: diff --git a/node/node_test.go b/node/node_test.go index d8f20fe9c..8888a9aa6 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -262,7 +262,7 @@ func TestCreateProposalBlock(t *testing.T) { // fill the evidence pool with more evidence // than can fit in a block - for i := 0; i < maxEvidence+1; i++ { + for i := 0; i <= maxEvidence; i++ { ev := types.NewMockRandomEvidence(height, time.Now(), proposerAddr, tmrand.Bytes(minEvSize)) err := evidencePool.AddEvidence(ev) require.NoError(t, err) diff --git a/state/helpers_test.go b/state/helpers_test.go index 2805af63c..1093157d8 100644 --- a/state/helpers_test.go +++ b/state/helpers_test.go @@ -146,6 +146,7 @@ func makeConsensusParams( blockBytes, blockGas int64, blockTimeIotaMs int64, evidenceAge int64, + maxNumEvidence uint32, ) types.ConsensusParams { return types.ConsensusParams{ Block: types.BlockParams{ @@ -156,6 +157,7 @@ func makeConsensusParams( Evidence: types.EvidenceParams{ MaxAgeNumBlocks: evidenceAge, MaxAgeDuration: time.Duration(evidenceAge), + MaxNum: maxNumEvidence, }, } } diff --git a/state/state_test.go b/state/state_test.go index 6ca196bb2..a506b1d27 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -986,7 +986,7 @@ func TestConsensusParamsChangesSaveLoad(t *testing.T) { } func TestApplyUpdates(t *testing.T) { - initParams := makeConsensusParams(1, 2, 3, 4) + initParams := makeConsensusParams(1, 2, 3, 4, 5) const maxAge int64 = 66 cases := [...]struct { init types.ConsensusParams @@ -1002,15 +1002,16 @@ func TestApplyUpdates(t *testing.T) { MaxGas: 55, }, }, - makeConsensusParams(44, 55, 3, 4)}, + makeConsensusParams(44, 55, 3, 4, 5)}, 3: {initParams, abci.ConsensusParams{ Evidence: &abci.EvidenceParams{ MaxAgeNumBlocks: maxAge, MaxAgeDuration: time.Duration(maxAge), + MaxNum: 10, }, }, - makeConsensusParams(1, 2, 3, maxAge)}, + makeConsensusParams(1, 2, 3, maxAge, 10)}, } for i, tc := range cases { diff --git a/state/validation.go b/state/validation.go index ba8c22e95..44a8edd71 100644 --- a/state/validation.go +++ b/state/validation.go @@ -127,7 +127,13 @@ func validateBlock(evidencePool EvidencePool, stateDB dbm.DB, state State, block } // Validate all evidence. - for _, ev := range block.Evidence.Evidence { + for idx, ev := range block.Evidence.Evidence { + // check that no evidence has been submitted more than once + for i := idx + 1; i < len(block.Evidence.Evidence); i++ { + if ev.Equal(block.Evidence.Evidence[i]) { + return types.NewErrEvidenceInvalid(ev, errors.New("evidence was submitted twice")) + } + } if evidencePool != nil { if evidencePool.IsCommitted(ev) { return types.NewErrEvidenceInvalid(ev, errors.New("evidence was already committed")) diff --git a/state/validation_test.go b/state/validation_test.go index 640aa8ba3..89add1715 100644 --- a/state/validation_test.go +++ b/state/validation_test.go @@ -203,7 +203,8 @@ func TestValidateBlockEvidence(t *testing.T) { require.NoError(t, proxyApp.Start()) defer proxyApp.Stop() - state, stateDB, privVals := makeState(3, 1) + state, stateDB, privVals := makeState(4, 1) + state.ConsensusParams.Evidence.MaxNum = 3 blockExec := sm.NewBlockExecutor( stateDB, log.TestingLogger(), @@ -215,8 +216,8 @@ func TestValidateBlockEvidence(t *testing.T) { for height := int64(1); height < validationTestsStopHeight; height++ { proposerAddr := state.Validators.GetProposer().Address - goodEvidence := types.NewMockEvidence(height, time.Now(), proposerAddr) maxNumEvidence := state.ConsensusParams.Evidence.MaxNum + t.Log(maxNumEvidence) if height > 1 { /* A block with too much evidence fails @@ -225,7 +226,7 @@ func TestValidateBlockEvidence(t *testing.T) { evidence := make([]types.Evidence, 0) // one more than the maximum allowed evidence for i := uint32(0); i <= maxNumEvidence; i++ { - evidence = append(evidence, goodEvidence) + evidence = append(evidence, types.NewMockEvidence(height, time.Now(), proposerAddr)) } block, _ := state.MakeBlock(height, makeTxs(height), lastCommit, evidence, proposerAddr) err := blockExec.ValidateBlock(state, block) @@ -240,7 +241,9 @@ func TestValidateBlockEvidence(t *testing.T) { evidence := make([]types.Evidence, 0) // precisely the amount of allowed evidence for i := uint32(0); i < maxNumEvidence; i++ { - evidence = append(evidence, goodEvidence) + // make different evidence for each validator + addr, _ := state.Validators.GetByIndex(int(i)) + evidence = append(evidence, types.NewMockEvidence(height, time.Now(), addr)) } var err error @@ -313,27 +316,24 @@ func TestValidateAlreadyPendingEvidence(t *testing.T) { require.NoError(t, err) } -// TODO: prevent committing duplicate votes -//func TestValidateDuplicateEvidenceShouldFail(t *testing.T) { -// var height int64 = 1 -// var evidence []types.Evidence -// state, stateDB, _ := makeState(1, int(height)) -// -// evpool := evmock.NewDefaultEvidencePool() -// blockExec := sm.NewBlockExecutor( -// stateDB, log.TestingLogger(), -// nil, -// nil, -// evpool) -// // A block with a couple pieces of evidence passes. -// block := makeBlock(state, height) -// addr, _ := state.Validators.GetByIndex(0) -// for i := 0; i < 2; i++ { -// evidence = append(evidence, types.NewMockEvidence(height, time.Now(), addr)) -// } -// block.Evidence.Evidence = evidence -// block.EvidenceHash = block.Evidence.Hash() -// err := blockExec.ValidateBlock(state, block) -// -// require.Error(t, err) -//} +func TestValidateDuplicateEvidenceShouldFail(t *testing.T) { + var height int64 = 1 + state, stateDB, _ := makeState(1, int(height)) + addr, _ := state.Validators.GetByIndex(0) + addr2, _ := state.Validators.GetByIndex(1) + ev := types.NewMockEvidence(height, defaultTestTime, addr) + ev2 := types.NewMockEvidence(height, defaultTestTime, addr2) + + blockExec := sm.NewBlockExecutor( + stateDB, log.TestingLogger(), + nil, + nil, + sm.MockEvidencePool{}) + // A block with a couple pieces of evidence passes. + block := makeBlock(state, height) + block.Evidence.Evidence = []types.Evidence{ev, ev2, ev2} + block.EvidenceHash = block.Evidence.Hash() + err := blockExec.ValidateBlock(state, block) + + require.Error(t, err) +} diff --git a/types/evidence.go b/types/evidence.go index dde7e8385..461ff4fba 100644 --- a/types/evidence.go +++ b/types/evidence.go @@ -1060,6 +1060,8 @@ func (e MockRandomEvidence) Hash() []byte { return []byte(fmt.Sprintf("%d-%x", e.EvidenceHeight, e.randBytes)) } +func (e MockRandomEvidence) Equal(ev Evidence) bool { return false } + // UNSTABLE type MockEvidence struct { EvidenceHeight int64 @@ -1090,9 +1092,8 @@ func (e MockEvidence) Bytes() []byte { } func (e MockEvidence) Verify(chainID string, pubKey crypto.PubKey) error { return nil } func (e MockEvidence) Equal(ev Evidence) bool { - e2 := ev.(MockEvidence) - return e.EvidenceHeight == e2.EvidenceHeight && - bytes.Equal(e.EvidenceAddress, e2.EvidenceAddress) + return e.EvidenceHeight == ev.Height() && + bytes.Equal(e.EvidenceAddress, ev.Address()) } func (e MockEvidence) ValidateBasic() error { return nil } func (e MockEvidence) String() string {