Skip to content

Conversation

@jurevans
Copy link
Collaborator

@jurevans jurevans commented Apr 1, 2025

Resolves #1952

  • If device is a Nano S, disable shielded key import
  • Update Ledger validation to also check against this (zip32 or masp functions only work if both device and version is correct)
  • Update warning copy on import

Testing

Build/install extension from this branch, then, assuming you are on > v3.0.0 of the Namada App:

Nano S:

  • In setup, import your Ledger
  • You should see the warning in the screenshot below
  • You should be able to successfully import just the transparent keys

Not-Nano S:

  • In setup, import your Ledger
  • You should not see any warning
  • You are guided through 3 steps: Transparent import, Viewing key import, Proof Gen / Auth key import

Notes

See: https://github.com/LedgerHQ/ledger-live/blob/06cfbf04e2d99e4e7916ace7a10cff8d37c5c38a/libs/ledgerjs/packages/types-devices/src/index.ts#L4

Ledger Live has enumerated the available device IDs here:

/**
 * DeviceModelId is a unique identifier to identify the model of a Ledger hardware wallet.
 */
export enum DeviceModelId {
  blue = "blue",
  nanoS = "nanoS",
  nanoSP = "nanoSP",
  nanoX = "nanoX",
  stax = "stax",
  europa = "europa",
}

Screenshots

NOTE I forced this message by setting the blacklisted device as nanoSP - this should be only be displayed for the nanoS:

image

@jurevans jurevans added bug Something isn't working App: Extension Ledger HW Wallet Ledger HW Wallet integration labels Apr 1, 2025
@jurevans jurevans added this to the Current Priorities milestone Apr 1, 2025
@jurevans jurevans self-assigned this Apr 1, 2025
@jurevans jurevans force-pushed the bug/disable-shielded-keys-nano-s branch 2 times, most recently from d59ad67 to 61ac83a Compare April 2, 2025 10:11
@brentstone brentstone removed this from the Current Priorities milestone Apr 2, 2025
Copy link
Collaborator

@mateuszjasiuk mateuszjasiuk left a comment

Choose a reason for hiding this comment

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

Tested and LGTM! Changed deviceId from nanoS to nanoSP for testing!

@jurevans jurevans force-pushed the bug/disable-shielded-keys-nano-s branch from 61ac83a to 22303b3 Compare April 3, 2025 09:31
@jurevans jurevans merged commit 7815f9f into main Apr 3, 2025
7 checks passed
@jurevans jurevans deleted the bug/disable-shielded-keys-nano-s branch April 3, 2025 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

App: Extension bug Something isn't working Ledger HW Wallet Ledger HW Wallet integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Keychain]: Bug - Disable shielded keys import for Nano-S only

4 participants