From 3fc4f33769ccf91289fd228e6a83d21eb8a97e5a Mon Sep 17 00:00:00 2001 From: Facundo Medica <14063057+facundomedica@users.noreply.github.com> Date: Sun, 8 Jan 2023 16:29:40 -0300 Subject: [PATCH] fix: use deliverState in prepare and processProposal at height 1 (#14505) --- CHANGELOG.md | 3 ++- baseapp/abci.go | 32 +++++++++++++++++++++--- baseapp/abci_test.go | 58 +++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 85 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 987aff34855c..e44d9111f9a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -217,7 +217,8 @@ extension interfaces. `module.Manager.Modules` is now of type `map[string]interf ### Bug Fixes -* (x/group) [#](https://github.com/cosmos/cosmos-sdk/pull) Fix wrong address set in `EventUpdateGroupPolicy`. +* (baseapp) [#14505](https://github.com/cosmos/cosmos-sdk/pull/14505) PrepareProposal and ProcessProposal now use deliverState for the first block in order to access changes made in InitChain. +* (x/group) (x/group) [#14526](https://github.com/cosmos/cosmos-sdk/pull/14526) Fix wrong address set in `EventUpdateGroupPolicy`. * (server) [#14441](https://github.com/cosmos/cosmos-sdk/pull/14441) Fix `--log_format` flag not working. * (x/upgrade) [#13936](https://github.com/cosmos/cosmos-sdk/pull/13936) Make downgrade verification work again * (x/group) [#13742](https://github.com/cosmos/cosmos-sdk/pull/13742) Fix `validate-genesis` when group policy accounts exist. diff --git a/baseapp/abci.go b/baseapp/abci.go index e704c386facc..aacfcb77ed64 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -248,11 +248,24 @@ func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBloc // Ref: https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-060-abci-1.0.md // Ref: https://github.com/tendermint/tendermint/blob/main/spec/abci/abci%2B%2B_basic_concepts.md func (app *BaseApp) PrepareProposal(req abci.RequestPrepareProposal) (resp abci.ResponsePrepareProposal) { - ctx := app.getContextForTx(runTxPrepareProposal, []byte{}) if app.prepareProposal == nil { panic("PrepareProposal method not set") } + // Tendermint must never call PrepareProposal with a height of 0. + // Ref: https://github.com/tendermint/tendermint/blob/059798a4f5b0c9f52aa8655fa619054a0154088c/spec/core/state.md?plain=1#L37-L38 + if req.Height < 1 { + panic("PrepareProposal called with invalid height") + } + + ctx := app.getContextForProposal(app.prepareProposalState.ctx, req.Height) + + ctx = ctx.WithVoteInfos(app.voteInfos). + WithBlockHeight(req.Height). + WithBlockTime(req.Time). + WithProposer(req.ProposerAddress). + WithConsensusParams(app.GetConsensusParams(ctx)) + defer func() { if err := recover(); err != nil { app.logger.Error( @@ -289,13 +302,15 @@ func (app *BaseApp) ProcessProposal(req abci.RequestProcessProposal) (resp abci. panic("app.ProcessProposal is not set") } - ctx := app.processProposalState.ctx. + ctx := app.getContextForProposal(app.processProposalState.ctx, req.Height) + + ctx = ctx. WithVoteInfos(app.voteInfos). WithBlockHeight(req.Height). WithBlockTime(req.Time). WithHeaderHash(req.Hash). WithProposer(req.ProposerAddress). - WithConsensusParams(app.GetConsensusParams(app.processProposalState.ctx)) + WithConsensusParams(app.GetConsensusParams(ctx)) defer func() { if err := recover(); err != nil { @@ -928,3 +943,14 @@ func SplitABCIQueryPath(requestPath string) (path []string) { return path } + +// getContextForProposal returns the right context for PrepareProposal and +// ProcessProposal. We use deliverState on the first block to be able to access +// any state changes made in InitChain. +func (app *BaseApp) getContextForProposal(ctx sdk.Context, height int64) sdk.Context { + if height == 1 { + ctx, _ = app.deliverState.ctx.CacheContext() + return ctx + } + return ctx +} diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 460c9c6cb16f..3675c3ea2046 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -1323,10 +1323,6 @@ func TestABCI_Proposal_HappyPath(t *testing.T) { ConsensusParams: &tmproto.ConsensusParams{}, }) - suite.baseApp.BeginBlock(abci.RequestBeginBlock{ - Header: tmproto.Header{Height: suite.baseApp.LastBlockHeight() + 1}, - }) - tx := newTxCounter(t, suite.txConfig, 0, 1) txBytes, err := suite.txConfig.TxEncoder()(tx) require.NoError(t, err) @@ -1347,6 +1343,7 @@ func TestABCI_Proposal_HappyPath(t *testing.T) { reqPrepareProposal := abci.RequestPrepareProposal{ MaxTxBytes: 1000, + Height: 1, } resPrepareProposal := suite.baseApp.PrepareProposal(reqPrepareProposal) require.Equal(t, 2, len(resPrepareProposal.Txs)) @@ -1362,6 +1359,10 @@ func TestABCI_Proposal_HappyPath(t *testing.T) { resProcessProposal := suite.baseApp.ProcessProposal(reqProcessProposal) require.Equal(t, abci.ResponseProcessProposal_ACCEPT, resProcessProposal.Status) + suite.baseApp.BeginBlock(abci.RequestBeginBlock{ + Header: tmproto.Header{Height: suite.baseApp.LastBlockHeight() + 1}, + }) + res := suite.baseApp.DeliverTx(abci.RequestDeliverTx{Tx: txBytes}) require.Equal(t, 1, pool.CountTx()) @@ -1369,6 +1370,51 @@ func TestABCI_Proposal_HappyPath(t *testing.T) { require.True(t, res.IsOK(), fmt.Sprintf("%v", res)) } +func TestABCI_Proposal_Read_State_PrepareProposal(t *testing.T) { + someKey := []byte("some-key") + + setInitChainerOpt := func(bapp *baseapp.BaseApp) { + bapp.SetInitChainer(func(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain { + ctx.KVStore(capKey1).Set(someKey, []byte("foo")) + return abci.ResponseInitChain{} + }) + } + + prepareOpt := func(bapp *baseapp.BaseApp) { + bapp.SetPrepareProposal(func(ctx sdk.Context, req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { + value := ctx.KVStore(capKey1).Get(someKey) + // We should be able to access any state written in InitChain + require.Equal(t, "foo", string(value)) + return abci.ResponsePrepareProposal{Txs: req.Txs} + }) + } + + suite := NewBaseAppSuite(t, setInitChainerOpt, prepareOpt) + + suite.baseApp.InitChain(abci.RequestInitChain{ + ConsensusParams: &tmproto.ConsensusParams{}, + }) + + reqPrepareProposal := abci.RequestPrepareProposal{ + MaxTxBytes: 1000, + Height: 1, // this value can't be 0 + } + resPrepareProposal := suite.baseApp.PrepareProposal(reqPrepareProposal) + require.Equal(t, 0, len(resPrepareProposal.Txs)) + + reqProposalTxBytes := [][]byte{} + reqProcessProposal := abci.RequestProcessProposal{ + Txs: reqProposalTxBytes, + } + + resProcessProposal := suite.baseApp.ProcessProposal(reqProcessProposal) + require.Equal(t, abci.ResponseProcessProposal_ACCEPT, resProcessProposal.Status) + + suite.baseApp.BeginBlock(abci.RequestBeginBlock{ + Header: tmproto.Header{Height: suite.baseApp.LastBlockHeight() + 1}, + }) +} + func TestABCI_PrepareProposal_ReachedMaxBytes(t *testing.T) { anteKey := []byte("ante-key") pool := mempool.NewSenderNonceMempool() @@ -1389,6 +1435,7 @@ func TestABCI_PrepareProposal_ReachedMaxBytes(t *testing.T) { reqPrepareProposal := abci.RequestPrepareProposal{ MaxTxBytes: 1500, + Height: 1, } resPrepareProposal := suite.baseApp.PrepareProposal(reqPrepareProposal) require.Equal(t, 10, len(resPrepareProposal.Txs)) @@ -1412,6 +1459,7 @@ func TestABCI_PrepareProposal_BadEncoding(t *testing.T) { reqPrepareProposal := abci.RequestPrepareProposal{ MaxTxBytes: 1000, + Height: 1, } resPrepareProposal := suite.baseApp.PrepareProposal(reqPrepareProposal) require.Equal(t, 1, len(resPrepareProposal.Txs)) @@ -1449,6 +1497,7 @@ func TestABCI_PrepareProposal_Failures(t *testing.T) { req := abci.RequestPrepareProposal{ MaxTxBytes: 1000, + Height: 1, } res := suite.baseApp.PrepareProposal(req) require.Equal(t, 1, len(res.Txs)) @@ -1468,6 +1517,7 @@ func TestABCI_PrepareProposal_PanicRecovery(t *testing.T) { req := abci.RequestPrepareProposal{ MaxTxBytes: 1000, + Height: 1, } require.NotPanics(t, func() {