From 8f592576c6cef1aa8bc9709d49a579636164ea2e Mon Sep 17 00:00:00 2001
From: William Banfield <wbanfield@gmail.com>
Date: Thu, 17 Mar 2022 16:25:38 -0400
Subject: [PATCH] state: panic on ResponsePrepareProposal validation error

---
 internal/state/execution.go      |  2 +-
 internal/state/execution_test.go | 58 ++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/internal/state/execution.go b/internal/state/execution.go
index 3423b00f6..e8c8c0f58 100644
--- a/internal/state/execution.go
+++ b/internal/state/execution.go
@@ -147,7 +147,7 @@ func (blockExec *BlockExecutor) CreateProposalBlock(
 	txrSet := types.NewTxRecordSet(rpp.TxRecords)
 
 	if err := txrSet.Validate(maxDataBytes, block.Txs); err != nil {
-		return nil, err
+		panic(fmt.Errorf("ResponsePrepareProposal validation: %w", err))
 	}
 
 	for _, rtx := range txrSet.RemovedTxs() {
diff --git a/internal/state/execution_test.go b/internal/state/execution_test.go
index e62735a4a..854adc47e 100644
--- a/internal/state/execution_test.go
+++ b/internal/state/execution_test.go
@@ -650,6 +650,64 @@ func TestEmptyPrepareProposal(t *testing.T) {
 	require.NoError(t, err)
 }
 
+// TestPrepareProposalPanicOnInvalid tests that the block creation logic panics
+// if the ResponsePrepareProposal returned from the application is invalid.
+func TestPrepareProposalPanicOnInvalid(t *testing.T) {
+	const height = 2
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	logger := log.TestingLogger()
+	eventBus := eventbus.NewDefault(logger)
+	require.NoError(t, eventBus.Start(ctx))
+
+	state, stateDB, privVals := makeState(t, 1, height)
+	stateStore := sm.NewStore(stateDB)
+
+	evpool := &mocks.EvidencePool{}
+	evpool.On("PendingEvidence", mock.Anything).Return([]types.Evidence{}, int64(0))
+
+	mp := &mpmocks.Mempool{}
+	mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything).Return(types.Txs{})
+
+	app := abcimocks.NewBaseMock()
+
+	// create an invalid ResponsePrepareProposal
+	rpp := abci.ResponsePrepareProposal{
+		ModifiedTx: true,
+		TxRecords: []*abci.TxRecord{
+			{
+				Action: abci.TxRecord_REMOVED,
+				Tx:     []byte("new tx"),
+			},
+		},
+	}
+	app.On("PrepareProposal", mock.Anything).Return(rpp, nil)
+
+	cc := abciclient.NewLocalClient(logger, app)
+	proxyApp := proxy.New(cc, logger, proxy.NopMetrics())
+	err := proxyApp.Start(ctx)
+	require.NoError(t, err)
+
+	blockExec := sm.NewBlockExecutor(
+		stateStore,
+		logger,
+		proxyApp,
+		mp,
+		evpool,
+		nil,
+		eventBus,
+	)
+	pa, _ := state.Validators.GetByIndex(0)
+	commit := makeValidCommit(ctx, t, height, types.BlockID{}, state.Validators, privVals)
+	require.Panics(t,
+		func() {
+			blockExec.CreateProposalBlock(ctx, height, state, commit, pa, nil)
+		})
+
+	mp.AssertExpectations(t)
+}
+
 // TestPrepareProposalRemoveTxs tests that any transactions marked as REMOVED
 // are not included in the block produced by CreateProposalBlock. The test also
 // ensures that any transactions removed are also removed from the mempool.