Uh oh!
There was an error while loading. Please reload this page.
fix(sdk-core): wire resolveEffectiveTxParams into EddsaMPCv2Utils (WCI-1111) - #9394
Conversation
4807ac9 to
394740dCompare7e84969 to
1da40aeCompare1da40ae to
eaae085Compareeaae085 to
ab7b2e9Compareab7b2e9 to
63e90fcCompare63e90fc to
691bdbfCompare691bdbf to
ce54229Comparece54229 to
0490122Compare0490122 to
e2ca6c4Comparee2ca6c4 to
f4c6b60Comparef4c6b60 to
0c44ba0Compare0c44ba0 to
9a3286bCompare9a3286b to
9d6b87cCompare0db73c7 to
71040eaCompare71040ea to
3c040fcCompare9ba6e8d to
3b026ebCompare3b026eb to
18f65ddCompare18f65dd to
cff0118Compare4b4f135 to
a70696aComparea70696a to
9cb52b1Compare7183351 to
7322e7dCompare7322e7d to
8e11038CompareReplace the unsafe `params.txParams || { recipients: [] }` fallback in
`EddsaMPCv2Utils.signRequestBase` with `resolveEffectiveTxParams`, which
throws `InvalidTransactionError` when recipients cannot be resolved and
the intent is not a known no-recipient type. This is the remediation for
Trail of Bits finding TOB-BITGOEDMPC-1 (WCI-1100): the EdDSA MPCv2
re-sign path previously substituted an empty-recipients object, allowing
`verifyTransaction` to be bypassed on coins that only validate when
recipients are non-empty (SOL, VET, Tempo, TRON).
ECDSA already used this pattern (ecdsaMPCv2.ts:958,965); this ports it
to EdDSA MPCv2. MPCv1 (eddsa.ts) is explicitly out of scope.
Also removes the now-redundant EdDSA MPCv2 pre-fetch block from
`wallet.ts:signTransactionTss` and the matching pre-derivation in
`BaseTssUtils.recreateTxRequest`, along with the `txParamsFromIntent`
helper function they both called. `resolveEffectiveTxParams` inside
`signRequestBase` now owns intent-based recipient derivation, so the
wallet layer no longer needs to pre-derive before calling `signTxRequest`.
Ticket: WCI-1111
Session-Id: 1c178dac-6528-4ee7-937d-974216871d68
Task-Id: e91df1ba-6cf4-4b0c-8df2-2588f555481e8e11038 to
1ecc1f5Comparef524fa0 to
85b5bb8Compare85b5bb8 to
ddc5199CompareSOL authorize stake account operations (intentType 'authorize') have no explicit transfer recipient. Without this entry, resolveEffectiveTxParams would throw InvalidTransactionError when signing an authorize transaction with no buildParams. Ticket: WCI-1111 Session-Id: 1c178dac-6528-4ee7-937d-974216871d68 Task-Id: e91df1ba-6cf4-4b0c-8df2-2588f555481e
ddc5199 to
fba4bbdCompareThere was a problem hiding this comment.
Requesting changes specifically on the bare 'authorize' allowlist entry in NO_RECIPIENT_TX_TYPES.
The EdDSA resolveEffectiveTxParams wiring looks correct for WCI-1111.
Marzooqa
commented
Aug 4, 2026
Removing |
abhijit0943
left a comment
There was a problem hiding this comment.
Discussed offline - Approving.
Agree with keeping authorize on the no-recipient allowlist for now so we don't break that signing path in this PR. That matches current behavior; proper intent-field validation for authorize belongs in a follow-up (coin-layerverifyTransaction), as noted in WCI-1110 / discussion here.
Please open a follow-up ticket for that and link it on this PR before merge if possible. The EdDSA resolveEffectiveTxParams wiring LGTM for WCI-1111.
Ticket -> WCI-1243 |
Uh oh!
There was an error while loading. Please reload this page.
What
signRequestBase: replaced the vulnerableparams.txParams || { recipients: [] }fallback withresolveEffectiveTxParams(txRequest, params.txParams, this.baseCoin.getChain()). The function throwsInvalidTransactionErrorwhen recipients cannot be resolved and the intent is not a recognised no-recipient type.signTransactionTss: removed the EdDSA MPCv2 special-case block that pre-fetched the txRequest and calledtxParamsFromIntentbefore handing off tosignTxRequest. That pre-fetch was introduced to work around the missing guard; now thatresolveEffectiveTxParamsowns intent-based derivation insidesignRequestBase(which already fetches the txRequest internally when given a string ID), the wallet-layer duplication is redundant. Removed the now-unusedtxParamsFromIntentimport.resolveEffectiveTxParamsguard suite tosignTxRequest.tscovering:stakingAuthorizeattack vector (no txParams, non-allowlisted intent → throws)txParamswith non-allowlisted intent → throwsdeactivateintentType with no txParams → succeedsconsolidateintentType with no txParams → succeedsstakingRequestIdsignal → succeedsWhy
Trail of Bits finding TOB-BITGOEDMPC-1 (WCI-1100): the EdDSA MPCv2 re-sign path silently substituted an empty-recipients object when
txParamswas absent. Several coin-levelverifyTransactionimplementations (SOL, VET, Tempo, TRON) skip output-matching validation whenrecipients.length === 0, allowing a compromised BitGo server to present a malicioustxHexthat signs without any client-side validation.ECDSA already used
resolveEffectiveTxParamsfor fail-closed behaviour (ecdsaMPCv2.ts:958,965andecdsa.ts:821,828). This PR ports the same pattern to EdDSA MPCv2. MPCv1 (eddsa.ts) is explicitly out of scope per the ticket.Depends on WCI-1110 (base branch
wci-1110-fix-resolve-effective-tx-params-sol), which fixesresolveEffectiveTxParamsto preservetokenNamefor SOL token transfers and adds thedeactivate/delegate/closeAssociatedTokenAccountallowlist entries needed by EdDSA coins.Test plan
signTxRequestthrowsInvalidTransactionErrorforstakingAuthorizeintent with no recipients and no txParamssignTxRequestthrows for emptytxParams.recipientswith non-allowlisted intentsignTxRequestsucceeds fordeactivateandconsolidate(allowlisted no-recipient intents)signTxRequestsucceeds when intent provides recipients and txParams is absentsignTxRequestsucceeds for staking intents withstakingRequestIdTicket: WCI-1111