feat: introduce custody ledger - #400
Conversation
288e3df to
9c06259
Compare
9c06259 to
72aa29f
Compare
TalDerei
left a comment
TalDerei
left a comment
There was a problem hiding this comment.
few nits, but otherwise the ledger support scaffolding looks great!
| // absent values now explicitly required | ||
| backupReminderSeen: undefined, | ||
| compactFrontierBlockHeight: undefined, | ||
| walletCreationBlockHeight: undefined, |
There was a problem hiding this comment.
comment: why the explicit requirement here?
| // lol | ||
| backupReminderSeen: | ||
| typeof old.backupReminderSeen === 'boolean' ? old.backupReminderSeen : undefined, | ||
| compactFrontierBlockHeight: | ||
| typeof old.compactFrontierBlockHeight === 'number' | ||
| ? old.compactFrontierBlockHeight | ||
| : undefined, | ||
| walletCreationBlockHeight: | ||
| typeof old.walletCreationBlockHeight === 'number' ? old.walletCreationBlockHeight : undefined, |
There was a problem hiding this comment.
comment: can't we do without this?
| // check the device info | ||
| // const deviceInfo = await ledgerApp.deviceInfo(); | ||
| // console.debug('deviceInfo', deviceInfo); | ||
|
|
||
| // check the app info | ||
| // const appInfo = await ledgerApp.appInfo(); | ||
| // console.debug('appInfo', appInfo); |
| // check the device info | ||
| // const deviceInfo = await ledgerApp.deviceInfo(); | ||
| // console.debug('deviceInfo', deviceInfo); |
| produce((state: AllSlices) => { | ||
| state.network.chainId = stored | ||
| ? AppParameters.fromJsonString(stored).chainId | ||
| ? AppParameters.fromJson(stored).chainId |
There was a problem hiding this comment.
comment: this caught my eye. v3 schema suggests that param is stored as a string, not a parsed object. let's revert this back to AppParameters.fromJsonString(stored).
| filter != null && | ||
| device != null && | ||
| filter.vendorId === device.vendorId && | ||
| filter.productId === device.productId && | ||
| filter.classCode === device.deviceClass && | ||
| filter.subclassCode === device.deviceSubclass && | ||
| filter.protocolCode === device.deviceProtocol && | ||
| filter.serialNumber === device.serialNumber; |
There was a problem hiding this comment.
comment: this seems super strict, can we relax the matching conditions to only vendorId/productId/serialNumber?
| > { | ||
| version(iv: FromV): ToV; | ||
| transform(fs: Partial<FromState>): ToState | Promise<ToState>; | ||
| transform(fs: Partial<FromState>): Explicit<ToState> | Promise<Explicit<ToState>>; |
There was a problem hiding this comment.
comment: explicit flag here requires older migrations set optional keys to undefined to satisfy the type. don't know what the implication of that is, but i'd rather not change this. let's revert and remove absent values that are now explicitly required.
| * @param passKey the correct key | ||
| * @throws Error if the key is incorrect | ||
| */ | ||
| protected async unseal(passKey: Key) { |
There was a problem hiding this comment.
comment: doesn't matter. but let's mark this as private.
| * @param to instance to which the implementations shall be bound | ||
| * @param unsealed decrypted data from the wallet's custody box | ||
| */ | ||
| export function bindCustodyImpl<T extends CustodyTypeName>( |
There was a problem hiding this comment.
comment: ack – glue layer that attaches to the right custody-specific impl.
2 participants
Adds foundational support for Ledger devices including storage schema updates, wallet model changes, and testing infrastructure. Onboarding and other UI is in #402
Key Changes
ledgerUsbcustody type in v3 schema with v2→v3 migrationWallet.fromJson()instead of direct protobuf parsingNotes
zondax/ledger-penumbracloned into a new directory rather than as a git submodule, as the makefiles require a.gitdirectory that isn't present in submodulesFiles Changed
packages/storage-chrome/src/versions/v3.ts- New custody type definitionpackages/storage-chrome/src/migrations/local-v2-v3.ts- Migration logicpackages/wallet/package.json- Ledger testing dependencies and scriptspackages/mock-chrome/src/mocks/storage-area.ts- Storage change eventsfromJsonStringwithfromJson