Conversation
ac8664f to
a510d81
Compare
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3071 +/- ##
==========================================
- Coverage 58.41% 58.40% -0.01%
==========================================
Files 2081 2081
Lines 171790 171918 +128
==========================================
+ Hits 100352 100410 +58
- Misses 62504 62572 +68
- Partials 8934 8936 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
a510d81 to
d987428
Compare
| if r := recover(); r != nil { | ||
| logger.Error("panic in giga synchronous executor", "panic", r, "stack", string(debug.Stack())) | ||
| result = &abci.ExecTxResult{ | ||
| Code: sdkerrors.ErrPanic.ABCICode(), |
There was a problem hiding this comment.
We need to differentiate between panic types otherwise returning everything as ABCICode would be apphash breaking i think.
We have 2 off the top of my head: scheduler.Abort and sdk.ErrorOutOfGas. We would want to check for OOG error type here jsut to be on the safe side.
| } | ||
| } | ||
| }() | ||
| result, execErr = app.executeEVMTxWithGigaExecutor(ctx, evmMsg, cache) |
There was a problem hiding this comment.
Hmm @arajasek we don't return result or execErr out from this func.
I recommend changing the function signature and use named return types prefixed with _ and use those instead. That is far less error prone against future refactors too.
| Code: code, | ||
| Log: log, | ||
| } | ||
| ms.Write() |
There was a problem hiding this comment.
since it's stateless checks it probably doesn't matter, but in the order flow we would not flush the store if ante returns an error https://github.com/sei-protocol/sei-chain/blob/main/sei-cosmos/baseapp/baseapp.go#L933-L943
Describe your changes and provide context
This PR improves and cleans up the validation performed by the Giga executor before running a tx:
Testing performed to validate your change
Bunch of new AI-generated tests. Will confirm it syncs mainnet shortly.