Skip to content

fix: gate wallet connect button on real provider hydration - #2131

Open
arentant wants to merge 1 commit into
devfrom
dev-providerReadinessFix
Open

fix: gate wallet connect button on real provider hydration#2131
arentant wants to merge 1 commit into
devfrom
dev-providerReadinessFix

Conversation

@arentant

Copy link
Copy Markdown
Collaborator

Problem

The connect button in the address picker treated a descriptor stub as connect-ready. Clicking one opened the generic wallet modal instead of the destination's own connectors, and the readiness logic behind it had accumulated in three places with four copies of the same isStub || !ready predicate and three separate 5000ms deadlines.

Tracing that turned up a latent hard-lock: WalletConnectionProvider.connectWallet declared props? as optional while every implementation requires a connector, and WalletModal.connect() was the one caller invoking it bare. Against the six adapters that destructure the parameter (fuel, bitcoin, svm, ton, tron, starknet) that throws at the parameter list — outside their own try/catch — so connect() rejected before setOpen(true). With no try/finally in handleConnect, setIsLoading(false) never ran: spinner stuck on "Connecting…" forever, no modal, no error, page reload required.

Changes

Readiness consolidation

  • isProviderHydrated (fails closed) beside isProviderConnectReady (fails open), with the deliberate polarity difference documented — isProviderConnectReady(undefined) === true vs isProviderHydrated(undefined) === false.
  • useProvidersConnectReady, useConnectorSourcesStatus and useWalletProviderReadiness collapsed into wallet-core behind one PROVIDER_HYDRATION_TIMEOUT_MS. The two ui-kit hooks move out of the presentational layer.
  • Split so the predicates stay React-free: lib/providerReadiness.ts is importable from service classes (ParadexConnectionService) without pulling in React or the context graph; hooks live in hooks/useProviderReadiness.ts. Neither sits under lib/walletConnect/ any more — they describe Layerswap's WalletConnectionProvider, not the WalletConnect protocol.

connectWallet contract

  • Now requires { connector }, and the bare call is deleted. That branch could only fire when a provider had zero connectors (with even one, hasConnectorPicker is true), so there was never anything to pass — it was a guaranteed no-op or the crash. Removing it also removed the hasConnectorPicker computation that existed only to gate it.
  • Requiring the parameter makes any future bare call a compile error (TS2554: Expected 1 arguments, but got 0) with no tsconfig changes, since argument arity is always checked. All eight adapters are untouched.

Button behavior

  • handleConnect wrapped in try/finally so isLoading always clears; failures go through ErrorHandler as a WalletError.
  • Deadline keyed by provider id, so switching destination mid-hydration can't let a brand-new stub inherit the previous provider's expiry and render as ready with zero load time.
  • The provider is passed to the modal even while still a stub — awaitLiveProvider waits for the live provider, keeping the destination's connectors in scope instead of degrading to the generic list.
  • Disabled while initializing, with aria-busy and an aria-live status region.

OKX Wallet icon

  • starknetkit's injectedWalletIcons covers argentX, braavos, keplr, metamask, fordefi, xverse only, so okxwallet fell through to its generic placeholder (Fordefi rendered fine right next to it because it is in that map). The OKX mark is now registered in the shared icon map and supplied via the connector's icon option, which takes precedence over starknetkit's fallback. Artwork extracted from @bigmi/client's OKX connector — already a dependency, and the source of the same icon for OKX on Bitcoin — so it stays consistent across chains with no network fetch.

Verification

  • tsc --noEmit clean across wallet-core, all nine adapters, wallets/all, ui-kit and widget/core.
  • pnpm --filter @layerswap/wallet-core test → 4/4.
  • wallet-core, ui-kit and widget/core all build clean including declaration emit.
  • Exercised in-browser: reproduced the "Initializing…" state under 20× CPU throttling + slow network, confirmed the click path, that the modal opens scoped to the destination's connectors (Bitcoin: Xverse/Unisat/Ctrl/OKX/Bitget/Leather/OneKey — no Ethereum/Solana/Starknet), and cancel returns cleanly. Zero console errors, zero uncaught exceptions.
  • The OKX icon was verified by pulling the constant out of the built dist and rendering it.

Notes for review

  • The connectWallet contract change is the one to scrutinize — it tightens a shared type. Every call site is covered: useWalletConnection.ts:77 and both paradex sites already passed { connector }.
  • Behavior change worth knowing: the button is now disabled while initializing (deliberate), which suppresses the hover/focus prefetch during that window. The mount-time loadById for stubs is what hydrates them instead.
  • Latent and left alone: tsconfig.wallet.json sets strict: false, so strictFunctionTypes/strictBindCallApply are off and function-type variance mismatches are invisible. Enabling both surfaces ~9 pre-existing errors (bitcoin 1, evm 4, paradex 2, svm 1, ton 1), none connectWallet-related. Worth a focused follow-up.
  • lib/walletConnect/ still holds non-protocol machinery (createConnectionStoreFromHook, descriptorStubStore, walletProvidersRegistry, walletDescriptorLoader, connectorSource, mapWallet). Out of scope here, but the same relocation applies.

🤖 Generated with Claude Code

The connect button treated a descriptor stub as connect-ready, so clicking one
opened the generic wallet modal instead of the destination's own connectors.
Gate it on real hydration instead, and consolidate the readiness logic that had
accumulated in three places.

Readiness:
- Add `isProviderHydrated` (fails closed) beside `isProviderConnectReady`
  (fails open) and document the deliberate polarity difference.
- Collapse `useProvidersConnectReady`, `useConnectorSourcesStatus` and
  `useWalletProviderReadiness` into wallet-core, replacing four inline
  `isStub || !ready` predicates and three separate 5000ms deadlines with one
  `PROVIDER_HYDRATION_TIMEOUT_MS`. The two ui-kit hooks move out of the
  presentational layer.
- Split the module so the predicates stay React-free: `lib/providerReadiness.ts`
  is importable from service classes (ParadexConnectionService) without pulling
  in React or the context graph; the hooks live in `hooks/useProviderReadiness.ts`.
  Neither belongs under `lib/walletConnect/` — they are about Layerswap's
  WalletConnectionProvider, not the WalletConnect protocol.

connectWallet contract:
- `WalletConnectionProvider.connectWallet` now requires `{ connector }`. It
  declared `props?` optional while every implementation needs one, and
  `WalletModal.connect()` was the only caller invoking it bare — which threw
  inside adapters that destructure the parameter, rejecting before the modal
  opened and leaving the button stuck on "Connecting..." forever.
- Drop that call. It could only fire when a provider had zero connectors (with
  even one, `hasConnectorPicker` is true), so there was never anything to pass.
  Requiring the parameter makes any future bare call a compile error.

Button behavior:
- Wrap `handleConnect` in try/finally so `isLoading` always clears, and report
  failures through ErrorHandler instead of an unhandled rejection.
- Key the hydration deadline by provider id so a newly selected provider cannot
  inherit the previous one's expiry and render as ready with zero load time.
- Pass the provider to the modal even while it is still a stub;
  `awaitLiveProvider` waits for the live provider, keeping the destination's
  connectors in scope instead of degrading to the generic list.
- Add `aria-busy` and an `aria-live` status region for the initializing state.

OKX icon:
- starknetkit ships `injectedWalletIcons` for argentX, braavos, keplr, metamask,
  fordefi and xverse only, so `okxwallet` fell through to its generic
  placeholder. Register the OKX mark in the shared icon map and supply it via
  the connector's `icon` option, which takes precedence over that fallback.

Verified: all wallet/ui-kit/widget packages typecheck, wallet-core tests pass,
and the flow was exercised in-browser under CPU/network throttling to reproduce
the initializing state.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Aug 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
layerswapapp Ready Ready Preview Aug 20, 2026 3:25pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
layerswap-explorer Ignored Ignored Aug 20, 2026 3:25pm

Request Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant