[nvjsr]

Problem

Approving a message-sign pending approval (WalletConnect personal_sign / EIP-712 eth_signTypedData_v4) crashes the SDK with:

TypeError: Cannot read properties of undefined (reading 'unsignedTx')

The crash is invisible to the PA approval itself — the server-side PUT succeeds (the txRequest moves to pendingDelivery), but the SDK's postApprove() unconditionally runs the TSS recreate-and-sign flow for TRANSACTION_REQUEST_FULL PAs, which reads txRequest.transactions[0].unsignedTx.serializedTxHex via recreateAndSignTSSTransaction → tssUtils.recreateTxRequest → signTxRequest. Message-only txRequests have an empty transactions array (truthy, so the assert passes), making transactions[0] undefined and the .unsignedTx read a TypeError.

Users see a scary "contact support@bitgo.com" error on a successful approval, then have to re-sign from the request's activity detail (which works because the sign path uses signAndSendMessageTxRequest — the retail fix in bitgo-retail #10649).

Reproduced in prod by emerick (Oct 27) on the retail Tasks page.

Root cause

PendingApproval.postApprove() had no check for message-only requests:

} else if (this.canRecreateTransaction(params) && this.baseCoin.supportsTss()) {
  await this.recreateAndSignTSSTransaction(params, reqId);
}

The TSS signer's ecdsa.ts assert checks truthiness (txRequest.transactions || txRequest.unsignedTxs) — an empty array is truthy, so transactions[0] is undefined.

Fix

1. postApprove() guard — new isMessageSigningRequest() helper reads the PA's echoed info.transactionRequestFull.intent.intentType and returns early for signMessage / signTypedStructuredData, skipping the auto recreate-and-sign. The message is signed separately via signAndSendMessageTxRequest once the approval resolves.
2. Hardened asserts — 3 ecdsa signTxRequest sites changed from truthiness to length > 0, turning any future empty-array crash into a clear assertion error.
3. Typed the PA info — PendingApprovalInfo now declares transactionRequestFull with intent, so the guard compiles without casts.

Testing

• 3 new unit tests in modules/bitgo/test/v2/unit/pendingApproval.ts:
  • signMessage intent → auto-sign skipped ✅
  • signTypedStructuredData intent → auto-sign skipped ✅
  • payment intent → auto-sign still runs (no false positive) ✅
• Full PA suite: 19 passing, 3 pre-existing failures (identical baseline)
• TSS/ecdsa suite: 55 passing, 5 pre-existing (identical baseline)
• tsc --build clean

After merge

Retail bumps its @bitgo-beta/sdk-core pin — no retail code change needed (the ApprovalApproveForm / useUpdatePendingApprovalMutation path is correct; the fix is entirely SDK-side).

Related

• bitgo-retail #10649 (merged) — fixed the sign path but not the approve path
• DEFI-1015 — the umbrella ticket for the WC message-sign PA flow
• DEFI-653 — message-sign policy (DEFI-1014 SDKs)

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

[linear-code]

DEFI-961

DEFI-1246

[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

[nvjsr]

@claude

[github-actions]

Review

The fix is targeted and sensible. Skipping the auto recreate-and-sign for message-only approvals matches the root cause. The assert hardening turns a TypeError into a clear error. The tests cover both message intents and the payment negative case.

Must fix: internal information in comments

• iPendingApproval.ts JSDoc: "echoed by wallet-platform (with finalizeUnwrap secrets stripped)" names an internal system and describes server internals. Suggested rewrite:

  Present on transactionRequestFull pending approvals. intent.intentType is 'signMessage' or 'signTypedStructuredData' for message-sign approvals, which have no unsigned transaction.

• pendingApproval.ts guard comment: "WalletConnect personal_sign / EIP-712 ... crashes on them" mixes in bug history. Suggested rewrite:

  Message-sign approvals have no unsigned transaction, so there is nothing to recreate and sign. The message is signed separately via signAndSendMessageTxRequest.

• The isMessageSigningRequest JSDoc ("callers that assume a transaction (recreate-and-sign, txHex reads) must skip them") describes callers rather than behavior. Trim it to "True when the approval's intent is a message-sign intent."
• The test comment "without the message-sign guard the auto-sign below would run" is change-rationale. Consider dropping it.
• The PR description mentions internal tickets (DEFI-1015, DEFI-653, DEFI-1014), a named person and an internal repo. None of that should reach commit messages or code. Note that commitlint requires a footer reference.

Suggestions

1. Inconsistent typing in the first assert (ecdsa.ts, ~L593). It writes (txRequest as TxRequest).unsignedTxs && txRequest.unsignedTxs.length, so the cast is used once and then dropped. The other two sites use no cast. Make them consistent. Consider extracting a small helper, since the same assert is now repeated 3 times.
2. Hardcoded intent strings. 'signMessage' and 'signTypedStructuredData' are matched as literals and typed as string. If an intent-type enum or constant already exists in sdk-core, reuse it so the two can't drift.
3. Silent early return. postApprove returns with no signal that signing is deferred. Document this in the approve() docs, because callers who relied on auto-sign will see neither a change in return value nor an error.
4. Scope of the guard. It sits only in the canRecreateTransaction && supportsTss() branch. Confirm the earlier branches of this postApprove case can't be reached by message-sign PAs. If they can, put the guard before the whole chain.
5. Detection relies on the optional info.transactionRequestFull.intent. If the server omits intent, the old crash path returns, though the stricter asserts now give a clearer error. Consider also treating an empty transactions array as a second signal.
6. Tests.
   • The hardened asserts in ecdsa.ts change behavior but have no new coverage. Add a test that transactions: [] throws Unable to find transactions in txRequest.
   • The PR notes "3 pre-existing failures" in the PA suite. Please confirm they're unrelated and don't mask these tests in CI.

Good practices

• Minimal change, with optional typing that doesn't break consumers.
• Defense in depth: fixing the cause and also making the failure mode explicit.
• A negative test (payment) guards against false positives.

No security or cryptographic concerns: no change to signing inputs, key handling or derivation.