From ce0215c435a356e2b399827c934a448fa47ee476 Mon Sep 17 00:00:00 2001
From: lasaro <lasaro@informal.systems>
Date: Wed, 6 Dec 2023 14:52:41 -0300
Subject: [PATCH] Add test missing in #1687 (#1712)

* Experimenting the fix from lx-xiang

* Fixes name in the example

* Reverts fix, so it is merged from the proper branch.

* Adds a test that fails because the validate is wrong and accepts a block that is larger than it should.

* Add changelog for the original PR

* Update internal/state/execution_test.go
---
 .../1687-consensus-fix-block-validation.md    |  3 +
 abci/example/kvstore/kvstore.go               |  4 +-
 internal/state/execution_test.go              | 57 +++++++++++++++++++
 3 files changed, 62 insertions(+), 2 deletions(-)
 create mode 100644 .changelog/unreleased/bug-fixes/1687-consensus-fix-block-validation.md

diff --git a/.changelog/unreleased/bug-fixes/1687-consensus-fix-block-validation.md b/.changelog/unreleased/bug-fixes/1687-consensus-fix-block-validation.md
new file mode 100644
index 00000000000..778f0b538b4
--- /dev/null
+++ b/.changelog/unreleased/bug-fixes/1687-consensus-fix-block-validation.md
@@ -0,0 +1,3 @@
+- `[mempool]` The calculation method of tx size returned by calling proxyapp should be consistent with that of mempool
+  ([\#1687](https://github.com/cometbft/cometbft/pull/1687))
+
diff --git a/abci/example/kvstore/kvstore.go b/abci/example/kvstore/kvstore.go
index 81421105249..f46ce718b56 100644
--- a/abci/example/kvstore/kvstore.go
+++ b/abci/example/kvstore/kvstore.go
@@ -156,10 +156,10 @@ func isValidTx(tx []byte) bool {
 	return false
 }
 
-// PrepareProposal is called when the node is a proposer. Tendermint stages a set of transactions to the application. As the
+// PrepareProposal is called when the node is a proposer. CometBFT stages a set of transactions to the application. As the
 // KVStore has two accepted formats, `:` and `=`, we modify all instances of `:` with `=` to make it consistent. Note: this is
 // quite a trivial example of transaction modification.
-// NOTE: we assume that Tendermint will never provide more transactions than can fit in a block.
+// NOTE: we assume that CometBFT will never provide more transactions than can fit in a block.
 func (app *Application) PrepareProposal(ctx context.Context, req *types.PrepareProposalRequest) (*types.PrepareProposalResponse, error) {
 	return &types.PrepareProposalResponse{Txs: app.formatTxs(ctx, req.Txs)}, nil
 }
diff --git a/internal/state/execution_test.go b/internal/state/execution_test.go
index 55445cf0b2d..396be7c9cae 100644
--- a/internal/state/execution_test.go
+++ b/internal/state/execution_test.go
@@ -908,6 +908,63 @@ func TestPrepareProposalErrorOnTooManyTxs(t *testing.T) {
 	mp.AssertExpectations(t)
 }
 
+// TestPrepareProposalCountSerializationOverhead tests that the block creation logic returns
+// an error if the ResponsePrepareProposal returned from the application is at the limit of
+// its size and will go beyond the limit upon serialization.
+func TestPrepareProposalCountSerializationOverhead(t *testing.T) {
+	const height = 2
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	state, stateDB, privVals := makeState(1, height)
+	// limit max block size
+	var bytesPerTx int64 = 4
+	const nValidators = 1
+	nonDataSize := 5000 - types.MaxDataBytes(5000, 0, nValidators)
+	state.ConsensusParams.Block.MaxBytes = bytesPerTx*1024 + nonDataSize
+	maxDataBytes := types.MaxDataBytes(state.ConsensusParams.Block.MaxBytes, 0, nValidators)
+
+	stateStore := sm.NewStore(stateDB, sm.StoreOptions{
+		DiscardABCIResponses: false,
+	})
+
+	evpool := &mocks.EvidencePool{}
+	evpool.On("PendingEvidence", mock.Anything).Return([]types.Evidence{}, int64(0))
+
+	txs := test.MakeNTxs(height, maxDataBytes/bytesPerTx)
+	mp := &mpmocks.Mempool{}
+	mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything).Return(txs)
+
+	app := &abcimocks.Application{}
+	app.On("PrepareProposal", mock.Anything, mock.Anything).Return(&abci.PrepareProposalResponse{
+		Txs: txs.ToSliceOfBytes(),
+	}, nil)
+
+	cc := proxy.NewLocalClientCreator(app)
+	proxyApp := proxy.NewAppConns(cc, proxy.NopMetrics())
+	err := proxyApp.Start()
+	require.NoError(t, err)
+	defer proxyApp.Stop() //nolint:errcheck // ignore for tests
+
+	blockStore := store.NewBlockStore(dbm.NewMemDB())
+	blockExec := sm.NewBlockExecutor(
+		stateStore,
+		log.NewNopLogger(),
+		proxyApp.Consensus(),
+		mp,
+		evpool,
+		blockStore,
+	)
+	pa, _ := state.Validators.GetByIndex(0)
+	commit, _, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals)
+	require.NoError(t, err)
+	block, err := blockExec.CreateProposalBlock(ctx, height, state, commit, pa)
+	require.Nil(t, block)
+	require.ErrorContains(t, err, "transaction data size exceeds maximum")
+
+	mp.AssertExpectations(t)
+}
+
 // TestPrepareProposalErrorOnPrepareProposalError tests when the client returns an error
 // upon calling PrepareProposal on it.
 func TestPrepareProposalErrorOnPrepareProposalError(t *testing.T) {