From f644dbfff91a5a8262c162f4a21be82b9f572c98 Mon Sep 17 00:00:00 2001 From: Eugene Chybisov Date: Fri, 18 Sep 2026 18:20:08 +0200 Subject: [PATCH 1/9] fix(client): detect Binance injected only as window.unisat `getInternalProvider` returned as soon as `binancew3w` existed, so a build that injects `binancew3w` without a `bitcoin` provider resolved undefined and never reached the `window.unisat.isBinance` fallback. The LI.FI widget's own detection accepted both paths, so the two disagreed and Binance could be listed while this connector could not drive it. Same shape as the BitKeep fix in #77, and the widget is removing its duplicate detection in favour of asking the connector. `unisat()` still rejects `isBinance`, so the two cannot claim the same injection. --- .changeset/binance-unisat-injection.md | 5 +++ .../client/src/connectors/binance.spec.ts | 35 +++++++++++++++++++ packages/client/src/connectors/binance.ts | 13 ++++--- 3 files changed, 46 insertions(+), 7 deletions(-) create mode 100644 .changeset/binance-unisat-injection.md create mode 100644 packages/client/src/connectors/binance.spec.ts diff --git a/.changeset/binance-unisat-injection.md b/.changeset/binance-unisat-injection.md new file mode 100644 index 0000000..8953139 --- /dev/null +++ b/.changeset/binance-unisat-injection.md @@ -0,0 +1,5 @@ +--- +'@bigmi/client': patch +--- + +Detect Binance when `window.binancew3w` carries no bitcoin provider but `window.unisat` reports `isBinance`. The check returned early on `binancew3w` alone, so that build fell through and Binance was reported unavailable. diff --git a/packages/client/src/connectors/binance.spec.ts b/packages/client/src/connectors/binance.spec.ts new file mode 100644 index 0000000..163103c --- /dev/null +++ b/packages/client/src/connectors/binance.spec.ts @@ -0,0 +1,35 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { binance } from './binance.js' + +afterEach(() => { + ;(globalThis as any).window = undefined +}) + +describe('binance getInternalProvider', () => { + it('resolves the Binance Web3 Wallet bitcoin provider', async () => { + const provider = { requestAccounts: async () => [] } + ;(globalThis as any).window = { binancew3w: { bitcoin: provider } } + const connector: any = binance()({} as any) + await expect(connector.getInternalProvider()).resolves.toBe(provider) + }) + + it('falls back to window.unisat when binancew3w carries no bitcoin provider', async () => { + const provider = { isBinance: true, requestAccounts: async () => [] } + ;(globalThis as any).window = { binancew3w: {}, unisat: provider } + const connector: any = binance()({} as any) + await expect(connector.getInternalProvider()).resolves.toBe(provider) + }) + + it('resolves window.unisat when only that is injected', async () => { + const provider = { isBinance: true, requestAccounts: async () => [] } + ;(globalThis as any).window = { unisat: provider } + const connector: any = binance()({} as any) + await expect(connector.getInternalProvider()).resolves.toBe(provider) + }) + + it('ignores a window.unisat that is not Binance', async () => { + ;(globalThis as any).window = { unisat: { isBitKeep: true } } + const connector: any = binance()({} as any) + await expect(connector.getInternalProvider()).resolves.toBeUndefined() + }) +}) diff --git a/packages/client/src/connectors/binance.ts b/packages/client/src/connectors/binance.ts index af4c5f8..960059e 100644 --- a/packages/client/src/connectors/binance.ts +++ b/packages/client/src/connectors/binance.ts @@ -97,15 +97,14 @@ export function binance( if (typeof window === 'undefined') { return } - if ('binancew3w' in window) { - const anyWindow: any = window + const anyWindow: any = window + if ('binancew3w' in window && anyWindow.binancew3w?.bitcoin) { return anyWindow.binancew3w.bitcoin } - if ('unisat' in window) { - const anyWindow: any = window - if (anyWindow.unisat.isBinance) { - return anyWindow.unisat - } + // Binance also ships as the sole `window.unisat` injection on some builds, + // and on others alongside a `binancew3w` that carries no bitcoin provider. + if ('unisat' in window && anyWindow.unisat?.isBinance) { + return anyWindow.unisat } }, async getProvider() { From 5dc5d3606c2dc86a927be10eea8d4cb789df979e Mon Sep 17 00:00:00 2001 From: Eugene Chybisov Date: Mon, 21 Sep 2026 11:09:13 +0200 Subject: [PATCH 2/9] fix(client): do not open MetaMask Bitcoin while reconnecting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `connect()` took no parameters, so it ignored `isReconnecting`, and it read accounts through `getAccounts()` — which calls the interactive `bitcoin:connect`. `reconnect()` runs on every mount and `isAuthorized()` returns true whenever the connected shim is set, so a returning user with a locked MetaMask got an unsolicited prompt on page load, and a rejection surfaced as `UserRejectedRequestError`. Read the session the wallet already restored when reconnecting, and keep the interactive path for a user-initiated connect. This is the same treatment `unisat`, `okx`, `binance`, `bitget` and `onekey` already have. This matters now because lifinance/widget#878 makes `metamask()` a default Bitcoin connector, which would have exposed every integrator to the prompt. --- .changeset/metamask-passive-reconnect.md | 5 ++ .../client/src/connectors/metamask.spec.ts | 56 +++++++++++++++++++ packages/client/src/connectors/metamask.ts | 18 +++++- 3 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 .changeset/metamask-passive-reconnect.md create mode 100644 packages/client/src/connectors/metamask.spec.ts diff --git a/.changeset/metamask-passive-reconnect.md b/.changeset/metamask-passive-reconnect.md new file mode 100644 index 0000000..2ca490f --- /dev/null +++ b/.changeset/metamask-passive-reconnect.md @@ -0,0 +1,5 @@ +--- +'@bigmi/client': patch +--- + +Stop MetaMask Bitcoin from opening the extension on page load. `connect()` ignored the `isReconnecting` flag and read accounts through the interactive `bitcoin:connect`, so `reconnect()` prompted a returning user whose wallet was locked. It now reads the session the wallet already restored, matching `unisat`, `okx`, `binance`, `bitget` and `onekey`. diff --git a/packages/client/src/connectors/metamask.spec.ts b/packages/client/src/connectors/metamask.spec.ts new file mode 100644 index 0000000..8289048 --- /dev/null +++ b/packages/client/src/connectors/metamask.spec.ts @@ -0,0 +1,56 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { metamask } from './metamask.js' + +const address = 'bc1q8h8s4zd9y0lkrx334aqnj4ykqs220ss735a3gh' +const publicKey = new Uint8Array([3, 203, 174, 220]) + +const account = { address, publicKey } + +function createWallet(connectSpy: ReturnType) { + return { + name: 'MetaMask', + accounts: [account], + features: { + 'bitcoin:connect': { connect: connectSpy }, + 'bitcoin:events': { on: vi.fn(() => () => {}) }, + }, + } +} + +afterEach(() => { + ;(globalThis as any).window = undefined +}) + +describe('metamask connector reconnect', () => { + it('does not open the wallet while reconnecting', async () => { + const connectSpy = vi.fn(async () => ({ accounts: [account] })) + const wallet = createWallet(connectSpy) + ;(globalThis as any).window = {} + const connector: any = metamask()({ + emitter: { emit: vi.fn() }, + storage: { setItem: vi.fn(), removeItem: vi.fn(), getItem: vi.fn() }, + } as any) + connector.getInternalProvider = async () => wallet + + const result = await connector.connect({ isReconnecting: true }) + + // `bitcoin:connect` opens MetaMask. Reconnect runs on page load, so it + // must read the restored session instead. + expect(connectSpy).not.toHaveBeenCalled() + expect(result.accounts.map((a: any) => a.address)).toEqual([address]) + }) + + it('opens the wallet for a user-initiated connect', async () => { + const connectSpy = vi.fn(async () => ({ accounts: [account] })) + const wallet = createWallet(connectSpy) + ;(globalThis as any).window = {} + const connector: any = metamask()({ + emitter: { emit: vi.fn() }, + storage: { setItem: vi.fn(), removeItem: vi.fn(), getItem: vi.fn() }, + } as any) + connector.getInternalProvider = async () => wallet + + await connector.connect() + expect(connectSpy).toHaveBeenCalledOnce() + }) +}) diff --git a/packages/client/src/connectors/metamask.ts b/packages/client/src/connectors/metamask.ts index 46f0ac6..f78478e 100644 --- a/packages/client/src/connectors/metamask.ts +++ b/packages/client/src/connectors/metamask.ts @@ -9,7 +9,10 @@ import { } from '@bigmi/core' import { getWallets } from '@wallet-standard/app' import type { Wallet, WalletAccount } from '@wallet-standard/base' -import { ConnectorChainIdDetectionError } from '../errors/connectors.js' +import { + ConnectorChainIdDetectionError, + ConnectorNotConnectedError, +} from '../errors/connectors.js' import { createConnector } from '../factories/createConnector.js' import type { CreateConnectorFn } from '../types/connector.js' import type { @@ -184,13 +187,22 @@ export function metamask( throw new MethodNotSupportedRpcError(method) } }, - async connect() { + async connect({ isReconnecting } = {}) { const wallet = await this.getInternalProvider() if (!wallet) { throw new ProviderNotFoundError() } try { - const accounts = await this.getAccounts() + // `bitcoin:connect` opens MetaMask, and reconnect runs on app mount, + // so it must only read the session the wallet already restored. + const accounts = isReconnecting + ? wallet.accounts + .map((account) => toAccount(account as WalletAccount)) + .filter((account) => account.purpose === 'payment') + : await this.getAccounts() + if (accounts.length === 0) { + throw new ConnectorNotConnectedError() + } const chainId = getAddressChainId(accounts[0].address) if (!unsubscribe) { From d21c8e3d0fa4f9960e4ce9e43ca269050086d958 Mon Sep 17 00:00:00 2001 From: Eugene Chybisov Date: Mon, 21 Sep 2026 11:33:05 +0200 Subject: [PATCH 3/9] fix(client): wait for MetaMask's restored session, and classify its absence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two problems with the passive reconnect path. `wallet.accounts` is filled by a session lookup the wallet does not await, so it reads empty from registration until that round trip resolves. `reconnect` polls only for the provider, which appears the moment the wallet enters the registry, so a wallet registering after the app mounts reported no session and nothing recovered it — the `bitcoin:events` subscription is created after the length check, so the later change event reached no listener. Poll the accounts too, for up to a second: one extension round trip, and `reconnect` already spends up to five seconds finding the provider. `ConnectorNotConnectedError` was thrown inside the `try` whose `catch` rethrows everything as `UserRejectedRequestError`, so a returning user with no session was told they had rejected a request they never saw. Throw it outside, as `unisat` and `okx` do. The earlier tests could not catch either: the fake wallet hardcoded `accounts`, which is the field whose timing is the bug. It now restores asynchronously, and a fourth case covers the absent session. --- .changeset/metamask-passive-reconnect.md | 2 + .../client/src/connectors/metamask.spec.ts | 73 +++++++++++++++---- packages/client/src/connectors/metamask.ts | 36 ++++++--- 3 files changed, 87 insertions(+), 24 deletions(-) diff --git a/.changeset/metamask-passive-reconnect.md b/.changeset/metamask-passive-reconnect.md index 2ca490f..036bc8f 100644 --- a/.changeset/metamask-passive-reconnect.md +++ b/.changeset/metamask-passive-reconnect.md @@ -3,3 +3,5 @@ --- Stop MetaMask Bitcoin from opening the extension on page load. `connect()` ignored the `isReconnecting` flag and read accounts through the interactive `bitcoin:connect`, so `reconnect()` prompted a returning user whose wallet was locked. It now reads the session the wallet already restored, matching `unisat`, `okx`, `binance`, `bitget` and `onekey`. + +The wallet fills its `accounts` from a session lookup it does not await, so that read is polled for up to a second rather than taken as a first snapshot — otherwise a wallet that registers after the app mounts reports no session and never recovers. An absent session now rejects with `ConnectorNotConnectedError` instead of `UserRejectedRequestError`, since nothing was shown to the user. diff --git a/packages/client/src/connectors/metamask.spec.ts b/packages/client/src/connectors/metamask.spec.ts index 8289048..182ee09 100644 --- a/packages/client/src/connectors/metamask.spec.ts +++ b/packages/client/src/connectors/metamask.spec.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, it, vi } from 'vitest' +import { ConnectorNotConnectedError } from '../errors/connectors.js' import { metamask } from './metamask.js' const address = 'bc1q8h8s4zd9y0lkrx334aqnj4ykqs220ss735a3gh' @@ -6,10 +7,23 @@ const publicKey = new Uint8Array([3, 203, 174, 220]) const account = { address, publicKey } -function createWallet(connectSpy: ReturnType) { +// The real MetaMaskWallet fills `accounts` from an un-awaited +// `#tryRestoringSession()`, so it reads empty until that round trip resolves. +function createWallet( + connectSpy: ReturnType, + { restoreAfterMs = 0 }: { restoreAfterMs?: number } = {} +) { + let restored = restoreAfterMs === 0 + if (restoreAfterMs > 0) { + setTimeout(() => { + restored = true + }, restoreAfterMs) + } return { name: 'MetaMask', - accounts: [account], + get accounts() { + return restored ? [account] : [] + }, features: { 'bitcoin:connect': { connect: connectSpy }, 'bitcoin:events': { on: vi.fn(() => () => {}) }, @@ -17,6 +31,15 @@ function createWallet(connectSpy: ReturnType) { } } +function createConnector(wallet: unknown) { + const connector: any = metamask()({ + emitter: { emit: vi.fn() }, + storage: { setItem: vi.fn(), removeItem: vi.fn(), getItem: vi.fn() }, + } as any) + connector.getInternalProvider = async () => wallet + return connector +} + afterEach(() => { ;(globalThis as any).window = undefined }) @@ -24,13 +47,8 @@ afterEach(() => { describe('metamask connector reconnect', () => { it('does not open the wallet while reconnecting', async () => { const connectSpy = vi.fn(async () => ({ accounts: [account] })) - const wallet = createWallet(connectSpy) ;(globalThis as any).window = {} - const connector: any = metamask()({ - emitter: { emit: vi.fn() }, - storage: { setItem: vi.fn(), removeItem: vi.fn(), getItem: vi.fn() }, - } as any) - connector.getInternalProvider = async () => wallet + const connector = createConnector(createWallet(connectSpy)) const result = await connector.connect({ isReconnecting: true }) @@ -40,15 +58,42 @@ describe('metamask connector reconnect', () => { expect(result.accounts.map((a: any) => a.address)).toEqual([address]) }) + it('waits for a session that is still being restored', async () => { + const connectSpy = vi.fn(async () => ({ accounts: [account] })) + ;(globalThis as any).window = {} + const connector = createConnector( + createWallet(connectSpy, { restoreAfterMs: 150 }) + ) + + const result = await connector.connect({ isReconnecting: true }) + + expect(connectSpy).not.toHaveBeenCalled() + expect(result.accounts.map((a: any) => a.address)).toEqual([address]) + }) + + it('reports an absent session as not connected, not as a rejection', async () => { + const connectSpy = vi.fn(async () => ({ accounts: [account] })) + ;(globalThis as any).window = {} + const connector = createConnector({ + name: 'MetaMask', + accounts: [], + features: { + 'bitcoin:connect': { connect: connectSpy }, + 'bitcoin:events': { on: vi.fn(() => () => {}) }, + }, + }) + + // Nothing was ever shown to the user, so this is not a user rejection. + await expect(connector.connect({ isReconnecting: true })).rejects.toThrow( + ConnectorNotConnectedError + ) + expect(connectSpy).not.toHaveBeenCalled() + }) + it('opens the wallet for a user-initiated connect', async () => { const connectSpy = vi.fn(async () => ({ accounts: [account] })) - const wallet = createWallet(connectSpy) ;(globalThis as any).window = {} - const connector: any = metamask()({ - emitter: { emit: vi.fn() }, - storage: { setItem: vi.fn(), removeItem: vi.fn(), getItem: vi.fn() }, - } as any) - connector.getInternalProvider = async () => wallet + const connector = createConnector(createWallet(connectSpy)) await connector.connect() expect(connectSpy).toHaveBeenCalledOnce() diff --git a/packages/client/src/connectors/metamask.ts b/packages/client/src/connectors/metamask.ts index f78478e..5771ac8 100644 --- a/packages/client/src/connectors/metamask.ts +++ b/packages/client/src/connectors/metamask.ts @@ -5,6 +5,7 @@ import { hexToUnit8Array, MethodNotSupportedRpcError, ProviderNotFoundError, + retryUntil, UserRejectedRequestError, } from '@bigmi/core' import { getWallets } from '@wallet-standard/app' @@ -192,17 +193,32 @@ export function metamask( if (!wallet) { throw new ProviderNotFoundError() } + // `bitcoin:connect` opens MetaMask, and reconnect runs on app mount, so + // it must only read the session the wallet already restored. The wallet + // fills `accounts` from an un-awaited session lookup, so poll rather + // than take the first snapshot. + const restored = isReconnecting + ? ((await retryUntil( + async () => { + const payment = wallet.accounts + .map((account) => toAccount(account as WalletAccount)) + .filter((account) => account.purpose === 'payment') + return payment.length > 0 ? payment : undefined + }, + // One extension round trip; `reconnect` already spends up to 5s + // finding the provider, so do not stack another long wait on it. + { timeout: 1000, interval: 50 } + )) ?? []) + : undefined + + // Outside the try: nothing was shown to the user, so this must not be + // reported as a rejection. + if (restored && restored.length === 0) { + throw new ConnectorNotConnectedError() + } + try { - // `bitcoin:connect` opens MetaMask, and reconnect runs on app mount, - // so it must only read the session the wallet already restored. - const accounts = isReconnecting - ? wallet.accounts - .map((account) => toAccount(account as WalletAccount)) - .filter((account) => account.purpose === 'payment') - : await this.getAccounts() - if (accounts.length === 0) { - throw new ConnectorNotConnectedError() - } + const accounts = restored ?? (await this.getAccounts()) const chainId = getAddressChainId(accounts[0].address) if (!unsubscribe) { From 0d97a37b4450b55e38839ba7aecc9aa962291481 Mon Sep 17 00:00:00 2001 From: Eugene Chybisov Date: Mon, 21 Sep 2026 11:50:52 +0200 Subject: [PATCH 4/9] fix(client): keep the store consistent when a disconnect fails `disconnect()` awaited `connector.disconnect()` before deleting the connection, so a throw skipped the delete, the listener cleanup and the state update alike. `xverse`, `oyl` and `leather` all throw `ProviderNotFoundError` once their provider is gone, and the surviving connection was then promoted to `current`: the store reported an account that could never sign, and every later disconnect hit the same dead entry. Detach it regardless and rethrow after the state settles, so the caller still learns it failed. Three follow-ups in the MetaMask connector, all on the reconnect path: - An account that cannot be parsed now drops out of the poll instead of ending it, which is the half-initialized case the poll exists to survive. - A failure while reconnecting keeps its own error type. Only the interactive path may report `UserRejectedRequestError`, since nothing is shown to the user otherwise. - `isAuthorized()` requires a restored account, as `unisat`, `okx`, `binance`, `bitget` and `onekey` do. The storage shim alone sent every returning user through the poll, even after revoking the site or locking the wallet. It reads the session rather than calling `getAccounts`, so it stays passive. --- .changeset/disconnect-state-consistency.md | 5 ++ .changeset/metamask-passive-reconnect.md | 2 + .../client/src/actions/disconnect.spec.ts | 75 +++++++++++++++++++ packages/client/src/actions/disconnect.ts | 27 +++++-- .../client/src/connectors/metamask.spec.ts | 28 +++++++ packages/client/src/connectors/metamask.ts | 32 +++++++- 6 files changed, 157 insertions(+), 12 deletions(-) create mode 100644 .changeset/disconnect-state-consistency.md create mode 100644 packages/client/src/actions/disconnect.spec.ts diff --git a/.changeset/disconnect-state-consistency.md b/.changeset/disconnect-state-consistency.md new file mode 100644 index 0000000..c0bf37d --- /dev/null +++ b/.changeset/disconnect-state-consistency.md @@ -0,0 +1,5 @@ +--- +'@bigmi/client': patch +--- + +Clear a connection even when the connector's `disconnect()` throws. `xverse`, `oyl` and `leather` throw `ProviderNotFoundError` once their provider is gone, which skipped the delete and left the connection to be promoted to `current` — so the store still reported an account that could never sign, and the next disconnect failed too. The state is now consistent and the error still reaches the caller. diff --git a/.changeset/metamask-passive-reconnect.md b/.changeset/metamask-passive-reconnect.md index 036bc8f..351843c 100644 --- a/.changeset/metamask-passive-reconnect.md +++ b/.changeset/metamask-passive-reconnect.md @@ -5,3 +5,5 @@ Stop MetaMask Bitcoin from opening the extension on page load. `connect()` ignored the `isReconnecting` flag and read accounts through the interactive `bitcoin:connect`, so `reconnect()` prompted a returning user whose wallet was locked. It now reads the session the wallet already restored, matching `unisat`, `okx`, `binance`, `bitget` and `onekey`. The wallet fills its `accounts` from a session lookup it does not await, so that read is polled for up to a second rather than taken as a first snapshot — otherwise a wallet that registers after the app mounts reports no session and never recovers. An absent session now rejects with `ConnectorNotConnectedError` instead of `UserRejectedRequestError`, since nothing was shown to the user. + +`isAuthorized()` now also requires a restored account, as `unisat`, `okx`, `binance`, `bitget` and `onekey` do, so a user who revoked the site or locked the wallet is not reconnected and does not pay for the poll. A failure while reconnecting keeps its own error type rather than becoming `UserRejectedRequestError`, and an account that cannot be parsed is skipped instead of ending the poll. diff --git a/packages/client/src/actions/disconnect.spec.ts b/packages/client/src/actions/disconnect.spec.ts new file mode 100644 index 0000000..cc6f2db --- /dev/null +++ b/packages/client/src/actions/disconnect.spec.ts @@ -0,0 +1,75 @@ +import { bitcoin, ChainId } from '@bigmi/core' +import { describe, expect, it } from 'vitest' +import { createConfig } from '../factories/createConfig.js' +import { disconnect } from './disconnect.js' + +const address = 'bc1q8h8s4zd9y0lkrx334aqnj4ykqs220ss735a3gh' + +const stubConnector = + (id: string, disconnectImpl: () => Promise) => (config: any) => ({ + id, + name: id, + type: 'UTXO' as const, + connect: async () => ({ + accounts: [ + { address, addressType: 'p2wpkh', publicKey: '00', purpose: 'payment' }, + ], + chainId: ChainId.BITCOIN_MAINNET, + }), + disconnect: disconnectImpl, + getAccounts: async () => [], + getChainId: async () => ChainId.BITCOIN_MAINNET, + getProvider: async () => ({}), + isAuthorized: async () => false, + onAccountsChanged: async () => {}, + onChainChanged: () => {}, + onDisconnect: async () => {}, + emitter: config.emitter, + }) + +function setup(disconnectImpl: () => Promise) { + const config = createConfig({ + chains: [bitcoin], + connectors: [stubConnector('xverse', disconnectImpl) as any], + client: () => ({}) as any, + }) as any + const connector = config.connectors[0] + config.setState((x: any) => ({ + ...x, + connections: new Map([ + [ + connector.uid, + { + accounts: [{ address }], + chainId: ChainId.BITCOIN_MAINNET, + connector, + }, + ], + ]), + current: connector.uid, + status: 'connected', + })) + return { config, connector } +} + +describe('disconnect', () => { + it('clears the connection when the connector disconnects', async () => { + const { config, connector } = setup(async () => {}) + await disconnect(config, { connector }) + expect(config.state.status).toBe('disconnected') + expect(config.state.connections.size).toBe(0) + }) + + it('clears the connection even when the connector throws', async () => { + // A provider that has gone away cannot be kept connected, or it is + // promoted to `current` and the account is reported as usable. + const { config, connector } = setup(async () => { + throw new Error('ProviderNotFoundError') + }) + await expect(disconnect(config, { connector })).rejects.toThrow( + 'ProviderNotFoundError' + ) + expect(config.state.connections.size).toBe(0) + expect(config.state.status).toBe('disconnected') + }) +}) diff --git a/packages/client/src/actions/disconnect.ts b/packages/client/src/actions/disconnect.ts index cae1bc0..730c9d9 100644 --- a/packages/client/src/actions/disconnect.ts +++ b/packages/client/src/actions/disconnect.ts @@ -24,8 +24,17 @@ export async function disconnect( const connections = config.state.connections + let disconnectError: unknown if (connector) { - await connector.disconnect() + // A provider that has gone away cannot be kept connected: leaving its + // connection in place promotes it to `current`, so the store still reports + // an account that can never sign. Detach it either way, and surface the + // failure to the caller once the state is consistent. + try { + await connector.disconnect() + } catch (error) { + disconnectError = error + } connector.emitter.off('change', config._internal.events.change) connector.emitter.off('disconnect', config._internal.events.disconnect) connector.emitter.on('connect', config._internal.events.connect) @@ -56,13 +65,15 @@ export async function disconnect( // Set recent connector if exists { const current = config.state.current - if (!current) { - return - } - const connector = config.state.connections.get(current)?.connector - if (!connector) { - return + const recent = current + ? config.state.connections.get(current)?.connector + : undefined + if (recent) { + await config.storage?.setItem('recentConnectorId', recent.id) } - await config.storage?.setItem('recentConnectorId', connector.id) + } + + if (disconnectError) { + throw disconnectError } } diff --git a/packages/client/src/connectors/metamask.spec.ts b/packages/client/src/connectors/metamask.spec.ts index 182ee09..3367e2b 100644 --- a/packages/client/src/connectors/metamask.spec.ts +++ b/packages/client/src/connectors/metamask.spec.ts @@ -98,4 +98,32 @@ describe('metamask connector reconnect', () => { await connector.connect() expect(connectSpy).toHaveBeenCalledOnce() }) + + it('skips an account it cannot parse instead of giving up', async () => { + const connectSpy = vi.fn(async () => ({ accounts: [account] })) + ;(globalThis as any).window = {} + const connector = createConnector({ + name: 'MetaMask', + // A half-initialized entry alongside a usable one. + accounts: [{ address: 'not-an-address' }, account], + features: { + 'bitcoin:connect': { connect: connectSpy }, + 'bitcoin:events': { on: vi.fn(() => () => {}) }, + }, + }) + + const result = await connector.connect({ isReconnecting: true }) + expect(result.accounts.map((a: any) => a.address)).toEqual([address]) + }) + + it('is not authorized when the session holds no account', async () => { + ;(globalThis as any).window = {} + const connector = createConnector({ + name: 'MetaMask', + accounts: [], + features: { 'bitcoin:events': { on: vi.fn(() => () => {}) } }, + }) + connector.config = undefined + await expect(connector.isAuthorized()).resolves.toBe(false) + }) }) diff --git a/packages/client/src/connectors/metamask.ts b/packages/client/src/connectors/metamask.ts index 5771ac8..bad8045 100644 --- a/packages/client/src/connectors/metamask.ts +++ b/packages/client/src/connectors/metamask.ts @@ -200,9 +200,16 @@ export function metamask( const restored = isReconnecting ? ((await retryUntil( async () => { - const payment = wallet.accounts - .map((account) => toAccount(account as WalletAccount)) - .filter((account) => account.purpose === 'payment') + // A half-initialized account can fail to parse. Skip it rather + // than ending the poll this exists to survive. + const payment = wallet.accounts.flatMap((account) => { + try { + const parsed = toAccount(account as WalletAccount) + return parsed.purpose === 'payment' ? [parsed] : [] + } catch { + return [] + } + }) return payment.length > 0 ? payment : undefined }, // One extension round trip; `reconnect` already spends up to 5s @@ -244,6 +251,11 @@ export function metamask( } return { accounts, chainId } } catch (error: any) { + // Nothing is shown to the user while reconnecting, so a failure there + // is not a rejection. + if (isReconnecting) { + throw error + } throw new UserRejectedRequestError(error.message) } }, @@ -291,7 +303,19 @@ export function metamask( shimDisconnect && // If shim exists in storage, connector is disconnected Boolean(await config.storage?.getItem(`${this.id}.connected`)) - return isConnected + if (!isConnected) { + return false + } + + const wallet = await this.getInternalProvider() + if (!wallet) { + return false + } + + // Reading the restored session here, rather than calling getAccounts, + // keeps this passive: a user who revoked the site or locked the wallet + // must not be reconnected, and must not pay for the poll in connect. + return wallet.accounts.length > 0 } catch { return false } From 1efb44410f0fa7305d01948410c82425e6bc5089 Mon Sep 17 00:00:00 2001 From: Eugene Chybisov Date: Mon, 21 Sep 2026 12:06:50 +0200 Subject: [PATCH 5/9] fix(client): keep isAuthorized off the racing session read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `isAuthorized()` had started requiring a restored account, to match the other connectors. For MetaMask that is wrong: the wallet fills `accounts` from a session lookup it does not await, and `reconnect` calls `isAuthorized()` in the same tick the wallet registers. It returned false on every reload, so `reconnect` skipped MetaMask and the wait added to `connect()` was never reached — reconnect was broken for the one wallet this work is about. Gate on the shim and the provider, and let `connect({ isReconnecting: true })` decide by waiting for the session. The test that was supposed to cover that branch never reached it: the storage stub resolved `undefined`, so `isAuthorized()` returned at the shim gate, and it passed identically with the branch deleted. The stub now reports the shim, and the restoring-session case fails if the accounts read comes back. `disconnect()` also clears the connector's shim when its `disconnect()` throws. The connector throws before writing its own, so the store said disconnected while storage still said connected, and the next reconnect would have restored what the user disconnected. The poll window moves to 1500ms with an accurate rationale: the provider lookup in `reconnect` returns as soon as the wallet registers, so this is the only meaningful wait inside the 5s `connect` budget. --- .changeset/disconnect-state-consistency.md | 2 +- .changeset/metamask-passive-reconnect.md | 2 +- .../client/src/actions/disconnect.spec.ts | 35 +++++++++++++++++ packages/client/src/actions/disconnect.ts | 9 +++++ .../client/src/connectors/metamask.spec.ts | 39 +++++++++++++++---- packages/client/src/connectors/metamask.ts | 22 +++++------ 6 files changed, 87 insertions(+), 22 deletions(-) diff --git a/.changeset/disconnect-state-consistency.md b/.changeset/disconnect-state-consistency.md index c0bf37d..cde4228 100644 --- a/.changeset/disconnect-state-consistency.md +++ b/.changeset/disconnect-state-consistency.md @@ -2,4 +2,4 @@ '@bigmi/client': patch --- -Clear a connection even when the connector's `disconnect()` throws. `xverse`, `oyl` and `leather` throw `ProviderNotFoundError` once their provider is gone, which skipped the delete and left the connection to be promoted to `current` — so the store still reported an account that could never sign, and the next disconnect failed too. The state is now consistent and the error still reaches the caller. +Clear a connection even when the connector's `disconnect()` throws. `xverse`, `oyl` and `leather` throw `ProviderNotFoundError` once their provider is gone, which skipped the delete and left the connection to be promoted to `current` — so the store still reported an account that could never sign, and the next disconnect failed too. The state is now consistent and the error still reaches the caller. The connector's own storage shim is cleared on that path too, so the store and storage cannot disagree and a later reconnect cannot silently restore what the user disconnected. diff --git a/.changeset/metamask-passive-reconnect.md b/.changeset/metamask-passive-reconnect.md index 351843c..c357ba6 100644 --- a/.changeset/metamask-passive-reconnect.md +++ b/.changeset/metamask-passive-reconnect.md @@ -6,4 +6,4 @@ Stop MetaMask Bitcoin from opening the extension on page load. `connect()` ignor The wallet fills its `accounts` from a session lookup it does not await, so that read is polled for up to a second rather than taken as a first snapshot — otherwise a wallet that registers after the app mounts reports no session and never recovers. An absent session now rejects with `ConnectorNotConnectedError` instead of `UserRejectedRequestError`, since nothing was shown to the user. -`isAuthorized()` now also requires a restored account, as `unisat`, `okx`, `binance`, `bitget` and `onekey` do, so a user who revoked the site or locked the wallet is not reconnected and does not pay for the poll. A failure while reconnecting keeps its own error type rather than becoming `UserRejectedRequestError`, and an account that cannot be parsed is skipped instead of ending the poll. +`isAuthorized()` deliberately keeps gating on the storage shim alone: the wallet fills its `accounts` from a lookup it does not await, so reading them there would race the restore, report false on every reload and skip reconnect entirely. `connect({ isReconnecting: true })` is what waits for the session and rejects when it never arrives. A failure while reconnecting keeps its own error type rather than becoming `UserRejectedRequestError`, and an account that cannot be parsed is skipped instead of ending the poll. diff --git a/packages/client/src/actions/disconnect.spec.ts b/packages/client/src/actions/disconnect.spec.ts index cc6f2db..bb32c5c 100644 --- a/packages/client/src/actions/disconnect.spec.ts +++ b/packages/client/src/actions/disconnect.spec.ts @@ -1,6 +1,7 @@ import { bitcoin, ChainId } from '@bigmi/core' import { describe, expect, it } from 'vitest' import { createConfig } from '../factories/createConfig.js' +import { createStorage } from '../factories/createStorage.js' import { disconnect } from './disconnect.js' const address = 'bc1q8h8s4zd9y0lkrx334aqnj4ykqs220ss735a3gh' @@ -28,10 +29,24 @@ const stubConnector = }) function setup(disconnectImpl: () => Promise) { + // The default storage is a no-op outside the browser, so the shim + // assertions below would pass vacuously. + const store = new Map() const config = createConfig({ chains: [bitcoin], connectors: [stubConnector('xverse', disconnectImpl) as any], client: () => ({}) as any, + storage: createStorage({ + storage: { + getItem: (key) => store.get(key) ?? null, + setItem: (key, value) => { + store.set(key, value) + }, + removeItem: (key) => { + store.delete(key) + }, + }, + }), }) as any const connector = config.connectors[0] config.setState((x: any) => ({ @@ -72,4 +87,24 @@ describe('disconnect', () => { expect(config.state.connections.size).toBe(0) expect(config.state.status).toBe('disconnected') }) + + it('clears the connector shim when its disconnect throws', async () => { + // The connector throws before writing its own shim, so the store would + // say disconnected while storage still said connected — and the next + // reload would silently restore what the user disconnected. + const { config, connector } = setup(async () => { + throw new Error('ProviderNotFoundError') + }) + await config.storage?.setItem(`${connector.id}.connected`, true) + + await expect(disconnect(config, { connector })).rejects.toThrow( + 'ProviderNotFoundError' + ) + await expect( + config.storage?.getItem(`${connector.id}.connected`) + ).resolves.toBeFalsy() + await expect( + config.storage?.getItem(`${connector.id}.disconnected`) + ).resolves.toBe(true) + }) }) diff --git a/packages/client/src/actions/disconnect.ts b/packages/client/src/actions/disconnect.ts index 730c9d9..385527a 100644 --- a/packages/client/src/actions/disconnect.ts +++ b/packages/client/src/actions/disconnect.ts @@ -34,6 +34,15 @@ export async function disconnect( await connector.disconnect() } catch (error) { disconnectError = error + // The connector threw before writing its own shim, so storage would + // still say connected while the store says otherwise — and the next + // reconnect would silently restore what the user disconnected. + try { + await Promise.all([ + config.storage?.setItem(`${connector.id}.disconnected`, true), + config.storage?.removeItem(`${connector.id}.connected`), + ]) + } catch {} } connector.emitter.off('change', config._internal.events.change) connector.emitter.off('disconnect', config._internal.events.disconnect) diff --git a/packages/client/src/connectors/metamask.spec.ts b/packages/client/src/connectors/metamask.spec.ts index 3367e2b..42ac551 100644 --- a/packages/client/src/connectors/metamask.spec.ts +++ b/packages/client/src/connectors/metamask.spec.ts @@ -31,10 +31,18 @@ function createWallet( } } -function createConnector(wallet: unknown) { +function createConnector(wallet: unknown, { connectedShim = true } = {}) { const connector: any = metamask()({ emitter: { emit: vi.fn() }, - storage: { setItem: vi.fn(), removeItem: vi.fn(), getItem: vi.fn() }, + storage: { + setItem: vi.fn(), + removeItem: vi.fn(), + // `isAuthorized` gates on this before anything else, so a stub that + // resolves undefined would skip every branch under test. + getItem: vi.fn(async (key: string) => + key.endsWith('.connected') ? connectedShim : undefined + ), + }, } as any) connector.getInternalProvider = async () => wallet return connector @@ -116,14 +124,29 @@ describe('metamask connector reconnect', () => { expect(result.accounts.map((a: any) => a.address)).toEqual([address]) }) - it('is not authorized when the session holds no account', async () => { + it('stays authorized while the session is still restoring', async () => { + const connectSpy = vi.fn(async () => ({ accounts: [account] })) ;(globalThis as any).window = {} - const connector = createConnector({ - name: 'MetaMask', - accounts: [], - features: { 'bitcoin:events': { on: vi.fn(() => () => {}) } }, + // `accounts` is empty until the un-awaited restore resolves. Reading it + // here would skip reconnect on every reload. + const connector = createConnector( + createWallet(connectSpy, { restoreAfterMs: 150 }) + ) + await expect(connector.isAuthorized()).resolves.toBe(true) + }) + + it('is not authorized without the connected shim', async () => { + const connectSpy = vi.fn(async () => ({ accounts: [account] })) + ;(globalThis as any).window = {} + const connector = createConnector(createWallet(connectSpy), { + connectedShim: false, }) - connector.config = undefined + await expect(connector.isAuthorized()).resolves.toBe(false) + }) + + it('is not authorized when the wallet is absent', async () => { + ;(globalThis as any).window = {} + const connector = createConnector(undefined) await expect(connector.isAuthorized()).resolves.toBe(false) }) }) diff --git a/packages/client/src/connectors/metamask.ts b/packages/client/src/connectors/metamask.ts index bad8045..d6987f4 100644 --- a/packages/client/src/connectors/metamask.ts +++ b/packages/client/src/connectors/metamask.ts @@ -212,9 +212,10 @@ export function metamask( }) return payment.length > 0 ? payment : undefined }, - // One extension round trip; `reconnect` already spends up to 5s - // finding the provider, so do not stack another long wait on it. - { timeout: 1000, interval: 50 } + // `reconnect` allows `connect` 5s, and the provider lookup before + // it returns as soon as the wallet registers, so this is the only + // meaningful wait. A stale shim pays it once per load. + { timeout: 1500, interval: 50 } )) ?? []) : undefined @@ -307,15 +308,12 @@ export function metamask( return false } - const wallet = await this.getInternalProvider() - if (!wallet) { - return false - } - - // Reading the restored session here, rather than calling getAccounts, - // keeps this passive: a user who revoked the site or locked the wallet - // must not be reconnected, and must not pay for the poll in connect. - return wallet.accounts.length > 0 + // Deliberately the shim alone. The wallet fills `accounts` from a + // session lookup it does not await, so reading it here would race the + // restore and report false on every reload, skipping reconnect + // entirely. `connect({ isReconnecting: true })` waits for the session + // and rejects if it never arrives. + return Boolean(await this.getInternalProvider()) } catch { return false } From 19791be7f58403139998bdaa66d59680b0c04f2d Mon Sep 17 00:00:00 2001 From: Eugene Chybisov Date: Mon, 21 Sep 2026 12:38:23 +0200 Subject: [PATCH 6/9] fix(client): harden the MetaMask account-change path Three defects that `metamask()` becoming a default connector makes reachable for every widget user. A `change` event mapped accounts through `toAccount` unguarded, while the reconnect poll next to it already skips what it cannot parse. `toAccount` throws on a missing `publicKey` or an address it cannot classify, and the throw escaped into MetaMask's own emitter: bigmi never learned the account had changed and kept signing against the previous address. `onAccountsChanged` treated only an empty raw list as a disconnect, then emitted `change` with the payment-filtered list. A Taproot-only selection filters to nothing, and an empty array is not nullish, so the connection kept `status: 'connected'` with no usable address and every route request failed. Filter first, and disconnect when nothing is left. `onDisconnect` left the connected shim in storage. `isAuthorized` gates on that shim, so a user who revoked the site inside MetaMask was reported authorized on every later load and paid for a session poll that could never succeed. Clear it. --- .changeset/metamask-passive-reconnect.md | 6 +- .../client/src/connectors/metamask.spec.ts | 77 +++++++++++++++++++ packages/client/src/connectors/metamask.ts | 33 +++++--- 3 files changed, 104 insertions(+), 12 deletions(-) diff --git a/.changeset/metamask-passive-reconnect.md b/.changeset/metamask-passive-reconnect.md index c357ba6..9f2b100 100644 --- a/.changeset/metamask-passive-reconnect.md +++ b/.changeset/metamask-passive-reconnect.md @@ -4,6 +4,8 @@ Stop MetaMask Bitcoin from opening the extension on page load. `connect()` ignored the `isReconnecting` flag and read accounts through the interactive `bitcoin:connect`, so `reconnect()` prompted a returning user whose wallet was locked. It now reads the session the wallet already restored, matching `unisat`, `okx`, `binance`, `bitget` and `onekey`. -The wallet fills its `accounts` from a session lookup it does not await, so that read is polled for up to a second rather than taken as a first snapshot — otherwise a wallet that registers after the app mounts reports no session and never recovers. An absent session now rejects with `ConnectorNotConnectedError` instead of `UserRejectedRequestError`, since nothing was shown to the user. +The wallet fills its `accounts` from a session lookup it does not await, so that read is polled briefly rather than taken as a first snapshot — otherwise a wallet that registers after the app mounts reports no session and never recovers. An absent session now rejects with `ConnectorNotConnectedError` instead of `UserRejectedRequestError`, since nothing was shown to the user. -`isAuthorized()` deliberately keeps gating on the storage shim alone: the wallet fills its `accounts` from a lookup it does not await, so reading them there would race the restore, report false on every reload and skip reconnect entirely. `connect({ isReconnecting: true })` is what waits for the session and rejects when it never arrives. A failure while reconnecting keeps its own error type rather than becoming `UserRejectedRequestError`, and an account that cannot be parsed is skipped instead of ending the poll. +`isAuthorized()` deliberately gates on the storage shim and the wallet's presence, not on its accounts: the wallet fills its `accounts` from a lookup it does not await, so reading them there would race the restore, report false on every reload and skip reconnect entirely. `connect({ isReconnecting: true })` is what waits for the session and rejects when it never arrives. A failure while reconnecting keeps its own error type rather than becoming `UserRejectedRequestError`, and an account that cannot be parsed is skipped instead of ending the poll. + +A `change` event no longer throws out of MetaMask's emitter when one account cannot be parsed, and a selection that leaves no payment address now disconnects instead of reporting a connected wallet with no usable address. The connected shim is cleared when the wallet itself disconnects, so revoking the site inside MetaMask is not retried on every load. diff --git a/packages/client/src/connectors/metamask.spec.ts b/packages/client/src/connectors/metamask.spec.ts index 42ac551..7ec1df7 100644 --- a/packages/client/src/connectors/metamask.spec.ts +++ b/packages/client/src/connectors/metamask.spec.ts @@ -149,4 +149,81 @@ describe('metamask connector reconnect', () => { const connector = createConnector(undefined) await expect(connector.isAuthorized()).resolves.toBe(false) }) + + it('ignores an unparseable account in a change event', async () => { + const connectSpy = vi.fn(async () => ({ accounts: [account] })) + ;(globalThis as any).window = {} + let fire: ((e: { accounts: unknown[] }) => void) | undefined + const emit = vi.fn() + const connector: any = metamask()({ + emitter: { emit }, + storage: { + setItem: vi.fn(), + removeItem: vi.fn(), + getItem: vi.fn(async () => true), + }, + } as any) + connector.getInternalProvider = async () => ({ + name: 'MetaMask', + accounts: [account], + features: { + 'bitcoin:connect': { connect: connectSpy }, + 'bitcoin:events': { + on: vi.fn((_event: string, handler: any) => { + fire = handler + return () => {} + }), + }, + }, + }) + + await connector.connect() + emit.mockClear() + // A throw here escapes into MetaMask's emitter, so bigmi would never + // learn the account changed and would keep signing the old address. + expect(() => + fire?.({ accounts: [{ address: 'not-an-address' }, account] }) + ).not.toThrow() + }) + + it('disconnects when no payment account is left', async () => { + const emit = vi.fn() + const connector: any = metamask()({ + emitter: { emit }, + storage: { + setItem: vi.fn(), + removeItem: vi.fn(), + getItem: vi.fn(async () => true), + }, + } as any) + + // A Taproot-only selection filters to nothing. Emitting `change` with an + // empty list leaves the store connected with no usable address. + await connector.onAccountsChanged([ + { + address: 'bc1p', + addressType: 'p2tr', + publicKey: '00', + purpose: 'ordinals', + }, + ]) + expect(emit).toHaveBeenCalledWith('disconnect') + }) + + it('clears the connected shim when the wallet disconnects', async () => { + const removeItem = vi.fn() + const connector: any = metamask()({ + emitter: { emit: vi.fn() }, + storage: { + setItem: vi.fn(), + removeItem, + getItem: vi.fn(async () => true), + }, + } as any) + + await connector.onDisconnect() + // Otherwise `isAuthorized()` stays true forever after the user revokes + // the site inside MetaMask. + expect(removeItem).toHaveBeenCalledWith('io.metamask.bitcoin.connected') + }) }) diff --git a/packages/client/src/connectors/metamask.ts b/packages/client/src/connectors/metamask.ts index d6987f4..33d0410 100644 --- a/packages/client/src/connectors/metamask.ts +++ b/packages/client/src/connectors/metamask.ts @@ -234,10 +234,16 @@ export function metamask( unsubscribe = wallet.features['bitcoin:events'].on( 'change', ({ accounts }) => { + // A throw here escapes into MetaMask's emitter, so bigmi would + // never learn the account changed. Skip what cannot be parsed. onAccountsChanged( - (accounts ?? []).map((account) => - toAccount(account as WalletAccount) - ) + (accounts ?? []).flatMap((account) => { + try { + return [toAccount(account as WalletAccount)] + } catch { + return [] + } + }) ) } ) @@ -319,21 +325,28 @@ export function metamask( } }, async onAccountsChanged(accounts) { - if (accounts.length === 0) { + // MetaMask exposes payment addresses only. A selection that filters to + // nothing — a Taproot-only account, say — would otherwise leave the + // store connected with no usable address. + const payment = accounts.filter( + (account) => account.purpose === 'payment' + ) + if (payment.length === 0) { this.onDisconnect() } else { - config.emitter.emit('change', { - accounts: accounts.filter((account) => account.purpose === 'payment'), - }) + config.emitter.emit('change', { accounts: payment }) } }, onChainChanged(chainId) { config.emitter.emit('change', { chainId }) }, async onDisconnect(_error) { - // No need to remove `${this.id}.disconnected` from storage because `onDisconnect` is typically - // only called when the wallet is disconnected through the wallet's interface, meaning the wallet - // actually disconnected and we don't need to simulate it. + // `isAuthorized` gates on the connected shim, so leaving it would keep + // reporting an authorized connector after the user revoked the site + // inside MetaMask, and every load would retry a session that is gone. + if (shimDisconnect) { + await config.storage?.removeItem(`${this.id}.connected`) + } config.emitter.emit('disconnect') }, })) From ed4e6c11d4ccd472ba0d63375829359d4154e419 Mon Sep 17 00:00:00 2001 From: Eugene Chybisov Date: Mon, 21 Sep 2026 12:55:37 +0200 Subject: [PATCH 7/9] fix(client): do not read an unparseable batch as a disconnect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parse guard and the payment-filter disconnect interacted badly. Skipping accounts `toAccount` cannot read left an empty list, which the disconnect rule then took to mean the wallet had reported nothing — so a single `change` event carrying only a half-initialized account disconnected the user and deleted the connected shim, leaving the next page load unable to reconnect either. That is the exact state the reconnect poll exists to ride out. Treat a non-empty batch that parses to nothing as transient and ignore the event. An empty batch, or one that parses to no payment address, still disconnects. `onDisconnect` also no longer depends on its storage write: a blocked `localStorage` threw before `emit('disconnect')`, leaving the store connected with no usable address — the state the payment-filter fix targets. --- .changeset/metamask-passive-reconnect.md | 2 +- .../client/src/connectors/metamask.spec.ts | 18 +++++++++++ packages/client/src/connectors/metamask.ts | 30 ++++++++++++------- 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/.changeset/metamask-passive-reconnect.md b/.changeset/metamask-passive-reconnect.md index 9f2b100..5feea50 100644 --- a/.changeset/metamask-passive-reconnect.md +++ b/.changeset/metamask-passive-reconnect.md @@ -8,4 +8,4 @@ The wallet fills its `accounts` from a session lookup it does not await, so that `isAuthorized()` deliberately gates on the storage shim and the wallet's presence, not on its accounts: the wallet fills its `accounts` from a lookup it does not await, so reading them there would race the restore, report false on every reload and skip reconnect entirely. `connect({ isReconnecting: true })` is what waits for the session and rejects when it never arrives. A failure while reconnecting keeps its own error type rather than becoming `UserRejectedRequestError`, and an account that cannot be parsed is skipped instead of ending the poll. -A `change` event no longer throws out of MetaMask's emitter when one account cannot be parsed, and a selection that leaves no payment address now disconnects instead of reporting a connected wallet with no usable address. The connected shim is cleared when the wallet itself disconnects, so revoking the site inside MetaMask is not retried on every load. +A `change` event no longer throws out of MetaMask's emitter when one account cannot be parsed, and a batch where nothing parses is treated as the transient half-initialized state rather than a disconnect, and a selection that leaves no payment address now disconnects instead of reporting a connected wallet with no usable address. The connected shim is cleared when the wallet itself disconnects, so revoking the site inside MetaMask is not retried on every load. diff --git a/packages/client/src/connectors/metamask.spec.ts b/packages/client/src/connectors/metamask.spec.ts index 7ec1df7..86c7036 100644 --- a/packages/client/src/connectors/metamask.spec.ts +++ b/packages/client/src/connectors/metamask.spec.ts @@ -226,4 +226,22 @@ describe('metamask connector reconnect', () => { // the site inside MetaMask. expect(removeItem).toHaveBeenCalledWith('io.metamask.bitcoin.connected') }) + + it('still reports the disconnect when storage refuses', async () => { + const emit = vi.fn() + const connector: any = metamask()({ + emitter: { emit }, + storage: { + setItem: vi.fn(), + // A blocked localStorage throws here. + removeItem: vi.fn(async () => { + throw new Error('SecurityError') + }), + getItem: vi.fn(async () => true), + }, + } as any) + + await expect(connector.onDisconnect()).resolves.toBeUndefined() + expect(emit).toHaveBeenCalledWith('disconnect') + }) }) diff --git a/packages/client/src/connectors/metamask.ts b/packages/client/src/connectors/metamask.ts index 33d0410..3159f84 100644 --- a/packages/client/src/connectors/metamask.ts +++ b/packages/client/src/connectors/metamask.ts @@ -236,15 +236,21 @@ export function metamask( ({ accounts }) => { // A throw here escapes into MetaMask's emitter, so bigmi would // never learn the account changed. Skip what cannot be parsed. - onAccountsChanged( - (accounts ?? []).flatMap((account) => { - try { - return [toAccount(account as WalletAccount)] - } catch { - return [] - } - }) - ) + const reported = accounts ?? [] + const parsed = reported.flatMap((account) => { + try { + return [toAccount(account as WalletAccount)] + } catch { + return [] + } + }) + // Nothing parsed out of a non-empty batch is the transient + // half-initialized state, not a disconnect. Reporting it as one + // would drop the session and the shim with it. + if (reported.length > 0 && parsed.length === 0) { + return + } + onAccountsChanged(parsed) } ) } @@ -345,7 +351,11 @@ export function metamask( // reporting an authorized connector after the user revoked the site // inside MetaMask, and every load would retry a session that is gone. if (shimDisconnect) { - await config.storage?.removeItem(`${this.id}.connected`) + // A blocked localStorage must not stop the disconnect being reported, + // or the store stays connected with no usable address. + try { + await config.storage?.removeItem(`${this.id}.connected`) + } catch {} } config.emitter.emit('disconnect') }, From 285b194171622c250d594d8478ba2a364710c760 Mon Sep 17 00:00:00 2001 From: Eugene Chybisov Date: Mon, 21 Sep 2026 13:10:14 +0200 Subject: [PATCH 8/9] fix(client): close four gaps in the MetaMask connect path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An empty interactive selection read `accounts[0].address` and threw a TypeError, which the catch relabelled as `UserRejectedRequestError` — so a user who simply holds no Bitcoin account saw raw JavaScript text and was told they had rejected something. The reconnect path already classified that state correctly; the interactive one now does too. A reconnect that finds no session left the connected shim in place, and `reconnect` swallows the error, so a site revoked while the tab was closed never self-healed: every later load paid the full poll and sat in `reconnecting`. Clear the shim there. `onDisconnect` never released the events subscription — only `disconnect` did, and this connector now reaches `onDisconnect` from `onAccountsChanged` too. A later connect would see a truthy `unsubscribe`, skip reattaching, and stay bound to a wallet object MetaMask had replaced, so account switches stopped reaching the widget. The poll guarded each account but not the `accounts` getter itself. A getter that throws while the wallet initializes ended the poll on its first tick, which is exactly the case the poll exists to survive. --- .changeset/metamask-passive-reconnect.md | 2 + .../client/src/connectors/metamask.spec.ts | 72 +++++++++++++++++++ packages/client/src/connectors/metamask.ts | 51 +++++++++---- 3 files changed, 113 insertions(+), 12 deletions(-) diff --git a/.changeset/metamask-passive-reconnect.md b/.changeset/metamask-passive-reconnect.md index 5feea50..066a2c1 100644 --- a/.changeset/metamask-passive-reconnect.md +++ b/.changeset/metamask-passive-reconnect.md @@ -9,3 +9,5 @@ The wallet fills its `accounts` from a session lookup it does not await, so that `isAuthorized()` deliberately gates on the storage shim and the wallet's presence, not on its accounts: the wallet fills its `accounts` from a lookup it does not await, so reading them there would race the restore, report false on every reload and skip reconnect entirely. `connect({ isReconnecting: true })` is what waits for the session and rejects when it never arrives. A failure while reconnecting keeps its own error type rather than becoming `UserRejectedRequestError`, and an account that cannot be parsed is skipped instead of ending the poll. A `change` event no longer throws out of MetaMask's emitter when one account cannot be parsed, and a batch where nothing parses is treated as the transient half-initialized state rather than a disconnect, and a selection that leaves no payment address now disconnects instead of reporting a connected wallet with no usable address. The connected shim is cleared when the wallet itself disconnects, so revoking the site inside MetaMask is not retried on every load. + +An empty selection — a user with no Bitcoin account — now rejects with `ConnectorNotConnectedError` on the interactive path too, rather than a `TypeError` relabelled as a rejection. A reconnect that finds no session clears the connected shim, so a site revoked while the tab was closed stops being retried on every load, and `onDisconnect` releases the events subscription so a later connect rebinds to the current wallet object. diff --git a/packages/client/src/connectors/metamask.spec.ts b/packages/client/src/connectors/metamask.spec.ts index 86c7036..b670334 100644 --- a/packages/client/src/connectors/metamask.spec.ts +++ b/packages/client/src/connectors/metamask.spec.ts @@ -244,4 +244,76 @@ describe('metamask connector reconnect', () => { await expect(connector.onDisconnect()).resolves.toBeUndefined() expect(emit).toHaveBeenCalledWith('disconnect') }) + + it('reports an empty interactive selection as not connected', async () => { + // MetaMask resolves with no payment account when the user holds none. + // Reading accounts[0] then throws a TypeError that the catch relabels as + // a rejection, showing raw JavaScript text for a wallet that rejected + // nothing. + const connectSpy = vi.fn(async () => ({ accounts: [] })) + ;(globalThis as any).window = {} + const connector = createConnector({ + name: 'MetaMask', + accounts: [], + features: { + 'bitcoin:connect': { connect: connectSpy }, + 'bitcoin:events': { on: vi.fn(() => () => {}) }, + }, + }) + + await expect(connector.connect()).rejects.toThrow( + ConnectorNotConnectedError + ) + }) + + it('clears the shim when a reconnect finds no session', async () => { + const removeItem = vi.fn() + ;(globalThis as any).window = {} + const connector: any = metamask()({ + emitter: { emit: vi.fn() }, + storage: { + setItem: vi.fn(), + removeItem, + getItem: vi.fn(async () => true), + }, + } as any) + connector.getInternalProvider = async () => ({ + name: 'MetaMask', + accounts: [], + features: { 'bitcoin:events': { on: vi.fn(() => () => {}) } }, + }) + + await expect(connector.connect({ isReconnecting: true })).rejects.toThrow( + ConnectorNotConnectedError + ) + // Otherwise a user who revoked the site while the tab was closed pays the + // poll on every later load, forever. + expect(removeItem).toHaveBeenCalledWith('io.metamask.bitcoin.connected') + }) + + it('survives an accounts getter that throws during the poll', async () => { + const connectSpy = vi.fn(async () => ({ accounts: [account] })) + ;(globalThis as any).window = {} + let throwing = true + setTimeout(() => { + throwing = false + }, 100) + const connector = createConnector({ + name: 'MetaMask', + get accounts(): any { + if (throwing) { + throw new TypeError('not ready') + } + return [account] + }, + features: { + 'bitcoin:connect': { connect: connectSpy }, + 'bitcoin:events': { on: vi.fn(() => () => {}) }, + }, + }) + + // A throw must not end the poll this code exists to provide. + const result = await connector.connect({ isReconnecting: true }) + expect(result.accounts.map((a: any) => a.address)).toEqual([address]) + }) }) diff --git a/packages/client/src/connectors/metamask.ts b/packages/client/src/connectors/metamask.ts index 3159f84..70db184 100644 --- a/packages/client/src/connectors/metamask.ts +++ b/packages/client/src/connectors/metamask.ts @@ -200,17 +200,22 @@ export function metamask( const restored = isReconnecting ? ((await retryUntil( async () => { - // A half-initialized account can fail to parse. Skip it rather - // than ending the poll this exists to survive. - const payment = wallet.accounts.flatMap((account) => { - try { - const parsed = toAccount(account as WalletAccount) - return parsed.purpose === 'payment' ? [parsed] : [] - } catch { - return [] - } - }) - return payment.length > 0 ? payment : undefined + // A half-initialized wallet can throw from the getter itself as + // well as from a single account, and either would end the poll + // this exists to survive. + try { + const payment = (wallet.accounts ?? []).flatMap((account) => { + try { + const parsed = toAccount(account as WalletAccount) + return parsed.purpose === 'payment' ? [parsed] : [] + } catch { + return [] + } + }) + return payment.length > 0 ? payment : undefined + } catch { + return undefined + } }, // `reconnect` allows `connect` 5s, and the provider lookup before // it returns as soon as the wallet registers, so this is the only @@ -222,11 +227,26 @@ export function metamask( // Outside the try: nothing was shown to the user, so this must not be // reported as a rejection. if (restored && restored.length === 0) { + // The session is gone — revoked while the tab was closed, say — and + // `reconnect` swallows this error, so clear the shim here or every + // later load pays the poll again. + if (shimDisconnect) { + try { + await config.storage?.removeItem(`${this.id}.connected`) + } catch {} + } + throw new ConnectorNotConnectedError() + } + + const accounts = restored ?? (await this.getAccounts()) + // [1] The interactive path can also come back empty, when the user holds + // no Bitcoin account. Reading accounts[0] would throw a TypeError that + // the catch below would relabel as a rejection. + if (accounts.length === 0) { throw new ConnectorNotConnectedError() } try { - const accounts = restored ?? (await this.getAccounts()) const chainId = getAddressChainId(accounts[0].address) if (!unsubscribe) { @@ -347,6 +367,13 @@ export function metamask( config.emitter.emit('change', { chainId }) }, async onDisconnect(_error) { + // Release the subscription, or a later connect sees a truthy + // `unsubscribe`, skips reattaching, and stays bound to a wallet object + // MetaMask has since replaced. + if (unsubscribe) { + unsubscribe() + unsubscribe = undefined + } // `isAuthorized` gates on the connected shim, so leaving it would keep // reporting an authorized connector after the user revoked the site // inside MetaMask, and every load would retry a session that is gone. From f509fbf5b613a94cd437abdf7c07cf2364fcc731 Mon Sep 17 00:00:00 2001 From: Eugene Chybisov Date: Mon, 21 Sep 2026 13:26:53 +0200 Subject: [PATCH 9/9] fix(client): restore the rejection mapping, and keep the shim on a slow load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two regressions from the previous commit. Moving `getAccounts()` out of the `try` dropped the `UserRejectedRequestError` mapping from the one call a user can actually reject, so a decline surfaced as MetaMask's raw error and consumers lost the documented contract. The surviving `try` then wrapped only the subscribe and the storage writes, so a blocked `localStorage` in an embedded iframe told the user they had rejected a connection that had in fact succeeded. Wrap just the interactive call, as `binance` does, and let the storage write fail quietly. Clearing the connected shim when a reconnect found no session assumed the poll could tell "no session" from "no answer yet". It cannot: the wallet restores once per load, un-awaited, and swallows a failure, and a cold MV3 service worker takes seconds. A returning user with a valid session was signed out for good. Keep the shim — a genuine revoke still clears it through `onDisconnect` — and spend more of the budget `reconnect` already allows. No test covered the interactive rejection mapping, which is why the first regression went unnoticed; there is one now, and it fails without the fix. --- .changeset/metamask-passive-reconnect.md | 2 +- .../client/src/connectors/metamask.spec.ts | 102 ++++++++++++----- packages/client/src/connectors/metamask.ts | 106 +++++++++--------- 3 files changed, 132 insertions(+), 78 deletions(-) diff --git a/.changeset/metamask-passive-reconnect.md b/.changeset/metamask-passive-reconnect.md index 066a2c1..46fd039 100644 --- a/.changeset/metamask-passive-reconnect.md +++ b/.changeset/metamask-passive-reconnect.md @@ -10,4 +10,4 @@ The wallet fills its `accounts` from a session lookup it does not await, so that A `change` event no longer throws out of MetaMask's emitter when one account cannot be parsed, and a batch where nothing parses is treated as the transient half-initialized state rather than a disconnect, and a selection that leaves no payment address now disconnects instead of reporting a connected wallet with no usable address. The connected shim is cleared when the wallet itself disconnects, so revoking the site inside MetaMask is not retried on every load. -An empty selection — a user with no Bitcoin account — now rejects with `ConnectorNotConnectedError` on the interactive path too, rather than a `TypeError` relabelled as a rejection. A reconnect that finds no session clears the connected shim, so a site revoked while the tab was closed stops being retried on every load, and `onDisconnect` releases the events subscription so a later connect rebinds to the current wallet object. +An empty selection — a user with no Bitcoin account — now rejects with `ConnectorNotConnectedError` on the interactive path too, rather than a `TypeError` relabelled as a rejection. A reconnect that finds no session keeps the connected shim, because the poll cannot tell an absent session from a wallet that has not answered yet and dropping it would permanently sign out a user whose session is valid; a genuine revoke still clears it through `onDisconnect`. Only the interactive `bitcoin:connect` maps to `UserRejectedRequestError`, so a blocked storage write no longer reports a successful connect as a rejection, and `onDisconnect` releases the events subscription so a later connect rebinds to the current wallet object. diff --git a/packages/client/src/connectors/metamask.spec.ts b/packages/client/src/connectors/metamask.spec.ts index b670334..13076dd 100644 --- a/packages/client/src/connectors/metamask.spec.ts +++ b/packages/client/src/connectors/metamask.spec.ts @@ -1,3 +1,4 @@ +import { UserRejectedRequestError } from '@bigmi/core' import { afterEach, describe, expect, it, vi } from 'vitest' import { ConnectorNotConnectedError } from '../errors/connectors.js' import { metamask } from './metamask.js' @@ -266,31 +267,6 @@ describe('metamask connector reconnect', () => { ) }) - it('clears the shim when a reconnect finds no session', async () => { - const removeItem = vi.fn() - ;(globalThis as any).window = {} - const connector: any = metamask()({ - emitter: { emit: vi.fn() }, - storage: { - setItem: vi.fn(), - removeItem, - getItem: vi.fn(async () => true), - }, - } as any) - connector.getInternalProvider = async () => ({ - name: 'MetaMask', - accounts: [], - features: { 'bitcoin:events': { on: vi.fn(() => () => {}) } }, - }) - - await expect(connector.connect({ isReconnecting: true })).rejects.toThrow( - ConnectorNotConnectedError - ) - // Otherwise a user who revoked the site while the tab was closed pays the - // poll on every later load, forever. - expect(removeItem).toHaveBeenCalledWith('io.metamask.bitcoin.connected') - }) - it('survives an accounts getter that throws during the poll', async () => { const connectSpy = vi.fn(async () => ({ accounts: [account] })) ;(globalThis as any).window = {} @@ -316,4 +292,80 @@ describe('metamask connector reconnect', () => { const result = await connector.connect({ isReconnecting: true }) expect(result.accounts.map((a: any) => a.address)).toEqual([address]) }) + + it('maps a declined interactive connect to a user rejection', async () => { + // `bitcoin:connect` is the only step a user can reject, and consumers + // rely on `UserRejectedRequestError` to tell a decline from a failure. + const connectSpy = vi.fn(async () => { + throw new Error('User rejected the request') + }) + ;(globalThis as any).window = {} + const connector = createConnector({ + name: 'MetaMask', + accounts: [], + features: { + 'bitcoin:connect': { connect: connectSpy }, + 'bitcoin:events': { on: vi.fn(() => () => {}) }, + }, + }) + + await expect(connector.connect()).rejects.toBeInstanceOf( + UserRejectedRequestError + ) + }) + + it('does not call a successful connect a rejection when storage fails', async () => { + const connectSpy = vi.fn(async () => ({ accounts: [account] })) + ;(globalThis as any).window = {} + const connector: any = metamask()({ + emitter: { emit: vi.fn() }, + storage: { + // A blocked localStorage in an embedded iframe throws here. + setItem: vi.fn(async () => { + throw new Error('SecurityError') + }), + removeItem: vi.fn(async () => { + throw new Error('SecurityError') + }), + getItem: vi.fn(async () => true), + }, + } as any) + connector.getInternalProvider = async () => ({ + name: 'MetaMask', + accounts: [account], + features: { + 'bitcoin:connect': { connect: connectSpy }, + 'bitcoin:events': { on: vi.fn(() => () => {}) }, + }, + }) + + const result = await connector.connect() + expect(result.accounts.map((a: any) => a.address)).toEqual([address]) + }) + + it('keeps the shim when a slow wallet has not restored yet', async () => { + const removeItem = vi.fn() + ;(globalThis as any).window = {} + const connector: any = metamask()({ + emitter: { emit: vi.fn() }, + storage: { + setItem: vi.fn(), + removeItem, + getItem: vi.fn(async () => true), + }, + } as any) + // A cold MV3 service worker can take seconds, and the wallet retries the + // restore on the next load. Dropping the shim would sign out a user whose + // session is still valid, permanently. + connector.getInternalProvider = async () => ({ + name: 'MetaMask', + accounts: [], + features: { 'bitcoin:events': { on: vi.fn(() => () => {}) } }, + }) + + await expect(connector.connect({ isReconnecting: true })).rejects.toThrow( + ConnectorNotConnectedError + ) + expect(removeItem).not.toHaveBeenCalledWith('io.metamask.bitcoin.connected') + }) }) diff --git a/packages/client/src/connectors/metamask.ts b/packages/client/src/connectors/metamask.ts index 70db184..1a20428 100644 --- a/packages/client/src/connectors/metamask.ts +++ b/packages/client/src/connectors/metamask.ts @@ -217,80 +217,82 @@ export function metamask( return undefined } }, - // `reconnect` allows `connect` 5s, and the provider lookup before - // it returns as soon as the wallet registers, so this is the only - // meaningful wait. A stale shim pays it once per load. - { timeout: 1500, interval: 50 } + // `reconnect` allows `connect` 5s and the provider lookup returns + // as soon as the wallet registers, so this is the only meaningful + // wait. A cold MV3 service worker can take seconds, so spend most + // of that budget rather than giving up on a valid session. + { timeout: 4000, interval: 50 } )) ?? []) : undefined // Outside the try: nothing was shown to the user, so this must not be // reported as a rejection. if (restored && restored.length === 0) { - // The session is gone — revoked while the tab was closed, say — and - // `reconnect` swallows this error, so clear the shim here or every - // later load pays the poll again. - if (shimDisconnect) { - try { - await config.storage?.removeItem(`${this.id}.connected`) - } catch {} - } + // Deliberately keeps the shim. The poll cannot tell "the wallet says + // there is no session" from "the wallet has not answered yet" — a + // cold MV3 worker takes seconds — and dropping it would sign out a + // user whose session is valid, for good. A genuine revoke arrives + // through `onDisconnect`, which does clear it. throw new ConnectorNotConnectedError() } - const accounts = restored ?? (await this.getAccounts()) - // [1] The interactive path can also come back empty, when the user holds - // no Bitcoin account. Reading accounts[0] would throw a TypeError that - // the catch below would relabel as a rejection. + // `bitcoin:connect` opens MetaMask and is the only step here a user can + // reject, so it alone maps to a rejection. Wrapping more would report a + // blocked storage write as one. + const requestAccounts = async (): Promise => { + try { + return await this.getAccounts() + } catch (error: any) { + throw new UserRejectedRequestError(error.message) + } + } + const accounts = restored ?? (await requestAccounts()) + + // An empty selection — a user holding no Bitcoin account — is not a + // rejection either, and reading accounts[0] would throw a TypeError. if (accounts.length === 0) { throw new ConnectorNotConnectedError() } - try { - const chainId = getAddressChainId(accounts[0].address) + const chainId = getAddressChainId(accounts[0].address) - if (!unsubscribe) { - const onAccountsChanged = this.onAccountsChanged.bind(this) - unsubscribe = wallet.features['bitcoin:events'].on( - 'change', - ({ accounts }) => { - // A throw here escapes into MetaMask's emitter, so bigmi would - // never learn the account changed. Skip what cannot be parsed. - const reported = accounts ?? [] - const parsed = reported.flatMap((account) => { - try { - return [toAccount(account as WalletAccount)] - } catch { - return [] - } - }) - // Nothing parsed out of a non-empty batch is the transient - // half-initialized state, not a disconnect. Reporting it as one - // would drop the session and the shim with it. - if (reported.length > 0 && parsed.length === 0) { - return + if (!unsubscribe) { + const onAccountsChanged = this.onAccountsChanged.bind(this) + unsubscribe = wallet.features['bitcoin:events'].on( + 'change', + ({ accounts }) => { + // A throw here escapes into MetaMask's emitter, so bigmi would + // never learn the account changed. Skip what cannot be parsed. + const reported = accounts ?? [] + const parsed = reported.flatMap((account) => { + try { + return [toAccount(account as WalletAccount)] + } catch { + return [] } - onAccountsChanged(parsed) + }) + // Nothing parsed out of a non-empty batch is the transient + // half-initialized state, not a disconnect. Reporting it as one + // would drop the session and the shim with it. + if (reported.length > 0 && parsed.length === 0) { + return } - ) - } + onAccountsChanged(parsed) + } + ) + } - // Remove disconnected shim if it exists - if (shimDisconnect) { + // Remove disconnected shim if it exists. A blocked storage must not + // fail a connection that already succeeded. + if (shimDisconnect) { + try { await Promise.all([ config.storage?.setItem(`${this.id}.connected`, true), config.storage?.removeItem(`${this.id}.disconnected`), ]) - } - return { accounts, chainId } - } catch (error: any) { - // Nothing is shown to the user while reconnecting, so a failure there - // is not a rejection. - if (isReconnecting) { - throw error - } - throw new UserRejectedRequestError(error.message) + } catch {} } + return { accounts, chainId } }, async disconnect() { if (unsubscribe) {