[dpkjnr]

What

A narrower alternative to #10134 that hardens verifyTssTransaction only, assuming the caller passes correct txParams. Signing routing, sdk-core plumbing, the legacy EvmCoin verifier, FLR and statics are untouched.

Payments (transfer, and stored payment treated the same way)

• Single native: destination and value match, and calldata is empty.
• Single ERC-20: a canonical transfer(address,uint256) whose recipient and amount match, with zero native value, sent to the declared token contract (resolved from statics; a token coin declares itself).
• Explicit calldata (dApp / WalletConnect): to, value and data must match exactly. Nonzero native value is allowed.
• Native batch: sent to the network batcher, with batch(address[],uint256[]) arrays matching the intent and value equal to their sum.
• Token batch: batchTransferFrom(address,address[],uint256[]) with matching arrays, zero value and the declared token contract. Top-level tokenName is not required.

Consolidation (verification.consolidationToBaseAddress): the existing base-address checks stay. An ERC-20 sweep must also be canonical, carry zero native value and call the declared token contract; a native sweep must carry no calldata. Sweep amounts are not compared.

Warnings only: a token that statics can't resolve, a network with no batcher configured, ERC-20 calldata where no token is declared (recipient, amount and value are still enforced), ERC-7984 payments, and intent types with no verifier (including contractCall / customTx). A warning never skips the other checks.

Deliberately not in this PR

• Rejecting contractCall / customTx: existing upstream tests send customTx through this verifier and expect it to pass.
• Legacy / type-0 signing-preimage routing, binding serialized bytes to signableHex on ECDSA, the EvmCoin legacy bypass, FLR atomic handling, and the Katana batcher address in statics.
• Plumbing that separates the stored intent from caller params.

Test changes

• New: sdk-coin-eth/test/unit/tssPaymentVerification.ts, which builds real hteth bytes and covers matches and definite mismatches for every rule above. It also checks that a known payment with the wrong function fails, that a missing batcher does not hide wrong batch amounts, and that explicit calldata may carry native value.
• sdk-coin-xdc/test/unit/xdcToken.ts: the fixture used mainnet USDC's contract for txdc:tmt, so it now uses the token's contract from statics.
• sdk-coin-bsc/test/unit/bsc.ts: a tampered WalletConnect payload is now rejected by the exact-calldata check, so the expected message changed.

Verification

• Node 24.22.1 unit suites: abstract-eth 188, sdk-coin-eth 549 (33 new), sdk-coin-evm 40, sdk-coin-bsc 43, sdk-coin-xdc 65, sdk-coin-polygon 71, sdk-coin-flr 61, sdk-coin-arbeth 56, all passing.
• No SDK replay against production data was run for this branch. Exact explicit-calldata matching relies on the earlier field-level SQL validation, which found no mismatches.

Ticket: CECHO-2460

🤖 Generated with Claude Code

[linear-code]

CECHO-2460

[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

[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.}

[dpkjnr]

@claude review

[github-actions]

Claude finished @dpkjnr's task in 29s —— View job

---

Review of #10136: harden EVM TSS payment and consolidation checks

I read the abstractEthLikeNewCoins.ts diff in full. I did not read the new test file, and I did not run the test suites. The approach is sound and the scope matches the PR description. The canonical-ABI re-encode check is a good way to catch trailing bytes, dirty padding and odd offsets. I found no blocking bugs, only the items below.

Should fix

1. Consolidation warning prints "undefined". In the consolidation ERC-20 branch (abstractEthLikeNewCoins.ts ~L4030), resolveTssToken(txParams.tokenName) returns { isToken: false } with no reason when no token is declared and the coin is a base coin. The warning then reads undefined: the consolidation token contract was not verified. The single-payment and batch paths already fall back with token.reason ?? 'payment declares no token'. Use the same fallback here, for example token.reason ?? 'consolidation declares no token'.

2. A consolidation with ERC-20 calldata passes without a contract check if no token is declared. The base-address and canonical checks still apply, but to can be any contract. This is the same trade-off you accept for payments. It is worth a test that pins the behavior (a warning, not a pass or a throw) so a later change doesn't alter it unnoticed.

Worth a look

3. Unparseable amounts throw a raw error. In verifyTssBatchPayment (the reduce over recipients), BigInt(String(recipient.amount)) throws a bare SyntaxError for a malformed amount such as "1.5" or "". Every other mismatch goes through throwRecipientMismatch. tssValuesEqual already wraps this in try/catch, so a small helper that returns a mismatch on a parse failure would keep the error behavior consistent.

4. A token batch with a native recipient is not rejected. In the token-batch path, a recipient with isToken: false just produces the "declares no token" warning. A mixed batch is therefore not flagged. If a batch moves exactly one asset, as the comment says, consider rejecting recipients that resolve to the native asset inside a batchTransferFrom.

5. Warning noise. warnTssVerificationIncomplete now fires for every non-dedicated intent type, and for transfer with no recipients, on every verify call. That is fine for rollout visibility, but console.warn per transaction can be noisy in the express server. Consider routing it through whatever logger the module uses elsewhere.

Notes and nits

• The explicit-calldata branch skips the token and contract checks and allows native value. The PR description states this. The safety rests on the caller's txParams being correct, and the PR defers stored-intent separation.
• The BSC and XDC test edits make sense: the fixture used the wrong token contract, and the changed error message comes from the stricter check.
• The PR says no replay against production data was run. For a change that now rejects transactions it used to accept (native consolidation with calldata, non-canonical ERC-20 encodings, payment now verified like transfer), a replay or a staged rollout would give more confidence than the SQL validation of explicit calldata alone.

[claude]

🔴 Approvers of ERC-7984 confidential sends from a base-coin EVM TSS wallet now get a signing rejection when they approve through the SDK. On the base branch these sign. On that path the stored payment intent carries no token name, so resolveTssToken returns isToken: false. The confidentialTransfer calldata then fails at :3699 with 'payment calldata is not a native send or an ERC-20 transfer(address,uint256)'. Fix: when no token is declared, look up to among this network's ERC-7984 contracts (getErc7984WrappersForNetwork(network).addresses) and only warn on a hit, the same as a declared confidential token.

Why this was flagged

An approver calls pendingApproval.approve(). That runs recreateTxRequest (baseTSSUtils.ts:628-632), which calls signTxRequest with no txParams. resolveEffectiveTxParams (recipientUtils.ts:131-163) then sets type to intent.intentType, which is 'payment' because wallet.ts:4559 stores every 'transfer' that way. The ECDSA callers (ecdsaMPCv2.ts:994/1004, ecdsa.ts:835/842) pass no chainName, so the amount.symbol fallback is skipped. A plain sendMany token recipient gets only amount.symbol (mpcUtils.ts:251), so tokenName ends up undefined. The diff now maps 'payment' to 'transfer' (:3863), and resolveTssToken(undefined) on a base coin returns { isToken: false } (:3629). The confidential check at :3695 is skipped, so the WP-built confidentialTransfer calldata is rejected at :3699. On the base branch only 'transfer' was checked, so this path signed.

Verification: wallet.ts:4559-4562 maps a transfer to intentType: 'payment'. recipientUtils.ts:138-140 takes tokenName only from tokenData?.tokenName. token.confidential is falsy, so line 3699 throws TxIntentMismatchRecipientError. On the base branch the old check was ['transfer'].includes(txParams.type), so these approvals signed.