Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a "Referrals" menu item to the user interface and updates the auto-registration hook to incorporate authentication status from Privy. A review comment identified potential race conditions where registration might occur before the user ID is available, as well as an issue where account switching wouldn't trigger a re-registration due to the persistent ref state.
| useEffect(() => { | ||
| async function autoRegister() { | ||
| if (!isConnected || !address || registeredRef.current) return; | ||
| if (!isConnected || !authenticated || !address || registeredRef.current) return; |
There was a problem hiding this comment.
The auto-registration logic has two potential issues:
- Race Condition: There is a possibility that
authenticatedbecomes true before theuserobject (and thususer.id) is fully loaded. If the effect runs at this moment, it will register the user without theirprivyUserId. SinceregisteredRef.currentis set totrueimmediately (on line 43), the hook won't retry onceuser.idbecomes available. - Account Switching: The
registeredRefpersists astrueeven if the walletaddresschanges. If a user switches accounts in their wallet without a page refresh, the new address will not be auto-registered because the guard clause will return early.
Consider waiting for user?.id when authenticated and ensuring the registration logic accounts for address changes.
| if (!isConnected || !authenticated || !address || registeredRef.current) return; | |
| if (!isConnected || !authenticated || !address || !user?.id || registeredRef.current) return; |
Deploying hyperterminal with
|
| Latest commit: |
d16ccf5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2d043c42.hyperterminal.pages.dev |
| Branch Preview URL: | https://fix-ui.hyperterminal.pages.dev |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds a 'Referrals' menu item to the user menu and refactors the referral auto-registration hook to improve reliability. Key changes include updating the logout flow to ensure the wallet disconnects correctly and enhancing the registration tracking logic in useAutoRegisterReferral. Review feedback suggests awaiting the asynchronous logout call to avoid race conditions and using a composite key of address and user ID for registration tracking to handle account switching.
| function handleLogout() { | ||
| if (authenticated) { | ||
| logout(); | ||
| } else { | ||
| disconnect(); | ||
| } | ||
| disconnect(); | ||
| } |
There was a problem hiding this comment.
The logout function from Privy is asynchronous. It is recommended to await it before disconnecting the wallet to ensure the authentication session is properly cleared and to avoid potential race conditions where the wallet is disconnected while Privy is still processing the logout.
| function handleLogout() { | |
| if (authenticated) { | |
| logout(); | |
| } else { | |
| disconnect(); | |
| } | |
| disconnect(); | |
| } | |
| async function handleLogout() { | |
| if (authenticated) { | |
| await logout(); | |
| } | |
| disconnect(); | |
| } |
| const registeredAddressRef = useRef<string | null>(null); | ||
|
|
||
| useEffect(() => { | ||
| async function autoRegister() { | ||
| if (!isConnected || !address || registeredRef.current) return; | ||
| if (!isConnected || !authenticated || !address) return; | ||
| if (authenticated && !user?.id) return; | ||
| if (registeredAddressRef.current === address) return; | ||
|
|
||
| registeredRef.current = true; | ||
| registeredAddressRef.current = address; |
There was a problem hiding this comment.
The current implementation only tracks the wallet address to prevent redundant registration calls. However, if a user switches their Privy account while using the same wallet address, the registration check will be skipped. Additionally, it's safer to check for the presence of API_URL before attempting to fetch. Using a composite key of address and user ID in the ref provides better tracking.
| const registeredAddressRef = useRef<string | null>(null); | |
| useEffect(() => { | |
| async function autoRegister() { | |
| if (!isConnected || !address || registeredRef.current) return; | |
| if (!isConnected || !authenticated || !address) return; | |
| if (authenticated && !user?.id) return; | |
| if (registeredAddressRef.current === address) return; | |
| registeredRef.current = true; | |
| registeredAddressRef.current = address; | |
| const registeredAddressRef = useRef<string | null>(null); | |
| useEffect(() => { | |
| async function autoRegister() { | |
| if (!isConnected || !authenticated || !address || !user?.id || !API_URL) return; | |
| const key = address + "-" + user.id; | |
| if (registeredAddressRef.current === key) return; | |
| registeredAddressRef.current = key; |
1 participant
No description provided.