Repository navigation
fix: ask the connector whether a Bitcoin wallet is installed - #878
Merged
Merged
Conversation
`installedWallets` filtered on `isWalletInstalled`, a second copy of the detection each bigmi connector already performs. The two had drifted, so a wallet that merely reports itself as MetaMask was listed and then bounced back to the menu on connect. Ask the connector instead. As a side effect the server now renders no Bitcoin wallets rather than guessing from an absent `window`, so the first client render no longer disagrees with it.
Twelve arms of `isWalletInstalled` re-derived from `window` what each bigmi connector's `getInternalProvider` already knows. Ten were identical, one (bitget) had drifted narrower, and MetaMask's was simply wrong: it keyed on `window.ethereum.isMetaMask`, which any wallet impersonating MetaMask sets. `getInstalledConnectors` now answers for Bitcoin, so these arms only had the power to disagree with it. `metaMask` keeps its EVM probe, which `walletTags` relies on, and `coinbase` is untouched.
🦋 Changeset detectedLatest commit: 355e81e The changes in this PR will be included in the next version bump. This PR includes changesets to release 33 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
E2E Examples — all passedAll examples passed in the latest run. |
Contributor
✅ E2E Dev Smoke — passing
4 passed · 0 failed · 0 skipped · 29s |
Contributor
E2E Playground results
Details
📥 Download full HTML report (open the run → Artifacts → |
The probe ran once on mount. That is safe for wallets that inject into `window` at document_start, but MetaMask announces Bitcoin through the Wallet Standard registry, which it can join later. A single read would then drop it for the rest of the page's life. Listen for the registry's own announce event and probe again. No new dependency: the event is the Wallet Standard protocol itself, and the answer still comes from the connector.
`@bigmi/client` 0.10.3 detects BitKeep injected only as `window.unisat`. Connector-backed detection asks the connector instead of re-deriving from `window`, so without this the Bitcoin list would stop showing that BitKeep build rather than keep showing it. `@bigmi/core` 0.9.2 maps a declined confirmation to a user rejection even when the wallet sends no rejection code, which is the case for MetaMask Bitcoin. A cancelled signature now reads "Signature required" rather than "Unknown Error". The ranges already admitted these versions; raising the minimums makes the dependency explicit so a consumer cannot resolve below it.
A multichain wallet's ecosystems were ordered by the sequence the six wallet lists happened to be combined in, preserved by the stable sort. Moving a block in `combineWalletLists` would silently reorder the menu. Name the order instead, and set it to Ethereum, Solana, Sui, Bitcoin, Tron, Stellar. A configured `walletEcosystemsOrder` still wins, and now composes with the default, so a partial order no longer lets the list-building order leak back in for the ecosystems it omits. Display only: `connectors[0]` is read solely when a wallet has exactly one ecosystem, so nothing changes about which one connects.
…ault Three things from review. The probe raced itself. `wallet-standard:register-wallet` fires for every Wallet Standard wallet, not just MetaMask, so probes overlap routinely; the `cancelled` flag guarded teardown but not ordering, letting a slower earlier probe restore a list from before the registration that triggered it. Sequence the probes and keep the previous array when nothing changed, so a registration no longer re-renders every Bitcoin consumer either. `metamask()` joins the default connectors. It was opt-in only because the widget refused `@metamask/*` dependencies, and those turned out to be needed solely for a manual registration the extension already performs itself. The connector imports `@bigmi/core` and `@wallet-standard/app`, both already loaded, so the playground drops four dependencies and ~31 KB. `BitcoinListItemButton` no longer consults `isWalletInstalled`. That call always returns true now, and the list it renders is already filtered, so it read as a guard while doing nothing.
`createDefaultBigmiConfig` appends `props.connectors` to the defaults, so an integrator who opted into `metamask()` before it became a default would get two connectors with the same id and see MetaMask twice in the menu. Keep the first connector per id when building the installed list, which is the first point where ids exist.
Review follow-ups. `handleDisconnect` dropped only the active connection. bigmi then promotes the next one and keeps the account connected, so a wallet that opened more than one — an integrator still passing `metamask()` now that it is a default — could not be disconnected from the menu. Drain them instead. The id filter in `getInstalledConnectors` keeps such a wallet from appearing twice, but it cannot prevent the second connection, and `createDefaultBigmiConfig` has no way to dedupe: `createConnector` returns the factory unchanged, so no id exists until `createConfig` sets it up. The new tests never ran. Neither monorepo has a root `test` script and no workflow invoked vitest, so the suites added here were evidence that never executed. Add the script, exclude the Playwright package from it, and run it in the release verification job. `--passWithNoTests` stops the packages that carry no test files from failing the run. Also: `pnpm dedupe` collapses the second `@bigmi/core` copy that the version bump introduced through `@lifi/sdk-provider-bitcoin`, `widget-light` moves to a minor because its peer ranges tighten, and the `connectors` option documents that a default is ignored and what makes a connector appear at all.
The Test step added to `verify` only runs on push and dispatch — that job carries `if: github.event_name != 'pull_request'` — so a broken test would first appear after the merge to main, where it blocks the release instead of the change that caused it. Add a workflow that runs `pnpm test` on PRs. Also corrects two things the tests contradict: `dynamic()` does not always appear, because its `getProvider()` throws when the wallet holds no connector and `getInstalledConnectors` treats a throw as unavailable; and the escaped em dash in widget-checkout's description was churn from rewriting that file programmatically.
The drain loop rethrew nothing, so a failure on the second connection left the first already dropped while `useAccountDisconnect` never learned the disconnect had failed. Keep the first error and rethrow it after the loop. `--passWithNoTests` now applies only to widget-light, the one package with a test script and no test files. Everywhere else the flag would have let a rename or a config change report green with zero tests — including the two packages that hold all of this PR's new coverage. The ecosystem-order changeset said it made the previous order explicit. It does not: Bitcoin moves from second to fourth for any multichain wallet not named in `walletEcosystemsOrder`. Say so.
The previous drain followed the active connection and stopped at the first error. bigmi keeps a connection whose `connector.disconnect()` throws, so the active one never advanced: a user who connected Xverse plus a second Bitcoin wallet and then removed the Xverse extension could not clear either. Every disconnect click retried the dead connection. The retained `firstError` was also unreachable past that break, and the comment claimed a drain the code did not perform. Enumerate `state.connections` instead and attempt each connector, so one broken wallet cannot hold the others connected, then rethrow the first error. Extracted as `disconnectAll` so the behaviour is testable without bigmi: four tests cover the drain, a mid-list failure still reaching later connectors, first-error reporting, and the empty case.
Attempting every connector meant a connection left behind by a removed extension could reject the whole disconnect, even though the wallet the user asked about was disconnected. Two callers then skipped their own work: `useAccountDisconnect` never emitted `WalletDisconnected`, so integrator analytics and UI went stale, and `BitcoinListItemButton` aborted before the `connect()` the disconnect was preparing for, so the click reported an error and connected nothing. Fail only when nothing could be disconnected. A lone connector that cannot be reached still surfaces its error, so a genuine failure is not swallowed.
chybisov
added a commit
to lifinance/bigmi
that referenced
this pull request
Sep 21, 2026
`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.
`@metamask/connect-evm` was removed alongside the two Bitcoin-only
`@metamask/*` packages, but it is not a Bitcoin package: `wagmi`'s EVM
`metaMask()` connector reaches it through `await import('@metamask/connect-evm')`,
and the playground still enables `metaMask: true`. pnpm does not install
optional peers, so a lockfile regeneration would have dropped it and broken
EVM MetaMask connect. Restored, and the changeset no longer claims every
`@metamask/*` package existed for the manual registration.
The re-probe listened only for `wallet-standard:register-wallet`. The eleven
`window` injectors announce nothing, so an extension enabled or installed
without a reload stayed hidden for the session — and unlike before, there is
no click-time `isWalletInstalled` fallback behind it. Probe on `focus` and
`visibilitychange` too, which is when returning from the extension's own UI.
Also drops the `@metamask/connect-multichain` knip exception, which named a
package the workspace no longer declares.
`disconnectAll` counted successes and rethrew when none succeeded. That predates the `@bigmi/client` fix which detaches a connection even when the wallet's own `disconnect()` throws, so the store is now consistent either way and the count no longer means anything. It mattered because `BitcoinListItemButton` disconnects before connecting: a user who connected Xverse, disabled the extension, then clicked Unisat got an error and no connection, and had to click a second time. Attempt every connection and carry on.
`defaultWalletEcosystemsOrder` is handed to the comparator by reference, so an integrator who read the export and mutated it — a `push` or a `reverse` — would reorder every wallet in the picker for the rest of the session. Freeze it and widen the comparator to `readonly`.
chybisov
added a commit
to lifinance/bigmi
that referenced
this pull request
Sep 21, 2026
* 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. * fix(client): do not open MetaMask Bitcoin while reconnecting `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. * fix(client): wait for MetaMask's restored session, and classify its absence 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. * 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. * fix(client): keep isAuthorized off the racing session read `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. * 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. * fix(client): do not read an unparseable batch as a disconnect 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. * fix(client): close four gaps in the MetaMask connect path 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. * fix(client): restore the rejection mapping, and keep the shim on a slow load 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.
Moves to `@bigmi/client` 0.10.4 and `@bigmi/react` 0.9.4, now that lifinance/bigmi#80 is merged and published. `0.10.4` carries what this branch depends on. The BitKeep and Binance detection fall-throughs matter most: asking the connector instead of re-deriving from `window` would otherwise narrow those two wallets rather than preserve them. It also stops a default `metamask()` connector prompting on page load, and keeps the store consistent when a wallet's own `disconnect()` throws. `@bigmi/core` stays on 0.9.2, which already carries the rejection mapping.
2 tasks done
chybisov
added a commit
that referenced
this pull request
Sep 21, 2026
Conflicts came from #878, which bumped `@bigmi/*` past this branch and reworked the Bitcoin connector detection. Resolved by taking the newer side per dependency: - `@bigmi/client` ^0.10.4 and `@bigmi/react` ^0.9.4 from main (both are latest) - `@lifi/sdk` ^4.8.0, `@lifi/sdk-provider-bitcoin` ^4.0.11, `@dynamic-labs/*` 5.9.0, `@reown/appkit-*` ^1.8.24, `@tanstack/react-query` ^5.103.1 and `@mysten/dapp-kit-react` ^2.1.32 from this branch - `@bigmi/react` peer floor ^0.9.4 from main, which #878 raised on purpose Main's deletions are honoured: `@metamask/bitcoin-wallet-standard`, `@metamask/connect-multichain` and `@metamask/multichain-api-client` stay removed from `widget-playground`, and its new `test` script in `widget-provider-bitcoin` is kept. `@reown/appkit` stays `>=1.8.20` in `widget-playground` and `examples/reown`. That open range mirrors the workspace override, and `pnpm up --latest` had narrowed it to a caret — the same slip already corrected for `viem`. The lockfile is resolved from scratch with a plain install, not `pnpm regen`, because its `pnpm dedupe` step splits the wagmi peer context (see the previous commit). All seven examples checked keep one shared wagmi instance.
Merged
2 of 3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which Linear task is linked to this PR?
JUMEMB-108 — QA found that a wallet merely reporting itself as MetaMask (Rabby) was offered a Bitcoin entry that then bounced back to the menu with no message.
Why was it implemented this way?
The reported bug is a symptom, not the problem.
isWalletInstalledcarried twelve Bitcoin arms that re-derived fromwindowexactly what each bigmi connector'sgetInternalProvider()already knows — and the two copies had drifted:unisat?.isBitKeep(fixed separately in lifinance/bigmi)window.ethereum.isMetaMask, which Rabby setsSo the fix deletes the copy rather than patching one arm.
getInstalledConnectorsasks each connector via its existing asyncgetProvider(); every bigmi connector resolvesundefinedwhen its wallet is absent (dynamicthrows, so a throw counts as absent too). That fixes the reported MetaMask case and the undiscovered bitget one, and stops the next drift before it starts.Alternatives rejected:
!isRabby && !isBraveWallet …) — wrong the day a new one ships.widget-provider— that package deliberately carries only@lifi/sdkandzustand, and this would add@wallet-standard/appto it.isInstalled()on bigmi's connector type — a new public API to fix one wallet, wheregetProvider()already covers twelve.'metaMask'keeps itswindow.ethereumprobe:wallet-management/src/utils/walletTags.ts:24uses it to choose Installed vs GetStarted for the EVM SDK connector, where registry-style detection would be wrong.'coinbase'is untouched.Async cost is nil. All twelve
getInternalProvider()bodies are synchronous reads insideasyncfunctions. Measured in a live browser: aPromise.allover twelve of them runs in under 1 ms and settles before the next macrotask, so no paint happens in between. No loading flash.No render loop.
connectorsis referentially stable —getConnectorsin@bigmi/clientmemoises withdeepEqualand returns the identical reference when unchanged, feedinguseSyncExternalStore.Incidental SSR improvement. The server now renders no Bitcoin wallets rather than guessing from an absent
window, so the first client render no longer disagrees with it.Visual showcase (Screenshots or Videos)
None — no visual change. The Rabby case needs two extensions to reproduce; see the testing note.
Checklist before requesting a review
API note — this is a minor bump
isWalletInstalledis a public export. It no longer answers for Bitcoin connector ids and returnstruefor them, as it already did for any wallet it does not explicitly know.metaMaskandcoinbaseare unchanged. The changeset says so.Open dependency — Binance detection
This branch requires
@bigmi/client ^0.10.3. In that releasebinance()returns as soon aswindow.binancew3wexists and never reaches thewindow.unisat.isBinancefallback, whereas theisWalletInstalledarm deleted here accepted both. On a build that injectsbinancew3wwithout a bitcoin provider, Binance would disappear from the Bitcoin list.Fixed in lifinance/bigmi#80. That PR is mergeable but awaiting review. Once it is released, raise this requirement to the new
@bigmi/clientversion before merging — exactly as was done for BitKeep and0.10.3.Parity for the other eleven connectors is confirmed against 0.10.3; Binance is the only arm that regresses.
Accepted trade-off
installedWalletsstarts empty and fills in an effect, so the very first paint can list no Bitcoin wallet if the menu mounts in the same commit as the provider. A synchronous first value would mean reintroducing thewindow-sniffing detection this PR removes, and the provider mounts long before a user opens the menu. Reviewed and deliberately left as is.Verified
pnpm build— all packages, exit 0pnpm check:types— 14 packages, 0 errorspnpm check(Biome) — cleanpnpm knip:check— clean@lifi/widget-provider— 32 tests pass (5 new forisWalletInstalled)@lifi/widget-provider-bitcoin— 5 new tests pass forgetInstalledConnectors, mutation-checked (removing thetry/catchfails 1, removing the filter fails 2)bigmi dependency — resolved
This needed
@bigmi/clientto widen BitKeep detection first, or it would have narrowed BitKeep listing. That shipped in@bigmi/client@0.10.3(lifinance/bigmi#77) and this branch now requires it, alongside@bigmi/core@0.9.2, which maps a declined confirmation to a user rejection even when the wallet sends no rejection code — so a cancelled MetaMask Bitcoin signature reads "Signature required" instead of "Unknown Error" (JUMEMB-112).Still needs a human
A Rabby-only browser profile must no longer show Bitcoin under MetaMask. I cannot install extensions.