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/.changeset/disconnect-state-consistency.md b/.changeset/disconnect-state-consistency.md new file mode 100644 index 0000000..cde4228 --- /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. 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 new file mode 100644 index 0000000..46fd039 --- /dev/null +++ b/.changeset/metamask-passive-reconnect.md @@ -0,0 +1,13 @@ +--- +'@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`. + +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 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 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/actions/disconnect.spec.ts b/packages/client/src/actions/disconnect.spec.ts new file mode 100644 index 0000000..bb32c5c --- /dev/null +++ b/packages/client/src/actions/disconnect.spec.ts @@ -0,0 +1,110 @@ +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' + +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) { + // 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) => ({ + ...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') + }) + + 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 cae1bc0..385527a 100644 --- a/packages/client/src/actions/disconnect.ts +++ b/packages/client/src/actions/disconnect.ts @@ -24,8 +24,26 @@ 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 + // 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) connector.emitter.on('connect', config._internal.events.connect) @@ -56,13 +74,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/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() { diff --git a/packages/client/src/connectors/metamask.spec.ts b/packages/client/src/connectors/metamask.spec.ts new file mode 100644 index 0000000..13076dd --- /dev/null +++ b/packages/client/src/connectors/metamask.spec.ts @@ -0,0 +1,371 @@ +import { UserRejectedRequestError } from '@bigmi/core' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { ConnectorNotConnectedError } from '../errors/connectors.js' +import { metamask } from './metamask.js' + +const address = 'bc1q8h8s4zd9y0lkrx334aqnj4ykqs220ss735a3gh' +const publicKey = new Uint8Array([3, 203, 174, 220]) + +const account = { address, publicKey } + +// 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', + get accounts() { + return restored ? [account] : [] + }, + features: { + 'bitcoin:connect': { connect: connectSpy }, + 'bitcoin:events': { on: vi.fn(() => () => {}) }, + }, + } +} + +function createConnector(wallet: unknown, { connectedShim = true } = {}) { + const connector: any = metamask()({ + emitter: { emit: 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 +} + +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] })) + ;(globalThis as any).window = {} + const connector = createConnector(createWallet(connectSpy)) + + 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('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] })) + ;(globalThis as any).window = {} + const connector = createConnector(createWallet(connectSpy)) + + 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('stays authorized while the session is still restoring', async () => { + const connectSpy = vi.fn(async () => ({ accounts: [account] })) + ;(globalThis as any).window = {} + // `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, + }) + 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) + }) + + 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') + }) + + 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') + }) + + 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('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]) + }) + + 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 46f0ac6..1a20428 100644 --- a/packages/client/src/connectors/metamask.ts +++ b/packages/client/src/connectors/metamask.ts @@ -5,11 +5,15 @@ import { hexToUnit8Array, MethodNotSupportedRpcError, ProviderNotFoundError, + retryUntil, UserRejectedRequestError, } 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,40 +188,111 @@ 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() - const chainId = getAddressChainId(accounts[0].address) + // `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 () => { + // 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 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 - if (!unsubscribe) { - const onAccountsChanged = this.onAccountsChanged.bind(this) - unsubscribe = wallet.features['bitcoin:events'].on( - 'change', - ({ accounts }) => { - onAccountsChanged( - (accounts ?? []).map((account) => - toAccount(account as WalletAccount) - ) - ) - } - ) + // Outside the try: nothing was shown to the user, so this must not be + // reported as a rejection. + if (restored && restored.length === 0) { + // 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() + } + + // `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() + } + + 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 + } + 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) { - throw new UserRejectedRequestError(error.message) + } catch {} } + return { accounts, chainId } }, async disconnect() { if (unsubscribe) { @@ -263,27 +338,54 @@ 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 + } + + // 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 } }, 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. + // 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. + if (shimDisconnect) { + // 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') }, }))