fix: address PR feedbacks

Author: mathieuartu

packages/keyring-eth-trezor/jest.config.js

@@ -25,8 +25,8 @@ module.exports = merge(baseConfig, {
     global: {
       branches: 60.92,
       functions: 93.15,
-      lines: 93.5,
-      statements: 93.59,
+      lines: 93.4,
+      statements: 93.5,
     },
   },
 });

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

@@ -627,9 +627,9 @@ describe('TrezorKeyringV2', () => {
       expect(account.options.entropy.derivationPath).toBe(`m/44'/60'/0'/0`);
     });
 
-    it('clears registry when switching HD paths', async () => {
-      // Create wrapper with account on BIP44 path
-      const inner = createInnerKeyring();
+    it('clears registry when switching HD paths via createAccounts', async () => {
+      // Create wrapper with account on BIP44 standard path
+      const inner = createInnerKeyring(); // Uses m/44'/60'/0'/0 by default
       inner.setAccountToUnlock(0);
       await inner.addAccounts(1);
 
@@ -643,21 +643,60 @@ describe('TrezorKeyringV2', () => {
       expect(initialAccounts).toHaveLength(1);
       const initialAccountId = getFirstAccount(initialAccounts).id;
 
-      // Switch to legacy MEW path and add account
-      // This simulates what createAccounts does when path changes
-      inner.setHdPath(`m/44'/60'/0'`);
-      inner.hdk = fakeHdKey; // Reset HDKey after setHdPath clears it
-      inner.setAccountToUnlock(0);
-      await inner.addAccounts(1);
+      // Spy on registry.clear to verify it's called when path changes
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const clearSpy = jest.spyOn((wrapper as any).registry, 'clear');
+
+      // inner.hdPath is m/44'/60'/0'/0
+      // Call createAccounts with legacy MEW path (m/44'/60'/0') - this should trigger path change
+      // We need to reset the HDKey after setHdPath clears it inside createAccounts
+      const originalSetHdPath = inner.setHdPath.bind(inner);
+      jest.spyOn(inner, 'setHdPath').mockImplementation((path) => {
+        originalSetHdPath(path);
+        inner.hdk = fakeHdKey;
+      });
+
+      await wrapper.createAccounts(derivePathOptions(`m/44'/60'/0'/0`));
 
-      // Call createAccounts with the new path - this should clear registry
-      await wrapper.createAccounts(derivePathOptions(`m/44'/60'/0'/1`));
+      // Verify registry.clear was called
+      expect(clearSpy).toHaveBeenCalled();
 
-      // The old account should no longer be accessible
+      // The old account should no longer be accessible because registry was cleared
       await expect(wrapper.getAccount(initialAccountId)).rejects.toThrow(
         /Account not found/u,
       );
     });
+
+    it('does not clear registry when using same HD path', async () => {
+      // Create wrapper with account on BIP44 standard path
+      const inner = createInnerKeyring();
+      inner.setAccountToUnlock(0);
+      await inner.addAccounts(1);
+
+      const wrapper = new TrezorKeyringV2({
+        legacyKeyring: inner,
+        entropySource,
+      });
+
+      // Get initial accounts - this populates the registry
+      const initialAccounts = await wrapper.getAccounts();
+      expect(initialAccounts).toHaveLength(1);
+      const initialAccountId = getFirstAccount(initialAccounts).id;
+
+      // Spy on registry.clear
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const clearSpy = jest.spyOn((wrapper as any).registry, 'clear');
+
+      // Call createAccounts with same HD path base (m/44'/60'/0'/0) but different index
+      await wrapper.createAccounts(derivePathOptions(`m/44'/60'/0'/0/1`));
+
+      // Verify registry.clear was NOT called
+      expect(clearSpy).not.toHaveBeenCalled();
+
+      // The original account should still be accessible
+      const account = await wrapper.getAccount(initialAccountId);
+      expect(account).toBeDefined();
+    });
   });
 
   describe('locked device handling', () => {

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

@@ -88,20 +88,13 @@ export class TrezorKeyringV2
 {
   readonly entropySource: EntropySourceId;
 
-  /**
-   * Tracks the current HD path to detect path changes.
-   * When the path changes, the registry must be cleared.
-   */
-  #currentHdPath: AllowedHdPath;
-
   constructor(options: TrezorKeyringV2Options) {
     super({
       type: options.type ?? KeyringType.Trezor,
       inner: options.legacyKeyring as TrezorKeyringAsEthKeyring,
       capabilities: trezorKeyringV2Capabilities,
     });
     this.entropySource = options.entropySource;
-    this.#currentHdPath = this.inner.hdPath as AllowedHdPath;
   }
 
   /**
@@ -130,9 +123,6 @@ export class TrezorKeyringV2
       // initialized HDKey. The registry will be populated lazily when
       // getAccounts() is called after the device is unlocked.
       await this.inner.deserialize(state);
-
-      // Sync the tracked HD path with the deserialized state
-      this.#currentHdPath = this.inner.hdPath as AllowedHdPath;
     });
   }
 
@@ -304,9 +294,8 @@ export class TrezorKeyringV2
       // If the HD path is changing, clear the registry to avoid stale accounts.
       // The TrezorKeyring operates on a single path at a time - accounts from
       // different paths cannot coexist in the inner keyring.
-      if (basePath !== this.#currentHdPath) {
+      if (basePath !== this.inner.hdPath) {
         this.registry.clear();
-        this.#currentHdPath = basePath;
       }
 
       this.inner.setHdPath(basePath);

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

@@ -203,7 +203,7 @@ export class TrezorKeyring implements Keyring {
           const newAccounts: Hex[] = [];
 
           for (let i = from; i < to; i++) {
-            const address = this.addressFromIndex(pathBase, i);
+            const address = this.#addressFromIndex(pathBase, i);
             if (!this.accounts.includes(address)) {
               this.accounts = [...this.accounts, address];
               newAccounts.push(address);
@@ -247,7 +247,7 @@ export class TrezorKeyring implements Keyring {
           const accounts = [];
 
           for (let i = from; i < to; i++) {
-            const address = this.addressFromIndex(pathBase, i);
+            const address = this.#addressFromIndex(pathBase, i);
             accounts.push({
               address,
               balance: null,
@@ -565,7 +565,7 @@ export class TrezorKeyring implements Keyring {
    * @param i - The derivation index.
    * @returns The checksummed address.
    */
-  protected addressFromIndex(basePath: string, i: number): Hex {
+  #addressFromIndex(basePath: string, i: number): Hex {
     const dkey = this.hdk.derive(`${basePath}/${i}`);
     const address = bytesToHex(publicToAddress(dkey.publicKey, true));
     return toChecksumAddress(address);
@@ -587,7 +587,7 @@ export class TrezorKeyring implements Keyring {
 
     if (typeof index === 'undefined') {
       for (let i = 0; i < MAX_INDEX; i++) {
-        if (checksummedAddress === this.addressFromIndex(pathBase, i)) {
+        if (checksummedAddress === this.#addressFromIndex(pathBase, i)) {
           index = i;
           break;
         }