fix(controllers): scope one-click auth chains to the connector's namespace - #5729
fix(controllers): scope one-click auth chains to the connector's namespace#5729svenvoskamp wants to merge 1 commit into
Conversation
…space
`WalletConnectConnector.authenticate()` passed `this.chains` to
`SIWXUtil.universalProviderAuthenticate`. That getter resolves to
`ChainController.getCaipNetworks()` with no namespace argument, i.e.
`getAllRequestedCaipNetworks()` — every registered namespace rather than
the connector's own, even though the connector is constructed per
namespace and already stores it as `this.chain`.
`universalProviderAuthenticate` then bails early:
const namespaces = new Set(chains.map(chain => chain.split(':')[0]))
if (!siwx || namespaces.size !== 1 || !namespaces.has('eip155')) {
return false
}
So registering any second namespace alongside EVM — Solana, for example —
made that set `{eip155, solana}` and silently disabled one-click auth for
every wallet. `connectWalletConnect()` fell back to `provider.connect()`
followed by a separate `personal_sign` round trip on every login, which is
markedly less reliable on mobile wallets; it reproduces as "Error signing
message" on Trust Wallet.
`getCaipNetworks()` already accepts a namespace, so pass `this.chain`.
Single-namespace apps are unaffected: the set was already of size one.
Scoped in `authenticate()` rather than in the `chains` getter because
`chains` is part of the public `ChainAdapterConnector` surface — the
bitcoin, tron and ton adapters read `connector.chains[0]` — so narrowing it
deserves its own change.
Added a regression test covering both an eip155 and a non-eip155 connector
while two namespaces are registered.
Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 246f056 The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
9 Skipped Deployments
|
Visual Regression Test Results ✅ PassedChromatic Build: https://www.chromatic.com/build?appId=6493191bf4b10fed8ca7353f&number=996 👉 Please review the visual changes in Chromatic and accept or reject them. |
📦 Bundle Size Check✅ All bundles are within size limits 📊 View detailed bundle sizes> @reown/appkit-monorepo@1.7.1 size /home/runner/work/appkit/appkit > size-limit |
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||
|
Closing this — the fix is wrong, and the failing E2E test is correct to reject it.
I had read the ChainController.setAccountProp(
'connectedWalletInfo',
walletInfo,
Array.from(namespaces)[0] as ChainNamespace
)So For the record, the problem that sent me here is real but lives elsewhere: on a multi-namespace app (EVM + Solana) with SIWX, mobile wallets always take the connect-then- Two things I noticed that may still be worth separate issues:
Sorry for the noise. |
1 participant
Description
WalletConnect one-click auth was silently skipped for every wallet as soon as a second chain namespace was registered.
WalletConnectConnector.authenticate()passedthis.chainstoSIWXUtil.universalProviderAuthenticate. That getter resolves toChainController.getCaipNetworks()with no namespace argument — i.e.getAllRequestedCaipNetworks(), every registered namespace — even though the connector is constructed per namespace and already stores it asthis.chain:universalProviderAuthenticatethen bails early (SIWXUtil.ts:391):So an app registering, say, Solana alongside EVM produced
{eip155, solana}→ size 2 →authenticate()returnedfalse, andconnectWalletConnect()fell back toprovider.connect()plus a separatepersonal_signround trip on every login. That fallback is markedly less reliable on mobile wallets — it reproduces as "Error signing message" on Trust Wallet mobile, where one-click auth would otherwise be used and works.getCaipNetworks()already accepts a namespace, so this passesthis.chain.Single-namespace apps are unaffected — the set was already size 1, so the value is identical.
Why
authenticate()and not thechainsgetterchainsis part of the publicChainAdapterConnectorsurface, and the bitcoin, tron and ton adapters readconnector.chains[0], so narrowing it deserves its own change. Two things worth noting for a follow-up:TonWalletConnectConnectorandTronWalletConnectConnectoralreadyoverride get chains()to scope by namespace, which suggests namespace-scoping is the intended semantics of the base getter and the eip155/solana connectors simply never got it.connector.chains[0]on a non-EVM connector can currently return a network from a different namespace.Also unused but not touched here:
WalletConnectConnector.Options.caipNetworksis accepted by the constructor and then discarded in favour ofthis.getCaipNetworks().Type of change
Associated Issues
None filed — found while debugging a multi-namespace (EVM + Solana) app with SIWX, and independently reproduced on SIWX in the laboratory app.
Showcase (Optional)
Effect on a hub registering 11 EVM chains plus Solana with
siwxandrequired: true:universalProviderAuthenticateeip155— before{eip155, solana}eip155— after{eip155}solana— after{solana}Checklist
Test notes
New
packages/controllers/tests/controllers/WalletConnectConnector.test.tscovers an eip155 and a non-eip155 connector while two namespaces are registered. Confirmed it is a real regression test — both cases fail onmainand pass with the fix.packages/controllers: 49/50 test files pass. The one failure,tests/features/ReownAuthentication.test.ts(25 tests), fails identically on unmodifiedmainin my environment — it expects a populatedprojectIdin request URLs — so it is pre-existing and unrelated.Not run locally:
eslintcrashes in anode:fscallback on my machine regardless of heap size, andtsc --noEmitinpackages/controllersreports 7 pre-existingChainNamespace/tronerrors from a stale localappkit-commonbuild. Neither implicates the changed files (0 errors reference them); leaving both to CI.I have not been able to verify the Trust Wallet mobile flow end-to-end against a preview build — happy to if someone can point me at one.
🤖 Generated with Claude Code