BlockAid/Balances Scan and Fetch Rate adjustments#505
Conversation
…passed since last focus
| onPoll(); | ||
| lastPollTimeRef.current = Date.now(); | ||
|
|
||
| pollingIntervalIdRef.current = setInterval(() => { |
There was a problem hiding this comment.
it seems like this will overwrite the timeout ID with the polling ID, will the timeout ever get cleaned up in clearAllTimers?
There was a problem hiding this comment.
it shouldn't be the case. the timers are cleared at the beginning of the useCallback before it gets overwritten
There was a problem hiding this comment.
but how will it clear both references if one ID overrides the other?
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [publicKey, network]); | ||
| useFocusedPolling({ | ||
| onPoll: handlePoll, |
There was a problem hiding this comment.
lastPollTimeRef starts at 0 so this should also act as an initial fetch on mount, is the mount effect redundant?
| const [pricedBalances, scanResult] = await Promise.all([ | ||
| fetchPricedBalances(set, balances, statePricedBalances, params), | ||
| scanBalances(set, balances, params.network), | ||
| Promise.resolve( | ||
| extractScanResultsFromBalances(balances, params.network), | ||
| ), | ||
| ]); |
There was a problem hiding this comment.
| const [pricedBalances, scanResult] = await Promise.all([ | |
| fetchPricedBalances(set, balances, statePricedBalances, params), | |
| scanBalances(set, balances, params.network), | |
| Promise.resolve( | |
| extractScanResultsFromBalances(balances, params.network), | |
| ), | |
| ]); | |
| const pricedBalances = await fetchPricedBalances(set, balances, statePricedBalances, params); | |
| const scanResults = extractScanResultsFromBalances(pricedBalances, params.network); |
Would it be a good call to make something like this? So we assure the scanResults are always reflecting the most up to date balances (e.g. in case the user has just added a new token).
There was a problem hiding this comment.
oh yes. makes sense
| export const DEFAULT_REFRESH_DELAY = 1000; | ||
|
|
||
| // Balances polling interval in milliseconds | ||
| export const BALANCES_FETCH_POLLING_INTERVAL = 60000; |
There was a problem hiding this comment.
since we are removing a redundant bulk-scan request on the frontend and only fetching on focus I think it'd be worth to keep it as 30secs so users don't need to wait too long to see a balance update
| setHasAttemptedInitialLoad(true); | ||
| setIsMounting(false); |
There was a problem hiding this comment.
I don't remember exactly why we've added this in the past, but I'd suggest testing this branch with new account creations (with no balances) and also test switching between wallet accounts while observing if there won't be any weird UI glitches with spinners flickering or stuff like that
There was a problem hiding this comment.
Seems like the behavior is close/the same to the main branch
Branch:
Screen.Recording.2025-10-29.at.12.54.33.mov
Main:
Screen.Recording.2025-10-29.at.12.57.20.mov
| clearTimeout(pollingIntervalIdRef.current); | ||
| clearInterval(pollingIntervalIdRef.current); |
There was a problem hiding this comment.
it looks confusing to have the same ref be a Timeout and an Interval at the same time. Could we use separate refs here?
| remainingTimeRef.current = interval; | ||
| startPolling(true); | ||
| } else { | ||
| pollingIntervalIdRef.current = setTimeout(() => { |
There was a problem hiding this comment.
could you add a comment explaining why we need to use setTimeout + setInterval here instead of calling startPolling(false)? Thanks
…ve redundant calls to fetchBalances
| // Set the "raw" balances right away as they don't depend on prices | ||
| set({ | ||
| balances, | ||
| isFunded: isFunded ?? false, | ||
| subentryCount: subentryCount ?? 0, | ||
| }); | ||
|
|
||
| // Get existing state priced balances to preserve price data |
There was a problem hiding this comment.
could we keep those comments? I think it helps clarifying things
closes #447
We were previously having rate limiting issues with Blockaid. It is known that they have already increased the rate limits on their APIs, but the improvements aim to ease it for the future as well. The 429 Errors were no longer reproducible by the time this ticket was started
Main changes are:
General Changes:
Screen.Recording.2025-10-28.at.09.25.41.mov
Returned data from balances:
Example of focused polling with start/stop logic on logs - set to 10s for testing, instead of 60s:
Screen.Recording.2025-10-28.at.10.39.58.mov
PR structure
Testing
These changes have been tested and confirmed to work as intended on small iOS screens.These changes have been tested and confirmed to work as intended on small Android screens.Release