Bug: connecting to a previous connected wallet #618 #622
Bug: connecting to a previous connected wallet #618 #622edehvictor wants to merge 3 commits intoGoodDollar:masterfrom
Conversation
There was a problem hiding this comment.
Hey - I've found 1 security issue, and left some high level feedback:
Security issues:
- Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (link)
General comments:
- In the
switchChainuseCallback, you're now relying onchainId,toggleNetworkModal, andsetToAddNetworkbut they are not included in the dependency array; consider adding them (or explicitly justifying why they are intentionally omitted) to avoid stale values after re-renders. - The error handling in
switchChainassumese.codeande.messageexist; to make this more robust, consider defensive access (e.g., optional chaining or normalizing the error into a known shape) before branching oncodeor sendingmessageto analytics.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the `switchChain` useCallback, you're now relying on `chainId`, `toggleNetworkModal`, and `setToAddNetwork` but they are not included in the dependency array; consider adding them (or explicitly justifying why they are intentionally omitted) to avoid stale values after re-renders.
- The error handling in `switchChain` assumes `e.code` and `e.message` exist; to make this more robust, consider defensive access (e.g., optional chaining or normalizing the error into a known shape) before branching on `code` or sending `message` to analytics.
## Individual Comments
### Comment 1
<location> `.env.example:12` </location>
<code_context>
phc_placeholder_key_123
</code_context>
<issue_to_address>
**security (generic-api-key):** Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
*Source: gitleaks*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
63d0f8e to
977ec30
Compare
|
@edehvictor Can you reset your commit and only commit the changes that you did. git reset |
|
Alright @L03TJ3, on it |
|
@edehvictor can you include a different format then a gif for demo? |
|
@sirpy tested to be working. after before requested changes (including demo-video, removing lingui included files) |
977ec30 to
9f4d774
Compare
|
Hi @L03TJ3, I've reset the branch and pushed a clean commit for #618. I've ensured the lingui files are excluded to keep the PR focused on the logic. Demo Video: Ready for review! |
Description
Summary of Change:
Resolved an issue where network switching was non-functional or ignored following a page refresh. The root cause was a synchronization lag in the AppKit initialized state upon hydration.
Motivation and Context:
Previously, returning users who refreshed the DApp were stuck on their current network because the initialization gate blocked the switchNetwork call. I've introduced a robust fallback in the switchChain callback that allows the app to recognize the user's wallet connection via the active address, even if the full provider state hasn't finished hydrating.
Key Updates:
Refined the catch block in NetworkModal to handle cases where initialized is false but a wallet is present.
Utilized setSelectedChain(chain) as a deferred preference to ensure UX continuity.
Preserved existing Error 4902 handling to prompt users to add missing networks (like Fuse or XDC).
Fixed Issue: Fixes #618
How Has This Been Tested?
Reload Test: Connected to Celo, refreshed the page, and verified that switching to Fuse immediately triggered the MetaMask "Add/Switch Network" prompt.
Persistence Test: Verified that the wallet remains connected and responsive across multiple reloads.
Modal UI Guard: Verified that the "Connect Wallet" modal still functions correctly when the user is fully disconnected.
Checklist:
[x] PR title matches follow: Bug: connecting to a previous connected wallet #618
[x] My code follows the style guidelines of this project
[x] I have followed all the instructions described in the initial task
[x] I have performed a self-review of my own code
[x] My changes generate no new warnings
[x] I have added reference to a related issue in the repository
[x] I have added a detailed description of the changes proposed
[x] I have pasted a gif showing the feature.

[x] @L03TJ3
Summary by Sourcery
Improve network switching robustness and tracking when reconnecting to previously connected wallets after refresh.
Bug Fixes:
Enhancements: