From 713bce4fef32120160aadc6d518a794d2c63ef00 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 22 Aug 2023 09:02:59 +0800 Subject: [PATCH 1/9] reimplement PreFinalizeBlockHook as PreBlocker --- baseapp/abci.go | 8 +------- baseapp/abci_test.go | 18 +++++++++++------- baseapp/baseapp.go | 25 ++++++++++++------------- baseapp/options.go | 8 -------- runtime/app.go | 2 +- simapp/app.go | 2 +- types/abci.go | 10 +--------- 7 files changed, 27 insertions(+), 46 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index dd1ceb5a1f41..3e4801a8a0b0 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -730,13 +730,7 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons WithHeaderHash(req.Hash) } - if app.preFinalizeBlockHook != nil { - if err := app.preFinalizeBlockHook(app.finalizeBlockState.ctx, req); err != nil { - return nil, err - } - } - - if err := app.preBlock(); err != nil { + if err := app.preBlock(req); err != nil { return nil, err } diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index a86e2ec3880c..8ffdf10ed77a 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -2035,7 +2035,7 @@ func TestABCI_HaltChain(t *testing.T) { } } -func TestBaseApp_PreFinalizeBlockHook(t *testing.T) { +func TestBaseApp_PreBlocker(t *testing.T) { db := dbm.NewMemDB() name := t.Name() logger := log.NewTestLogger(t) @@ -2045,9 +2045,11 @@ func TestBaseApp_PreFinalizeBlockHook(t *testing.T) { require.NoError(t, err) wasHookCalled := false - app.SetPreFinalizeBlockHook(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) error { + app.SetPreBlocker(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) (sdk.ResponsePreBlock, error) { wasHookCalled = true - return nil + return sdk.ResponsePreBlock{ + ConsensusParamsChanged: true, + }, nil }) app.Seal() @@ -2060,8 +2062,8 @@ func TestBaseApp_PreFinalizeBlockHook(t *testing.T) { _, err = app.InitChain(&abci.RequestInitChain{}) require.NoError(t, err) - app.SetPreFinalizeBlockHook(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) error { - return errors.New("some error") + app.SetPreBlocker(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) (sdk.ResponsePreBlock, error) { + return sdk.ResponsePreBlock{}, errors.New("some error") }) app.Seal() @@ -2122,7 +2124,7 @@ func TestBaseApp_VoteExtensions(t *testing.T) { return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}, nil }) - app.SetPreFinalizeBlockHook(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) error { + app.SetPreBlocker(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) (sdk.ResponsePreBlock, error) { count := uint64(0) pricesSum := uint64(0) for _, v := range req.Txs { @@ -2141,7 +2143,9 @@ func TestBaseApp_VoteExtensions(t *testing.T) { ctx.KVStore(capKey1).Set([]byte("avgPrice"), buf) } - return nil + return sdk.ResponsePreBlock{ + ConsensusParamsChanged: true, + }, nil }) } diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index f59ee0d582b4..9b11f21a770b 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -74,17 +74,16 @@ type BaseApp struct { anteHandler sdk.AnteHandler // ante handler for fee and auth postHandler sdk.PostHandler // post handler, optional, e.g. for tips - initChainer sdk.InitChainer // ABCI InitChain handler - preBlocker sdk.PreBlocker // logic to run before BeginBlocker - beginBlocker sdk.BeginBlocker // (legacy ABCI) BeginBlock handler - endBlocker sdk.EndBlocker // (legacy ABCI) EndBlock handler - processProposal sdk.ProcessProposalHandler // ABCI ProcessProposal handler - prepareProposal sdk.PrepareProposalHandler // ABCI PrepareProposal - extendVote sdk.ExtendVoteHandler // ABCI ExtendVote handler - verifyVoteExt sdk.VerifyVoteExtensionHandler // ABCI VerifyVoteExtension handler - prepareCheckStater sdk.PrepareCheckStater // logic to run during commit using the checkState - precommiter sdk.Precommiter // logic to run during commit using the deliverState - preFinalizeBlockHook sdk.PreFinalizeBlockHook // logic to run before FinalizeBlock + initChainer sdk.InitChainer // ABCI InitChain handler + preBlocker sdk.PreBlocker // logic to run before BeginBlocker + beginBlocker sdk.BeginBlocker // (legacy ABCI) BeginBlock handler + endBlocker sdk.EndBlocker // (legacy ABCI) EndBlock handler + processProposal sdk.ProcessProposalHandler // ABCI ProcessProposal handler + prepareProposal sdk.PrepareProposalHandler // ABCI PrepareProposal + extendVote sdk.ExtendVoteHandler // ABCI ExtendVote handler + verifyVoteExt sdk.VerifyVoteExtensionHandler // ABCI VerifyVoteExtension handler + prepareCheckStater sdk.PrepareCheckStater // logic to run during commit using the checkState + precommiter sdk.Precommiter // logic to run during commit using the deliverState addrPeerFilter sdk.PeerFilter // filter peers by address and port idPeerFilter sdk.PeerFilter // filter peers by node ID @@ -669,10 +668,10 @@ func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) (sdk.Context return ctx.WithMultiStore(msCache), msCache } -func (app *BaseApp) preBlock() error { +func (app *BaseApp) preBlock(req *abci.RequestFinalizeBlock) error { if app.preBlocker != nil { ctx := app.finalizeBlockState.ctx - rsp, err := app.preBlocker(ctx) + rsp, err := app.preBlocker(ctx, req) if err != nil { return err } diff --git a/baseapp/options.go b/baseapp/options.go index 29e517899316..1046ad3a42f3 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -216,14 +216,6 @@ func (app *BaseApp) SetPrecommiter(precommiter sdk.Precommiter) { app.precommiter = precommiter } -func (app *BaseApp) SetPreFinalizeBlockHook(hook sdk.PreFinalizeBlockHook) { - if app.sealed { - panic("SetPreFinalizeBlockHook() on sealed BaseApp") - } - - app.preFinalizeBlockHook = hook -} - func (app *BaseApp) SetAnteHandler(ah sdk.AnteHandler) { if app.sealed { panic("SetAnteHandler() on sealed BaseApp") diff --git a/runtime/app.go b/runtime/app.go index ed6de6adb9da..ab4630e4828c 100644 --- a/runtime/app.go +++ b/runtime/app.go @@ -153,7 +153,7 @@ func (a *App) Load(loadLatest bool) error { } // PreBlocker application updates every pre block -func (a *App) PreBlocker(ctx sdk.Context) (sdk.ResponsePreBlock, error) { +func (a *App) PreBlocker(ctx sdk.Context, _ *abci.RequestFinalizeBlock) (sdk.ResponsePreBlock, error) { return a.ModuleManager.PreBlock(ctx) } diff --git a/simapp/app.go b/simapp/app.go index 1d7ed0776fb8..859a722fcadb 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -583,7 +583,7 @@ func (app *SimApp) setPostHandler() { func (app *SimApp) Name() string { return app.BaseApp.Name() } // PreBlocker application updates every pre block -func (app *SimApp) PreBlocker(ctx sdk.Context) (sdk.ResponsePreBlock, error) { +func (app *SimApp) PreBlocker(ctx sdk.Context, _ *abci.RequestFinalizeBlock) (sdk.ResponsePreBlock, error) { return app.ModuleManager.PreBlock(ctx) } diff --git a/types/abci.go b/types/abci.go index ff40d1109a27..3417b64f4255 100644 --- a/types/abci.go +++ b/types/abci.go @@ -31,7 +31,7 @@ type ExtendVoteHandler func(Context, *abci.RequestExtendVote) (*abci.ResponseExt type VerifyVoteExtensionHandler func(Context, *abci.RequestVerifyVoteExtension) (*abci.ResponseVerifyVoteExtension, error) // PreBlocker runs code before the `BeginBlocker`. -type PreBlocker func(Context) (ResponsePreBlock, error) +type PreBlocker func(Context, *abci.RequestFinalizeBlock) (ResponsePreBlock, error) // BeginBlocker defines a function type alias for executing application // business logic before transactions are executed. @@ -51,14 +51,6 @@ type BeginBlocker func(Context) (BeginBlock, error) // and allows for existing EndBlock functionality within applications. type EndBlocker func(Context) (EndBlock, error) -// PreFinalizeBlockHook defines a function type alias for executing logic right -// before FinalizeBlock is called (but after its context has been set up). It is -// intended to allow applications to perform computation on vote extensions and -// persist their results in state. -// -// Note: returning an error will make FinalizeBlock fail. -type PreFinalizeBlockHook func(Context, *abci.RequestFinalizeBlock) error - // EndBlock defines a type which contains endblock events and validator set updates type EndBlock struct { ValidatorUpdates []abci.ValidatorUpdate From 744311846bc0d2618efd5a2731c1704da80af73a Mon Sep 17 00:00:00 2001 From: mmsqe Date: Sat, 26 Aug 2023 21:16:32 +0800 Subject: [PATCH 2/9] update doc --- CHANGELOG.md | 2 +- docs/architecture/adr-064-abci-2.0.md | 18 +++++++++++------- docs/architecture/adr-068-preblock.md | 1 + .../build/building-apps/04-vote-extensions.md | 2 +- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05945b0f3004..c2f1052e6d9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,7 +57,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes -* (types) [#16583](https://github.com/cosmos/cosmos-sdk/pull/16583), [#17372](https://github.com/cosmos/cosmos-sdk/pull/17372), [#17421](https://github.com/cosmos/cosmos-sdk/pull/17421) Introduce `PreBlock`, which runs before begin blocker other modules, and allows to modify consensus parameters, and the changes are visible to the following state machine logics. +* (types) [#16583](https://github.com/cosmos/cosmos-sdk/pull/16583), [#17372](https://github.com/cosmos/cosmos-sdk/pull/17372), [#17421](https://github.com/cosmos/cosmos-sdk/pull/17421), [#17713](https://github.com/cosmos/cosmos-sdk/pull/17713) Introduce `PreBlock`, which runs before begin blocker other modules, and allows to modify consensus parameters, and the changes are visible to the following state machine logics and reimplement `PreFinalizeBlockHook` as `PreBlocker`. * (baseapp) [#17518](https://github.com/cosmos/cosmos-sdk/pull/17518) Utilizing voting power from vote extensions (CometBFT) instead of the current bonded tokens (x/staking) to determine if a set of vote extensions are valid. * (config) [#17649](https://github.com/cosmos/cosmos-sdk/pull/17649) Fix `mempool.max-txs` configuration is invalid in `app.config`. * (mempool) [#17668](https://github.com/cosmos/cosmos-sdk/pull/17668) Fix: `PriorityNonceIterator.Next()` nil pointer ref for min priority at the end of iteration. diff --git a/docs/architecture/adr-064-abci-2.0.md b/docs/architecture/adr-064-abci-2.0.md index cdbd88024d88..c0dc7f746e81 100644 --- a/docs/architecture/adr-064-abci-2.0.md +++ b/docs/architecture/adr-064-abci-2.0.md @@ -293,7 +293,7 @@ decision based on the vote extensions. In certain contexts, it may be useful or necessary for applications to persist data derived from vote extensions. In order to facilitate this use case, we propose -to allow app developers to define a pre-FinalizeBlock hook which will be called +to allow app developers to define a pre-Blocker hook which will be called at the very beginning of `FinalizeBlock`, i.e. before `BeginBlock` (see below). Note, we cannot allow applications to directly write to the application state @@ -301,7 +301,7 @@ during `ProcessProposal` because during replay, CometBFT will NOT call `ProcessP which would result in an incomplete state view. ```go -func (a MyApp) PreFinalizeBlockHook(ctx sdk.Context, req.RequestFinalizeBlock) error { +func (a MyApp) PreBlocker(ctx sdk.Context, req *abci.RequestFinalizeBlock) error { voteExts := GetVoteExtensions(ctx, req.Txs) // Process and perform some compute on vote extensions, storing any resulting @@ -353,13 +353,17 @@ we can come up with new types and names altogether. func (app *BaseApp) FinalizeBlock(req abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { ctx := ... - if app.preFinalizeBlockHook != nil { - if err := app.preFinalizeBlockHook(ctx, req); err != nil { + if app.preBlocker != nil { + ctx := app.finalizeBlockState.ctx + rsp, err := app.preBlocker(ctx, req) + if err != nil { return nil, err } + if rsp.ConsensusParamsChanged { + app.finalizeBlockState.ctx = ctx.WithConsensusParams(app.GetConsensusParams(ctx)) + } } - - beginBlockResp := app.beginBlock(ctx, req) + beginBlockResp, err := app.beginBlock(req) appendBlockEventAttr(beginBlockResp.Events, "begin_block") txExecResults := make([]abci.ExecTxResult, 0, len(req.Txs)) @@ -368,7 +372,7 @@ func (app *BaseApp) FinalizeBlock(req abci.RequestFinalizeBlock) (*abci.Response txExecResults = append(txExecResults, result) } - endBlockResp := app.endBlock(ctx, req) + endBlockResp, err := app.endBlock(app.finalizeBlockState.ctx) appendBlockEventAttr(beginBlockResp.Events, "end_block") return abci.ResponseFinalizeBlock{ diff --git a/docs/architecture/adr-068-preblock.md b/docs/architecture/adr-068-preblock.md index dc0cd2a24564..86692c412c4d 100644 --- a/docs/architecture/adr-068-preblock.md +++ b/docs/architecture/adr-068-preblock.md @@ -58,3 +58,4 @@ The new ctx must be passed to all the other lifecycle methods. * [1] https://github.com/cosmos/cosmos-sdk/issues/16494 * [2] https://github.com/cosmos/cosmos-sdk/pull/16583 * [3] https://github.com/cosmos/cosmos-sdk/pull/17421 +* [4] https://github.com/cosmos/cosmos-sdk/pull/17713 diff --git a/docs/docs/build/building-apps/04-vote-extensions.md b/docs/docs/build/building-apps/04-vote-extensions.md index 8b9fb5943f20..f78f4c677ffe 100644 --- a/docs/docs/build/building-apps/04-vote-extensions.md +++ b/docs/docs/build/building-apps/04-vote-extensions.md @@ -77,7 +77,7 @@ will be available to the application during the subsequent `FinalizeBlock` call. An example of how a pre-FinalizeBlock hook could look like is shown below: ```go -app.SetPreFinalizeBlockHook(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) error { +app.SetPreBlocker(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) error { allVEs := []VE{} // store all parsed vote extensions here for _, tx := range req.Txs { // define a custom function that tries to parse the tx as a vote extension From 43a9d5f7942ea235cc248bb3e0fd70ad95725ab4 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Wed, 13 Sep 2023 21:42:04 +0800 Subject: [PATCH 3/9] Apply suggestions from code review --- types/abci.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/types/abci.go b/types/abci.go index 3417b64f4255..6c5a9a4f43e9 100644 --- a/types/abci.go +++ b/types/abci.go @@ -30,7 +30,12 @@ type ExtendVoteHandler func(Context, *abci.RequestExtendVote) (*abci.ResponseExt // pre-commit vote extension. type VerifyVoteExtensionHandler func(Context, *abci.RequestVerifyVoteExtension) (*abci.ResponseVerifyVoteExtension, error) -// PreBlocker runs code before the `BeginBlocker`. +// PreBlocker runs code before the `BeginBlocker` and defines a function type alias for executing logic right +// before FinalizeBlock is called (but after its context has been set up). It is +// intended to allow applications to perform computation on vote extensions and +// persist their results in state. +// +// Note: returning an error will make FinalizeBlock fail. type PreBlocker func(Context, *abci.RequestFinalizeBlock) (ResponsePreBlock, error) // BeginBlocker defines a function type alias for executing application From 7537d3258a5fd548c2a52f0917aac46fcab1ed4c Mon Sep 17 00:00:00 2001 From: mmsqe Date: Wed, 13 Sep 2023 23:19:32 +0800 Subject: [PATCH 4/9] Apply suggestions from code review --- x/upgrade/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x/upgrade/CHANGELOG.md b/x/upgrade/CHANGELOG.md index 46980cc8560c..0785d93371ff 100644 --- a/x/upgrade/CHANGELOG.md +++ b/x/upgrade/CHANGELOG.md @@ -41,3 +41,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#16511](https://github.com/cosmos/cosmos-sdk/pull/16511) `plan.DownloadURLWithChecksum` has been renamed to `plan.DownloadURL` and does not validate the URL anymore. Call `plan.ValidateURL` before calling `plan.DownloadURL` to validate the URL. * [#16511](https://github.com/cosmos/cosmos-sdk/pull/16511) `plan.DownloadUpgrade` does not validate URL anymore. Call `plan.ValidateURL` before calling `plan.DownloadUpgrade` to validate the URL. * [#16227](https://github.com/cosmos/cosmos-sdk/issues/16227) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey`, methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context` and return an `error`. `UpgradeHandler` now receives a `context.Context`. `GetUpgradedClient`, `GetUpgradedConsensusState`, `GetUpgradePlan` now return a specific error for "not found". + +### Bug Fixes + +* (types) [#16583](https://github.com/cosmos/cosmos-sdk/pull/16583), [#17372](https://github.com/cosmos/cosmos-sdk/pull/17372), [#17421](https://github.com/cosmos/cosmos-sdk/pull/17421), [#17713](https://github.com/cosmos/cosmos-sdk/pull/17713) Introduce `PreBlock`, which runs before begin blocker other modules, and allows to modify consensus parameters, and the changes are visible to the following state machine logics and reimplement `PreFinalizeBlockHook` as `PreBlocker`. From b1ae0293d702a053400df88df1800805d21cfe30 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Wed, 13 Sep 2023 23:35:08 +0800 Subject: [PATCH 5/9] Update x/upgrade/CHANGELOG.md Co-authored-by: Julien Robert --- x/upgrade/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/upgrade/CHANGELOG.md b/x/upgrade/CHANGELOG.md index 0785d93371ff..bfd682d31993 100644 --- a/x/upgrade/CHANGELOG.md +++ b/x/upgrade/CHANGELOG.md @@ -44,4 +44,4 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes -* (types) [#16583](https://github.com/cosmos/cosmos-sdk/pull/16583), [#17372](https://github.com/cosmos/cosmos-sdk/pull/17372), [#17421](https://github.com/cosmos/cosmos-sdk/pull/17421), [#17713](https://github.com/cosmos/cosmos-sdk/pull/17713) Introduce `PreBlock`, which runs before begin blocker other modules, and allows to modify consensus parameters, and the changes are visible to the following state machine logics and reimplement `PreFinalizeBlockHook` as `PreBlocker`. +* [#17421](https://github.com/cosmos/cosmos-sdk/pull/17421) Replace ` BeginBlock` by `PreBlock`. Read [ADR-68](https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-068-preblock.md) for more information. From 021ead228edcc7b502d8585f378f647406a1de17 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 14 Sep 2023 03:20:49 +0800 Subject: [PATCH 6/9] Update CHANGELOG.md Co-authored-by: Aleksandr Bezobchuk --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2f1052e6d9e..bc19104819fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,7 +57,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes -* (types) [#16583](https://github.com/cosmos/cosmos-sdk/pull/16583), [#17372](https://github.com/cosmos/cosmos-sdk/pull/17372), [#17421](https://github.com/cosmos/cosmos-sdk/pull/17421), [#17713](https://github.com/cosmos/cosmos-sdk/pull/17713) Introduce `PreBlock`, which runs before begin blocker other modules, and allows to modify consensus parameters, and the changes are visible to the following state machine logics and reimplement `PreFinalizeBlockHook` as `PreBlocker`. +* (types) [#16583](https://github.com/cosmos/cosmos-sdk/pull/16583), [#17372](https://github.com/cosmos/cosmos-sdk/pull/17372), [#17421](https://github.com/cosmos/cosmos-sdk/pull/17421), [#17713](https://github.com/cosmos/cosmos-sdk/pull/17713) Introduce `PreBlock`, which executes in `FinalizeBlock` before `BeginBlock`. It allows the application to modify consensus parameters and have access to VE state. Note, `PreFinalizeBlockHook` is replaced by`PreBlocker`. * (baseapp) [#17518](https://github.com/cosmos/cosmos-sdk/pull/17518) Utilizing voting power from vote extensions (CometBFT) instead of the current bonded tokens (x/staking) to determine if a set of vote extensions are valid. * (config) [#17649](https://github.com/cosmos/cosmos-sdk/pull/17649) Fix `mempool.max-txs` configuration is invalid in `app.config`. * (mempool) [#17668](https://github.com/cosmos/cosmos-sdk/pull/17668) Fix: `PriorityNonceIterator.Next()` nil pointer ref for min priority at the end of iteration. From edd52d84dd631c48cefb746146cd4259a5b4ab1f Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 14 Sep 2023 16:54:07 +0800 Subject: [PATCH 7/9] change to ptr --- baseapp/abci_test.go | 12 +++++------ runtime/app.go | 2 +- simapp/app.go | 2 +- simapp/go.mod | 1 + simapp/gomod2nix.toml | 3 --- types/abci.go | 2 +- types/module/module.go | 6 +++--- types/module/module_test.go | 6 +++--- x/upgrade/abci.go | 41 ++++++++++++++++++++++--------------- x/upgrade/go.mod | 1 + 10 files changed, 42 insertions(+), 34 deletions(-) diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 8ffdf10ed77a..b46481af981a 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -2045,9 +2045,9 @@ func TestBaseApp_PreBlocker(t *testing.T) { require.NoError(t, err) wasHookCalled := false - app.SetPreBlocker(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) (sdk.ResponsePreBlock, error) { + app.SetPreBlocker(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) (*sdk.ResponsePreBlock, error) { wasHookCalled = true - return sdk.ResponsePreBlock{ + return &sdk.ResponsePreBlock{ ConsensusParamsChanged: true, }, nil }) @@ -2062,8 +2062,8 @@ func TestBaseApp_PreBlocker(t *testing.T) { _, err = app.InitChain(&abci.RequestInitChain{}) require.NoError(t, err) - app.SetPreBlocker(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) (sdk.ResponsePreBlock, error) { - return sdk.ResponsePreBlock{}, errors.New("some error") + app.SetPreBlocker(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) (*sdk.ResponsePreBlock, error) { + return nil, errors.New("some error") }) app.Seal() @@ -2124,7 +2124,7 @@ func TestBaseApp_VoteExtensions(t *testing.T) { return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}, nil }) - app.SetPreBlocker(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) (sdk.ResponsePreBlock, error) { + app.SetPreBlocker(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) (*sdk.ResponsePreBlock, error) { count := uint64(0) pricesSum := uint64(0) for _, v := range req.Txs { @@ -2143,7 +2143,7 @@ func TestBaseApp_VoteExtensions(t *testing.T) { ctx.KVStore(capKey1).Set([]byte("avgPrice"), buf) } - return sdk.ResponsePreBlock{ + return &sdk.ResponsePreBlock{ ConsensusParamsChanged: true, }, nil }) diff --git a/runtime/app.go b/runtime/app.go index ab4630e4828c..ca7f45ffbaee 100644 --- a/runtime/app.go +++ b/runtime/app.go @@ -153,7 +153,7 @@ func (a *App) Load(loadLatest bool) error { } // PreBlocker application updates every pre block -func (a *App) PreBlocker(ctx sdk.Context, _ *abci.RequestFinalizeBlock) (sdk.ResponsePreBlock, error) { +func (a *App) PreBlocker(ctx sdk.Context, _ *abci.RequestFinalizeBlock) (*sdk.ResponsePreBlock, error) { return a.ModuleManager.PreBlock(ctx) } diff --git a/simapp/app.go b/simapp/app.go index 296356b889eb..0c24f4e1cc29 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -583,7 +583,7 @@ func (app *SimApp) setPostHandler() { func (app *SimApp) Name() string { return app.BaseApp.Name() } // PreBlocker application updates every pre block -func (app *SimApp) PreBlocker(ctx sdk.Context, _ *abci.RequestFinalizeBlock) (sdk.ResponsePreBlock, error) { +func (app *SimApp) PreBlocker(ctx sdk.Context, _ *abci.RequestFinalizeBlock) (*sdk.ResponsePreBlock, error) { return app.ModuleManager.PreBlock(ctx) } diff --git a/simapp/go.mod b/simapp/go.mod index 3edc1c71ce35..afb097c6ec85 100644 --- a/simapp/go.mod +++ b/simapp/go.mod @@ -205,6 +205,7 @@ replace ( cosmossdk.io/x/feegrant => ../x/feegrant cosmossdk.io/x/nft => ../x/nft cosmossdk.io/x/upgrade => ../x/upgrade + cosmossdk.io/api => ../api ) // Below are the long-lived replace of the SimApp diff --git a/simapp/gomod2nix.toml b/simapp/gomod2nix.toml index 46daacc9118f..fb878e2baf96 100644 --- a/simapp/gomod2nix.toml +++ b/simapp/gomod2nix.toml @@ -16,9 +16,6 @@ schema = 3 [mod."cloud.google.com/go/storage"] version = "v1.31.0" hash = "sha256-d59Q6JjMga6/7d1TtFW9GuAq+6O2U4LhNesz3Knmo0E=" - [mod."cosmossdk.io/api"] - version = "v0.7.1-0.20230820170544-1bd37053e0c0" - hash = "sha256-iASrjR6a28d+feKXAtP4lEoKlx+TBZm/Q7IBJhR6Omo=" [mod."cosmossdk.io/collections"] version = "v0.4.0" hash = "sha256-minFyzgO/D+Oda4E3B1qvOAN5qd65SjS6nmjca4cp/8=" diff --git a/types/abci.go b/types/abci.go index 6c5a9a4f43e9..8325f5dadf0c 100644 --- a/types/abci.go +++ b/types/abci.go @@ -36,7 +36,7 @@ type VerifyVoteExtensionHandler func(Context, *abci.RequestVerifyVoteExtension) // persist their results in state. // // Note: returning an error will make FinalizeBlock fail. -type PreBlocker func(Context, *abci.RequestFinalizeBlock) (ResponsePreBlock, error) +type PreBlocker func(Context, *abci.RequestFinalizeBlock) (*ResponsePreBlock, error) // BeginBlocker defines a function type alias for executing application // business logic before transactions are executed. diff --git a/types/module/module.go b/types/module/module.go index a3c7b2ed62f0..615efc001773 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -744,21 +744,21 @@ func (m Manager) RunMigrations(ctx context.Context, cfg Configurator, fromVM Ver // PreBlock performs begin block functionality for upgrade module. // It takes the current context as a parameter and returns a boolean value // indicating whether the migration was successfully executed or not. -func (m *Manager) PreBlock(ctx sdk.Context) (sdk.ResponsePreBlock, error) { +func (m *Manager) PreBlock(ctx sdk.Context) (*sdk.ResponsePreBlock, error) { ctx = ctx.WithEventManager(sdk.NewEventManager()) paramsChanged := false for _, moduleName := range m.OrderPreBlockers { if module, ok := m.Modules[moduleName].(appmodule.HasPreBlocker); ok { rsp, err := module.PreBlock(ctx) if err != nil { - return sdk.ResponsePreBlock{}, err + return nil, err } if rsp.IsConsensusParamsChanged() { paramsChanged = true } } } - return sdk.ResponsePreBlock{ + return &sdk.ResponsePreBlock{ ConsensusParamsChanged: paramsChanged, }, nil } diff --git a/types/module/module_test.go b/types/module/module_test.go index 2a348667eb3a..69af1f7ab0b1 100644 --- a/types/module/module_test.go +++ b/types/module/module_test.go @@ -482,7 +482,7 @@ func TestCoreAPIManager_PreBlock(t *testing.T) { require.Equal(t, 2, len(mm.Modules)) require.Equal(t, 1, len(mm.OrderPreBlockers)) - mockAppModule1.EXPECT().PreBlock(gomock.Any()).Times(1).Return(sdk.ResponsePreBlock{ + mockAppModule1.EXPECT().PreBlock(gomock.Any()).Times(1).Return(&sdk.ResponsePreBlock{ ConsensusParamsChanged: true, }, nil) res, err := mm.PreBlock(sdk.Context{}) @@ -490,7 +490,7 @@ func TestCoreAPIManager_PreBlock(t *testing.T) { require.True(t, res.ConsensusParamsChanged) // test false - mockAppModule1.EXPECT().PreBlock(gomock.Any()).Times(1).Return(sdk.ResponsePreBlock{ + mockAppModule1.EXPECT().PreBlock(gomock.Any()).Times(1).Return(&sdk.ResponsePreBlock{ ConsensusParamsChanged: false, }, nil) res, err = mm.PreBlock(sdk.Context{}) @@ -498,7 +498,7 @@ func TestCoreAPIManager_PreBlock(t *testing.T) { require.False(t, res.ConsensusParamsChanged) // test error - mockAppModule1.EXPECT().PreBlock(gomock.Any()).Times(1).Return(sdk.ResponsePreBlock{}, errors.New("some error")) + mockAppModule1.EXPECT().PreBlock(gomock.Any()).Times(1).Return(nil, errors.New("some error")) _, err = mm.PreBlock(sdk.Context{}) require.EqualError(t, err, "some error") } diff --git a/x/upgrade/abci.go b/x/upgrade/abci.go index 58064868c8a9..2ce5ea30666d 100644 --- a/x/upgrade/abci.go +++ b/x/upgrade/abci.go @@ -6,6 +6,7 @@ import ( "fmt" "time" + "cosmossdk.io/core/appmodule" storetypes "cosmossdk.io/store/types" "cosmossdk.io/x/upgrade/keeper" "cosmossdk.io/x/upgrade/types" @@ -22,17 +23,14 @@ import ( // The purpose is to ensure the binary is switched EXACTLY at the desired block, and to allow // a migration to be executed if needed upon this switch (migration defined in the new binary) // skipUpgradeHeightArray is a set of block heights for which the upgrade must be skipped -func PreBlocker(ctx context.Context, k *keeper.Keeper) (sdk.ResponsePreBlock, error) { - rsp := sdk.ResponsePreBlock{ - ConsensusParamsChanged: false, - } +func PreBlocker(ctx context.Context, k *keeper.Keeper) (appmodule.ResponsePreBlock, error) { defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker) sdkCtx := sdk.UnwrapSDKContext(ctx) blockHeight := sdkCtx.HeaderInfo().Height plan, err := k.GetUpgradePlan(ctx) if err != nil && !errors.Is(err, types.ErrNoUpgradePlanFound) { - return rsp, err + return nil, err } found := err == nil @@ -46,7 +44,7 @@ func PreBlocker(ctx context.Context, k *keeper.Keeper) (sdk.ResponsePreBlock, er if !found || !plan.ShouldExecute(blockHeight) || (plan.ShouldExecute(blockHeight) && k.IsSkipHeight(blockHeight)) { lastAppliedPlan, _, err := k.GetLastCompletedUpgrade(ctx) if err != nil { - return rsp, err + return nil, err } if lastAppliedPlan != "" && !k.HasHandler(lastAppliedPlan) { @@ -57,13 +55,15 @@ func PreBlocker(ctx context.Context, k *keeper.Keeper) (sdk.ResponsePreBlock, er appVersion = cp.Version.App } - return rsp, fmt.Errorf("wrong app version %d, upgrade handler is missing for %s upgrade plan", appVersion, lastAppliedPlan) + return nil, fmt.Errorf("wrong app version %d, upgrade handler is missing for %s upgrade plan", appVersion, lastAppliedPlan) } } } if !found { - return rsp, nil + return &sdk.ResponsePreBlock{ + ConsensusParamsChanged: false, + }, nil } logger := k.Logger(ctx) @@ -76,7 +76,12 @@ func PreBlocker(ctx context.Context, k *keeper.Keeper) (sdk.ResponsePreBlock, er logger.Info(skipUpgradeMsg) // Clear the upgrade plan at current height - return rsp, k.ClearUpgradePlan(ctx) + if err := k.ClearUpgradePlan(ctx); err != nil { + return nil, err + } + return &sdk.ResponsePreBlock{ + ConsensusParamsChanged: false, + }, nil } // Prepare shutdown if we don't have an upgrade handler for this upgrade name (meaning this software is out of date) @@ -85,25 +90,27 @@ func PreBlocker(ctx context.Context, k *keeper.Keeper) (sdk.ResponsePreBlock, er // store migrations. err := k.DumpUpgradeInfoToDisk(blockHeight, plan) if err != nil { - return rsp, fmt.Errorf("unable to write upgrade info to filesystem: %w", err) + return nil, fmt.Errorf("unable to write upgrade info to filesystem: %w", err) } upgradeMsg := BuildUpgradeNeededMsg(plan) logger.Error(upgradeMsg) // Returning an error will end up in a panic - return rsp, errors.New(upgradeMsg) + return nil, errors.New(upgradeMsg) } // We have an upgrade handler for this upgrade name, so apply the upgrade logger.Info(fmt.Sprintf("applying upgrade \"%s\" at %s", plan.Name, plan.DueAt())) sdkCtx = sdkCtx.WithBlockGasMeter(storetypes.NewInfiniteGasMeter()) - err = k.ApplyUpgrade(sdkCtx, plan) - return sdk.ResponsePreBlock{ + if err := k.ApplyUpgrade(sdkCtx, plan); err != nil { + return nil, err + } + return &sdk.ResponsePreBlock{ // the consensus parameters might be modified in the migration, // refresh the consensus parameters in context. ConsensusParamsChanged: true, - }, err + }, nil } // if we have a pending upgrade, but it is not yet time, make sure we did not @@ -113,9 +120,11 @@ func PreBlocker(ctx context.Context, k *keeper.Keeper) (sdk.ResponsePreBlock, er logger.Error(downgradeMsg) // Returning an error will end up in a panic - return rsp, errors.New(downgradeMsg) + return nil, errors.New(downgradeMsg) } - return rsp, nil + return &sdk.ResponsePreBlock{ + ConsensusParamsChanged: false, + }, nil } // BuildUpgradeNeededMsg prints the message that notifies that an upgrade is needed. diff --git a/x/upgrade/go.mod b/x/upgrade/go.mod index f654587ba662..0163d406e71f 100644 --- a/x/upgrade/go.mod +++ b/x/upgrade/go.mod @@ -183,5 +183,6 @@ require ( replace ( github.com/cosmos/cosmos-sdk => ../../. + cosmossdk.io/api => ../../api github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.9.1 ) From fc5c23554f0e2215b16942d8288dd49b772f4644 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 14 Sep 2023 19:12:41 +0800 Subject: [PATCH 8/9] update doc --- UPGRADING.md | 2 +- docs/docs/build/building-apps/03-app-upgrade.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/UPGRADING.md b/UPGRADING.md index d8e858b9396f..e9a061c7d459 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -134,7 +134,7 @@ app.ModuleManager.SetOrderBeginBlockers( // ... // -+func (app *SimApp) PreBlocker(ctx sdk.Context, req abci.RequestBeginBlock) (sdk.ResponsePreBlock, error) { ++func (app *SimApp) PreBlocker(ctx sdk.Context, req abci.RequestBeginBlock) (*sdk.ResponsePreBlock, error) { + return app.ModuleManager.PreBlock(ctx, req) +} ``` diff --git a/docs/docs/build/building-apps/03-app-upgrade.md b/docs/docs/build/building-apps/03-app-upgrade.md index f59dff94ca5b..d5a88c5d7cc4 100644 --- a/docs/docs/build/building-apps/03-app-upgrade.md +++ b/docs/docs/build/building-apps/03-app-upgrade.md @@ -60,7 +60,7 @@ In addition to basic module wiring, setup the upgrade Keeper for the app and the keeper's PreBlocker method: ```go -func (app *myApp) PreBlocker(ctx sdk.Context, req req.RequestFinalizeBlock) (sdk.ResponsePreBlock, error) { +func (app *myApp) PreBlocker(ctx sdk.Context, req req.RequestFinalizeBlock) (*sdk.ResponsePreBlock, error) { // For demonstration sake, the app PreBlocker only returns the upgrade module pre-blocker. // In a real app, the module manager should call all pre-blockers // return return app.ModuleManager.PreBlock(ctx, req) From 82d188ed305024b0fa403734b263527ba09d56d2 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 14 Sep 2023 20:19:21 +0800 Subject: [PATCH 9/9] Apply suggestions from code review --- UPGRADING.md | 2 +- simapp/go.mod | 1 - simapp/gomod2nix.toml | 3 +++ x/upgrade/go.mod | 1 - 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/UPGRADING.md b/UPGRADING.md index e9a061c7d459..0f0d4a3ecb64 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -134,7 +134,7 @@ app.ModuleManager.SetOrderBeginBlockers( // ... // -+func (app *SimApp) PreBlocker(ctx sdk.Context, req abci.RequestBeginBlock) (*sdk.ResponsePreBlock, error) { ++func (app *SimApp) PreBlocker(ctx sdk.Context, req *abci.RequestFinalizeBlock) (*sdk.ResponsePreBlock, error) { + return app.ModuleManager.PreBlock(ctx, req) +} ``` diff --git a/simapp/go.mod b/simapp/go.mod index afb097c6ec85..3edc1c71ce35 100644 --- a/simapp/go.mod +++ b/simapp/go.mod @@ -205,7 +205,6 @@ replace ( cosmossdk.io/x/feegrant => ../x/feegrant cosmossdk.io/x/nft => ../x/nft cosmossdk.io/x/upgrade => ../x/upgrade - cosmossdk.io/api => ../api ) // Below are the long-lived replace of the SimApp diff --git a/simapp/gomod2nix.toml b/simapp/gomod2nix.toml index fb878e2baf96..46daacc9118f 100644 --- a/simapp/gomod2nix.toml +++ b/simapp/gomod2nix.toml @@ -16,6 +16,9 @@ schema = 3 [mod."cloud.google.com/go/storage"] version = "v1.31.0" hash = "sha256-d59Q6JjMga6/7d1TtFW9GuAq+6O2U4LhNesz3Knmo0E=" + [mod."cosmossdk.io/api"] + version = "v0.7.1-0.20230820170544-1bd37053e0c0" + hash = "sha256-iASrjR6a28d+feKXAtP4lEoKlx+TBZm/Q7IBJhR6Omo=" [mod."cosmossdk.io/collections"] version = "v0.4.0" hash = "sha256-minFyzgO/D+Oda4E3B1qvOAN5qd65SjS6nmjca4cp/8=" diff --git a/x/upgrade/go.mod b/x/upgrade/go.mod index 0163d406e71f..f654587ba662 100644 --- a/x/upgrade/go.mod +++ b/x/upgrade/go.mod @@ -183,6 +183,5 @@ require ( replace ( github.com/cosmos/cosmos-sdk => ../../. - cosmossdk.io/api => ../../api github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.9.1 )