fix(walletapi): simplify sync loop to zero-SCID-only polling#30
Open
moralpriest wants to merge 1 commit into
Open
fix(walletapi): simplify sync loop to zero-SCID-only polling#30moralpriest wants to merge 1 commit into
moralpriest wants to merge 1 commit into
Conversation
Remove the test_connectivity() gate from the sync loop condition. IsDaemonOnline() is sufficient to determine connection state since it checks both WS and RPC client availability. Remove EntriesNative iteration from the sync loop. Previously the loop synced every known token SCID on each iteration, making many sequential RPC calls. Now it only syncs the native DERO (zero SCID). Token balances sync on demand when wallet API callers explicitly request them. This change prevents a concurrent map iteration and write panic when running SyncHistory as a background goroutine (which also accesses EntriesNative).
Dirtybird99
reviewed
Jul 11, 2026
Dirtybird99
left a comment
There was a problem hiding this comment.
The !IsDaemonOnline() guard flip is a genuine improvement (the old code fell through to a failing sync attempt when the socket was nil), and Sync_Wallet_Memory_With_Daemon() is exactly internal(zerohash), so the registered-no-entries path is unchanged. Three qualifications:
- "Token balances sync on demand when wallet API callers request them" holds for the wallet JSON-RPC server (
rpc_getbalance.gosyncs the SCID first) but not for library consumers callingGet_Balance_scid()/Get_Balance()directly — those return the stored map value with no sync, so embedded-walletapi UIs will show stale token balances indefinitely, and token history stops updating outside RPC GetBalance calls. Deserves a loud release-note callout. - "Prevents concurrent map iteration and write panic" — this removes the
sync_looprange, but the second race from #26 remains:Save_Wallet()'sjson.Marshal(w.account)iteratesEntriesNativeunlocked while a backgroundSyncHistorywrites it (details on #26). - For unregistered wallets,
daemon_heightnow freezes after connect (details and a suggested fix on #29).
Also worth stating explicitly in the description: #26 must not merge before this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Simplify the sync loop to only sync the native DERO (zero SCID) and remove the test_connectivity() gate from the loop condition.
Problem
The previous sync loop made RPC calls for every known token SCID on each iteration. This added overhead and created a concurrent map access issue when SyncHistory ran in the background (see PR #26). Additionally, the loop condition used test_connectivity() which made redundant RPC calls every 3 seconds.
Changes
Notes