Revert "Always reset state between proposals (#285)" (#292)

This reverts commit db47ec3. ## Describe your changes and provide context With optimistic processing this is not applicable on our side ## Testing performed to validate your change Launched a cluster with 3.0.3, with only this change applied and see that it app hashes ![image](https://github.com/sei-protocol/sei-cosmos/assets/18161326/5d6b0606-aba5-4556-b2c9-6417572eee26)
sei-protocol · Jun 15, 2023 · 7574c7c · 7574c7c
1 parent ed659fe
commit 7574c7c
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 119 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -37,18 +37,12 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
-
-## Pending
-
-### Bugfixes
-* (baseapp) [#285](https://github.com/sei-protocol/sei-cosmos/pull/285) Reset state before calling PrepareProposal and ProcessProposal. Copy of [#15487](https://github.com/cosmos/cosmos-sdk/pull/15487)
-
 ## v0.45.9 - 2022-10-14
 
 ATTENTION:
 
-This is a security release for the
-[Dragonberry security advisory](https://forum.cosmos.network/t/ibc-security-advisory-dragonberry/7702).
+This is a security release for the 
+[Dragonberry security advisory](https://forum.cosmos.network/t/ibc-security-advisory-dragonberry/7702). 
 
 All users should upgrade immediately.
 
@@ -72,7 +66,7 @@ replace (
 * [#13323](https://github.com/cosmos/cosmos-sdk/pull/13323) Ensure `withdraw_rewards` rewards are emitted from all actions that result in rewards being withdrawn.
 * [#13321](https://github.com/cosmos/cosmos-sdk/pull/13321) Add flag to disable fast node migration and usage.
 * (store) [#13326](https://github.com/cosmos/cosmos-sdk/pull/13326) Implementation of ADR-038 file StreamingService, backport #8664.
-* (store) [#13540](https://github.com/cosmos/cosmos-sdk/pull/13540) Default fastnode migration to false to prevent suprises. Operators must enable it, unless they have it enabled already.
+* (store) [#13540](https://github.com/cosmos/cosmos-sdk/pull/13540) Default fastnode migration to false to prevent suprises. Operators must enable it, unless they have it enabled already. 
 
 ### API Breaking Changes
 

diff --git a/baseapp/abci.go b/baseapp/abci.go
@@ -932,14 +932,12 @@ func (app *BaseApp) PrepareProposal(ctx context.Context, req *abci.RequestPrepar
 			},
 		},
 	}
-
-	if req.Height == 1 {
-		// In the first block, app.processProposalState.ctx will already be initialized
+	if app.prepareProposalState == nil {
+		app.setPrepareProposalState(header)
+	} else {
+		// In the first block, app.prepareProposalState.ctx will already be initialized
 		// by InitChain. Context is now updated with Header information.
 		app.setPrepareProposalHeader(header)
-	} else {
-		// always reset state given that ProcessProposal can timeout and be called again
-		app.setPrepareProposalState(header)
 	}
 
 	app.preparePrepareProposalState()
@@ -982,14 +980,12 @@ func (app *BaseApp) ProcessProposal(ctx context.Context, req *abci.RequestProces
 			},
 		},
 	}
-
-	if req.Height == 1 {
+	if app.processProposalState == nil {
+		app.setProcessProposalState(header)
+	} else {
 		// In the first block, app.processProposalState.ctx will already be initialized
 		// by InitChain. Context is now updated with Header information.
 		app.setProcessProposalHeader(header)
-	} else {
-		// always reset state given that ProcessProposal can timeout and be called again
-		app.setProcessProposalState(header)
 	}
 
 	// add block gas meter
@@ -1000,6 +996,8 @@ func (app *BaseApp) ProcessProposal(ctx context.Context, req *abci.RequestProces
 		gasMeter = sdk.NewInfiniteGasMeter()
 	}
 
+	// NOTE: header hash is not set in NewContext, so we manually set it here
+
 	app.prepareProcessProposalState(gasMeter, req.Hash)
 
 	if app.processProposalHandler != nil {

diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go
@@ -12,7 +12,6 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/testutil"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	"github.com/cosmos/cosmos-sdk/utils"
 )
 
 func TestGetBlockRentionHeight(t *testing.T) {
@@ -201,101 +200,3 @@ func (ps *paramStore) Get(_ sdk.Context, key []byte, ptr interface{}) {
 		panic(err)
 	}
 }
-
-// TestABCI_Proposal_Reset_State ensures that state is reset between runs of
-// PrepareProposal and ProcessProposal in case they are called multiple times.
-// This is only valid for heights > 1, given that on height 1 we always set the
-// state to be deliverState.
-func TestABCI_Proposal_Reset_State_Between_Calls(t *testing.T) {
-	someKey := []byte("some-key")
-
-	prepareOpt := func(bapp *BaseApp) {
-		bapp.SetPrepareProposalHandler(func(ctx sdk.Context, req *abci.RequestPrepareProposal) (*abci.ResponsePrepareProposal, error) {
-			// This key should not exist given that we reset the state on every call.
-			require.False(t, ctx.KVStore(capKey1).Has(someKey))
-			ctx.KVStore(capKey1).Set(someKey, someKey)
-			return &abci.ResponsePrepareProposal{
-				TxRecords: utils.Map(req.Txs, func(tx []byte) *abci.TxRecord {
-					return &abci.TxRecord{Action: abci.TxRecord_UNMODIFIED, Tx: tx}
-				}),
-			}, nil
-		})
-	}
-
-	processOpt := func(bapp *BaseApp) {
-		bapp.SetProcessProposalHandler(func(ctx sdk.Context, req *abci.RequestProcessProposal) (*abci.ResponseProcessProposal, error) {
-			// This key should not exist given that we reset the state on every call.
-			require.False(t, ctx.KVStore(capKey1).Has(someKey))
-			ctx.KVStore(capKey1).Set(someKey, someKey)
-			return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}, nil
-		})
-	}
-
-	app := setupBaseApp(
-		t,
-		processOpt,
-		prepareOpt,
-	)
-
-	_, err := app.InitChain(context.Background(), &abci.RequestInitChain{
-		Validators: []abci.ValidatorUpdate{},
-		// ConsensusParams: simapp.DefaultConsensusParams,
-		// AppStateBytes:   stateBytes,
-	})
-	ctx := app.NewContext(false, tmproto.Header{})
-
-	require.Nil(t, err)
-
-	// Genesis at height 1
-	reqPrepareProposal := abci.RequestPrepareProposal{
-		MaxTxBytes: 1000,
-		Height:     1, // this value can't be 0
-	}
-
-	resPrepareProposal, _ := app.PrepareProposal(ctx.Context(), &reqPrepareProposal)
-	require.Equal(t, 0, len(resPrepareProposal.TxRecords))
-
-	reqProposalTxBytes := [][]byte{}
-	reqProcessProposal := abci.RequestProcessProposal{
-		Txs:    reqProposalTxBytes,
-		Height: 1,
-	}
-
-	resProcessProposal, _ := app.ProcessProposal(ctx.Context(), &reqProcessProposal)
-	require.Equal(t, abci.ResponseProcessProposal_ACCEPT, resProcessProposal.Status)
-
-	app.BeginBlock(ctx, abci.RequestBeginBlock{
-		Header: tmproto.Header{Height: app.LastBlockHeight() + 1},
-	})
-
-	// Post Genesis
-
-	reqPrepareProposal = abci.RequestPrepareProposal{
-		MaxTxBytes: 1000,
-		Height:     2, // this value can't be 0
-	}
-
-	// Let's pretend something happened and PrepareProposal gets called many
-	// times, this must be safe to do.
-	for i := 0; i < 5; i++ {
-		resPrepareProposal, _ := app.PrepareProposal(ctx.Context(), &reqPrepareProposal)
-		require.Equal(t, 0, len(resPrepareProposal.TxRecords))
-	}
-
-	reqProposalTxBytes = [][]byte{}
-	reqProcessProposal = abci.RequestProcessProposal{
-		Txs:    reqProposalTxBytes,
-		Height: 2,
-	}
-
-	// Let's pretend something happened and ProcessProposal gets called many
-	// times, this must be safe to do.
-	for i := 0; i < 5; i++ {
-		resProcessProposal, _ := app.ProcessProposal(ctx.Context(), &reqProcessProposal)
-		require.Equal(t, abci.ResponseProcessProposal_ACCEPT, resProcessProposal.Status)
-	}
-
-	app.BeginBlock(ctx, abci.RequestBeginBlock{
-		Header: tmproto.Header{Height: app.LastBlockHeight() + 1},
-	})
-}