Skip to content

feat(sdk-core, express): consume userKeySigningRequired from coinSpecific#8712

Merged
zahin-mohammad merged 2 commits intomasterfrom
zahinmohammad/wcn-471-refactorsdk-consume-userkeysigningrequired-from
May 7, 2026
Merged

feat(sdk-core, express): consume userKeySigningRequired from coinSpecific#8712
zahin-mohammad merged 2 commits intomasterfrom
zahinmohammad/wcn-471-refactorsdk-consume-userkeysigningrequired-from

Conversation

@zahin-mohammad
Copy link
Copy Markdown
Contributor

@zahin-mohammad zahin-mohammad commented May 7, 2026

Summary

Phase 2 of the userKeySigningRequired location refactor (parent: WCN-460). The trading-account BitGo-key signing gate in BitGoJS now reads walletData.coinSpecific.ofc.userKeySigningRequired, falling back to the top-level userKeySigningRequired only when the coinSpecific subdocument is absent. The top-level field is marked @deprecated and will be removed in a follow-up major release (Phase 3).

The express WalletResponse codec gains typed visibility for coinSpecific.ofc.userKeySigningRequired via t.intersection([t.partial({ ofc: ... }), t.UnknownRecord]). The intersection preserves the existing permissive t.UnknownRecord decode for wallets carrying other coinSpecific subdocuments (eth, terc20, etc.) — silently rejecting previously-decoding wallet responses would itself be an AP-0002 breaking change.

  • feat(sdk-core): extends WalletCoinSpecific with ofc?: { userKeySigningRequired?: boolean }, shifts the read in signPayloadByBitGoKey(), marks WalletData.userKeySigningRequired deprecated.
  • feat(express): types coinSpecific.ofc on the WalletResponse codec.

Issue Number

WCN-471https://linear.app/bitgo/issue/WCN-471

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

sdk-core (TradingAccount suite — 16 passing, +2 new cases):

  • existing case (coinSpecific.ofc.userKeySigningRequired: true throws) — renamed and re-asserted
  • new: top-level userKeySigningRequired: true still throws when coinSpecific.ofc is absent (fallback)
  • new: coinSpecific.ofc takes precedence over top-level when both are present

express (io-ts decode tests suite — 23 passing, +4 new cases):

  • decodes wallets with only non-ofc coinSpecific subdocuments (e.g. coinSpecific.eth)
  • decodes wallets with coinSpecific.ofc.userKeySigningRequired: true
  • decodes wallets with empty coinSpecific
  • rejects coinSpecific.ofc.userKeySigningRequired of wrong type
yarn workspace @bitgo/sdk-core run unit-test --grep "TradingAccount"
yarn workspace @bitgo/express run unit-test --grep "io-ts decode tests"
yarn workspace @bitgo/sdk-core run lint
yarn workspace @bitgo/express run lint

Phase 3 (out of scope, follow-up PR)

Removal of the top-level fallback and the WalletData.userKeySigningRequired field is an AP-0002 breaking change, however no breaking change release will made as this is a feature still in development.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My code compiles correctly (sdk-core, express tsc clean)
  • My commits follow Conventional Commits and the ticket is referenced
  • I have added tests covering both the new coinSpecific shape and the top-level fallback
  • New and existing unit tests pass locally with my changes

🤖 Generated with Claude Code

@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 7, 2026

WCN-471

Comment thread modules/express/src/typedRoutes/schemas/wallet.ts Outdated
Comment thread modules/sdk-core/src/bitgo/wallet/iWallet.ts Outdated
Comment thread modules/sdk-core/src/bitgo/wallet/iWallet.ts Outdated
Comment thread modules/sdk-core/test/unit/bitgo/trading/tradingAccount.ts Outdated
@zahin-mohammad zahin-mohammad requested a deployment to breaking-changes-override May 7, 2026 16:34 — with GitHub Actions Waiting
zahin-mohammad and others added 2 commits May 7, 2026 12:35
Shift the trading-account BitGo-key signing gate to read
`coinSpecific.userKeySigningRequired` and fall back to the top-level
`userKeySigningRequired` only when the coinSpecific subdocument does not
carry it. The OFC subdocument's toJSON in bitgo-microservices flattens its
fields directly into `coinSpecific`, so the field surfaces in the API
response as `coinSpecific.userKeySigningRequired`. The top-level field is
now `@deprecated` and will be removed in a follow-up major release. Tests
cover both the coinSpecific shape and the top-level fallback, plus the
precedence rule when both are present.

Ticket: WCN-471

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…onse codec

Narrow the WalletResponse `coinSpecific` codec from a bare `t.UnknownRecord`
to an intersection that adds typed visibility for
`coinSpecific.userKeySigningRequired` while keeping the unknown-record
permissiveness intact for unrelated subdocument fields. The OFC subdocument
toJSON in bitgo-microservices flattens its fields directly into coinSpecific,
so the field surfaces unwrapped on the response. Add codec-decode tests
covering wallets with arbitrary coinSpecific fields, the typed
userKeySigningRequired shape, an empty coinSpecific, and rejection of
wrong-typed values.

Ticket: WCN-471

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zahin-mohammad zahin-mohammad force-pushed the zahinmohammad/wcn-471-refactorsdk-consume-userkeysigningrequired-from branch from c9b247d to 0d223d5 Compare May 7, 2026 16:36
@zahin-mohammad
Copy link
Copy Markdown
Contributor Author

Addressed all four review comments — pushed amended commits in place of the originals.

Confirmed in bitgo-microservices/packages/wallet-platform/app/models/v2/coins/ofc/ofc.schema.ts (defineSubDocumentToJson lines 48–55) that the OFC subdocument's toJSON flattens its fields directly under coinSpecific — there is no ofc wrapper key in the API response. The original Linear ticket description was wrong about the shape.

Updated:

  • WalletCoinSpecific.userKeySigningRequired?: boolean (no nested ofc)
  • Gate read: walletData.coinSpecific?.userKeySigningRequired ?? walletData.userKeySigningRequired
  • Express codec: t.intersection([t.partial({ userKeySigningRequired: t.boolean }), t.UnknownRecord])
  • Deprecation JSDoc points to coinSpecific.userKeySigningRequired
  • Test mocks use flat shape (coinSpecific: {} / coinSpecific: { userKeySigningRequired: true })
  • Decode tests now exercise wallets with arbitrary flat coinSpecific fields

All tests/lint pass. PR description updated implicitly via the new commit messages.

@zahin-mohammad zahin-mohammad changed the title feat(sdk-core, express): consume userKeySigningRequired from coinSpecific.ofc feat(sdk-core, express): consume userKeySigningRequired from coinSpecific May 7, 2026
@zahin-mohammad zahin-mohammad requested a review from alextse-bg May 7, 2026 16:43
@zahin-mohammad zahin-mohammad marked this pull request as ready for review May 7, 2026 16:43
@zahin-mohammad zahin-mohammad requested review from a team as code owners May 7, 2026 16:43
@zahin-mohammad zahin-mohammad deployed to breaking-changes-override May 7, 2026 16:44 — with GitHub Actions Active
Comment thread modules/express/src/typedRoutes/schemas/wallet.ts
@zahin-mohammad zahin-mohammad merged commit 9370461 into master May 7, 2026
22 of 23 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