From c12b50b8d4b3ee67b16b97237934d87ba5c97f9b Mon Sep 17 00:00:00 2001
From: Sergio Mena <sergio@informal.systems>
Date: Sun, 12 Mar 2023 18:42:39 +0100
Subject: [PATCH 1/2] Review and fix propoal coherence in all applications in
 the code base

---
 abci/example/kvstore/kvstore.go |  2 +-
 state/helpers_test.go           | 24 +++++++++++++++++++++++-
 test/e2e/app/app.go             |  2 ++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/abci/example/kvstore/kvstore.go b/abci/example/kvstore/kvstore.go
index f0e0bf9c1cf..a23bf98c69c 100644
--- a/abci/example/kvstore/kvstore.go
+++ b/abci/example/kvstore/kvstore.go
@@ -173,7 +173,7 @@ func (app *Application) formatTxs(ctx context.Context, blockData [][]byte) [][]b
 func (app *Application) ProcessProposal(ctx context.Context, req *types.RequestProcessProposal) (*types.ResponseProcessProposal, error) {
 	for _, tx := range req.Txs {
 		// As CheckTx is a full validity check we can simply reuse this
-		if resp, err := app.CheckTx(ctx, &types.RequestCheckTx{Tx: tx}); resp.Code != CodeTypeOK || err != nil {
+		if resp, err := app.CheckTx(ctx, &types.RequestCheckTx{Tx: tx}); err != nil || resp.Code != CodeTypeOK {
 			return &types.ResponseProcessProposal{Status: types.ResponseProcessProposal_REJECT}, nil
 		}
 	}
diff --git a/state/helpers_test.go b/state/helpers_test.go
index 4c6fd5813fc..f9631d3728e 100644
--- a/state/helpers_test.go
+++ b/state/helpers_test.go
@@ -274,7 +274,29 @@ func (app *testApp) Commit(_ context.Context, _ *abci.RequestCommit) (*abci.Resp
 	return &abci.ResponseCommit{RetainHeight: 1}, nil
 }
 
-func (app *testApp) ProcessProposal(_ context.Context, req *abci.RequestProcessProposal) (*abci.ResponseProcessProposal, error) {
+func (app *testApp) PrepareProposal(
+	_ context.Context,
+	req *abci.RequestPrepareProposal,
+) (*abci.ResponsePrepareProposal, error) {
+	txs := make([][]byte, 0, len(req.Txs))
+	var totalBytes int64
+	for _, tx := range req.Txs {
+		if len(tx) == 0 {
+			continue
+		}
+		totalBytes += int64(len(tx))
+		if totalBytes > req.MaxTxBytes {
+			break
+		}
+		txs = append(txs, tx)
+	}
+	return &abci.ResponsePrepareProposal{Txs: txs}, nil
+}
+
+func (app *testApp) ProcessProposal(
+	_ context.Context,
+	req *abci.RequestProcessProposal,
+) (*abci.ResponseProcessProposal, error) {
 	for _, tx := range req.Txs {
 		if len(tx) == 0 {
 			return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil
diff --git a/test/e2e/app/app.go b/test/e2e/app/app.go
index 3ee46697e07..a6bff325cd8 100644
--- a/test/e2e/app/app.go
+++ b/test/e2e/app/app.go
@@ -336,6 +336,7 @@ func (app *Application) PrepareProposal(
 			if strings.HasPrefix(string(tx), extTxPrefix) {
 				continue
 			}
+			// Coherence: No need to call parseTx, as the check is stateless and has been performed by CheckTx
 			txs[i] = tx
 			totalBytes += int64(len(tx))
 		}
@@ -360,6 +361,7 @@ func (app *Application) PrepareProposal(
 		if totalBytes > req.MaxTxBytes {
 			break
 		}
+		// Coherence: No need to call parseTx, as the check is stateless and has been performed by CheckTx
 		txs = append(txs, tx)
 	}
 

From 564569f03a8963c4236f4409e20e2ffc1d68b77b Mon Sep 17 00:00:00 2001
From: Jasmina Malicevic <jasmina.dustinac@gmail.com>
Date: Tue, 14 Mar 2023 10:32:08 +0100
Subject: [PATCH 2/2] Fixed comment for Commit

---
 state/execution.go | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/state/execution.go b/state/execution.go
index 02502c0c4bc..a6773d2dbd1 100644
--- a/state/execution.go
+++ b/state/execution.go
@@ -340,7 +340,10 @@ func (blockExec *BlockExecutor) VerifyVoteExtension(ctx context.Context, vote *t
 
 // Commit locks the mempool, runs the ABCI Commit message, and updates the
 // mempool.
-// It returns the result of calling abci.Commit (the AppHash) and the height to retain (if any).
+// It returns the result of calling abci.Commit which is the height to retain (if any)).
+// The application is expected to have persisted its state (if any) before returning
+// from the ABCI Commit call. This is the only place where the application should
+// persist its state.
 // The Mempool must be locked during commit and update because state is
 // typically reset on Commit and old txs must be replayed against committed
 // state before new txs are run in the mempool, lest they be invalid.