CSHLD-1451: Pass bridgingParams/sbtcWithdrawParams through to /tx/initiate for custody sBTC mint/burn

[abhi-bitgo]

Summary

Custody (custodial, non-TSS) sBTC mint/burn requests go through sendMany -> initiateTransaction -> POST /tx/initiate, but that path silently dropped bridgingParams (BTC->sBTC mint) and sbtcWithdrawParams (sBTC burn) since TxSendBody doesn't declare them, even though BGMS already supports both fields on this route.

Linear: CSHLD-1451

Changes

• wallet.ts: added whitelistedInitiateParams, scoped to initiateTransaction//tx/initiate only (leaving sendTransaction//tx/send untouched), and extended initiateTransaction's encoding codec with a local t.intersection for bridgingParams/sbtcWithdrawParams, mirroring the existing attestation pass-through pattern for the same TxSendBody gap.
• BuildParams.ts: added structured io-ts codecs SbtcBridgingParams/BridgingParams, mirroring iWallet.ts's TS interfaces, to validate bridgingParams with the same rigor as the existing SbtcWithdrawParams codec.
• Added regression tests: codec-level tests for the new BridgingParams/SbtcBridgingParams shapes, plus wallet-level tests confirming custodial sendMany forwards these fields to /tx/initiate while non-custodial sendMany does not forward them to /tx/send.

Test Plan

• BuildParams.ts unit tests pass (8/8)
• tsc --noEmit / eslint clean on touched files
• modules/bitgo wallet.ts test suite (blocked locally by an unrelated missing @bitgo/sdk-coin-pearl workspace symlink — needs verification in CI)

[linear-code]

CSHLD-1451

[abhi-bitgo]

@claude review this pr

[github-actions]

Claude finished @abhi-bitgo's task —— View job

---

PR Review: CSHLD-1451 - Pass bridgingParams/sbtcWithdrawParams to /tx/initiate

Review Progress:

• Analyze PR context and changes
• Review BuildParams.ts changes
• Review wallet.ts implementation
• Review test coverage
• Check code quality and best practices
• Provide comprehensive feedback

---

Overall Assessment: ✅ APPROVED

This is a well-implemented PR that addresses a specific gap in the custody sBTC workflow. The implementation is clean, follows existing patterns, and includes comprehensive test coverage.

🔍 Detailed Review

Problem & Solution ✅

• Problem clearly identified: Custody sBTC operations were silently dropping bridgingParams/sbtcWithdrawParams because TxSendBody doesn't declare them
• Solution is appropriate: Local intersection with the missing fields for /tx/initiate only, preserving existing behavior for /tx/send
• Follows established patterns: Mirrors the existing attestation pass-through pattern

Code Quality & Architecture ✅

modules/sdk-core/src/bitgo/wallet/BuildParams.ts

• Excellent codec design: New SbtcBridgingParams and BridgingParams codecs properly mirror TypeScript interfaces
• Proper validation: SbtcBridgingParams requires all 4 fields, supports both number and string for amounts (matches interface)
• Good documentation: Clear JSDoc comments explaining purpose and relationship to interfaces

modules/sdk-core/src/bitgo/wallet/wallet.ts

• Smart scoping: whitelistedInitiateParams only affects /tx/initiate, leaving /tx/send untouched as intended
• Clean implementation: Uses t.intersection to extend TxSendBody locally without upstream changes
• Excellent documentation: Comprehensive comments explaining why this workaround is needed and when to remove it
• Proper routing: Custodial wallets → /tx/initiate, non-custodial → /tx/send (existing behavior preserved)

Test Coverage ✅

BuildParams.ts unit tests

• Comprehensive codec validation: Tests required fields, optional fields, type unions (number/string)
• Edge case coverage: Empty objects, incomplete nested shapes, wrong types
• Clear test structure: Well-organized with descriptive test names

Wallet integration tests

• End-to-end validation: Tests actual sendMany → API routing behavior
• Proper mocking: Uses nock to verify correct API calls with expected parameters
• Critical scenarios covered:
  • Custodial wallets pass bridgingParams to /tx/initiate ✅
  • Custodial wallets pass sbtcWithdrawParams to /tx/initiate ✅
  • Non-custodial wallets don't forward these params to /tx/send ✅

🔧 Technical Correctness

Type Safety ✅

• io-ts codecs properly validate runtime data
• TypeScript interfaces and codecs are kept in sync
• Union types correctly handle number | string for amounts

API Routing Logic ✅

// Custodial: sendMany → initiateTransaction → /tx/initiate (server builds/signs)
if (this._wallet.type === 'custodial') {
  return this.initiateTransaction(selectParams, reqId); // ✅ includes bridging params
}
// Non-custodial: sendMany → prebuildAndSignTransaction → sendTransaction → /tx/send
return this.sendTransaction(finalTxParams, reqId); // ✅ excludes build-time params

Backward Compatibility ✅

• No breaking changes to existing APIs
• Only additive changes to parameter handling
• Existing workflows unaffected

🧪 Testing Notes

• The PR mentions local test blockage due to missing @bitgo/sdk-coin-pearl workspace symlink
• This appears to be a local development environment issue, not a code problem
• The BuildParams tests should run independently and are well-structured

📋 Minor Suggestions

1. Consider adding JSDoc to whitelistedInitiateParams:

/** 
 * Parameters whitelisted for /tx/initiate requests. Extends whitelistedSendParams
 * with bridging-specific fields that are build-time only and shouldn't go to /tx/send.
 */
const whitelistedInitiateParams = [...whitelistedSendParams, 'bridgingParams', 'sbtcWithdrawParams'];

2. Test enhancement opportunity (non-blocking): Could add a test verifying that the parameters are properly validated by the new codecs in the integration flow.

✅ Approval Criteria Met

• Addresses the stated problem correctly
• Maintains backward compatibility
• Follows existing code patterns and conventions
• Includes comprehensive test coverage
• Uses proper TypeScript/io-ts patterns
• Clear documentation and comments
• No security concerns identified

Recommendation: LGTM - Ready to merge 🚀

---

Job run: View details