feat!: move evidence back to Tendermint block (#897)

* feat!: move evidence back to Tendermint block * fix: lint * fix: some unit tests * revert: node_test See: https://github.com/tendermint/tendermint/blob/d32df22c8b43e87e6323b7b1bd114e6e82ed7a27/node/node_test.go#L316 * remove unnecessary copy of txs * revert: mempool v0 dataSize modification * revert: TestMaxProposalBlockSize to resolve TODO * mempool v0 dataSize modification (part 2)
celestiaorg · Nov 29, 2022 · dcd66f7 · dcd66f7
1 parent 2ec23f8
commit dcd66f7
Show file tree

Hide file tree

Showing 33 changed files with 1,795 additions and 1,661 deletions.
diff --git a/blockchain/msgs_test.go b/blockchain/msgs_test.go
@@ -81,7 +81,7 @@ func TestBcStatusResponseMessageValidateBasic(t *testing.T) {
 
 // nolint:lll // ignore line length in tests
 func TestBlockchainMessageVectors(t *testing.T) {
-	block := types.MakeBlock(int64(3), factory.MakeData([]types.Tx{types.Tx("Hello World")}, nil, nil), nil)
+	block := types.MakeBlock(int64(3), factory.MakeData([]types.Tx{types.Tx("Hello World")}, nil), nil, nil)
 	block.Version.Block = 11 // overwrite updated protocol version
 
 	bpb, err := block.ToProto()
@@ -98,7 +98,7 @@ func TestBlockchainMessageVectors(t *testing.T) {
 			BlockRequest: &bcproto.BlockRequest{Height: math.MaxInt64}}},
 			"0a0a08ffffffffffffffff7f"},
 		{"BlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_BlockResponse{
-			BlockResponse: &bcproto.BlockResponse{Block: bpb}}}, "1a93010a90010a5b0a02080b1803220b088092b8c398feffffff012a0212003a20c4da88e876062aa1543400d50d0eaa0dac88096057949cfb7bca7f3a48c04bf96a20e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b85512310a0b48656c6c6f20576f726c641a003220c4da88e876062aa1543400d50d0eaa0dac88096057949cfb7bca7f3a48c04bf9"},
+			BlockResponse: &bcproto.BlockResponse{Block: bpb}}}, "1a93010a90010a5b0a02080b1803220b088092b8c398feffffff012a0212003a20c4da88e876062aa1543400d50d0eaa0dac88096057949cfb7bca7f3a48c04bf96a20e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855122f0a0b48656c6c6f20576f726c643220c4da88e876062aa1543400d50d0eaa0dac88096057949cfb7bca7f3a48c04bf91a00"},
 		{"NoBlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_NoBlockResponse{
 			NoBlockResponse: &bcproto.NoBlockResponse{Height: 1}}}, "12020801"},
 		{"NoBlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_NoBlockResponse{

diff --git a/blockchain/v0/reactor_test.go b/blockchain/v0/reactor_test.go
@@ -292,8 +292,9 @@ func makeTxs(height int64) (txs []types.Tx) {
 func makeBlock(height int64, state sm.State, lastCommit *types.Commit) *types.Block {
 	block, _ := state.MakeBlock(
 		height,
-		factory.MakeData(makeTxs(height), nil, nil),
+		factory.MakeData(makeTxs(height), nil),
 		lastCommit,
+		nil,
 		state.Validators.GetProposer().Address,
 	)
 	return block

diff --git a/blockchain/v1/peer_test.go b/blockchain/v1/peer_test.go
@@ -277,5 +277,5 @@ func checkByStoppingPeerTimer(t *testing.T, peer *BpPeer, running bool) {
 }
 
 func makeSmallBlock(height int) *types.Block {
-	return types.MakeBlock(int64(height), factory.MakeDataFromTxs([]types.Tx{types.Tx("foo")}), nil)
+	return types.MakeBlock(int64(height), factory.MakeDataFromTxs([]types.Tx{types.Tx("foo")}), nil, nil)
 }
diff --git a/blockchain/v1/pool_test.go b/blockchain/v1/pool_test.go
@@ -82,7 +82,7 @@ func makeBlockPool(bcr *testBcR, height int64, peers []BpPeer, blocks map[int64]
 		bPool.peers[p.id].RequestSent(h)
 		if p.create {
 			// simulate that a block at height h has been received
-			_ = bPool.peers[p.id].AddBlock(types.MakeBlock(h, factory.MakeDataFromTxs(txs), nil), 100)
+			_ = bPool.peers[p.id].AddBlock(types.MakeBlock(h, factory.MakeDataFromTxs(txs), nil, nil), 100)
 		}
 	}
 	return bPool
@@ -393,7 +393,7 @@ func TestBlockPoolAddBlock(t *testing.T) {
 			pool: makeBlockPool(testBcR, 10, []BpPeer{{ID: "P1", Height: 100}}, map[int64]tPBlocks{}),
 			args: args{
 				peerID:    "P2",
-				block:     types.MakeBlock(int64(10), factory.MakeDataFromTxs(txs), nil),
+				block:     types.MakeBlock(int64(10), factory.MakeDataFromTxs(txs), nil, nil),
 				blockSize: 100,
 			},
 			poolWanted: makeBlockPool(testBcR, 10, []BpPeer{{ID: "P1", Height: 100}}, map[int64]tPBlocks{}),
@@ -405,7 +405,7 @@ func TestBlockPoolAddBlock(t *testing.T) {
 				map[int64]tPBlocks{10: {"P1", false}}),
 			args: args{
 				peerID:    "P1",
-				block:     types.MakeBlock(int64(11), factory.MakeDataFromTxs(txs), nil),
+				block:     types.MakeBlock(int64(11), factory.MakeDataFromTxs(txs), nil, nil),
 				blockSize: 100,
 			},
 			poolWanted: makeBlockPool(testBcR, 10,
@@ -419,7 +419,7 @@ func TestBlockPoolAddBlock(t *testing.T) {
 				map[int64]tPBlocks{10: {"P1", true}, 11: {"P1", false}}),
 			args: args{
 				peerID:    "P1",
-				block:     types.MakeBlock(int64(10), factory.MakeDataFromTxs(txs), nil),
+				block:     types.MakeBlock(int64(10), factory.MakeDataFromTxs(txs), nil, nil),
 				blockSize: 100,
 			},
 			poolWanted: makeBlockPool(testBcR, 10,
@@ -433,7 +433,7 @@ func TestBlockPoolAddBlock(t *testing.T) {
 				map[int64]tPBlocks{10: {"P1", false}}),
 			args: args{
 				peerID:    "P2",
-				block:     types.MakeBlock(int64(10), factory.MakeDataFromTxs(txs), nil),
+				block:     types.MakeBlock(int64(10), factory.MakeDataFromTxs(txs), nil, nil),
 				blockSize: 100,
 			},
 			poolWanted: makeBlockPool(testBcR, 10,
@@ -447,7 +447,7 @@ func TestBlockPoolAddBlock(t *testing.T) {
 				map[int64]tPBlocks{10: {"P1", false}}),
 			args: args{
 				peerID:    "P1",
-				block:     types.MakeBlock(int64(10), factory.MakeDataFromTxs(txs), nil),
+				block:     types.MakeBlock(int64(10), factory.MakeDataFromTxs(txs), nil, nil),
 				blockSize: 100,
 			},
 			poolWanted: makeBlockPool(testBcR, 10,

diff --git a/blockchain/v1/reactor_fsm_test.go b/blockchain/v1/reactor_fsm_test.go
@@ -143,7 +143,7 @@ func sBlockRespEv(current, expected string, peerID p2p.ID, height int64, prevBlo
 		data: bReactorEventData{
 			peerID: peerID,
 			height: height,
-			block:  types.MakeBlock(height, factory.MakeDataFromTxs(txs), nil),
+			block:  types.MakeBlock(height, factory.MakeDataFromTxs(txs), nil, nil),
 			length: 100},
 		wantState:     expected,
 		wantNewBlocks: append(prevBlocks, height),
@@ -160,7 +160,7 @@ func sBlockRespEvErrored(current, expected string,
 		data: bReactorEventData{
 			peerID: peerID,
 			height: height,
-			block:  types.MakeBlock(height, factory.MakeDataFromTxs(txs), nil),
+			block:  types.MakeBlock(height, factory.MakeDataFromTxs(txs), nil, nil),
 			length: 100},
 		wantState:        expected,
 		wantErr:          wantErr,

diff --git a/blockchain/v1/reactor_test.go b/blockchain/v1/reactor_test.go
@@ -361,6 +361,7 @@ func makeBlock(height int64, state sm.State, lastCommit *types.Commit) *types.Bl
 		height,
 		factory.MakeDataFromTxs(makeTxs(height)),
 		lastCommit,
+		nil,
 		state.Validators.GetProposer().Address,
 	)
 	return block

diff --git a/blockchain/v2/reactor_test.go b/blockchain/v2/reactor_test.go
@@ -461,6 +461,7 @@ func makeBlock(height int64, state sm.State, lastCommit *types.Commit) *types.Bl
 		height,
 		factory.MakeDataFromTxs(makeTxs(height)),
 		lastCommit,
+		nil,
 		state.Validators.GetProposer().Address,
 	)
 	return block

diff --git a/consensus/replay_test.go b/consensus/replay_test.go
@@ -291,7 +291,7 @@ func (w *crashingWAL) Start() error { return w.next.Start() }
 func (w *crashingWAL) Stop() error  { return w.next.Stop() }
 func (w *crashingWAL) Wait()        { w.next.Wait() }
 
-//------------------------------------------------------------------------------------------
+// ------------------------------------------------------------------------------------------
 type testSim struct {
 	GenesisState sm.State
 	Config       *cfg.Config
@@ -995,6 +995,7 @@ func makeBlock(state sm.State, lastBlock *types.Block, lastBlockMeta *types.Bloc
 		height,
 		factory.MakeDataFromTxs([]types.Tx{}),
 		lastCommit,
+		nil,
 		state.Validators.GetProposer().Address,
 	)
 }

diff --git a/evidence/pool_test.go b/evidence/pool_test.go
@@ -193,7 +193,7 @@ func TestEvidencePoolUpdate(t *testing.T) {
 	ev := types.NewMockDuplicateVoteEvidenceWithValidator(height, defaultEvidenceTime.Add(21*time.Minute),
 		val, evidenceChainID)
 	lastCommit := makeCommit(height, val.PrivKey.PubKey().Address())
-	block := types.MakeBlock(height+1, factory.MakeData([]types.Tx{}, []types.Evidence{ev}, nil), lastCommit)
+	block := types.MakeBlock(height+1, factory.MakeData([]types.Tx{}, nil), lastCommit, []types.Evidence{ev})
 
 	// update state (partially)
 	state.LastBlockHeight = height + 1
@@ -405,7 +405,7 @@ func initializeBlockStore(db dbm.DB, state sm.State, valAddr []byte) *store.Bloc
 
 	for i := int64(1); i <= state.LastBlockHeight; i++ {
 		lastCommit := makeCommit(i-1, valAddr)
-		block, _ := state.MakeBlock(i, factory.MakeData([]types.Tx{}, nil, nil), lastCommit,
+		block, _ := state.MakeBlock(i, factory.MakeData([]types.Tx{}, nil), lastCommit, nil,
 			state.Validators.GetProposer().Address)
 		block.Header.Time = defaultEvidenceTime.Add(time.Duration(i) * time.Minute)
 		block.Header.Version = tmversion.Consensus{Block: version.BlockProtocol, App: 1}

diff --git a/mempool/v0/clist_mempool.go b/mempool/v0/clist_mempool.go
@@ -194,7 +194,9 @@ func (mem *CListMempool) TxsWaitChan() <-chan struct{} {
 
 // It blocks if we're waiting on Update() or Reap().
 // cb: A callback from the CheckTx command.
-//     It gets called from another goroutine.
+//
+//	It gets called from another goroutine.
+//
 // CONTRACT: Either cb will get called, or err returned.
 //
 // Safe for concurrent use by multiple goroutines.
@@ -310,7 +312,7 @@ func (mem *CListMempool) reqResCb(
 }
 
 // Called from:
-//  - resCbFirstTime (lock not held) if tx is valid
+//   - resCbFirstTime (lock not held) if tx is valid
 func (mem *CListMempool) addTx(memTx *mempoolTx) {
 	e := mem.txs.PushBack(memTx)
 	mem.txsMap.Store(memTx.tx.Key(), e)
@@ -319,8 +321,8 @@ func (mem *CListMempool) addTx(memTx *mempoolTx) {
 }
 
 // Called from:
-//  - Update (lock held) if tx was committed
-// 	- resCbRecheck (lock not held) if tx was invalidated
+//   - Update (lock held) if tx was committed
+//   - resCbRecheck (lock not held) if tx was invalidated
 func (mem *CListMempool) removeTx(tx types.Tx, elem *clist.CElement, removeFromCache bool) {
 	mem.txs.Remove(elem)
 	elem.DetachPrev()
@@ -534,8 +536,7 @@ func (mem *CListMempool) ReapMaxBytesMaxGas(maxBytes, maxGas int64) types.Txs {
 
 		txs = append(txs, memTx.tx)
 
-		// we subtract 4 here to account for extra bytes we add to the additional data fields
-		dataSize := types.ComputeProtoSizeForTxs([]types.Tx{memTx.tx}) - 4
+		dataSize := types.ComputeProtoSizeForTxs([]types.Tx{memTx.tx})
 
 		// Check total size requirement
 		if maxBytes > -1 && runningSize+dataSize > maxBytes {

diff --git a/mempool/v0/clist_mempool_test.go b/mempool/v0/clist_mempool_test.go
@@ -149,11 +149,11 @@ func TestReapMaxBytesMaxGas(t *testing.T) {
 		{20, 0, -1, 0},
 		{20, 0, 10, 0},
 		{20, 10, 10, 0},
-		{20, 28, 10, 1}, // account for overhead in Data{}
+		{20, 24, 10, 1},
 		{20, 240, 5, 5},
-		{20, 240, -1, 12},
+		{20, 240, -1, 10},
 		{20, 240, 10, 10},
-		{20, 240, 15, 12},
+		{20, 240, 15, 10},
 		{20, 20000, -1, 20},
 		{20, 20000, 5, 5},
 		{20, 20000, 30, 20},

diff --git a/mempool/v1/mempool_test.go b/mempool/v1/mempool_test.go
@@ -342,15 +342,15 @@ func TestTxMempool_ReapMaxBytesMaxGas(t *testing.T) {
 	ensurePrioritized(reapedTxs)
 	require.Equal(t, len(tTxs), txmp.Size())
 	require.Equal(t, int64(5690), txmp.SizeBytes())
-	require.GreaterOrEqual(t, len(reapedTxs), 15)
+	require.GreaterOrEqual(t, len(reapedTxs), 16)
 
 	// Reap by both transaction bytes and gas, where the size yields 31 reaped
 	// transactions and the gas limit reaps 25 transactions.
 	reapedTxs = txmp.ReapMaxBytesMaxGas(1500, 30)
 	ensurePrioritized(reapedTxs)
 	require.Equal(t, len(tTxs), txmp.Size())
 	require.Equal(t, int64(5690), txmp.SizeBytes())
-	require.Len(t, reapedTxs, 24)
+	require.Len(t, reapedTxs, 25)
 }
 
 func TestTxMempool_ReapMaxTxs(t *testing.T) {

diff --git a/node/node_test.go b/node/node_test.go
@@ -313,9 +313,7 @@ func TestCreateProposalBlock(t *testing.T) {
 
 	// check that the part set does not exceed the maximum block size
 	partSet := block.MakePartSet(partSize)
-	// TODO(ismail): properly fix this test
-	// https://github.com/tendermint/tendermint/issues/77
-	assert.Less(t, partSet.ByteSize(), int64(maxBytes)*2)
+	assert.Less(t, partSet.ByteSize(), int64(maxBytes))
 
 	partSetFromHeader := types.NewPartSetFromHeader(partSet.Header())
 	for partSetFromHeader.Count() < partSetFromHeader.Total() {
@@ -372,7 +370,7 @@ func TestMaxProposalBlockSize(t *testing.T) {
 
 	// fill the mempool with one txs just below the maximum size
 	txLength := int(types.MaxDataBytesNoEvidence(maxBytes, 1))
-	tx := tmrand.Bytes(txLength - 4 - 5) // to account for the varint
+	tx := tmrand.Bytes(txLength - 4)
 	err = mempool.CheckTx(tx, nil, mempl.TxInfo{})
 	assert.NoError(t, err)
 
@@ -393,9 +391,7 @@ func TestMaxProposalBlockSize(t *testing.T) {
 
 	pb, err := block.ToProto()
 	require.NoError(t, err)
-	// TODO(ismail): fix this test properly
-	// https://github.com/tendermint/tendermint/issues/77
-	assert.Less(t, int64(pb.Size()), maxBytes*2)
+	assert.Less(t, int64(pb.Size()), maxBytes)
 
 	// check that the part set does not exceed the maximum block size
 	partSet := block.MakePartSet(partSize)

diff --git a/proto/tendermint/types/block.pb.go b/proto/tendermint/types/block.pb.go