feat(sdk-core,express): plumb userKeySigningRequired through generateWallet

[zahin-mohammad]

Summary

• Add optional userKeySigningRequired?: boolean to GenerateWalletOptions and GenerateGoAccountWalletOptionsCodec in @bitgo/sdk-core, forwarded into the coinSpecific payload of POST /wallet/add when creating OFC trading wallets via generateGoAccountWallet.
• Add the same field to the Express GenerateWalletBody io-ts codec so POST /api/v2/{coin}/wallet/generate accepts and forwards it to the SDK.
• CAAS clients create OFC wallets through Express or the SDK, not by hitting the raw wallet-platform endpoint directly. The userKeySigningRequired flag accepted at wallet creation time (WCN-1441) was unreachable via the public SDK/Express surface, leaving clients no way to opt out of user-key signing at creation.

Test plan

• yarn unit-test in modules/sdk-core passes (new walletsGoAccount.ts suite: 3 tests — false forwarded, true forwarded, omitted → no coinSpecific)
• yarn unit-test in modules/express passes (new typed-route tests: happy path + codec rejection of non-boolean)
• Integration: SDK generateWallet({ type: 'trading', userKeySigningRequired: false }) on an OFC coin → created wallet has coinSpecific.ofc.userKeySigningRequired === false
• Integration: userKeySigningRequired omitted → coinSpecific absent from request body (default preserved)

Ticket: WCN-1442

[linear-code]

WCN-1442

[rishikeshdadam136]

@claude can you please review this PR? Also, could you check this specific comment (#9335 (comment)) and let me know if it is valid?

[github-actions]

Claude finished @rishikeshdadam136's task —— View job

---

PR Review Analysis: Critical Data Structure Mismatch Found ⚠️

Todo List:

• Read and analyze all changed files
• Investigate the specific comment about coinSpecific structure mismatch
• Examine the data flow in wallet generation
• Provide comprehensive review feedback
• Validate the reviewer's technical concern

Critical Issue: Data Structure Mismatch

The reviewer's concern is 100% valid and critical. This PR will not work as intended due to a fundamental data structure mismatch.

Current Implementation (Incorrect)

In wallets.ts:263, the code creates a nested structure:

coinSpecific: { [this.baseCoin.getChain()]: { userKeySigningRequired } }

This produces:

{
  "coinSpecific": {
    "ofc": {
      "userKeySigningRequired": false
    }
  }
}

Expected Structure (Correct)

Based on evidence from the codebase, BGMS expects a flat structure:

{
  "coinSpecific": {
    "userKeySigningRequired": false  // directly under coinSpecific
  }
}

Evidence Supporting This Analysis

1. Express decode tests explicitly test the flat structure:

   coinSpecific: { userKeySigningRequired: true, needsKeySigningRequired: false }

2. Trading account implementation reads from the flat path:

   walletData.coinSpecific?.userKeySigningRequired ?? walletData.userKeySigningRequired ?? true

3. Other test files show wallets with flat coinSpecific:

   coinSpecific: { userKeySigningRequired: false }

4. The existing unit tests in this PR are testing the wrong structure - they validate coinSpecific.ofc.userKeySigningRequired instead of coinSpecific.userKeySigningRequired

Impact

• ❌ BGMS will read body.coinSpecific.userKeySigningRequired as undefined
• ❌ This causes BGMS to default to userKeySigningRequired = true
• ❌ The feature becomes completely non-functional
• ❌ Clients cannot opt out of user-key signing at wallet creation

Required Fix

Change wallets.ts:263 from:

coinSpecific: { [this.baseCoin.getChain()]: { userKeySigningRequired } }

To:

coinSpecific: { userKeySigningRequired }

And update the unit tests accordingly to validate the correct flat structure.

Other Review Comments

✅ Positive Aspects

• Well-documented interfaces: The userKeySigningRequired parameter is properly documented
• Good Express integration: The typed routes correctly accept and validate the parameter
• Comprehensive test coverage: Both Express and SDK tests are included
• Proper TypeScript typing: All interfaces are properly typed with optional boolean

⚠️ Additional Concerns

1. Type inconsistency: modules/sdk-core/src/bitgo/wallet/iWallets.ts:266 uses JSDoc comment format instead of TypeScript doc comment
2. Test structure mismatch: All SDK unit tests expect the incorrect nested structure and need updating

Recommendations

1. Priority 1: Fix the data structure issue in wallets.ts:263
2. Priority 1: Update all unit tests to validate the correct flat structure
3. Priority 2: Consider integration testing against a mock BGMS endpoint to verify the fix
4. Priority 3: Update JSDoc to use proper TypeScript comment format

This is a breaking bug that prevents the feature from working. The fix should be straightforward but requires updating both implementation and tests.

---

Job Run: View Details

[zahin-mohammad]

@rishikeshdadam136 Thanks for the review. The flagged coinSpecific structure mismatch was valid and is now fixed (commit 805a095):

• wallets.ts builds the flat shape coinSpecific: { userKeySigningRequired } — matching what BGMS reads in extractAdditionalCreateWalletData (body.coinSpecific.userKeySigningRequired). The chain-nesting is gone.
• Unit tests updated to assert the flat shape; all pass.
• Branch rebased onto latest master.

[rishikeshdadam136]

Looks Good

[rishikeshdadam136]

Looks Good