Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
fix: address some PR feedbacks
  • Loading branch information
mathieuartu committed Dec 11, 2025
commit aa6f016d7f5a36494aa78a3a6c379a0d16d94274
1 change: 0 additions & 1 deletion packages/keyring-eth-hd/src/hd-keyring-v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ export class HdKeyringV2
groupIndex: addressIndex,
derivationPath: `${this.inner.hdPath}/${addressIndex}`,
},
exportable: true,
},
};

Expand Down
9 changes: 4 additions & 5 deletions packages/keyring-eth-qr/src/qr-keyring-v2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,15 +243,14 @@ describe('QrKeyringV2', () => {
);
});

it('uses empty string for derivationPath when getPathFromAddress returns undefined', async () => {
it('throws error when getPathFromAddress returns undefined', async () => {
const { wrapper, inner } = await createWrapperWithAccounts(1);

jest.spyOn(inner, 'getPathFromAddress').mockReturnValue(undefined);

const accounts = await wrapper.getAccounts();
const account = getFirstAccount(accounts);

expect(account.options.entropy.derivationPath).toBe('');
await expect(wrapper.getAccounts()).rejects.toThrow(
/derivation path not found in keyring/u,
);
});
});

Expand Down
16 changes: 12 additions & 4 deletions packages/keyring-eth-qr/src/qr-keyring-v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,20 @@ export class QrKeyringV2
* @param addressIndex - The account index in the derivation path.
* @returns The created Bip44Account.
*/
#createAccount(
#createKeyringAccount(
address: Hex,
addressIndex: number,
): Bip44Account<KeyringAccount> {
const id = this.registry.register(address);

const derivationPath = this.inner.getPathFromAddress(address);

if (!derivationPath) {
throw new Error(
`Cannot create account for address ${address}: derivation path not found in keyring.`,
);
}

const account: Bip44Account<KeyringAccount> = {
id,
type: EthAccountType.Eoa,
Expand All @@ -110,7 +118,7 @@ export class QrKeyringV2
type: KeyringAccountEntropyTypeOption.Mnemonic,
id: this.entropySource,
groupIndex: addressIndex,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the addressIndex only make sense for DeviceMode.HD? Since for DeviceMode.ACCOUNT we're defaulting to the arrayIndex, this won't really match the real account/address index used during the derivation?

This might also be pretty hard to go from derivation path -> index if the "QR device" can use any arbitrary path. So I wonder if DeviceMode.ACCOUNT should be treated as private key here? 🤔

NOTE: I might have misunderstood the legacy QR keyring, so correct me if I'm wrong 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm a bit confused here as well. I'll set up something internally so we can share knowledge around this area 😅

derivationPath: this.inner.getPathFromAddress(address) ?? '',
derivationPath,
},
},
};
Expand Down Expand Up @@ -155,7 +163,7 @@ export class QrKeyringV2
addressIndex = arrayIndex;
}

return this.#createAccount(address, addressIndex);
return this.#createKeyringAccount(address, addressIndex);
});
}

Expand Down Expand Up @@ -202,7 +210,7 @@ export class QrKeyringV2
throw new Error('Failed to create new account');
}

const newAccount = this.#createAccount(newAddress, targetIndex);
const newAccount = this.#createKeyringAccount(newAddress, targetIndex);

return [newAccount];
});
Expand Down
1 change: 0 additions & 1 deletion packages/keyring-eth-simple/src/simple-keyring-v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ export class SimpleKeyringV2
entropy: {
type: KeyringAccountEntropyTypeOption.PrivateKey,
},
exportable: true,
},
};

Expand Down
Loading