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.
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.