fix: bind all public methods in the FormoAnalytics constructor - #334
Merged
Conversation
The provider passes the SDK instance directly as the React context
value, so useFormo consumers can destructure methods
(const { reset } = useFormo()). Only 9 methods were bound; page,
reset, cleanup, the consent methods, and the public-on-class helpers
lost `this` when destructured, and reset crashed with "Cannot set
property currentUserId of undefined" (reported on the RN SDK, same
gap here).
Bind every public method in the constructor and add a regression
spec that asserts each is a bound own property and that a
destructured reset() works.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Merged
yosriady
added a commit
that referenced
this pull request
Aug 24, 2026
…ndings Five findings, four real, one of which forced a better design. - P1: announcement-driven cleanup untracked anything absent from the announcement list, and a registered provider is NEVER announced - the next wallet announcement stopped its events. Registered providers are now exempt (WeakSet), with a test that announces MetaMask after registration and proves the registered provider still captures. - registerProvider reported success even when the request wrapper failed to install, which would recreate the silent loss the API exists to close. Adoption now happens only after tracking is confirmed, and the method returns the tracker's verdict. - registerProvider is bound in the constructor like every other public method (the #334 rule), so destructuring from useFormo() works. - The peer-name cache was connector-keyed with no invalidation, so a reconnect to a DIFFERENT wallet kept the old name. Connection-keying, the obvious fix, turned out worse: the only per-session reader fires before any lookup can resolve, so the name never surfaced at all (and an awaited lookup is not an option - every path from store signal to emission is synchronous by design, and adding an await demonstrably dropped connects under rapid connect/disconnect cycles). Final design: names keyed by CONNECTOR so they serve reads, lookups guarded per CONNECTION so every new session re-resolves and overwrites. A wallet switch behind WalletConnect can mislabel at most the one event between the new session's start and its resolution, then self-corrects; the spec pins both halves. - The new spec restores global descriptors instead of deleting keys Node itself defines (deleting Event/addEventListener broke 23 later tests - caught locally, never landed). 1133 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
yosriady
added a commit
that referenced
this pull request
Aug 27, 2026
…ming (#364) * feat: registerProvider for constructed wallets, and peer naming for WalletConnect Closes the P-2403 gap. Two changes, one cause: WalletConnect is invisible to the SDK on both axes. Discovery covers EIP-6963 announcements and window.ethereum - every injected wallet and nothing else. A WalletConnect or Ledger provider is CONSTRUCTED by the app and announces nothing, so its whole session was silently untracked: production shows one WalletConnect detect event in 30 days against 81k for MetaMask. registerProvider(provider, info?) is the missing entry point: the identical pipeline a discovered provider takes (registry, detect event, lifecycle listeners, request wrapper), plus adoption of a session that already exists at registration, seeded from the provider's SYNCHRONOUS accounts state - no RPC ever goes on the wallet transport, and WalletConnect's serialised relay socket is the very case that rule exists for. Refused in wagmi mode, where the connector system already tracks the session and wrapping the same provider twice would double-report. Second axis: WalletConnect is a transport, not a wallet. The signing wallet (Ledger Live, MetaMask Mobile, Safe, ...) names itself in the session's peer metadata, and reporting provider_name 'WalletConnect' hides every wallet behind it - Ledger reads as zero in production while its sessions were tracked under the transport's name. Both paths now name the peer: registerProvider reads it synchronously at registration; the wagmi path resolves it through the connector's async getProvider(), fire-and-forget and cached, because connect emission is deliberately synchronous and must never wait. The first wagmi connect may honestly say 'WalletConnect'; every event after resolution names the real wallet. Twelve new tests: the pinned today's-failure case (unregistered constructed provider produces nothing), end-to-end capture through the wrapped request, no-RPC session adoption, peer naming, caller overrides, flag fallback, idempotency, wagmi refusal, and the wagmi peer cache including the never-blocks guarantee. 1130 passing. Bundle 55.98 kB brotlied; budget raised 55.5 -> 56.5 KB for the two new capture capabilities. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: harden registerProvider and the peer cache against the review findings Five findings, four real, one of which forced a better design. - P1: announcement-driven cleanup untracked anything absent from the announcement list, and a registered provider is NEVER announced - the next wallet announcement stopped its events. Registered providers are now exempt (WeakSet), with a test that announces MetaMask after registration and proves the registered provider still captures. - registerProvider reported success even when the request wrapper failed to install, which would recreate the silent loss the API exists to close. Adoption now happens only after tracking is confirmed, and the method returns the tracker's verdict. - registerProvider is bound in the constructor like every other public method (the #334 rule), so destructuring from useFormo() works. - The peer-name cache was connector-keyed with no invalidation, so a reconnect to a DIFFERENT wallet kept the old name. Connection-keying, the obvious fix, turned out worse: the only per-session reader fires before any lookup can resolve, so the name never surfaced at all (and an awaited lookup is not an option - every path from store signal to emission is synchronous by design, and adding an await demonstrably dropped connects under rapid connect/disconnect cycles). Final design: names keyed by CONNECTOR so they serve reads, lookups guarded per CONNECTION so every new session re-resolves and overwrites. A wallet switch behind WalletConnect can mislabel at most the one event between the new session's start and its resolution, then self-corrects; the spec pins both halves. - The new spec restores global descriptors instead of deleting keys Node itself defines (deleting Event/addEventListener broke 23 later tests - caught locally, never landed). 1133 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: second review round on registerProvider Four more findings, all real: - Registered instances shared an rdns-derived uuid, and EIP-6963 consumers (mipd included) deduplicate on uuid, so two WalletConnect registrations would collapse into one. Each registration now gets crypto.randomUUID(), with a monotonic fallback. - A provider that defeats the request wrapper left its already-attached lifecycle listeners behind, leaking callbacks that hold the instance for the life of the page. The failure branch now unwinds through untrackProvider before returning false. - A previous session's slow getProvider() could resolve after the new session's and overwrite the fresh peer name with the old wallet's. Only the newest kicked connection's resolution may write. - The frozen-provider test's tolerant else-branch was dead code: wrapping reassigns provider.request, so a frozen provider deterministically fails. The test now asserts the refusal outcome flatly. Three further comments re-reported the pre-fix diff (cleanup exemption, success verdict, connector staleness); already fixed in the previous two commits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: route wrapped requests to the live SDK instance, resolve peers per read Codex review round, three findings, all real. The middle one is a pre-existing bug bigger than this PR. - The request wrapper survives an SDK rebuild (nothing restores provider.request) and closes over the instance that installed it, whose event queue is CLOSED after cleanup. Re-registration saw 'already wrapped', reported success, and every request-derived event died silently in the dead instance's queue - for ANY provider, window.ethereum included, not just registered ones. The wrapper now reads an owner slot per call and routes to the current instance; re-registration rebinds the slot. A test rebuilds the SDK over the same provider and proves the second instance captures. - Peer metadata was captured only at registration, so the recommended register-early-then-connect order froze the generic 'WalletConnect' name forever. The registry now resolves the peer LIVE per read (only over the generic name - caller overrides stay), so a session formed after registration names its signer on every event from then on. Tested. - The wagmi peer cache could lag a session behind on connect events. Mitigated with kicks at every flow entry (status, address change, seed); the residual window is one generic-or-previous name on a session's first connect, bounded by the ordering guard, documented rather than hidden - an in-flow await is not an option (it reorders transitions and drops connects, measured). Bundle 56.59 kB; budget 56.5 -> 57 KB: the owner routing is ~0.4 kB and fixes silent event loss on every SDK rebuild. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: owner list, live-only peer naming, and lookup-failure invalidation Second codex round, four findings, all real: - A single owner slot was last-registration-wins: with two live instances (multi write-key pages), cleaning up the newest routed every request event into its closed queue while an older instance sat live. The slot is now the LIST of registrants and the wrapper dispatches to the newest one still live, so a cleanup degrades to newest-live-wins. The list also fixes the frozen-after-wrap rebind: it is attached at install time, so re-registration mutates it without touching the provider - and a wrapper somehow missing its list refuses the registration rather than claiming success. - registerProvider baked the registration-time peer name into stored metadata, which infoFor's generic-name guard then never replaced - a provider reused across sessions stayed attributed to its first wallet forever. Stored metadata now stays generic ('WalletConnect'); every read resolves the peer live, the detect event included, and a caller's explicit name still wins. - A new wagmi session whose getProvider() rejects kept serving the previous wallet's cached name. A failed lookup now drops the entry (guarded so an old session's late failure cannot clear a newer resolution); generic is honest, stale is not. Three new tests: cross-session renaming on the 1193 path, newest-live routing across two instances, and cache invalidation on lookup failure. 1138 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * style: avoid this-aliasing in the owner dispatch Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: prune owner lists on cleanup, bound every stale-name window Third codex round, three P1 claims: two real, one documented as the long-standing single-observer semantics. - Disposed trackers stayed in the owner lists on long-lived provider objects, retaining each old instance's whole object graph across rebuilds - unbounded under HMR. cleanup() now prunes the instance from every list it joined; a test counts the list down to zero. - A hung lookup, or one resolving WITHOUT peer metadata, kept serving the previous wallet's name to the new session. Both now invalidate: no-peer resolutions delete the entry (disproven, not just unproven), and a 3s grace timer clears an unsettled lookup's inherited name, guarded against out-of-order clears. - Multi write-key pages registering the SAME provider keep the request wrapper's long-standing single-observer semantics, upgraded from first-registration-wins to newest-LIVE-wins; lifecycle events reach every instance either way. Documented at the API; full fan-out of request observations is a separate feature, not a silent half-build. 1139 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: merge ownership when a request replacement forces a re-wrap Fourth codex round, one finding: a wallet that replaces provider.request defeats the wrapper marker and forces a fresh wrap, and that install overwrote the owner list with [this], discarding every other live registrant - after the re-wrapper's cleanup, requests routed to a disposed tracker while a live instance sat unowned. The install now merges into any existing list. Tested: two registrants, a wallet-forced re-wrap, the re-wrapper torn down, the remaining instance still captures. 1140 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: re-verify the request wrapper on every registration; pin the KyberSwap-over-WalletConnect shape in e2e Fifth codex round, one finding: an already-tracked provider skipped the tracking pipeline on re-registration, so after a wallet replaced provider.request, registerProvider returned true on the strength of lifecycle listeners alone while request capture stayed dead. adoptExternalProvider now re-verifies the wrapper every time - it reinstalls a displaced wrapper, rebinds an intact one, and refuses when it cannot - and the multi-instance re-wrap test drives the PUBLIC path. Also lands the end-to-end proof the KyberSwap complaint asked for, as four permanent behaviour rows in a new walletconnect harness mode. The provider models the real EthereumProvider (plain accounts array, numeric chainId, session.peer metadata - shapes verified against the published package) and the session exists BEFORE the SDK does: unregistered -> [] (the pre-364 gap, pinned) registered -> detect + connect, both named ~Ledger Live (adoption) signature -> requested + confirmed transaction -> started + broadcasted + confirmed sessionSwap -> disconnect + connect ~MetaMask Mobile (live renaming) 19 rows in the table now; all pass against the built branch. 1140 unit tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: verify the wrapper actually installed before reporting success Sixth codex round, one finding: an accessor or Proxy can ACCEPT the request assignment without storing it, letting registration report success while capture stays dead. The install now reads the property back and refuses when the assignment was swallowed. Tested with a swallowing setter. 1141 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: a flagless provider's late session still upgrades its identity Seventh codex round, one finding, real and in the recommended flow: a WalletConnect-compatible provider with no isWalletConnect flag, registered BEFORE its session exists, detects as 'Injected Provider' - and the live-peer gate only upgraded the name 'WalletConnect', so the signer never surfaced and the rdns stayed io.injected.provider forever. The gate now accepts both generic names: the peer appearing later is itself the proof of what the provider was, so name AND rdns upgrade together. Caller-set names still win. The delayed-session test now drives the flagless path (the flag it set was masking exactly this) and asserts the rdns. The real v2 EthereumProvider is unaffected either way: its isWalletConnect is a truthy method. 1141 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: settle the size budget at 57.5 KB The branch grows the bundle ~1.5 kB total: registerProvider, live peer naming on both paths, and owner-routed wrappers that fix silent event loss on every SDK rebuild. Two log strings trimmed; further shaving is brotli noise at this granularity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: layered wrappers instrument once; failed peer lookups retry Triage of the GitHub bots' six comments on the loop's intermediate commits: two stale (fixed by the owner-list design and live naming), one accepted and documented (a caller name of exactly 'WalletConnect' is the generic transport name, so the live peer still replaces it), one duplicate pair, and two real residuals, both fixed: - Layered wrappers double-dispatched after a rebuild: a third-party library wraps OUR wrapper, the rebuilt instance wraps the outer function, and both layers routed to the same live tracker - one user request, doubled events. Every dispatch issues its wallet call synchronously by design, so an in-flight WeakSet spanning exactly that synchronous window lets the inner layer pass straight through; a concurrent request can never be inside the window. Tested: chained wrappers, rebuild, one request, exactly two signature events. - A peer lookup that failed or resolved before the session populated its peer left the connection permanently disqualified from retrying, so the session stayed generic (or stale) forever. Failure and no-peer outcomes now clear the lookup guard; the next event retries. 1142 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: two findings from the live MetaMask Mobile pairing A real phone approving a real WalletConnect session found two things no mock, harness, or nine review rounds had: - provider.accounts was EMPTY on the live session while the approved account sat in session.namespaces ('eip155:11155111:0x...'), so registerProvider's adoption saw nothing and the restored session's connect never fired. Adoption now falls back to the namespaces - the session's ground truth - still via synchronous property reads only. - The transaction the user cancelled on the phone produced NO rejected event: WalletConnect wallets reject with sdkError USER_REJECTED {code: 5000} (and 5001-5005 variants), not EIP-1193's 4001, and the SDK matched 4001 alone - every WalletConnect rejection on either path was silently uncounted. Rejection detection is now one shared predicate (4001, the 5000 family, viem's typed error, cause-chain walked) used by the 1193 wrapper's three sites and the wagmi handler. Both verified against the live session's actual shapes; two new tests pin them. 1144 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: four review findings on the live-test delta - P1, and it overturns a trade-off defended for three rounds: the connect flow reads the peer cache in the same tick it kicks the lookup, so a retained name DETERMINISTICALLY attributed a reconnect-to-a-different- wallet to the old wallet. A new connection now clears the cache synchronously; wrong is worse than generic. Honest consequence, documented in the code: no wagmi event today observably carries the resolved name - the cache exists for the mid-session attribution work (wallet names on signature/transaction events), which fires after resolution. - The namespaces fallback iterated EVERY namespace; a session carrying Solana first would feed a non-EVM address into EVM adoption, fail validation, and drop the whole adoption. eip155 only now. - registerProvider after cleanup() attached listeners to a terminally closed instance and reported success. A cleaned-up instance refuses. - A registration while tracking was suppressed (opt-out, excluded route) had its adoption refused with nothing to retry it - a live session may never emit accountsChanged again. Adoption now retries on opt-in and on navigation, idempotently. 1146 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: adoption prefers the active chain's account Ported from the example's review: a WalletConnect session can authorize DIFFERENT accounts per chain, and the namespaces fallback took whichever eip155 entry came first. Entries matching the provider's active chain now come first (hex or numeric chainId), so the adopted address is the one this chain actually authorized. Tested with a per-chain session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: guard retries after cleanup, correct a pre-pairing detect, parse hex chains properly Review round on the previous fixes: the retry paths could drive a torn-down tracker when cleanup raced the opt-in timer or a page hit (guarded on isCleanedUp); a flagless provider registered before pairing kept its generic detect forever (the retry now emits the corrected WalletConnect detect once the peer proves the identity, deduped by the session rdns marker); and the active-chain prefix parsed hex manually, mishandling uppercase 0X (shared parseChainId now). 1147 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
yosriady
added a commit
that referenced
this pull request
Aug 27, 2026
* feat: registerProvider for constructed wallets, and peer naming for WalletConnect Closes the P-2403 gap. Two changes, one cause: WalletConnect is invisible to the SDK on both axes. Discovery covers EIP-6963 announcements and window.ethereum - every injected wallet and nothing else. A WalletConnect or Ledger provider is CONSTRUCTED by the app and announces nothing, so its whole session was silently untracked: production shows one WalletConnect detect event in 30 days against 81k for MetaMask. registerProvider(provider, info?) is the missing entry point: the identical pipeline a discovered provider takes (registry, detect event, lifecycle listeners, request wrapper), plus adoption of a session that already exists at registration, seeded from the provider's SYNCHRONOUS accounts state - no RPC ever goes on the wallet transport, and WalletConnect's serialised relay socket is the very case that rule exists for. Refused in wagmi mode, where the connector system already tracks the session and wrapping the same provider twice would double-report. Second axis: WalletConnect is a transport, not a wallet. The signing wallet (Ledger Live, MetaMask Mobile, Safe, ...) names itself in the session's peer metadata, and reporting provider_name 'WalletConnect' hides every wallet behind it - Ledger reads as zero in production while its sessions were tracked under the transport's name. Both paths now name the peer: registerProvider reads it synchronously at registration; the wagmi path resolves it through the connector's async getProvider(), fire-and-forget and cached, because connect emission is deliberately synchronous and must never wait. The first wagmi connect may honestly say 'WalletConnect'; every event after resolution names the real wallet. Twelve new tests: the pinned today's-failure case (unregistered constructed provider produces nothing), end-to-end capture through the wrapped request, no-RPC session adoption, peer naming, caller overrides, flag fallback, idempotency, wagmi refusal, and the wagmi peer cache including the never-blocks guarantee. 1130 passing. Bundle 55.98 kB brotlied; budget raised 55.5 -> 56.5 KB for the two new capture capabilities. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: harden registerProvider and the peer cache against the review findings Five findings, four real, one of which forced a better design. - P1: announcement-driven cleanup untracked anything absent from the announcement list, and a registered provider is NEVER announced - the next wallet announcement stopped its events. Registered providers are now exempt (WeakSet), with a test that announces MetaMask after registration and proves the registered provider still captures. - registerProvider reported success even when the request wrapper failed to install, which would recreate the silent loss the API exists to close. Adoption now happens only after tracking is confirmed, and the method returns the tracker's verdict. - registerProvider is bound in the constructor like every other public method (the #334 rule), so destructuring from useFormo() works. - The peer-name cache was connector-keyed with no invalidation, so a reconnect to a DIFFERENT wallet kept the old name. Connection-keying, the obvious fix, turned out worse: the only per-session reader fires before any lookup can resolve, so the name never surfaced at all (and an awaited lookup is not an option - every path from store signal to emission is synchronous by design, and adding an await demonstrably dropped connects under rapid connect/disconnect cycles). Final design: names keyed by CONNECTOR so they serve reads, lookups guarded per CONNECTION so every new session re-resolves and overwrites. A wallet switch behind WalletConnect can mislabel at most the one event between the new session's start and its resolution, then self-corrects; the spec pins both halves. - The new spec restores global descriptors instead of deleting keys Node itself defines (deleting Event/addEventListener broke 23 later tests - caught locally, never landed). 1133 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: second review round on registerProvider Four more findings, all real: - Registered instances shared an rdns-derived uuid, and EIP-6963 consumers (mipd included) deduplicate on uuid, so two WalletConnect registrations would collapse into one. Each registration now gets crypto.randomUUID(), with a monotonic fallback. - A provider that defeats the request wrapper left its already-attached lifecycle listeners behind, leaking callbacks that hold the instance for the life of the page. The failure branch now unwinds through untrackProvider before returning false. - A previous session's slow getProvider() could resolve after the new session's and overwrite the fresh peer name with the old wallet's. Only the newest kicked connection's resolution may write. - The frozen-provider test's tolerant else-branch was dead code: wrapping reassigns provider.request, so a frozen provider deterministically fails. The test now asserts the refusal outcome flatly. Three further comments re-reported the pre-fix diff (cleanup exemption, success verdict, connector staleness); already fixed in the previous two commits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: route wrapped requests to the live SDK instance, resolve peers per read Codex review round, three findings, all real. The middle one is a pre-existing bug bigger than this PR. - The request wrapper survives an SDK rebuild (nothing restores provider.request) and closes over the instance that installed it, whose event queue is CLOSED after cleanup. Re-registration saw 'already wrapped', reported success, and every request-derived event died silently in the dead instance's queue - for ANY provider, window.ethereum included, not just registered ones. The wrapper now reads an owner slot per call and routes to the current instance; re-registration rebinds the slot. A test rebuilds the SDK over the same provider and proves the second instance captures. - Peer metadata was captured only at registration, so the recommended register-early-then-connect order froze the generic 'WalletConnect' name forever. The registry now resolves the peer LIVE per read (only over the generic name - caller overrides stay), so a session formed after registration names its signer on every event from then on. Tested. - The wagmi peer cache could lag a session behind on connect events. Mitigated with kicks at every flow entry (status, address change, seed); the residual window is one generic-or-previous name on a session's first connect, bounded by the ordering guard, documented rather than hidden - an in-flow await is not an option (it reorders transitions and drops connects, measured). Bundle 56.59 kB; budget 56.5 -> 57 KB: the owner routing is ~0.4 kB and fixes silent event loss on every SDK rebuild. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: owner list, live-only peer naming, and lookup-failure invalidation Second codex round, four findings, all real: - A single owner slot was last-registration-wins: with two live instances (multi write-key pages), cleaning up the newest routed every request event into its closed queue while an older instance sat live. The slot is now the LIST of registrants and the wrapper dispatches to the newest one still live, so a cleanup degrades to newest-live-wins. The list also fixes the frozen-after-wrap rebind: it is attached at install time, so re-registration mutates it without touching the provider - and a wrapper somehow missing its list refuses the registration rather than claiming success. - registerProvider baked the registration-time peer name into stored metadata, which infoFor's generic-name guard then never replaced - a provider reused across sessions stayed attributed to its first wallet forever. Stored metadata now stays generic ('WalletConnect'); every read resolves the peer live, the detect event included, and a caller's explicit name still wins. - A new wagmi session whose getProvider() rejects kept serving the previous wallet's cached name. A failed lookup now drops the entry (guarded so an old session's late failure cannot clear a newer resolution); generic is honest, stale is not. Three new tests: cross-session renaming on the 1193 path, newest-live routing across two instances, and cache invalidation on lookup failure. 1138 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * style: avoid this-aliasing in the owner dispatch Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: prune owner lists on cleanup, bound every stale-name window Third codex round, three P1 claims: two real, one documented as the long-standing single-observer semantics. - Disposed trackers stayed in the owner lists on long-lived provider objects, retaining each old instance's whole object graph across rebuilds - unbounded under HMR. cleanup() now prunes the instance from every list it joined; a test counts the list down to zero. - A hung lookup, or one resolving WITHOUT peer metadata, kept serving the previous wallet's name to the new session. Both now invalidate: no-peer resolutions delete the entry (disproven, not just unproven), and a 3s grace timer clears an unsettled lookup's inherited name, guarded against out-of-order clears. - Multi write-key pages registering the SAME provider keep the request wrapper's long-standing single-observer semantics, upgraded from first-registration-wins to newest-LIVE-wins; lifecycle events reach every instance either way. Documented at the API; full fan-out of request observations is a separate feature, not a silent half-build. 1139 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: merge ownership when a request replacement forces a re-wrap Fourth codex round, one finding: a wallet that replaces provider.request defeats the wrapper marker and forces a fresh wrap, and that install overwrote the owner list with [this], discarding every other live registrant - after the re-wrapper's cleanup, requests routed to a disposed tracker while a live instance sat unowned. The install now merges into any existing list. Tested: two registrants, a wallet-forced re-wrap, the re-wrapper torn down, the remaining instance still captures. 1140 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: re-verify the request wrapper on every registration; pin the KyberSwap-over-WalletConnect shape in e2e Fifth codex round, one finding: an already-tracked provider skipped the tracking pipeline on re-registration, so after a wallet replaced provider.request, registerProvider returned true on the strength of lifecycle listeners alone while request capture stayed dead. adoptExternalProvider now re-verifies the wrapper every time - it reinstalls a displaced wrapper, rebinds an intact one, and refuses when it cannot - and the multi-instance re-wrap test drives the PUBLIC path. Also lands the end-to-end proof the KyberSwap complaint asked for, as four permanent behaviour rows in a new walletconnect harness mode. The provider models the real EthereumProvider (plain accounts array, numeric chainId, session.peer metadata - shapes verified against the published package) and the session exists BEFORE the SDK does: unregistered -> [] (the pre-364 gap, pinned) registered -> detect + connect, both named ~Ledger Live (adoption) signature -> requested + confirmed transaction -> started + broadcasted + confirmed sessionSwap -> disconnect + connect ~MetaMask Mobile (live renaming) 19 rows in the table now; all pass against the built branch. 1140 unit tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: verify the wrapper actually installed before reporting success Sixth codex round, one finding: an accessor or Proxy can ACCEPT the request assignment without storing it, letting registration report success while capture stays dead. The install now reads the property back and refuses when the assignment was swallowed. Tested with a swallowing setter. 1141 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: a flagless provider's late session still upgrades its identity Seventh codex round, one finding, real and in the recommended flow: a WalletConnect-compatible provider with no isWalletConnect flag, registered BEFORE its session exists, detects as 'Injected Provider' - and the live-peer gate only upgraded the name 'WalletConnect', so the signer never surfaced and the rdns stayed io.injected.provider forever. The gate now accepts both generic names: the peer appearing later is itself the proof of what the provider was, so name AND rdns upgrade together. Caller-set names still win. The delayed-session test now drives the flagless path (the flag it set was masking exactly this) and asserts the rdns. The real v2 EthereumProvider is unaffected either way: its isWalletConnect is a truthy method. 1141 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: settle the size budget at 57.5 KB The branch grows the bundle ~1.5 kB total: registerProvider, live peer naming on both paths, and owner-routed wrappers that fix silent event loss on every SDK rebuild. Two log strings trimmed; further shaving is brotli noise at this granularity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: layered wrappers instrument once; failed peer lookups retry Triage of the GitHub bots' six comments on the loop's intermediate commits: two stale (fixed by the owner-list design and live naming), one accepted and documented (a caller name of exactly 'WalletConnect' is the generic transport name, so the live peer still replaces it), one duplicate pair, and two real residuals, both fixed: - Layered wrappers double-dispatched after a rebuild: a third-party library wraps OUR wrapper, the rebuilt instance wraps the outer function, and both layers routed to the same live tracker - one user request, doubled events. Every dispatch issues its wallet call synchronously by design, so an in-flight WeakSet spanning exactly that synchronous window lets the inner layer pass straight through; a concurrent request can never be inside the window. Tested: chained wrappers, rebuild, one request, exactly two signature events. - A peer lookup that failed or resolved before the session populated its peer left the connection permanently disqualified from retrying, so the session stayed generic (or stale) forever. Failure and no-peer outcomes now clear the lookup guard; the next event retries. 1142 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: two findings from the live MetaMask Mobile pairing A real phone approving a real WalletConnect session found two things no mock, harness, or nine review rounds had: - provider.accounts was EMPTY on the live session while the approved account sat in session.namespaces ('eip155:11155111:0x...'), so registerProvider's adoption saw nothing and the restored session's connect never fired. Adoption now falls back to the namespaces - the session's ground truth - still via synchronous property reads only. - The transaction the user cancelled on the phone produced NO rejected event: WalletConnect wallets reject with sdkError USER_REJECTED {code: 5000} (and 5001-5005 variants), not EIP-1193's 4001, and the SDK matched 4001 alone - every WalletConnect rejection on either path was silently uncounted. Rejection detection is now one shared predicate (4001, the 5000 family, viem's typed error, cause-chain walked) used by the 1193 wrapper's three sites and the wagmi handler. Both verified against the live session's actual shapes; two new tests pin them. 1144 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: four review findings on the live-test delta - P1, and it overturns a trade-off defended for three rounds: the connect flow reads the peer cache in the same tick it kicks the lookup, so a retained name DETERMINISTICALLY attributed a reconnect-to-a-different- wallet to the old wallet. A new connection now clears the cache synchronously; wrong is worse than generic. Honest consequence, documented in the code: no wagmi event today observably carries the resolved name - the cache exists for the mid-session attribution work (wallet names on signature/transaction events), which fires after resolution. - The namespaces fallback iterated EVERY namespace; a session carrying Solana first would feed a non-EVM address into EVM adoption, fail validation, and drop the whole adoption. eip155 only now. - registerProvider after cleanup() attached listeners to a terminally closed instance and reported success. A cleaned-up instance refuses. - A registration while tracking was suppressed (opt-out, excluded route) had its adoption refused with nothing to retry it - a live session may never emit accountsChanged again. Adoption now retries on opt-in and on navigation, idempotently. 1146 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: wallet attribution on signature and transaction events The live pairing exposed it plainly: every signature and transaction row landed with provider_name EMPTY, so 'how many transactions came through Ledger' was unanswerable in the warehouse for ANY wallet. Attribution existed only on detect/connect/disconnect/identify. Both capture paths now attribute request-derived events: - EIP-1193: providerName + rdns from the registry, resolved live per event, so a WalletConnect session names its actual signer ('Ledger Live', 'MetaMask Wallet') on every signature, transaction, and batch call - including receipt-poll confirmations. - wagmi: providerName from the connector, peer-cache aware. This is the peer cache's FIRST observable consumer: mid-session events fire after the lookup resolves, so a WalletConnect connector's signatures and transactions name the signer even though its connect stays generic (the invalidation rule from review). Attribution is captured at broadcast on pending records so a connection change before the receipt cannot relabel the confirmation. The walletconnect e2e row now asserts ~Ledger Live on the full signature and transaction streams. 1148 unit tests; 15 e2e rows; browser suite green; 57.45 kB against the 57.5 budget. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: adoption prefers the active chain's account Ported from the example's review: a WalletConnect session can authorize DIFFERENT accounts per chain, and the namespaces fallback took whichever eip155 entry came first. Entries matching the provider's active chain now come first (hex or numeric chainId), so the adopted address is the one this chain actually authorized. Tested with a per-chain session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: guard retries after cleanup, correct a pre-pairing detect, parse hex chains properly Review round on the previous fixes: the retry paths could drive a torn-down tracker when cleanup raced the opt-in timer or a page hit (guarded on isCleanedUp); a flagless provider registered before pairing kept its generic detect forever (the retry now emits the corrected WalletConnect detect once the peer proves the identity, deduped by the session rdns marker); and the active-chain prefix parsed hex manually, mishandling uppercase 0X (shared parseChainId now). 1147 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: budget 58 KB - the 364 merge left 4 bytes of headroom debt Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
yosriady
added a commit
that referenced
this pull request
Aug 27, 2026
…367) * feat: registerProvider for constructed wallets, and peer naming for WalletConnect Closes the P-2403 gap. Two changes, one cause: WalletConnect is invisible to the SDK on both axes. Discovery covers EIP-6963 announcements and window.ethereum - every injected wallet and nothing else. A WalletConnect or Ledger provider is CONSTRUCTED by the app and announces nothing, so its whole session was silently untracked: production shows one WalletConnect detect event in 30 days against 81k for MetaMask. registerProvider(provider, info?) is the missing entry point: the identical pipeline a discovered provider takes (registry, detect event, lifecycle listeners, request wrapper), plus adoption of a session that already exists at registration, seeded from the provider's SYNCHRONOUS accounts state - no RPC ever goes on the wallet transport, and WalletConnect's serialised relay socket is the very case that rule exists for. Refused in wagmi mode, where the connector system already tracks the session and wrapping the same provider twice would double-report. Second axis: WalletConnect is a transport, not a wallet. The signing wallet (Ledger Live, MetaMask Mobile, Safe, ...) names itself in the session's peer metadata, and reporting provider_name 'WalletConnect' hides every wallet behind it - Ledger reads as zero in production while its sessions were tracked under the transport's name. Both paths now name the peer: registerProvider reads it synchronously at registration; the wagmi path resolves it through the connector's async getProvider(), fire-and-forget and cached, because connect emission is deliberately synchronous and must never wait. The first wagmi connect may honestly say 'WalletConnect'; every event after resolution names the real wallet. Twelve new tests: the pinned today's-failure case (unregistered constructed provider produces nothing), end-to-end capture through the wrapped request, no-RPC session adoption, peer naming, caller overrides, flag fallback, idempotency, wagmi refusal, and the wagmi peer cache including the never-blocks guarantee. 1130 passing. Bundle 55.98 kB brotlied; budget raised 55.5 -> 56.5 KB for the two new capture capabilities. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: harden registerProvider and the peer cache against the review findings Five findings, four real, one of which forced a better design. - P1: announcement-driven cleanup untracked anything absent from the announcement list, and a registered provider is NEVER announced - the next wallet announcement stopped its events. Registered providers are now exempt (WeakSet), with a test that announces MetaMask after registration and proves the registered provider still captures. - registerProvider reported success even when the request wrapper failed to install, which would recreate the silent loss the API exists to close. Adoption now happens only after tracking is confirmed, and the method returns the tracker's verdict. - registerProvider is bound in the constructor like every other public method (the #334 rule), so destructuring from useFormo() works. - The peer-name cache was connector-keyed with no invalidation, so a reconnect to a DIFFERENT wallet kept the old name. Connection-keying, the obvious fix, turned out worse: the only per-session reader fires before any lookup can resolve, so the name never surfaced at all (and an awaited lookup is not an option - every path from store signal to emission is synchronous by design, and adding an await demonstrably dropped connects under rapid connect/disconnect cycles). Final design: names keyed by CONNECTOR so they serve reads, lookups guarded per CONNECTION so every new session re-resolves and overwrites. A wallet switch behind WalletConnect can mislabel at most the one event between the new session's start and its resolution, then self-corrects; the spec pins both halves. - The new spec restores global descriptors instead of deleting keys Node itself defines (deleting Event/addEventListener broke 23 later tests - caught locally, never landed). 1133 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: second review round on registerProvider Four more findings, all real: - Registered instances shared an rdns-derived uuid, and EIP-6963 consumers (mipd included) deduplicate on uuid, so two WalletConnect registrations would collapse into one. Each registration now gets crypto.randomUUID(), with a monotonic fallback. - A provider that defeats the request wrapper left its already-attached lifecycle listeners behind, leaking callbacks that hold the instance for the life of the page. The failure branch now unwinds through untrackProvider before returning false. - A previous session's slow getProvider() could resolve after the new session's and overwrite the fresh peer name with the old wallet's. Only the newest kicked connection's resolution may write. - The frozen-provider test's tolerant else-branch was dead code: wrapping reassigns provider.request, so a frozen provider deterministically fails. The test now asserts the refusal outcome flatly. Three further comments re-reported the pre-fix diff (cleanup exemption, success verdict, connector staleness); already fixed in the previous two commits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: route wrapped requests to the live SDK instance, resolve peers per read Codex review round, three findings, all real. The middle one is a pre-existing bug bigger than this PR. - The request wrapper survives an SDK rebuild (nothing restores provider.request) and closes over the instance that installed it, whose event queue is CLOSED after cleanup. Re-registration saw 'already wrapped', reported success, and every request-derived event died silently in the dead instance's queue - for ANY provider, window.ethereum included, not just registered ones. The wrapper now reads an owner slot per call and routes to the current instance; re-registration rebinds the slot. A test rebuilds the SDK over the same provider and proves the second instance captures. - Peer metadata was captured only at registration, so the recommended register-early-then-connect order froze the generic 'WalletConnect' name forever. The registry now resolves the peer LIVE per read (only over the generic name - caller overrides stay), so a session formed after registration names its signer on every event from then on. Tested. - The wagmi peer cache could lag a session behind on connect events. Mitigated with kicks at every flow entry (status, address change, seed); the residual window is one generic-or-previous name on a session's first connect, bounded by the ordering guard, documented rather than hidden - an in-flow await is not an option (it reorders transitions and drops connects, measured). Bundle 56.59 kB; budget 56.5 -> 57 KB: the owner routing is ~0.4 kB and fixes silent event loss on every SDK rebuild. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: owner list, live-only peer naming, and lookup-failure invalidation Second codex round, four findings, all real: - A single owner slot was last-registration-wins: with two live instances (multi write-key pages), cleaning up the newest routed every request event into its closed queue while an older instance sat live. The slot is now the LIST of registrants and the wrapper dispatches to the newest one still live, so a cleanup degrades to newest-live-wins. The list also fixes the frozen-after-wrap rebind: it is attached at install time, so re-registration mutates it without touching the provider - and a wrapper somehow missing its list refuses the registration rather than claiming success. - registerProvider baked the registration-time peer name into stored metadata, which infoFor's generic-name guard then never replaced - a provider reused across sessions stayed attributed to its first wallet forever. Stored metadata now stays generic ('WalletConnect'); every read resolves the peer live, the detect event included, and a caller's explicit name still wins. - A new wagmi session whose getProvider() rejects kept serving the previous wallet's cached name. A failed lookup now drops the entry (guarded so an old session's late failure cannot clear a newer resolution); generic is honest, stale is not. Three new tests: cross-session renaming on the 1193 path, newest-live routing across two instances, and cache invalidation on lookup failure. 1138 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * style: avoid this-aliasing in the owner dispatch Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: prune owner lists on cleanup, bound every stale-name window Third codex round, three P1 claims: two real, one documented as the long-standing single-observer semantics. - Disposed trackers stayed in the owner lists on long-lived provider objects, retaining each old instance's whole object graph across rebuilds - unbounded under HMR. cleanup() now prunes the instance from every list it joined; a test counts the list down to zero. - A hung lookup, or one resolving WITHOUT peer metadata, kept serving the previous wallet's name to the new session. Both now invalidate: no-peer resolutions delete the entry (disproven, not just unproven), and a 3s grace timer clears an unsettled lookup's inherited name, guarded against out-of-order clears. - Multi write-key pages registering the SAME provider keep the request wrapper's long-standing single-observer semantics, upgraded from first-registration-wins to newest-LIVE-wins; lifecycle events reach every instance either way. Documented at the API; full fan-out of request observations is a separate feature, not a silent half-build. 1139 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: merge ownership when a request replacement forces a re-wrap Fourth codex round, one finding: a wallet that replaces provider.request defeats the wrapper marker and forces a fresh wrap, and that install overwrote the owner list with [this], discarding every other live registrant - after the re-wrapper's cleanup, requests routed to a disposed tracker while a live instance sat unowned. The install now merges into any existing list. Tested: two registrants, a wallet-forced re-wrap, the re-wrapper torn down, the remaining instance still captures. 1140 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: re-verify the request wrapper on every registration; pin the KyberSwap-over-WalletConnect shape in e2e Fifth codex round, one finding: an already-tracked provider skipped the tracking pipeline on re-registration, so after a wallet replaced provider.request, registerProvider returned true on the strength of lifecycle listeners alone while request capture stayed dead. adoptExternalProvider now re-verifies the wrapper every time - it reinstalls a displaced wrapper, rebinds an intact one, and refuses when it cannot - and the multi-instance re-wrap test drives the PUBLIC path. Also lands the end-to-end proof the KyberSwap complaint asked for, as four permanent behaviour rows in a new walletconnect harness mode. The provider models the real EthereumProvider (plain accounts array, numeric chainId, session.peer metadata - shapes verified against the published package) and the session exists BEFORE the SDK does: unregistered -> [] (the pre-364 gap, pinned) registered -> detect + connect, both named ~Ledger Live (adoption) signature -> requested + confirmed transaction -> started + broadcasted + confirmed sessionSwap -> disconnect + connect ~MetaMask Mobile (live renaming) 19 rows in the table now; all pass against the built branch. 1140 unit tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: verify the wrapper actually installed before reporting success Sixth codex round, one finding: an accessor or Proxy can ACCEPT the request assignment without storing it, letting registration report success while capture stays dead. The install now reads the property back and refuses when the assignment was swallowed. Tested with a swallowing setter. 1141 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: a flagless provider's late session still upgrades its identity Seventh codex round, one finding, real and in the recommended flow: a WalletConnect-compatible provider with no isWalletConnect flag, registered BEFORE its session exists, detects as 'Injected Provider' - and the live-peer gate only upgraded the name 'WalletConnect', so the signer never surfaced and the rdns stayed io.injected.provider forever. The gate now accepts both generic names: the peer appearing later is itself the proof of what the provider was, so name AND rdns upgrade together. Caller-set names still win. The delayed-session test now drives the flagless path (the flag it set was masking exactly this) and asserts the rdns. The real v2 EthereumProvider is unaffected either way: its isWalletConnect is a truthy method. 1141 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: settle the size budget at 57.5 KB The branch grows the bundle ~1.5 kB total: registerProvider, live peer naming on both paths, and owner-routed wrappers that fix silent event loss on every SDK rebuild. Two log strings trimmed; further shaving is brotli noise at this granularity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: layered wrappers instrument once; failed peer lookups retry Triage of the GitHub bots' six comments on the loop's intermediate commits: two stale (fixed by the owner-list design and live naming), one accepted and documented (a caller name of exactly 'WalletConnect' is the generic transport name, so the live peer still replaces it), one duplicate pair, and two real residuals, both fixed: - Layered wrappers double-dispatched after a rebuild: a third-party library wraps OUR wrapper, the rebuilt instance wraps the outer function, and both layers routed to the same live tracker - one user request, doubled events. Every dispatch issues its wallet call synchronously by design, so an in-flight WeakSet spanning exactly that synchronous window lets the inner layer pass straight through; a concurrent request can never be inside the window. Tested: chained wrappers, rebuild, one request, exactly two signature events. - A peer lookup that failed or resolved before the session populated its peer left the connection permanently disqualified from retrying, so the session stayed generic (or stale) forever. Failure and no-peer outcomes now clear the lookup guard; the next event retries. 1142 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: two findings from the live MetaMask Mobile pairing A real phone approving a real WalletConnect session found two things no mock, harness, or nine review rounds had: - provider.accounts was EMPTY on the live session while the approved account sat in session.namespaces ('eip155:11155111:0x...'), so registerProvider's adoption saw nothing and the restored session's connect never fired. Adoption now falls back to the namespaces - the session's ground truth - still via synchronous property reads only. - The transaction the user cancelled on the phone produced NO rejected event: WalletConnect wallets reject with sdkError USER_REJECTED {code: 5000} (and 5001-5005 variants), not EIP-1193's 4001, and the SDK matched 4001 alone - every WalletConnect rejection on either path was silently uncounted. Rejection detection is now one shared predicate (4001, the 5000 family, viem's typed error, cause-chain walked) used by the 1193 wrapper's three sites and the wagmi handler. Both verified against the live session's actual shapes; two new tests pin them. 1144 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: four review findings on the live-test delta - P1, and it overturns a trade-off defended for three rounds: the connect flow reads the peer cache in the same tick it kicks the lookup, so a retained name DETERMINISTICALLY attributed a reconnect-to-a-different- wallet to the old wallet. A new connection now clears the cache synchronously; wrong is worse than generic. Honest consequence, documented in the code: no wagmi event today observably carries the resolved name - the cache exists for the mid-session attribution work (wallet names on signature/transaction events), which fires after resolution. - The namespaces fallback iterated EVERY namespace; a session carrying Solana first would feed a non-EVM address into EVM adoption, fail validation, and drop the whole adoption. eip155 only now. - registerProvider after cleanup() attached listeners to a terminally closed instance and reported success. A cleaned-up instance refuses. - A registration while tracking was suppressed (opt-out, excluded route) had its adoption refused with nothing to retry it - a live session may never emit accountsChanged again. Adoption now retries on opt-in and on navigation, idempotently. 1146 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: wallet attribution on signature and transaction events The live pairing exposed it plainly: every signature and transaction row landed with provider_name EMPTY, so 'how many transactions came through Ledger' was unanswerable in the warehouse for ANY wallet. Attribution existed only on detect/connect/disconnect/identify. Both capture paths now attribute request-derived events: - EIP-1193: providerName + rdns from the registry, resolved live per event, so a WalletConnect session names its actual signer ('Ledger Live', 'MetaMask Wallet') on every signature, transaction, and batch call - including receipt-poll confirmations. - wagmi: providerName from the connector, peer-cache aware. This is the peer cache's FIRST observable consumer: mid-session events fire after the lookup resolves, so a WalletConnect connector's signatures and transactions name the signer even though its connect stays generic (the invalidation rule from review). Attribution is captured at broadcast on pending records so a connection change before the receipt cannot relabel the confirmation. The walletconnect e2e row now asserts ~Ledger Live on the full signature and transaction streams. 1148 unit tests; 15 e2e rows; browser suite green; 57.45 kB against the 57.5 budget. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: adoption prefers the active chain's account Ported from the example's review: a WalletConnect session can authorize DIFFERENT accounts per chain, and the namespaces fallback took whichever eip155 entry came first. Entries matching the provider's active chain now come first (hex or numeric chainId), so the adopted address is the one this chain actually authorized. Tested with a per-chain session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: guard retries after cleanup, correct a pre-pairing detect, parse hex chains properly Review round on the previous fixes: the retry paths could drive a torn-down tracker when cleanup raced the opt-in timer or a page hit (guarded on isCleanedUp); a flagless provider registered before pairing kept its generic detect forever (the retry now emits the corrected WalletConnect detect once the peer proves the identity, deduped by the session rdns marker); and the active-chain prefix parsed hex manually, mishandling uppercase 0X (shared parseChainId now). 1147 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: hybrid capture - wagmi mode instruments the connector's provider Wagmi mode watched the store and caches, which see HOOK-driven calls only. Imperative viem calls (walletClient.sendTransaction, .signMessage, .writeContract, raw request) create no mutation and were silently lost. The audit of a real integration (KyberSwap) put numbers on it: the entire cross-chain adapter family, treasury sends, and the OAuth LOGIN signature - invisible, with an app-side mutation bridge as the only workaround, and every future call site a fresh chance to forget it. Every viem client in a wagmi app is built on the connector's EIP-1193 provider, so wagmi mode now installs the SAME request wrapper the 1193 path uses on that provider (resolved fire-and-forget per connection). Lifecycle stays store-driven; only requests route through the wrapper. Double counting is prevented deterministically: TanStack dispatches a mutation's pending state BEFORE its mutationFn issues the wallet call (verified against query-core), so a hook-driven request always finds a matching pending mutation and the wrapper stands down - the mutation handler keeps the capture and its ABI enrichment. An imperative call never matches and the wrapper captures it. Matching is by mutation type with cheap parameter refinement (sendTransaction compares ), erring toward NOT skipping: a duplicate is visible, a silent loss is not. Confirmations cannot double - each layer settles only hashes it captured, behind the existing observed-hash gates. Consequence for integrators: wagmi mode now captures BOTH call styles with zero app changes. The KyberSwap mutation bridge becomes unnecessary; registerProvider remains for non-wagmi apps. Five tests drive the real SDK in wagmi mode: imperative sign and send captured end to end, hook-owned requests skipped, different-transaction and settled mutations not over-matched. 1154 passing. Budget 57.5 -> 58.5 KB for the feature. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: make hybrid capture opt-in, off by default Reopened from the closed first cut with the design decision honored: wagmi mode's BASELINE never touches the signing transport. Instrumenting the connector's provider now requires an explicit options.wagmi.captureImperative: true - an auditable configuration decision, not an implied behavior change on upgrade. Default-off is pinned by a test. Threat-model note, stated honestly: this option constrains HONEST code, not a compromised SDK - malicious code ignores options and can intercept any in-page provider regardless of whether a wrapper feature exists. The real defenses against supply-chain compromise are the ones already in force (npm provenance, two runtime dependencies, pinned CI actions, the size budget as a tamper canary) plus integrator-side pinning. What the flag provides is a security POSTURE: teams can attest the SDK runs in observe-only mode, and enabling transport instrumentation is a reviewed, deliberate act. 1155 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: rename the opt-in to wagmi.eip1193Fallback Clearer name for what it is: falling back to the EIP-1193 request wrapper for wallet calls the wagmi caches cannot see. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Summary
Same class of bug as the customer-reported RN SDK crash (companion PR: getformo/sdk-react-native#85): the provider passes the
FormoAnalyticsinstance directly as the React context value, souseFormoconsumers can destructure methods off it. Only 9 methods were bound in the constructor.page,reset,cleanup,optOutTracking,optInTracking,hasOptedOutTracking, and the public-on-class helpers (syncPrivyActiveChain,isTrackingSuppressed,getTrackedProvidersCount,getProviderState,syncWalletState) lostthiswhen destructured. A destructuredreset()crashed withCannot set property currentUserId of undefined.Changes
test/methodBinding.spec.ts: each public method must be a bound own property of the instance, and a destructuredreset()must clear identity state without a throw.Testing
pnpm test: 772 tests, all pass.examples/with-react(CRA build + Playwright): with published 1.35.0, a destructuredreset()crashes withCannot set properties of undefined (setting 'currentUserId'). With this branch's build overlaid, the SDK initializes and the same destructuredreset()succeeds.🤖 Generated with Claude Code