types: remove nested evidence field from block (#7765)

* types: replaced EvidenceData in block structure with EvidenceList

* types: introduced toProto, fromProto functions to EvidenceList

* updated Changelog

* Removed comments from tests

CHANGELOG_PENDING.md

@@ -62,10 +62,12 @@ Special thanks to external contributors on this release:
 
 - [internal/protoio] \#7325 Optimized `MarshalDelimited` by inlining the common case and using a `sync.Pool` in the worst case. (@odeke-em)
 - [consensus] \#6969 remove logic to 'unlock' a locked block.
+- [evidence] \#7700 Evidence messages contain single Evidence instead of EvidenceList (@jmalicevic)
 - [pubsub] \#7319 Performance improvements for the event query API (@creachadair)
 - [node] \#7521 Define concrete type for seed node implementation (@spacech1mp)
 - [rpc] \#7612 paginate mempool /unconfirmed_txs rpc endpoint (@spacech1mp)
 - [light] [\#7536](https://github.com/tendermint/tendermint/pull/7536) rpc /status call returns info about the light client (@jmalicevic)
+- [types] \#7765 Replace EvidenceData with EvidenceList to avoid unnecessary nesting of evidence fields within a block. (@jmalicevic)
 
 ### BUG FIXES
 

internal/consensus/byzantine_test.go

@@ -265,8 +265,8 @@ func TestByzantinePrevoteEquivocation(t *testing.T) {
 
 				require.NotNil(t, msg)
 				block := msg.Data().(types.EventDataNewBlock).Block
-				if len(block.Evidence.Evidence) != 0 {
-					evidenceFromEachValidator[j] = block.Evidence.Evidence[0]
+				if len(block.Evidence) != 0 {
+					evidenceFromEachValidator[j] = block.Evidence[0]
 					return
 				}
 			}

internal/consensus/reactor_test.go

@@ -471,7 +471,7 @@ func TestReactorWithEvidence(t *testing.T) {
 			}
 
 			block := msg.Data().(types.EventDataNewBlock).Block
-			require.Len(t, block.Evidence.Evidence, 1)
+			require.Len(t, block.Evidence, 1)
 		}(sub)
 	}
 

internal/consensus/state.go

@@ -1960,7 +1960,7 @@ func (cs *State) RecordMetrics(height int64, block *types.Block) {
 		byzantineValidatorsCount int64
 	)
 
-	for _, ev := range block.Evidence.Evidence {
+	for _, ev := range block.Evidence {
 		if dve, ok := ev.(*types.DuplicateVoteEvidence); ok {
 			if _, val := cs.Validators.GetByAddress(dve.VoteA.ValidatorAddress); val != nil {
 				byzantineValidatorsCount++

internal/evidence/pool_test.go

@@ -241,7 +241,7 @@ func TestEvidencePoolUpdate(t *testing.T) {
 
 	require.Equal(t, uint32(3), pool.Size())
 
-	pool.Update(state, block.Evidence.Evidence)
+	pool.Update(state, block.Evidence)
 
 	// a) Update marks evidence as committed so pending evidence should be empty
 	evList, _ = pool.PendingEvidence(defaultEvidenceMaxBytes)

internal/state/execution.go

@@ -163,7 +163,7 @@ func (blockExec *BlockExecutor) ValidateBlock(state State, block *types.Block) e
 		return err
 	}
 
-	err = blockExec.evpool.CheckEvidence(block.Evidence.Evidence)
+	err = blockExec.evpool.CheckEvidence(block.Evidence)
 	if err != nil {
 		return err
 	}
@@ -233,7 +233,7 @@ func (blockExec *BlockExecutor) ApplyBlock(
 	}
 
 	// Update evpool with the latest state.
-	blockExec.evpool.Update(state, block.Evidence.Evidence)
+	blockExec.evpool.Update(state, block.Evidence)
 
 	// Update the app hash and save the state.
 	state.AppHash = appHash
@@ -365,7 +365,7 @@ func execBlockOnProxyApp(
 	commitInfo := getBeginBlockValidatorInfo(block, store, initialHeight)
 
 	byzVals := make([]abci.Evidence, 0)
-	for _, evidence := range block.Evidence.Evidence {
+	for _, evidence := range block.Evidence {
 		byzVals = append(byzVals, evidence.ABCI()...)
 	}
 
@@ -578,8 +578,8 @@ func fireEvents(
 		logger.Error("failed publishing new block header", "err", err)
 	}
 
-	if len(block.Evidence.Evidence) != 0 {
-		for _, ev := range block.Evidence.Evidence {
+	if len(block.Evidence) != 0 {
+		for _, ev := range block.Evidence {
 			if err := eventBus.PublishEventNewEvidence(ctx, types.EventDataNewEvidence{
 				Evidence: ev,
 				Height:   block.Height,

internal/state/execution_test.go

@@ -225,7 +225,7 @@ func TestBeginBlockByzantineValidators(t *testing.T) {
 
 	block, err := sf.MakeBlock(state, 1, new(types.Commit))
 	require.NoError(t, err)
-	block.Evidence = types.EvidenceData{Evidence: ev}
+	block.Evidence = ev
 	block.Header.EvidenceHash = block.Evidence.Hash()
 	bps, err := block.MakePartSet(testPartSize)
 	require.NoError(t, err)

node/node_test.go

@@ -313,7 +313,7 @@ func TestCreateProposalBlock(t *testing.T) {
 
 	evList, size := evidencePool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes)
 	require.Less(t, size, state.ConsensusParams.Evidence.MaxBytes+1)
-	evData := &types.EvidenceData{Evidence: evList}
+	evData := types.EvidenceList(evList)
 	require.EqualValues(t, size, evData.ByteSize())
 
 	// fill the mempool with more txs

test/e2e/tests/evidence_test.go

@@ -17,8 +17,8 @@ func TestEvidence_Misbehavior(t *testing.T) {
 	testnet := loadTestnet(t)
 	seenEvidence := 0
 	for _, block := range blocks {
-		if len(block.Evidence.Evidence) != 0 {
-			seenEvidence += len(block.Evidence.Evidence)
+		if len(block.Evidence) != 0 {
+			seenEvidence += len(block.Evidence)
 		}
 	}
 	require.Equal(t, testnet.Evidence, seenEvidence,

types/block.go

@@ -44,7 +44,7 @@ type Block struct {
 
 	Header     `json:"header"`
 	Data       `json:"data"`
-	Evidence   EvidenceData `json:"evidence"`
+	Evidence   EvidenceList `json:"evidence"`
 	LastCommit *Commit      `json:"last_commit"`
 }
 
@@ -80,8 +80,8 @@ func (b *Block) ValidateBasic() error {
 		return fmt.Errorf("wrong Header.DataHash. Expected %X, got %X", w, g)
 	}
 
-	// NOTE: b.Evidence.Evidence may be nil, but we're just looping.
-	for i, ev := range b.Evidence.Evidence {
+	// NOTE: b.Evidence may be nil, but we're just looping.
+	for i, ev := range b.Evidence {
 		if err := ev.ValidateBasic(); err != nil {
 			return fmt.Errorf("invalid evidence (#%d): %v", i, err)
 		}
@@ -316,7 +316,7 @@ func MakeBlock(height int64, txs []Tx, lastCommit *Commit, evidence []Evidence)
 		Data: Data{
 			Txs: txs,
 		},
-		Evidence:   EvidenceData{Evidence: evidence},
+		Evidence:   evidence,
 		LastCommit: lastCommit,
 	}
 	block.fillHeader()
@@ -1083,97 +1083,6 @@ func DataFromProto(dp *tmproto.Data) (Data, error) {
 	return *data, nil
 }
 
-//-----------------------------------------------------------------------------
-
-// EvidenceData contains any evidence of malicious wrong-doing by validators
-type EvidenceData struct {
-	Evidence EvidenceList `json:"evidence"`
-
-	// Volatile. Used as cache
-	hash     tmbytes.HexBytes
-	byteSize int64
-}
-
-// Hash returns the hash of the data.
-func (data *EvidenceData) Hash() tmbytes.HexBytes {
-	if data.hash == nil {
-		data.hash = data.Evidence.Hash()
-	}
-	return data.hash
-}
-
-// ByteSize returns the total byte size of all the evidence
-func (data *EvidenceData) ByteSize() int64 {
-	if data.byteSize == 0 && len(data.Evidence) != 0 {
-		pb, err := data.ToProto()
-		if err != nil {
-			panic(err)
-		}
-		data.byteSize = int64(pb.Size())
-	}
-	return data.byteSize
-}
-
-// StringIndented returns a string representation of the evidence.
-func (data *EvidenceData) StringIndented(indent string) string {
-	if data == nil {
-		return "nil-Evidence"
-	}
-	evStrings := make([]string, tmmath.MinInt(len(data.Evidence), 21))
-	for i, ev := range data.Evidence {
-		if i == 20 {
-			evStrings[i] = fmt.Sprintf("... (%v total)", len(data.Evidence))
-			break
-		}
-		evStrings[i] = fmt.Sprintf("Evidence:%v", ev)
-	}
-	return fmt.Sprintf(`EvidenceData{
-%s  %v
-%s}#%v`,
-		indent, strings.Join(evStrings, "\n"+indent+"  "),
-		indent, data.hash)
-}
-
-// ToProto converts EvidenceData to protobuf
-func (data *EvidenceData) ToProto() (*tmproto.EvidenceList, error) {
-	if data == nil {
-		return nil, errors.New("nil evidence data")
-	}
-
-	evi := new(tmproto.EvidenceList)
-	eviBzs := make([]tmproto.Evidence, len(data.Evidence))
-	for i := range data.Evidence {
-		protoEvi, err := EvidenceToProto(data.Evidence[i])
-		if err != nil {
-			return nil, err
-		}
-		eviBzs[i] = *protoEvi
-	}
-	evi.Evidence = eviBzs
-
-	return evi, nil
-}
-
-// FromProto sets a protobuf EvidenceData to the given pointer.
-func (data *EvidenceData) FromProto(eviData *tmproto.EvidenceList) error {
-	if eviData == nil {
-		return errors.New("nil evidenceData")
-	}
-
-	eviBzs := make(EvidenceList, len(eviData.Evidence))
-	for i := range eviData.Evidence {
-		evi, err := EvidenceFromProto(&eviData.Evidence[i])
-		if err != nil {
-			return err
-		}
-		eviBzs[i] = evi
-	}
-	data.Evidence = eviBzs
-	data.byteSize = int64(eviData.Size())
-
-	return nil
-}
-
 //--------------------------------------------------------------------------------
 
 // BlockID

types/block_test.go

@@ -52,7 +52,7 @@ func TestBlockAddEvidence(t *testing.T) {
 
 	block := MakeBlock(h, txs, commit, evList)
 	require.NotNil(t, block)
-	require.Equal(t, 1, len(block.Evidence.Evidence))
+	require.Equal(t, 1, len(block.Evidence))
 	require.NotNil(t, block.EvidenceHash)
 }
 
@@ -109,7 +109,7 @@ func TestBlockValidateBasic(t *testing.T) {
 		}, true},
 		{"Invalid Evidence", func(blk *Block) {
 			emptyEv := &DuplicateVoteEvidence{}
-			blk.Evidence = EvidenceData{Evidence: []Evidence{emptyEv}}
+			blk.Evidence = []Evidence{emptyEv}
 		}, true},
 	}
 	for i, tc := range testCases {
@@ -700,7 +700,7 @@ func TestBlockProtoBuf(t *testing.T) {
 	evidenceTime := time.Date(2019, 1, 1, 0, 0, 0, 0, time.UTC)
 	evi, err := NewMockDuplicateVoteEvidence(ctx, h, evidenceTime, "block-test-chain")
 	require.NoError(t, err)
-	b2.Evidence = EvidenceData{Evidence: EvidenceList{evi}}
+	b2.Evidence = EvidenceList{evi}
 	b2.EvidenceHash = b2.Evidence.Hash()
 
 	b3 := MakeBlock(h, []Tx{}, c1, []Evidence{})
@@ -729,7 +729,7 @@ func TestBlockProtoBuf(t *testing.T) {
 			require.NoError(t, err, tc.msg)
 			require.EqualValues(t, tc.b1.Header, block.Header, tc.msg)
 			require.EqualValues(t, tc.b1.Data, block.Data, tc.msg)
-			require.EqualValues(t, tc.b1.Evidence.Evidence, block.Evidence.Evidence, tc.msg)
+			require.EqualValues(t, tc.b1.Evidence, block.Evidence, tc.msg)
 			require.EqualValues(t, *tc.b1.LastCommit, *block.LastCommit, tc.msg)
 		} else {
 			require.Error(t, err, tc.msg)
@@ -760,46 +760,6 @@ func TestDataProtoBuf(t *testing.T) {
 	}
 }
 
-// TestEvidenceDataProtoBuf ensures parity in converting to and from proto.
-func TestEvidenceDataProtoBuf(t *testing.T) {
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-
-	const chainID = "mychain"
-	ev, err := NewMockDuplicateVoteEvidence(ctx, math.MaxInt64, time.Now(), chainID)
-	require.NoError(t, err)
-	data := &EvidenceData{Evidence: EvidenceList{ev}}
-	_ = data.ByteSize()
-	testCases := []struct {
-		msg      string
-		data1    *EvidenceData
-		expPass1 bool
-		expPass2 bool
-	}{
-		{"success", data, true, true},
-		{"empty evidenceData", &EvidenceData{Evidence: EvidenceList{}}, true, true},
-		{"fail nil Data", nil, false, false},
-	}
-
-	for _, tc := range testCases {
-		protoData, err := tc.data1.ToProto()
-		if tc.expPass1 {
-			require.NoError(t, err, tc.msg)
-		} else {
-			require.Error(t, err, tc.msg)
-		}
-
-		eviD := new(EvidenceData)
-		err = eviD.FromProto(protoData)
-		if tc.expPass2 {
-			require.NoError(t, err, tc.msg)
-			require.Equal(t, tc.data1, eviD, tc.msg)
-		} else {
-			require.Error(t, err, tc.msg)
-		}
-	}
-}
-
 // exposed for testing
 func MakeRandHeader() Header {
 	chainID := "test"

types/evidence.go

@@ -15,6 +15,7 @@ import (
 	"github.com/tendermint/tendermint/crypto/merkle"
 	"github.com/tendermint/tendermint/crypto/tmhash"
 	"github.com/tendermint/tendermint/internal/jsontypes"
+	tmmath "github.com/tendermint/tendermint/libs/math"
 	tmrand "github.com/tendermint/tendermint/libs/rand"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 )
@@ -554,6 +555,73 @@ func LightClientAttackEvidenceFromProto(lpb *tmproto.LightClientAttackEvidence)
 // EvidenceList is a list of Evidence. Evidences is not a word.
 type EvidenceList []Evidence
 
+// StringIndented returns a string representation of the evidence.
+func (evl EvidenceList) StringIndented(indent string) string {
+	if evl == nil {
+		return "nil-Evidence"
+	}
+	evStrings := make([]string, tmmath.MinInt(len(evl), 21))
+	for i, ev := range evl {
+		if i == 20 {
+			evStrings[i] = fmt.Sprintf("... (%v total)", len(evl))
+			break
+		}
+		evStrings[i] = fmt.Sprintf("Evidence:%v", ev)
+	}
+	return fmt.Sprintf(`EvidenceList{
+%s  %v
+%s}#%v`,
+		indent, strings.Join(evStrings, "\n"+indent+"  "),
+		indent, evl.Hash())
+}
+
+// ByteSize returns the total byte size of all the evidence
+func (evl EvidenceList) ByteSize() int64 {
+	if len(evl) != 0 {
+		pb, err := evl.ToProto()
+		if err != nil {
+			panic(err)
+		}
+		return int64(pb.Size())
+	}
+	return 0
+}
+
+// FromProto sets a protobuf EvidenceList to the given pointer.
+func (evl *EvidenceList) FromProto(eviList *tmproto.EvidenceList) error {
+	if eviList == nil {
+		return errors.New("nil evidence list")
+	}
+
+	eviBzs := make(EvidenceList, len(eviList.Evidence))
+	for i := range eviList.Evidence {
+		evi, err := EvidenceFromProto(&eviList.Evidence[i])
+		if err != nil {
+			return err
+		}
+		eviBzs[i] = evi
+	}
+	*evl = eviBzs
+	return nil
+}
+
+// ToProto converts EvidenceList to protobuf
+func (evl *EvidenceList) ToProto() (*tmproto.EvidenceList, error) {
+	if evl == nil {
+		return nil, errors.New("nil evidence list")
+	}
+
+	eviBzs := make([]tmproto.Evidence, len(*evl))
+	for i, v := range *evl {
+		protoEvi, err := EvidenceToProto(v)
+		if err != nil {
+			return nil, err
+		}
+		eviBzs[i] = *protoEvi
+	}
+	return &tmproto.EvidenceList{Evidence: eviBzs}, nil
+}
+
 func (evl EvidenceList) MarshalJSON() ([]byte, error) {
 	lst := make([]json.RawMessage, len(evl))
 	for i, ev := range evl {

types/evidence_test.go

@@ -33,6 +33,44 @@ func TestEvidenceList(t *testing.T) {
 	assert.False(t, evl.Has(&DuplicateVoteEvidence{}))
 }
 
+// TestEvidenceListProtoBuf to ensure parity in protobuf output and input
+func TestEvidenceListProtoBuf(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	const chainID = "mychain"
+	ev, err := NewMockDuplicateVoteEvidence(ctx, math.MaxInt64, time.Now(), chainID)
+	require.NoError(t, err)
+	data := EvidenceList{ev}
+	testCases := []struct {
+		msg      string
+		data1    *EvidenceList
+		expPass1 bool
+		expPass2 bool
+	}{
+		{"success", &data, true, true},
+		{"empty evidenceData", &EvidenceList{}, true, true},
+		{"fail nil Data", nil, false, false},
+	}
+
+	for _, tc := range testCases {
+		protoData, err := tc.data1.ToProto()
+		if tc.expPass1 {
+			require.NoError(t, err, tc.msg)
+		} else {
+			require.Error(t, err, tc.msg)
+		}
+
+		eviD := new(EvidenceList)
+		err = eviD.FromProto(protoData)
+		if tc.expPass2 {
+			require.NoError(t, err, tc.msg)
+			require.Equal(t, tc.data1, eviD, tc.msg)
+		} else {
+			require.Error(t, err, tc.msg)
+		}
+	}
+}
 func randomDuplicateVoteEvidence(ctx context.Context, t *testing.T) *DuplicateVoteEvidence {
 	t.Helper()
 	val := NewMockPV()