Skip to content

Conversation

@mrdanish26
Copy link
Contributor

@mrdanish26 mrdanish26 commented Jan 21, 2026

TICKET: WP-7507

Problem
Address validation failed for Self-Managed Custodial (SMC) TSS wallets on EdDSA-based coins (e.g., Solana). The client-side verification used m/{index} while the wallet derivation used m/{prefix}/{index}, causing a mismatch.

Fix
Extract the derivation prefix from the User keychain's derivedFromParentWithSeed for SMC (cold) TSS wallets
Pass the prefix to address verification to construct the correct derivation path
Use m/{prefix}/{index} when a prefix is present, otherwise m/{index}

Changes

  • wallet.ts: Extract derivationPrefix from User keychain for SMC TSS wallets
  • iBaseCoin.ts: Add optional derivationPrefix to TssVerifyAddressOptions and VerifyAddressOptions
  • addressVerification.ts: Use derivationPrefix when constructing the derivation path
  • Tests: Add unit tests for derivation prefix extraction and path construction

Testing

  • Added unit tests for SMC wallet prefix extraction
  • Added unit tests for derivation path construction with/without prefix

Impact

  • Fixes address validation for SMC TSS wallets
  • Backward compatible (prefix is optional)
  • No impact on custodial or non-TSS wallets

@mrdanish26 mrdanish26 requested review from a team as code owners January 21, 2026 23:11
@mrdanish26 mrdanish26 marked this pull request as draft January 21, 2026 23:12
@mrdanish26 mrdanish26 force-pushed the create-address-issue/WP-7507 branch 3 times, most recently from b4a37c1 to b7d2cf4 Compare January 22, 2026 00:47
@mrdanish26 mrdanish26 requested a review from Marzooqa January 22, 2026 02:04
@mrdanish26 mrdanish26 marked this pull request as ready for review January 22, 2026 02:04
@mrdanish26 mrdanish26 requested review from a team as code owners January 22, 2026 02:04
@mrdanish26 mrdanish26 requested a review from Copilot January 22, 2026 02:10
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes address validation for Self-Managed Custodial (SMC) TSS wallets on EdDSA-based coins like Solana. The issue occurred because client-side verification used m/{index} derivation paths while wallet derivation used m/{prefix}/{index}, causing a mismatch.

Changes:

  • Extracts derivation prefix from User keychain's derivedFromParentWithSeed for cold TSS wallets
  • Adds optional derivationPrefix parameter to address verification interfaces
  • Updates derivation path construction to use prefix when available
  • Adds comprehensive unit tests for prefix extraction and validation scenarios
  • Excludes a tar security advisory that doesn't affect the project's usage pattern

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
modules/sdk-core/src/bitgo/wallet/wallet.ts Extracts derivation prefix from User keychain for SMC TSS wallets and passes it to address verification
modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts Adds optional derivationPrefix field to TssVerifyAddressOptions and VerifyAddressOptions interfaces with documentation
modules/sdk-core/src/bitgo/utils/tss/addressVerification.ts Updates derivation path construction to use {derivationPrefix}/{index} when prefix is present, removes circular dependency with lazy import
modules/sdk-core/test/unit/bitgo/wallet/walletTssAddressVerification.ts Adds comprehensive unit tests for custodial, cold/SMC, and non-TSS wallet scenarios with edge cases
modules/sdk-core/test/unit/bitgo/utils/tss/addressVerification.ts Adds unit tests for extractCommonKeychain function and derivation path format validation
modules/sdk-coin-sol/test/unit/sol.ts Adds integration test verifying address validation with derivation prefix for Solana SMC wallets
.iyarc Excludes tar vulnerability GHSA-r6q2-hw4h-h46w that only affects extraction, not the project's packing usage

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mrdanish26 mrdanish26 force-pushed the create-address-issue/WP-7507 branch from b7d2cf4 to cf10521 Compare January 22, 2026 02:35
@Marzooqa Marzooqa force-pushed the create-address-issue/WP-7507 branch from cf10521 to 2730531 Compare January 22, 2026 08:00
@Marzooqa
Copy link
Contributor

Marzooqa commented Jan 22, 2026

Changes the commit 2730531955bbfc9a98b526c9acb91e1641b4b824:
Move derivation prefix computation inside verifyMPCWalletAddress so callers only need to pass derivedFromParentWithSeed. This enables any endpoint / method to verify SMC wallet addresses without computing the prefix themselves.

@Marzooqa Marzooqa merged commit 98f9908 into master Jan 22, 2026
32 of 33 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.

6 participants