diff --git a/baseapp/abci.go b/baseapp/abci.go index c3a9a7b88a6f..f4b30b9c9f87 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -379,6 +379,14 @@ func (app *BaseApp) PrepareProposal(req *abci.RequestPrepareProposal) (resp *abc return nil, errors.New("PrepareProposal handler not set") } + // Abort any running OE so it cannot overlap with `PrepareProposal`. This could happen if optimistic + // `internalFinalizeBlock` from previous round takes a long time, but consensus has moved on to next round. + // Overlap is undesirable, since `internalFinalizeBlock` and `PrepareProoposal` could share access to + // in-memory structs depending on application implementation. + // No-op if OE is not enabled. + // Similar call to Abort() is done in `ProcessProposal`. + app.optimisticExec.Abort() + // Always reset state given that PrepareProposal can timeout and be called // again in a subsequent round. header := cmtproto.Header{ @@ -904,7 +912,7 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (res *abci.Res 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) + aborted := app.optimisticExec.AbortIfNeeded(req) // Wait for the OE to finish, regardless of whether it was aborted or not res, err = app.optimisticExec.WaitResult() diff --git a/baseapp/oe/optimistic_execution.go b/baseapp/oe/optimistic_execution.go index c0c8e2575ac2..63148bbc4a7c 100644 --- a/baseapp/oe/optimistic_execution.go +++ b/baseapp/oe/optimistic_execution.go @@ -120,7 +120,7 @@ func (oe *OptimisticExecution) Execute(req *abci.RequestProcessProposal) { // 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 { +func (oe *OptimisticExecution) AbortIfNeeded(req *abci.RequestFinalizeBlock) bool { if oe == nil { return false } @@ -128,8 +128,8 @@ func (oe *OptimisticExecution) AbortIfNeeded(reqHash []byte) bool { oe.mtx.Lock() defer oe.mtx.Unlock() - if !bytes.Equal(oe.request.Hash, reqHash) { - 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) + if !bytes.Equal(oe.request.Hash, req.Hash) { + oe.logger.Error("OE aborted due to hash mismatch", "oe_hash", hex.EncodeToString(oe.request.Hash), "req_hash", hex.EncodeToString(req.Hash), "oe_height", oe.request.Height, "req_height", req.Height) oe.cancelFunc() return true } else if oe.abortRate > 0 && rand.Intn(100) < oe.abortRate { diff --git a/baseapp/oe/optimistic_execution_test.go b/baseapp/oe/optimistic_execution_test.go index 0b92244783cd..186db798d084 100644 --- a/baseapp/oe/optimistic_execution_test.go +++ b/baseapp/oe/optimistic_execution_test.go @@ -27,8 +27,15 @@ func TestOptimisticExecution(t *testing.T) { 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"))) - + assert.False(t, oe.AbortIfNeeded( + &abci.RequestFinalizeBlock{ + Hash: []byte("test"), + }, + )) + assert.True(t, oe.AbortIfNeeded( + &abci.RequestFinalizeBlock{ + Hash: []byte("wrong_hash"), + }, + )) oe.Reset() }