Skip to content

fix(types): widen constructor to accept ISigner (parent's contract)#88

Merged
claudiovb merged 1 commit into
mainfrom
fix/constructor-widen-to-isigner
Jul 2, 2026
Merged

fix(types): widen constructor to accept ISigner (parent's contract)#88
claudiovb merged 1 commit into
mainfrom
fix/constructor-widen-to-isigner

Conversation

@AlonzoRicardo

Copy link
Copy Markdown
Contributor

Summary

The abstract WalletManager (in @tetherto/wdk-wallet) accepts string | Uint8Array | ISigner. WalletManagerEvm's constructor accepts string | Uint8Array | ISignerEvm — stricter than the parent's contract, since ISignerEvm extends ISigner with extra required members (isDerivable, index, path, address, …).

That is a TypeScript subtyping violation:

error TS2345: Argument of type 'typeof WalletManagerEvm' is not assignable
              to parameter of type 'typeof WalletManager'.
  Types of parameters 'seedOrSigner' and 'signer' are incompatible.
    Type 'ISigner' is not assignable to type 'string | Uint8Array | ISignerEvm'.
      Type 'ISigner' is missing the following properties from type 'ISignerEvm':
        isDerivable, index, path, address, and 5 more.

which breaks wdk.registerWallet('ethereum', WalletManagerEvm, ...) (the standard consumer entry-point in @tetherto/wdk) under strict TypeScript. This is the second half of tetherto/wdk#72 — the getAccount half was already resolved by #40.

Fix

Widen the JSDoc @param type on the constructor from ISignerEvm to ISigner:

- * @param {string|Uint8Array|ISignerEvm} seedOrSigner - A BIP-39 seed phrase, seed bytes, or a root signer.
+ * @param {string|Uint8Array|ISigner} seedOrSigner - A BIP-39 seed phrase, seed bytes, or a root signer. Root signers must be derivable — non-derivable signers (e.g. private-key signers) can only be registered by name via `addSigner`.

Runtime behavior is unchanged: any signer whose isDerivable is falsy — which includes any plain ISigner without the EVM-specific extension — is already rejected by the existing if (!signer.isDerivable) guard. The widened type space does not admit any new successfully-constructed instances at runtime.

Scope

  • src/wallet-manager-evm.js: @typedef import + @param change
  • types/src/wallet-manager-evm.d.ts: surgically edited to mirror the source change; the rest of types/ is left as-is to avoid unrelated regeneration drift
  • tests/wallet-manager-evm.test.js: 1 new test asserting a bare ISigner (no isDerivable) is rejected at construction

Test plan

  • npm run lint — clean
  • npm test — 111/111 (was 110; +1 for the new rejection test)
  • npm run build:types — emits constructor(seedOrSigner: string | Uint8Array | ISigner, config?: EvmWalletConfig)
  • Verified against wdk-core's docs TypeScript snippet: the constructor subtyping error is resolved

Marked as draft

Opening as draft since Issue #72 has an established history and I want to leave room for feedback on the description text and the additional test before it goes to review.

The abstract WalletManager (in @tetherto/wdk-wallet) accepts
`string | Uint8Array | ISigner`. WalletManagerEvm's
constructor accepts `string | Uint8Array | ISignerEvm` —
stricter than the parent's contract, since ISignerEvm
extends ISigner with extra required members (isDerivable,
index, path, address, …).

That is a TypeScript subtyping violation: `typeof
WalletManagerEvm` is not assignable to `typeof
WalletManager`, which breaks
`wdk.registerWallet('ethereum', WalletManagerEvm, ...)`
under strict TS in consumers like @tetherto/wdk.

Widen the JSDoc @param to ISigner. Runtime behavior is
unchanged: any signer whose `isDerivable` is falsy (which
includes any plain ISigner without the EVM extension) is
already rejected by the existing `if (!signer.isDerivable)`
guard, so the widened type space does not admit any new
successfully-constructed instances at runtime.

Closes tetherto/wdk#72.
@claudiovb
claudiovb marked this pull request as ready for review July 2, 2026 12:49
@claudiovb
claudiovb merged commit 47682a3 into main Jul 2, 2026
6 checks passed
@jonathunne jonathunne mentioned this pull request Jul 2, 2026
5 tasks
@jonathunne jonathunne mentioned this pull request Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants