Skip to content

Commit 685acf4

Browse files
committed
fix: PR feedbacks + remove #mode cache
1 parent 3a51d54 commit 685acf4

3 files changed

Lines changed: 16 additions & 37 deletions

File tree

packages/keyring-eth-qr/src/qr-keyring-v2.test.ts

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ describe('QrKeyringV2', () => {
218218
});
219219

220220
describe('getAccounts', () => {
221-
it('updates capabilities for pre-paired Account mode device', async () => {
221+
it('returns correct capabilities immediately for pre-paired Account mode device', async () => {
222222
const inner = new QrKeyring({
223223
bridge: getMockBridge(),
224224
ur: KNOWN_CRYPTO_ACCOUNT_UR,
@@ -230,19 +230,9 @@ describe('QrKeyringV2', () => {
230230
entropySource: ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS.xfp,
231231
});
232232

233-
// Before getAccounts, capabilities are default HD mode
234-
expect(wrapper.capabilities.custom).toBeUndefined();
235-
236-
// After getAccounts, capabilities are updated to Account mode
237-
await wrapper.getAccounts();
238-
233+
// Capabilities are correct immediately - no need to call getAccounts first
239234
expect(wrapper.capabilities).toStrictEqual({
240235
scopes: [EthScope.Eoa],
241-
bip44: {
242-
deriveIndex: false,
243-
derivePath: false,
244-
discover: false,
245-
},
246236
custom: {
247237
createAccounts: true,
248238
},
@@ -384,11 +374,6 @@ describe('QrKeyringV2', () => {
384374
// After deserializing Account mode state, capabilities should update
385375
expect(wrapper.capabilities).toStrictEqual({
386376
scopes: [EthScope.Eoa],
387-
bip44: {
388-
deriveIndex: false,
389-
derivePath: false,
390-
discover: false,
391-
},
392377
custom: {
393378
createAccounts: true,
394379
},

packages/keyring-eth-qr/src/qr-keyring-v2.ts

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,6 @@ const HD_MODE_CAPABILITIES: KeyringCapabilities = {
4646
*/
4747
const ACCOUNT_MODE_CAPABILITIES: KeyringCapabilities = {
4848
scopes: [EthScope.Eoa],
49-
bip44: {
50-
deriveIndex: false,
51-
derivePath: false,
52-
discover: false,
53-
},
5449
custom: {
5550
createAccounts: true,
5651
},
@@ -105,12 +100,6 @@ export class QrKeyringV2
105100
{
106101
readonly entropySource: EntropySourceId;
107102

108-
/**
109-
* Cached device mode for synchronous capabilities lookup.
110-
* Updated during deserialization and device state queries.
111-
*/
112-
#mode: DeviceMode | undefined;
113-
114103
constructor(options: QrKeyringV2Options) {
115104
super({
116105
type: KeyringType.Qr,
@@ -124,7 +113,7 @@ export class QrKeyringV2
124113
// that returns capabilities based on the current device mode.
125114
Object.defineProperty(this, 'capabilities', {
126115
get: (): KeyringCapabilities => {
127-
return this.#mode === DeviceMode.ACCOUNT
116+
return this.inner.getMode() === DeviceMode.ACCOUNT
128117
? ACCOUNT_MODE_CAPABILITIES
129118
: HD_MODE_CAPABILITIES;
130119
},
@@ -153,20 +142,17 @@ export class QrKeyringV2
153142
const state = await this.inner.serialize();
154143

155144
if (!state?.initialized) {
156-
this.#mode = undefined;
157145
return undefined;
158146
}
159147

160148
if (state.keyringMode === DeviceMode.ACCOUNT) {
161-
this.#mode = DeviceMode.ACCOUNT;
162149
return {
163150
mode: DeviceMode.ACCOUNT,
164151
indexes: state.indexes,
165152
paths: state.paths,
166153
};
167154
}
168155

169-
this.#mode = DeviceMode.HD;
170156
return {
171157
mode: DeviceMode.HD,
172158
indexes: state.indexes,
@@ -276,7 +262,7 @@ export class QrKeyringV2
276262
}
277263

278264
// Account mode: validate address exists in paths map
279-
const { paths } = deviceState as { paths: Record<Hex, string> };
265+
const { paths } = deviceState;
280266
if (paths[address] === undefined) {
281267
throw new Error(
282268
`Address ${address} not found in device paths. This indicates an inconsistent keyring state.`,
@@ -355,12 +341,12 @@ export class QrKeyringV2
355341
);
356342
}
357343

358-
const address = availableAddresses[addressIndex] as Hex;
344+
const address = availableAddresses[addressIndex];
359345

360346
// Check if already exists
361347
const currentAccounts = await this.getAccounts();
362348
const existingAccount = currentAccounts.find(
363-
(account) => account.address.toLowerCase() === address.toLowerCase(),
349+
(account) => account.address.toLowerCase() === address?.toLowerCase(),
364350
);
365351
if (existingAccount) {
366352
return [existingAccount];
@@ -412,8 +398,7 @@ export class QrKeyringV2
412398
(account) =>
413399
account.options.entropy?.type ===
414400
KeyringAccountEntropyTypeOption.Mnemonic &&
415-
(account.options.entropy as { groupIndex: number }).groupIndex ===
416-
targetIndex,
401+
account.options.entropy.groupIndex === targetIndex,
417402
);
418403

419404
if (existingAccount) {

packages/keyring-eth-qr/src/qr-keyring.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,15 @@ export class QrKeyring implements Keyring {
253253
return source.name;
254254
}
255255

256+
/**
257+
* Get the mode of the paired device.
258+
*
259+
* @returns The device mode, or undefined if no device is paired.
260+
*/
261+
getMode(): DeviceMode | undefined {
262+
return this.#device?.getDeviceDetails().keyringMode;
263+
}
264+
256265
/**
257266
* Fetch the first page of accounts. If the keyring is not currently initialized,
258267
* it will trigger a scan request to initialize it.

0 commit comments

Comments
 (0)