[mukeshsp]

Summary

Ton.verifyTransaction compared only (address, amount) and stripped the ?memoId= suffix from recipient addresses, so a prebuilt transaction whose on-chain comment differed from the intended memoId verified successfully — enabling exchange-deposit redirection. This PR binds the parsed transfer comment to the declared memoId on both the tton WASM and mainnet TonWeb paths.

Linear: CSHLD-2160

Changes

• verifyTransaction now compares {address, amount, memo} per output on both paths; the expected memo is resolved from the ?memoId= address suffix and recipient.memo (agree-or-reject, empty = absent). A prebuild whose comment is different from, missing from, or extra to the declared memoId fails verification with Tx outputs does not match with expected txParams recipients.
• WASM explain (extractOutputs) and TonWeb explain (Transaction / TokenTransaction) expose the parsed comment as outputs[].memo — plain-send body comments and jetton forward comments. Protocol payloads (SingleNominator/Whales opcodes) are not exposed as memo.
• Fixed the TonWeb jetton payload parser to accept the empty forward-payload cell that TokenTransferBuilder always writes for comment-less transfers — previously every comment-less jetton prebuild failed parsing (and thus verification) with invalid raw transaction.
• Regenerated the memo fixtures so they genuinely carry the declared comment (the old signedSendTransactionForMemoId.tx was byte-identical to the comment-less fixture — it literally demonstrated the vulnerability); the old comment-carrying jetton bytes are preserved as signedTokenSendTransactionWithComment for negative tests.
• Added verify coverage for missing/different/extra comments on tton (WASM), the jetton token, and a new mainnet bitgo.coin('ton') (TonWeb) describe that previously had zero verify coverage.

Test Plan

• Local: tsx smoke exercising verifyTransaction on tton, ton, and tton:ukwny-us — positives plus all four negative classes (missing/different/extra comment, plain and token), explain memo exposure/absence, and SingleNominator/Whales no-memo-leak regressions: 21/21 pass.
• CI: yarn unit-test --scope @bitgo/sdk-coin-ton.

Behavior notes

• recipient.memo is now a hard declaration for TON. Verification treats it as the expected on-chain comment (together with the ?memoId= address suffix); callers that populated recipient.memo for any other purpose will now fail verification with Tx outputs does not match with expected txParams recipients.
• Protocol-text comments cannot verify as user memos (accepted limitation, consistent with the existing transaction classification on both paths): a plain-send comment that is exactly Deposit, Withdraw, or the single-nominator withdraw-all text is classified as a staking protocol comment and suppressed from outputs[].memo, so a transfer whose intended memo happens to equal one of those strings cannot pass verification. Pinned by test in test/unit/ton.ts.

[linear-code]

CSHLD-2160

[github-actions]

⚠️ Compatibility checks are failing on Node.js 27-nightly (experimental, non-blocking). This does not block merge, but it signals an incompatibility to fix before this release becomes supported.

View run

[mukeshsp]

@claude review

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

[mukeshsp]

@claude review and share verdict

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Still open from earlier reviews (1):

• Unresolved: 1 minor or pre-existing.

[mukeshsp]

Confirmed — you were right about the snake decoder. Verified against the spec and the canonical reader, and fixed in a25d22b:

Verification of your claim

• TL-B SnakeData (docs.ton.org, Token metadata → Snake data encoding): cons#_ {bn:#} {n:#} b:(bits bn) next:^(SnakeData ~n) — continuation cells carry raw bits, no length prefix; the reference flattenSnakeCell decoder reads whole cells and follows refs.
• @ton/core 0.63.1 dist/boc/utils/strings.js readBuffer (the reader behind readString, used by wallets/explorers that credit deposits): loads all remaining bits raw, throws on non-byte-aligned bits, throws on remainingRefs ∉ {0,1}, follows the single ref.

Fix

• loadComment now mirrors readBuffer exactly: per cell, byte-alignment check, at-most-one-ref check, whole-cell text, recursive single-ref follow. The 8-bit byte-count prefix is gone, along with its throw.
• Fail-closed is preserved — but at the points where the standard reader itself throws (previously a crafted [0x00] continuation could bind as "1234" while chain readers saw "1234\0"; that class of mismatch is now impossible).

Fallout you predicted, handled

• Both snake fixtures regenerated without the 0x04 length byte (plain + token); tests still assert full-text '12345678' explain, memoId=1234 reject, and undeclared-comment reject.
• The malformed-continuation fixture/test was replaced: the new fixture carries two continuation refs (standard reader throws there too), asserting explain rejects with Invalid transaction and verify rejects with invalid raw transaction.
• Whales: added a mainnet-path test — deposit explains fine through loadComment and does not expose its comment as memo (only TransactionType.Send does).
• jetton explain adds memo only when a comment exists; in-repo consumers were checked and the deep-equal tests updated. Comment-less token explanations are unchanged.
• yarn.lock is local-only, not in the PR diff.

All 18 smoke assertions pass through real bitgo.coin('ton') / bitgo.coin('ton:usdt') objects; module tsc --build clean.

[mukeshsp]

Follow-up coverage added in the latest push (1ce3061):

• Plain 'Deposit' op-0 classification (the actual loadComment branch you asked about): new fixture signedSendTransactionWithDepositComment + test asserting the comment is classified as a whales vesting deposit and not exposed as memo. The earlier whales test only exercised the dedicated TON_WHALES_DEPOSIT_OPCODE branch, so this closes the gap — explainTransaction exposes no type field, so memo suppression is the observable.
• WASM/tton path alignment: explainTonTransaction's extractOutputs was exposing op-0 protocol text comments (Deposit/Withdraw/single-nominator) as memo, while the TonWeb path classifies them as staking types and suppresses — a whales vesting deposit would have verified differently across the two paths. The WASM extractor now mirrors the TonWeb suppression (verified empirically: wasm parseTransaction surfaces memo: 'Deposit' with transactionType: 'Transfer').
• Builder message-encoding coverage restored: the two setMessage calls dropped from tokenTransferBuilder tests (they conflicted with the regenerated comment-less fixture byte-assertions) are replaced by a new test that builds with setMessage('jetton testing') and asserts the comment round-trips through the serialized forward payload via WASM explain.

Smoke through real coin objects: 'Deposit' suppressed on both paths, '1234' control exposed on both, builder roundtrip memo correct, comment-less token send still memo-less. Module tsc --build clean.

[mukeshsp]

Review verdict: approve (no remaining findings) at 3bacce9. Static review of the diff only; tests were not run locally, so CI (yarn unit-test --scope @bitgo/sdk-coin-ton) remains the gate.

• WASM extractOutputs now suppresses Deposit / Withdraw / single-nominator withdraw-all comments, matching the TonWeb type === Send gate, so both verify paths agree.
• The jetton forward comment is exposed consistently on both paths, and the custom_payload ref is now consumed so the forward payload is read from the right cell.
• The strict loadComment and the empty forward-payload handling fail closed on malformed cells.
• The setMessage round-trip test restores builder coverage, and the protocol-text limitation is pinned by a test and documented in the PR description.

Optional follow-up: add a tton (WASM) twin of the new "Deposit as declared memo" test. It currently only runs through mainnetCoin, though the WASM path uses the same guard.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}