Follow-up fixes to main PrepareProposal implementation (#9162)

* -----start------ * [cherrypicked] state: panic on ResponsePrepareProposal validation error (#8145) * state: panic on ResponsePrepareProposal validation error * lint++ Co-authored-by: Sam Kleinman <garen@tychoish.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * [cherrypicked] abci++: remove CheckTx call from PrepareProposal flow (#8176) * [cherrypicked] abci++: correct max-size check to only operate on added and unmodified (#8242) * [cherrypicked] Remove `ModifiedTxStatus` from the spec and the code (#8210) * Outstanding abci-gen changes to 'pb.go' files * Removed modified_tx_status from spec and protobufs * Fix sed for OSX * Regenerated abci protobufs with 'abci-proto-gen' * Code changes. UTs e2e tests passing * Recovered UT: TestPrepareProposalModifiedTxStatusFalse * Adapted UT * Fixed UT * Revert "Fix sed for OSX" This reverts commit e576708. * Update internal/state/execution_test.go Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> * Update abci/example/kvstore/kvstore.go Co-authored-by: M. J. Fromberger <fromberger@interchain.io> * Update internal/state/execution_test.go Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> * Update spec/abci++/abci++_tmint_expected_behavior_002_draft.md Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> * Addressed some comments * Added one test that tests error at the ABCI client + Fixed some mock calls * Addressed remaining comments * Update abci/example/kvstore/kvstore.go Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> * Update abci/example/kvstore/kvstore.go Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> * Update abci/example/kvstore/kvstore.go Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> * Update spec/abci++/abci++_tmint_expected_behavior_002_draft.md Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> * Addressed William's latest comments * Adressed Michael's comment * Fixed UT * Some md fixes * More md fixes * gofmt Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> Co-authored-by: M. J. Fromberger <fromberger@interchain.io> * make proto-gen * Fixed testcase on PrepareProposal error * mockery Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> Co-authored-by: Sam Kleinman <garen@tychoish.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: M. J. Fromberger <fromberger@interchain.io>
tendermint · Aug 3, 2022 · b2409b3 · b2409b3
1 parent d268e56
commit b2409b3
Show file tree

Hide file tree

Showing 18 changed files with 347 additions and 186 deletions.
diff --git a/abci/example/kvstore/kvstore.go b/abci/example/kvstore/kvstore.go
@@ -170,8 +170,3 @@ func (app *Application) Query(reqQuery types.RequestQuery) (resQuery types.Respo
 
 	return resQuery
 }
-
-func (app *Application) PrepareProposal(
-	req types.RequestPrepareProposal) types.ResponsePrepareProposal {
-	return types.ResponsePrepareProposal{}
-}
diff --git a/abci/example/kvstore/persistent_kvstore.go b/abci/example/kvstore/persistent_kvstore.go
@@ -176,7 +176,7 @@ func (app *PersistentKVStoreApplication) ApplySnapshotChunk(
 
 func (app *PersistentKVStoreApplication) PrepareProposal(
 	req types.RequestPrepareProposal) types.ResponsePrepareProposal {
-	return types.ResponsePrepareProposal{TxRecords: app.substPrepareTx(req.Txs)}
+	return types.ResponsePrepareProposal{TxRecords: app.substPrepareTx(req.Txs, req.MaxTxBytes)}
 }
 
 //---------------------------------------------
@@ -310,28 +310,32 @@ func (app *PersistentKVStoreApplication) execPrepareTx(tx []byte) types.Response
 }
 
 // substPrepareTx substitutes all the transactions prefixed with 'prepare' in the
-// proposal for transactions with the prefix strips.
+// proposal for transactions with the prefix stripped.
 // It marks all of the original transactions as 'REMOVED' so that
 // Tendermint will remove them from its mempool.
-func (app *PersistentKVStoreApplication) substPrepareTx(blockData [][]byte) []*types.TxRecord {
-	trs := make([]*types.TxRecord, len(blockData))
+func (app *PersistentKVStoreApplication) substPrepareTx(blockData [][]byte, maxTxBytes int64) []*types.TxRecord {
+	trs := make([]*types.TxRecord, 0, len(blockData))
 	var removed []*types.TxRecord
-	for i, tx := range blockData {
+	var totalBytes int64
+	for _, tx := range blockData {
+		txMod := tx
+		action := types.TxRecord_UNMODIFIED
 		if isPrepareTx(tx) {
 			removed = append(removed, &types.TxRecord{
 				Tx:     tx,
 				Action: types.TxRecord_REMOVED,
 			})
-			trs[i] = &types.TxRecord{
-				Tx:     bytes.TrimPrefix(tx, []byte(PreparePrefix)),
-				Action: types.TxRecord_ADDED,
-			}
-			continue
+			txMod = bytes.TrimPrefix(tx, []byte(PreparePrefix))
+			action = types.TxRecord_ADDED
 		}
-		trs[i] = &types.TxRecord{
-			Tx:     tx,
-			Action: types.TxRecord_UNMODIFIED,
+		totalBytes += int64(len(txMod))
+		if totalBytes > maxTxBytes {
+			break
 		}
+		trs = append(trs, &types.TxRecord{
+			Tx:     txMod,
+			Action: action,
+		})
 	}
 
 	return append(trs, removed...)

diff --git a/abci/types/application.go b/abci/types/application.go
@@ -98,7 +98,19 @@ func (BaseApplication) ApplySnapshotChunk(req RequestApplySnapshotChunk) Respons
 }
 
 func (BaseApplication) PrepareProposal(req RequestPrepareProposal) ResponsePrepareProposal {
-	return ResponsePrepareProposal{}
+	trs := make([]*TxRecord, 0, len(req.Txs))
+	var totalBytes int64
+	for _, tx := range req.Txs {
+		totalBytes += int64(len(tx))
+		if totalBytes > req.MaxTxBytes {
+			break
+		}
+		trs = append(trs, &TxRecord{
+			Action: TxRecord_UNMODIFIED,
+			Tx:     tx,
+		})
+	}
+	return ResponsePrepareProposal{TxRecords: trs}
 }
 
 //-------------------------------------------------------

diff --git a/abci/types/types.pb.go b/abci/types/types.pb.go
diff --git a/consensus/mempool_test.go b/consensus/mempool_test.go
@@ -259,5 +259,18 @@ func (app *CounterApplication) Commit() abci.ResponseCommit {
 
 func (app *CounterApplication) PrepareProposal(
 	req abci.RequestPrepareProposal) abci.ResponsePrepareProposal {
-	return abci.ResponsePrepareProposal{}
+
+	trs := make([]*abci.TxRecord, 0, len(req.Txs))
+	var totalBytes int64
+	for _, tx := range req.Txs {
+		totalBytes += int64(len(tx))
+		if totalBytes > req.MaxTxBytes {
+			break
+		}
+		trs = append(trs, &abci.TxRecord{
+			Action: abci.TxRecord_UNMODIFIED,
+			Tx:     tx,
+		})
+	}
+	return abci.ResponsePrepareProposal{TxRecords: trs}
 }
diff --git a/consensus/state.go b/consensus/state.go
@@ -1224,7 +1224,11 @@ func (cs *State) createProposalBlock() (*types.Block, error) {
 
 	proposerAddr := cs.privValidatorPubKey.Address()
 
-	return cs.blockExec.CreateProposalBlock(cs.Height, cs.state, commit, proposerAddr, cs.LastCommit.GetVotes())
+	ret, err := cs.blockExec.CreateProposalBlock(cs.Height, cs.state, commit, proposerAddr, cs.LastCommit.GetVotes())
+	if err != nil {
+		panic(err)
+	}
+	return ret, nil
 }
 
 // Enter: `timeoutPropose` after entering Propose.

diff --git a/proto/tendermint/abci/types.proto b/proto/tendermint/abci/types.proto
@@ -298,8 +298,7 @@ message ResponseApplySnapshotChunk {
 }
 
 message ResponsePrepareProposal {
-  bool                             modified_tx             = 1;
-  repeated TxRecord                tx_records              = 2;
+  repeated TxRecord                tx_records              = 1;
 }
 
 //----------------------------------------

diff --git a/proxy/client.go b/proxy/client.go
@@ -11,6 +11,8 @@ import (
 	e2e "github.com/tendermint/tendermint/test/e2e/app"
 )
 
+//go:generate mockery --case underscore --name ClientCreator
+
 // ClientCreator creates new ABCI clients.
 type ClientCreator interface {
 	// NewABCIClient returns a new ABCI client.