From a57c937e84c0f88124022d4ebe190bd99adecf1f Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Thu, 15 Jun 2023 21:50:46 +0200 Subject: [PATCH 01/29] feat: Optimistic Execution --- baseapp/abci.go | 54 +++++++++++++++++++++++++++++++-- baseapp/baseapp.go | 10 ++++-- baseapp/optimistic_execution.go | 11 +++++++ 3 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 baseapp/optimistic_execution.go diff --git a/baseapp/abci.go b/baseapp/abci.go index ded22c971933..ac0ff88166a8 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -1,9 +1,11 @@ package baseapp import ( + "bytes" "context" "crypto/sha256" "fmt" + "log" "os" "sort" "strings" @@ -532,6 +534,34 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil } + if app.oeEnabled { + if app.oeInfo == nil { + completionSignal := make(chan struct{}) + app.oeInfo = &OptimisticExecutionInfo{ + Completion: completionSignal, + Request: &abci.RequestFinalizeBlock{ + Txs: req.Txs, + DecidedLastCommit: req.ProposedLastCommit, + Misbehavior: req.Misbehavior, + Hash: req.Hash, + Height: req.Height, + Time: req.Time, + NextValidatorsHash: req.NextValidatorsHash, + ProposerAddress: req.ProposerAddress, + }, + } + + go func() { + log.Println("Running OE ✅") + app.oeInfo.Response, app.oeInfo.Error = app.internalFinalizeBlock(app.oeInfo.Request) + app.oeInfo.Completion <- struct{}{} + }() + + } else if !bytes.Equal(app.oeInfo.Request.Hash, req.Hash) { + app.oeInfo.Aborted = true + } + } + return resp, nil } @@ -645,7 +675,7 @@ func (app *BaseApp) VerifyVoteExtension(req *abci.RequestVerifyVoteExtension) (r // skipped. This is to support compatibility with proposers injecting vote // extensions into the proposal, which should not themselves be executed in cases // where they adhere to the sdk.Tx interface. -func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { +func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { var events []abci.Event if err := app.validateFinalizeBlockHeight(req); err != nil { @@ -711,7 +741,11 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons WithHeaderHash(req.Hash) } - beginBlock := app.beginBlock(req) + beginBlock, err := app.beginBlock(req) + if err != nil { + return nil, err + } + events = append(events, beginBlock.Events...) // Iterate over all raw transactions in the proposal and attempt to execute @@ -747,6 +781,22 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons }, nil } +func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { + defer func() { + app.oeInfo = nil + }() + + if app.oeInfo != nil && app.oeEnabled { + <-app.oeInfo.Completion + if !app.oeInfo.Aborted && bytes.Equal(app.oeInfo.Request.Hash, req.Hash) { + return app.oeInfo.Response, app.oeInfo.Error + } + } + + log.Println("NOT running OE ❌") + return app.internalFinalizeBlock(req) +} + // Commit implements the ABCI interface. It will commit all state that exists in // the deliver state's multi-store and includes the resulting commit ID in the // returned abci.ResponseCommit. Commit will set the check state based on the diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 5e548dc77bb6..3f44939c9980 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -179,6 +179,9 @@ type BaseApp struct { chainID string cdc codec.Codec + + oeInfo *OptimisticExecutionInfo + oeEnabled bool } // NewBaseApp returns a reference to an initialized BaseApp. It accepts a @@ -197,6 +200,7 @@ func NewBaseApp( msgServiceRouter: NewMsgServiceRouter(), txDecoder: txDecoder, fauxMerkleMode: false, + oeEnabled: true, } for _, option := range options { @@ -679,7 +683,7 @@ func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) (sdk.Context return ctx.WithMultiStore(msCache), msCache } -func (app *BaseApp) beginBlock(req *abci.RequestFinalizeBlock) sdk.BeginBlock { +func (app *BaseApp) beginBlock(req *abci.RequestFinalizeBlock) (sdk.BeginBlock, error) { var ( resp sdk.BeginBlock err error @@ -688,7 +692,7 @@ func (app *BaseApp) beginBlock(req *abci.RequestFinalizeBlock) sdk.BeginBlock { if app.beginBlocker != nil { resp, err = app.beginBlocker(app.finalizeBlockState.ctx) if err != nil { - panic(err) + return resp, err } // append BeginBlock attributes to all events in the EndBlock response @@ -702,7 +706,7 @@ func (app *BaseApp) beginBlock(req *abci.RequestFinalizeBlock) sdk.BeginBlock { resp.Events = sdk.MarkEventsToIndex(resp.Events, app.indexEvents) } - return resp + return resp, nil } func (app *BaseApp) deliverTx(tx []byte) *abci.ExecTxResult { diff --git a/baseapp/optimistic_execution.go b/baseapp/optimistic_execution.go new file mode 100644 index 000000000000..94913938e326 --- /dev/null +++ b/baseapp/optimistic_execution.go @@ -0,0 +1,11 @@ +package baseapp + +import abci "github.com/cometbft/cometbft/abci/types" + +type OptimisticExecutionInfo struct { + Aborted bool + Completion chan struct{} + Request *abci.RequestFinalizeBlock + Response *abci.ResponseFinalizeBlock + Error error +} From 023256ec3082e842d4f47e57c520c7f49d2e49ce Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Thu, 15 Jun 2023 22:09:02 +0200 Subject: [PATCH 02/29] fix panic recovery --- baseapp/abci.go | 35 ++++++++++++++++++++------------- baseapp/baseapp.go | 6 +++--- baseapp/optimistic_execution.go | 1 + simapp/upgrades.go | 7 +++++++ x/gov/types/v1/params.go | 2 +- 5 files changed, 33 insertions(+), 18 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index ac0ff88166a8..b0e96faf7559 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -552,6 +552,12 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc } go func() { + defer func() { + if r := recover(); r != nil { + log.Println("⚡️⚡️⚡️ RECOVERED PANIC IN OE") + app.oeInfo.Panic = r + } + }() log.Println("Running OE ✅") app.oeInfo.Response, app.oeInfo.Error = app.internalFinalizeBlock(app.oeInfo.Request) app.oeInfo.Completion <- struct{}{} @@ -665,16 +671,6 @@ func (app *BaseApp) VerifyVoteExtension(req *abci.RequestVerifyVoteExtension) (r return resp, err } -// FinalizeBlock will execute the block proposal provided by RequestFinalizeBlock. -// Specifically, it will execute an application's BeginBlock (if defined), followed -// by the transactions in the proposal, finally followed by the application's -// EndBlock (if defined). -// -// For each raw transaction, i.e. a byte slice, BaseApp will only execute it if -// it adheres to the sdk.Tx interface. Otherwise, the raw transaction will be -// skipped. This is to support compatibility with proposers injecting vote -// extensions into the proposal, which should not themselves be executed in cases -// where they adhere to the sdk.Tx interface. func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { var events []abci.Event @@ -741,10 +737,7 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci WithHeaderHash(req.Hash) } - beginBlock, err := app.beginBlock(req) - if err != nil { - return nil, err - } + beginBlock := app.beginBlock(req) events = append(events, beginBlock.Events...) @@ -781,12 +774,26 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci }, nil } +// FinalizeBlock will execute the block proposal provided by RequestFinalizeBlock. +// Specifically, it will execute an application's BeginBlock (if defined), followed +// by the transactions in the proposal, finally followed by the application's +// EndBlock (if defined). +// +// For each raw transaction, i.e. a byte slice, BaseApp will only execute it if +// it adheres to the sdk.Tx interface. Otherwise, the raw transaction will be +// skipped. This is to support compatibility with proposers injecting vote +// extensions into the proposal, which should not themselves be executed in cases +// where they adhere to the sdk.Tx interface. func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { defer func() { app.oeInfo = nil }() if app.oeInfo != nil && app.oeEnabled { + if app.oeInfo.Panic != nil { + panic(app.oeInfo.Panic) + } + <-app.oeInfo.Completion if !app.oeInfo.Aborted && bytes.Equal(app.oeInfo.Request.Hash, req.Hash) { return app.oeInfo.Response, app.oeInfo.Error diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 3f44939c9980..845972901f2b 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -683,7 +683,7 @@ func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) (sdk.Context return ctx.WithMultiStore(msCache), msCache } -func (app *BaseApp) beginBlock(req *abci.RequestFinalizeBlock) (sdk.BeginBlock, error) { +func (app *BaseApp) beginBlock(req *abci.RequestFinalizeBlock) sdk.BeginBlock { var ( resp sdk.BeginBlock err error @@ -692,7 +692,7 @@ func (app *BaseApp) beginBlock(req *abci.RequestFinalizeBlock) (sdk.BeginBlock, if app.beginBlocker != nil { resp, err = app.beginBlocker(app.finalizeBlockState.ctx) if err != nil { - return resp, err + panic(err) } // append BeginBlock attributes to all events in the EndBlock response @@ -706,7 +706,7 @@ func (app *BaseApp) beginBlock(req *abci.RequestFinalizeBlock) (sdk.BeginBlock, resp.Events = sdk.MarkEventsToIndex(resp.Events, app.indexEvents) } - return resp, nil + return resp } func (app *BaseApp) deliverTx(tx []byte) *abci.ExecTxResult { diff --git a/baseapp/optimistic_execution.go b/baseapp/optimistic_execution.go index 94913938e326..b4ed92e62d14 100644 --- a/baseapp/optimistic_execution.go +++ b/baseapp/optimistic_execution.go @@ -8,4 +8,5 @@ type OptimisticExecutionInfo struct { Request *abci.RequestFinalizeBlock Response *abci.ResponseFinalizeBlock Error error + Panic interface{} } diff --git a/simapp/upgrades.go b/simapp/upgrades.go index d59e78108d72..3a8bb456d15f 100644 --- a/simapp/upgrades.go +++ b/simapp/upgrades.go @@ -18,6 +18,13 @@ import ( const UpgradeName = "v047-to-v050" func (app SimApp) RegisterUpgradeHandlers() { + app.UpgradeKeeper.SetUpgradeHandler( + "v2", + func(ctx context.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { + return app.ModuleManager.RunMigrations(ctx, app.Configurator(), fromVM) + }, + ) + app.UpgradeKeeper.SetUpgradeHandler( UpgradeName, func(ctx context.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { diff --git a/x/gov/types/v1/params.go b/x/gov/types/v1/params.go index 5633ebb4c59c..bd476ca31b24 100644 --- a/x/gov/types/v1/params.go +++ b/x/gov/types/v1/params.go @@ -86,7 +86,7 @@ func DefaultParams() Params { sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, DefaultMinDepositTokens)), sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, DefaultMinExpeditedDepositTokens)), DefaultPeriod, - DefaultPeriod, + time.Minute*2, DefaultExpeditedPeriod, DefaultQuorum.String(), DefaultThreshold.String(), From 14b80c40c0a3fa90137b1b22eca141c215850367 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Thu, 15 Jun 2023 22:14:21 +0200 Subject: [PATCH 03/29] remove test changes --- simapp/upgrades.go | 7 ------- x/gov/types/v1/params.go | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/simapp/upgrades.go b/simapp/upgrades.go index 3a8bb456d15f..d59e78108d72 100644 --- a/simapp/upgrades.go +++ b/simapp/upgrades.go @@ -18,13 +18,6 @@ import ( const UpgradeName = "v047-to-v050" func (app SimApp) RegisterUpgradeHandlers() { - app.UpgradeKeeper.SetUpgradeHandler( - "v2", - func(ctx context.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { - return app.ModuleManager.RunMigrations(ctx, app.Configurator(), fromVM) - }, - ) - app.UpgradeKeeper.SetUpgradeHandler( UpgradeName, func(ctx context.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { diff --git a/x/gov/types/v1/params.go b/x/gov/types/v1/params.go index bd476ca31b24..5633ebb4c59c 100644 --- a/x/gov/types/v1/params.go +++ b/x/gov/types/v1/params.go @@ -86,7 +86,7 @@ func DefaultParams() Params { sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, DefaultMinDepositTokens)), sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, DefaultMinExpeditedDepositTokens)), DefaultPeriod, - time.Minute*2, + DefaultPeriod, DefaultExpeditedPeriod, DefaultQuorum.String(), DefaultThreshold.String(), From deaf6b76565e2ffa5f108b4c5c660ff664c07df3 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Fri, 16 Jun 2023 20:45:50 +0200 Subject: [PATCH 04/29] fix test --- baseapp/abci_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 9d12ed1ac121..52fbc56a85b3 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -1171,11 +1171,11 @@ func TestABCI_Proposal_HappyPath(t *testing.T) { res, err := suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{ Height: suite.baseApp.LastBlockHeight() + 1, - Txs: [][]byte{txBytes}, + Txs: reqProposalTxBytes[:], }) require.NoError(t, err) - require.Equal(t, 1, pool.CountTx()) + require.Equal(t, 0, pool.CountTx()) require.NotEmpty(t, res.TxResults[0].Events) require.True(t, res.TxResults[0].IsOK(), fmt.Sprintf("%v", res)) From f30e4a71b8b8b20a864dd9e384150c7b94660efb Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Fri, 16 Jun 2023 20:52:24 +0200 Subject: [PATCH 05/29] make comet panic instead of sdk --- baseapp/abci.go | 15 ++++----------- baseapp/baseapp.go | 8 ++++---- baseapp/optimistic_execution.go | 1 - 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index b0e96faf7559..afe003d7f9a1 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -552,12 +552,6 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc } go func() { - defer func() { - if r := recover(); r != nil { - log.Println("⚡️⚡️⚡️ RECOVERED PANIC IN OE") - app.oeInfo.Panic = r - } - }() log.Println("Running OE ✅") app.oeInfo.Response, app.oeInfo.Error = app.internalFinalizeBlock(app.oeInfo.Request) app.oeInfo.Completion <- struct{}{} @@ -737,7 +731,10 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci WithHeaderHash(req.Hash) } - beginBlock := app.beginBlock(req) + beginBlock, err := app.beginBlock(req) + if err != nil { + return nil, err + } events = append(events, beginBlock.Events...) @@ -790,10 +787,6 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons }() if app.oeInfo != nil && app.oeEnabled { - if app.oeInfo.Panic != nil { - panic(app.oeInfo.Panic) - } - <-app.oeInfo.Completion if !app.oeInfo.Aborted && bytes.Equal(app.oeInfo.Request.Hash, req.Hash) { return app.oeInfo.Response, app.oeInfo.Error diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 845972901f2b..7659aef2422c 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -683,7 +683,7 @@ func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) (sdk.Context return ctx.WithMultiStore(msCache), msCache } -func (app *BaseApp) beginBlock(req *abci.RequestFinalizeBlock) sdk.BeginBlock { +func (app *BaseApp) beginBlock(req *abci.RequestFinalizeBlock) (sdk.BeginBlock, error) { var ( resp sdk.BeginBlock err error @@ -692,7 +692,7 @@ func (app *BaseApp) beginBlock(req *abci.RequestFinalizeBlock) sdk.BeginBlock { if app.beginBlocker != nil { resp, err = app.beginBlocker(app.finalizeBlockState.ctx) if err != nil { - panic(err) + return resp, err } // append BeginBlock attributes to all events in the EndBlock response @@ -706,7 +706,7 @@ func (app *BaseApp) beginBlock(req *abci.RequestFinalizeBlock) sdk.BeginBlock { resp.Events = sdk.MarkEventsToIndex(resp.Events, app.indexEvents) } - return resp + return resp, nil } func (app *BaseApp) deliverTx(tx []byte) *abci.ExecTxResult { @@ -754,7 +754,7 @@ func (app *BaseApp) endBlock(ctx context.Context) (sdk.EndBlock, error) { if app.endBlocker != nil { eb, err := app.endBlocker(app.finalizeBlockState.ctx) if err != nil { - panic(err) + return endblock, err } // append EndBlock attributes to all events in the EndBlock response diff --git a/baseapp/optimistic_execution.go b/baseapp/optimistic_execution.go index b4ed92e62d14..94913938e326 100644 --- a/baseapp/optimistic_execution.go +++ b/baseapp/optimistic_execution.go @@ -8,5 +8,4 @@ type OptimisticExecutionInfo struct { Request *abci.RequestFinalizeBlock Response *abci.ResponseFinalizeBlock Error error - Panic interface{} } From 573d107fc685ff4ba8234ecd94c41ebae24ef4db Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Tue, 20 Jun 2023 16:34:18 +0200 Subject: [PATCH 06/29] add abort channel --- baseapp/abci.go | 18 +++++++++++++----- baseapp/optimistic_execution.go | 3 ++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index afe003d7f9a1..15cb06f0d21b 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -536,9 +536,9 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc if app.oeEnabled { if app.oeInfo == nil { - completionSignal := make(chan struct{}) app.oeInfo = &OptimisticExecutionInfo{ - Completion: completionSignal, + Completion: make(chan struct{}), + Abort: make(chan struct{}), Request: &abci.RequestFinalizeBlock{ Txs: req.Txs, DecidedLastCommit: req.ProposedLastCommit, @@ -558,7 +558,8 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc }() } else if !bytes.Equal(app.oeInfo.Request.Hash, req.Hash) { - app.oeInfo.Aborted = true + // if we got a new proposal, abort the previous one + app.oeInfo.Abort <- struct{}{} } } @@ -745,8 +746,15 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci // vote extensions, so skip those. txResults := make([]*abci.ExecTxResult, 0, len(req.Txs)) for _, rawTx := range req.Txs { - if _, err := app.txDecoder(rawTx); err == nil { - txResults = append(txResults, app.deliverTx(rawTx)) + // check before every tx if we should abort + select { + case <-app.oeInfo.Abort: + app.oeInfo.Aborted = true + return nil, nil + default: + if _, err := app.txDecoder(rawTx); err == nil { + txResults = append(txResults, app.deliverTx(rawTx)) + } } } diff --git a/baseapp/optimistic_execution.go b/baseapp/optimistic_execution.go index 94913938e326..5478007a6406 100644 --- a/baseapp/optimistic_execution.go +++ b/baseapp/optimistic_execution.go @@ -3,8 +3,9 @@ package baseapp import abci "github.com/cometbft/cometbft/abci/types" type OptimisticExecutionInfo struct { - Aborted bool Completion chan struct{} + Abort chan struct{} + Aborted bool Request *abci.RequestFinalizeBlock Response *abci.ResponseFinalizeBlock Error error From 17b5ca462798c92cc2c40492997eccd0aa00a47a Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Tue, 20 Jun 2023 18:01:14 +0200 Subject: [PATCH 07/29] fix abort --- baseapp/abci.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 15cb06f0d21b..f7a06ed381c8 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -747,15 +747,19 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci txResults := make([]*abci.ExecTxResult, 0, len(req.Txs)) for _, rawTx := range req.Txs { // check before every tx if we should abort - select { - case <-app.oeInfo.Abort: - app.oeInfo.Aborted = true - return nil, nil - default: - if _, err := app.txDecoder(rawTx); err == nil { - txResults = append(txResults, app.deliverTx(rawTx)) + if app.oeEnabled && app.oeInfo != nil { + select { + case <-app.oeInfo.Abort: + app.oeInfo.Aborted = true + return nil, nil + default: + continue } } + + if _, err := app.txDecoder(rawTx); err == nil { + txResults = append(txResults, app.deliverTx(rawTx)) + } } if app.finalizeBlockState.ms.TracingEnabled() { From d371c163e2206365e9dc59cd1ca561e8ba4436d8 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Tue, 20 Jun 2023 18:53:46 +0200 Subject: [PATCH 08/29] clean up phase1 --- baseapp/abci.go | 53 +++++++++------------- baseapp/optimistic_execution.go | 79 ++++++++++++++++++++++++++++++--- 2 files changed, 92 insertions(+), 40 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index f7a06ed381c8..2b1a20fedaad 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -1,7 +1,6 @@ package baseapp import ( - "bytes" "context" "crypto/sha256" "fmt" @@ -536,30 +535,13 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc if app.oeEnabled { if app.oeInfo == nil { - app.oeInfo = &OptimisticExecutionInfo{ - Completion: make(chan struct{}), - Abort: make(chan struct{}), - Request: &abci.RequestFinalizeBlock{ - Txs: req.Txs, - DecidedLastCommit: req.ProposedLastCommit, - Misbehavior: req.Misbehavior, - Hash: req.Hash, - Height: req.Height, - Time: req.Time, - NextValidatorsHash: req.NextValidatorsHash, - ProposerAddress: req.ProposerAddress, - }, - } - - go func() { - log.Println("Running OE ✅") - app.oeInfo.Response, app.oeInfo.Error = app.internalFinalizeBlock(app.oeInfo.Request) - app.oeInfo.Completion <- struct{}{} - }() - - } else if !bytes.Equal(app.oeInfo.Request.Hash, req.Hash) { - // if we got a new proposal, abort the previous one - app.oeInfo.Abort <- struct{}{} + app.oeInfo = SetupOptimisticExecution(req, app.internalFinalizeBlock) + app.oeInfo.Execute() + } else if app.oeInfo.AbortIfNeeded(req.Hash) { + // TODO: this will block until the OE is aborted, which can take a bit. Maybe do it async? + // if aborted, restart with the new proposal + app.oeInfo = SetupOptimisticExecution(req, app.internalFinalizeBlock) + app.oeInfo.Execute() } } @@ -737,6 +719,14 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci return nil, err } + // First check for an abort signal after beginBlock, as it's the first place + // we spend any significant amount of time. + if app.oeEnabled && app.oeInfo != nil { + if app.oeInfo.ShouldAbort() { + return nil, nil + } + } + events = append(events, beginBlock.Events...) // Iterate over all raw transactions in the proposal and attempt to execute @@ -748,12 +738,8 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci for _, rawTx := range req.Txs { // check before every tx if we should abort if app.oeEnabled && app.oeInfo != nil { - select { - case <-app.oeInfo.Abort: - app.oeInfo.Aborted = true + if app.oeInfo.ShouldAbort() { return nil, nil - default: - continue } } @@ -799,9 +785,10 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons }() if app.oeInfo != nil && app.oeEnabled { - <-app.oeInfo.Completion - if !app.oeInfo.Aborted && bytes.Equal(app.oeInfo.Request.Hash, req.Hash) { - return app.oeInfo.Response, app.oeInfo.Error + // check if the hash we got is the same as the one we are executing + if !app.oeInfo.AbortIfNeeded(req.Hash) { + // no need to abort OE, wait for the result + return app.oeInfo.WaitResult() } } diff --git a/baseapp/optimistic_execution.go b/baseapp/optimistic_execution.go index 5478007a6406..2c666ffa18f4 100644 --- a/baseapp/optimistic_execution.go +++ b/baseapp/optimistic_execution.go @@ -1,12 +1,77 @@ package baseapp -import abci "github.com/cometbft/cometbft/abci/types" +import ( + "bytes" + "log" + + abci "github.com/cometbft/cometbft/abci/types" +) type OptimisticExecutionInfo struct { - Completion chan struct{} - Abort chan struct{} - Aborted bool - Request *abci.RequestFinalizeBlock - Response *abci.ResponseFinalizeBlock - Error error + completeSignal chan struct{} + abortSignal chan struct{} + fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) + Aborted bool + Request *abci.RequestFinalizeBlock + Response *abci.ResponseFinalizeBlock + Error error +} + +func SetupOptimisticExecution( + req *abci.RequestProcessProposal, + fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error), +) *OptimisticExecutionInfo { + return &OptimisticExecutionInfo{ + completeSignal: make(chan struct{}), + abortSignal: make(chan struct{}), + fn: fn, + Request: &abci.RequestFinalizeBlock{ + Txs: req.Txs, + DecidedLastCommit: req.ProposedLastCommit, + Misbehavior: req.Misbehavior, + Hash: req.Hash, + Height: req.Height, + Time: req.Time, + NextValidatorsHash: req.NextValidatorsHash, + ProposerAddress: req.ProposerAddress, + }, + } +} + +func (oe *OptimisticExecutionInfo) Execute() { + go func() { + log.Println("Running OE ✅") + oe.Response, oe.Error = oe.fn(oe.Request) + oe.completeSignal <- struct{}{} + }() +} + +// AbortIfNeeded +// If the request hash is not the same as the one in the OE, then abort the OE +// and wait for the abort to happen. Returns true if the OE was aborted. +func (oe *OptimisticExecutionInfo) AbortIfNeeded(reqHash []byte) bool { + if !bytes.Equal(oe.Request.Hash, reqHash) { + oe.abortSignal <- struct{}{} + // wait for the abort to happen + <-oe.completeSignal + oe.Aborted = true + return true + } + return false +} + +// ShouldAbort must only be used in the fn passed to SetupOptimisticExecution to +// check if the OE was aborted and return as soon as possible. +func (oe *OptimisticExecutionInfo) ShouldAbort() bool { + select { + case <-oe.abortSignal: + return true + default: + return false + } +} + +func (oe *OptimisticExecutionInfo) WaitResult() (*abci.ResponseFinalizeBlock, error) { + <-oe.completeSignal + return oe.Response, oe.Error } From 20f03256d9aad68bf3a6f7420c4bf2d10be6bcbc Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Fri, 30 Jun 2023 12:58:34 +0200 Subject: [PATCH 09/29] testing testing --- baseapp/abci.go | 19 ++++++++----------- baseapp/optimistic_execution.go | 2 ++ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 6e9dfef64824..aeee437b427d 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -532,12 +532,15 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil } - if app.oeEnabled { + // Only execute optimistic execution if OE is enabled and the block height is greater than the initial height. + // This is to simplify the logic and avoid having to deal with the InitChain state. + if app.oeEnabled && req.Height > app.initialHeight { if app.oeInfo == nil { app.oeInfo = SetupOptimisticExecution(req, app.internalFinalizeBlock) app.oeInfo.Execute() } else if app.oeInfo.AbortIfNeeded(req.Hash) { // TODO: this will block until the OE is aborted, which can take a bit. Maybe do it async? + // IMO it's not worth it, we could defer it but that's going to open up a whole can of worms. // if aborted, restart with the new proposal app.oeInfo = SetupOptimisticExecution(req, app.internalFinalizeBlock) app.oeInfo.Execute() @@ -688,14 +691,7 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci // by InitChain. Context is now updated with Header information. app.finalizeBlockState.ctx = app.finalizeBlockState.ctx. WithBlockHeader(header). - WithBlockHeight(req.Height). - WithHeaderInfo(coreheader.Info{ - ChainID: app.chainID, - Height: req.Height, - Time: req.Time, - Hash: req.Hash, - AppHash: app.LastCommitID().Hash, - }) + WithBlockHeight(req.Height) } gasMeter := app.getBlockGasMeter(app.finalizeBlockState.ctx) @@ -732,7 +728,7 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci // First check for an abort signal after beginBlock, as it's the first place // we spend any significant amount of time. - if app.oeEnabled && app.oeInfo != nil { + if app.oeInfo != nil { if app.oeInfo.ShouldAbort() { return nil, nil } @@ -748,11 +744,12 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci txResults := make([]*abci.ExecTxResult, 0, len(req.Txs)) for _, rawTx := range req.Txs { // check before every tx if we should abort - if app.oeEnabled && app.oeInfo != nil { + if app.oeInfo != nil { if app.oeInfo.ShouldAbort() { return nil, nil } } + var response *abci.ExecTxResult if _, err := app.txDecoder(rawTx); err == nil { diff --git a/baseapp/optimistic_execution.go b/baseapp/optimistic_execution.go index 2c666ffa18f4..58d1ed40a7f8 100644 --- a/baseapp/optimistic_execution.go +++ b/baseapp/optimistic_execution.go @@ -8,6 +8,7 @@ import ( ) type OptimisticExecutionInfo struct { + // we could use generics here in the future to allow other types of req/resp completeSignal chan struct{} abortSignal chan struct{} fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) @@ -62,6 +63,7 @@ func (oe *OptimisticExecutionInfo) AbortIfNeeded(reqHash []byte) bool { // ShouldAbort must only be used in the fn passed to SetupOptimisticExecution to // check if the OE was aborted and return as soon as possible. +// TODO: figure out a better name, maybe ReturnEarly? func (oe *OptimisticExecutionInfo) ShouldAbort() bool { select { case <-oe.abortSignal: From b855c1a6de0bfaa6eddee73ee27a6d2d298f3ec5 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Thu, 20 Jul 2023 14:57:24 +0200 Subject: [PATCH 10/29] progress --- baseapp/abci.go | 60 +++++++++++++------------- baseapp/optimistic_execution.go | 75 +++++++++++++++++++++------------ 2 files changed, 78 insertions(+), 57 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index ac6031aa54a5..19bd56c2cb57 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -4,7 +4,6 @@ import ( "context" "crypto/sha256" "fmt" - "log" "sort" "strings" "time" @@ -493,6 +492,12 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc // processed the first block, as we want to avoid overwriting the finalizeState // after state changes during InitChain. if req.Height > app.initialHeight { + // abort any running OE + if app.oeEnabled && app.oeInfo != nil && app.oeInfo.Running() { + app.oeInfo.Abort() + _, _ = app.oeInfo.WaitResult() // ignore the result + } + app.setState(execModeFinalize, header) } @@ -534,18 +539,10 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc } // Only execute optimistic execution if OE is enabled and the block height is greater than the initial height. - // This is to simplify the logic and avoid having to deal with the InitChain state. + // During the first block we'll be carrying state from InitChain, so it would be impossible for us to easily revert. if app.oeEnabled && req.Height > app.initialHeight { - if app.oeInfo == nil { - app.oeInfo = SetupOptimisticExecution(req, app.internalFinalizeBlock) - app.oeInfo.Execute() - } else if app.oeInfo.AbortIfNeeded(req.Hash) { - // TODO: this will block until the OE is aborted, which can take a bit. Maybe do it async? - // IMO it's not worth it, we could defer it but that's going to open up a whole can of worms. - // if aborted, restart with the new proposal - app.oeInfo = SetupOptimisticExecution(req, app.internalFinalizeBlock) - app.oeInfo.Execute() - } + app.oeInfo = SetupOptimisticExecution(req, app.internalFinalizeBlock) + app.oeInfo.Execute() } return resp, nil @@ -733,10 +730,8 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci // First check for an abort signal after beginBlock, as it's the first place // we spend any significant amount of time. - if app.oeInfo != nil { - if app.oeInfo.ShouldAbort() { - return nil, nil - } + if app.oeInfo != nil && app.oeInfo.ShouldAbort() { + return nil, nil } events = append(events, beginBlock.Events...) @@ -749,10 +744,8 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci txResults := make([]*abci.ExecTxResult, 0, len(req.Txs)) for _, rawTx := range req.Txs { // check before every tx if we should abort - if app.oeInfo != nil { - if app.oeInfo.ShouldAbort() { - return nil, nil - } + if app.oeInfo != nil && app.oeInfo.ShouldAbort() { + return nil, nil } var response *abci.ExecTxResult @@ -792,7 +785,7 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci TxResults: txResults, ValidatorUpdates: endBlock.ValidatorUpdates, ConsensusParamUpdates: &cp, - AppHash: app.workingHash(), + // AppHash: app.workingHash(), }, nil } @@ -807,20 +800,27 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci // extensions into the proposal, which should not themselves be executed in cases // where they adhere to the sdk.Tx interface. func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { - defer func() { - app.oeInfo = nil - }() - if app.oeInfo != nil && app.oeEnabled { // check if the hash we got is the same as the one we are executing - if !app.oeInfo.AbortIfNeeded(req.Hash) { - // no need to abort OE, wait for the result - return app.oeInfo.WaitResult() + aborted := app.oeInfo.AbortIfNeeded(req.Hash) + // Wait for the OE to finish, regardless of whether it was aborted or not + res, err := app.oeInfo.WaitResult() + + // only return if we are not aborting + if !aborted { + res.AppHash = app.workingHash() + return res, err } + + // if it was aborted, we need to reset the state and continue + app.finalizeBlockState = nil } - log.Println("NOT running OE ❌") - return app.internalFinalizeBlock(req) + // if no OE is running, just run the block (this is either a block replay or a OE that got aborted) + app.oeInfo = nil + res, err := app.internalFinalizeBlock(req) + res.AppHash = app.workingHash() + return res, err } // checkHalt checkes if height or time exceeds halt-height or halt-time respectively. diff --git a/baseapp/optimistic_execution.go b/baseapp/optimistic_execution.go index 58d1ed40a7f8..3f20052c4dc7 100644 --- a/baseapp/optimistic_execution.go +++ b/baseapp/optimistic_execution.go @@ -3,19 +3,25 @@ package baseapp import ( "bytes" "log" + "math/rand" + "sync" + "time" abci "github.com/cometbft/cometbft/abci/types" ) type OptimisticExecutionInfo struct { + mtx sync.RWMutex + stopCh chan struct{} + shouldAbort bool + running bool + // we could use generics here in the future to allow other types of req/resp - completeSignal chan struct{} - abortSignal chan struct{} - fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) - Aborted bool - Request *abci.RequestFinalizeBlock - Response *abci.ResponseFinalizeBlock - Error error + fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) + Request *abci.RequestFinalizeBlock + Response *abci.ResponseFinalizeBlock + Error error + executionTime time.Duration } func SetupOptimisticExecution( @@ -23,9 +29,8 @@ func SetupOptimisticExecution( fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error), ) *OptimisticExecutionInfo { return &OptimisticExecutionInfo{ - completeSignal: make(chan struct{}), - abortSignal: make(chan struct{}), - fn: fn, + stopCh: make(chan struct{}), + fn: fn, Request: &abci.RequestFinalizeBlock{ Txs: req.Txs, DecidedLastCommit: req.ProposedLastCommit, @@ -40,10 +45,17 @@ func SetupOptimisticExecution( } func (oe *OptimisticExecutionInfo) Execute() { + log.Println("Start OE ✅") + start := time.Now() + oe.running = true go func() { - log.Println("Running OE ✅") - oe.Response, oe.Error = oe.fn(oe.Request) - oe.completeSignal <- struct{}{} + resp, err := oe.fn(oe.Request) + oe.mtx.Lock() + oe.executionTime = time.Since(start) + oe.Response, oe.Error = resp, err + oe.running = false + close(oe.stopCh) + oe.mtx.Unlock() }() } @@ -51,29 +63,38 @@ func (oe *OptimisticExecutionInfo) Execute() { // If the request hash is not the same as the one in the OE, then abort the OE // and wait for the abort to happen. Returns true if the OE was aborted. func (oe *OptimisticExecutionInfo) AbortIfNeeded(reqHash []byte) bool { - if !bytes.Equal(oe.Request.Hash, reqHash) { - oe.abortSignal <- struct{}{} - // wait for the abort to happen - <-oe.completeSignal - oe.Aborted = true - return true + oe.mtx.Lock() + defer oe.mtx.Unlock() + if rand.Intn(100) > 80 || !bytes.Equal(oe.Request.Hash, reqHash) { + log.Println("OE aborted ❌") + oe.shouldAbort = true } - return false + return oe.shouldAbort +} + +func (oe *OptimisticExecutionInfo) Abort() { + oe.mtx.Lock() + defer oe.mtx.Unlock() + oe.shouldAbort = true } // ShouldAbort must only be used in the fn passed to SetupOptimisticExecution to // check if the OE was aborted and return as soon as possible. // TODO: figure out a better name, maybe ReturnEarly? func (oe *OptimisticExecutionInfo) ShouldAbort() bool { - select { - case <-oe.abortSignal: - return true - default: - return false - } + defer oe.mtx.RUnlock() + oe.mtx.RLock() + return oe.shouldAbort +} + +func (oe *OptimisticExecutionInfo) Running() bool { + defer oe.mtx.RUnlock() + oe.mtx.RLock() + return oe.running } func (oe *OptimisticExecutionInfo) WaitResult() (*abci.ResponseFinalizeBlock, error) { - <-oe.completeSignal + <-oe.stopCh + log.Println("OE took ⏱", oe.executionTime) return oe.Response, oe.Error } From 265e32d4e172ee59bb3c1f4e85fa6b08c522825b Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Fri, 21 Jul 2023 14:16:53 +0200 Subject: [PATCH 11/29] fix --- baseapp/abci.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 19bd56c2cb57..c01a3c905ad1 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -808,7 +808,9 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons // only return if we are not aborting if !aborted { - res.AppHash = app.workingHash() + if res != nil { + res.AppHash = app.workingHash() + } return res, err } @@ -819,7 +821,9 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons // if no OE is running, just run the block (this is either a block replay or a OE that got aborted) app.oeInfo = nil res, err := app.internalFinalizeBlock(req) - res.AppHash = app.workingHash() + if res != nil { + res.AppHash = app.workingHash() + } return res, err } From c835fa75da60fa6de5590108d429331a66592c90 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Thu, 27 Jul 2023 12:12:07 +0200 Subject: [PATCH 12/29] progress --- baseapp/abci.go | 25 +++++----- baseapp/baseapp.go | 25 +++++----- baseapp/{ => oe}/optimistic_execution.go | 61 +++++++++++++----------- baseapp/options.go | 5 ++ 4 files changed, 63 insertions(+), 53 deletions(-) rename baseapp/{ => oe}/optimistic_execution.go (53%) diff --git a/baseapp/abci.go b/baseapp/abci.go index c01a3c905ad1..3d91b4889ffd 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -21,6 +21,7 @@ import ( snapshottypes "cosmossdk.io/store/snapshots/types" storetypes "cosmossdk.io/store/types" + "github.com/cosmos/cosmos-sdk/baseapp/oe" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" @@ -493,9 +494,9 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc // after state changes during InitChain. if req.Height > app.initialHeight { // abort any running OE - if app.oeEnabled && app.oeInfo != nil && app.oeInfo.Running() { - app.oeInfo.Abort() - _, _ = app.oeInfo.WaitResult() // ignore the result + if app.optimisticExecEnabled && app.optimisticExec != nil && app.optimisticExec.Running() { + app.optimisticExec.Abort() + _, _ = app.optimisticExec.WaitResult() // ignore the result } app.setState(execModeFinalize, header) @@ -540,9 +541,8 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc // Only execute optimistic execution if OE is enabled and the block height is greater than the initial height. // During the first block we'll be carrying state from InitChain, so it would be impossible for us to easily revert. - if app.oeEnabled && req.Height > app.initialHeight { - app.oeInfo = SetupOptimisticExecution(req, app.internalFinalizeBlock) - app.oeInfo.Execute() + if app.optimisticExecEnabled && req.Height > app.initialHeight { + app.optimisticExec = oe.Execute(req, app.internalFinalizeBlock, app.logger) } return resp, nil @@ -730,7 +730,7 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci // First check for an abort signal after beginBlock, as it's the first place // we spend any significant amount of time. - if app.oeInfo != nil && app.oeInfo.ShouldAbort() { + if app.optimisticExec != nil && app.optimisticExec.ShouldAbort() { return nil, nil } @@ -744,7 +744,7 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci txResults := make([]*abci.ExecTxResult, 0, len(req.Txs)) for _, rawTx := range req.Txs { // check before every tx if we should abort - if app.oeInfo != nil && app.oeInfo.ShouldAbort() { + if app.optimisticExec != nil && app.optimisticExec.ShouldAbort() { return nil, nil } @@ -785,7 +785,6 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci TxResults: txResults, ValidatorUpdates: endBlock.ValidatorUpdates, ConsensusParamUpdates: &cp, - // AppHash: app.workingHash(), }, nil } @@ -800,11 +799,11 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci // extensions into the proposal, which should not themselves be executed in cases // where they adhere to the sdk.Tx interface. func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { - if app.oeInfo != nil && app.oeEnabled { + if app.optimisticExec != nil && app.optimisticExecEnabled { // check if the hash we got is the same as the one we are executing - aborted := app.oeInfo.AbortIfNeeded(req.Hash) + aborted := app.optimisticExec.AbortIfNeeded(req.Hash) // Wait for the OE to finish, regardless of whether it was aborted or not - res, err := app.oeInfo.WaitResult() + res, err := app.optimisticExec.WaitResult() // only return if we are not aborting if !aborted { @@ -819,7 +818,7 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons } // if no OE is running, just run the block (this is either a block replay or a OE that got aborted) - app.oeInfo = nil + app.optimisticExec = nil res, err := app.internalFinalizeBlock(req) if res != nil { res.AppHash = app.workingHash() diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index e394b62b2a38..15a9307d110a 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -22,6 +22,7 @@ import ( "cosmossdk.io/store/snapshots" storetypes "cosmossdk.io/store/types" + "github.com/cosmos/cosmos-sdk/baseapp/oe" "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" servertypes "github.com/cosmos/cosmos-sdk/server/types" @@ -181,8 +182,8 @@ type BaseApp struct { cdc codec.Codec - oeInfo *OptimisticExecutionInfo - oeEnabled bool + optimisticExec *oe.OptimisticExecution + optimisticExecEnabled bool } // NewBaseApp returns a reference to an initialized BaseApp. It accepts a @@ -192,16 +193,16 @@ func NewBaseApp( name string, logger log.Logger, db dbm.DB, txDecoder sdk.TxDecoder, options ...func(*BaseApp), ) *BaseApp { app := &BaseApp{ - logger: logger, - name: name, - db: db, - cms: store.NewCommitMultiStore(db, logger, storemetrics.NewNoOpMetrics()), // by default we use a no-op metric gather in store - storeLoader: DefaultStoreLoader, - grpcQueryRouter: NewGRPCQueryRouter(), - msgServiceRouter: NewMsgServiceRouter(), - txDecoder: txDecoder, - fauxMerkleMode: false, - oeEnabled: true, + logger: logger, + name: name, + db: db, + cms: store.NewCommitMultiStore(db, logger, storemetrics.NewNoOpMetrics()), // by default we use a no-op metric gather in store + storeLoader: DefaultStoreLoader, + grpcQueryRouter: NewGRPCQueryRouter(), + msgServiceRouter: NewMsgServiceRouter(), + txDecoder: txDecoder, + fauxMerkleMode: false, + optimisticExecEnabled: true, } for _, option := range options { diff --git a/baseapp/optimistic_execution.go b/baseapp/oe/optimistic_execution.go similarity index 53% rename from baseapp/optimistic_execution.go rename to baseapp/oe/optimistic_execution.go index 3f20052c4dc7..95ab7c01eec9 100644 --- a/baseapp/optimistic_execution.go +++ b/baseapp/oe/optimistic_execution.go @@ -1,16 +1,16 @@ -package baseapp +package oe import ( "bytes" - "log" "math/rand" "sync" "time" + "cosmossdk.io/log" abci "github.com/cometbft/cometbft/abci/types" ) -type OptimisticExecutionInfo struct { +type OptimisticExecution struct { mtx sync.RWMutex stopCh chan struct{} shouldAbort bool @@ -18,20 +18,23 @@ type OptimisticExecutionInfo struct { // we could use generics here in the future to allow other types of req/resp fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) - Request *abci.RequestFinalizeBlock - Response *abci.ResponseFinalizeBlock - Error error + request *abci.RequestFinalizeBlock + response *abci.ResponseFinalizeBlock + err error executionTime time.Duration + logger log.Logger } -func SetupOptimisticExecution( +// Execute initializes the OE and starts it in a goroutine. +func Execute( req *abci.RequestProcessProposal, fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error), -) *OptimisticExecutionInfo { - return &OptimisticExecutionInfo{ + logger log.Logger, +) *OptimisticExecution { + oe := &OptimisticExecution{ stopCh: make(chan struct{}), fn: fn, - Request: &abci.RequestFinalizeBlock{ + request: &abci.RequestFinalizeBlock{ Txs: req.Txs, DecidedLastCommit: req.ProposedLastCommit, Misbehavior: req.Misbehavior, @@ -41,38 +44,40 @@ func SetupOptimisticExecution( NextValidatorsHash: req.NextValidatorsHash, ProposerAddress: req.ProposerAddress, }, + logger: logger, } -} -func (oe *OptimisticExecutionInfo) Execute() { - log.Println("Start OE ✅") + oe.logger.Debug("OE started") start := time.Now() oe.running = true go func() { - resp, err := oe.fn(oe.Request) + resp, err := oe.fn(oe.request) oe.mtx.Lock() oe.executionTime = time.Since(start) - oe.Response, oe.Error = resp, err + oe.logger.Debug("OE finished", "duration", oe.executionTime) + oe.response, oe.err = resp, err oe.running = false close(oe.stopCh) oe.mtx.Unlock() }() + + return oe } -// AbortIfNeeded -// If the request hash is not the same as the one in the OE, then abort the OE -// and wait for the abort to happen. Returns true if the OE was aborted. -func (oe *OptimisticExecutionInfo) AbortIfNeeded(reqHash []byte) bool { +// AbortIfNeeded aborts the OE if the request hash is not the same as the one in +// the running OE. Returns true if the OE was aborted. +func (oe *OptimisticExecution) AbortIfNeeded(reqHash []byte) bool { oe.mtx.Lock() defer oe.mtx.Unlock() - if rand.Intn(100) > 80 || !bytes.Equal(oe.Request.Hash, reqHash) { - log.Println("OE aborted ❌") + if rand.Intn(100) > 80 || !bytes.Equal(oe.request.Hash, reqHash) { + oe.logger.Debug("OE aborted") oe.shouldAbort = true } return oe.shouldAbort } -func (oe *OptimisticExecutionInfo) Abort() { +// Abort aborts the OE unconditionally. +func (oe *OptimisticExecution) Abort() { oe.mtx.Lock() defer oe.mtx.Unlock() oe.shouldAbort = true @@ -80,21 +85,21 @@ func (oe *OptimisticExecutionInfo) Abort() { // ShouldAbort must only be used in the fn passed to SetupOptimisticExecution to // check if the OE was aborted and return as soon as possible. -// TODO: figure out a better name, maybe ReturnEarly? -func (oe *OptimisticExecutionInfo) ShouldAbort() bool { +func (oe *OptimisticExecution) ShouldAbort() bool { defer oe.mtx.RUnlock() oe.mtx.RLock() return oe.shouldAbort } -func (oe *OptimisticExecutionInfo) Running() bool { +// Running returns true if the OE is still running. +func (oe *OptimisticExecution) Running() bool { defer oe.mtx.RUnlock() oe.mtx.RLock() return oe.running } -func (oe *OptimisticExecutionInfo) WaitResult() (*abci.ResponseFinalizeBlock, error) { +// WaitResult waits for the OE to finish and returns the result. +func (oe *OptimisticExecution) WaitResult() (*abci.ResponseFinalizeBlock, error) { <-oe.stopCh - log.Println("OE took ⏱", oe.executionTime) - return oe.Response, oe.Error + return oe.response, oe.err } diff --git a/baseapp/options.go b/baseapp/options.go index fbb15a6c5b17..46b6de62ca1c 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -94,6 +94,11 @@ func SetChainID(chainID string) func(*BaseApp) { return func(app *BaseApp) { app.chainID = chainID } } +// SetOptimisticExecution enables or disables optimistic execution. +func SetOptimisticExecution(enable bool) func(*BaseApp) { + return func(app *BaseApp) { app.optimisticExecEnabled = enable } +} + func (app *BaseApp) SetName(name string) { if app.sealed { panic("SetName() on sealed BaseApp") From 06cb990c5d545d69066ef52f8d0069ace0821ebe Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Thu, 27 Jul 2023 13:50:28 +0200 Subject: [PATCH 13/29] lint --- baseapp/oe/optimistic_execution.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/baseapp/oe/optimistic_execution.go b/baseapp/oe/optimistic_execution.go index 95ab7c01eec9..45d4180bdb6a 100644 --- a/baseapp/oe/optimistic_execution.go +++ b/baseapp/oe/optimistic_execution.go @@ -6,8 +6,9 @@ import ( "sync" "time" - "cosmossdk.io/log" abci "github.com/cometbft/cometbft/abci/types" + + "cosmossdk.io/log" ) type OptimisticExecution struct { From f2aec1db781527d1ffde72391dee97e0845cef72 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Thu, 27 Jul 2023 16:21:32 +0200 Subject: [PATCH 14/29] progress --- baseapp/abci.go | 9 ++++----- baseapp/baseapp.go | 23 ++++++++++----------- baseapp/oe/optimistic_execution.go | 32 +++++++++++++----------------- baseapp/options.go | 3 ++- 4 files changed, 31 insertions(+), 36 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 3d91b4889ffd..9d7fe4affe41 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -21,7 +21,6 @@ import ( snapshottypes "cosmossdk.io/store/snapshots/types" storetypes "cosmossdk.io/store/types" - "github.com/cosmos/cosmos-sdk/baseapp/oe" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" @@ -494,7 +493,7 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc // after state changes during InitChain. if req.Height > app.initialHeight { // abort any running OE - if app.optimisticExecEnabled && app.optimisticExec != nil && app.optimisticExec.Running() { + if app.optimisticExec != nil && app.optimisticExec.Running() { app.optimisticExec.Abort() _, _ = app.optimisticExec.WaitResult() // ignore the result } @@ -541,8 +540,8 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc // Only execute optimistic execution if OE is enabled and the block height is greater than the initial height. // During the first block we'll be carrying state from InitChain, so it would be impossible for us to easily revert. - if app.optimisticExecEnabled && req.Height > app.initialHeight { - app.optimisticExec = oe.Execute(req, app.internalFinalizeBlock, app.logger) + if app.optimisticExec != nil && req.Height > app.initialHeight { + app.optimisticExec.Execute(req, app.internalFinalizeBlock, app.logger) } return resp, nil @@ -799,7 +798,7 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci // extensions into the proposal, which should not themselves be executed in cases // where they adhere to the sdk.Tx interface. func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { - if app.optimisticExec != nil && app.optimisticExecEnabled { + if app.optimisticExec != nil { // check if the hash we got is the same as the one we are executing aborted := app.optimisticExec.AbortIfNeeded(req.Hash) // Wait for the OE to finish, regardless of whether it was aborted or not diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 15a9307d110a..dd8893c09c4f 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -182,8 +182,7 @@ type BaseApp struct { cdc codec.Codec - optimisticExec *oe.OptimisticExecution - optimisticExecEnabled bool + optimisticExec *oe.OptimisticExecution } // NewBaseApp returns a reference to an initialized BaseApp. It accepts a @@ -193,16 +192,16 @@ func NewBaseApp( name string, logger log.Logger, db dbm.DB, txDecoder sdk.TxDecoder, options ...func(*BaseApp), ) *BaseApp { app := &BaseApp{ - logger: logger, - name: name, - db: db, - cms: store.NewCommitMultiStore(db, logger, storemetrics.NewNoOpMetrics()), // by default we use a no-op metric gather in store - storeLoader: DefaultStoreLoader, - grpcQueryRouter: NewGRPCQueryRouter(), - msgServiceRouter: NewMsgServiceRouter(), - txDecoder: txDecoder, - fauxMerkleMode: false, - optimisticExecEnabled: true, + logger: logger, + name: name, + db: db, + cms: store.NewCommitMultiStore(db, logger, storemetrics.NewNoOpMetrics()), // by default we use a no-op metric gather in store + storeLoader: DefaultStoreLoader, + grpcQueryRouter: NewGRPCQueryRouter(), + msgServiceRouter: NewMsgServiceRouter(), + txDecoder: txDecoder, + fauxMerkleMode: false, + optimisticExec: &oe.OptimisticExecution{}, } for _, option := range options { diff --git a/baseapp/oe/optimistic_execution.go b/baseapp/oe/optimistic_execution.go index 45d4180bdb6a..7f63f6b03d72 100644 --- a/baseapp/oe/optimistic_execution.go +++ b/baseapp/oe/optimistic_execution.go @@ -27,26 +27,24 @@ type OptimisticExecution struct { } // Execute initializes the OE and starts it in a goroutine. -func Execute( +func (oe *OptimisticExecution) Execute( req *abci.RequestProcessProposal, fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error), logger log.Logger, -) *OptimisticExecution { - oe := &OptimisticExecution{ - stopCh: make(chan struct{}), - fn: fn, - request: &abci.RequestFinalizeBlock{ - Txs: req.Txs, - DecidedLastCommit: req.ProposedLastCommit, - Misbehavior: req.Misbehavior, - Hash: req.Hash, - Height: req.Height, - Time: req.Time, - NextValidatorsHash: req.NextValidatorsHash, - ProposerAddress: req.ProposerAddress, - }, - logger: logger, +) { + oe.stopCh = make(chan struct{}) + oe.fn = fn + oe.request = &abci.RequestFinalizeBlock{ + Txs: req.Txs, + DecidedLastCommit: req.ProposedLastCommit, + Misbehavior: req.Misbehavior, + Hash: req.Hash, + Height: req.Height, + Time: req.Time, + NextValidatorsHash: req.NextValidatorsHash, + ProposerAddress: req.ProposerAddress, } + oe.logger = logger oe.logger.Debug("OE started") start := time.Now() @@ -61,8 +59,6 @@ func Execute( close(oe.stopCh) oe.mtx.Unlock() }() - - return oe } // AbortIfNeeded aborts the OE if the request hash is not the same as the one in diff --git a/baseapp/options.go b/baseapp/options.go index 46b6de62ca1c..e2a35c07336e 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -12,6 +12,7 @@ import ( snapshottypes "cosmossdk.io/store/snapshots/types" storetypes "cosmossdk.io/store/types" + "github.com/cosmos/cosmos-sdk/baseapp/oe" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -96,7 +97,7 @@ func SetChainID(chainID string) func(*BaseApp) { // SetOptimisticExecution enables or disables optimistic execution. func SetOptimisticExecution(enable bool) func(*BaseApp) { - return func(app *BaseApp) { app.optimisticExecEnabled = enable } + return func(app *BaseApp) { app.optimisticExec = &oe.OptimisticExecution{} } } func (app *BaseApp) SetName(name string) { From 18b666e067128e94cc003a1b53859f2cd0f40533 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Mon, 31 Jul 2023 16:52:31 +0200 Subject: [PATCH 15/29] fix race condition --- baseapp/baseapp.go | 2 +- baseapp/oe/optimistic_execution.go | 9 ++++++++- baseapp/options.go | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index dd8893c09c4f..480e82474f28 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -201,7 +201,7 @@ func NewBaseApp( msgServiceRouter: NewMsgServiceRouter(), txDecoder: txDecoder, fauxMerkleMode: false, - optimisticExec: &oe.OptimisticExecution{}, + optimisticExec: oe.NewOptimisticExecution(), } for _, option := range options { diff --git a/baseapp/oe/optimistic_execution.go b/baseapp/oe/optimistic_execution.go index 7f63f6b03d72..57fd2ab454c2 100644 --- a/baseapp/oe/optimistic_execution.go +++ b/baseapp/oe/optimistic_execution.go @@ -26,12 +26,19 @@ type OptimisticExecution struct { logger log.Logger } +func NewOptimisticExecution() *OptimisticExecution { + return &OptimisticExecution{} +} + // Execute initializes the OE and starts it in a goroutine. func (oe *OptimisticExecution) Execute( req *abci.RequestProcessProposal, fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error), logger log.Logger, ) { + oe.mtx.Lock() + defer oe.mtx.Unlock() + oe.stopCh = make(chan struct{}) oe.fn = fn oe.request = &abci.RequestFinalizeBlock{ @@ -83,8 +90,8 @@ func (oe *OptimisticExecution) Abort() { // ShouldAbort must only be used in the fn passed to SetupOptimisticExecution to // check if the OE was aborted and return as soon as possible. func (oe *OptimisticExecution) ShouldAbort() bool { - defer oe.mtx.RUnlock() oe.mtx.RLock() + defer oe.mtx.RUnlock() return oe.shouldAbort } diff --git a/baseapp/options.go b/baseapp/options.go index e2a35c07336e..0dc89e685cc6 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -97,7 +97,7 @@ func SetChainID(chainID string) func(*BaseApp) { // SetOptimisticExecution enables or disables optimistic execution. func SetOptimisticExecution(enable bool) func(*BaseApp) { - return func(app *BaseApp) { app.optimisticExec = &oe.OptimisticExecution{} } + return func(app *BaseApp) { app.optimisticExec = oe.NewOptimisticExecution() } } func (app *BaseApp) SetName(name string) { From 125e942897f3259105b5f8fd07dd05e1914dcc86 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Mon, 31 Jul 2023 17:17:33 +0200 Subject: [PATCH 16/29] progress --- baseapp/abci.go | 27 ++++++++++++++---------- baseapp/baseapp.go | 3 ++- baseapp/oe/optimistic_execution.go | 33 +++++++++++++++++++++++------- baseapp/options.go | 8 +++++--- 4 files changed, 49 insertions(+), 22 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index b258eeaf6094..0646bfb9d509 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -493,7 +493,7 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc // after state changes during InitChain. if req.Height > app.initialHeight { // abort any running OE - if app.optimisticExec != nil && app.optimisticExec.Running() { + if app.optimisticExec.Running() { app.optimisticExec.Abort() _, _ = app.optimisticExec.WaitResult() // ignore the result } @@ -540,8 +540,8 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc // Only execute optimistic execution if OE is enabled and the block height is greater than the initial height. // During the first block we'll be carrying state from InitChain, so it would be impossible for us to easily revert. - if app.optimisticExec != nil && req.Height > app.initialHeight { - app.optimisticExec.Execute(req, app.internalFinalizeBlock, app.logger) + if app.optimisticExec.Enabled() && req.Height > app.initialHeight { + app.optimisticExec.Execute(req) } return resp, nil @@ -729,7 +729,7 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci // First check for an abort signal after beginBlock, as it's the first place // we spend any significant amount of time. - if app.optimisticExec != nil && app.optimisticExec.ShouldAbort() { + if app.optimisticExec.ShouldAbort() { return nil, nil } @@ -742,11 +742,6 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci // vote extensions, so skip those. txResults := make([]*abci.ExecTxResult, 0, len(req.Txs)) for _, rawTx := range req.Txs { - // check before every tx if we should abort - if app.optimisticExec != nil && app.optimisticExec.ShouldAbort() { - return nil, nil - } - var response *abci.ExecTxResult if _, err := app.txDecoder(rawTx); err == nil { @@ -764,6 +759,11 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci ) } + // check after every tx if we should abort + if app.optimisticExec.ShouldAbort() { + return nil, nil + } + txResults = append(txResults, response) } @@ -776,6 +776,11 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci return nil, err } + // check after endBlock if we should abort, to avoid propagating the result + if app.optimisticExec.ShouldAbort() { + return nil, nil + } + events = append(events, endBlock.Events...) cp := app.GetConsensusParams(app.finalizeBlockState.ctx) @@ -798,7 +803,7 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci // extensions into the proposal, which should not themselves be executed in cases // where they adhere to the sdk.Tx interface. func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { - if app.optimisticExec != nil { + if app.optimisticExec.Enabled() { // check if the hash we got is the same as the one we are executing aborted := app.optimisticExec.AbortIfNeeded(req.Hash) // Wait for the OE to finish, regardless of whether it was aborted or not @@ -817,7 +822,7 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons } // if no OE is running, just run the block (this is either a block replay or a OE that got aborted) - app.optimisticExec = nil + app.optimisticExec.Reset() res, err := app.internalFinalizeBlock(req) if res != nil { res.AppHash = app.workingHash() diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 480e82474f28..a924bb8c09ce 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -201,9 +201,10 @@ func NewBaseApp( msgServiceRouter: NewMsgServiceRouter(), txDecoder: txDecoder, fauxMerkleMode: false, - optimisticExec: oe.NewOptimisticExecution(), } + SetOptimisticExecution()(app) + for _, option := range options { option(app) } diff --git a/baseapp/oe/optimistic_execution.go b/baseapp/oe/optimistic_execution.go index 57fd2ab454c2..e0029570322c 100644 --- a/baseapp/oe/optimistic_execution.go +++ b/baseapp/oe/optimistic_execution.go @@ -26,21 +26,33 @@ type OptimisticExecution struct { logger log.Logger } -func NewOptimisticExecution() *OptimisticExecution { - return &OptimisticExecution{} +func NewOptimisticExecution(logger log.Logger, fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error)) *OptimisticExecution { + return &OptimisticExecution{logger: logger, fn: fn} +} + +func (oe *OptimisticExecution) Reset() { + oe.mtx.Lock() + defer oe.mtx.Unlock() + oe.request = nil + oe.response = nil + oe.err = nil + oe.executionTime = 0 + oe.shouldAbort = false + oe.running = false +} + +func (oe *OptimisticExecution) Enabled() bool { + return oe != nil } // Execute initializes the OE and starts it in a goroutine. func (oe *OptimisticExecution) Execute( req *abci.RequestProcessProposal, - fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error), - logger log.Logger, ) { oe.mtx.Lock() defer oe.mtx.Unlock() oe.stopCh = make(chan struct{}) - oe.fn = fn oe.request = &abci.RequestFinalizeBlock{ Txs: req.Txs, DecidedLastCommit: req.ProposedLastCommit, @@ -51,7 +63,6 @@ func (oe *OptimisticExecution) Execute( NextValidatorsHash: req.NextValidatorsHash, ProposerAddress: req.ProposerAddress, } - oe.logger = logger oe.logger.Debug("OE started") start := time.Now() @@ -90,6 +101,10 @@ func (oe *OptimisticExecution) Abort() { // ShouldAbort must only be used in the fn passed to SetupOptimisticExecution to // check if the OE was aborted and return as soon as possible. func (oe *OptimisticExecution) ShouldAbort() bool { + if oe == nil { + return false + } + oe.mtx.RLock() defer oe.mtx.RUnlock() return oe.shouldAbort @@ -97,8 +112,12 @@ func (oe *OptimisticExecution) ShouldAbort() bool { // Running returns true if the OE is still running. func (oe *OptimisticExecution) Running() bool { - defer oe.mtx.RUnlock() + if oe == nil { + return false + } + oe.mtx.RLock() + defer oe.mtx.RUnlock() return oe.running } diff --git a/baseapp/options.go b/baseapp/options.go index 0dc89e685cc6..91bd397125e7 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -95,9 +95,11 @@ func SetChainID(chainID string) func(*BaseApp) { return func(app *BaseApp) { app.chainID = chainID } } -// SetOptimisticExecution enables or disables optimistic execution. -func SetOptimisticExecution(enable bool) func(*BaseApp) { - return func(app *BaseApp) { app.optimisticExec = oe.NewOptimisticExecution() } +// SetOptimisticExecution enables optimistic execution. +func SetOptimisticExecution() func(*BaseApp) { + return func(app *BaseApp) { + app.optimisticExec = oe.NewOptimisticExecution(app.logger, app.internalFinalizeBlock) + } } func (app *BaseApp) SetName(name string) { From 0f1ad3b9921bb0d43b04df5c8999885e4b9deaae Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Tue, 1 Aug 2023 10:35:07 +0200 Subject: [PATCH 17/29] progress --- Makefile | 2 +- baseapp/abci.go | 8 +++----- baseapp/baseapp.go | 2 -- baseapp/oe/optimistic_execution.go | 7 +++++++ server/util.go | 1 + 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index bd18ff583402..a809b03d7ba5 100644 --- a/Makefile +++ b/Makefile @@ -472,7 +472,7 @@ localnet-stop: # localnet-start will run a 4-node testnet locally. The nodes are # based off the docker images in: ./contrib/images/simd-env -localnet-start: localnet-stop localnet-build-env localnet-build-nodes +localnet-start: localnet-stop localnet-build-nodes # localnet-debug will run a 4-node testnet locally in debug mode # you can read more about the debug mode here: ./contrib/images/simd-dlv/README.md diff --git a/baseapp/abci.go b/baseapp/abci.go index 0646bfb9d509..f6b4557b6bcc 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -811,18 +811,16 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons // only return if we are not aborting if !aborted { - if res != nil { - res.AppHash = app.workingHash() - } + res.AppHash = app.workingHash() return res, err } - // if it was aborted, we need to reset the state and continue + // if it was aborted, we need to reset the state app.finalizeBlockState = nil + app.optimisticExec.Reset() } // if no OE is running, just run the block (this is either a block replay or a OE that got aborted) - app.optimisticExec.Reset() res, err := app.internalFinalizeBlock(req) if res != nil { res.AppHash = app.workingHash() diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index a924bb8c09ce..cd01eba68361 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -203,8 +203,6 @@ func NewBaseApp( fauxMerkleMode: false, } - SetOptimisticExecution()(app) - for _, option := range options { option(app) } diff --git a/baseapp/oe/optimistic_execution.go b/baseapp/oe/optimistic_execution.go index e0029570322c..5502de1e5c28 100644 --- a/baseapp/oe/optimistic_execution.go +++ b/baseapp/oe/optimistic_execution.go @@ -30,6 +30,9 @@ func NewOptimisticExecution(logger log.Logger, fn func(*abci.RequestFinalizeBloc return &OptimisticExecution{logger: logger, fn: fn} } +// Reset resets the OE context. Must be called whenever we want to invalidate +// the current OE. For example when on FinalizeBlock we want to process the +// block async, we run Reset() to make sure ShouldAbort() returns always false. func (oe *OptimisticExecution) Reset() { oe.mtx.Lock() defer oe.mtx.Unlock() @@ -82,6 +85,10 @@ func (oe *OptimisticExecution) Execute( // AbortIfNeeded aborts the OE if the request hash is not the same as the one in // the running OE. Returns true if the OE was aborted. func (oe *OptimisticExecution) AbortIfNeeded(reqHash []byte) bool { + if oe == nil { + return false + } + oe.mtx.Lock() defer oe.mtx.Unlock() if rand.Intn(100) > 80 || !bytes.Equal(oe.request.Hash, reqHash) { diff --git a/server/util.go b/server/util.go index 40842f4b2c36..9c2fe9e9859b 100644 --- a/server/util.go +++ b/server/util.go @@ -516,6 +516,7 @@ func DefaultBaseappOptions(appOpts types.AppOptions) []func(*baseapp.BaseApp) { baseapp.SetIAVLDisableFastNode(cast.ToBool(appOpts.Get(FlagDisableIAVLFastNode))), defaultMempool, baseapp.SetChainID(chainID), + baseapp.SetOptimisticExecution(), } } From c798e174c918635429ac74e33dcaded1bb44ce6a Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Wed, 2 Aug 2023 16:07:26 +0200 Subject: [PATCH 18/29] progress --- Makefile | 2 +- baseapp/abci.go | 8 ++--- baseapp/oe/optimistic_execution.go | 49 +++++++++++++++++++++++++++--- baseapp/options.go | 4 +-- 4 files changed, 51 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index a809b03d7ba5..bd18ff583402 100644 --- a/Makefile +++ b/Makefile @@ -472,7 +472,7 @@ localnet-stop: # localnet-start will run a 4-node testnet locally. The nodes are # based off the docker images in: ./contrib/images/simd-env -localnet-start: localnet-stop localnet-build-nodes +localnet-start: localnet-stop localnet-build-env localnet-build-nodes # localnet-debug will run a 4-node testnet locally in debug mode # you can read more about the debug mode here: ./contrib/images/simd-dlv/README.md diff --git a/baseapp/abci.go b/baseapp/abci.go index aac2d3d1346d..2eeb52e48dd3 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -734,7 +734,7 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci // First check for an abort signal after beginBlock, as it's the first place // we spend any significant amount of time. - if app.optimisticExec.ShouldAbort() { + if app.optimisticExec.Running() && app.optimisticExec.ShouldAbort() { return nil, nil } @@ -765,7 +765,7 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci } // check after every tx if we should abort - if app.optimisticExec.ShouldAbort() { + if app.optimisticExec.Running() && app.optimisticExec.ShouldAbort() { return nil, nil } @@ -782,7 +782,7 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci } // check after endBlock if we should abort, to avoid propagating the result - if app.optimisticExec.ShouldAbort() { + if app.optimisticExec.Running() && app.optimisticExec.ShouldAbort() { return nil, nil } @@ -808,7 +808,7 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci // extensions into the proposal, which should not themselves be executed in cases // where they adhere to the sdk.Tx interface. func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { - if app.optimisticExec.Enabled() { + if app.optimisticExec.Initialized() { // check if the hash we got is the same as the one we are executing aborted := app.optimisticExec.AbortIfNeeded(req.Hash) // Wait for the OE to finish, regardless of whether it was aborted or not diff --git a/baseapp/oe/optimistic_execution.go b/baseapp/oe/optimistic_execution.go index 5502de1e5c28..170d9d496cba 100644 --- a/baseapp/oe/optimistic_execution.go +++ b/baseapp/oe/optimistic_execution.go @@ -16,6 +16,7 @@ type OptimisticExecution struct { stopCh chan struct{} shouldAbort bool running bool + initialized bool // we could use generics here in the future to allow other types of req/resp fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) @@ -24,10 +25,23 @@ type OptimisticExecution struct { err error executionTime time.Duration logger log.Logger + + // debugging options + abortRate int // number from 0 to 100 } -func NewOptimisticExecution(logger log.Logger, fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error)) *OptimisticExecution { - return &OptimisticExecution{logger: logger, fn: fn} +func NewOptimisticExecution(logger log.Logger, fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error), opts ...func(*OptimisticExecution)) *OptimisticExecution { + oe := &OptimisticExecution{logger: logger, fn: fn} + for _, opt := range opts { + opt(oe) + } + return oe +} + +func WithAbortRate(rate int) func(*OptimisticExecution) { + return func(oe *OptimisticExecution) { + oe.abortRate = rate + } } // Reset resets the OE context. Must be called whenever we want to invalidate @@ -42,12 +56,25 @@ func (oe *OptimisticExecution) Reset() { oe.executionTime = 0 oe.shouldAbort = false oe.running = false + oe.initialized = false } func (oe *OptimisticExecution) Enabled() bool { return oe != nil } +// Initialized returns true if the OE was initialized, meaning that it contains +// a request and it was run or it is running. +func (oe *OptimisticExecution) Initialized() bool { + if oe == nil { + return false + } + oe.mtx.RLock() + defer oe.mtx.RUnlock() + + return oe.initialized +} + // Execute initializes the OE and starts it in a goroutine. func (oe *OptimisticExecution) Execute( req *abci.RequestProcessProposal, @@ -70,11 +97,13 @@ func (oe *OptimisticExecution) Execute( oe.logger.Debug("OE started") start := time.Now() oe.running = true + oe.initialized = true + go func() { resp, err := oe.fn(oe.request) oe.mtx.Lock() oe.executionTime = time.Since(start) - oe.logger.Debug("OE finished", "duration", oe.executionTime) + oe.logger.Debug("OE finished", "duration", oe.executionTime.String()) oe.response, oe.err = resp, err oe.running = false close(oe.stopCh) @@ -91,10 +120,20 @@ func (oe *OptimisticExecution) AbortIfNeeded(reqHash []byte) bool { oe.mtx.Lock() defer oe.mtx.Unlock() - if rand.Intn(100) > 80 || !bytes.Equal(oe.request.Hash, reqHash) { - oe.logger.Debug("OE aborted") + + if !bytes.Equal(oe.request.Hash, reqHash) { + oe.logger.Debug("OE aborted due to hash mismatch", "oe_hash", oe.request.Hash, "req_hash", reqHash) oe.shouldAbort = true } + + // test abort rate + if oe.abortRate > 0 && !oe.shouldAbort { + oe.shouldAbort = rand.Intn(100) < oe.abortRate + if oe.shouldAbort { + oe.logger.Debug("OE aborted due to test abort rate") + } + } + return oe.shouldAbort } diff --git a/baseapp/options.go b/baseapp/options.go index 91bd397125e7..3aa68e024b2c 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -96,9 +96,9 @@ func SetChainID(chainID string) func(*BaseApp) { } // SetOptimisticExecution enables optimistic execution. -func SetOptimisticExecution() func(*BaseApp) { +func SetOptimisticExecution(opts ...func(*oe.OptimisticExecution)) func(*BaseApp) { return func(app *BaseApp) { - app.optimisticExec = oe.NewOptimisticExecution(app.logger, app.internalFinalizeBlock) + app.optimisticExec = oe.NewOptimisticExecution(app.logger, app.internalFinalizeBlock, opts...) } } From 74147f1fef4e6765c425ed6b8612f393abaf6c56 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Thu, 17 Aug 2023 12:45:07 +0200 Subject: [PATCH 19/29] added mutext to mempools --- types/mempool/priority_nonce.go | 12 +++++++++++- types/mempool/sender_nonce.go | 12 +++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/types/mempool/priority_nonce.go b/types/mempool/priority_nonce.go index 401526b7b254..1c080b3a58f5 100644 --- a/types/mempool/priority_nonce.go +++ b/types/mempool/priority_nonce.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "math" + "sync" "github.com/huandu/skiplist" @@ -49,6 +50,7 @@ type ( // priority to other sender txs and must be partially ordered by both sender-nonce // and priority. PriorityNonceMempool[C comparable] struct { + mtx sync.Mutex priorityIndex *skiplist.SkipList priorityCounts map[C]int senderIndices map[string]*skiplist.SkipList @@ -194,7 +196,9 @@ func (mp *PriorityNonceMempool[C]) NextSenderTx(sender string) sdk.Tx { // Inserting a duplicate tx with a different priority overwrites the existing tx, // changing the total order of the mempool. func (mp *PriorityNonceMempool[C]) Insert(ctx context.Context, tx sdk.Tx) error { - if mp.cfg.MaxTx > 0 && mp.CountTx() >= mp.cfg.MaxTx { + mp.mtx.Lock() + defer mp.mtx.Unlock() + if mp.cfg.MaxTx > 0 && mp.priorityIndex.Len() >= mp.cfg.MaxTx { return ErrMempoolTxMaxCapacity } else if mp.cfg.MaxTx < 0 { return nil @@ -341,6 +345,8 @@ func (i *PriorityNonceIterator[C]) Tx() sdk.Tx { // NOTE: It is not safe to use this iterator while removing transactions from // the underlying mempool. func (mp *PriorityNonceMempool[C]) Select(_ context.Context, _ [][]byte) Iterator { + mp.mtx.Lock() + defer mp.mtx.Unlock() if mp.priorityIndex.Len() == 0 { return nil } @@ -409,12 +415,16 @@ func senderWeight[C comparable](txPriority TxPriority[C], senderCursor *skiplist // CountTx returns the number of transactions in the mempool. func (mp *PriorityNonceMempool[C]) CountTx() int { + mp.mtx.Lock() + defer mp.mtx.Unlock() return mp.priorityIndex.Len() } // Remove removes a transaction from the mempool in O(log n) time, returning an // error if unsuccessful. func (mp *PriorityNonceMempool[C]) Remove(tx sdk.Tx) error { + mp.mtx.Lock() + defer mp.mtx.Unlock() sigs, err := tx.(signing.SigVerifiableTx).GetSignaturesV2() if err != nil { return err diff --git a/types/mempool/sender_nonce.go b/types/mempool/sender_nonce.go index c013072dfecd..7645bda33993 100644 --- a/types/mempool/sender_nonce.go +++ b/types/mempool/sender_nonce.go @@ -6,6 +6,7 @@ import ( "encoding/binary" "fmt" "math/rand" // #nosec // math/rand is used for random selection and seeded from crypto/rand + "sync" "github.com/huandu/skiplist" @@ -31,6 +32,7 @@ var DefaultMaxTx = 0 // Note that PrepareProposal could choose to stop iteration before reaching the // end if maxBytes is reached. type SenderNonceMempool struct { + mtx sync.Mutex senders map[string]*skiplist.SkipList rnd *rand.Rand maxTx int @@ -116,7 +118,9 @@ func (snm *SenderNonceMempool) NextSenderTx(sender string) sdk.Tx { // Insert adds a tx to the mempool. It returns an error if the tx does not have // at least one signer. Note, priority is ignored. func (snm *SenderNonceMempool) Insert(_ context.Context, tx sdk.Tx) error { - if snm.maxTx > 0 && snm.CountTx() >= snm.maxTx { + snm.mtx.Lock() + defer snm.mtx.Unlock() + if snm.maxTx > 0 && len(snm.existingTx) >= snm.maxTx { return ErrMempoolTxMaxCapacity } if snm.maxTx < 0 { @@ -155,6 +159,8 @@ func (snm *SenderNonceMempool) Insert(_ context.Context, tx sdk.Tx) error { // NOTE: It is not safe to use this iterator while removing transactions from // the underlying mempool. func (snm *SenderNonceMempool) Select(_ context.Context, _ [][]byte) Iterator { + snm.mtx.Lock() + defer snm.mtx.Unlock() var senders []string senderCursors := make(map[string]*skiplist.Element) @@ -184,12 +190,16 @@ func (snm *SenderNonceMempool) Select(_ context.Context, _ [][]byte) Iterator { // CountTx returns the total count of txs in the mempool. func (snm *SenderNonceMempool) CountTx() int { + snm.mtx.Lock() + defer snm.mtx.Unlock() return len(snm.existingTx) } // Remove removes a tx from the mempool. It returns an error if the tx does not // have at least one signer or the tx was not found in the pool. func (snm *SenderNonceMempool) Remove(tx sdk.Tx) error { + snm.mtx.Lock() + defer snm.mtx.Unlock() sigs, err := tx.(signing.SigVerifiableTx).GetSignaturesV2() if err != nil { return err From 0d45c3c5f13242a24806b743efc27c1c1cdeeb92 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Sun, 27 Aug 2023 13:34:22 +0200 Subject: [PATCH 20/29] add test and do some refactor --- Makefile | 2 +- baseapp/abci.go | 43 +++++++---- baseapp/abci_test.go | 42 ++++++++++ baseapp/oe/optimistic_execution.go | 118 ++++++++++++----------------- contrib/images/simd-env/Dockerfile | 7 +- server/util.go | 1 - simapp/app.go | 7 +- simapp/app_v2.go | 2 +- 8 files changed, 126 insertions(+), 96 deletions(-) diff --git a/Makefile b/Makefile index fc5f821e9523..27977bda1275 100644 --- a/Makefile +++ b/Makefile @@ -472,7 +472,7 @@ localnet-stop: # localnet-start will run a 4-node testnet locally. The nodes are # based off the docker images in: ./contrib/images/simd-env -localnet-start: localnet-stop localnet-build-env localnet-build-nodes +localnet-start: localnet-stop localnet-build-nodes # localnet-debug will run a 4-node testnet locally in debug mode # you can read more about the debug mode here: ./contrib/images/simd-dlv/README.md diff --git a/baseapp/abci.go b/baseapp/abci.go index 8c1d564b3d6a..4c0203b94e2d 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -495,11 +495,7 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc // after state changes during InitChain. if req.Height > app.initialHeight { // abort any running OE - if app.optimisticExec.Running() { - app.optimisticExec.Abort() - _, _ = app.optimisticExec.WaitResult() // ignore the result - } - + app.optimisticExec.Abort() app.setState(execModeFinalize, header) } @@ -540,8 +536,12 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil } - // Only execute optimistic execution if OE is enabled and the block height is greater than the initial height. - // During the first block we'll be carrying state from InitChain, so it would be impossible for us to easily revert. + // Only execute optimistic execution if OE is enabled and the block height + // is greater than the initial height. During the first block we'll be carrying + // state from InitChain, so it would be impossible for us to easily revert. + // After the first block has been processed, the next blocks will get executed + // optimistically, so that when the ABCI client calls `FinalizeBlock` the app + // can have a response ready. if app.optimisticExec.Enabled() && req.Height > app.initialHeight { app.optimisticExec.Execute(req) } @@ -659,7 +659,7 @@ func (app *BaseApp) VerifyVoteExtension(req *abci.RequestVerifyVoteExtension) (r return resp, err } -func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { +func (app *BaseApp) internalFinalizeBlock(ctx context.Context, req *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { var events []abci.Event if err := app.checkHalt(req.Height, req.Time); err != nil { @@ -736,8 +736,11 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci // First check for an abort signal after beginBlock, as it's the first place // we spend any significant amount of time. - if app.optimisticExec.Running() && app.optimisticExec.ShouldAbort() { - return nil, nil + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + // continue } events = append(events, beginBlock.Events...) @@ -767,8 +770,11 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci } // check after every tx if we should abort - if app.optimisticExec.Running() && app.optimisticExec.ShouldAbort() { - return nil, nil + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + // continue } txResults = append(txResults, response) @@ -784,8 +790,11 @@ func (app *BaseApp) internalFinalizeBlock(req *abci.RequestFinalizeBlock) (*abci } // check after endBlock if we should abort, to avoid propagating the result - if app.optimisticExec.Running() && app.optimisticExec.ShouldAbort() { - return nil, nil + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + // continue } events = append(events, endBlock.Events...) @@ -818,7 +827,9 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons // only return if we are not aborting if !aborted { - res.AppHash = app.workingHash() + if res != nil { + res.AppHash = app.workingHash() + } return res, err } @@ -828,7 +839,7 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons } // if no OE is running, just run the block (this is either a block replay or a OE that got aborted) - res, err := app.internalFinalizeBlock(req) + res, err := app.internalFinalizeBlock(context.Background(), req) if res != nil { res.AppHash = app.workingHash() } diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index e24af9706503..e1b039342724 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -2197,3 +2197,45 @@ func TestBaseApp_VoteExtensions(t *testing.T) { committedAvgPrice := suite.baseApp.NewContext(true).KVStore(capKey1).Get([]byte("avgPrice")) require.Equal(t, avgPrice, committedAvgPrice) } + +func TestOptimisticExecution(t *testing.T) { + suite := NewBaseAppSuite(t, baseapp.SetOptimisticExecution()) + + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ + ConsensusParams: &cmtproto.ConsensusParams{}, + }) + require.NoError(t, err) + + // run 50 blocks + for i := 0; i < 50; i++ { + tx := newTxCounter(t, suite.txConfig, 0, 1) + txBytes, err := suite.txConfig.TxEncoder()(tx) + require.NoError(t, err) + + reqProcProp := abci.RequestProcessProposal{ + Txs: [][]byte{txBytes}, + Height: suite.baseApp.LastBlockHeight() + 1, + Hash: []byte("some-hash" + strconv.FormatInt(suite.baseApp.LastBlockHeight()+1, 10)), + } + + respProcProp, err := suite.baseApp.ProcessProposal(&reqProcProp) + require.Equal(t, abci.ResponseProcessProposal_ACCEPT, respProcProp.Status) + require.NoError(t, err) + + reqFinalizeBlock := abci.RequestFinalizeBlock{ + Height: reqProcProp.Height, + Txs: reqProcProp.Txs, + Hash: reqProcProp.Hash, + } + + respFinalizeBlock, err := suite.baseApp.FinalizeBlock(&reqFinalizeBlock) + require.NoError(t, err) + require.Len(t, respFinalizeBlock.TxResults, 1) + fmt.Println(reqFinalizeBlock.Height) + + _, err = suite.baseApp.Commit() + require.NoError(t, err) + } + + require.Equal(t, int64(50), suite.baseApp.LastBlockHeight()) +} diff --git a/baseapp/oe/optimistic_execution.go b/baseapp/oe/optimistic_execution.go index 170d9d496cba..d70da2df643e 100644 --- a/baseapp/oe/optimistic_execution.go +++ b/baseapp/oe/optimistic_execution.go @@ -2,6 +2,8 @@ package oe import ( "bytes" + "context" + "encoding/hex" "math/rand" "sync" "time" @@ -11,27 +13,30 @@ import ( "cosmossdk.io/log" ) +// FinalizeBlockFunc is the function that is called by the OE to finalize the +// block. It is the same as the one in the ABCI app. +type FinalizeBlockFunc func(context.Context, *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) + +// OptimisticExecution is a struct that contains the OE context. It is used to +// run the FinalizeBlock function in a goroutine, and to abort it if needed. type OptimisticExecution struct { - mtx sync.RWMutex + finalizeBlockFunc FinalizeBlockFunc // ABCI FinalizeBlock function with a context + logger log.Logger + + mtx sync.Mutex stopCh chan struct{} - shouldAbort bool - running bool - initialized bool - - // we could use generics here in the future to allow other types of req/resp - fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) - request *abci.RequestFinalizeBlock - response *abci.ResponseFinalizeBlock - err error - executionTime time.Duration - logger log.Logger - - // debugging options - abortRate int // number from 0 to 100 + request *abci.RequestFinalizeBlock + response *abci.ResponseFinalizeBlock + err error + cancelFunc func() // cancel function for the context + initialized bool // A boolean value indicating whether the struct has been initialized + + // debugging/testing options + abortRate int // number from 0 to 100 that determines the percentage of OE that should be aborted } -func NewOptimisticExecution(logger log.Logger, fn func(*abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error), opts ...func(*OptimisticExecution)) *OptimisticExecution { - oe := &OptimisticExecution{logger: logger, fn: fn} +func NewOptimisticExecution(logger log.Logger, fn FinalizeBlockFunc, opts ...func(*OptimisticExecution)) *OptimisticExecution { + oe := &OptimisticExecution{logger: logger, finalizeBlockFunc: fn} for _, opt := range opts { opt(oe) } @@ -45,17 +50,13 @@ func WithAbortRate(rate int) func(*OptimisticExecution) { } // Reset resets the OE context. Must be called whenever we want to invalidate -// the current OE. For example when on FinalizeBlock we want to process the -// block async, we run Reset() to make sure ShouldAbort() returns always false. +// the current OE. func (oe *OptimisticExecution) Reset() { oe.mtx.Lock() defer oe.mtx.Unlock() oe.request = nil oe.response = nil oe.err = nil - oe.executionTime = 0 - oe.shouldAbort = false - oe.running = false oe.initialized = false } @@ -69,16 +70,14 @@ func (oe *OptimisticExecution) Initialized() bool { if oe == nil { return false } - oe.mtx.RLock() - defer oe.mtx.RUnlock() + oe.mtx.Lock() + defer oe.mtx.Unlock() return oe.initialized } // Execute initializes the OE and starts it in a goroutine. -func (oe *OptimisticExecution) Execute( - req *abci.RequestProcessProposal, -) { +func (oe *OptimisticExecution) Execute(req *abci.RequestProcessProposal) { oe.mtx.Lock() defer oe.mtx.Unlock() @@ -94,18 +93,18 @@ func (oe *OptimisticExecution) Execute( ProposerAddress: req.ProposerAddress, } - oe.logger.Debug("OE started") - start := time.Now() - oe.running = true + oe.logger.Debug("OE started", "height", req.Height, "hash", hex.EncodeToString(req.Hash), "time", req.Time.String()) + ctx, cancel := context.WithCancel(context.Background()) + oe.cancelFunc = cancel oe.initialized = true go func() { - resp, err := oe.fn(oe.request) + start := time.Now() + resp, err := oe.finalizeBlockFunc(ctx, oe.request) oe.mtx.Lock() - oe.executionTime = time.Since(start) - oe.logger.Debug("OE finished", "duration", oe.executionTime.String()) + executionTime := time.Since(start) + oe.logger.Debug("OE finished", "duration", executionTime.String(), "height", req.Height, "hash", hex.EncodeToString(req.Hash)) oe.response, oe.err = resp, err - oe.running = false close(oe.stopCh) oe.mtx.Unlock() }() @@ -122,49 +121,28 @@ func (oe *OptimisticExecution) AbortIfNeeded(reqHash []byte) bool { defer oe.mtx.Unlock() if !bytes.Equal(oe.request.Hash, reqHash) { - oe.logger.Debug("OE aborted due to hash mismatch", "oe_hash", oe.request.Hash, "req_hash", reqHash) - oe.shouldAbort = true - } - - // test abort rate - if oe.abortRate > 0 && !oe.shouldAbort { - oe.shouldAbort = rand.Intn(100) < oe.abortRate - if oe.shouldAbort { - oe.logger.Debug("OE aborted due to test abort rate") - } + oe.logger.Error("OE aborted due to hash mismatch", "oe_hash", hex.EncodeToString(oe.request.Hash), "req_hash", hex.EncodeToString(reqHash), "oe_height", oe.request.Height, "req_height", oe.request.Height) + oe.cancelFunc() + return true + } else if oe.abortRate > 0 && rand.Intn(100) < oe.abortRate { + // this is for test purposes only, we can emulate a certain percentage of + // OE needed to be aborted. + oe.cancelFunc() + oe.logger.Error("OE aborted due to test abort rate") + return true } - return oe.shouldAbort + return false } -// Abort aborts the OE unconditionally. +// Abort aborts the OE unconditionally and waits for it to finish. func (oe *OptimisticExecution) Abort() { - oe.mtx.Lock() - defer oe.mtx.Unlock() - oe.shouldAbort = true -} - -// ShouldAbort must only be used in the fn passed to SetupOptimisticExecution to -// check if the OE was aborted and return as soon as possible. -func (oe *OptimisticExecution) ShouldAbort() bool { - if oe == nil { - return false + if oe == nil || oe.cancelFunc == nil { + return } - oe.mtx.RLock() - defer oe.mtx.RUnlock() - return oe.shouldAbort -} - -// Running returns true if the OE is still running. -func (oe *OptimisticExecution) Running() bool { - if oe == nil { - return false - } - - oe.mtx.RLock() - defer oe.mtx.RUnlock() - return oe.running + oe.cancelFunc() + <-oe.stopCh } // WaitResult waits for the OE to finish and returns the result. diff --git a/contrib/images/simd-env/Dockerfile b/contrib/images/simd-env/Dockerfile index 7fee9105230a..7672e3c22f11 100644 --- a/contrib/images/simd-env/Dockerfile +++ b/contrib/images/simd-env/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.20-alpine AS build +FROM arm64v8/golang:1.21-alpine AS build RUN apk add build-base git linux-headers @@ -18,9 +18,8 @@ RUN go mod download COPY ./ /work RUN LEDGER_ENABLED=false make clean build - -FROM alpine AS run -RUN apk add bash curl jq +FROM arm64v8/alpine AS run +RUN apk add bash curl jq iproute2 EXPOSE 26656 26657 ENTRYPOINT ["/usr/bin/wrapper.sh"] CMD ["start", "--log_format", "plain"] diff --git a/server/util.go b/server/util.go index e0922aa120a7..521a61d41090 100644 --- a/server/util.go +++ b/server/util.go @@ -517,7 +517,6 @@ func DefaultBaseappOptions(appOpts types.AppOptions) []func(*baseapp.BaseApp) { defaultMempool, baseapp.SetChainID(chainID), baseapp.SetQueryGasLimit(cast.ToUint64(appOpts.Get(FlagQueryGasLimit))), - baseapp.SetOptimisticExecution(), } } diff --git a/simapp/app.go b/simapp/app.go index 20ff245460c1..6ccf92eb27a9 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -33,6 +33,9 @@ import ( upgradetypes "cosmossdk.io/x/upgrade/types" abci "github.com/cometbft/cometbft/abci/types" dbm "github.com/cosmos/cosmos-db" + "github.com/cosmos/gogoproto/proto" + "github.com/spf13/cast" + "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" @@ -103,8 +106,6 @@ import ( "github.com/cosmos/cosmos-sdk/x/staking" stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - "github.com/cosmos/gogoproto/proto" - "github.com/spf13/cast" ) const appName = "SimApp" @@ -241,7 +242,7 @@ func NewSimApp( voteExtHandler := NewVoteExtensionHandler() voteExtHandler.SetHandlers(bApp) } - baseAppOptions = append(baseAppOptions, voteExtOp) + baseAppOptions = append(baseAppOptions, voteExtOp, baseapp.SetOptimisticExecution()) bApp := baseapp.NewBaseApp(appName, logger, db, txConfig.TxDecoder(), baseAppOptions...) bApp.SetCommitMultiStoreTracer(traceStore) diff --git a/simapp/app_v2.go b/simapp/app_v2.go index 50bb5cb3687d..ac916bc03788 100644 --- a/simapp/app_v2.go +++ b/simapp/app_v2.go @@ -220,7 +220,7 @@ func NewSimApp( voteExtHandler := NewVoteExtensionHandler() voteExtHandler.SetHandlers(bApp) } - baseAppOptions = append(baseAppOptions, voteExtOp) + baseAppOptions = append(baseAppOptions, voteExtOp, baseapp.SetOptimisticExecution()) app.App = appBuilder.Build(db, traceStore, baseAppOptions...) From b008a8adf23e21043d8f30601f9ecf9d43d2b62b Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Sun, 27 Aug 2023 13:37:01 +0200 Subject: [PATCH 21/29] undo test changes --- Makefile | 2 +- baseapp/abci_test.go | 1 - contrib/images/simd-env/Dockerfile | 6 +++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 27977bda1275..fc5f821e9523 100644 --- a/Makefile +++ b/Makefile @@ -472,7 +472,7 @@ localnet-stop: # localnet-start will run a 4-node testnet locally. The nodes are # based off the docker images in: ./contrib/images/simd-env -localnet-start: localnet-stop localnet-build-nodes +localnet-start: localnet-stop localnet-build-env localnet-build-nodes # localnet-debug will run a 4-node testnet locally in debug mode # you can read more about the debug mode here: ./contrib/images/simd-dlv/README.md diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index e1b039342724..82f255ceaceb 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -2231,7 +2231,6 @@ func TestOptimisticExecution(t *testing.T) { respFinalizeBlock, err := suite.baseApp.FinalizeBlock(&reqFinalizeBlock) require.NoError(t, err) require.Len(t, respFinalizeBlock.TxResults, 1) - fmt.Println(reqFinalizeBlock.Height) _, err = suite.baseApp.Commit() require.NoError(t, err) diff --git a/contrib/images/simd-env/Dockerfile b/contrib/images/simd-env/Dockerfile index 7672e3c22f11..8757d34b5628 100644 --- a/contrib/images/simd-env/Dockerfile +++ b/contrib/images/simd-env/Dockerfile @@ -1,4 +1,4 @@ -FROM arm64v8/golang:1.21-alpine AS build +FROM golang:1.21-alpine AS build RUN apk add build-base git linux-headers @@ -18,8 +18,8 @@ RUN go mod download COPY ./ /work RUN LEDGER_ENABLED=false make clean build -FROM arm64v8/alpine AS run -RUN apk add bash curl jq iproute2 +FROM alpine AS run +RUN apk add bash curl jq EXPOSE 26656 26657 ENTRYPOINT ["/usr/bin/wrapper.sh"] CMD ["start", "--log_format", "plain"] From 78b233d35c4bd15dd9e6e7db7db5c8741cf47564 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Sun, 27 Aug 2023 13:39:34 +0200 Subject: [PATCH 22/29] fix --- baseapp/abci.go | 4 ++++ contrib/images/simd-env/Dockerfile | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 4c0203b94e2d..3ae0893a1dac 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -659,6 +659,10 @@ func (app *BaseApp) VerifyVoteExtension(req *abci.RequestVerifyVoteExtension) (r return resp, err } +// internalFinalizeBlock it's what actually executes the block, called by the Optimistic +// Execution flow or by the FinalizeBlock ABCI method. The context received is +// only used to handle early cancellation, for anything related to state app.finalizeBlockState.ctx +// must be used. func (app *BaseApp) internalFinalizeBlock(ctx context.Context, req *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { var events []abci.Event diff --git a/contrib/images/simd-env/Dockerfile b/contrib/images/simd-env/Dockerfile index 8757d34b5628..7fee9105230a 100644 --- a/contrib/images/simd-env/Dockerfile +++ b/contrib/images/simd-env/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.21-alpine AS build +FROM golang:1.20-alpine AS build RUN apk add build-base git linux-headers @@ -18,6 +18,7 @@ RUN go mod download COPY ./ /work RUN LEDGER_ENABLED=false make clean build + FROM alpine AS run RUN apk add bash curl jq EXPOSE 26656 26657 From 4f90f04409956652c82a7d278d468868ace499ef Mon Sep 17 00:00:00 2001 From: Facundo Medica <14063057+facundomedica@users.noreply.github.com> Date: Tue, 29 Aug 2023 12:18:32 -0300 Subject: [PATCH 23/29] Update baseapp/abci.go Co-authored-by: Aleksandr Bezobchuk --- baseapp/abci.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 3ae0893a1dac..2752df381662 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -659,7 +659,7 @@ func (app *BaseApp) VerifyVoteExtension(req *abci.RequestVerifyVoteExtension) (r return resp, err } -// internalFinalizeBlock it's what actually executes the block, called by the Optimistic +// internalFinalizeBlock executes the block, called by the Optimistic // Execution flow or by the FinalizeBlock ABCI method. The context received is // only used to handle early cancellation, for anything related to state app.finalizeBlockState.ctx // must be used. From 2b574d50d87258dbec15378bf038d1fe71fe51aa Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Sat, 9 Sep 2023 06:16:48 +0200 Subject: [PATCH 24/29] only start optimistic execution if processProposal resp is accepted --- baseapp/abci.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 2752df381662..140284ce9ada 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -536,13 +536,16 @@ func (app *BaseApp) ProcessProposal(req *abci.RequestProcessProposal) (resp *abc return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil } - // Only execute optimistic execution if OE is enabled and the block height - // is greater than the initial height. During the first block we'll be carrying - // state from InitChain, so it would be impossible for us to easily revert. + // Only execute optimistic execution if the proposal is accepted, OE is + // enabled and the block height is greater than the initial height. During + // the first block we'll be carrying state from InitChain, so it would be + // impossible for us to easily revert. // After the first block has been processed, the next blocks will get executed // optimistically, so that when the ABCI client calls `FinalizeBlock` the app // can have a response ready. - if app.optimisticExec.Enabled() && req.Height > app.initialHeight { + if resp.Status == abci.ResponseProcessProposal_ACCEPT && + app.optimisticExec.Enabled() && + req.Height > app.initialHeight { app.optimisticExec.Execute(req) } From 1c4743ad2edbd140ed9eae4e71c862b77ec84f42 Mon Sep 17 00:00:00 2001 From: Facundo Date: Mon, 18 Sep 2023 09:58:35 +0200 Subject: [PATCH 25/29] godoc + tests --- baseapp/baseapp.go | 3 +++ baseapp/oe/optimistic_execution.go | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 24a4afb273e9..cba9d45afcb8 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -177,6 +177,9 @@ type BaseApp struct { cdc codec.Codec + // optimisticExec contains the context required for Optimistic Execution, + // including the goroutine handling.This is experimental and must be enabled + // by developers. optimisticExec *oe.OptimisticExecution } diff --git a/baseapp/oe/optimistic_execution.go b/baseapp/oe/optimistic_execution.go index d70da2df643e..a42a3670a70d 100644 --- a/baseapp/oe/optimistic_execution.go +++ b/baseapp/oe/optimistic_execution.go @@ -35,7 +35,9 @@ type OptimisticExecution struct { abortRate int // number from 0 to 100 that determines the percentage of OE that should be aborted } +// NewOptimisticExecution initializes the Optimistic Execution context but does not start it. func NewOptimisticExecution(logger log.Logger, fn FinalizeBlockFunc, opts ...func(*OptimisticExecution)) *OptimisticExecution { + logger = logger.With("module", "oe") oe := &OptimisticExecution{logger: logger, finalizeBlockFunc: fn} for _, opt := range opts { opt(oe) @@ -43,6 +45,9 @@ func NewOptimisticExecution(logger log.Logger, fn FinalizeBlockFunc, opts ...fun return oe } +// WithAbortRate sets the abort rate for the OE. The abort rate is a number from +// 0 to 100 that determines the percentage of OE that should be aborted. +// This is for testing purposes only and must not be used in production. func WithAbortRate(rate int) func(*OptimisticExecution) { return func(oe *OptimisticExecution) { oe.abortRate = rate From 8cda1f1efd1cfe7738a0f5c5f4c9a6037cc489db Mon Sep 17 00:00:00 2001 From: Facundo Date: Mon, 18 Sep 2023 12:50:33 +0200 Subject: [PATCH 26/29] add file --- baseapp/oe/optimistic_execution.go | 2 +- baseapp/oe/optimistic_execution_test.go | 33 +++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 baseapp/oe/optimistic_execution_test.go diff --git a/baseapp/oe/optimistic_execution.go b/baseapp/oe/optimistic_execution.go index a42a3670a70d..2a6d34770955 100644 --- a/baseapp/oe/optimistic_execution.go +++ b/baseapp/oe/optimistic_execution.go @@ -37,7 +37,7 @@ type OptimisticExecution struct { // NewOptimisticExecution initializes the Optimistic Execution context but does not start it. func NewOptimisticExecution(logger log.Logger, fn FinalizeBlockFunc, opts ...func(*OptimisticExecution)) *OptimisticExecution { - logger = logger.With("module", "oe") + logger = logger.With(log.ModuleKey, "oe") oe := &OptimisticExecution{logger: logger, finalizeBlockFunc: fn} for _, opt := range opts { opt(oe) diff --git a/baseapp/oe/optimistic_execution_test.go b/baseapp/oe/optimistic_execution_test.go new file mode 100644 index 000000000000..b1fe622097ce --- /dev/null +++ b/baseapp/oe/optimistic_execution_test.go @@ -0,0 +1,33 @@ +package oe + +import ( + "context" + "errors" + "testing" + + "cosmossdk.io/log" + abci "github.com/cometbft/cometbft/abci/types" + "github.com/stretchr/testify/assert" +) + +func testFinalizeBlock(_ context.Context, _ *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { + return nil, errors.New("test error") +} + +func TestOptimisticExecution(t *testing.T) { + oe := NewOptimisticExecution(log.NewNopLogger(), testFinalizeBlock) + assert.True(t, oe.Enabled()) + oe.Execute(&abci.RequestProcessProposal{ + Hash: []byte("test"), + }) + assert.True(t, oe.Initialized()) + + resp, err := oe.WaitResult() + assert.Nil(t, resp) + assert.EqualError(t, err, "test error") + + assert.False(t, oe.AbortIfNeeded([]byte("test"))) + assert.True(t, oe.AbortIfNeeded([]byte("wrong_hash"))) + + oe.Reset() +} From 0065196bb3d3164c69856e5a8ba7aedbf35066b5 Mon Sep 17 00:00:00 2001 From: Facundo Date: Mon, 18 Sep 2023 12:52:08 +0200 Subject: [PATCH 27/29] cl++ --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba1efbcbed52..d2c62dff3bc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features +* (baseapp) [#16581](https://github.com/cosmos/cosmos-sdk/pull/16581) Implement Optimistic Execution * (client/keys) [#17639](https://github.com/cosmos/cosmos-sdk/pull/17639) Allows using and saving public keys encoded as base64 * (client) [#17513](https://github.com/cosmos/cosmos-sdk/pull/17513) Allow overwritting `client.toml`. Use `client.CreateClientConfig` in place of `client.ReadFromClientConfig` and provide a custom template and a custom config. * (x/bank) [#14224](https://github.com/cosmos/cosmos-sdk/pull/14224) Allow injection of restrictions on transfers using `AppendSendRestriction` or `PrependSendRestriction`. From 9d6c8b1a072bb339dbf541104c1f929c9e3123b0 Mon Sep 17 00:00:00 2001 From: Facundo Date: Mon, 18 Sep 2023 12:53:00 +0200 Subject: [PATCH 28/29] cl++ --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2c62dff3bc2..4a5ec9d8e987 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,7 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features -* (baseapp) [#16581](https://github.com/cosmos/cosmos-sdk/pull/16581) Implement Optimistic Execution +* (baseapp) [#16581](https://github.com/cosmos/cosmos-sdk/pull/16581) Implement Optimistic Execution as an experimental feature (not enabled by default). * (client/keys) [#17639](https://github.com/cosmos/cosmos-sdk/pull/17639) Allows using and saving public keys encoded as base64 * (client) [#17513](https://github.com/cosmos/cosmos-sdk/pull/17513) Allow overwritting `client.toml`. Use `client.CreateClientConfig` in place of `client.ReadFromClientConfig` and provide a custom template and a custom config. * (x/bank) [#14224](https://github.com/cosmos/cosmos-sdk/pull/14224) Allow injection of restrictions on transfers using `AppendSendRestriction` or `PrependSendRestriction`. From 8bdd23dd39d121887aa56ed482a240d1fcb5540b Mon Sep 17 00:00:00 2001 From: Facundo Date: Mon, 18 Sep 2023 13:19:24 +0200 Subject: [PATCH 29/29] lint --- baseapp/oe/optimistic_execution_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/baseapp/oe/optimistic_execution_test.go b/baseapp/oe/optimistic_execution_test.go index b1fe622097ce..0b92244783cd 100644 --- a/baseapp/oe/optimistic_execution_test.go +++ b/baseapp/oe/optimistic_execution_test.go @@ -5,9 +5,10 @@ import ( "errors" "testing" - "cosmossdk.io/log" abci "github.com/cometbft/cometbft/abci/types" "github.com/stretchr/testify/assert" + + "cosmossdk.io/log" ) func testFinalizeBlock(_ context.Context, _ *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) {