node icon indicating copy to clipboard operation
node copied to clipboard

Debug trace tx changes

Open skosito opened this issue 1 year ago • 3 comments

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 MsgEthereumTx is found in sdk.Msg of ethermint tx and passed to grpc query to trace
  • also, all predecessors of this msg both from block and cosmos tx (can have multiple MsgEthereumTx msgs) 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 ParseTxResult method 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 MsgIndex because it doesn't have corresponding sdk.Msg so can not get MsgEthereumTx from sdk.Msg
  • for these tx to get MsgEthereumTx i just created eth LegacyTx to recreate MsgEthereumTx using 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

skosito avatar May 21 '24 17:05 skosito

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

github-actions[bot] avatar May 21 '24 17:05 github-actions[bot]

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

Impacted file tree graph

@@             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:

... and 204 files with indirect coverage changes

codecov[bot] avatar May 21 '24 18:05 codecov[bot]

@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

skosito avatar May 23 '24 18:05 skosito

will reopen PR with more changes, closing this one

skosito avatar May 28 '24 21:05 skosito