Repository navigation
feat(sdk-core): forward WalletConnect message origin - #10133
Conversation
|
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
nvjsr
left a comment
nvjsr
left a comment
There was a problem hiding this comment.
Reviewed end to end.
- Marker forwarding: all four message/typed-data intent builders (buildSignMessageRequest → /msgrequests, createTxRequestWithIntentForTypedDataSigning → /txrequests, the deprecated message-signing builder) forward isWalletConnectTx; conditional spread omits the key when unset, so the wire body stays absent-or-true — matches the WP codec contract (t.literal(true)).
- Typing: isWalletConnectTx?: true (literal) on IntentOptionsForMessage / IntentOptionsForTypedData / both Populated intents / WalletSignMessageBase — prevents false at compile time, consistent with DEFI-1013's send marker.
- Resume path: when signing resumes by txRequestId the marker is not re-sent and the existing request keeps its original classification — asserted by the resume tests.
- No txChannel key is ever constructed client-side (hasOwnProperty assertions), and tests cover the full marker/absent × msgrequests/txrequests/typed-data matrix plus wallet-level teth/tsol flows.
Verified against the WP side (bitgo-microservices#64175): /msgrequests MessageIntent and the /txrequests CreateTransactionRequestIntentCodec both declare t.literal(true), so every shape this SDK emits is accepted server-side.
Ticket: DEFI-1069
ddee01e
|
Conflict with master resolved (merge commit ddee01e — CHANGELOG: Unreleased block kept above the newly-released 40.5.1 section; all marker code intact, diff vs master purely additive). Push moved the head, so reviews went stale and HITL re-evaluates from zero: the merge commit is unsigned (GPG card unavailable), which requires 2 CODEOWNERS approvals on the new head. @pranishnepal @pritam-gembali — could you re-approve? Ticket: DEFI-1069 |
ddee01e to
30761b2
Compare
|
Master moved twice since your reviews (40.5.1 and 40.6.0 releases), so the PR needed a re-merge — head is now a78c526, conflict-free and all commits GPG-signed. HITL needs just 1 CODEOWNERS approval on the new head (signer excluded). @pranishnepal @pritam-gembali — one re-approval unblocks merge. Code changes vs the head you approved are unchanged; only the CHANGELOG note moved into the 40.6.0 Features section. Ticket: DEFI-1069 |
Allow WalletConnect message and typed-data signing requests to carry
isWalletConnectTx: true, forwarded on the signMessage (POST
/wallet/{id}/msgrequests) and signTypedData (POST /wallet/{id}/txrequests)
intent bodies so wallet-platform can classify the message txRequest's
txChannel (DEFI-1069). Literal-true typing on all four intent option and
populated-intent types; conditional spread omits the key when unset so the
wire body stays absent-or-true, matching the wallet-platform codec
(t.literal(true)). Resuming an existing request by txRequestId keeps its
original classification. Observability metadata only — never policy,
approvals, or signing.
Ticket: DEFI-1069
a78c526 to
4ebad0e
Compare
Description
Forward the optional
isWalletConnectTxmarker through message and typed-data signing intents, including both message request routes. It is metadata, not an authorization signal.WalletConnect message requests need this marker so wallet-platform can classify their origin and display security-check details. Deployment also requires wallet-platform support and a newly published SDK consumed by the clients.
Issue Number
DEFI-1069
Type of change
How Has This Been Tested?
Checklist:
Ticket: DEFI-1069