fix: address review comments on PR #95

- export ISignerEvm only from src/signers/index.js
- drop redundant @extends tag on ISignerEvm
- path/index return null on PrivateKeySignerEvm (null convention,
  aligned in ISignerEvm, WalletAccountEvm and wdk-wallet's IWalletAccount)
- unify path/index descriptions across signer docs
- reorder test cases so both signer suites follow the same order

Deferred for the ongoing interface discussion:
- moving sign() to ISigner: #95 (comment)
- derive() doc spec alignment: #95 (comment)

Author: claudiovb

index.js

@@ -40,5 +40,3 @@ export { default as WalletAccountReadOnlyEvm } from './src/wallet-account-read-o
 export { default as WalletAccountEvm } from './src/wallet-account-evm.js'
 
 export { ISigner } from '@tetherto/wdk-wallet'
-
-export { ISignerEvm } from './src/signers/signer-evm.js'

src/signers/private-key-signer-evm.js

@@ -53,8 +53,6 @@ export default class PrivateKeySignerEvm {
     this._wallet = new BaseWallet(this._signingKey, null)
     /** @private */
     this._address = this._wallet.address
-    /** @private */
-    this._path = undefined
   }
 
   /**
@@ -66,19 +64,18 @@ export default class PrivateKeySignerEvm {
   get isDerivable () { return false }
 
   /**
-   * The account index. Always undefined for private key signers: a raw key has no
-   * BIP-44 position, so reporting an index would be misleading.
+   * The last component index of the BIP 0044 derivation path.
    *
-   * @type {number|undefined}
+   * @type {number | null}
    */
-  get index () { return undefined }
+  get index () { return null }
 
   /**
-   * The derivation path. Always undefined for private key signers.
+   * The BIP 0044 derivation path.
    *
-   * @type {string|undefined}
+   * @type {string | null}
    */
-  get path () { return this._path }
+  get path () { return null }
 
   /**
    * The account's address.

src/signers/seed-signer-evm.js

@@ -99,7 +99,7 @@ export default class SeedSignerEvm {
   }
 
   /**
-   * The last component index of the derivation path.
+   * The last component index of the BIP 0044 derivation path.
    *
    * @type {number}
    */
@@ -108,7 +108,7 @@ export default class SeedSignerEvm {
   }
 
   /**
-   * The full derivation path of this signer's account.
+   * The BIP 0044 derivation path.
    *
    * @type {string}
    */

src/signers/signer-evm.js

@@ -23,23 +23,22 @@ import { ISigner, NotImplementedError } from '@tetherto/wdk-wallet'
 /**
  * Interface for EVM signers, extending the base `ISigner` from `@tetherto/wdk-wallet`.
  *
- * @extends {ISigner}
  * @interface
  */
 export class ISignerEvm extends ISigner {
   /**
-   * The last component index for the derivation path of this signer, when applicable.
+   * The last component index of the BIP 0044 derivation path.
    *
-   * @type {number | undefined}
+   * @type {number | null}
    */
   get index () {
     throw new NotImplementedError('index')
   }
 
   /**
-   * The full derivation path of this signer's account, when applicable.
+   * The BIP 0044 derivation path.
    *
-   * @type {string | undefined}
+   * @type {string | null}
    */
   get path () {
     throw new NotImplementedError('path')

src/wallet-account-evm.js

@@ -89,20 +89,20 @@ export default class WalletAccountEvm extends WalletAccountReadOnlyEvm {
   }
 
   /**
-   * The derivation path's index of this account, or undefined if the account's
+   * The derivation path's index of this account, or null if the account's
    * signer is not bound to a BIP-44 position (e.g. private-key signers).
    *
-   * @type {number | undefined}
+   * @type {number | null}
    */
   get index () {
     return this._signer.index
   }
 
   /**
    * The derivation path of this account (see [BIP-44](https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki)),
-   * or undefined if the account's signer is not bound to a BIP-44 position (e.g. private-key signers).
+   * or null if the account's signer is not bound to a BIP-44 position (e.g. private-key signers).
    *
-   * @type {string | undefined}
+   * @type {string | null}
    */
   get path () {
     return this._signer.path

tests/signers/private-key-signer-evm.test.js

@@ -60,8 +60,8 @@ describe('PrivateKeySignerEvm', () => {
 
       expect(signer.isDerivable).toBe(false)
       expect(signer.address).toBe(EXPECTED_ADDRESS)
-      expect(signer.path).toBeUndefined()
-      expect(signer.index).toBeUndefined()
+      expect(signer.path).toBeNull()
+      expect(signer.index).toBeNull()
 
       signer.dispose()
     })

tests/signers/seed-signer-evm.test.js

@@ -59,20 +59,6 @@ const EXPECTED_TYPED_DATA_SIGNATURE = '0xd5d54d9a7fe501ab5dc1532a443a4f70bc8b6ad
 
 describe('SeedSignerEvm', () => {
   describe('constructor', () => {
-    test('should throw if the seed phrase is invalid', () => {
-      expect(() => { new SeedSignerEvm('invalid seed phrase') }) // eslint-disable-line no-new
-        .toThrow('The seed phrase is invalid.')
-    })
-
-    test('should throw if both seed and root are provided', async () => {
-      const root = new SeedSignerEvm(VALID_SEED_PHRASE)
-      const child = await root.derive("0'/0/0")
-      expect(() => { new SeedSignerEvm(VALID_SEED_PHRASE, { root: child }) }) // eslint-disable-line no-new
-        .toThrow('Provide either a seed or a root, not both.')
-      child.dispose()
-      root.dispose()
-    })
-
     test('should create a signer with the account at index 0 by default', () => {
       const signer = new SeedSignerEvm(VALID_SEED_PHRASE)
 
@@ -99,6 +85,20 @@ describe('SeedSignerEvm', () => {
 
       signer.dispose()
     })
+
+    test('should throw if the seed phrase is invalid', () => {
+      expect(() => { new SeedSignerEvm('invalid seed phrase') }) // eslint-disable-line no-new
+        .toThrow('The seed phrase is invalid.')
+    })
+
+    test('should throw if both seed and root are provided', async () => {
+      const root = new SeedSignerEvm(VALID_SEED_PHRASE)
+      const child = await root.derive("0'/0/0")
+      expect(() => { new SeedSignerEvm(VALID_SEED_PHRASE, { root: child }) }) // eslint-disable-line no-new
+        .toThrow('Provide either a seed or a root, not both.')
+      child.dispose()
+      root.dispose()
+    })
   })
 
   describe('keyPair', () => {
@@ -113,15 +113,6 @@ describe('SeedSignerEvm', () => {
   })
 
   describe('derive', () => {
-    test('should throw if the path is invalid', async () => {
-      const signer = new SeedSignerEvm(VALID_SEED_PHRASE)
-
-      await expect(signer.derive("a'/b/c"))
-        .rejects.toThrow('invalid path component')
-
-      signer.dispose()
-    })
-
     test('should derive a child signer with the correct address and path', async () => {
       const root = new SeedSignerEvm(VALID_SEED_PHRASE)
       const child = await root.derive("0'/0/0")
@@ -137,6 +128,15 @@ describe('SeedSignerEvm', () => {
       root.dispose()
     })
 
+    test('should throw if the path is invalid', async () => {
+      const signer = new SeedSignerEvm(VALID_SEED_PHRASE)
+
+      await expect(signer.derive("a'/b/c"))
+        .rejects.toThrow('invalid path component')
+
+      signer.dispose()
+    })
+
     test('should throw when deriving from a disposed signer', async () => {
       const root = new SeedSignerEvm(VALID_SEED_PHRASE)
       root.dispose()

types/index.d.ts

@@ -2,7 +2,6 @@ export { default } from "./src/wallet-manager-evm.js";
 export { default as WalletAccountReadOnlyEvm } from "./src/wallet-account-read-only-evm.js";
 export { default as WalletAccountEvm } from "./src/wallet-account-evm.js";
 export { ISigner } from "@tetherto/wdk-wallet";
-export { ISignerEvm } from "./src/signers/signer-evm.js";
 export type TypedDataDomain = import("ethers").TypedDataDomain;
 export type TypedDataField = import("ethers").TypedDataField;
 export type AuthorizationRequest = import("ethers").AuthorizationRequest;

types/src/signers/private-key-signer-evm.d.ts

@@ -18,8 +18,6 @@ export default class PrivateKeySignerEvm implements ISignerEvm {
     private _wallet;
     /** @private */
     private _address;
-    /** @private */
-    private _path;
     /**
      * Whether this signer can derive child signers. Always false: a private-key signer is a
      * single standalone account and is bound directly to a wallet account.
@@ -28,18 +26,17 @@ export default class PrivateKeySignerEvm implements ISignerEvm {
      */
     get isDerivable(): boolean;
     /**
-     * The account index. Always undefined for private key signers: a raw key has no
-     * BIP-44 position, so reporting an index would be misleading.
+     * The last component index of the BIP 0044 derivation path.
      *
-     * @type {number|undefined}
+     * @type {number | null}
      */
-    get index(): number | undefined;
+    get index(): number | null;
     /**
-     * The derivation path. Always undefined for private key signers.
+     * The BIP 0044 derivation path.
      *
-     * @type {string|undefined}
+     * @type {string | null}
      */
-    get path(): string | undefined;
+    get path(): string | null;
     /**
      * The account's address.
      *

types/src/signers/seed-signer-evm.d.ts

@@ -32,13 +32,13 @@ export default class SeedSignerEvm implements ISignerEvm {
      */
     get isDerivable(): boolean;
     /**
-     * The last component index of the derivation path.
+     * The last component index of the BIP 0044 derivation path.
      *
      * @type {number}
      */
     get index(): number;
     /**
-     * The full derivation path of this signer's account.
+     * The BIP 0044 derivation path.
      *
      * @type {string}
      */