WIP: zetaclient evm outbound tx index by nonce to supplement outtx tracker

[brewmaster012]

Description

Currently outbound tracker is susceptible to being filled by invalid hashes and missing entries. Invalid hashes require manual cleanup by group proposal and missing entries require constant monitoring and filling in.

This PoC PR adds simple logic in zetaclient to index outtx from TSS address by nonce, therefore avoid relying solely on outtx tracker for discovery of outbound tx hashes on Ethereum.

Overhead (both memory and computation) added is minimal as it piggy backs on existing block by block scanner in inbound tx processing.

Tested in localnet E2E by disabling outbound tracker in Ethereum client.

How Has This Been Tested?

• [ ] Tested CCTX in localnet
• [ ] Tested in development environment
• [ ] Go unit tests
• [ ] Go integration tests
• [ ] Tested via GitHub Actions

[coderabbitai[bot]]

[!IMPORTANT]

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Walkthrough

The recent changes encompass the addition of a new entry in the changelog for version 19.0.0, detailing a fix for the outbound tracker affecting confirmation and processing on EVM chains. Furthermore, the Observer class in the zetaclient has been enhanced to include a method for handling both TSS gas asset reception and outbound transactions. Corresponding updates have been made to the associated test cases to reflect these modifications.

Changes

Files	Change Summary
changelog.md	Added a new entry for version 19.0.0 detailing a fix for the outbound tracker that impacts confirmation and processing on EVM chains.
zetaclient/chains/evm/observer/inbound.go, zetaclient/chains/evm/observer/inbound_test.go	Renamed the ObserveTSSReceiveInBlock method to ObserveTSSReceiveInBlockAndOutbound, modifying its functionality to include handling of outbound transactions. Updated corresponding test cases to reflect this change.

Sequence Diagram(s)

sequenceDiagram
    participant Observer
    participant TransactionHandler
    participant EVMChain

    Observer->>EVMChain: ObserveTSSReceiveInBlockAndOutbound(blockNumber)
    EVMChain->>TransactionHandler: Check for confirmed transactions
    TransactionHandler-->>EVMChain: Confirmed transactions found
    EVMChain-->>Observer: Send transaction receipt
    Observer->>Observer: Process inbound and outbound transactions

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 97.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 66.93%. Comparing base (cc7b347) to head (cf8f943). Report is 3 commits behind head on develop.

Files with missing lines	Patch %	Lines
zetaclient/chains/evm/observer/inbound.go	0.00%	1 Missing :warning:

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2735      +/-   ##
===========================================
+ Coverage    66.80%   66.93%   +0.13%     
===========================================
  Files          373      373              
  Lines        20993    21028      +35     
===========================================
+ Hits         14025    14076      +51     
+ Misses        6315     6288      -27     
- Partials       653      664      +11     

Files with missing lines	Coverage Δ
zetaclient/chains/evm/observer/outbound.go	55.41% <100.00%> (+8.99%)	:arrow_up:
zetaclient/chains/evm/signer/signer.go	47.48% <100.00%> (+0.33%)	:arrow_up:
zetaclient/testutils/testdata.go	86.95% <100.00%> (+1.12%)	:arrow_up:
zetaclient/chains/evm/observer/inbound.go	37.36% <0.00%> (-0.08%)	:arrow_down:

[ws4charlie]

I see know the logic and idea. It would still be proper to define a new method like ObserveTSSOutbound, and calling this function in ObserveInbound. Ideally we should have this logic in outbound.go as it pertains to outbounds but the current state of the code doesn't allow it. ObserveInbound is fine as we iterate the blocks there.

I think the ObserveTSSReceiveInBlockAndOutbound naming is a bit confusing and the method is still called in ObserverTSSReceive which makes the control flow a bit more implicit.

Also ObserverTSSReceive will be removed in the future once we fully migrate to V2 contract and wind down v1 compatibility

Tested in localnet E2E by disabling outbound tracker in Ethereum client.

Is it possible to share the line you removed to disable the outbound trackers? Just wanted to do some checks as in the current code it seems we still rely on outbound trackers to watch the outbounds

I think this PR was intended to be a supplement to (not to disable) outbound trackers. I think it is possible to minimize our reliance on outbound trackers in future and restrict its usage to only missed outbound (similar to inbound tracker). The new observation might look like:

• New impl will scan block and filter outbound transactions and won't solely relay on trackers, this could completely remove the complexity of tracker reporters.
• New impl will scan block by block sequentially and post vote sequentially. The current impl doesn't care the order by which the votes are posted.
• New impl will need to scan quick enough to catch up external chain in order to not cause delay. The current impl doesn't need to catch up on blocks.

overall: we could switch to a block-scanning model to minimize reliance on outbound trackers while still keep them for scenarios similar to inbound trackers.

I created a issue open to discussion: https://github.com/zeta-chain/node/issues/2805

[brewmaster012]

Is it possible to share the line you removed to disable the outbound trackers? Just wanted to do some checks as in the current code it seems we still rely on outbound trackers to watch the outbounds

i tested in localnet by disabling reporting to outbound tracker.

[brewmaster012]

Also ObserverTSSReceive will be removed in the future once we fully migrate to V2 contract and wind down v1 compatibility

then this function will just become ObserveOutbound essentially a local indexer for all outbound txs. We can find a place for it that is more logical.