Feature: support safe login within app#22
Conversation
There was a problem hiding this comment.
Pull request overview
Adds Safe (Gnosis Safe) in-app detection so the library can automatically initialize an EIP-1193 provider and account when embedded as a Safe App.
Changes:
- Added
SafeSyncto detect Safe App environment and wire upSafeAppProvider+ account/chain state. - Extended the Web3 account context with
isSafeAppplus a newuseIsSafeApp()hook. - Added
@safe-global/safe-apps-sdkand@safe-global/safe-apps-providerdependencies.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/Web3AccountContext.tsx |
Implements Safe App detection/sync and exposes isSafeApp via context + hook. |
package.json |
Adds Safe SDK/provider dependencies needed for Safe App integration. |
package-lock.json |
Locks the newly added Safe packages and transitive deps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return; | ||
| } | ||
| // Safe is handled by SafeSync, not here | ||
| if (savedRdns === 'safe') { |
There was a problem hiding this comment.
The new savedRdns === 'safe' early-return means that if a user previously opened the app inside Safe (so web3Account-chosenEip1193ProviderRdns is persisted as safe) and later opens it in a normal top-level browser window, autoReconnectProvider will never clear the saved value or set web3Account to null. Because SafeSync also bails out when not in an iframe, this can leave web3Account stuck as undefined and isRestoringSession stuck true indefinitely. Consider clearing the stored rdns (and setting web3Account/eip1193Provider to null) when Safe detection is not possible or fails, or only skipping auto-reconnect when Safe detection has positively succeeded in this session.
| if (savedRdns === 'safe') { | |
| if (savedRdns === 'safe') { | |
| // If we're not running inside an iframe, Safe cannot handle this session. | |
| // Clear the persisted Safe rdns and reset state so we don't get stuck restoring. | |
| const isIframe = typeof window !== 'undefined' && window.self !== window.top; | |
| if (!isIframe) { | |
| props.localStorageClient.removeValue('web3Account-chosenEip1193ProviderRdns'); | |
| setWeb3Account(null); | |
| setEip1193Provider(null); | |
| } |
| } | ||
| }; | ||
| detectSafe(); | ||
| }, [props]); |
There was a problem hiding this comment.
SafeSync's effect depends on [props], which will typically change identity every render and can cause repeated Safe detection attempts (multiple dynamic imports/timeouts) until hasSetAccountRef.current flips. It would be more stable to depend on the specific props used (or none, if they are stable) and/or memoize the callbacks passed into SafeSync to avoid rerunning the detection work unnecessarily.
| }, [props]); | |
| }, [ | |
| props.localStorageClient, | |
| props.setEip1193Provider, | |
| props.setWeb3Account, | |
| props.setWeb3ChainId, | |
| props.setIsSafeApp, | |
| props.setLoginCount, | |
| ]); |
| hasSetAccountRef.current = true; | ||
| props.setIsSafeApp(true); | ||
| const safeProvider = new SafeAppProvider(safeInfo, safeSDK); | ||
| const eip1193Provider = safeProvider as unknown as Eip1193Provider; | ||
| props.setEip1193Provider(eip1193Provider); | ||
| props.localStorageClient.setValue('web3Account-chosenEip1193ProviderRdns', 'safe'); | ||
| const browserProvider = new EthersBrowserProvider(eip1193Provider); | ||
| const signer = await browserProvider.getSigner(); | ||
| const address = await signer.getAddress(); | ||
| props.setWeb3Account({ address, signer }); | ||
| props.setWeb3ChainId(safeInfo.chainId); |
There was a problem hiding this comment.
hasSetAccountRef.current is set to true before the async signer/address/chainId sync completes. If any of the subsequent awaits throw (e.g., provider instantiation or getSigner()), the ref will prevent further retries and can leave state partially updated (e.g., isSafeApp true but no account). Consider only setting the ref after the critical state is fully established, or resetting/cleaning up state in the catch path so the app can recover.
Description
Screenshots:
Checklist: