-
Notifications
You must be signed in to change notification settings - Fork 24
perf(ui): collapse queued timeline reconciles, and measure what is left #990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 3 commits
55b2572
48a1a8f
4e62aaa
00dbee7
a7670c3
9f6c276
5e84693
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ | |
|
|
||
| import Combine | ||
| import Foundation | ||
| import OSLog | ||
| import SwiftDashSDK | ||
|
|
||
| private let kMaxProgressDelta = 0.1 // 10% | ||
|
|
@@ -304,9 +305,37 @@ extension SyncingActivityMonitor { | |
|
|
||
| applyProgressWithPeakSmoothing(sdkProgress) | ||
| isSyncing = (mapped == .syncing) | ||
| let wasDone = state == .syncDone | ||
| state = mapped | ||
|
|
||
| // `syncDone` is derived purely from the SPV network phases — it knows | ||
| // nothing about how much of what was scanned is durably persisted. | ||
| // The two can be far apart: the durable watermark is what a relaunch | ||
| // resumes from, and what the transaction list is built out of, so a | ||
| // wallet can report "synced" while rows are still materializing. | ||
| // | ||
| // Logged at the transition rather than gated on, because whether the | ||
| // watermark reliably reaches the tip is exactly what is unproven. One | ||
| // line per completion answers it from an ordinary session. | ||
| if mapped == .syncDone && !wasDone { | ||
| logDurableWatermarkAtCompletion(scannedTip: sdkSyncProgress.headers?.currentHeight ?? 0) | ||
| } | ||
| } | ||
|
|
||
| /// One-shot read of the persisted sync height at the moment the UI first | ||
| /// calls a sync complete, next to the height that was actually scanned. | ||
| /// A single fetch on a state transition — not a per-tick cost. | ||
| private func logDurableWatermarkAtCompletion(scannedTip: UInt32) { | ||
| guard let durable = SwiftDashSDKWalletSource.persistedSyncedHeight() else { | ||
| Self.logger.warning("⛓️ SYNCSTATE :: reported done at tip \(scannedTip, privacy: .public); durable watermark unavailable") | ||
| return | ||
| } | ||
| let behind = scannedTip > durable ? scannedTip - durable : 0 | ||
| Self.logger.info("⛓️ SYNCSTATE :: reported done — scanned tip \(scannedTip, privacy: .public), durable watermark \(durable, privacy: .public), behind by \(behind, privacy: .public) block(s)") | ||
|
Comment on lines
+327
to
+340
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Preserve unavailable and contradictory heights in the diagnostic. When The PR objective is to report the scanned tip, durable watermark, and block difference, so these fallback values should not hide missing data. 🤖 Prompt for AI AgentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Wrap the completion log message. Line 334 exceeds the repository's 180-character Swift line limit. Break the message construction across shorter source lines while retaining the logged fields. As per coding guidelines, Swift files must use 4-space indentation and a 180-character line limit (100 recommended). 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| } | ||
|
|
||
| private static let logger = Logger(subsystem: "org.dashfoundation.dash", category: "sync-state") | ||
|
|
||
| /// Map SwiftDashSDK's per-phase progress to the snapshot fields the | ||
| /// existing UI consumers expect. The phase priority order | ||
| /// (headers → filterHeaders → filters → masternodes → finished) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: dashpay/dashwallet-ios
Length of output: 160
🏁 Script executed:
Repository: dashpay/dashwallet-ios
Length of output: 50379
🏁 Script executed:
Repository: dashpay/dashwallet-ios
Length of output: 50379
🏁 Script executed:
Repository: dashpay/dashwallet-ios
Length of output: 50379
🏁 Script executed:
Repository: dashpay/dashwallet-ios
Length of output: 50379
Bind completion logging to the subscribed wallet ID.
SyncingActivityMonitor.sharedreceives singleton SPV updates without awalletId, whilepersistedSyncedHeight()reads the active wallet at log time. After a wallet switch, a queued.syncDoneupdate can read the new wallet’s height or suppress its first completion log. Associate each completion with the wallet ID captured when its SPV progress subscription starts.🤖 Prompt for AI Agents
Source: Learnings