feat(ton): adjacent TON tasks #3075
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces significant updates to the ZetaChain node, including new features such as support for stateful precompiled contracts, enhancements for Bitcoin integration, and improvements in error handling and logging. It also adds new test cases for the TON blockchain, refactors existing functions, and updates the changelog for version 21.0.0. Additionally, various fixes are implemented to enhance the reliability and functionality of the node. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
|
!!!WARNING!!! Be very careful about using 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 Pay extra attention to the way |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3075 +/- ##
===========================================
- Coverage 63.47% 63.40% -0.07%
===========================================
Files 422 422
Lines 29821 29910 +89
===========================================
+ Hits 18928 18964 +36
- Misses 10059 10105 +46
- Partials 834 841 +7
|
swift1337
requested review from
brewmaster012,
kingpinXD,
lumtis,
ws4charlie,
skosito and
fbac
as code owners
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Outside diff range and nitpick comments (20)
e2e/e2etests/test_ton_deposit_refund.go (2)
13-13: Enhance error message for input validation.The length check could be more descriptive about the expected argument.
-require.Len(r, args, 1) +require.Len(r, args, 1, "TestTONDepositAndCallRefund requires exactly one argument: deposit amount")
30-36: Enhance logging for transaction execution.Add more detailed logging to help with debugging test failures.
+r.Logger.Info("Executing TON deposit and call transaction", + "amount", amount, + "target", reverterAddr.String(), + "data_length", len(data)) cctx, err := r.TONDepositAndCall( &r.TONDeployer.Wallet, amount, reverterAddr, data, runner.TONExpectStatus(cctypes.CctxStatus_Reverted), )e2e/e2etests/test_ton_withdrawal.go (2)
34-36: Document the purpose of the boolean parameter in GetBalanceOf calls.The new boolean parameter's purpose and impact on balance retrieval behavior should be documented for better maintainability and understanding.
Add a comment explaining what the
trueflag represents:+ // GetBalanceOf with `true` parameter to [document the specific behavior] tonRecipientBalanceBefore, err := deployer.GetBalanceOf(r.Ctx, tonRecipient.GetAddress(), true)Also applies to: 61-63
Line range hint
1-89: Consider enhancing test robustness with additional assertions.While the test is well-structured, consider adding these improvements:
- Assert that the balance difference matches the withdrawal amount (accounting for fees)
- Add timeout context for external calls
Example implementation for balance difference assertion:
// Calculate and verify the exact balance changes balanceDiff := tonRecipientBalanceAfter.Sub(tonRecipientBalanceBefore) require.Equal(r, amount.String(), balanceDiff.String(), "Recipient balance should increase by exact withdrawal amount")Example implementation for timeout context:
ctx, cancel := context.WithTimeout(r.Ctx, 30*time.Second) defer cancel() // Use ctx instead of r.Ctx in external callse2e/runner/setup_ton.go (2)
58-63: Document the new boolean parameter in GetBalanceOf.The purpose of the third parameter (
true) inGetBalanceOfis not immediately clear. Consider adding a comment explaining its significance or use a named parameter for better clarity.- gwBalance, err := deployer.GetBalanceOf(ctx, gwAccount.ID, true) + // includeUnconfirmed=true to include pending transactions in balance + gwBalance, err := deployer.GetBalanceOf(ctx, gwAccount.ID, includeUnconfirmed: true)
74-75: Document the reason for increased deposit amount.The deposit amount has been increased significantly from 1000 to 10000 TON. Consider adding a comment explaining why this increase was necessary.
- veryFirstDeposit := toncontracts.Coins(10000) + // Increased initial deposit to 10000 TON to accommodate for multiple concurrent withdrawals + veryFirstDeposit := toncontracts.Coins(10000)pkg/contracts/ton/gateway_msg.go (1)
27-34: Consider enhancing error code documentation and structure.While the
ExitCodetype is well-documented with links to TVM and Zeta error codes, consider these improvements:
- Add documentation for
ExitCodeInvalidSeqnoexplaining its specific meaning and when it occurs.- Consider using iota for better maintainability if more error codes will be added.
Example enhancement:
type ExitCode uint32 const ( - ExitCodeInvalidSeqno ExitCode = 109 + // ExitCodeInvalidSeqno indicates that the sequence number in the message + // does not match the expected value in the contract state + ExitCodeInvalidSeqno ExitCode = iota + 109 )e2e/runner/ton/deployer.go (1)
60-66: Enhance documentation and consider timeout handling.While the implementation is correct, the documentation could be more comprehensive to help users make informed decisions about the
waitparameter.Consider updating the documentation as follows:
-// GetBalanceOf returns the balance of a given account. -// wait=true waits for account activation. +// GetBalanceOf returns the balance of a given account in TON coins. +// Parameters: +// - ctx: Context for timeout and cancellation +// - id: TON account ID to check +// - wait: If true, waits for account activation before checking balance. +// This is useful when checking balance right after deployment. +// Returns: +// - math.Uint: Account balance in TON coins +// - error: On context cancellation, network issues, or if account activation timeout occursAdditionally, consider adding context timeout validation before the wait operation:
func (d *Deployer) GetBalanceOf(ctx context.Context, id ton.AccountID, wait bool) (math.Uint, error) { + if ctx.Err() != nil { + return math.Uint{}, errors.Wrap(ctx.Err(), "context error") + } if wait { if err := d.waitForAccountActivation(ctx, id); err != nil { return math.Uint{}, errors.Wrap(err, "failed to wait for account activation") } }zetaclient/chains/ton/observer/observer_test.go (1)
184-190: Consider enhancing TxToInboundTracker with documentation and validation.The method would benefit from:
- Documentation explaining the conversion logic
- Validation of the input transaction
+// TxToInboundTracker converts a TON transaction to an InboundTracker. +// The tracker is initialized with the chain ID of the test suite and +// uses the transaction's hash as the tracker's hash. func (ts *testSuite) TxToInboundTracker(tx ton.Transaction) cc.InboundTracker { + if tx.Hash() == nil { + panic("invalid transaction: nil hash") + } return cc.InboundTracker{ ChainId: ts.chain.ChainId, TxHash: liteapi.TransactionToHashString(tx), CoinType: coin.CoinType_Gas, } }zetaclient/chains/ton/signer/signer_test.go (1)
90-97: Simplify test data to focus on the core functionality.The test messages contain unnecessary verbose logs and repeated patterns that don't contribute to testing the exit code extraction functionality.
Consider simplifying the test messages to focus on the essential patterns:
- `unable to send external message: error code: 0 message: - cannot apply external message to current state : - External message was not accepted\nCannot run message on account: inbound external message rejected by - transaction CC8803E21EDA7E6487D191380725A82CD75316E1C131496E1A5636751CE60347: - \nexitcode=109, steps=108, gas_used=0\nVM Log (truncated):\n...INT 0\nexecute THROWIFNOT - 105\nexecute MYADDR\nexecute XCHG s1,s4\nexecute SDEQ\nexecute THROWIF 112\nexecute OVER\nexecute - EQINT 0\nexecute THROWIF 106\nexecute GETGLOB - 3\nexecute NEQ\nexecute THROWIF 109\ndefault exception handler, terminating vm with exit code 109\n` + "transaction rejected: exitcode=109"zetaclient/chains/ton/observer/inbound.go (2)
32-34: Add documentation for the new methodThe
watchInboundTrackermethod lacks documentation explaining its purpose and behavior. Consider adding a godoc comment.+// watchInboundTracker periodically processes inbound trackers that might have been missed +// during normal observation. func (ob *Observer) watchInboundTracker(ctx context.Context) error {🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 32-33: zetaclient/chains/ton/observer/inbound.go#L32-L33
Added lines #L32 - L33 were not covered by tests
164-219: Consider adding circuit breaker patternFor the tracker processing functionality, consider implementing a circuit breaker pattern to prevent overwhelming the system during high load or error conditions. This could include:
- Rate limiting for tracker processing
- Backoff strategy for repeated errors
- Metrics for system health monitoring
Example implementation:
type CircuitBreaker struct { failures int maxFailures int resetAfter time.Duration lastFailure time.Time } func (cb *CircuitBreaker) AllowOperation() bool { if time.Since(cb.lastFailure) > cb.resetAfter { cb.failures = 0 return true } return cb.failures < cb.maxFailures }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 168-169: zetaclient/chains/ton/observer/inbound.go#L168-L169
Added lines #L168 - L169 were not covered by tests
[warning] 173-174: zetaclient/chains/ton/observer/inbound.go#L173-L174
Added lines #L173 - L174 were not covered by tests
[warning] 184-185: zetaclient/chains/ton/observer/inbound.go#L184-L185
Added lines #L184 - L185 were not covered by tests
[warning] 190-191: zetaclient/chains/ton/observer/inbound.go#L190-L191
Added lines #L190 - L191 were not covered by tests
[warning] 197-206: zetaclient/chains/ton/observer/inbound.go#L197-L206
Added lines #L197 - L206 were not covered by tests
[warning] 213-214: zetaclient/chains/ton/observer/inbound.go#L213-L214
Added lines #L213 - L214 were not covered by testse2e/runner/zeta.go (2)
105-113: Consider adding documentation for the private method.The implementation is clean and thread-safe. Consider adding a documentation comment to explain the purpose of the status parameter and return value.
Add this documentation:
// waitForMinedCCTXFromIndex waits for a CCTX with the given index to reach the specified status. // Returns the CCTX if found and status matches, panics otherwise.
Line range hint
114-144: Consider enhancing timeout and pagination handling.The implementation could be improved in several ways:
- Use
context.WithTimeoutfor cleaner timeout handling- Make the page size configurable
- Return an error instead of calling
FailNow()Consider this refactoring:
func (r *E2ERunner) WaitForSpecificCCTX( filter func(*types.CrossChainTx) bool, status types.CctxStatus, timeout time.Duration, -) *types.CrossChainTx { +) (*types.CrossChainTx, error) { var ( - ctx = r.Ctx + ctx, cancel = context.WithTimeout(r.Ctx, timeout) start = time.Now() reqQuery = &types.QueryAllCctxRequest{ - Pagination: &query.PageRequest{Reverse: true}, + Pagination: &query.PageRequest{ + Reverse: true, + Limit: 100, // Make this configurable + }, } ) + defer cancel() - for time.Since(start) < timeout { + for { + select { + case <-ctx.Done(): + return nil, fmt.Errorf("WaitForSpecificCCTX: timed out after %v", timeout) + default: res, err := r.CctxClient.CctxAll(ctx, reqQuery) - require.NoError(r, err) + if err != nil { + return nil, fmt.Errorf("WaitForSpecificCCTX: failed to query CCTXs: %w", err) + } for i := range res.CrossChainTx { tx := res.CrossChainTx[i] if filter(tx) { - return r.waitForMinedCCTXFromIndex(tx.Index, status) + return r.waitForMinedCCTXFromIndex(tx.Index, status), nil } } time.Sleep(time.Second) + } } - - r.Logger.Error("WaitForSpecificCCTX: No CCTX found. Timed out") - r.FailNow() - - return nil }zetaclient/chains/ton/observer/inbound_test.go (1)
Line range hint
19-391: Consider enhancing test organization and documentationThe test cases are well-structured but could benefit from:
- Adding test case descriptions using
t.Helper()functions for common setup patterns- Documenting expected behavior in test case names using BDD-style naming
Example refactor for test helper:
func setupTestWithGateway(t *testing.T) (*testSuite, *Observer) { t.Helper() ts := newTestSuite(t) ob, err := New(ts.baseObserver, ts.liteClient, ts.gateway) require.NoError(t, err) return ts, ob }Example BDD-style test naming:
-t.Run("Deposit", func(t *testing.T) { +t.Run("When_ValidDeposit_Should_CreateCCTX", func(t *testing.T) {zetaclient/orchestrator/orchestrator.go (1)
759-764: Add test coverage for TON outbound task logging.The structured logging implementation for TON outbound tasks is well-designed and consistent with the codebase patterns. However, these changes lack test coverage.
Would you like me to help create unit tests for this logging functionality? The tests would verify:
- Logger initialization with correct fields
- Field values propagation
- Integration with the background worker
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 759-764: zetaclient/orchestrator/orchestrator.go#L759-L764
Added lines #L759 - L764 were not covered by testschangelog.md (1)
54-54: Consider expanding the changelog entry with more details.The current entry "ton: withdraw concurrent, deposit & revert." could be more descriptive to better reflect the scope of changes mentioned in the PR objectives, particularly the implementation of inbound trackers and specific test scenarios.
Consider expanding the entry to:
-* [3075](https://github.com/zeta-chain/node/pull/3075) - ton: withdraw concurrent, deposit & revert. +* [3075](https://github.com/zeta-chain/node/pull/3075) - ton: add E2E tests for concurrent withdrawals, deposit refunds, and inbound tracking.e2e/runner/ton.go (2)
Line range hint
130-153: Handle Errors Properly inSendWithdrawTONZRC20The function
SendWithdrawTONZRC20does not return an error, yet it can encounter errors during execution, such as failures in approving or withdrawing tokens. To ensure robust error handling and allow callers to react appropriately, consider modifying the function signature to return an error and propagate any encountered errors.Apply this diff to modify the function signature and error handling:
func (r *E2ERunner) SendWithdrawTONZRC20( to ton.AccountID, amount *big.Int, approveAmount *big.Int, -) *ethtypes.Transaction { +) (*ethtypes.Transaction, error) { // approve tx, err := r.TONZRC20.Approve(r.ZEVMAuth, r.TONZRC20Addr, approveAmount) + if err != nil { + return nil, err + } receipt := utils.MustWaitForTxReceipt(r.Ctx, r.ZEVMClient, tx, r.Logger, r.ReceiptTimeout) utils.RequireTxSuccessful(r, receipt, "approve") // withdraw tx, err = r.TONZRC20.Withdraw(r.ZEVMAuth, []byte(to.ToRaw()), amount) + if err != nil { + return nil, err + } r.Logger.EVMTransaction(*tx, "withdraw") // wait for tx receipt receipt = utils.MustWaitForTxReceipt(r.Ctx, r.ZEVMClient, tx, r.Logger, r.ReceiptTimeout) utils.RequireTxSuccessful(r, receipt, "withdraw") r.Logger.Info("Receipt txhash %s status %d", receipt.TxHash, receipt.Status) - return tx + return tx, nil }
Line range hint
135-153: Refactor Receipt Handling to Reduce Code DuplicationThe logic for waiting for transaction receipts and verifying transaction success is duplicated for both the approval and withdrawal transactions. Refactoring this logic into a helper method will enhance code readability and maintainability.
Introduce a helper method to handle receipt waiting and success verification:
func (r *E2ERunner) waitForTransaction(tx *ethtypes.Transaction, action string) error { receipt := utils.MustWaitForTxReceipt(r.Ctx, r.ZEVMClient, tx, r.Logger, r.ReceiptTimeout) if receipt.Status != ethtypes.ReceiptStatusSuccessful { return fmt.Errorf("%s transaction failed: %s", action, receipt.TxHash.Hex()) } r.Logger.Info("Receipt txhash %s status %d", receipt.TxHash.Hex(), receipt.Status) return nil }Update
SendWithdrawTONZRC20to use the helper method:// approve tx, err := r.TONZRC20.Approve(r.ZEVMAuth, r.TONZRC20Addr, approveAmount) if err != nil { return nil, err } -receipt := utils.MustWaitForTxReceipt(r.Ctx, r.ZEVMClient, tx, r.Logger, r.ReceiptTimeout) -utils.RequireTxSuccessful(r, receipt, "approve") +if err := r.waitForTransaction(tx, "approve"); err != nil { + return nil, err +} // withdraw tx, err = r.TONZRC20.Withdraw(r.ZEVMAuth, []byte(to.ToRaw()), amount) if err != nil { return nil, err } r.Logger.EVMTransaction(*tx, "withdraw") // wait for tx receipt -receipt = utils.MustWaitForTxReceipt(r.Ctx, r.ZEVMClient, tx, r.Logger, r.ReceiptTimeout) -utils.RequireTxSuccessful(r, receipt, "withdraw") -r.Logger.Info("Receipt txhash %s status %d", receipt.TxHash, receipt.Status) +if err := r.waitForTransaction(tx, "withdraw"); err != nil { + return nil, err +} return tx, nilzetaclient/chains/ton/signer/signer.go (1)
261-262: Handle Parsing Errors Explicitly inextractExitCodeWhen parsing the exit code fails (lines 261-262 and 266-267), the function silently returns
(0, false). Consider logging these parsing failures or handling them explicitly to aid in debugging if unexpected error formats are encountered.Also applies to: 266-267
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 261-262: zetaclient/chains/ton/signer/signer.go#L261-L262
Added lines #L261 - L262 were not covered by tests
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (18)
changelog.md(1 hunks)cmd/zetae2e/local/local.go(1 hunks)e2e/e2etests/e2etests.go(3 hunks)e2e/e2etests/test_ton_deposit_refund.go(1 hunks)e2e/e2etests/test_ton_withdrawal.go(2 hunks)e2e/e2etests/test_ton_withdrawal_concurrent.go(1 hunks)e2e/runner/setup_ton.go(2 hunks)e2e/runner/ton.go(6 hunks)e2e/runner/ton/deployer.go(1 hunks)e2e/runner/zeta.go(2 hunks)pkg/contracts/ton/gateway_msg.go(1 hunks)zetaclient/chains/ton/observer/inbound.go(5 hunks)zetaclient/chains/ton/observer/inbound_test.go(14 hunks)zetaclient/chains/ton/observer/observer.go(1 hunks)zetaclient/chains/ton/observer/observer_test.go(5 hunks)zetaclient/chains/ton/signer/signer.go(4 hunks)zetaclient/chains/ton/signer/signer_test.go(1 hunks)zetaclient/orchestrator/orchestrator.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (17)
cmd/zetae2e/local/local.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/e2etests/e2etests.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/e2etests/test_ton_deposit_refund.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/e2etests/test_ton_withdrawal.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/e2etests/test_ton_withdrawal_concurrent.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/runner/setup_ton.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/runner/ton.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/runner/ton/deployer.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/runner/zeta.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
pkg/contracts/ton/gateway_msg.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/ton/observer/inbound.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/ton/observer/inbound_test.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/ton/observer/observer.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/ton/observer/observer_test.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/ton/signer/signer.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/ton/signer/signer_test.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/orchestrator/orchestrator.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
🪛 GitHub Check: codecov/patch
zetaclient/chains/ton/observer/inbound.go
[warning] 29-29: zetaclient/chains/ton/observer/inbound.go#L29
Added line #L29 was not covered by tests
[warning] 32-33: zetaclient/chains/ton/observer/inbound.go#L32-L33
Added lines #L32 - L33 were not covered by tests
[warning] 36-36: zetaclient/chains/ton/observer/inbound.go#L36
Added line #L36 was not covered by tests
[warning] 42-43: zetaclient/chains/ton/observer/inbound.go#L42-L43
Added lines #L42 - L43 were not covered by tests
[warning] 47-47: zetaclient/chains/ton/observer/inbound.go#L47
Added line #L47 was not covered by tests
[warning] 51-52: zetaclient/chains/ton/observer/inbound.go#L51-L52
Added lines #L51 - L52 were not covered by tests
[warning] 66-66: zetaclient/chains/ton/observer/inbound.go#L66
Added line #L66 was not covered by tests
[warning] 94-94: zetaclient/chains/ton/observer/inbound.go#L94
Added line #L94 was not covered by tests
[warning] 96-96: zetaclient/chains/ton/observer/inbound.go#L96
Added line #L96 was not covered by tests
[warning] 98-98: zetaclient/chains/ton/observer/inbound.go#L98
Added line #L98 was not covered by tests
[warning] 168-169: zetaclient/chains/ton/observer/inbound.go#L168-L169
Added lines #L168 - L169 were not covered by tests
[warning] 173-174: zetaclient/chains/ton/observer/inbound.go#L173-L174
Added lines #L173 - L174 were not covered by tests
[warning] 184-185: zetaclient/chains/ton/observer/inbound.go#L184-L185
Added lines #L184 - L185 were not covered by tests
[warning] 190-191: zetaclient/chains/ton/observer/inbound.go#L190-L191
Added lines #L190 - L191 were not covered by tests
[warning] 197-206: zetaclient/chains/ton/observer/inbound.go#L197-L206
Added lines #L197 - L206 were not covered by tests
[warning] 213-214: zetaclient/chains/ton/observer/inbound.go#L213-L214
Added lines #L213 - L214 were not covered by tests
zetaclient/chains/ton/observer/observer.go
[warning] 89-93: zetaclient/chains/ton/observer/observer.go#L89-L93
Added lines #L89 - L93 were not covered by tests
[warning] 97-98: zetaclient/chains/ton/observer/observer.go#L97-L98
Added lines #L97 - L98 were not covered by tests
zetaclient/chains/ton/signer/signer.go
[warning] 92-93: zetaclient/chains/ton/signer/signer.go#L92-L93
Added lines #L92 - L93 were not covered by tests
[warning] 98-103: zetaclient/chains/ton/signer/signer.go#L98-L103
Added lines #L98 - L103 were not covered by tests
[warning] 161-161: zetaclient/chains/ton/signer/signer.go#L161
Added line #L161 was not covered by tests
[warning] 224-231: zetaclient/chains/ton/signer/signer.go#L224-L231
Added lines #L224 - L231 were not covered by tests
[warning] 233-237: zetaclient/chains/ton/signer/signer.go#L233-L237
Added lines #L233 - L237 were not covered by tests
[warning] 239-241: zetaclient/chains/ton/signer/signer.go#L239-L241
Added lines #L239 - L241 were not covered by tests
[warning] 244-254: zetaclient/chains/ton/signer/signer.go#L244-L254
Added lines #L244 - L254 were not covered by tests
[warning] 261-262: zetaclient/chains/ton/signer/signer.go#L261-L262
Added lines #L261 - L262 were not covered by tests
[warning] 266-267: zetaclient/chains/ton/signer/signer.go#L266-L267
Added lines #L266 - L267 were not covered by tests
zetaclient/orchestrator/orchestrator.go
[warning] 759-764: zetaclient/orchestrator/orchestrator.go#L759-L764
Added lines #L759 - L764 were not covered by tests
🔇 Additional comments (14)
e2e/e2etests/test_ton_withdrawal.go (1)
Line range hint 82-86: Verify transaction status in addition to amount.
The test verifies the withdrawal amount but should also verify the transaction status to ensure it was successful.
Let's verify if there are similar tests checking transaction status:
e2e/runner/setup_ton.go (4)
14-14: LGTM: Import addition is appropriate.
The new import is necessary for cross-chain transaction status verification.
66-68: LGTM: Proper error handling.
The error handling follows best practices by wrapping errors with context.
71-73: LGTM: Clean gateway initialization.
The gateway setup is clear and well-structured.
78-83: LGTM: Robust deposit status verification.
The switch statement properly verifies both the error condition and the transaction status, ensuring the deposit is fully processed before proceeding.
Let's verify the usage of CctxStatus_OutboundMined status:
✅ Verification successful
Consistent usage of CctxStatus_OutboundMined status verified
The codebase shows consistent usage of CctxStatus_OutboundMined status across multiple components:
- Core status definition in
x/crosschain/types/status.go - Consistent verification in deposit flows across different chains (ETH, BTC, TON, Solana)
- Proper status checks in test suites validating the behavior
- Uniform usage in orchestrator and gateway components
The implementation in setup_ton.go follows the established pattern used throughout the codebase for verifying successful cross-chain transactions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if CctxStatus_OutboundMined is consistently used for deposit verification
rg "CctxStatus_OutboundMined" --type go
Length of output: 11659
pkg/contracts/ton/gateway_msg.go (1)
27-34: Integration with signer component looks good.
The new ExitCode type is well-positioned for use by the signer component for error handling and logging. The implementation aligns with the PR objectives for enhancing TON operations.
zetaclient/chains/ton/observer/observer_test.go (2)
16-16: LGTM: Import addition is appropriate.
The new import is correctly placed and necessary for the coin type definition used in the inbound tracker implementation.
178-182: LGTM: Mock setup method follows established patterns.
The implementation follows the consistent pattern used in other mock setup methods in the test suite.
zetaclient/chains/ton/observer/inbound.go (2)
22-24: LGTM: Constants follow Go naming conventions
The constants are well-defined with clear purposes. The logSampleRate of 10 provides a good balance between logging visibility and volume reduction.
Line range hint 29-66: Add test coverage for new functionality
The static analysis indicates that the new methods lack test coverage. This is particularly important for blockchain operations where reliability is crucial. Consider adding tests for:
watchInboundTrackerinboundTickerwith different task functionsprocessInboundTrackerswith various error scenarios
Would you like me to help generate comprehensive test cases for these methods?
Also applies to: 164-219
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 29-29: zetaclient/chains/ton/observer/inbound.go#L29
Added line #L29 was not covered by tests
[warning] 32-33: zetaclient/chains/ton/observer/inbound.go#L32-L33
Added lines #L32 - L33 were not covered by tests
[warning] 36-36: zetaclient/chains/ton/observer/inbound.go#L36
Added line #L36 was not covered by tests
[warning] 42-43: zetaclient/chains/ton/observer/inbound.go#L42-L43
Added lines #L42 - L43 were not covered by tests
[warning] 47-47: zetaclient/chains/ton/observer/inbound.go#L47
Added line #L47 was not covered by tests
[warning] 51-52: zetaclient/chains/ton/observer/inbound.go#L51-L52
Added lines #L51 - L52 were not covered by tests
e2e/runner/zeta.go (1)
102-103: LGTM: Clean refactoring to improve code reuse.
The method maintains backward compatibility while delegating to a more flexible private implementation.
e2e/e2etests/e2etests.go (1)
68-72: LGTM: TON test name constants are well-defined.
The new test name constants follow the established naming pattern and are properly grouped under the TON tests section.
e2e/e2etests/test_ton_withdrawal_concurrent.go (1)
19-80: Test function is well-structured and effectively tests concurrent withdrawals
The implementation of TestTONWithdrawConcurrent is clear and correctly utilizes goroutines and a wait group to simulate concurrent withdrawals. The test cases are well-prepared, and assertions properly validate the expected outcomes.
zetaclient/chains/ton/signer/signer.go (1)
233-237:
Ensure Correct Error Unwrapping for LiteServerErrorC
The error unwrapping using errors.As (lines 233-237) may not always yield the expected LiteServerErrorC if the error wrapping changes. Verify that the error chain consistently uses LiteServerErrorC and consider adding checks or logging to handle unexpected cases.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 233-237: zetaclient/chains/ton/signer/signer.go#L233-L237
Added lines #L233 - L237 were not covered by tests
…/ton-enhancements # Conflicts: # changelog.md
# Conflicts: # changelog.md
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Closes #2935
Closes #2946
Closes #3044
Summary by CodeRabbit
Release Notes for Version 21.0.0
New Features
Bug Fixes
Testing Improvements
Documentation