refactor(zetaclient)!: TON observer-signer #3418

swift1337 · 2025-01-27T12:58:16Z

This PR implements a V2 orchestrator for TON similar to BTC in #3332.

Create EVM observerSigner
Fix Unit tests
Fix E2E tests

Closes #3300

Summary by CodeRabbit

Based on the comprehensive summary, here are the release notes:

New Features
- Added support for TON (The Open Network) blockchain in the ZetaChain node
- Introduced orchestrator V2 with TON observer-signer functionality
Refactoring
- Restructured configuration and package management for TON-related components
- Simplified method names and import paths in TON-related modules
- Removed ticker-based observation mechanisms
Improvements
- Enhanced blockchain interoperability
- Optimized code structure for better maintainability
- Streamlined error handling and logging mechanisms
Chores
- Cleaned up unused code and imports
- Removed deprecated methods and functions

coderabbitai · 2025-01-27T12:58:23Z

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 pull request introduces a comprehensive refactoring of the TON (The Open Network) blockchain support in the ZetaChain node, implementing Orchestrator V2 for TON integration. The changes involve restructuring configuration management, observer-signer functionality, and bootstrapping processes across multiple packages. The implementation focuses on modularizing TON-specific components while maintaining a consistent approach to blockchain integration.

Changes

File	Change Summary
`changelog.md`	Added new feature for TON observer-signer
`zetaclient/chains/ton/config/config.go`	Renamed package, simplified configuration interfaces and methods
`zetaclient/chains/ton/observer/*`	Refactored observation methods, renamed to exported methods
`zetaclient/orchestrator/*`	Removed TON-specific legacy implementations, added V2 bootstrap support
`zetaclient/chains/ton/ton.go`	New file implementing TON chain observer-signer lifecycle management

Assessment against linked issues

Objective	Addressed	Explanation
Create `ObserverSigner` wrapper	✅
Refactor orchestrator to v2 for TON	✅
Use scheduler from issue #3299	✅

Suggested Labels

breaking:proto, V2_TESTS, no-changelog, chain:ton

Suggested Reviewers

lumtis
gartnera
ws4charlie

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.

github-actions · 2025-01-27T12:58:38Z

!!!WARNING!!!
nosec detected in the following files: zetaclient/chains/ton/ton.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 · 2025-01-27T13:20:32Z

Codecov Report

Attention: Patch coverage is 6.45161% with 174 lines in your changes missing coverage. Please review.

Project coverage is 63.13%. Comparing base (5b8dd05) to head (69e9eb7).
Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
zetaclient/chains/ton/ton.go	0.00%	127 Missing ⚠️
zetaclient/orchestrator/v2_bootstrap.go	23.52%	23 Missing and 3 partials ⚠️
zetaclient/chains/ton/observer/observer.go	0.00%	7 Missing ⚠️
zetaclient/chains/ton/config/config.go	0.00%	6 Missing ⚠️
zetaclient/orchestrator/bootstrap.go	0.00%	4 Missing ⚠️
zetaclient/orchestrator/orchestrator.go	0.00%	3 Missing ⚠️
zetaclient/chains/ton/liteapi/client.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3418      +/-   ##
===========================================
+ Coverage    63.00%   63.13%   +0.12%     
===========================================
  Files          436      437       +1     
  Lines        30892    30827      -65     
===========================================
- Hits         19465    19463       -2     
+ Misses       10617    10550      -67     
- Partials       810      814       +4

Files with missing lines	Coverage Δ
zetaclient/chains/ton/observer/inbound.go	`62.44% <100.00%> (+7.68%)`	⬆️
zetaclient/chains/ton/observer/outbound.go	`27.89% <100.00%> (+3.69%)`	⬆️
zetaclient/chains/ton/signer/signer.go	`45.91% <ø> (+1.12%)`	⬆️
zetaclient/orchestrator/v2_orchestrator.go	`69.47% <100.00%> (-2.93%)`	⬇️
zetaclient/chains/ton/liteapi/client.go	`11.88% <0.00%> (ø)`
zetaclient/orchestrator/orchestrator.go	`26.52% <0.00%> (+3.64%)`	⬆️
zetaclient/orchestrator/bootstrap.go	`0.00% <0.00%> (ø)`
zetaclient/chains/ton/config/config.go	`0.00% <0.00%> (ø)`
zetaclient/chains/ton/observer/observer.go	`20.96% <0.00%> (+8.93%)`	⬆️
zetaclient/orchestrator/v2_bootstrap.go	`57.51% <23.52%> (-12.24%)`	⬇️
... and 1 more

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (20)

zetaclient/chains/ton/signer/signer.go (2)
81-81: Consider enhancing context handling for improved reliability.

The implementation is solid, but could benefit from additional context handling for better reliability in distributed systems.

Consider adding context cancellation handling and timeout configuration:
 func (s *Signer) TryProcessOutbound(
 	ctx context.Context,
 	cctx *cc.CrossChainTx,
 	proc *outboundprocessor.Processor,
 	outboundID string,
 	zetacore interfaces.ZetacoreClient,
 	zetaBlockHeight uint64,
 ) {
+	// Add timeout to prevent hanging operations
+	ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
+	defer cancel()
+
 	proc.StartTryProcess(outboundID)
 	defer proc.EndTryProcess(outboundID)
+
+	// Handle context cancellation
+	if err := ctx.Err(); err != nil {
+		s.Logger().Std.Error().Err(err).Str("outbound.id", outboundID).Msg("Context cancelled")
+		return
+	}
Line range hint 42-45: Consider making cache configuration more flexible.

The signatures cache size is hardcoded. For production systems, this should be configurable to accommodate different deployment scenarios.

Consider making the cache size configurable:
+// Config represents Signer configuration
+type Config struct {
+	SignaturesCacheSize int
+}
+
 // New Signer constructor.
-func New(baseSigner *base.Signer, client LiteClient, gateway *toncontracts.Gateway) *Signer {
-	sigCache, _ := lru.New(signaturesHashSize)
+func New(baseSigner *base.Signer, client LiteClient, gateway *toncontracts.Gateway, config Config) *Signer {
+	if config.SignaturesCacheSize <= 0 {
+		config.SignaturesCacheSize = signaturesHashSize
+	}
+	sigCache, _ := lru.New(config.SignaturesCacheSize)
zetaclient/chains/ton/observer/observer.go (4)

69-71: Add missing test coverage for PostGasPrice.

Since lines 70-71 are not covered, consider creating a unit test that verifies both successful and failure paths of config.FetchGasConfig.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 70-71: zetaclient/chains/ton/observer/observer.go#L70-L71
Added lines #L70 - L71 were not covered by tests

76-76: Consider robust error handling for ParseGasPrice.

In addition to returning the error, a log entry or fallback mechanism might be helpful if the price cannot be parsed.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 76-76: zetaclient/chains/ton/observer/observer.go#L76
Added line #L76 was not covered by tests

96-97: Add a dedicated test for CheckRPCStatus.

You could mock GetBlockHeader to produce success and failure cases to ensure correct logic under different RPC responses.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 97-97: zetaclient/chains/ton/observer/observer.go#L97
Added line #L97 was not covered by tests

104-105: Revisit the two-case switch structure.

A simple if err != nil { ... } else if block.NotMaster { ... } may be more readable.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 104-105: zetaclient/chains/ton/observer/observer.go#L104-L105
Added lines #L104 - L105 were not covered by tests

zetaclient/chains/ton/ton.go (9)

20-26: Struct definition for TON is clear but untested.

Consider adding a small struct-level test or ensuring that each field’s usage is covered by downstream tests.

28-36: New constructor is straightforward but unverified by tests.

You could validate the newly created TON object in a unit test to check default field values and error paths.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 29-35: zetaclient/chains/ton/ton.go#L29-L35
Added lines #L29 - L35 were not covered by tests

38-40: Chain() method is minimal and likely needs coverage.

Even a small test confirming that Chain() returns the expected result can raise coverage measurably.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 38-39: zetaclient/chains/ton/ton.go#L38-L39
Added lines #L38 - L39 were not covered by tests

42-100: Start method: thorough scheduling logic but complex flow.

Consider splitting out block-subscription logic from ticker scheduling to improve readability.

Add test scenarios for scheduling intervals, skipping logic, and error handling from zctx.FromContext.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 42-45: zetaclient/chains/ton/ton.go#L42-L45
Added lines #L42 - L45 were not covered by tests

[warning] 47-50: zetaclient/chains/ton/ton.go#L47-L50
Added lines #L47 - L50 were not covered by tests

[warning] 52-55: zetaclient/chains/ton/ton.go#L52-L55
Added lines #L52 - L55 were not covered by tests

[warning] 57-59: zetaclient/chains/ton/ton.go#L57-L59
Added lines #L57 - L59 were not covered by tests

[warning] 61-63: zetaclient/chains/ton/ton.go#L61-L63
Added lines #L61 - L63 were not covered by tests

[warning] 65-67: zetaclient/chains/ton/ton.go#L65-L67
Added lines #L65 - L67 were not covered by tests

[warning] 69-71: zetaclient/chains/ton/ton.go#L69-L71
Added lines #L69 - L71 were not covered by tests

[warning] 73-75: zetaclient/chains/ton/ton.go#L73-L75
Added lines #L73 - L75 were not covered by tests

[warning] 77-79: zetaclient/chains/ton/ton.go#L77-L79
Added lines #L77 - L79 were not covered by tests

[warning] 81-88: zetaclient/chains/ton/ton.go#L81-L88
Added lines #L81 - L88 were not covered by tests

[warning] 90-100: zetaclient/chains/ton/ton.go#L90-L100
Added lines #L90 - L100 were not covered by tests

103-105: Stop method lacks direct coverage.

You might confirm that the schedule group stops gracefully by verifying side effects or using a mock scheduler in a test.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 103-105: zetaclient/chains/ton/ton.go#L103-L105
Added lines #L103 - L105 were not covered by tests

108-111: group method is purely descriptive but rarely tested.

A simple test ensuring correct group name formation can help keep coverage consistent.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 108-111: zetaclient/chains/ton/ton.go#L108-L111
Added lines #L108 - L111 were not covered by tests

114-172: scheduleCCTX uses concurrency with go t.signer.TryProcessOutbound().

Ensure that any context cancellation or error from TryProcessOutbound is properly tracked.

Consider a wait group or error channel if you need synchronous finalization before exit.
Also, add coverage for each branch in the loop, especially around VoteOutboundIfConfirmed.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 114-117: zetaclient/chains/ton/ton.go#L114-L117
Added lines #L114 - L117 were not covered by tests

[warning] 119-124: zetaclient/chains/ton/ton.go#L119-L124
Added lines #L119 - L124 were not covered by tests

[warning] 127-132: zetaclient/chains/ton/ton.go#L127-L132
Added lines #L127 - L132 were not covered by tests

[warning] 134-144: zetaclient/chains/ton/ton.go#L134-L144
Added lines #L134 - L144 were not covered by tests

[warning] 148-158: zetaclient/chains/ton/ton.go#L148-L158
Added lines #L148 - L158 were not covered by tests

[warning] 161-168: zetaclient/chains/ton/ton.go#L161-L168
Added lines #L161 - L168 were not covered by tests

174-189: updateChainParams method is well-defined but untested.

A lightweight test mocking zctx.FromContext and verifying that updated chain.Params() reflect on the signer and observer can be valuable.

191-195: outboundLogger helps with structured logging but is uncovered.

Check that the logger is appended correctly and tested with at least one example using a mock or capturing logs.

zetaclient/orchestrator/v2_bootstrap.go (2)

135-169: bootstrapTON parallels bootstrapBitcoin and bootstrapEVM.

For consistency, factor out shared logic (such as retrieving configs and base observers) to reduce duplication. Also add test coverage verifying that an invalid chain or missing config triggers correct error behavior.

217-235: makeTONClient method for creating a TON client and gateway

Ensure that an empty or invalid LiteClientConfigURL or gatewayAddr is tested.

Add a small integration or unit test to validate error paths (e.g., invalid URL, parse failures).

zetaclient/chains/ton/observer/outbound.go (1)

50-52: Consider partial or concurrent handling of outbound trackers
Processing large numbers of trackers sequentially could block other operations. Consider chunked or parallel processing for scalability.

zetaclient/chains/ton/observer/inbound.go (2)

25-29: Guard against high transaction volume
Observing all net-new transactions might strain performance if the account has a large backlog. Consider adding pagination or batching.

119-120: Improve scalability for inbound trackers
This function iterates trackers in a single pass. For substantial traffic, a concurrent or batched approach might improve throughput.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 840b6e7 and fa9e0e4.

📒 Files selected for processing (19)

changelog.md (1 hunks)
cmd/zetae2e/config/clients.go (2 hunks)
zetaclient/chains/ton/config/config.go (3 hunks)
zetaclient/chains/ton/liteapi/client.go (2 hunks)
zetaclient/chains/ton/liteapi/client_live_test.go (2 hunks)
zetaclient/chains/ton/observer/inbound.go (2 hunks)
zetaclient/chains/ton/observer/inbound_test.go (8 hunks)
zetaclient/chains/ton/observer/observer.go (4 hunks)
zetaclient/chains/ton/observer/outbound.go (1 hunks)
zetaclient/chains/ton/observer/outbound_test.go (1 hunks)
zetaclient/chains/ton/signer/signer.go (1 hunks)
zetaclient/chains/ton/signer/signer_test.go (1 hunks)
zetaclient/chains/ton/ton.go (1 hunks)
zetaclient/orchestrator/bootstrap.go (2 hunks)
zetaclient/orchestrator/bootstrap_test.go (0 hunks)
zetaclient/orchestrator/orchestrator.go (2 hunks)
zetaclient/orchestrator/v2_bootstrap.go (4 hunks)
zetaclient/orchestrator/v2_bootstrap_test.go (1 hunks)
zetaclient/orchestrator/v2_orchestrator.go (1 hunks)

💤 Files with no reviewable changes (1)

zetaclient/orchestrator/bootstrap_test.go

🧰 Additional context used

📓 Path-based instructions (17)

zetaclient/chains/ton/signer/signer_test.go (1)