fix: resolve Safe wallet approval loop in staking flow#40
fix: resolve Safe wallet approval loop in staking flow#40BowTiedSwan wants to merge 4 commits intomainfrom
Conversation
…ully loaded before evaluating stake amounts. Enhance user experience by updating loading states and implementing polling for allowance verification after approval transactions.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on January 18
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| // Keep current state and return false to avoid false positives | ||
| // This prevents the approval loop issue with Safe wallets | ||
| console.log("Data still loading - maintaining current approval state"); | ||
| return needsApproval; // Return current state instead of assuming |
There was a problem hiding this comment.
Bug: Stale closure returns outdated approval state value
The checkAndUpdateApprovalNeeded callback returns needsApproval when data is loading, but needsApproval is not included in the useCallback dependencies at line 705. This creates a stale closure where the returned value reflects the state when the callback was created, not the current state. Additionally, the comment on line 675 states "return false to avoid false positives" but the code returns needsApproval which could be true, contradicting the stated intent of preventing the approval loop issue.
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on January 18
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Bug: Duplicate useEffects cause double calls to approval check
After adding !isLoadingData to the first useEffect's condition and isLoadingData to its dependencies, both useEffects at lines 568-574 and 576-583 now have identical conditions and dependencies. This causes checkAndUpdateApprovalNeeded to be invoked twice whenever stakeAmount, checkAndUpdateApprovalNeeded, or isLoadingData changes, resulting in redundant state updates and potential race conditions.
app/builders/[slug]/page.tsx#L567-L583
…on. Implement logging for detected wallets, improve configuration for injected wallets, and add troubleshooting documentation for common connection issues.
| showEnhancedLoadingToast("Confirm approval in wallet...", "approval-tx"); | ||
| } | ||
| if (isApproveTxSuccess) { | ||
| if (isApproveTxSuccess && lastApprovedAmount) { |
There was a problem hiding this comment.
Bug: BigInt zero is falsy causing stuck UI state
The condition isApproveTxSuccess && lastApprovedAmount uses lastApprovedAmount in a boolean context where BigInt 0n is falsy. If a user enters a sub-wei amount (e.g., less than 0.000000000000000001), parseFloat validation passes but parseEther rounds to 0n. When the approval transaction succeeds, the success handling block is skipped because 0n is falsy, so resetApproveContract() is never called and the "Confirm approval in wallet..." loading toast remains indefinitely, leaving the UI stuck in a pending state.
| // Improve wallet detection for injected wallets like Rabby | ||
| multiInjectedProviderDiscovery: true, | ||
| // Increase connection timeout for slower wallets | ||
| connectors: [], |
There was a problem hiding this comment.
Bug: Empty connectors array may break wallet connections
The comment states "Increase connection timeout for slower wallets" but the actual code sets connectors: [] (an empty array), which doesn't configure any timeout. Passing an empty connectors array to defaultWagmiConfig could override default wallet connectors and potentially break wallet connection functionality. The code doesn't match the stated intent in the comment, suggesting either incomplete implementation or accidentally committed code.
…tic reconnection on app load, cleaning up expired proposals, and improving error handling for connection timeouts. Add user-friendly toast notifications for connection issues and streamline localStorage management for WalletConnect data.
| changeUnsub?.(); | ||
| disconnectUnsub?.(); | ||
| errorUnsub?.(); | ||
| }; |
There was a problem hiding this comment.
Bug: Connector event listeners never properly unsubscribed
The connector event subscription code assumes connector.emitter?.on?.(...) returns an unsubscribe function, storing the results in variables like connectUnsub. However, wagmi's emitter follows standard EventEmitter patterns where on() returns void or undefined, not an unsubscribe function. The cleanup calls connectUnsub?.() etc., which are no-ops since these values are undefined. Event listeners are never removed, causing memory leaks and potentially stale handlers continuing to execute after component re-renders. The correct cleanup pattern would use connector.emitter?.off?.(event, handler).
There was a problem hiding this comment.
Bug: Duplicate useEffects trigger double approval checks
Two consecutive useEffect hooks at lines 568-574 and 576-583 now have identical dependency arrays [stakeAmount, checkAndUpdateApprovalNeeded, isLoadingData] and identical conditional logic. After this PR added the !isLoadingData check to the first effect, both effects perform the same operation. This causes checkAndUpdateApprovalNeeded() to be called twice whenever any dependency changes, leading to redundant API calls and potentially confusing race conditions.
app/builders/[slug]/page.tsx#L567-L583
1 participant
Problem
Users with Safe (multi-sig) wallets were stuck in an infinite approval loop when staking MOR tokens. After approving token spending, the UI continued to request another approval instead of allowing them to stake. Page reloads made the issue worse, requiring fresh approvals each time.
Root Cause
Solution
1. Removed Aggressive Assumptions
checkAndUpdateApprovalNeeded()to maintain current state instead of assuming approval needed when data is loading2. Implemented Allowance Polling with Safe Detection
pollAllowanceUntilUpdated()that automatically detects Safe wallets3. Added Loading State Guards
4. Improved Button Text
Testing
Files Changed
hooks/useStakingContractInteractions.ts- Core polling logic and approval state managementapp/builders/[slug]/page.tsx- Loading guards and UI state improvementsImpact
Users can now successfully approve and stake with Safe wallets in a single approval. No more infinite loops or page reload issues.
Note
Fixes Safe wallet approval loop by deferring approval checks and polling allowance; adds wallet detection, WalletConnect cleanup/reconnect, and UI loading guards.
pollAllowanceUntilUpdated) and tracklastApprovedAmountto confirm approvals.isLoadingDataand tx states.multiInjectedProviderDiscoveryand featured wallets in Web3Modal.docs/RABBY_WALLET_TROUBLESHOOTING.md; minor doc touch-ups.Written by Cursor Bugbot for commit c991dfb. This will update automatically on new commits. Configure here.