CP-12136: Fix avalanche_getAccountPubKey for ledger#3650
Merged
ruijialin-avalabs merged 22 commits intomainfrom Mar 18, 2026
Merged
CP-12136: Fix avalanche_getAccountPubKey for ledger#3650ruijialin-avalabs merged 22 commits intomainfrom
ruijialin-avalabs merged 22 commits intomainfrom
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
ruijialin-avalabs
requested review from
B0Y3R-AVA,
alexnicolae-ava,
atn4z7,
bogdandobritoiu and
onghwan
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Ledger public key retrieval to correctly support both BIP44 (LEDGER) and Ledger Live (LEDGER_LIVE) derivation paths, and reshapes persisted Ledger wallet key material to be stored per-account.
Changes:
- Add
derivationPathTypesupport when building derivation paths for Ledger/Ledger Live public key retrieval. - Persist Ledger
publicKeysas a per-account map ({ [accountIndex]: PublicKey[] }) and derive SECP256K1 pubkeys from xpubs for BIP44 where needed. - Extend Ledger key retrieval to return address-level public keys and wire derivation-path selection through onboarding/add-account flows.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core-mobile/app/services/wallet/WalletService.tsx | Selects derivation path type based on Ledger vs Ledger Live when requesting account pubkeys. |
| packages/core-mobile/app/services/wallet/LedgerWallet.ts | Switches to per-account stored keys and adds xpub-based pubkey derivation for BIP44. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.ts | Updates wallet-secret write shape for per-account publicKeys and simplifies return typing. |
| packages/core-mobile/app/services/ledger/types.ts | Updates Ledger wallet data types and adds publicKeys to AvalancheKey. |
| packages/core-mobile/app/services/ledger/LedgerService.ts | Adds derivation-path selection and returns derived address-level pubkeys alongside xpubs. |
| packages/core-mobile/app/new/features/ledger/utils/index.ts | Updates Zod schema for the new per-account publicKeys persistence format. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionScreen.tsx | Threads selected derivation-path type into getAvalancheKeys calls. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionOnboardingScreen.tsx | Passes additional props into AppConnectionScreen for derivation-path handling. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionAddAccountScreen.tsx | Passes derivation-path type during add-account setup flow. |
| packages/core-mobile/app/new/features/ledger/hooks/useLedgerWallet.ts | Stores publicKeys per-account and consumes publicKeys returned by getAvalancheKeys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
packages/core-mobile/app/new/features/ledger/screens/AppConnectionOnboardingScreen.tsx
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Ledger key handling in core-mobile to correctly return account public keys for Avalanche/EVM when using different Ledger derivation schemes (BIP44 vs Ledger Live), and refactors stored Ledger wallet key data to be per-account.
Changes:
- Pass
derivationPathTypethroughWalletService.getPublicKeywhen the wallet isLEDGERvsLEDGER_LIVE. - Refactor Ledger wallet secret
publicKeysstorage from a flat array to a per-account map, and update related wallet/policy code paths. - Add
LedgerWallet.getPublicKeyForlogic to derive SECP256K1 public keys from stored xpubs, plus unit tests for xpub-based derivation.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core-mobile/app/services/wallet/WalletService.tsx | Uses derivationPathType when building derivation paths for Ledger vs Ledger Live. |
| packages/core-mobile/app/services/wallet/LedgerWallet.ts | Switches to per-account public key storage and adds xpub-based SECP256K1 pubkey derivation. |
| packages/core-mobile/app/services/wallet/LedgerWallet.test.ts | Adds extensive coverage for getPublicKeyFor xpub derivation behavior. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.ts | Updates wallet secret parsing and policy persistence to the new per-account publicKeys shape. |
| packages/core-mobile/app/services/ledger/types.ts | Updates ledger data types to support per-account public keys and includes publicKeys in AvalancheKey. |
| packages/core-mobile/app/services/ledger/LedgerService.ts | Extends getAvalancheKeys to accept derivation type and returns derived address-level public keys. |
| packages/core-mobile/app/new/features/ledger/utils/index.ts | Updates LedgerWalletSecretSchema to the new per-account publicKeys record format. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionScreen.tsx | Threads selected derivation path through Ledger key retrieval. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionOnboardingScreen.tsx | Provides default derivation path selection when onboarding. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionAddAccountScreen.tsx | Passes derivation path type during “add account” flow. |
| packages/core-mobile/app/new/features/ledger/hooks/useLedgerWallet.ts | Stores/updates Ledger wallet secrets using per-account publicKeys and includes publicKeys from getAvalancheKeys. |
Comments suppressed due to low confidence (2)
packages/core-mobile/app/services/wallet/LedgerWallet.ts:891
- The curve match logic compares
pk.curveagainst string literals ('secp256k1'/'ed25519'). Sincepk.curveis typed as theCurveenum, this can trigger TS2367 (no overlap) and is also redundant. Comparingpk.curve === curveis sufficient and keeps type-safety.
const matchingPublicKey = pubkey.find(pk => {
const curveMatches =
(curve === Curve.SECP256K1 && pk.curve === 'secp256k1') ||
(curve === Curve.ED25519 && pk.curve === 'ed25519')
return curveMatches && pk.derivationPath === derivationPath
packages/core-mobile/app/services/ledger/LedgerService.ts:1047
getAvalancheKeysnow accepts aderivationPath/derivation type, butgetAllAddresses()andgetExtendedPublicKeys()still derive paths using the default (BIP44) derivation type. Ifledger_livederivation differs, the returnedaddresses/xpubscan be out of sync with thepublicKeys[*].derivationPathyou compute below. Consider threading the derivationPathType through those helpers (or skipping xpub/address derivation that is incompatible with Ledger Live).
async getAvalancheKeys(
accountIndex: number,
isTestnet: boolean,
derivationPath: LedgerDerivationPathType = LedgerDerivationPathType.BIP44
): Promise<AvalancheKey> {
Logger.info('Getting Avalanche keys')
// Get addresses for display
const addresses = await this.getAllAddresses(accountIndex, 1, isTestnet)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the mobile Ledger integration to support both BIP44 (LEDGER) and Ledger Live (LEDGER_LIVE) derivation-path selection when retrieving account public keys, and changes persisted Ledger wallet key material to be stored per-account.
Changes:
- Plumbs
derivationPathTypethrough public-key retrieval and onboarding/add-account flows for Ledger vs Ledger Live. - Reshapes persisted Ledger
publicKeysinto a per-account map and adds logic to derive SECP256K1 pubkeys from xpubs where needed. - Extends derivation-path parsing (
toSegments) to support 5-segment ED25519/Solana paths and adds/updates related tests.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Lockfile checksum update. |
| packages/core-mobile/app/utils/toSegments.ts | Adds curve-aware parsing for ED25519 (5-segment) vs SECP256K1 (6-segment) paths. |
| packages/core-mobile/app/utils/toSegments.test.ts | Adds ED25519/Solana path parsing tests. |
| packages/core-mobile/app/services/wallet/WalletService.tsx | Wires derivationPathType into EVM/AVM derivation paths for public key retrieval. |
| packages/core-mobile/app/services/wallet/LedgerWallet.ts | Switches to per-account publicKeys storage and adds xpub-based pubkey derivation fallback. |
| packages/core-mobile/app/services/wallet/LedgerWallet.test.ts | Updates existing tests for per-account key storage and adds xpub-derivation coverage. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.ts | Updates wallet-secret parsing and persists BTC policy updates back into per-account publicKeys. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.test.ts | Updates tests for the new per-account publicKeys secret shape. |
| packages/core-mobile/app/services/ledger/types.ts | Updates Ledger wallet data types to use per-account publicKeys; extends AvalancheKey with publicKeys. |
| packages/core-mobile/app/services/ledger/LedgerService.ts | Extends Avalanche key retrieval to return derivation-path-aware address-level public keys. |
| packages/core-mobile/app/new/features/ledger/utils/index.ts | Updates LedgerWalletSecretSchema to the new persisted publicKeys shape. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionScreen.tsx | Passes selected derivation-path type through to Ledger key retrieval. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionOnboardingScreen.tsx | Wires onboarding-selected derivation path into the connection flow. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionAddAccountScreen.tsx | Wires add-account derivation-path selection into the connection flow. |
| packages/core-mobile/app/new/features/ledger/hooks/useLedgerWallet.ts | Stores and updates Ledger secrets using per-account publicKeys plus derivation-path selection. |
Comments suppressed due to low confidence (1)
packages/core-mobile/app/services/wallet/LedgerWallet.ts:86
LedgerWalletnow assumesledgerData.publicKeysis a per-account map (PerAccountPublicKeys). Existing persisted wallet secrets may still havepublicKeysas a flat array (the previous shape), in which casethis.publicKeys[accountIndex]will not be an array and key lookup/signing will break at runtime. Consider normalizing legacy shapes in the constructor (e.g., detectArray.isArray(ledgerData.publicKeys)and wrap into{0: publicKeys}or migrate based on derivation paths) to preserve backward compatibility for existing users.
export class LedgerWallet implements Wallet {
private deviceId: string
private derivationPathSpec: LedgerDerivationPathType
private extendedPublicKeys?: PerAccountExtendedPublicKeys
private publicKeys: PerAccountPublicKeys
private bitcoinWallet?: BitcoinLedgerWallet
private walletId: string
constructor(ledgerData: LedgerWalletData & { walletId: string }) {
this.deviceId = ledgerData.deviceId
this.derivationPathSpec = ledgerData.derivationPathSpec
this.publicKeys = ledgerData.publicKeys
this.walletId = ledgerData.walletId
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the mobile Ledger integration to correctly retrieve account public keys for both BIP44 (LEDGER) and Ledger Live (LEDGER_LIVE) derivation schemes, and changes persisted Ledger key material to be stored per-account.
Changes:
- Thread
derivationPathTypethrough derivation-path construction for Ledger vs Ledger Live when retrieving EVM/AVM public keys. - Reshape persisted Ledger
publicKeysinto a per-account map and add logic to derive SECP256K1 pubkeys from stored xpubs for BIP44/backward-compat. - Extend Ledger onboarding/add-account flows and LedgerService key retrieval to return address-level public keys.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Lockfile checksum update. |
| packages/core-mobile/app/utils/toSegments.ts | Add curve-aware parsing to support ED25519 5-segment paths. |
| packages/core-mobile/app/utils/toSegments.test.ts | Add tests for Solana ED25519 5-segment derivation paths. |
| packages/core-mobile/app/services/wallet/WalletService.tsx | Select derivation-path scheme for Ledger vs Ledger Live when building EVM/AVM paths. |
| packages/core-mobile/app/services/wallet/LedgerWallet.ts | Store public keys per-account and derive pubkeys from xpubs for BIP44 when needed. |
| packages/core-mobile/app/services/wallet/LedgerWallet.test.ts | Update mocks for per-account publicKeys and add tests for xpub derivation/fallbacks. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.ts | Update persisted wallet-secret updates to write per-account publicKeys; add schema validation. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.test.ts | Update tests to reflect per-account publicKeys persistence. |
| packages/core-mobile/app/services/ledger/types.ts | Update Ledger wallet data types to use per-account public keys. |
| packages/core-mobile/app/services/ledger/LedgerService.ts | Return address-level public keys and accept derivation-path scheme in getAvalancheKeys. |
| packages/core-mobile/app/new/features/ledger/utils/index.ts | Update wallet-secret schema to parse per-account publicKeys. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionScreen.tsx | Thread selected derivation-path type into key retrieval during onboarding/add-account. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionOnboardingScreen.tsx | Provide default derivation-path selection to connection screen. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionAddAccountScreen.tsx | Pass add-account derivation-path selection through to connection screen. |
| packages/core-mobile/app/new/features/ledger/hooks/useLedgerWallet.ts | Persist Ledger public keys per-account and store address-level pubkeys instead of addresses. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Ledger public key retrieval so avalanche_getAccountPubKey works correctly for both BIP44 (LEDGER) and Ledger Live (LEDGER_LIVE) derivation paths, and changes how Ledger key material is persisted so it’s stored per-account.
Changes:
- Thread
derivationPathType(bip44vsledger_live) through derivation-path building and Ledger key retrieval flows. - Persist Ledger
publicKeysas{ [accountIndex]: PublicKeyInfo[] }and add SECP256K1 pubkey derivation-from-xpub fallback for legacy BIP44 data. - Extend derivation-path parsing to support 5-segment ED25519 paths (Solana) and update onboarding/add-account flows accordingly.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updates lockfile checksum for react-native-webview. |
| packages/core-mobile/app/utils/toSegments.ts | Adds curve-aware parsing to support ED25519 5-segment paths and updates validation signature/messages. |
| packages/core-mobile/app/utils/toSegments.test.ts | Adds ED25519/Solana path test coverage. |
| packages/core-mobile/app/services/wallet/WalletService.tsx | Passes derivationPathType into derivation path building for Ledger vs Ledger Live. |
| packages/core-mobile/app/services/wallet/LedgerWallet.ts | Reshapes stored keys to per-account map and adds xpub-based SECP256K1 pubkey derivation fallback. |
| packages/core-mobile/app/services/wallet/LedgerWallet.test.ts | Updates mocks for per-account keys and adds coverage for getPublicKeyFor derivation/fallback. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.ts | Updates wallet-secret parsing/updating to the new per-account publicKeys storage. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.test.ts | Updates tests for the new persisted wallet secret shape. |
| packages/core-mobile/app/services/ledger/types.ts | Updates Ledger wallet data types to use per-account publicKeys and adds publicKeys to AvalancheKey. |
| packages/core-mobile/app/services/ledger/LedgerService.ts | Adds derivation-path selection and returns address-level public keys for both BIP44 and Ledger Live. |
| packages/core-mobile/app/new/features/ledger/utils/index.ts | Introduces LedgerWalletSecretSchema update to validate/transform per-account publicKeys. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionScreen.tsx | Threads selected derivation path into key fetching and effect dependencies. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionOnboardingScreen.tsx | Passes selected derivation path into connection screen. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionAddAccountScreen.tsx | Passes derivation path type through add-account flow. |
| packages/core-mobile/app/new/features/ledger/hooks/useLedgerWallet.ts | Stores per-account publicKeys and updates secret update flows (add account / Solana update). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the mobile Ledger integration to correctly retrieve account public keys for both “BIP44” and “Ledger Live” derivation path styles, and changes how Ledger key material is persisted so keys are stored per-account (with additional logic to derive SECP256K1 pubkeys from xpubs where needed).
Changes:
- Thread a
derivationPathTypethrough public-key retrieval so Ledger vs Ledger Live paths can be selected consistently. - Reshape persisted Ledger
publicKeysinto a per-account map and extend key retrieval to support address-level pubkeys (including xpub-derived pubkeys for BIP44). - Wire derivation-path selection through onboarding and add-account flows, and update tests accordingly.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Lockfile checksum update. |
| packages/core-mobile/app/utils/toSegments.ts | Add curve-aware parsing to support ED25519 (Solana) 5-segment paths. |
| packages/core-mobile/app/utils/toSegments.test.ts | Extend coverage for ED25519/Solana parsing and updated error strings. |
| packages/core-mobile/app/services/wallet/WalletService.tsx | Pass derivationPathType for Ledger vs Ledger Live into derivation-path building. |
| packages/core-mobile/app/services/wallet/LedgerWallet.ts | Store publicKeys per account, add xpub→pubkey derivation fallback, and adjust BTC policy usage. |
| packages/core-mobile/app/services/wallet/LedgerWallet.test.ts | Update fixtures for per-account publicKeys and add extensive getPublicKeyFor test coverage. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.ts | Parse and persist wallet-secret public keys per account when storing BTC policy details. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.test.ts | Update expectations for per-account publicKeys structure. |
| packages/core-mobile/app/services/ledger/types.ts | Update persisted Ledger wallet data types to use per-account publicKeys; extend AvalancheKey. |
| packages/core-mobile/app/services/ledger/LedgerService.ts | Add derivation-path selection to getAvalancheKeys and return address-level public keys. |
| packages/core-mobile/app/new/features/ledger/utils/index.ts | Update LedgerWalletSecretSchema to new per-account publicKeys shape. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionScreen.tsx | Plumb selected derivation-path type into key retrieval. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionOnboardingScreen.tsx | Provide derivation-path selection default during onboarding. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionAddAccountScreen.tsx | Pass derivation-path type through add-account setup. |
| packages/core-mobile/app/new/features/ledger/hooks/useLedgerWallet.ts | Persist Ledger key material per-account and store returned address-level public keys. |
Comments suppressed due to low confidence (1)
packages/core-mobile/app/services/wallet/LedgerWallet.ts:82
LedgerWalletnow assumesledgerData.publicKeysis a per-account map, but existing persisted Ledger secrets may still contain the legacyPublicKey[]array format. SinceWalletFactorypasses the stored JSON straight through, this constructor should normalize/migratepublicKeysat runtime (e.g., detect array vs record and convert to{[accountIndex]: PublicKey[]}) to avoid breaking existing Ledger wallets after upgrade.
private publicKeys: PerAccountPublicKeys
private bitcoinWallet?: BitcoinLedgerWallet
private walletId: string
constructor(ledgerData: LedgerWalletData & { walletId: string }) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Ledger public key retrieval to support both BIP44 (LEDGER) and Ledger Live (LEDGER_LIVE) derivation path types end-to-end, and restructures persisted Ledger key material to be stored per account (including returning/storing address-level public keys).
Changes:
- Thread
derivationPathTypethrough derivation path building for Ledger vs Ledger Live (RPC pubkey retrieval + onboarding/add-account flows). - Reshape Ledger wallet secrets to store
publicKeysas a per-account map and add xpub→pubkey derivation fallback for BIP44. - Extend/adjust tests for the new key storage and derivation behaviors (including ED25519/Solana path parsing).
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updates lockfile checksum for react-native-webview fork entry. |
| packages/core-mobile/app/utils/toSegments.ts | Adds curve-aware derivation path parsing (5-segment ED25519 vs 6-segment SECP256K1). |
| packages/core-mobile/app/utils/toSegments.test.ts | Updates error expectations and adds ED25519/Solana parsing test cases. |
| packages/core-mobile/app/services/wallet/WalletService.tsx | Passes DerivationPathType when deriving Ledger/Ledger Live paths for pubkey retrieval/caching. |
| packages/core-mobile/app/services/wallet/LedgerWallet.ts | Switches to per-account publicKeys, adds xpub-based pubkey derivation, adjusts signing path behavior. |
| packages/core-mobile/app/services/wallet/LedgerWallet.test.ts | Updates mocks for per-account publicKeys and adds extensive coverage for xpub-derived pubkeys. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.ts | Updates persisted wallet secret shape to store BTC policy under per-account publicKeys and validates secret via schema. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.test.ts | Updates expectations for per-account publicKeys storage. |
| packages/core-mobile/app/services/ledger/types.ts | Updates types to include publicKeys in AvalancheKey and define PerAccountPublicKeys. |
| packages/core-mobile/app/services/ledger/LedgerService.ts | Adds derivation-path selection; for BIP44 derives address-level pubkeys from xpubs; for Ledger Live fetches pubkeys directly. |
| packages/core-mobile/app/new/features/ledger/utils/index.ts | Introduces/updates a Zod schema for Ledger wallet secrets to the new per-account publicKeys shape. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionScreen.tsx | Wires selected derivation path into key retrieval calls. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionOnboardingScreen.tsx | Passes selected derivation path (defaulting to BIP44) into connection flow. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionAddAccountScreen.tsx | Passes selected derivation path into add-account connection flow. |
| packages/core-mobile/app/new/features/ledger/hooks/useLedgerWallet.ts | Stores/updates per-account publicKeys and uses publicKeys returned from LedgerService.getAvalancheKeys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
ruijialin-avalabs
changed the title
ruijialin-avalabs
force-pushed
the
avalanche_getAccountPubKey-ledger
branch
from
991e2ee to
584afc1
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the mobile wallet’s Ledger integration to correctly retrieve account public keys for both BIP44 (LEDGER) and Ledger Live (LEDGER_LIVE) derivation-path styles, and changes persisted Ledger key material to be stored per account.
Changes:
- Thread
derivationPathTypethrough derivation-path building for Ledger/Ledger Live public key retrieval. - Reshape persisted Ledger
publicKeysinto a per-account map and add xpub-based secp256k1 pubkey derivation fallback for BIP44. - Update onboarding/add-account flows and tests to use/validate the new key material shape and derivation-path selection.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Lockfile checksum update. |
| packages/core-mobile/app/utils/toSegments.ts | Extend derivation path parsing to support ED25519 5-segment paths and improve validation. |
| packages/core-mobile/app/utils/toSegments.test.ts | Add/adjust tests for new toSegments behavior (incl. ED25519). |
| packages/core-mobile/app/services/wallet/WalletService.tsx | Pass derivation-path type based on Ledger wallet variant when requesting pubkeys. |
| packages/core-mobile/app/services/wallet/LedgerWallet.ts | Persist and use per-account public keys; add xpub-derived pubkey fallback; adjust signing path logic. |
| packages/core-mobile/app/services/wallet/LedgerWallet.test.ts | Update tests for per-account publicKeys and new pubkey derivation behavior. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.ts | Update storage logic for per-account publicKeys and add schema parsing. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.test.ts | Update tests for the new per-account publicKeys shape. |
| packages/core-mobile/app/services/ledger/types.ts | Extend Ledger key types (AvalancheKey now includes publicKeys; wallet data uses per-account keys). |
| packages/core-mobile/app/services/ledger/LedgerService.ts | Return address-level public keys and support both derivation-path types in key retrieval. |
| packages/core-mobile/app/new/features/ledger/utils/index.ts | Update wallet secret schema to per-account publicKeys record + numeric key transform. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionScreen.tsx | Wire selected derivation-path type into Ledger key retrieval. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionOnboardingScreen.tsx | Provide default derivation-path selection in onboarding. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionAddAccountScreen.tsx | Pass derivation-path type for add-account flow. |
| packages/core-mobile/app/new/features/ledger/hooks/useLedgerWallet.ts | Store/update per-account publicKeys (and xpubs where applicable) in wallet secrets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the mobile Ledger integration to correctly retrieve and persist public keys for both BIP44 (LEDGER) and Ledger Live (LEDGER_LIVE) derivation path schemes, and reshapes persisted Ledger key material to be stored per-account.
Changes:
- Thread derivation-path selection through public-key retrieval (RPC-facing) and onboarding/add-account flows.
- Reshape Ledger wallet secret storage to persist
publicKeysas a per-account map and (for BIP44) derive SECP256K1 pubkeys from stored xpubs when needed. - Extend Ledger service/wallet logic and tests to support address-level public key retrieval for both derivation path types.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updates lockfile checksum for react-native-webview. |
| packages/core-mobile/app/utils/toSegments.ts | Adds curve-aware derivation path parsing (5-segment ED25519 vs 6-segment SECP256K1). |
| packages/core-mobile/app/utils/toSegments.test.ts | Updates error strings and adds ED25519/Solana path test coverage. |
| packages/core-mobile/app/services/wallet/WalletService.tsx | Wires derivationPathType into derivation path building for Ledger/Ledger Live pubkey retrieval. |
| packages/core-mobile/app/services/wallet/LedgerWallet.ts | Switches Ledger publicKeys storage to per-account map and adds xpub-based pubkey derivation for legacy BIP44 data. |
| packages/core-mobile/app/services/wallet/LedgerWallet.test.ts | Updates tests for per-account public key storage and adds extensive getPublicKeyFor coverage. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.ts | Updates wallet-secret persistence to write policy details into per-account publicKeys. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.test.ts | Updates tests to match per-account publicKeys storage shape. |
| packages/core-mobile/app/services/ledger/types.ts | Updates Ledger wallet data types to store publicKeys per account and adds publicKeys to AvalancheKey. |
| packages/core-mobile/app/services/ledger/LedgerService.ts | Adds derivation-path-type support to getAvalancheKeys and returns address-level public keys. |
| packages/core-mobile/app/new/features/ledger/utils/index.ts | Introduces LedgerWalletSecretSchema and public key schema for persisted wallet secrets. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionScreen.tsx | Passes selected derivation path into key retrieval and updates dependency list. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionOnboardingScreen.tsx | Defaults selected derivation path for onboarding. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionAddAccountScreen.tsx | Threads derivation path selection into add-account connection screen. |
| packages/core-mobile/app/new/features/ledger/hooks/useLedgerWallet.ts | Stores publicKeys per account (and xpubs for BIP44) and updates add-account/update flows accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
1058
to
1064
| async getAvalancheKeys( | ||
| accountIndex: number, | ||
| isTestnet: boolean | ||
| isTestnet: boolean, | ||
| derivationPath: LedgerDerivationPathType = LedgerDerivationPathType.BIP44 | ||
| ): Promise<AvalancheKey> { | ||
| Logger.info('Getting Avalanche keys') | ||
|
|
Comment on lines
+88
to
+94
| .record(z.string(), z.array(PublicKeyInfoSchema)) | ||
| .transform( | ||
| record => | ||
| Object.fromEntries( | ||
| Object.entries(record).map(([k, v]) => [Number(k), v]) | ||
| ) as Record<number, z.infer<typeof PublicKeyInfoSchema>[]> | ||
| ) |
Comment on lines
75
to
81
| private deviceId: string | ||
| private derivationPathSpec: LedgerDerivationPathType | ||
| private extendedPublicKeys?: PerAccountExtendedPublicKeys | ||
| private publicKeys: PublicKey[] | ||
| private publicKeys: PerAccountPublicKeys | ||
| private bitcoinWallet?: BitcoinLedgerWallet | ||
| private walletId: string | ||
|
|
Comment on lines
110
to
+113
| // Parse the current wallet data | ||
| const currentWalletData = JSON.parse(walletSecretResult.value) | ||
| const currentWalletData = LedgerWalletSecretSchema.parse( | ||
| JSON.parse(walletSecretResult.value) | ||
| ) |
| derivationPath: "m/44'/60'/1'/0/0", // account 1, not stored | ||
| curve: Curve.SECP256K1 | ||
| }) | ||
| ).rejects.toThrow('No public keys available for LedgerWallet') |
B0Y3R-AVA
reviewed
Mar 17, 2026
| const extendedKeys = await this.getExtendedPublicKeys(accountIndex) | ||
| const derivationPathType = | ||
| derivationPath === LedgerDerivationPathType.BIP44 | ||
| ? 'bip44' |
Contributor
There was a problem hiding this comment.
would love for these to be consts
B0Y3R-AVA
approved these changes
Mar 17, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Ledger public key retrieval and persisted wallet key material to correctly support both BIP44 (LEDGER) and Ledger Live (LEDGER_LIVE) derivation paths, with public keys stored per-account to support multi-account flows and address-level key lookups.
Changes:
- Thread
derivationPathTypethrough derivation-path building for Ledger/Ledger Live key retrieval. - Reshape persisted Ledger
publicKeysinto a per-account map and add xpub-based SECP256K1 pubkey derivation fallback for legacy data. - Extend onboarding/add-account flows to select derivation type and store address-level public keys.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Lockfile checksum update. |
| packages/core-mobile/app/utils/toSegments.ts | Allow ED25519 5-segment derivation paths; refactor segment validation. |
| packages/core-mobile/app/utils/toSegments.test.ts | Update error expectations; add ED25519 path parsing tests. |
| packages/core-mobile/app/services/wallet/WalletService.tsx | Pass derivationPathType for Ledger/Ledger Live when building paths. |
| packages/core-mobile/app/services/wallet/LedgerWallet.ts | Store publicKeys per-account; derive SECP256K1 pubkeys from xpubs; adjust signing paths. |
| packages/core-mobile/app/services/wallet/LedgerWallet.test.ts | Update fixtures and add extensive getPublicKeyFor coverage. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.ts | Parse wallet secrets via schema; update BTC policy storage for per-account publicKeys. |
| packages/core-mobile/app/services/wallet/BitcoinWalletPolicyService.test.ts | Adjust tests for per-account publicKeys shape. |
| packages/core-mobile/app/services/ledger/types.ts | Add publicKeys to AvalancheKey; change wallet publicKeys to per-account map type. |
| packages/core-mobile/app/services/ledger/LedgerService.ts | Add derivation type selection in getAvalancheKeys; return address-level pubkeys; avoid redundant open-app APDU. |
| packages/core-mobile/app/new/features/ledger/utils/index.ts | Update persisted wallet secret schema for per-account publicKeys. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionScreen.tsx | Pass selected derivation type into getAvalancheKeys. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionOnboardingScreen.tsx | Provide default selected derivation type during onboarding. |
| packages/core-mobile/app/new/features/ledger/screens/AppConnectionAddAccountScreen.tsx | Pass derivation type for add-account flow. |
| packages/core-mobile/app/new/features/ledger/hooks/useLedgerWallet.ts | Persist publicKeys per-account; store address-level keys from getAvalancheKeys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| derivationPath: "m/44'/60'/1'/0/0", // account 1, not stored | ||
| curve: Curve.SECP256K1 | ||
| }) | ||
| ).rejects.toThrow('No public keys available for LedgerWallet') |
Comment on lines
+88
to
+94
| .record(z.string(), z.array(PublicKeyInfoSchema)) | ||
| .transform( | ||
| record => | ||
| Object.fromEntries( | ||
| Object.entries(record).map(([k, v]) => [Number(k), v]) | ||
| ) as Record<number, z.infer<typeof PublicKeyInfoSchema>[]> | ||
| ) |
Comment on lines
+611
to
+628
| // For C chain (EVM): bip44 uses m/44'/60'/{accountIndex}' or Ledger Live (m/44'/60'/0') depending on wallet type | ||
| const accountPath = | ||
| chainAlias === 'C' | ||
| ? `m/44'/60'/${accountIndex}'` | ||
| ? this.isBIP44() | ||
| ? `m/44'/60'/0'` | ||
| : `m/44'/60'/${accountIndex}'` | ||
| : `m/44'/9000'/${accountIndex}'` | ||
|
|
||
| // Build signing paths from external indices | ||
| // For C-chain: always use 0/0 (first external address) | ||
| // For C-chain: bip44 uses 0/<accountIndex> or Ledger Live always use 0/0 (first external address) | ||
| // For X/P-chain: use external indices from UTXO analysis (default to [0] → '0/0' if empty) | ||
| const externalIndices = transaction.externalIndices ?? [] | ||
| const hasIndices = externalIndices.length > 0 | ||
| const signingPaths = | ||
| chainAlias === 'C' | ||
| ? ['0/0'] | ||
| ? this.isBIP44() | ||
| ? [`0/${accountIndex}`] | ||
| : ['0/0'] |
onghwan
approved these changes
Mar 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Ticket: CP-12136 Ticket: CP-13640
iOS: 7765
Android: 7766
This PR updates Ledger public key retrieval to correctly support both BIP44 (
LEDGER) and Ledger Live (LEDGER_LIVE) derivation paths, and reshapes persisted Ledger wallet key material to be stored per-account.Changes:
derivationPathTypesupport when building derivation paths for Ledger/Ledger Live public key retrieval.publicKeysas a per-account map ({ [accountIndex]: PublicKey[] }) and derive SECP256K1 pubkeys from xpubs for BIP44 where needed.Screenshots/Videos
Include relevant screenshots or screen recordings of iOS and Android.
Testing
Dev Testing (if applicable)
Setup bip-44 ledger wallet
open extension playground: https://ava-labs.github.io/extension-avalanche-playground/rpc-calls
send avalanche_getAccountPubKey rpc to wallet
make sure you get the evm and avalanche pubkey back
Setup ledger-live ledger wallet
open extension playground: https://ava-labs.github.io/extension-avalanche-playground/rpc-calls
send avalanche_getAccountPubKey rpc to wallet
make sure you get the same evm and avalanche pubkey back
QA Testing (if applicable)
Checklist
Please check all that apply (if applicable)