Skip to content

fix(sdk-core): use explicit undefined check for mpcv2PartyId#8456

Merged
zahin-mohammad merged 2 commits intomasterfrom
fix/WAL-387-mpcv2-party-id-falsy-check
Apr 9, 2026
Merged

fix(sdk-core): use explicit undefined check for mpcv2PartyId#8456
zahin-mohammad merged 2 commits intomasterfrom
fix/WAL-387-mpcv2-party-id-falsy-check

Conversation

@zahin-mohammad
Copy link
Copy Markdown
Contributor

@zahin-mohammad zahin-mohammad commented Apr 9, 2026

Summary

  • Falsy check params.mpcv2PartyId ? ... : 0 treats party ID 0 as falsy. Replace with an explicit undefined check so the intent is correct and doesn't rely on the fallback coincidentally matching the desired default.

Why no unit test

  • signRequestBase is a private method with heavy dependencies (DklsDsg.Dsg, openpgp, DklsComms, BitGo API, wallet/coin interfaces) and zero existing test infrastructure — the mocking effort far outweighs the value for a one-line fix.
  • The bug is coincidentally harmless: the falsy fallback (0) matches the actual desired default, so the old code produced correct results in all cases. The fix makes the intent correct rather than relying on that coincidence.

Test plan

  • Existing unit tests pass (yarn unit-test-changed)
  • Code review confirms the !== undefined check is correct

Ticket: WAL-387

@linear
Copy link
Copy Markdown

linear bot commented Apr 9, 2026

Falsy check `params.mpcv2PartyId ? ... : 0` treats party ID 0 as
falsy, causing `getSignatureShareRoundOne` to receive `undefined`
while the `Dsg` constructor receives `0` when `mpcv2PartyId` is
not supplied. Replace with an explicit undefined check to ensure
both callers receive the same value.

Ticket: WAL-387
@zahin-mohammad zahin-mohammad force-pushed the fix/WAL-387-mpcv2-party-id-falsy-check branch from 6fef5f8 to c2c3a79 Compare April 9, 2026 11:39
@zahin-mohammad zahin-mohammad marked this pull request as ready for review April 9, 2026 12:16
@zahin-mohammad zahin-mohammad requested review from a team as code owners April 9, 2026 12:16
@zahin-mohammad zahin-mohammad merged commit 0190ec1 into master Apr 9, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants