fix(dashpay): notice identity keys made on another device - #981
Merged
Conversation
The "Enable DashPay" intro on the Contacts tab decided from the local SwiftData key rows alone, so an identity whose DIP-15 pair was added on another device kept showing the intro forever - offering an IdentityUpdate that would no-op. - When the local rows claim keys are missing, an async check against Platform's authoritative identityGetKeys corrects the intro on screen load (single-flight; a per-session cache skips re-querying once the pair is confirmed - Platform never silently removes keys). The key evaluation is shared with the upgrader via a new DWIdentityKeyUpgrader.missingDashPayPurposes helper instead of copied. - The Storage Explorer's identity detail and Public Keys list gain pull-to-refresh: DWIdentityReloader drives loadIdentity(atIndex:), the Rust pipeline that re-fetches the identity from Platform and folds it - public keys included - back through the persistence channel, so the @Query-backed views update live. Co-Authored-By: Claude Fable 5 <[email protected]>
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughDashPay key verification now uses Platform-authoritative data with per-identity caching. Identity reloads are centralized in ChangesDashPay key synchronization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ContactsViewModel
participant SwiftDashSDKContactsService
participant Platform
ContactsViewModel->>SwiftDashSDKContactsService: Request missingDashPayKeyCountOnPlatform
SwiftDashSDKContactsService->>Platform: Query identity keys
Platform-->>SwiftDashSDKContactsService: Return key data or nil
SwiftDashSDKContactsService-->>ContactsViewModel: Return missing-key count
ContactsViewModel->>ContactsViewModel: Update DashPay state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@DashWallet/Sources/Infrastructure/SwiftDashSDK/Contacts/SwiftDashSDKContactsService.swift`:
- Around line 807-819: Bind platform key-count results to the identity queried:
update SwiftDashSDKContactsService.missingDashPayKeyCountOnPlatform to accept or
return the identity ID, and in ContactsScreen capture the active identity before
starting the task, discard results whose identity is no longer active, and
trigger reconciliation for the newly active identity. Apply these changes at
DashWallet/Sources/Infrastructure/SwiftDashSDK/Contacts/SwiftDashSDKContactsService.swift:807-819
and DashWallet/Sources/UI/DashPay/Contacts/SwiftUI/ContactsScreen.swift:130-138.
In
`@DashWallet/Sources/UI/Menu/Tools/StorageExplorer/StorageModelListViews.swift`:
- Around line 86-90: Move the identity reload operations out of the SwiftUI
views into a shared `@MainActor` ObservableObject ViewModel. In
DashWallet/Sources/UI/Menu/Tools/StorageExplorer/StorageModelListViews.swift:86-90,
replace the direct DWIdentityReloader call with the ViewModel’s current-user
reload method; in
DashWallet/Sources/UI/Menu/Tools/StorageExplorer/StorageRecordDetailViews.swift:94-99,
route the selected-identity reload through the same ViewModel boundary. Preserve
both refresh behaviors while keeping SDK calls out of the View structs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f53d7f8e-1839-4959-970f-4f4ebaa9fb8c
📒 Files selected for processing (5)
DashWallet/Sources/Infrastructure/SwiftDashSDK/Contacts/SwiftDashSDKContactsService.swiftDashWallet/Sources/Infrastructure/SwiftDashSDK/Identity/DWIdentityKeyUpgrader.swiftDashWallet/Sources/UI/DashPay/Contacts/SwiftUI/ContactsScreen.swiftDashWallet/Sources/UI/Menu/Tools/StorageExplorer/StorageModelListViews.swiftDashWallet/Sources/UI/Menu/Tools/StorageExplorer/StorageRecordDetailViews.swift
…ewModel Review feedback (#981): - missingDashPayKeyCountOnPlatform takes the identity id explicitly and the Contacts view model captures it before the await - a wallet switch mid-flight discards the stale answer and re-checks the new identity instead of applying one identity's count to another. - The Storage Explorer pull-to-refresh actions route through a shared @mainactor StorageIdentityReloadModel instead of calling the SDK reload path from inside the View structs. Co-Authored-By: Claude Fable 5 <[email protected]>
QuantumExplorer
commented
Aug 11, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@DashWallet/Sources/UI/Menu/Tools/StorageExplorer/StorageModelListViews.swift`:
- Around line 107-111: Update the .refreshable handler in the DASHPAY section to
reload every identity represented in records, rather than only calling
reloadModel.reloadCurrentUserIdentity(). Iterate over the records’ identities
and invoke the appropriate reload operation for each while preserving the
existing pull-to-refresh behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b521b4a-c7be-4613-9249-c42cba42bee7
📒 Files selected for processing (4)
DashWallet/Sources/Infrastructure/SwiftDashSDK/Contacts/SwiftDashSDKContactsService.swiftDashWallet/Sources/UI/DashPay/Contacts/SwiftUI/ContactsScreen.swiftDashWallet/Sources/UI/Menu/Tools/StorageExplorer/StorageModelListViews.swiftDashWallet/Sources/UI/Menu/Tools/StorageExplorer/StorageRecordDetailViews.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- DashWallet/Sources/UI/DashPay/Contacts/SwiftUI/ContactsScreen.swift
- DashWallet/Sources/Infrastructure/SwiftDashSDK/Contacts/SwiftDashSDKContactsService.swift
- DashWallet/Sources/UI/Menu/Tools/StorageExplorer/StorageRecordDetailViews.swift
…identity The list spans identities; refreshing only the current user's left other local identities' keys stale. Foreign identities stay skipped - loadIdentity(atIndex:) probes the wallet's own DIP-9 tree and cannot refresh an identity this wallet doesn't own. Co-Authored-By: Claude Fable 5 <[email protected]>
Pull-to-refresh alone is an invisible affordance - "no way to update this" was the first field report. The identity detail and Public Keys list gain a toolbar refresh button driving the same reload, with a spinner while in flight. (SwiftUI.ProgressView qualified - the app has its own ProgressView type shadowing it.) Co-Authored-By: Claude Fable 5 <[email protected]>
…page The user-facing key list (Identities -> detail -> Public keys) rendered a push-time snapshot with no way to re-fetch - keys added on another device never appeared. The page gains a refresh button and pull-to-refresh driving IdentitiesViewModel.refreshIdentityKeys, which reloads the identity through the Rust load pipeline and rebuilds the rows; the page and the detail's key count render the live projection. Only the active wallet's own identities are refreshable (loadIdentity probes its DIP-9 tree); the affordance hides otherwise. Co-Authored-By: Claude Fable 5 <[email protected]>
Field testing found the wallet's own identity persisted with isLocal == false, which hid the refresh button and silently no-opped the pull gesture. What the reload actually requires is that the identity belongs to the ACTIVE wallet (loadIdentity probes its DIP-9 tree) - gate on the wallet relationship instead, and log skips rather than dropping them silently. The stale-isLocal persistence itself is an SDK-side follow-up. Co-Authored-By: Claude Fable 5 <[email protected]>
Member
Author
|
Heads-up for reviewers: the key-refresh feature in this PR depends on dashpay/platform#4374 (fold fetched on-chain state into already-known identities on load) — without it, 🤖 Posted by Claude Code |
QuantumExplorer
added a commit
that referenced
this pull request
Aug 11, 2026
Field testing found the wallet's own identity persisted with isLocal == false, which hid the refresh button and silently no-opped the pull gesture (#981 moved the key-refresh gates to the wallet relationship as a workaround). The SDK now defines isLocal as "this device can act as the identity" — via the wallet linkage OR imported key material (masternode voting/owner/payout keys, pasted user keys) — promoted by its writers and healed at startup (dashpay/platform#4375). Update the remaining app-side readers, written against the dead "Local Only / On Network" badge reading: - Identities list badge: the orange "Local Only" badge (which would have appeared on the user's OWN identities post-fix) becomes an "Observed" badge on the rare non-local rows. - Identity detail sheet: the "Status: Local Only / On Network" row becomes "Access: Local / Observed" — "Local" rather than "In Wallet" because imported-key identities are local without a wallet; the separate Wallet row names the wallet when there is one. - refreshFromNetwork: drop the '!row.isLocal' filter — it would have skipped exactly the wallet's own identities; every persisted row is Platform-confirmed, so refresh them all. Note: until the platform-side fix is pulled into ../platform, rows still carry isLocal == false and every identity shows the "Observed" badge — land/pull dashpay/platform#4375 first. Co-Authored-By: Claude Fable 5 <[email protected]>
QuantumExplorer
added a commit
that referenced
this pull request
Aug 12, 2026
The SDK now defines isLocal as 'this identity is yours or deliberately tracked here': wallet-derived identities are always local, manual adds are local, and only incidental (observed) rows are false — with the persister promoting on wallet linkage and a startup heal for the constant-false era (dashpay/platform#4375). Update the three app-side readers written against the old dead 'Local Only / On Network' badge reading: - Identities list badge: the orange 'Local Only' badge (which would now appear on every wallet identity) becomes an 'Observed' badge on the rare incidental rows. - Identity detail sheet: the always-on Status row is replaced by an 'Observed' row shown only for incidental rows — the Wallet row already names the owner otherwise. - refreshFromNetwork: drop the '!row.isLocal' filter, which would have skipped exactly the wallet's own identities. The key-refresh gates stay on the wallet relationship (#981) — that operation needs the wallet's DIP-9 tree specifically, which isLocal deliberately does not claim. Land/pull dashpay/platform#4375 first: until then rows still carry the constant false and every identity shows the 'Observed' badge. Co-Authored-By: Claude Fable 5 <[email protected]>
QuantumExplorer
added a commit
that referenced
this pull request
Aug 12, 2026
#983) The SDK now defines isLocal as 'this identity is yours or deliberately tracked here': wallet-derived identities are always local, manual adds are local, and only incidental (observed) rows are false — with the persister promoting on wallet linkage and a startup heal for the constant-false era (dashpay/platform#4375). Update the three app-side readers written against the old dead 'Local Only / On Network' badge reading: - Identities list badge: the orange 'Local Only' badge (which would now appear on every wallet identity) becomes an 'Observed' badge on the rare incidental rows. - Identity detail sheet: the always-on Status row is replaced by an 'Observed' row shown only for incidental rows — the Wallet row already names the owner otherwise. - refreshFromNetwork: drop the '!row.isLocal' filter, which would have skipped exactly the wallet's own identities. The key-refresh gates stay on the wallet relationship (#981) — that operation needs the wallet's DIP-9 tree specifically, which isLocal deliberately does not claim. Land/pull dashpay/platform#4375 first: until then rows still carry the constant false and every identity shows the 'Observed' badge. Co-authored-by: Claude Fable 5 <[email protected]>
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
1 participant
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.
Problem
The Contacts tab's "Enable DashPay" intro decides from the local SwiftData key rows alone. An identity whose DIP-15 ENCRYPTION/DECRYPTION pair was added on another device keeps showing the intro forever — offering a fee-bearing IdentityUpdate that would only no-op.
Fix
Authoritative re-check on load. When the local rows claim the pair is missing, the view model fires an async check against Platform's
identityGetKeysand corrects the intro — clearing it for identities already enabled elsewhere. Single-flight guarded; a per-session cache skips re-querying once the pair is confirmed (Platform never silently removes keys — they can only be explicitly disabled). The key evaluation is shared with the upgrade broadcast via a newDWIdentityKeyUpgrader.missingDashPayPurposeshelper rather than duplicated.Pull-to-refresh where keys are viewed. The Storage Explorer's identity detail and Public Keys list gain
.refreshable:DWIdentityReloaderdrivesloadIdentity(atIndex:)— the Rust pipeline that re-fetches the identity from Platform and folds it, public keys included, back through the persistence channel — so the@Query-backed views update live under the pull.Verification
Clean
dashpayscheme build (arm64 sim) against platformv4.2-dev@08edcfd1. Cross-device repro requires a second device adding keys; the code path reuses the exact Platform key-set read the upgrader already ships (identityGetKeys+ the same enabled-ECDSA evaluation).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes