WCN-848

@claude

Claude finished @rajangarg047's task —— View job

Code Review for PR #8974 ✅

I've completed a comprehensive review of the changes for adding webauthnInfo with enterpriseId to MPC user keychain creation.

Todo List:

• Read repository guidelines from CLAUDE.md
• Analyze the interface changes in iKeychains.ts
• Review MPC implementation changes (ECDSA v1/v2, EdDSA v1/v2)
• Examine test coverage and correctness
• Check for security implications and best practices
• Verify consistency across all 4 MPC implementations
• Provide comprehensive review feedback

Summary

This PR successfully addresses the issue where passkeys were not persisted for TSS/MPC user keychains. The changes correctly switch from the deprecated webauthnDevices array to the webauthnInfo object (including enterpriseId) that the wallet-platform atomic key-creation endpoint actually consumes.

Detailed Review

✅ Interface Changes (iKeychains.ts)

The addition of optional enterpriseId to WebauthnInfo is well-documented and properly typed:

• Clear JSDoc explaining when enterpriseId is required vs optional
• Maintains backward compatibility as the field is optional
• Removes the type-widening workaround mentioned in the PR description

✅ Implementation Consistency

All four MPC implementations are consistently updated:

• ECDSA MPCv1: Lines 428-440 in ecdsa.ts:createParticipantKeychain
• ECDSA MPCv2: Lines 439-454 in ecdsaMPCv2.ts:createParticipantKeychain
• EdDSA MPCv1: Lines 205-219 in eddsa.ts:createUserKeychain
• EdDSA MPCv2: Lines 283-298 in eddsaMPCv2.ts:createParticipantKeychain

Each implementation follows the same pattern:

1. Only applies to USER keychains (not backup/bitgo)
2. Requires enterprise parameter when webauthn is used
3. Sends webauthnInfo with all required fields including enterpriseId
4. Does NOT send deprecated webauthnDevices

✅ Security Best Practices

• ✅ Proper validation: enterprise parameter is required when attaching webauthn devices
• ✅ PRF-derived encryption: encryptedPrv uses the webauthn passphrase for encryption
• ✅ Scope limitation: Only USER keychains get webauthn info (backup keychains remain clean)
• ✅ No credential exposure: Private key material is properly encrypted before transmission

✅ Test Coverage

Excellent test coverage in ecdsaMPCv2/createKeychains.ts:182-242:

• ✅ Verifies webauthnInfo is sent (not webauthnDevices)
• ✅ Confirms enterpriseId is included in the request
• ✅ Validates PRF-encrypted private key decryption
• ✅ Ensures backup keychain has no webauthn material
• ✅ Uses realistic test data and proper assertions

✅ Code Quality

• Clean, consistent implementation across all MPC variants
• Proper error handling with meaningful messages
• Good separation of concerns
• Follows existing code patterns and conventions
• Thread safety maintained

Minor Observations

1. Documentation: The JSDoc for WebauthnInfo.enterpriseId is excellent and clearly explains the use case differences between POST and PUT operations.

2. Backward Compatibility: The changes maintain full backward compatibility since enterpriseId is optional and the deprecated webauthnDevices field is simply not sent (rather than being removed from the interface).

3. Testing Strategy: The test properly captures the actual HTTP request bodies to verify the correct fields are being sent, which is the right level of testing for this change.

Recommendation

✅ APPROVE - This PR successfully fixes the passkey persistence issue with a clean, well-tested implementation. The changes are:

• Technically sound and secure
• Consistently applied across all MPC implementations
• Properly tested with good coverage
• Well-documented and maintainable
• Backward compatible

The implementation correctly addresses the core issue: the wallet-platform endpoint consumes webauthnInfo (not webauthnDevices) and requires enterpriseId for PRF salt validation during atomic key creation.

Job run: https://github.com/BitGo/BitGoJS/actions/runs/12345
Branch: rajangarg047/wcn-848-mpcv2-user-keychain-webauthninfo