Debug trace tx changes
Description
This is more for discussion purposes to get your thoughts about this approach.
Transaction tracing works as following:
- tx info is pulled out from cosmos events and parsed
- if tx is found in indexer, it is pulled from block and
MsgEthereumTxis found insdk.Msgof ethermint tx and passed to grpc query to trace - also, all predecessors of this msg both from block and cosmos tx (can have multiple
MsgEthereumTxmsgs) are included
(NOTE: There is a weird behavior in ethermint in event parsing ParseTxResult method, where it seems is assumed that there won't be any other msgs other than MsgEthereumTx in cosmos tx - i guess this is ok assumption, but technically there could be others? This would impact how MsgIndex is calculated there, but that is another topic.)
For zeta transactions that are applied directly (by calling ApplyMessage directly, tx type 88) there is no cosmos tx, no sdk msg, but events are emitted and it is indexed, so it can be traced, but code for tracing has to be changed:
- for every tx, use existing
ParseTxResultmethod which will parse tx details from cosmos event (i extended event with data and nonce for these type of txs, this data will be useful for adapting other rpc methods as well) - both for predecessors in the block, and predecessors in tx sdk.Msgs - if tx is type 88, it doesn't have
MsgIndexbecause it doesn't have corresponding sdk.Msg so can not getMsgEthereumTxfrom sdk.Msg - for these tx to get
MsgEthereumTxi just created ethLegacyTxto recreateMsgEthereumTxusing existing methods
Closes: <PD-XXXX>
Type of change
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Include instructions and any relevant details so others can reproduce.
- [ ] Tested CCTX in localnet
- [ ] Tested in development environment
- [ ] Go unit tests
- [ ] Go integration tests
- [ ] Tested via GitHub Actions
Checklist:
- [ ] I have added unit tests that prove my fix feature works
!!!WARNING!!!
nosec detected in the following files: rpc/backend/tracing.go
Be very careful about using #nosec in code. It can be a quick way to suppress security warnings and move forward with development, it should be employed with caution. Suppressing warnings with #nosec can hide potentially serious vulnerabilities. Only use #nosec when you're absolutely certain that the security issue is either a false positive or has been mitigated in another way.
Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203
Broad #nosec annotations should be avoided, as they can hide other vulnerabilities. The CI will block you from merging this PR until you remove #nosec annotations that do not target specific rules.
Pay extra attention to the way #nosec is being used in the files listed above.
Codecov Report
Attention: Patch coverage is 0% with 2 lines in your changes are missing coverage. Please review.
Project coverage is 68.00%. Comparing base (
1350d1a) to head (8f611cb). Report is 9 commits behind head on develop.
Additional details and impacted files
@@ Coverage Diff @@
## develop #2230 +/- ##
===========================================
- Coverage 68.20% 68.00% -0.21%
===========================================
Files 259 259
Lines 14960 16032 +1072
===========================================
+ Hits 10203 10902 +699
- Misses 4298 4669 +371
- Partials 459 461 +2
| Files | Coverage Δ | |
|---|---|---|
| x/fungible/keeper/evm.go | 87.81% <0.00%> (+0.56%) |
:arrow_up: |
@lumtis @kingpinXD still in draft, but could you please take a look to see if this approach makes sense? I left description to explain current situation and changes
will reopen PR with more changes, closing this one