Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: zetatools cctx tracker #3455

Merged
merged 20 commits into from
Feb 7, 2025
Merged

feat: zetatools cctx tracker #3455

merged 20 commits into from
Feb 7, 2025

Conversation

kingpinXD
Copy link
Contributor

@kingpinXD kingpinXD commented Feb 3, 2025

Description

Closes #3447

Examples from localnet

  • Cross-Chain TX Deposit and Withdrawal
zetatool track-cctx 0xb8173ae11ac3bec02ba52aa8c6f0bb8192aca1d4d4525506f5b2ee42c931f6f9 1337
{"level":"info","time":"2025-02-05T18:54:45-05:00","message":"CCTX Identifier: 0x2cbd9c3e480da4e1c99817f3b54dd3072863c8c8e9276cf3dc35ec2e47433dd7 Status: OutboundMined"}
{"level":"info","time":"2025-02-05T18:54:45-05:00","message":"CCTX Identifier: 0x11752e9fb537615cd63a836c5f132ab813b5c78550aabfcdb796f7466b1d2e69 Status: OutboundMined"}
  • BTC Deposit
zetatool track-cctx 4725846e192561e40f6f57380c8580c60c1efa1695bec51f9c25424e89feee26 18444
{"level":"info","time":"2025-02-05T18:52:03-05:00","message":"CCTX Identifier: 0x7ac5a5b02d6e1911453824bef448f527bb1c80ab6d79f47a4061b7ddc9f911b9 Status: OutboundMined"}
  • BTC Withdrawal
zetatool track-cctx 0xe9683f27cad59fbda51f812928efac3efd55dfe84c0bbbda1f3a4c2ea46575dc 101
{"level":"info","time":"2025-02-05T18:57:42-05:00","message":"CCTX Identifier: 0x4bd5e4c2e629d3272ef3ac237503548adfabf2cff978cd507a4dd902e8d52e1a Status: OutboundMined"}
  • Erc20 Deposit and Withdrawal
zetatool track-cctx 0x3a7bd45cc8d5e07fa40dac67a4148a52f698212e672f17425018a04c95b50928 1337
{"level":"info","time":"2025-02-05T18:58:23-05:00","message":"CCTX Identifier: 0x0a4a7bd145bf4d9d1161d07a9ffcff943f76a9a060e78bd91735fd0e97296673 Status: OutboundMined"}
{"level":"info","time":"2025-02-05T18:58:23-05:00","message":"CCTX Identifier: 0x5613c984810f4d1ea1d78b86e4ae3bfd98fb9fc16b56e89cde14095735201a9d Status: OutboundMined"}
  • Eth Deposit
zetatool track-cctx 0xb9e49cb9f8b9db052fb6e4b7ec69ab47e944f5543eaf5ee53d5165fb3812cf30 1337
{"level":"info","time":"2025-02-05T19:01:46-05:00","message":"CCTX Identifier: 0x65ef578d634b8ede809d1e0627354a950f25cecae8c5d95d37d04cf14997b0d9 Status: OutboundMined"}
  • Eth Withdrawal (TSS Disabled)
zetatool track-cctx 0xf451b5c8744f312dd2c40dbfe6df6b3c804e2f42db81fcc287e23284d517ed61 101
{"level":"info","time":"2025-02-05T19:28:44-05:00","message":"CCTX Identifier: 0x21ebe6be6f8c4b0a42bd36c101584a6908cfd546a6b922842ad610a280e23346 Status: PendingOutboundSigning"}
  • Eth Withdrawal (Outbound voting disabled
zetatool track-cctx 0xf451b5c8744f312dd2c40dbfe6df6b3c804e2f42db81fcc287e23284d517ed61 101
{"level":"info","time":"2025-02-05T19:34:34-05:00","message":"CCTX Identifier: 0xa14385e299afbb3f82922c66b067d31de241bdd4116f44be7edfec56d2187a57 Status: PendingOutboundVoting"}
  • Erc20 Deposit and Withdrawal (Outbound voting disabled)
[zetatool-cctx-tracker] zetatool track-cctx 0x09533565f447591f44fcb32bc85e1380427e1f794b3fae936a15bc07d9f7400b 1337
{"level":"info","time":"2025-02-05T19:58:17-05:00","message":"CCTX Identifier: 0x21f9854f5f9a390a94f014f3354bb198cf0f4fe9bc223abfe03e176fc1ee25e6 Status: OutboundMined"}
{"level":"info","time":"2025-02-05T19:58:17-05:00","message":"CCTX Identifier: 0xe33269bc15f09d34d5583dfc3981c8752d023ff6e81839360708b57b1c26e418 Status: PendingOutboundVoting"}
  • Eth Deposit (Inbound voting disabled)
zetatool track-cctx 0x4a64f64c901ffe67cf9b71d59ed8c9cdb8d79b7caaf60f6e4e08e6ce11805a53 1337
{"level":"info","time":"2025-02-05T20:09:41-05:00","message":"CCTX Identifier: 0x85df87108c3074269765e74709d948f93f2a9be64bb5097d55a706a7e4c6313e Status: PendingInboundVoting"}
  • Solana Deposit
 zetatool track-cctx 2WWaMNPjikLSSRKuydRDy5m3YWg52JULBAqiQuxe4mmJfgXLSCetzArMkxxy2aXVGPbKRwKPSkXqhsKfvJdkY25r 902
{"level":"info","time":"2025-02-06T10:59:00-05:00","message":"CCTX Identifier: 0x327148bbfd14b325f4dc1b554de8ce610a69f6e4b639f59921f6c2e4907896c4 Status: OutboundMined"}
  • Solana Withdrawal
zetatool track-cctx 0xa95c5d0c71e836dbd8889514ee10ec9700a9e4690f652bfc73486c53a0f3b5b1 101
{"level":"info","time":"2025-02-06T11:07:32-05:00","message":"CCTX Identifier: 0x01ff56d527df25859afcbb3fc070941c1c72376f5572238f1a2b09906f106d84 Status: OutboundMined"}

Summary by CodeRabbit

  • New Features
    • Introduced a new CLI command for tracking cross-chain transaction statuses.
    • Enhanced the ballot-fetching command for clearer results.
  • Documentation
    • Updated CLI documentation and readme with revised command usage and examples.
  • Configuration
    • Revised network endpoints for better connectivity.
    • Added a debug flag to provide more detailed output.

Copy link
Contributor

coderabbitai bot commented Feb 3, 2025

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

This PR introduces a new track-cctx command in the changelog and implements comprehensive tracking for cross-chain transactions (CCTX) in zetatools. A new CCTX package is added with the TrackingDetails struct and methods to update inbound and outbound statuses. Bitcoin test initiation is refactored using a new helper function. The chains package is restructured with updated function signatures and error handling, and the CLI now includes both track-cctx and get-ballot commands. Enhancements to configuration, context management, and documentation are also provided alongside minor logging and formatting improvements.

Changes

File(s) Change Summary
changelog.md Added an "Unreleased" section with a new feature entry for the track-cctx command referencing PR 3455.
cmd/zetae2e/local/bitcoin.go, cmd/zetae2e/local/local.go Introduced a new startBitcoinTests function and refactored localE2ETest to delegate Bitcoin test execution to this helper.
cmd/zetatool/cctx/* (cctx_details.go, cctx_status.go, inbound.go, outbound.go) Added the CCTX package with the TrackingDetails struct, status constants, and multiple methods for updating transaction status, handling inbound/outbound checks, and printing details.
cmd/zetatool/chains/* (bitcoin.go, evm.go, solana.go) Updated function signatures (using pointer parameters), renamed methods to Pascal case, improved error handling, and shifted package naming from “inbound” to “chains”.
cmd/zetatool/cli/* (cctx_tracker.go, inbound_ballot.go) Added new CLI commands: track-cctx for tracking CCTX status and get-ballot for fetching an inbound ballot/cctx identifier.
cmd/zetatool/config/* Updated RPC URLs and ZetaChain IDs, added the FlagDebug constant and a new PrivateNetConfig() function.
cmd/zetatool/context/context.go Introduced a new Context package encapsulating context, configuration, ZetaCore client, inbound details, and logging.
cmd/zetatool/main.go Removed the import of the old inbound package, added the new CLI commands, and introduced a persistent debug flag.
docs/cli/zetatool/* (get_ballot.md, readme.md, track_cctx.md) Updated documentation to reflect new CLI commands and usage details; provided concise examples and parameter explanations.
Others (e2e tests, x/observer migrations, zetaclient/zetacore/tx.go) Added minor logging (e.g., printing transaction hashes and migration messages) and improved code formatting for readability.

Sequence Diagram(s)

sequenceDiagram
  participant U as User
  participant C as CLI (track-cctx)
  participant CTX as Context (NewContext)
  participant TD as TrackingDetails
  participant ZC as ZetaCore Client/RPC

  U->>C: Run "track-cctx [inboundHash] [chainID] [--config filename]"
  C->>CTX: Initialize context with inboundHash, chainID, config
  CTX-->>C: Return Context object
  C->>TD: Call trackCCTX(ctx)
  TD->>ZC: Check inbound/outbound transaction status
  ZC-->>TD: Return transaction status details
  TD-->>C: Return updated CCTX details
  C->>U: Display CCTX tracking result
Loading
sequenceDiagram
  participant U as User
  participant C as CLI (get-ballot)
  participant CTX as Context (NewContext)
  participant TD as TrackingDetails
  participant ZC as ZetaCore Client/RPC

  U->>C: Run "get-ballot [inboundHash] [chainID] [--config filename]"
  C->>CTX: Initialize context with inboundHash, chainID, config
  CTX-->>C: Return Context object
  C->>TD: Call CheckInbound(ctx)
  TD->>ZC: Fetch inbound ballot identifier
  ZC-->>TD: Return ballot details
  TD-->>C: Return inbound ballot information
  C->>U: Display ballot identifier
Loading

Assessment against linked issues

Objective Addressed Explanation
Enable tracking a CCTX through its various states (e.g., PendingOutbound, OutboundMined, PendingRevert, Reverted, Aborted) (#3447)

Possibly related PRs

Suggested labels

chain:bitcoin, no-changelog, E2E

Suggested reviewers

  • lumtis
  • swift1337
  • fbac
  • skosito

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
🪧 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>, please review it.
    • 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @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 generate docstrings to generate docstrings for this PR. (Beta)
  • @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.

Copy link

github-actions bot commented Feb 3, 2025

!!!WARNING!!!
nosec detected in the following files: cmd/zetatool/chains/bitcoin.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 github-actions bot added the nosec label Feb 3, 2025
@kingpinXD kingpinXD changed the title Zetatool cctx tracker feat: zetatools cctx tracker Feb 3, 2025
Copy link

codecov bot commented Feb 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.42%. Comparing base (2640b35) to head (b77bc7e).
Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3455   +/-   ##
========================================
  Coverage    65.42%   65.42%           
========================================
  Files          442      442           
  Lines        30525    30527    +2     
========================================
+ Hits         19970    19972    +2     
  Misses        9700     9700           
  Partials       855      855           
Files with missing lines Coverage Δ
zetaclient/zetacore/tx.go 84.12% <100.00%> (+0.52%) ⬆️

@kingpinXD kingpinXD marked this pull request as ready for review February 3, 2025 19:05
@kingpinXD kingpinXD requested a review from a team as a code owner February 3, 2025 19:05
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🔭 Outside diff range comments (1)
cmd/zetatool/config/config.go (1)

68-80: Configuration Validation Missing in PrivateNetConfig

The current implementation returns hardcoded configuration values without any checks, and the connectivity tests confirm that none of the localhost endpoints are accessible. This lack of validation could lead to runtime issues when the application attempts to interact with these services.

  • The configuration function does not validate endpoint format or connectivity.
  • The shell script output indicates that ZetaChain RPC, Ethereum RPC, and Solana RPC endpoints are unreachable.
  • Adding runtime checks (e.g., verifying URL format and connectivity) would help catch misconfigurations early.
🔗 Analysis chain

Consider adding configuration validation.

The PrivateNetConfig function returns hardcoded values without validation. Consider adding validation for the localhost URLs and credentials.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if localhost URLs are accessible
# Note: This script helps validate the configuration but should be adapted based on your specific needs

echo "Checking localhost URLs..."
curl -s -o /dev/null -w "%{http_code}" http://127.0.0.1:26657 || echo "ZetaChain RPC not accessible"
curl -s -o /dev/null -w "%{http_code}" http://127.0.0.1:8545 || echo "Ethereum RPC not accessible"
curl -s -o /dev/null -w "%{http_code}" http://127.0.0.1:8899 || echo "Solana RPC not accessible"

Length of output: 454

🧹 Nitpick comments (33)
cmd/zetatool/chains/bitcoin.go (2)

21-80: Consider more descriptive error handling and naming.
While this function logically retrieves and validates Bitcoin transaction data, consider returning enriched error messages to ease debugging. Also, “BitcoinBallotIdentifier” might benefit from a more aligned name (e.g., BitcoinCctxIdentifier) since it computes a CCTX identifier rather than performing ballot-specific logic.


83-116: Validate event more thoroughly or log if abnormal.
identifierFromBtcEvent succinctly constructs a CCTX-inbound vote message. However, consider logging or wrapping errors (e.g., when DecodeMemoBytes or GetSatoshis fails) to simplify troubleshooting and preserve context in production logs.

cmd/zetatool/cctx/outbound.go (3)

20-36: Clarify inbound vs. outbound chain usage.
CheckOutbound fetches the chain via ctx.GetInboundChain(). Renaming or confirming the source (e.g., ctx.GetOutboundChain()) could reduce confusion, ensuring the naming clearly matches its purpose.


38-85: Log retrieval issues for EVM transaction checks.
Currently, failures in retrieving or confirming EVM transactions are silently skipped via continue. Logging warnings or attaching context to these errors can aid in diagnosing issues, especially if confirmations fail intermittently.


114-173: Add minimal logging on skipped Bitcoin transactions.
Similar to the EVM approach, errors in retrieving Bitcoin transactions are handled with continue, silently ignoring potential network or parsing issues. Logging or wrapping these errors would promote clearer production diagnostics.

cmd/zetatool/cctx/cctx_details.go (3)

28-44: Add logs for unmapped CCTX statuses.
While UpdateStatusFromZetacoreCCTX sets unknown status by default, consider logging the unmapped status value, which could assist in debugging missing or future status codes from zetacore.


54-71: Improve error context.
UpdateCCTXStatus simply sets c.Message if an error occurs. Adding contextual information (e.g., the failing function name or external call details) would significantly aid root-cause analysis.


94-119: Outbound signing and broadcast logic is clear but lightly logged.
While UpdateHashListAndPendingStatus transitions statuses properly, consider logging transitions or discovered trackers for clarity during troubleshooting.

cmd/zetatool/chains/evm.go (1)

29-36: Mapping networks to RPC endpoints
The map-based approach is concise. For future expansions, ensure coverage of new EVM networks so that an undefined network does not silently return an empty string.

cmd/zetatool/cctx/inbound.go (1)

149-274: All-encompassing EVM inbound processing
This function correctly handles various EVM contracts and events. Consider modularizing parts of this function for improved readability.

cmd/zetatool/chains/solana.go (2)

23-24: Consider documenting the empty string parameter.

The first parameter in NewMsgVoteInbound is set to an empty string without explanation. Consider adding a comment explaining why this parameter is empty or use a named constant to make the intent clear.


38-40: Consider making protocol version and status configurable.

The protocol version and inbound status are hardcoded. Consider making these configurable through parameters to make the function more flexible for future changes.

 func VoteMsgFromSolEvent(event *clienttypes.InboundEvent,
-	zetaChainID int64) (*crosschaintypes.MsgVoteInbound, error) {
+	zetaChainID int64,
+	protocolVersion crosschaintypes.ProtocolContractVersion,
+	status crosschaintypes.InboundStatus) (*crosschaintypes.MsgVoteInbound, error) {
e2e/e2etests/test_eth_withdraw.go (1)

28-28: Enhance log message with context.

The log message could be more descriptive to aid in debugging. Consider adding more context about the withdrawal operation.

-	fmt.Println("withdraw tx", tx.Hash().Hex())
+	fmt.Printf("ETH withdrawal transaction initiated - hash: %s\n", tx.Hash().Hex())
cmd/zetatool/cli/inbound_ballot.go (2)

28-30: Improve error message for chain ID parsing.

The error message could be more descriptive by including the invalid value and expected format.

-		return fmt.Errorf("failed to parse chain id")
+		return fmt.Errorf("failed to parse chain id %q: %w", args[1], err)

48-55: Standardize logging style.

The code mixes Printf and Print. Consider standardizing the logging style for consistency.

-		log.Printf(
-			"Ballot Identifier: %s, warning the inbound hash might not be confirmed yet",
-			cctxDetails.CCTXIdentifier,
-		)
+		log.Printf("Ballot Identifier: %s, warning the inbound hash might not be confirmed yet", cctxDetails.CCTXIdentifier)
-	log.Print("Ballot Identifier: ", cctxDetails.CCTXIdentifier)
+	log.Printf("Ballot Identifier: %s", cctxDetails.CCTXIdentifier)
cmd/zetatool/config/config_test.go (1)

31-55: Consider using table-driven tests.

The test could be refactored to use table-driven tests for better maintainability and readability. This would make it easier to add new test cases and reduce code duplication.

 func TestGetConfig(t *testing.T) {
+	tests := []struct {
+		name           string
+		chain          chains.Chain
+		configFile     string
+		expectedRPC    string
+		expectedError  error
+	}{
+		{
+			name:        "Default Ethereum config",
+			chain:      chains.Ethereum,
+			configFile: "",
+			expectedRPC: "https://zetachain-mainnet.g.allthatnode.com:443/archive/tendermint",
+		},
+		// Add more test cases...
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg, err := config.GetConfig(tt.chain, tt.configFile)
+			if tt.expectedError != nil {
+				require.Error(t, err)
+				require.ErrorIs(t, err, tt.expectedError)
+				return
+			}
+			require.NoError(t, err)
+			require.Equal(t, tt.expectedRPC, cfg.ZetaChainRPC)
+		})
+	}
cmd/zetatool/context/context.go (2)

24-51: Consider enhancing error handling in NewContext.

The error handling could be improved by:

  1. Adding validation for empty inboundHash
  2. Adding validation for empty configFile
  3. Using more descriptive error messages
 func NewContext(ctx context.Context, inboundChainID int64, inboundHash string, configFile string) (*Context, error) {
+    if inboundHash == "" {
+        return nil, fmt.Errorf("inbound hash cannot be empty")
+    }
     observationChain, found := chains.GetChainFromChainID(inboundChainID, []chains.Chain{})
     if !found {
-        return nil, fmt.Errorf("chain not supported,chain id: %d", inboundChainID)
+        return nil, fmt.Errorf("chain not supported: chain ID %d", inboundChainID)
     }

39-42: Consider using a more configurable logger setup.

The current logger setup with zerolog.Nop() is hardcoded. Consider making it configurable to allow for different logging behaviors in different environments.

+type LogConfig struct {
+    Output io.Writer
+    Level  zerolog.Level
+}
+
 func NewContext(ctx context.Context, inboundChainID int64, inboundHash string, configFile string) (*Context, error) {
     // ... existing code ...
-    logger := zerolog.New(zerolog.ConsoleWriter{
-        Out:        zerolog.Nop(),
-        TimeFormat: time.RFC3339,
-    }).With().Timestamp().Logger()
+    logConfig := getLogConfig() // implement based on environment
+    logger := zerolog.New(logConfig.Output).Level(logConfig.Level).With().Timestamp().Logger()
cmd/zetatool/cctx/inbound_test.go (2)

14-49: Consider adding more test cases for error scenarios.

The test cases focus on happy paths. Consider adding test cases for:

  1. Invalid chain IDs
  2. Invalid hash formats
  3. Empty hashes
  4. Unsupported chains
 tt := []struct {
     name                     string
     inboundHash              string
     inboundChainID           int64
     expectedBallotIdentifier string
     expectError              bool
 }{
+    {
+        name:           "Invalid Chain ID",
+        inboundHash:    "0x123",
+        inboundChainID: -1,
+        expectError:    true,
+    },
+    {
+        name:           "Empty Hash",
+        inboundHash:    "",
+        inboundChainID: chains.Ethereum.ChainId,
+        expectError:    true,
+    },
     // existing test cases...
 }

51-62: Enhance test error handling and assertions.

The test could benefit from more specific error assertions and better error messages.

 for _, tc := range tt {
     t.Run(tc.name, func(t *testing.T) {
         ctx, err := zetatoolcontext.NewContext(context.Background(), tc.inboundChainID, tc.inboundHash, "")
-        require.NoError(t, err)
+        if tc.expectError {
+            require.Error(t, err)
+            return
+        }
+        require.NoError(t, err, "failed to create context")
         c := cctx.NewCCTXDetails()
         err = c.CheckInbound(ctx)
-        require.NoError(t, err)
+        require.NoError(t, err, "failed to check inbound")
         if !tc.expectError && c.CCTXIdentifier != tc.expectedBallotIdentifier {
-            t.Errorf("expected %s, got %s", tc.expectedBallotIdentifier, c.CCTXIdentifier)
+            t.Errorf("ballot identifier mismatch:\nexpected: %s\ngot: %s", tc.expectedBallotIdentifier, c.CCTXIdentifier)
         }
     })
 }
cmd/zetatool/cctx/cctx_status.go (2)

6-34: Consider using iota consistently for status constants.

The status constants mix explicit values with iota. Consider using iota consistently for better maintainability.

 const (
     Unknown Status = iota
-    PendingOutbound Status = 1
-    OutboundMined   Status = 2
-    PendingRevert   Status = 3
-    Reverted        Status = 4
-    Aborted         Status = 5
+    PendingOutbound
+    OutboundMined
+    PendingRevert
+    Reverted
+    Aborted
     // ... rest of the constants
 )

36-67: Consider using a map for String() method.

The switch statement could be replaced with a map for better maintainability and performance.

+var statusStrings = map[Status]string{
+    PendingInboundConfirmation: "PendingInboundConfirmation",
+    PendingInboundVoting:       "PendingInboundVoting",
+    PendingOutbound:            "PendingOutbound",
+    // ... rest of the mappings
+}
+
 func (s Status) String() string {
-    switch s {
-    case PendingInboundConfirmation:
-        return "PendingInboundConfirmation"
-    // ... rest of the cases
-    default:
-        return "Unknown"
-    }
+    if str, ok := statusStrings[s]; ok {
+        return str
+    }
+    return "Unknown"
 }
cmd/zetatool/cli/cctx_tracker.go (3)

28-30: Enhance error message with parsed value.

The error message could be more helpful by including the value that failed to parse.

-    return fmt.Errorf("failed to parse chain id")
+    return fmt.Errorf("failed to parse chain id %q: %w", args[1], err)

59-62: Improve error message clarity.

The error message could be more specific about what failed during the ballot identifier check.

-    return cctxDetails, fmt.Errorf("failed to get ballot identifier: %v", err)
+    return cctxDetails, fmt.Errorf("failed to get ballot identifier for inbound hash %s: %w", ctx.InboundHash, err)

53-97: Consider breaking down the tracking logic.

The function is well-commented but could benefit from being split into smaller, focused methods to improve maintainability.

Consider extracting the following methods:

  1. checkAndUpdateInbound
  2. updateOutboundDetails
  3. checkAndUpdateOutbound

This would make the main flow easier to follow and test.

cmd/zetae2e/local/bitcoin.go (1)

17-75: LGTM! Well-organized test initialization.

The function effectively organizes Bitcoin tests into basic and advanced categories with clear separation of deposit and withdrawal tests.

Consider documenting the test categories in a comment block for better maintainability.

+// Bitcoin test categories:
+// 1. Basic deposit tests: Standard deposits, donations, and cross-chain swaps
+// 2. Advanced deposit tests: Edge cases like dust amounts and reverts
+// 3. Basic withdrawal tests: Standard SegWit withdrawals and error cases
+// 4. Advanced withdrawal tests: Various address types (Taproot, Legacy, P2SH, P2WSH)
 func startBitcoinTests(
docs/cli/zetatool/readme.md (2)

3-3: Fix punctuation in the introductory sentence.
There is a missing space after the period in "Zetachain.It". Please update it to "Zetachain. It" for improved readability.


4-5: Standardize terminology and clarify command descriptions.
Both command descriptions use "chain id". For clarity and consistency, change these to "chain ID". In addition, the description for the track-cctx command contains an extraneous "using" (i.e. "using from")—consider revising it to "Track the status of a cctx from the inbound hash and chain ID."

docs/cli/zetatool/get_ballot.md (2)

17-17: Clarify configuration description.
The sentence "When not provided, the configuration in the file is user" is unclear. Consider revising it to "When not provided, the tool uses the default user configuration."


20-20: Enhance punctuation and phrasing in RPC details.
There is a missing space after the period in "default rpcs.It". Rephrase the sentence to:
"If not provided, the tool automatically uses the default RPCs and can fetch the required RPC based on the chain ID."

[style]

🧰 Tools
🪛 LanguageTool

[uncategorized] ~20-~20: A comma might be missing here.
Context: ...needed for the tool to function, if not provided the tool automatically uses the default...

(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)


[style] ~20-~20: As a shorter alternative for ‘able to’, consider using “can”.
Context: ... automatically uses the default rpcs.It is able to fetch the rpc needed using the chain ID...

(BE_ABLE_TO)

docs/cli/zetatool/track_cctx.md (3)

3-3: Correct heading capitalization.
"TRack the status of a CCTX" should be updated to "Track the status of a CCTX" to follow proper title case formatting.


17-17: Clarify configuration information.
The sentence "When not provided, the configuration in the file is user" is ambiguous. Please change it to "When not provided, the tool uses the default user configuration."


21-22: Improve punctuation and phrasing in RPC details.
There is a missing space after "rpcs.It" and the sentence would benefit from rephrasing. A suggested revision is:
"If not provided, the tool automatically uses the default RPCs and can fetch the required RPC based on the chain ID."

[style]

🧰 Tools
🪛 LanguageTool

[style] ~21-~21: As a shorter alternative for ‘able to’, consider using “can”.
Context: ... automatically uses the default rpcs.It is able to fetch the rpc needed using the chain ID...

(BE_ABLE_TO)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9106fe6 and 837c288.

📒 Files selected for processing (25)
  • changelog.md (1 hunks)
  • cmd/zetae2e/local/bitcoin.go (1 hunks)
  • cmd/zetae2e/local/local.go (1 hunks)
  • cmd/zetatool/cctx/cctx_details.go (1 hunks)
  • cmd/zetatool/cctx/cctx_status.go (1 hunks)
  • cmd/zetatool/cctx/inbound.go (1 hunks)
  • cmd/zetatool/cctx/inbound_test.go (1 hunks)
  • cmd/zetatool/cctx/outbound.go (1 hunks)
  • cmd/zetatool/chains/bitcoin.go (4 hunks)
  • cmd/zetatool/chains/evm.go (8 hunks)
  • cmd/zetatool/chains/solana.go (1 hunks)
  • cmd/zetatool/cli/cctx_tracker.go (1 hunks)
  • cmd/zetatool/cli/inbound_ballot.go (1 hunks)
  • cmd/zetatool/config/config.go (2 hunks)
  • cmd/zetatool/config/config_test.go (1 hunks)
  • cmd/zetatool/context/context.go (1 hunks)
  • cmd/zetatool/inbound/inbound.go (0 hunks)
  • cmd/zetatool/inbound/solana.go (0 hunks)
  • cmd/zetatool/main.go (2 hunks)
  • docs/cli/zetatool/get_ballot.md (1 hunks)
  • docs/cli/zetatool/readme.md (1 hunks)
  • docs/cli/zetatool/track_cctx.md (1 hunks)
  • e2e/e2etests/test_eth_withdraw.go (2 hunks)
  • x/observer/migrations/v9/migrate.go (2 hunks)
  • zetaclient/zetacore/tx.go (1 hunks)
💤 Files with no reviewable changes (2)
  • cmd/zetatool/inbound/solana.go
  • cmd/zetatool/inbound/inbound.go
✅ Files skipped from review due to trivial changes (1)
  • x/observer/migrations/v9/migrate.go
🧰 Additional context used
📓 Path-based instructions (18)
e2e/e2etests/test_eth_withdraw.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetatool/config/config_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/zetacore/tx.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetatool/cctx/inbound_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetae2e/local/bitcoin.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetae2e/local/local.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetatool/config/config.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetatool/chains/solana.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetatool/main.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetatool/cli/inbound_ballot.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetatool/context/context.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetatool/chains/bitcoin.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetatool/cctx/outbound.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetatool/cli/cctx_tracker.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetatool/cctx/cctx_status.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetatool/cctx/inbound.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetatool/cctx/cctx_details.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetatool/chains/evm.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

🪛 LanguageTool
docs/cli/zetatool/get_ballot.md

[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...c580f44ee2b3eb5ed"} ``` - inboundHash: The inbound hash of the transaction for...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~20-~20: A comma might be missing here.
Context: ...needed for the tool to function, if not provided the tool automatically uses the default...

(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)


[style] ~20-~20: As a shorter alternative for ‘able to’, consider using “can”.
Context: ... automatically uses the default rpcs.It is able to fetch the rpc needed using the chain ID...

(BE_ABLE_TO)

docs/cli/zetatool/track_cctx.md

[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...us: OutboundMined"} ``` - inboundHash: The inbound hash of the transaction for...

(UNLIKELY_OPENING_PUNCTUATION)


[style] ~21-~21: As a shorter alternative for ‘able to’, consider using “can”.
Context: ... automatically uses the default rpcs.It is able to fetch the rpc needed using the chain ID...

(BE_ABLE_TO)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: start-e2e-test / e2e
🔇 Additional comments (29)
cmd/zetatool/chains/bitcoin.go (1)

1-1: No issues with the package and imports.
This line simply reflects the new package name, which appears consistent with the updated directory structure.

cmd/zetatool/cctx/outbound.go (1)

87-112: Ensure Solana transaction confirmation logic matches requirement.
This snippet marks transactions as confirmed solely on a successful fetch. Confirm that Solana requires no additional “confirmations” threshold. Otherwise, consider extending the check to properly handle partial confirmations.

cmd/zetatool/cctx/cctx_details.go (2)

11-19: Struct fields appear well-defined.
The fields in TrackingDetails effectively capture essential CCTX parameters. The Message field is a helpful free-form slot for storing error or status details.


129-166: State transition methods maintain clarity.
Transitions like updateInboundConfirmation, updateOutboundSigning, updateOutboundConfirmation, and updateOutboundVoting convey a clean state machine approach. No pressing concerns.

cmd/zetatool/chains/evm.go (10)

1-1: Renaming the package to chains is consistent with the file's purpose
This aligns well with the rest of the code and clarifies chain-specific functionalities.


19-19: Importing context aligns with the new context-based design
The approach leverages a centralized mechanism for passing configuration and logger references.


38-48: Sufficient error checks on RPC creation
This function neatly handles invalid or missing RPC URLs, returning informative error messages.


50-71: Clear transaction retrieval flow
The logic properly accounts for absent or pending transactions, reducing unexpected behaviors in subsequent operations.


73-105: Consistent bridging of event data into inbound message
The function cleanly transforms event attributes into a cross-chain message. Consider verifying coverage through unit tests.


106-135: Sound handling of ERC20 deposit events
Donation messages are omitted effectively. The logic to convert contract events into inbound messages is straightforward.


136-167: Accurate representation of gas-based inbound transactions
Captures relevant data for the inbound vote message. Additional inline documentation could aid future maintainers.


168-205: Appropriate handling of deposit with optional cross-chain call
Marking cross-chain calls via a boolean flag adds clarity. Consider verifying the deposit logic with integration tests.


206-236: Robust deposit-and-call functionality
The approach closely parallels deposit-only flows but allows for immediate contract execution. The structure remains consistent with V2 logic.


238-261: Inbound call scenario with no asset transfer
Using CoinType_NoAssetCall helps differentiate this flow from deposit-based transactions. Implementation appears reliable.

cmd/zetatool/cctx/inbound.go (6)

1-1: Introduction of the cctx package
Placing cross-chain transaction tracking in a dedicated package fosters maintainability and logical separation.


3-25: Effective inclusion of required dependencies
Packages for EVM, Solana, Bitcoin, and related modules are neatly grouped. Remove any unused imports if encountered.


27-85: Robust detection of inbound chain and dispatch
Using a switch-case block to handle chain-specific actions is clear and extensible.


87-147: Thorough handling of Bitcoin inbound logic
RPC setup, network parameter retrieval, and TSS address checks are all well handled. Ensures minimal risk of connection issues.


276-332: Potential re-use of a single message structure for multiple events
Currently, the final event in the loop overwrites preceding data. If multiple inbound events are valid in a single transaction, verify whether you need to track them individually.


334-359: Properly retrieving ZetaChain CCTX references
The checks for multiple or missing CCTX references showcase a solid defensive approach to unexpected states.

cmd/zetatool/main.go (3)

9-9: Refined import structure for CLI commands
Moving command definitions to a specialized cli package promotes maintainability and organization.


19-20: Enhanced root command with inbound and CCTX tracking
Adding the new commands broadens tool functionality for managing cross-chain transactions.


22-23: Persistent debug flag
Providing a debug mode helps diagnose issues and fosters clearer troubleshooting when commands fail.

cmd/zetatool/context/context.go (1)

53-75: LGTM: Clean getter implementations.

The getter methods are well-implemented, providing clean access to the context fields while maintaining encapsulation.

zetaclient/zetacore/tx.go (1)

93-95: LGTM: Improved code formatting.

The multi-line formatting of the GetOutboundTracker call improves readability.

cmd/zetatool/cli/cctx_tracker.go (1)

16-23: LGTM! Well-structured command definition.

The command follows Cobra's best practices with proper argument validation and clear usage description.

cmd/zetatool/config/config.go (1)

17-17: LGTM! Flag constant follows naming convention.

The FlagDebug constant follows the established naming pattern.

cmd/zetae2e/local/local.go (1)

294-294: LGTM! Clean refactoring of Bitcoin test initialization.

The refactoring improves code organization by moving Bitcoin test setup to a dedicated function while maintaining the existing functionality.

changelog.md (1)

3-8: Changelog entry for the new feature.
The new entry for the track-cctx command is clear and properly references PR [3455]. Ensure that future changelog entries maintain this consistent format.

@kingpinXD kingpinXD marked this pull request as draft February 4, 2025 18:48
@kingpinXD kingpinXD marked this pull request as ready for review February 6, 2025 16:23
Copy link
Contributor

@ws4charlie ws4charlie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This two CLI commands are useful for fast check!

@kingpinXD kingpinXD enabled auto-merge February 7, 2025 14:35
@kingpinXD kingpinXD added this pull request to the merge queue Feb 7, 2025
Merged via the queue into develop with commit 1d5ec6d Feb 7, 2025
44 checks passed
@kingpinXD kingpinXD deleted the zetatool-cctx-tracker branch February 7, 2025 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zetatools : enable tracking a cctx through its various states
3 participants