Always reset OE in FinalizeBlock #57
Conversation
|
@teddyding your pull request is missing a changelog! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
baseapp/abci.go
Outdated
| res.AppHash = app.workingHash() | ||
| } | ||
|
|
||
| app.optimisticExec.Reset() | ||
| return res, err | ||
| } | ||
|
|
There was a problem hiding this comment.
Change potentially affects state.
Call sequence:
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).FinalizeBlock (baseapp/abci.go:894)
|
Maybe it's better to reset right after |
Agree it's better to have one single call to I don't believe this affects performance. It's mostly a semantic fix: it makes sense for the OE state (including |
teddyding
force-pushed
the
td/reset-oe
branch
from
676b1f5 to
e515fe6
Compare
| // Wait for the OE to finish, regardless of whether it was aborted or not | ||
| res, err = app.optimisticExec.WaitResult() | ||
|
|
||
| app.optimisticExec.Reset() | ||
|
|
||
| // only return if we are not aborting | ||
| if !aborted { |
There was a problem hiding this comment.
Change potentially affects state.
Call sequence:
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).FinalizeBlock (baseapp/abci.go:894)
Rationale - got LGTM from Cosmos folks
Dependency workflow failing - won't fix in this PR since change is not related.