Skip to content

feat(ton): integrate new functionality#3977

Merged
swift1337 merged 17 commits intodevelopfrom
feat/ton-new-methods
Jun 24, 2025
Merged

feat(ton): integrate new functionality#3977
swift1337 merged 17 commits intodevelopfrom
feat/ton-new-methods

Conversation

@swift1337
Copy link
Contributor

@swift1337 swift1337 commented Jun 13, 2025

Updates

  • Update Gateway code
  • Parse and vote for call operation
  • Support increase_seqno outbound method (unblocks tx "cancellation" similarly to SOL and SUI)
  • Restrict masterchain -1 addresses
  • Restrict non-compliant outbounds

E2E [WIP]

  • call
  • withdraw_restricted
  • withdraw_masterchain

Related

Summary by CodeRabbit

  • New Features

    • Added support for TON contract call operations, enabling cross-chain calls from TON to ZEVM.
    • Introduced compliance checks and restrictions for TON withdrawals to certain addresses and workchains.
    • Implemented new handling for TON sequence number increments with reason codes.
    • Expanded end-to-end tests to cover TON call and restricted withdrawal scenarios.
  • Bug Fixes

    • Improved error messages and test assertions for better clarity and debugging.
  • Refactor

    • Streamlined outbound message composition, signature handling, and transaction tracking for TON operations.
    • Enhanced logging and error handling throughout TON-related modules.
  • Tests

    • Added new test cases and test data for TON call and increase-seqno operations.
    • Improved test coverage for compliance and edge-case scenarios.
  • Chores

    • Updated configuration and documentation to reflect new TON features and stricter linting rules.

@swift1337 swift1337 self-assigned this Jun 13, 2025
@swift1337 swift1337 added the TON_TESTS Runs TON E2E tests label Jun 13, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jun 13, 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 update introduces new TON contract operations—specifically, support for "call" and "increase_seqno"—across the protocol, observer, and signer components. It extends cross-chain transaction handling, compliance logic, test coverage, contract parsing, and signature management to accommodate these new operations. The changes also refactor withdrawal logic, enhance error reporting, and add new test data and helper functions for robust validation.

Changes

Files / Grouped Paths Change Summary
.golangci.yml Changed exclusion mode for generated code from "lax" to "strict" in lint and formatter sections.
changelog.md Added changelog entry for new TON features: call, increase_seqno, compliance.
cmd/zetae2e/local/local.go Appended three new TON test names to the local E2E test list.
e2e/e2etests/e2etests.go Added constants and test entries for TON call and new withdrawal scenarios.
e2e/e2etests/helpers.go Refactored random payload generation into a new helper function.
e2e/e2etests/test_ton_call.go Introduced new E2E test for TON-to-ZEVM call operation.
e2e/e2etests/test_ton_withdraw_masterchain.go, e2e/e2etests/test_ton_withdraw_restricted.go Added E2E tests for restricted and masterchain TON withdrawals.
e2e/e2etests/test_ton_withdrawal.go, e2e/e2etests/test_ton_withdrawal_concurrent.go Updated withdrawal logic to use revert options struct; adjusted imports.
e2e/runner/run.go Simplified and unified panic recovery and logging in test goroutine.
e2e/runner/ton.go Added TONCall method, unified call option handling, improved logging, updated withdrawal logic to use revert options.
e2e/utils/contracts.go Enhanced assertion error messages for contract call verification.
pkg/contracts/ton/gateway.compiled.json Updated contract binary and metadata fields.
pkg/contracts/ton/gateway_msg.go Added OpCall and OpIncreaseSeqno operations, new structs, signature handling, and refactored message serialization.
pkg/contracts/ton/gateway_parse.go Added parsing for OpCall and OpIncreaseSeqno, refactored outbound parsing and signature shifting.
pkg/contracts/ton/gateway_send.go Extended interface with signature/hash methods; added SendCall method for Gateway.
pkg/contracts/ton/gateway_test.go Added tests for Call ABI and IncreaseSeqno transaction parsing.
pkg/contracts/ton/gateway_tx.go Added OutboundAuth struct and methods for extracting authorization from transactions.
pkg/contracts/ton/testdata/08-call-abi.json, pkg/contracts/ton/testdata/09-increase-seqno.json Added test data for call and increase_seqno operations.
testutil/sample/ton.go Added sample transaction generators for Call and IncreaseSeqno operations.
testutil/sample/zetaclient.go Added new restricted TON address constant for compliance testing.
zetaclient/chains/ton/observer/inbound.go Added explicit handling and voting logic for OpCall; refactored deposit voting.
zetaclient/chains/ton/observer/inbound_test.go Added test for inbound Call transaction handling.
zetaclient/chains/ton/observer/outbound.go Unified outbound authorization extraction, added handling for increase_seqno, removed redundant helpers.
zetaclient/chains/ton/rpc/client_live_test.go Added exitCode field to transaction simplification output.
zetaclient/chains/ton/rpc/types.go Improved error message for invalid transaction hash string format.
zetaclient/chains/ton/signer/signer.go Refactored to use outbound message abstraction, updated method signatures, streamlined signing and tracking.
zetaclient/chains/ton/signer/signer_compose.go New file: logic for composing outbound messages based on compliance and workchain checks.
zetaclient/chains/ton/signer/signer_test.go Added test coverage for invalid receiver scenario and increase_seqno.
zetaclient/chains/ton/signer/signer_tracker.go Updated tracking to use outbound struct and unified outbound filter logic.
zetaclient/chains/ton/ton.go Removed duplicate registration of post_gas_price task.
zetaclient/compliance/compliance.go Broadened compliance check to accept additional addresses.

Sequence Diagram(s)

TON Call Operation Flow

sequenceDiagram
    participant User
    participant E2E Runner
    participant TON Gateway Contract
    participant ZEVM Contract

    User->>E2E Runner: Initiate TON call test
    E2E Runner->>TON Gateway Contract: SendCall(amount, recipient, callData)
    TON Gateway Contract->>ZEVM Contract: Execute call with payload
    ZEVM Contract-->>TON Gateway Contract: Process result
    TON Gateway Contract-->>E2E Runner: Return transaction context
    E2E Runner-->>User: Verify call and assert results
Loading

TON Withdrawal with Compliance/Workchain Check

sequenceDiagram
    participant ZEVM User
    participant E2E Runner
    participant GatewayZEVM
    participant TON Gateway
    participant Compliance Logic

    ZEVM User->>E2E Runner: Request withdrawal to TON address
    E2E Runner->>GatewayZEVM: WithdrawTONZRC20(recipient, amount, revertOptions)
    GatewayZEVM->>Compliance Logic: Check recipient/workchain
    alt Valid recipient and workchain
        Compliance Logic-->>GatewayZEVM: Allow withdrawal
        GatewayZEVM->>TON Gateway: Send Withdrawal
    else Restricted or invalid
        Compliance Logic-->>GatewayZEVM: Block withdrawal
        GatewayZEVM->>TON Gateway: Send IncreaseSeqno (revert reason)
    end
    TON Gateway-->>E2E Runner: Return transaction/cctx
    E2E Runner-->>ZEVM User: Assert result and compliance
Loading

Suggested labels

zetaclient, CONSENSUS_BREAKING_ACK

Suggested reviewers

  • lumtis
  • brewmaster012

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ 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.
    • Explain this complex logic.
    • 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 explain this code block.
    • @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 explain its main purpose.
    • @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.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this 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.

@swift1337 swift1337 changed the title feat(ton): integrate new functionality feat(ton): integrate new functionality [WIP] Jun 13, 2025
@codecov
Copy link

codecov bot commented Jun 13, 2025

Codecov Report

Attention: Patch coverage is 63.73057% with 70 lines in your changes missing coverage. Please review.

Project coverage is 64.83%. Comparing base (13d2a28) to head (847875f).
Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
zetaclient/chains/ton/observer/outbound.go 26.00% 37 Missing ⚠️
zetaclient/chains/ton/signer/signer_compose.go 73.43% 15 Missing and 2 partials ⚠️
zetaclient/chains/ton/observer/inbound.go 88.46% 4 Missing and 2 partials ⚠️
zetaclient/chains/ton/observer/observer.go 0.00% 5 Missing ⚠️
zetaclient/chains/ton/signer/signer.go 55.55% 2 Missing and 2 partials ⚠️
zetaclient/chains/ton/rpc/types.go 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3977   +/-   ##
========================================
  Coverage    64.82%   64.83%           
========================================
  Files          474      475    +1     
  Lines        34808    34860   +52     
========================================
+ Hits         22566    22602   +36     
- Misses       11191    11209   +18     
+ Partials      1051     1049    -2     
Files with missing lines Coverage Δ
zetaclient/chains/ton/signer/signer_tracker.go 71.42% <100.00%> (+9.16%) ⬆️
zetaclient/chains/ton/ton.go 57.81% <ø> (-0.33%) ⬇️
zetaclient/compliance/compliance.go 22.85% <100.00%> (+7.23%) ⬆️
zetaclient/chains/ton/rpc/types.go 52.86% <0.00%> (ø)
zetaclient/chains/ton/signer/signer.go 38.53% <55.55%> (-7.87%) ⬇️
zetaclient/chains/ton/observer/observer.go 20.96% <0.00%> (-1.07%) ⬇️
zetaclient/chains/ton/observer/inbound.go 62.06% <88.46%> (+0.14%) ⬆️
zetaclient/chains/ton/signer/signer_compose.go 73.43% <73.43%> (ø)
zetaclient/chains/ton/observer/outbound.go 27.11% <26.00%> (-1.85%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link

github-actions bot commented Jun 17, 2025

!!!WARNING!!!
nosec detected in the following files: zetaclient/chains/ton/signer/signer_compose.go, pkg/contracts/ton/gateway_parse.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 Jun 17, 2025
Copy link
Contributor

@lumtis lumtis left a comment

Choose a reason for hiding this comment

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

Looking good to me so far outside of the ci failing

@swift1337 swift1337 marked this pull request as ready for review June 18, 2025 13:07
@swift1337 swift1337 requested a review from a team as a code owner June 18, 2025 13:07
@swift1337 swift1337 requested a review from lumtis June 18, 2025 13:08
@swift1337 swift1337 changed the title feat(ton): integrate new functionality [WIP] feat(ton): integrate new functionality Jun 18, 2025
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: 12

🔭 Outside diff range comments (2)
zetaclient/chains/ton/rpc/client_live_test.go (1)

175-183: Guard against nil pointer dereference when reading exitCode

ComputePh, TrPhaseComputeVm, or Vm can be nil for non-compute transactions, causing tests to panic.

-		"exitCode":         tx.Transaction.Description.TransOrd.ComputePh.TrPhaseComputeVm.Vm.ExitCode,
+		"exitCode": func() int32 {
+			if ord, ok := tx.Transaction.Description.(*tlb.TransactionDescrOrdinary); ok {
+				if ph := ord.TransOrd.ComputePh; ph != nil && ph.TrPhaseComputeVm != nil && ph.TrPhaseComputeVm.Vm != nil {
+					return ph.TrPhaseComputeVm.Vm.ExitCode
+				}
+			}
+			return -1 // unknown / not applicable
+		}(),

Prevents flakiness when the gateway emits storage-only or bounce transactions.

pkg/contracts/ton/gateway_msg.go (1)

18-24: Define opcode constants explicitly to safeguard against accidental renumbering

Using iota means future insertions above OpCall will silently shift its value, desyncing the off-chain parser from deployed contracts.
Assign explicit numbers instead, matching the .fc contract:

const (
    OpDonate         Op = 100
    OpDeposit        Op = 101
    OpDepositAndCall Op = 102
    OpCall           Op = 103
)
🧹 Nitpick comments (31)
.golangci.yml (1)

65-67: Enforce strict exclusion of generated files in linters

Applying strict here mirrors the formatter exclusion, preventing unintentional lint suppression on non-generated code. Optionally, audit your generated outputs and consider adding explicit path patterns if any files lack the autogenerated header.

testutil/sample/zetaclient.go (1)

15-16: Validate TON address at build-time, not only at runtime

RestrictedTonAddressTest is stored as a plain string. A malformed literal would silently compile and only be caught during E2E execution.

-	RestrictedTonAddressTest = "0:fffffbd865df68188ea84d6615086c26a7b5912a60bc55fded2cdb029b67cdef"
+	// compile-time validation – panics if the string is not a valid TON account-ID
+	RestrictedTonAddressTest = ton.MustParseAccountID(
+		"0:fffffbd865df68188ea84d6615086c26a7b5912a60bc55fded2cdb029b67cdef",
+	).String()

Using ton.MustParseAccountID (already imported elsewhere in the repo) guarantees the format is correct and prevents debugging surprises later on.

Also applies to: 46-47

e2e/e2etests/helpers.go (2)

15-18: Minor naming clarity

randomPayload actually returns a hex-encoded string, not raw bytes. A more explicit name (e.g. randomHexPayload) would avoid mis-use.

No functional issue – flagging only for readability.


20-26: Avoid allocating the slice twice

randomPayload allocates once for the byte slice and once more when encoding to hex.
For tight loops this can be trimmed by re-using the same buffer:

-func randomPayloadBytes(r *runner.E2ERunner) []byte {
-	bytes := make([]byte, 50)
-	_, err := rand.Read(bytes)
-	require.NoError(r, err)
-	return bytes
+func randomPayloadBytes(r *runner.E2ERunner) []byte {
+	b := make([]byte, 50)
+	_, err := rand.Read(b)
+	require.NoError(r, err)
+	return b // single allocation kept
 }

Purely micro-optimisation – feel free to skip.

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

522-526: Keep test list alphabetically sorted for easier maintenance

The TON test slice is mostly alphabetical; inserting the three new names out of order makes future merges harder.

-			e2etests.TestTONCallName,
 			e2etests.TestTONWithdrawName,
+			e2etests.TestTONWithdrawMasterchainName,
+			e2etests.TestTONWithdrawRestrictedName,
+			e2etests.TestTONCallName,
 			e2etests.TestTONWithdrawRestrictedName,
-			e2etests.TestTONWithdrawMasterchainName,

Not critical, but maintaining order reduces merge conflicts.

e2e/e2etests/test_ton_withdrawal.go (1)

47-48: Zero-value RevertOptions{} may hide intent.

Passing an empty struct works, but conveying the intended semantics in tests is clearer when you declare a well-named constant:

var noRevertOpts = gatewayzevm.RevertOptions{}
cctx := r.WithdrawTONZRC20(receiver.GetAddress(), amount.BigInt(), noRevertOpts)

This improves readability and avoids the impression that additional fields were forgotten.

pkg/contracts/ton/testdata/08-call-abi.json (1)

1-8: Consider trimming oversized fixture data.

The embedded BOC string is ~900 bytes. If multiple such fixtures accumulate, test binaries grow noticeably. Storing the blob in a compressed .gz (and inflating at test time) or keeping it in a dedicated testdata directory outside of .go packages can mitigate size bloat while retaining fidelity.

e2e/runner/run.go (1)

53-56: Limit-to-4 KB stack dump risks truncating valuable trace information

A 4 KB buffer is often insufficient for deeply nested panics, resulting in truncated traces and lost context.
Prefer debug.Stack() (returns the full current-goroutine trace) or dynamically grow the buffer.

- buf := make([]byte, 4096)
- n := runtime.Stack(buf, false)
- r.Logger.Print("panic: %+v. Stack: %s", recoverVal, string(buf[:n]))
+ stack := debug.Stack() // imports "runtime/debug"
+ r.Logger.Print("panic: %+v. Stack:\n%s", recoverVal, stack)
zetaclient/chains/ton/observer/inbound_test.go (1)

221-228: Assert IsCrossChainCall for consistency with earlier tests

The Call-only path is still a cross-chain invocation; asserting the flag prevents silent regressions.

- assert.Equal(t, call.Recipient.Hex(), cctx.Receiver)
- assert.Equal(t, coin.CoinType_NoAssetCall, cctx.CoinType)
+ assert.Equal(t, call.Recipient.Hex(), cctx.Receiver)
+ assert.Equal(t, coin.CoinType_NoAssetCall, cctx.CoinType)
+ assert.True(t, cctx.IsCrossChainCall)
zetaclient/chains/ton/signer/signer_test.go (2)

111-120: Tracker order assumptions may produce flaky tests

trackerBag is filled via a mocked callback that can be invoked concurrently.
The assertions assume a deterministic ordering (trackerBag[0] is the withdrawal).
If the signer’s goroutines change or become parallel, the slice order may invert and break the test spuriously.

Prefer identification by nonce/hash instead of slice index:

trackers := map[uint64]string{}
for _, tr := range ts.trackerBag {
    trackers[tr.nonce] = tr.hash
}
require.Equal(t,
    rpc.TransactionToHashString(withdrawalTx),
    trackers[uint64(nonce)],
)

46-56: Hard-coded duplicate setup: extract helpers to keep test concise

Both CCTX blocks differ only by receiver / nonce. Consider a small factory helper to avoid repetition and highlight intent.

func makeCCTX(id, recv string, nonce uint64) *cc.CrossChainTx { … }

Also applies to: 66-76

e2e/e2etests/test_ton_call.go (1)

33-40: Random payload without seeding ⇒ non-deterministic binary size

randomPayloadBytes likely uses rand.Reader, which is fine, but for reproducible CI runs you may want
– at minimum – to log the payload length or seed from r.Seed (if any).
This helps debugging failed assertions when the payload length accidentally violates gas limits.

No functional issue, just a reliability consideration.

e2e/e2etests/test_ton_withdraw_restricted.go (2)

30-34: Value captured but never asserted

senderZRC20BalanceBefore is logged but not used in any assertion.
Either remove the variable or add a post-withdrawal balance check to guarantee economic correctness.

after, _ := r.TONZRC20.BalanceOf(&bind.CallOpts{}, zevmSender)
require.True(r, after.Cmp(senderZRC20BalanceBefore) <= 0)

56-64: Outbound index assumption may break

The test always inspects OutboundParams[0]. If the ordering of outbound params changes, the check will mis-classify the transaction.
Search by Asset or Hash type instead:

var tonOutbound *cctypes.OutboundParams
for _, op := range cctx.OutboundParams {
    if op.ReceiverChainId == r.ChainID("TON") { // pseudo helper
        tonOutbound = op
        break
    }
}
require.NotNil(r, tonOutbound)
e2e/e2etests/test_ton_withdraw_masterchain.go (2)

29-33: Unused balance snapshot

Same remark as in the restricted-withdraw test: you capture the pre-withdraw balance but never validate the post-state. Consider asserting that funds were not deducted when the cctx is reverted.
This materially verifies the revert path.


56-64: Assuming first outbound is TON may be fragile

Identical to the restricted-withdraw test: do not rely on slice ordering when multiple outbounds are present.

pkg/contracts/ton/gateway_send.go (1)

77-94: SendCall duplicates SendDepositAndCall logic – extract common helper

The implementations differ only by the body writer. Extract a private helper:

func (gw *Gateway) sendOp(
    ctx context.Context, s Sender, amount math.Uint,
    build func(cell *boc.Cell) error, sendMode uint8,
) error

This eliminates three near-identical functions (SendDeposit, SendDepositAndCall, SendCall) reducing maintenance overhead.

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

321-360: Reduce duplication between voteDeposit and voteCall helpers

Both helpers differ only in three constants. Consider a single voteInboundGeneric with parameters coinType, amount, isCall.
This cuts maintenance surface and guards against future divergence.

pkg/contracts/ton/gateway_test.go (1)

230-237: Minor: assert exit-code for completeness

While the parsed OpCall path is expected to succeed, the test does not assert parsedTX.ExitCode == 0.
Adding the check strengthens the regression guard.

assert.Zero(t, parsedTX.ExitCode)
pkg/contracts/ton/gateway_parse.go (1)

160-186: Validate full body consumption & guard against malformed Call payloads

parseCall stops reading as soon as it has fetched the call-data ref, but it never checks whether there is any unexpected remainder in the cell. Consider asserting that body.Exhausted() (or an equivalent final read) succeeds to reject malformed messages that prepend a valid OpCall header to junk data.

This tiny extra check hardens the parser at negligible cost.

pkg/contracts/ton/gateway_tx.go (1)

72-114: Deduplicate logic in OutboundAuth()

The two large if blocks for OpWithdraw and OpIncreaseSeqno share the same three-step pattern: cast, fetch signer, build OutboundAuth. A compact switch keeps it DRY and reduces future maintenance:

 switch tx.Operation {
 case OpWithdraw:
-    w, err := tx.Withdrawal()
+    w, err := tx.Withdrawal()
     ...
-    return OutboundAuth{Seqno: w.Seqno, Sig: w.Sig, Signer: signer}, nil
 case OpIncreaseSeqno:
-    is, err := tx.IncreaseSeqno()
+    is, err := tx.IncreaseSeqno()
     ...
-    return OutboundAuth{Seqno: is.Seqno, Sig: is.Sig, Signer: signer}, nil
 default:
     ...
 }

Keeps the method ~30 % shorter while behaving identically.

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

98-107: nonce is computed twice

nonce := cctx.GetCurrentOutboundParam().TssNonce is defined at L98, and composeOutbound recalculates the same nonce internally. Returning the already-known nonce from composeOutbound (or accepting it as argument) avoids duplication and the risk of the two values diverging if future logic changes.

e2e/e2etests/e2etests.go (1)

97-101: Keep test-name constants and registration blocks alphabetically sorted

The TON section is growing fast; maintaining alphabetical order (or another deterministic order) for constants and AllE2ETests entries eases look-ups and reduces merge conflicts.

Also applies to: 853-882

e2e/runner/ton.go (2)

362-402: Bound the wait-loop back-off to avoid excessive sleeps

tonWaitForTx adds 500 ms * attempts to the fixed 1 s sleep.
After a few dozen iterations this grows to multi-minute pauses and can easily overrun the 2-minute context timeout while the goroutine is sleeping.

Recommend switching to an exponential back-off with an upper cap (e.g. 5 s) to keep the loop responsive and predictable:

- inc := 500 * time.Millisecond * time.Duration(attempts)
- time.Sleep(interval + inc)
+ sleep := time.Duration(math.Min(
+   float64(interval)*math.Pow(1.5, float64(attempts)),
+   float64(5*time.Second),
+))
+ time.Sleep(sleep)

318-330: Avoid redundant endless approvals

SendWithdrawTONZRC20 unconditionally calls ApproveTONZRC20 for the entire balance each withdrawal.
This:

  1. Wastes gas on repeated approvals.
  2. Leaves the runner’s wallet permanently approved for unlimited allowance, increasing blast radius if the gateway is compromised.

Prefer an idempotent check (allowance < amount ⇒ approve) or pass the exact amount as allowance.

pkg/contracts/ton/gateway_msg.go (2)

124-140: DRY: consolidate duplicated body writers

writeCallBody and writeDepositAndCallBody differ only by opcode. Extract a shared helper to avoid divergence:

func writeRecipientAndCall(b *boc.Cell, op Op, recipient eth.Address, data []byte) error { … }

func writeDepositAndCallBody(...) { return writeRecipientAndCall(b, OpDepositAndCall, recipient, data) }
func writeCallBody(...)           { return writeRecipientAndCall(b, OpCall,           recipient, data) }

258-274: Validate signature recovery id before deriving signer

deriveSigner normalises v by subtracting 27 but does not check that the resulting value is 0/1.
Invalid v will pass through to SigToPub, which returns an opaque error that can be hard to diagnose.

Add an explicit guard:

if sigCopy[64] != 27 && sigCopy[64] != 28 && sigCopy[64] > 1 {
    return eth.Address{}, errors.New("invalid signature recovery id")
}
zetaclient/chains/ton/signer/signer_tracker.go (2)

24-28: Avoid value/type shadowing – rename the parameter

The value parameter is named outbound, identical to its type. While legal, this easily trips readers and tooling (“value of type outbound”). A terse alias such as ob or msg keeps intent clear without sacrificing brevity.

-func (s *Signer) trackOutbound(
-	ctx context.Context,
-	zetacore interfaces.ZetacoreClient,
-	outbound outbound,
-	prevState rpc.Account,
+func (s *Signer) trackOutbound(
+	ctx context.Context,
+	zetacore interfaces.ZetacoreClient,
+	ob outbound,
+	prevState rpc.Account,
 ) error {

41-47: Polling strategy re-downloads the same range – tighten window & expose knobs

lt/hash remain frozen at the prevState snapshot. Each loop therefore re-fetches all historical transactions, which is wasteful for busy gateways and inflates RPC latency.

Minimal refactor:

- txs, err := s.rpc.GetTransactionsSince(ctx, acc, lt, hash)
+ txs, err := s.rpc.GetTransactionsSince(ctx, acc, lt, hash)
   ...
- if len(results) == 0 {
-   time.Sleep(tick)
-   continue
- }
+ if len(results) == 0 {
+   // advance cursor to the newest item we just inspected
+   if len(txs) > 0 {
+       lt   = txs[0].Lt()
+       hash = txs[0].Hash()
+   }
+   time.Sleep(tick)
+   continue
+ }

Additionally, consider wiring timeout/tick through config to avoid hard-coded magic numbers.

testutil/sample/ton.go (2)

129-149: TONCallProps ignores call value – tests may mis-simulate real funds transfer

The generated internal message stores only the flat fee (tonSampleTxFee). If the contract relies on c.Amount (or similar) being forwarded, downstream parsing and fee accounting tests will give false positives.

- Value:  tlb.CurrencyCollection{Grams: tlb.Coins(tonSampleTxFee)},
+ Value:  tlb.CurrencyCollection{Grams: tlb.Coins(tonSampleTxFee + c.Value.Uint64())},

Adjust to whatever field represents the TON value sent with the call.


176-191: Minor: variable is shadows readability & zero-fee external message

  1. Using is as the identifier for IncreaseSeqno looks cryptic (is.AsBody()). Rename to inc or seq.
  2. externalMessageInfo sets ImportFee = 0; yet TotalTONFees defaults to tonSampleTxFee. To mimic real behaviour, explicitly charge the fee in the external message or document why it’s omitted.
-func TONIncreaseSeqnoProps(t *testing.T, acc ton.AccountID, is toncontracts.IncreaseSeqno) TONTransactionProps {
-    body, err := is.AsBody()
+func TONIncreaseSeqnoProps(t *testing.T, acc ton.AccountID, inc toncontracts.IncreaseSeqno) TONTransactionProps {
+    body, err := inc.AsBody()

Consider adding a Value of tonSampleTxFee to ImportFee for realism.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 4f24176 and d524cb1.

📒 Files selected for processing (34)
  • .golangci.yml (2 hunks)
  • changelog.md (1 hunks)
  • cmd/zetae2e/local/local.go (1 hunks)
  • e2e/e2etests/e2etests.go (3 hunks)
  • e2e/e2etests/helpers.go (1 hunks)
  • e2e/e2etests/test_ton_call.go (1 hunks)
  • e2e/e2etests/test_ton_withdraw_masterchain.go (1 hunks)
  • e2e/e2etests/test_ton_withdraw_restricted.go (1 hunks)
  • e2e/e2etests/test_ton_withdrawal.go (2 hunks)
  • e2e/e2etests/test_ton_withdrawal_concurrent.go (2 hunks)
  • e2e/runner/run.go (1 hunks)
  • e2e/runner/ton.go (6 hunks)
  • e2e/utils/contracts.go (1 hunks)
  • pkg/contracts/ton/gateway.compiled.json (1 hunks)
  • pkg/contracts/ton/gateway_msg.go (6 hunks)
  • pkg/contracts/ton/gateway_parse.go (5 hunks)
  • pkg/contracts/ton/gateway_send.go (2 hunks)
  • pkg/contracts/ton/gateway_test.go (3 hunks)
  • pkg/contracts/ton/gateway_tx.go (3 hunks)
  • pkg/contracts/ton/testdata/08-call-abi.json (1 hunks)
  • pkg/contracts/ton/testdata/09-increase-seqno.json (1 hunks)
  • testutil/sample/ton.go (2 hunks)
  • testutil/sample/zetaclient.go (2 hunks)
  • zetaclient/chains/ton/observer/inbound.go (4 hunks)
  • zetaclient/chains/ton/observer/inbound_test.go (2 hunks)
  • zetaclient/chains/ton/observer/outbound.go (8 hunks)
  • zetaclient/chains/ton/rpc/client_live_test.go (1 hunks)
  • zetaclient/chains/ton/rpc/types.go (1 hunks)
  • zetaclient/chains/ton/signer/signer.go (5 hunks)
  • zetaclient/chains/ton/signer/signer_compose.go (1 hunks)
  • zetaclient/chains/ton/signer/signer_test.go (2 hunks)
  • zetaclient/chains/ton/signer/signer_tracker.go (3 hunks)
  • zetaclient/chains/ton/ton.go (0 hunks)
  • zetaclient/compliance/compliance.go (1 hunks)
💤 Files with no reviewable changes (1)
  • zetaclient/chains/ton/ton.go
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

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

  • e2e/e2etests/test_ton_withdrawal.go
  • zetaclient/chains/ton/rpc/types.go
  • zetaclient/chains/ton/rpc/client_live_test.go
  • cmd/zetae2e/local/local.go
  • e2e/runner/run.go
  • e2e/e2etests/test_ton_withdrawal_concurrent.go
  • e2e/e2etests/helpers.go
  • testutil/sample/zetaclient.go
  • e2e/utils/contracts.go
  • zetaclient/compliance/compliance.go
  • e2e/e2etests/test_ton_withdraw_masterchain.go
  • e2e/e2etests/test_ton_call.go
  • pkg/contracts/ton/gateway_test.go
  • e2e/e2etests/test_ton_withdraw_restricted.go
  • zetaclient/chains/ton/signer/signer_test.go
  • zetaclient/chains/ton/signer/signer_tracker.go
  • zetaclient/chains/ton/observer/inbound_test.go
  • pkg/contracts/ton/gateway_send.go
  • zetaclient/chains/ton/signer/signer_compose.go
  • testutil/sample/ton.go
  • pkg/contracts/ton/gateway_tx.go
  • zetaclient/chains/ton/observer/inbound.go
  • zetaclient/chains/ton/observer/outbound.go
  • pkg/contracts/ton/gateway_parse.go
  • e2e/e2etests/e2etests.go
  • zetaclient/chains/ton/signer/signer.go
  • e2e/runner/ton.go
  • pkg/contracts/ton/gateway_msg.go
🧠 Learnings (1)
zetaclient/chains/ton/signer/signer_test.go (3)
Learnt from: gartnera
PR: zeta-chain/node#3632
File: zetaclient/chains/solana/signer/signer.go:304-304
Timestamp: 2025-03-04T22:39:58.395Z
Learning: The Solana signer implementation in zetaclient/chains/solana/signer/signer.go has limited test coverage, particularly for the transaction broadcasting logic with fallback scenarios. Adding this coverage has been acknowledged as a potential future improvement outside the scope of immediate fixes.
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/orchestrator.go:192-217
Timestamp: 2024-07-04T23:46:38.428Z
Learning: The `GetUpdatedSigner` method in `zetaclient/orchestrator/orchestrator.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go` and `zetaclient/orchestrator/orchestrator_test.go`.
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/chain_activate.go:184-247
Timestamp: 2024-07-05T00:02:31.446Z
Learning: The `CreateSignerObserverBTC` function in `zetaclient/orchestrator/chain_activate.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go`.
⏰ Context from checks skipped due to timeout of 90000ms (11)
  • GitHub Check: test-sim-nondeterminism / sim
  • GitHub Check: test-sim-after-import / sim
  • GitHub Check: test-sim-import-export / sim
  • GitHub Check: test-sim-fullappsimulation / sim
  • GitHub Check: build-and-test
  • GitHub Check: gosec
  • GitHub Check: lint
  • GitHub Check: build-zetanode
  • GitHub Check: semgrep/ci
  • GitHub Check: analyze (go)
  • GitHub Check: build
🔇 Additional comments (12)
.golangci.yml (1)

24-26: Enforce strict exclusion of generated files in formatters

Switching generated from lax to strict ensures that only files with the proper autogenerated header (e.g., “DO NOT EDIT”) are skipped by formatters, tightening lint coverage on handwritten code. Verify that all your generated artifacts (including new TON contract stubs) include the standard header so they’re correctly excluded.

changelog.md (1)

10-11: Changelog entry OK

Entry correctly references the new TON functionality and PR number.

e2e/e2etests/test_ton_withdrawal.go (1)

9-9: Validate default import name of gatewayzevm.sol.

If the generated package inside pkg/gatewayzevm.sol is declared as package gatewayzevm, the implicit import name will be gatewayzevm and this line will not compile without an explicit alias:

import gatewayzevm "github.com/zeta-chain/protocol-contracts/pkg/gatewayzevm.sol"

Please double-check the actual package clause in the generated bindings and add an alias if it differs from gatewayzevm; otherwise the build will fail.

zetaclient/chains/ton/rpc/types.go (1)

212-215: Enhanced error context – good improvement.

Including the offending string in the error message materially aids debugging while keeping the surface area low.

e2e/utils/contracts.go (1)

45-56: More descriptive assertion messages – thumbs-up.

Augmenting require.Equal and require.EqualValues with explicit messages provides immediate diagnostics when tests fail without adding runtime cost.

e2e/e2etests/test_ton_withdrawal_concurrent.go (2)

12-13: Same import-name concern as in single-withdrawal test.

Ensure the default package name of github.com/zeta-chain/protocol-contracts/pkg/gatewayzevm.sol matches the identifier gatewayzevm; otherwise add an explicit alias for build stability.


50-51: Parameter capture is safe – good call.

tx is evaluated before the goroutine is launched and passed by value, preventing the classic loop-variable capture pitfall. Nicely done.

pkg/contracts/ton/gateway.compiled.json (1)

2-5: Binary blob update acknowledged

Contract hash & binaries updated; no actionable review feedback within the JSON artefact.
Ensure corresponding ABI and on-chain deployment hashes match before merge.

pkg/contracts/ton/testdata/09-increase-seqno.json (1)

1-8: Test fixture addition looks good

The new sequence-number increment fixture provides useful coverage for cancel-outbound scenarios.

pkg/contracts/ton/gateway_send.go (1)

80-87: No runtime guard on minimum fee

Docstring states “amount should be ≥ calculate_gas_fee(op::call)” yet the function does not verify this pre-condition.
Consider adding a lightweight check calling gw.GetTxFee internally to fail fast and return a descriptive error instead of broadcasting a doomed transaction.

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

130-136: Error wording misleads for non-TSS signatures

determineReceiveStatus returns an error when the signer is not TSS, yet the caller treats any error as fatal and skips voting entirely. For non-TSS signatures (e.g. replayed or malicious tx) it may be safer to return (chains.ReceiveStatus_failed, nil) so the system records a negative vote instead of silently doing nothing.

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

81-87: Guard against nil/empty signatures & use constant-time comparison

auth.Sig == ob.message.Signature() assumes both operands are non-nil and uses the == operator, which for slices or pointers only checks address equality and may leak timing.

  1. Bail early when auth extraction fails or signature is empty.
  2. Prefer bytes.Equal (or a method provided by ton.Signature) for constant-time comparison.
- return err == nil && auth.Seqno == ob.seqno && auth.Sig == ob.message.Signature()
+ if err != nil {
+     return false
+ }
+ sig := ob.message.Signature()
+ if len(auth.Sig) == 0 || len(sig) == 0 {
+     return false
+ }
+ return auth.Seqno == ob.seqno && bytes.Equal(auth.Sig, sig)

@swift1337 swift1337 requested a review from lumtis June 19, 2025 15:02
@swift1337 swift1337 requested a review from a team June 19, 2025 15:02
@swift1337 swift1337 added this pull request to the merge queue Jun 24, 2025
Merged via the queue into develop with commit 5bd04ca Jun 24, 2025
47 of 48 checks passed
@swift1337 swift1337 deleted the feat/ton-new-methods branch June 24, 2025 16:56
lumtis pushed a commit that referenced this pull request Jul 2, 2025
* Add ton.call operation

* vote inbound call

* update ton's gateway code

* e2e: ton_to_zevm_call

* Add increaseSeqno parsing

* ton: signer: increase_seqno integration

* ton: observer: increase_seqno integration

* Fix bugs. E2E for increase_seqno

* Update changelog

* Address PR comments

* Simplify inbound voting

* outbounds: validate nonce & simplify code

* bump gw
github-merge-queue bot pushed a commit that referenced this pull request Jul 9, 2025
* add empty test dapp

* add test

* generate

* update gomod

* fix imports

* smaller message

* fix size

* ci: generate TypeScript types (#3978)

* test: lower Bitcoin E2E deposit tx fee to make nightly test cheaper (#3989)

lower Bitcoin E2E deposit test tx fee

* test: add connector fund migration e2e test using contracts only  (#3976)

* update go mod

* add a new message for migrating funds

* add migrate funds to e2e

* rebase from develop

* add v2 e2e test to check flow

* add zeta gateway deposit to zetaclient

* add github workflow

* update comments

* fix code formating

* fix code formating

* fix code formating

* fix unit tests

* fix unit tests

* revert to old command to start e2e test

* remove message for migration and refactor to using contract directly

* add changelog

* generate files after removing new message

* update generated files

* update generated files

* resolve comments 1

* remove v2 from naming

* generate files

* chore: fix some comments (#3993)

Signed-off-by: yingshanghuangqiao <yingshanghuangqiao@foxmail.com>

* feat(ton): integrate new functionality (#3977)

* Add ton.call operation

* vote inbound call

* update ton's gateway code

* e2e: ton_to_zevm_call

* Add increaseSeqno parsing

* ton: signer: increase_seqno integration

* ton: observer: increase_seqno integration

* Fix bugs. E2E for increase_seqno

* Update changelog

* Address PR comments

* Simplify inbound voting

* outbounds: validate nonce & simplify code

* bump gw

* refactor: update generated files (#4000)

update generated files

* ci: run simualtion tests nightly (#3999)

* update sim.yml

* add changelog

* update generated files

* generate

* add test for deposit with big payload

* generate

* update contract version

* try removing test

* fix wrong method called

* remove redundant log

* generate

* add version condition

---------

Signed-off-by: yingshanghuangqiao <yingshanghuangqiao@foxmail.com>
Co-authored-by: Denis Fadeev <denis@fadeev.org>
Co-authored-by: Charlie Chen <34498985+ws4charlie@users.noreply.github.com>
Co-authored-by: Tanmay <tanmay@zetachain.com>
Co-authored-by: yingshanghuangqiao <yingshanghuangqiao@foxmail.com>
Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TON: Add compliance checks

3 participants