diff --git a/CHANGELOG.md b/CHANGELOG.md index 120332450..151622329 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,12 +37,18 @@ 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. @@ -66,7 +72,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 index 10515bd52..48807f1ca 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -932,12 +932,14 @@ func (app *BaseApp) PrepareProposal(ctx context.Context, req *abci.RequestPrepar }, }, } - if app.prepareProposalState == nil { - app.setPrepareProposalState(header) - } else { - // In the first block, app.prepareProposalState.ctx will already be initialized + + if req.Height == 1 { + // In the first block, app.processProposalState.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() @@ -980,12 +982,14 @@ func (app *BaseApp) ProcessProposal(ctx context.Context, req *abci.RequestProces }, }, } - if app.processProposalState == nil { - app.setProcessProposalState(header) - } else { + + if req.Height == 1 { // 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 @@ -996,8 +1000,6 @@ 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 index c3e1dde6a..81428418e 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -12,6 +12,7 @@ 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) { @@ -200,3 +201,101 @@ 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}, + }) +}