feat(dashpay): identity banner, network contact search, and shielded-first identity top-up - #942
Merged
Merged
Conversation
Once DashPay is enabled, the tab opens with who you are: a Dash-blue gradient header (design option C) with the ringed avatar, display name, and @username - the row opens the read-only profile sheet - plus the add-contact button on the title line. The system nav bar now fronts only the pre-enable intro; the search field overlaps the banner seam. Identity fields come from DWCurrentUserIdentityInfo via the view model. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Typing in the Contacts search now also queries DPNS: under the local sections, an "On the Dash network" section shows up to three usernames not already in your lists (own identity excluded), debounced like the add-contact search. Tapping a result or "Show more results" opens AddContactScreen prefilled with the query - the send/accept/eligibility machinery stays there, unduplicated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The profile sheet's "Platform Credits" row actually displayed the wallet's DIP-17 Platform address balance, not the identity's. It is now "Identity Account Balance", reading the identity's own credit balance from the persisted row, with an info alert explaining how it differs from the wallet balances (pays Platform fees, not spendable as Dash) and a Top Up flow: preset amounts with fiat equivalents, PIN-gated, funded from the Transparent balance via the SDK's topUpIdentityWithFunding, displaying the returned post-transition balance. Nothing broadcasts before the PIN gate passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Top-ups now default to the Shielded balance for privacy: a two-step route that unshields the amount to the wallet's own next Platform receive address (proven execution), then claims the landed credits minus the policy fee headroom into an IdentityTopUp - the identity receives most of the amount and the headroom stays in the Platform balance. Platform and Transparent remain as explicit alternatives behind a linkability warning (funding from a transparent balance publicly links those coins to the identity). One PIN gate covers the chosen route; the confirm button narrates the two shielded steps. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Presets drop 0.01 (fixed fees would be a large share of it, especially on the shielded route) in favor of 0.05 / 0.1 / a Custom decimal field with a 0.01 DASH floor and fiat preview. An "Estimated fees" line shows the per-source estimate: the consensus-pinned unshield minimum (pure estimateShieldedFee FFI) plus the observed ~0.0004 DASH top-up transition fee, in DASH and local currency. The sheet opens full-height so the confirm button no longer clips at the medium detent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The shielded identity top-up unshields the principal to an own address and immediately claims most of it, so the next sync observes only the small remainder - the exact-amount own-unshield match missed it and the residue rendered as a generic "Received" row. The suppression rule is now coverage-based: an observed delta on the unshield's own destination address that is at most the credited principal is the own operation's residue. Blind spot (an external payment on the same fresh address in the same window, at most the principal) documented; the previous exact rule had the equivalent one. Compile-ready tests updated to the new semantics with a top-up residue case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughChangesDashPay identity and contacts
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant IdentityTopUpSheet
participant IdentityTopUpViewModel
participant Wallet
participant Identity
IdentityTopUpSheet->>IdentityTopUpViewModel: Submit validated amount and funding source
IdentityTopUpViewModel->>Wallet: Authorize route and process funding
Wallet->>Identity: Transfer funds and claim credits
IdentityTopUpViewModel->>IdentityTopUpSheet: Return refreshed balance
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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: 5
🧹 Nitpick comments (5)
DashWalletTests/SwiftDashSDKCoreLifecycleTests.swift (1)
249-253: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the non-positive delta boundary.
unshieldCoversDeltarequiresobservedDeltaDuffs > 0, but this test does not check zero or negative deltas. Add both assertions to protect the lower-bound contract.Proposed test additions
XCTAssertTrue(PlatformAddressActivityUnitPolicy.unshieldCoversDelta( creditedAmountCredits: creditedAmount, observedDeltaDuffs: 10_000_000)) + XCTAssertFalse(PlatformAddressActivityUnitPolicy.unshieldCoversDelta( + creditedAmountCredits: creditedAmount, + observedDeltaDuffs: 0)) + XCTAssertFalse(PlatformAddressActivityUnitPolicy.unshieldCoversDelta( + creditedAmountCredits: creditedAmount, + observedDeltaDuffs: -1))🤖 Prompt for 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. In `@DashWalletTests/SwiftDashSDKCoreLifecycleTests.swift` around lines 249 - 253, Extend the test coverage around PlatformAddressActivityUnitPolicy.unshieldCoversDelta by adding assertions that zero and negative observedDeltaDuffs return false, while preserving the existing positive-delta and unit-mismatch assertions.DashWallet/Sources/UI/DashPay/Profile/SDKIdentityProfileSheet.swift (4)
316-325: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the named credits-per-duff constant instead of the literal
1000.
PlatformPaymentIdentityFundingPolicy.creditsPerDuffis already used in this file at lines 469 and 524, andPlatformAddressActivityStore.duffs(fromCredits:)performs this exact conversion. The literal1000is repeated here and at lines 493-494 and 820. If the rate ever changes, these sites drift silently.♻️ Proposed refactor
- let duffs = credits / 1000 + let duffs = credits / PlatformPaymentIdentityFundingPolicy.creditsPerDuff🤖 Prompt for 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. In `@DashWallet/Sources/UI/DashPay/Profile/SDKIdentityProfileSheet.swift` around lines 316 - 325, Update the identityBalanceFormatted conversion to divide credits by PlatformPaymentIdentityFundingPolicy.creditsPerDuff instead of the literal 1000, preserving the existing zero-balance and formatted DASH output behavior.
687-697: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe validation message does not match every rejected input.
customDuffsreturns nil for three cases: an unparseable value, an amount above the 1000 DASH cap (line 806), and an amount below the floor. All three show "Enter at least %@ DASH". A user who types 5000 reads an incorrect instruction. Consider distinguishing the above-cap and unparseable cases.🤖 Prompt for 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. In `@DashWallet/Sources/UI/DashPay/Profile/SDKIdentityProfileSheet.swift` around lines 687 - 697, Update the validation message in the custom amount display near customDuffs to distinguish rejected inputs: retain the minimum-DASH message only for values below IdentityTopUpViewModel.customMinimumDuffs, show an appropriate above-cap message for values exceeding the 1000 DASH limit, and provide suitable feedback for unparseable input. Use the existing validation/parsing logic around IdentityTopUpViewModel rather than treating every nil customDuffs result identically.
431-439: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueThe shielded fee estimate falls back to 0 without any signal.
try?at line 434 maps an estimation failure tonil, and?? 0then hides it. The sheet shows only the top-up transition fee. The user underestimates the shielded route cost. Consider showing an unavailable-estimate state instead of a silently low number.🤖 Prompt for 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. In `@DashWallet/Sources/UI/DashPay/Profile/SDKIdentityProfileSheet.swift` around lines 431 - 439, Update estimatedFeeCredits so a failed PlatformWalletManager.estimateShieldedFee call is represented as an unavailable estimate rather than silently converted to zero; propagate an optional or equivalent failure state through the caller and prevent displaying only observedTopUpTransitionFeeCredits for shielded sources, while preserving the existing successful estimate and non-shielded behavior.
489-497: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueApply the SwiftLint
pattern_matching_keywordsfix.Move the
letout of the associated values.♻️ Proposed fix
- if case .insufficient(let required, let available) = error { + if case let .insufficient(required, available) = error {🤖 Prompt for 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. In `@DashWallet/Sources/UI/DashPay/Profile/SDKIdentityProfileSheet.swift` around lines 489 - 497, Update the pattern match in the PlatformPaymentIdentityFundingPolicy.PlanningError catch block so the let binding is placed before the associated-value pattern, preserving the existing insufficient-funds handling and error message behavior.Source: Linters/SAST tools
🤖 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/en.lproj/Localizable.strings`:
- Around line 2359-2360: Update the localized identity profile balance
description to say that topping up converts Dash from the selected balance,
rather than specifically from the Transparent balance, while preserving the rest
of the message.
In `@DashWallet/Sources/UI/DashPay/Contacts/SwiftUI/ContactsScreen.swift`:
- Around line 68-88: Update updateNetworkSearch to clear networkSearchResults
immediately after confirming the trimmed query is eligible and before starting
the debounce task, so stale results are not displayed while the new search is
pending.
- Around line 51-56: Remove the force unwrap in ownDisplayTitle by using
optional binding to return the non-empty ownDisplayName value; otherwise
preserve the existing ownUsername and localized “My identity” fallback chain.
In `@DashWallet/Sources/UI/DashPay/Profile/SDKIdentityProfileSheet.swift`:
- Around line 176-181: Update the balance explanation and related documentation
in ProfileSDKIdentityProfileSheet, including the alert message and the doc
comments near the identity balance and IdentityTopUpSheet, to describe top-up
funding as supporting Shielded, Platform, and Transparent balances rather than
Transparent-only. Preserve Shielded as the default funding source.
- Around line 516-523: Update the top-up error flow around the destination guard
in the profile sheet by introducing a dedicated typed error, such as
TopUpError.noPlatformReceiveAddress, whose localized description contains the
actionable Platform sync message. Throw that error from the guard instead of
assigning errorMessage there, so the existing generic catch can display the
preserved localized description without overwriting it with
ServiceError.noWallet.
---
Nitpick comments:
In `@DashWallet/Sources/UI/DashPay/Profile/SDKIdentityProfileSheet.swift`:
- Around line 316-325: Update the identityBalanceFormatted conversion to divide
credits by PlatformPaymentIdentityFundingPolicy.creditsPerDuff instead of the
literal 1000, preserving the existing zero-balance and formatted DASH output
behavior.
- Around line 687-697: Update the validation message in the custom amount
display near customDuffs to distinguish rejected inputs: retain the minimum-DASH
message only for values below IdentityTopUpViewModel.customMinimumDuffs, show an
appropriate above-cap message for values exceeding the 1000 DASH limit, and
provide suitable feedback for unparseable input. Use the existing
validation/parsing logic around IdentityTopUpViewModel rather than treating
every nil customDuffs result identically.
- Around line 431-439: Update estimatedFeeCredits so a failed
PlatformWalletManager.estimateShieldedFee call is represented as an unavailable
estimate rather than silently converted to zero; propagate an optional or
equivalent failure state through the caller and prevent displaying only
observedTopUpTransitionFeeCredits for shielded sources, while preserving the
existing successful estimate and non-shielded behavior.
- Around line 489-497: Update the pattern match in the
PlatformPaymentIdentityFundingPolicy.PlanningError catch block so the let
binding is placed before the associated-value pattern, preserving the existing
insufficient-funds handling and error message behavior.
In `@DashWalletTests/SwiftDashSDKCoreLifecycleTests.swift`:
- Around line 249-253: Extend the test coverage around
PlatformAddressActivityUnitPolicy.unshieldCoversDelta by adding assertions that
zero and negative observedDeltaDuffs return false, while preserving the existing
positive-delta and unit-mismatch assertions.
🪄 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: d56722aa-f356-4437-93fe-51c369ec94d5
📒 Files selected for processing (6)
DashWallet/Sources/Models/PlatformAddress/PlatformAddressActivityStore.swiftDashWallet/Sources/UI/DashPay/Contacts/SwiftUI/AddContactScreen.swiftDashWallet/Sources/UI/DashPay/Contacts/SwiftUI/ContactsScreen.swiftDashWallet/Sources/UI/DashPay/Profile/SDKIdentityProfileSheet.swiftDashWallet/en.lproj/Localizable.stringsDashWalletTests/SwiftDashSDKCoreLifecycleTests.swift
- Balance explainer and doc comments no longer claim Transparent-only funding: top-ups convert from a chosen balance, Shielded by default. - Typed TopUpError.noPlatformReceiveAddress so the generic catch surfaces the actionable message instead of overwriting it. - Network search clears stale results when a new query starts. - ownDisplayTitle drops the force unwrap for optional binding. - unshieldCoversDelta tests cover the zero/negative delta boundary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 9, 2026
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.
Issue being fixed or feature implemented
Post-enable, the Contacts tab treated the user's DashPay identity as an afterthought: no visible "who am I", search only filtered the local lists, the profile sheet's "Platform Credits" row silently displayed the wallet's DIP-17 Platform address balance rather than the identity's, and there was no way to top up the identity's credit balance from the app at all.
What was done?
Dash-blue identity banner (Contacts tab) — once DashPay is enabled, the tab opens with who you are: a gradient header with the ringed avatar, display name, and
@username · View profile(opens the read-only profile sheet), plus the add-contact button on the title line. The system nav bar now fronts only the pre-enable intro; the search field overlaps the banner seam. Identity fields come fromDWCurrentUserIdentityInfovia the view model. (Editing stays on the Home avatar flow, which owns the editor's delegate wiring —TODO(dashpay-banner-edit).)Network results in Contacts search — typing ≥2 characters still filters the local sections, and now also runs a debounced DPNS prefix search: an "On the Dash network" section shows up to three usernames not already in your lists (own identity excluded). Tapping a result or "Show more results" opens
AddContactScreenprefilled with the query (newinitialQuery:— search auto-fires on appear), so the send/accept/eligibility machinery stays in one place, unduplicated.Identity Account Balance (profile sheet) — the "Platform Credits" row was miswired: it rendered
platformPaymentCreditsAsDuffs(the wallet's Platform address balance) on an identity sheet. It is now "Identity Account Balance", reading the identity's own credit balance from the persisted row, with an ⓘ alert explaining how it differs from the wallet balances (pays Platform fees; not spendable as regular Dash).Shielded-first identity top-up — a Top Up flow on that row, defaulting to the Shielded balance for privacy:
shieldedUnshield, proven execution, fee paid on the shielded side), then claim the landed credits minusPlatformPaymentIdentityFundingPolicy.feeHeadroomCreditsinto onetopUpFromAddressesIdentityTopUp. The transition's fee comes out of the supplied credits, so the identity receives most of the amount and the unclaimed headroom stays in the Platform balance (stated in the sheet). The confirm button narrates both steps.PlatformPaymentIdentityFundingPolicyinput selection; Transparent uses the Core asset-lock route (topUpIdentityWithFunding).estimateShieldedFeeFFI) plus the observed ~0.0004 DASH top-up transition fee.Observed-payments residue fix — the shielded top-up's step 2 spends most of the landed credits before the next sync, so the address's observed net delta (the small remainder) no longer matched the ledger's exact-amount own-unshield suppression and rendered as a bogus "Received" row. The suppression is now coverage-based: a positive delta on the unshield's own destination address that is at most the credited principal is the own operation's residue. Deltas larger than the principal still record. The symmetric blind spot is documented in the policy; compile-ready tests updated with a top-up-residue case.
How Has This Been Tested?
Manual, on mainnet with a real identity:
Clean
dashpaybuilds throughout, including against the rebuilt SwiftDashSDK xcframework carrying platform PR #4342. (Unit-test target pre-existing broken; the policy tests are compile-ready.)Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes