WCI-1358: Forward enterprise on tss/settings and keychain add calls - #9543
Conversation
…alls BitGoJS already had enterprise available at every call site into GET /api/v2/tss/settings and the subsequent EdDSA MPCv2 keychains.add(...) calls, but never sent it, so per-enterprise Flipt rollouts on the wallet-platform side (WCI-830) can never take effect for SDK-driven wallet/key creation. Thread it through: - Keychains.createMpc() and all 5 Wallets GET /tss/settings call sites now send enterprise as a query param when available. - EddsaMPCv2Utils threads enterprise into the user/backup keychain add() calls for both the standard and external-signer paths, including a pre-existing gap where the backup keychain never received it even though the user keychain did. Fully backward compatible: enterprise remains optional everywhere, and omitting it produces the exact same request as before. Existing nock/sinon mocks for GET /tss/settings expected no query string; loosened to .query(true) since none of those tests were asserting on query params, plus two new dedicated tests proving enterprise is forwarded when supplied and omitted when it isn't. Ticket: WCI-1358
d376a1d to
25f747b
Compare
|
Claude finished @Marzooqa's task —— View job Code Review for WCI-1358Todo List:
Review SummaryThis PR successfully implements forwarding the ✅ Strengths1. Comprehensive Coverage
2. Backward CompatibilityThe implementation correctly maintains backward compatibility:
3. Test CoverageExcellent test coverage with two dedicated behavioral tests in
🔍 Minor Observations1. Consistent PatternThe implementation follows a consistent pattern across all files: .query({ enterprise: params.enterprise })This is clean and handles the undefined case properly since 2. EddsaMPCv2 FixThe fix in // Before: only user keychain got enterprise implicitly
// After: both user and backup keychains explicitly get enterprise
keychains.add({ source: 'user', keyType: 'tss' as KeyType, commonKeychain, isMPCv2: true, enterprise }),
keychains.add({ source: 'backup', keyType: 'tss' as KeyType, commonKeychain, isMPCv2: true, enterprise }),3. Test QualityThe new tests in
🎯 RecommendationsNo Issues FoundThis is a clean, well-implemented change that:
Final Assessment✅ APPROVE - This PR is ready to merge. The implementation is correct, well-tested, and maintains backward compatibility while successfully forwarding the |
Summary
Keychains.createMpc()and all 5Walletscall sites that hitGET /api/v2/tss/settings(hot, cold, custodial, external-signer, ECDSA version-bump) now sendenterpriseas a query param when the caller supplied one. Previously this value was computed/available but silently dropped, never sent over the wire.EddsaMPCv2Utilsthreadsenterpriseinto thekeychains.add(...)calls for both the standard and external-signer EdDSA MPCv2 key-creation paths — including fixing a pre-existing gap where the backup keychain never receivedenterpriseeven though the user keychain did.enterpriseremains optional everywhere; omitting it produces the exact same request as before this PR.enterpriseId-scoped Flipt segments once this reaches the server.Test plan
keychains.ts,wallets.ts(bitgo module) andeddsaMPCv2.ts/eddsaExternalSigner.ts/walletsExternalSigner.ts(sdk-core) all pass locallykeychains.tsprove the actual behavior:enterpriseis forwarded when supplied (exact query match, not a loosened matcher), and omitted entirely (no query string) when it isn't🤖 Generated with Claude Code