Skip to content

feat: multi-root layout in keycard module for vaults#9209

Merged
s84krish merged 1 commit into
masterfrom
sibikrishnan/wcn-1193-vault-keycard-multi-root-layout-in-the-key-card-module
Jul 8, 2026
Merged

feat: multi-root layout in keycard module for vaults#9209
s84krish merged 1 commit into
masterfrom
sibikrishnan/wcn-1193-vault-keycard-multi-root-layout-in-the-key-card-module

Conversation

@s84krish

@s84krish s84krish commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Ticket: WCN-1193

Description

Keycard generation for wallet vault recovery. Combines 4 root keys into single boxes - upon decryption expect a JSON containing keys for each signing scheme (mock below):

{ 
"secp256k1Multisig":"xprv9s21ZrQH143K44CWrDtJ8fRrU94Yk7scQVBaUZce…",  
"ecdsaMpc": "uQAFaGJpZ1NMaXN0g5ghAxipGHYY…",                        
"eddsaMpc": "uQADaGtleVNoYXJlmIQCAwAY6…",                           
"ed25519Multisig": "xprv9s21ZrQH143K2aZdhLN92yi2Ei47MZ1JCEvjRSmxJ6…"      
}

Vault keycard support and data handling:

  • Added new types (VaultRootKeyType, VaultKeycardRoots, GenerateVaultQrDataParams) and constants (VAULT_ROOT_ORDER) to represent and order vault root keys, and updated interfaces to support vault-specific data (modules/key-card/src/types.ts). [1] [2]
  • Implemented generateVaultQrData, which serializes all four vault roots per role into JSON objects for each keycard box, and ensures correct selection of encrypted or public key material (modules/key-card/src/generateQrData.ts).
  • Added parseVaultKeycardBox to parse and validate the vault box JSON data, ensuring all roots are present and correctly formatted (modules/key-card/src/parseKeycard.ts).

Keycard rendering and layout improvements:

  • Updated the drawKeycard function to accept pageBreakBeforeIndices, allowing custom page breaks for different keycard layouts (vault vs. wallet), and supporting default current layout (modules/key-card/src/drawKeycard.ts, modules/key-card/src/types.ts). [1] [2] [3] [4]
  • Added the generateVaultKeycard function, generating vault keycards using the new vault data format and rendering logic, and exported all new APIs from the main module (modules/key-card/src/index.ts). [1] [2]
  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Generate keycard PDF locally, attached below. Full e2e flow can be tested upon WRW integration.

vault-keycard.pdf

@linear-code

linear-code Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

WCN-1193

@s84krish s84krish force-pushed the sibikrishnan/wcn-1193-vault-keycard-multi-root-layout-in-the-key-card-module branch from 4126eb9 to 15fdb25 Compare July 8, 2026 15:34
@s84krish s84krish changed the title feat(vault-keycard): multi-root layout in keycard module for vaults feat: multi-root layout in keycard module for vaults Jul 8, 2026
@s84krish

s84krish commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

@claude

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @s84krish's task —— View job


Code Review for Multi-Root Vault Keycard Implementation

Todo List:

  • Read repository guidelines (CLAUDE.md)
  • Review type definitions and constants (types.ts)
  • Review vault QR data generation logic (generateQrData.ts)
  • Review vault keycard parsing functionality (parseKeycard.ts)
  • Review keycard drawing updates (drawKeycard.ts)
  • Review main module exports (index.ts)
  • Review test coverage (vaultQrData.ts)
  • Check for security considerations
  • Verify TypeScript best practices
  • Provide final review summary

✅ Code Review Summary

This PR implements a solid foundation for vault keycard generation with multi-root layout support. The implementation is well-structured and follows established patterns in the codebase.

🎯 Strengths

  1. Clean Architecture: The implementation extends the existing keycard system elegantly without breaking changes
  2. Type Safety: Excellent use of TypeScript with proper type definitions and union types
  3. Consistent API: New vault functions follow the same patterns as existing wallet keycard functions
  4. Comprehensive Testing: Thorough test coverage with realistic scenarios and error cases
  5. Security-Conscious: Proper encryption/decryption handling and validation of sensitive data
  6. Documentation: Good inline comments explaining complex logic, especially around JSON packing/unpacking

📝 Detailed Findings

Type Definitions (types.ts)

  • ✅ Good: VaultRootKeyType is well-defined with specific string literals
  • ✅ Good: VAULT_ROOT_ORDER provides stable ordering for consistent rendering/scanning
  • ✅ Good: VaultKeycardRoots uses Record type for type-safe key-value mapping
  • ✅ Good: GenerateVaultQrDataParams extends base params cleanly

QR Data Generation (generateQrData.ts)

  • ✅ Good: selectRootPrivateKey() correctly prefers reducedEncryptedPrv for MPC, falls back to encryptedPrv (lines 224-228)
  • ✅ Good: buildVaultBoxData() packs all 4 roots into JSON following VAULT_ROOT_ORDER (lines 243-254)
  • ✅ Good: Proper error handling with descriptive messages for missing roots/keys
  • ✅ Good: generateVaultQrData() maintains compatibility with existing QrData structure

Parsing Logic (parseKeycard.ts)

  • ✅ Good: parseVaultKeycardBox() validates JSON structure and ensures all roots are present (lines 22-39)
  • ✅ Good: Comprehensive error handling for malformed JSON, wrong types, missing roots
  • ✅ Good: Existing PDF parsing logic untouched, maintains backward compatibility

Drawing Updates (drawKeycard.ts)

  • ✅ Good: pageBreakBeforeIndices parameter allows flexible layout control (line 117)
  • ✅ Good: Default behavior preserved with DEFAULT_PAGE_BREAK_INDICES = [2] (line 34)
  • ✅ Good: No breaking changes to existing drawing logic

Main Module (index.ts)

  • ✅ Good: generateVaultKeycard() function provides clean high-level API (lines 59-67)
  • ✅ Good: Custom page breaks [1, 2] for vault layout vs default [2] for wallets (line 64)
  • ✅ Good: All new exports properly exposed

Test Coverage (vaultQrData.ts)

  • ✅ Excellent: Comprehensive test suite covering happy path, error cases, and edge cases
  • ✅ Good: Realistic test data using actual key material sizes from different signing schemes
  • ✅ Good: Round-trip testing ensures data integrity through the full generate→parse→decrypt cycle
  • ✅ Good: PDF parsing integration test validates end-to-end compatibility (lines 159-187)

🔒 Security Assessment

  • ✅ Safe: No hardcoded secrets or credentials
  • ✅ Safe: Proper encryption/decryption using established @bitgo/sdk-api functions
  • ✅ Safe: Validation of JSON structure before parsing
  • ✅ Safe: No any types used, following repository TypeScript guidelines

🚀 Performance & Maintainability

  • ✅ Good: Efficient JSON packing keeps existing 4-box layout while supporting 4 roots per box
  • ✅ Good: No breaking changes to existing APIs
  • ✅ Good: Clear separation of concerns between generation, parsing, and rendering
  • ✅ Good: Good error messages aid debugging

💡 Minor Suggestions (Optional)

  1. Consider: Adding JSDoc comments to the new public functions (generateVaultQrData, generateVaultKeycard) for better API documentation
  2. Consider: Export VaultRootKeyType and VAULT_ROOT_ORDER from main index.ts if external consumers need them

Recommendation: APPROVE

This implementation is well-crafted, thoroughly tested, and ready for merge. It successfully extends the keycard system to support vault multi-root layouts while maintaining backward compatibility and following established patterns.

The code demonstrates good software engineering practices with proper type safety, comprehensive testing, and clear documentation. The architecture allows for future extensibility while keeping the current implementation focused and maintainable.


Branch: sibikrishnan/wcn-1193-vault-keycard-multi-root-layout-in-the-key-card-module | Job Run: #15833416536

@s84krish s84krish marked this pull request as ready for review July 8, 2026 15:52
@s84krish s84krish requested review from a team as code owners July 8, 2026 15:52
@s84krish s84krish requested a review from ranga-r2 July 8, 2026 15:52
Comment thread modules/key-card/src/parseKeycard.ts
Comment thread modules/key-card/src/types.ts
@s84krish s84krish merged commit a57f112 into master Jul 8, 2026
27 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