feat(dashpay): DPNS username marketplace in the Explore tab - #946
Conversation
Search-driven marketplace over the DPNS v2 trade surface (transferable=1, tradeMode=1): search any name with live sale state (documentList on the parentNameAndLabel index returns the full domain documents including $price), browse the names on your identity (records.identity index), list/re-price (setDocumentPrice), delist (transfer-to-self - consensus clears $price on every transfer, verified in rs-drive), buy listed names (purchaseDocument with the confirmed price pinned; consensus rejects seller-side price changes, surfaced as a typed priceChanged error after an authoritative re-read), gift transfers with username-or-identity-id recipient resolution, and direct registration of unclaimed non-contested labels. Purchases pre-check the buyer identity's credit balance against price plus a documented fee reserve. Every mutation is PIN-gated before signing, using the critical auth key (id 1). No $price index exists on the contract, so a global for-sale browse is deliberately absent (tracked in the platform marketplace task); per-name trade history awaits the SDK's revision-history query - the detail sheet shows only the facts the current document carries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a DPNS username marketplace to the wallet. The change includes SDK service operations, contested-name handling, wallet synchronization, Explore Dash navigation, SwiftUI search and ownership views, trade and registration flows, localization, and Xcode project wiring. ChangesUsername Marketplace
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant UsernameMarketplaceScreen
participant UsernameMarketplaceService
participant WalletSDK
User->>UsernameMarketplaceScreen: select marketplace action
UsernameMarketplaceScreen->>UsernameMarketplaceService: submit marketplace operation
UsernameMarketplaceService->>WalletSDK: submit marketplace request
WalletSDK-->>UsernameMarketplaceService: return operation result
UsernameMarketplaceService-->>UsernameMarketplaceScreen: return success or error
UsernameMarketplaceScreen-->>User: refresh lists and show feedback
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Platform #4348 shipped the wallet layer this screen was designed
against; the client now consumes it end to end:
- Reads use the typed surface: searchDpnsMarketplace (live sale state),
myDpnsMarketplaceNames (local rows, no network - including retained
sold/transferred departures, now rendered in a "No longer yours"
section with the counterparty), dpnsMarketplaceNameState (live detail),
and dpnsNameHistory - the detail sheet now shows the real trade
timeline (registered / listed / bought with counterparties and price /
transferred, with delist rendered as "Removed from sale").
- Trade ops use the orchestrated calls with typed errors surfaced in
user terms (notForSale, priceChanged, insufficientIdentityCredits with
the top-up hint, contestedNameNotTradable). Delist uses the dedicated
delistDpnsName; the app-side documentList parser, manual pre-flight
re-read, and balance pre-check are gone (SDK owns them).
- Seller clarity: a listed name that isn't yours says "For sale by an
independent user" on the search row, carries a callout on the detail
sheet ("offered by an independent user on the Dash network - not by
Dash or this app; the seller sets the price"), and the purchase
confirmation repeats it.
- The recurring DPNS marketplace sync starts with the wallet (same
best-effort contract as the shielded/DashPay loops); My Names
pull-to-refresh runs a syncDpnsMarketplace pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (6)
DashWallet/Sources/Infrastructure/SwiftDashSDK/UsernameMarketplaceService.swift (1)
23-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve the SwiftLint warnings.
Sort
SwiftDashSDKbeforeSwiftData. Usecase let .insufficientIdentityCredits(_, required, available). Replace the three-value tuple fromrequireOwnContext()with a private context type.Proposed cleanup
-import SwiftData import SwiftDashSDK +import SwiftData ... -case .insufficientIdentityCredits(_, let required, let available): +case let .insufficientIdentityCredits(_, required, available):As per coding guidelines, “Follow the applicable language conventions: … SwiftFormat/SwiftLint.”
Also applies to: 230-230, 244-250
🤖 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/Infrastructure/SwiftDashSDK/UsernameMarketplaceService.swift` around lines 23 - 26, Resolve the SwiftLint issues in UsernameMarketplaceService by ordering the imports with SwiftDashSDK before SwiftData, changing insufficientIdentityCredits pattern matching to case let .insufficientIdentityCredits(_, required, available), and replacing the three-value requireOwnContext() tuple with a private context type while updating its consumers accordingly.Sources: Coding guidelines, Linters/SAST tools
DashWallet/Sources/UI/Explore Dash/UsernameMarketplaceScreen.swift (5)
912-919: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRound the duffs conversion explicitly.
NSDecimalNumber.int64Valuetruncates any sub-duff remainder without notice, andUInt64(exactly:)cannot fail for a non-negativeInt64, so the guard adds nothing. Round the decimal with an explicitNSDecimalNumberHandlerbefore conversion, so0.000000019DASH does not become a different listed price than the user typed.🤖 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/Explore` Dash/UsernameMarketplaceScreen.swift around lines 912 - 919, Update the priceDuffs conversion to round duffs explicitly with an NSDecimalNumberHandler before converting the result to UInt64, rather than relying on NSDecimalNumber.int64Value truncation. Remove the ineffective UInt64(exactly:) wrapping and preserve the existing input normalization and price bounds.
137-144: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAwait the local reload in
refreshFromNetwork.
loadMyNames()starts a detached task and returns at once.refreshFromNetworktherefore finishes before the rows are re-read, so the pull-to-refresh indicator disappears while the list is still loading. Consider an awaited variant of the local read.🤖 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/Explore` Dash/UsernameMarketplaceScreen.swift around lines 137 - 144, Update refreshFromNetwork to await completion of the local username reload instead of calling loadMyNames(), which returns before its detached task finishes. Add or reuse an async variant of loadMyNames that performs the read synchronously within the awaited flow, while preserving the existing error handling and list refresh behavior.
519-519: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueResolve the SwiftLint warnings.
SwiftLint reports
attributesviolations at lines 519, 908, 1019, and 1144: place@Environment(\.dismiss)on its own line. It also reportspattern_matching_keywordsat lines 719, 732, and 737, andsorted_importsat lines 25-26. The coding guidelines require SwiftLint conformance for Swift files.As per coding guidelines: "Follow the applicable language conventions: … SwiftFormat/SwiftLint for Swift".🛠️ Example for line 519
- `@Environment`(\.dismiss) private var dismiss + `@Environment`(\.dismiss) + private var dismiss🤖 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/Explore` Dash/UsernameMarketplaceScreen.swift at line 519, Update UsernameMarketplaceScreen.swift to satisfy SwiftLint: place each `@Environment`(\.dismiss) declaration on its own line at the affected locations, revise the pattern-matching keyword usage at the reported cases, and reorder the imports alphabetically according to sorted_imports.Sources: Coding guidelines, Linters/SAST tools
441-444: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the duplicated
shortIdhelper.
shortId(_:)is defined identically here and inMarketplaceNameDetailSheetat lines 753-756. Move it to one shared helper, for example aDataextension in this file.🤖 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/Explore` Dash/UsernameMarketplaceScreen.swift around lines 441 - 444, Extract the duplicated shortId(_:) implementation from the current screen and MarketplaceNameDetailSheet into one shared Data helper, such as a file-level Data extension. Update both callers to use the shared helper and remove the duplicate private definitions while preserving the existing Base58 truncation format.
834-839: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMove the balance read out of
body.
identityBalanceCredits(identityId:container:)runs synchronously during view evaluation. SwiftUI re-evaluatesbodyon every state change, so a model-container read repeats on the main thread each time. Load the value once in.taskand store it in@State.#!/bin/bash # Check whether identityBalanceCredits performs a persistent-store fetch. fd -t f 'UsernameMarketplaceService.swift' --exec rg -n -C10 'identityBalanceCredits'🤖 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/Explore` Dash/UsernameMarketplaceScreen.swift around lines 834 - 839, Move the identityBalanceCredits call out of identityBalanceLine and the view body: add an optional `@State` value for the loaded balance, populate it once in the view’s .task using the existing identity and model-container inputs, and have identityBalanceLine render from that state instead of reading the container synchronously. Preserve the existing fallback behavior while the value is unavailable.
🤖 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 2575-2577: Update the localized string value for the two arguments
to use positional format specifiers, assigning distinct indices to the needed
and available balance values while preserving the existing wording and argument
order.
- Around line 10-11: Propagate the marketplace localization key "%1$@ listed for
%2$@ DASH" from the English catalog to every non-English localization catalog
and the Transifex source using BartyCrouch or equivalent Xcode-aware tooling.
Preserve each catalog’s existing format and UTF-16LE encoding.
In
`@DashWallet/Sources/Infrastructure/SwiftDashSDK/UsernameMarketplaceService.swift`:
- Line 127: Replace direct authorizer.authorize() calls in all five transaction
methods with a private authorize() wrapper that maps
DWIdentityAuthorizer.AuthError.cancelled and authorization failures to the
service’s corresponding errors, matching SwiftDashSDKContactsService.authorize()
behavior. Ensure each transaction method uses the wrapper so cancellation
remains distinct from failed marketplace actions.
- Around line 125-132: Update setPrice to validate priceDuffs against the
UInt64.max / 1,000 limit before multiplying it into priceCredits. If the value
exceeds that limit, throw the existing user-facing invalid-price error;
otherwise preserve the current authorization and wallet.setDpnsNamePrice flow.
In `@DashWallet/Sources/UI/Explore` Dash/UsernameMarketplaceScreen.swift:
- Line 1112: Update transfer() at the SwiftDashSDKHost.shared.wallet guard to
assign the missing-wallet error to resolveError before returning, so the user
receives feedback when no wallet is available.
- Around line 796-804: Update the Buy button action to require a non-nil
name.priceCredits before calling viewModel.service.purchase; if the price is
unavailable, abort the action without dismissing or submitting a purchase. Pass
the unwrapped price as expectedPriceCredits and preserve the existing success
flow for valid prices.
- Around line 346-355: Update the searchRow sale-price VStack to require both
name.isForSale and a non-nil name.priceDuffs, matching the stateRow condition;
keep displaying the existing formatted price when the record is currently for
sale.
- Around line 154-171: Update the action flow around the Task using
isPerformingAction and perform so the action-state cleanup occurs immediately
after operation() completes, before the success-banner delay. Move
auto-dismissal of successMessage into a separate task, preserving the delayed
conditional dismissal without keeping action buttons disabled or blocking new
perform calls.
- Around line 1146-1148: Update the isContested computed property to normalize
label with the SDK’s dpnsNormalizeLabel, matching updateSearch, before passing
it to UsernameMarketplaceService.isContestedEligible; remove the direct
lowercased() normalization so look-alike characters are handled consistently.
- Around line 79-83: Update isValidLabel to validate against ASCII letters (A-Z,
a-z), digits (0-9), and hyphen explicitly instead of using Unicode-aware
isLetter and isNumber. Preserve the existing length and leading/trailing hyphen
checks.
---
Nitpick comments:
In
`@DashWallet/Sources/Infrastructure/SwiftDashSDK/UsernameMarketplaceService.swift`:
- Around line 23-26: Resolve the SwiftLint issues in UsernameMarketplaceService
by ordering the imports with SwiftDashSDK before SwiftData, changing
insufficientIdentityCredits pattern matching to case let
.insufficientIdentityCredits(_, required, available), and replacing the
three-value requireOwnContext() tuple with a private context type while updating
its consumers accordingly.
In `@DashWallet/Sources/UI/Explore` Dash/UsernameMarketplaceScreen.swift:
- Around line 912-919: Update the priceDuffs conversion to round duffs
explicitly with an NSDecimalNumberHandler before converting the result to
UInt64, rather than relying on NSDecimalNumber.int64Value truncation. Remove the
ineffective UInt64(exactly:) wrapping and preserve the existing input
normalization and price bounds.
- Around line 137-144: Update refreshFromNetwork to await completion of the
local username reload instead of calling loadMyNames(), which returns before its
detached task finishes. Add or reuse an async variant of loadMyNames that
performs the read synchronously within the awaited flow, while preserving the
existing error handling and list refresh behavior.
- Line 519: Update UsernameMarketplaceScreen.swift to satisfy SwiftLint: place
each `@Environment`(\.dismiss) declaration on its own line at the affected
locations, revise the pattern-matching keyword usage at the reported cases, and
reorder the imports alphabetically according to sorted_imports.
- Around line 441-444: Extract the duplicated shortId(_:) implementation from
the current screen and MarketplaceNameDetailSheet into one shared Data helper,
such as a file-level Data extension. Update both callers to use the shared
helper and remove the duplicate private definitions while preserving the
existing Base58 truncation format.
- Around line 834-839: Move the identityBalanceCredits call out of
identityBalanceLine and the view body: add an optional `@State` value for the
loaded balance, populate it once in the view’s .task using the existing identity
and model-container inputs, and have identityBalanceLine render from that state
instead of reading the container synchronously. Preserve the existing fallback
behavior while the value is unavailable.
🪄 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: 2d3d630a-b738-47ec-aeea-dd72d46f8ffc
📒 Files selected for processing (6)
DashWallet.xcodeproj/project.pbxprojDashWallet/Sources/Infrastructure/SwiftDashSDK/PlatformAddressSyncCoordinator.swiftDashWallet/Sources/Infrastructure/SwiftDashSDK/UsernameMarketplaceService.swiftDashWallet/Sources/UI/Explore Dash/ExploreMenuScreen.swiftDashWallet/Sources/UI/Explore Dash/UsernameMarketplaceScreen.swiftDashWallet/en.lproj/Localizable.strings
| /* Username marketplace: listing success — name, then price */ | ||
| "%1$@ listed for %2$@ DASH" = "%1$@ listed for %2$@ DASH"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether a sample of the new marketplace keys exists in the other locale catalogs.
fd -t f 'Localizable.strings' -d 3 | while read -r f; do
printf '%s: ' "$f"
iconv -f UTF-16LE -t UTF-8 "$f" 2>/dev/null | rg -c '"Username Marketplace"' \
|| rg -c '"Username Marketplace"' "$f" || echo 0
doneRepository: dashpay/dashwallet-ios
Length of output: 4093
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- English diff ---'
git diff --unified=3 -- DashWallet/en.lproj/Localizable.strings
printf '%s\n' '--- Localization and Transifex files ---'
git ls-files | rg '(^|/)(Localizable\.strings|.*transifex.*|.*txconfig.*|.*\.strings)$' | head -200
printf '%s\n' '--- Encoding samples ---'
for f in DashWallet/en.lproj/Localizable.strings DashWallet/fr.lproj/Localizable.strings Localizable.strings; do
if [ -f "$f" ]; then
printf '%s: ' "$f"
file "$f"
fi
doneRepository: dashpay/dashwallet-ios
Length of output: 3659
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
def read_strings(path):
raw = Path(path).read_bytes()
for encoding in ("utf-16", "utf-16-le", "utf-8"):
try:
return raw.decode(encoding)
except UnicodeDecodeError:
pass
raise UnicodeDecodeError("unknown", raw, 0, 1, f"cannot decode {path}")
path = "DashWallet/en.lproj/Localizable.strings"
text = read_strings(path)
lines = text.splitlines()
print("--- English entries at the referenced ranges ---")
for start, end in [(1, 80), (350, 380), (440, 455), (530, 545), (580, 610)]:
print(f"[{start}-{end}]")
for number in range(start, min(end, len(lines)) + 1):
print(f"{number}: {lines[number - 1]}")
print("--- Repository localization metadata ---")
for path in sorted(Path(".").rglob("*")):
if path.is_file() and any(term in path.name.lower() for term in ("transifex", "txconfig", "localization")):
print(path)
PYRepository: dashpay/dashwallet-ios
Length of output: 8235
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
KEY_RE = re.compile(r'^"((?:\\.|[^"])*)"\s*=')
ranges = [(10, 11), (46, 47), (58, 68), (369, 370), (446, 447),
(538, 539), (587, 588), (599, 603)]
def decode(path):
raw = path.read_bytes()
for encoding in ("utf-16", "utf-16-le", "utf-8"):
try:
return raw.decode(encoding)
except UnicodeDecodeError:
continue
raise RuntimeError(f"Cannot decode {path}")
def keys(path):
return {
m.group(1)
for line in decode(path).splitlines()
if (m := KEY_RE.match(line))
}
english_path = Path("DashWallet/en.lproj/Localizable.strings")
english_lines = decode(english_path).splitlines()
wanted = []
for start, end in ranges:
for line_number in range(start, end + 1):
if line_number <= len(english_lines):
match = KEY_RE.match(english_lines[line_number - 1])
if match and match.group(1) not in wanted:
wanted.append(match.group(1))
print("--- Keys under review ---")
for key in wanted:
print(key)
print("--- Missing keys by locale catalog ---")
for path in sorted(Path("DashWallet").glob("*.lproj/Localizable.strings")):
missing = [key for key in wanted if key not in keys(path)]
print(f"{path}: {len(missing)} missing")
for key in missing:
print(f" {key}")
root = Path("Localizable.strings")
if root.exists():
missing = [key for key in wanted if key not in keys(root)]
print(f"--- Root Transifex candidate: {root} ---")
print(f"{len(missing)} missing")
for key in missing:
print(f" {key}")
print("--- Localization metadata candidates ---")
for path in sorted(Path(".").rglob("*")):
if path.is_file() and any(term in path.name.lower() for term in ("transifex", "txconfig")):
print(path)
PYRepository: dashpay/dashwallet-ios
Length of output: 20432
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Localization tooling and source references ---'
git ls-files | rg -i '(^|/)(\.?barty|bartycrouch|transifex|txconfig|localiz|.*\.strings$)' | head -300
printf '%s\n' '--- References to the root localization source ---'
rg -n -i 'Transifex|BartyCrouch|Localizable\.strings|localization' \
--glob '!DashWallet/*\.lproj/Localizable.strings' \
--glob '!WatchApp/*\.lproj/Interface.strings' \
. | head -200
printf '%s\n' '--- Root localization source header ---'
python3 - <<'PY'
from pathlib import Path
path = Path("Localizable.strings")
raw = path.read_bytes()
for encoding in ("utf-16", "utf-16-le", "utf-8"):
try:
text = raw.decode(encoding)
break
except UnicodeDecodeError:
pass
for number, line in enumerate(text.splitlines()[:25], 1):
print(f"{number}: {line}")
PYRepository: dashpay/dashwallet-ios
Length of output: 18146
Propagate the marketplace keys to all localization catalogs.
The keys are absent from every non-English catalog and the Transifex source. Use BartyCrouch or Xcode-aware tooling, then preserve UTF-16LE encoding.
🤖 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/en.lproj/Localizable.strings` around lines 10 - 11, Propagate the
marketplace localization key "%1$@ listed for %2$@ DASH" from the English
catalog to every non-English localization catalog and the Transifex source using
BartyCrouch or equivalent Xcode-aware tooling. Preserve each catalog’s existing
format and UTF-16LE encoding.
Sources: Coding guidelines, Learnings
| private var isContested: Bool { | ||
| UsernameMarketplaceService.isContestedEligible(normalizedLabel: label.lowercased()) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the SDK normalizer for the contested check.
updateSearch uses sdk.dpnsNormalizeLabel because DPNS folds look-alike characters, as the file header states. isContested uses label.lowercased() instead. A label such as a1ice normalizes differently from its lowercased form, so a contested label can bypass this check and show the Register button. Apply the same normalizer here.
🛠️ Proposed fix
private var isContested: Bool {
- UsernameMarketplaceService.isContestedEligible(normalizedLabel: label.lowercased())
+ let normalized = (try? SwiftDashSDKHost.shared.sdk?.dpnsNormalizeLabel(label)) ?? nil
+ return UsernameMarketplaceService.isContestedEligible(
+ normalizedLabel: normalized ?? label.lowercased())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private var isContested: Bool { | |
| UsernameMarketplaceService.isContestedEligible(normalizedLabel: label.lowercased()) | |
| } | |
| private var isContested: Bool { | |
| let normalized = (try? SwiftDashSDKHost.shared.sdk?.dpnsNormalizeLabel(label)) ?? nil | |
| return UsernameMarketplaceService.isContestedEligible( | |
| normalizedLabel: normalized ?? label.lowercased()) | |
| } |
🤖 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/Explore` Dash/UsernameMarketplaceScreen.swift around
lines 1146 - 1148, Update the isContested computed property to normalize label
with the SDK’s dpnsNormalizeLabel, matching updateSearch, before passing it to
UsernameMarketplaceService.isContestedEligible; remove the direct lowercased()
normalization so look-alike characters are handled consistently.
The register sheet previously dead-ended on contested-eligible labels
("must be requested through the username flow" + Cancel). It now
submits the request natively:
- Contested labels get a "Request" sheet: what the masternode vote is,
a pre-submit check of the label's network state (fresh vote / already
an active vote you'd join as a contender / locked by a past vote, in
which case there is no submit button), the protocol's 0.2 DASH
vote-resolution fund as the request cost next to the identity
balance (with the top-up hint when short), and an honest footnote
that the fund isn't returned if another contender wins.
- Submission is the same registerDpnsName transition the setup flow
uses, with the same step-3.5 bookkeeping: bookmark via
DWContestedNameStatusService (keeps the not-yet-owned label out of
every username surface and arms the Home-appear win/loss
reconciliation), contested-cache sync, and the authoritative voting
deadline recorded once Platform indexes the contest.
- My Names grows an "In network vote" section: pending requests render
with the voting deadline when known, refreshed by pull-to-refresh.
Re-requesting a label you're already contending for is caught in the
sheet instead of failing at broadcast.
- The search register-row subtitle says up front that short names are
decided by a network vote.
- finalizeWon now backfills the global username mirror only when it's
empty: a contested win on a SECOND name (requested from the
marketplace) no longer displaces the username the user already shows
everywhere.
- The client-side contested predicate now delegates to the SDK's own
dash_sdk_dpns_is_contested_username instead of a hand-rolled check.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
DashWallet/Sources/UI/Explore Dash/UsernameMarketplaceScreen.swift (1)
1285-1286: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove
@ViewBuilderonto the same line as the property declaration.SwiftLint reports the
attributesrule for both computed properties. The rule requires attributes on the same line as variables.♻️ Proposed fix
- `@ViewBuilder` - private var contestedContent: some View { + `@ViewBuilder` private var contestedContent: some View {- `@ViewBuilder` - private var requestCostCard: some View { + `@ViewBuilder` private var requestCostCard: some View {Also applies to: 1339-1340
🤖 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/Explore` Dash/UsernameMarketplaceScreen.swift around lines 1285 - 1286, Move the `@ViewBuilder` attribute onto the same line as each computed property declaration for contestedContent and the other affected property, resolving the SwiftLint attributes violations without changing their implementations.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/Sources/Infrastructure/SwiftDashSDK/UsernameMarketplaceService.swift`:
- Around line 280-285: Update the winner-handling branch in
UsernameMarketplaceService to return .locked for every resolved winner,
including non-"LOCKED" identity values. Preserve the existing .activeContest
path only for labels without a resolved winner, so
RegisterNameSheet.contestedContent does not offer requests for already-won
labels.
- Around line 214-235: Capture WalletEnvironment.network before the async
authorization and registration work, then reuse that value for both the
voting-state handling and the network-explicit
DWContestedNameStatusService.recordSubmission overload. Replace the label-only
recordSubmission call so the bookmark remains scoped to the network active when
the operation began.
In `@DashWallet/Sources/UI/Explore` Dash/UsernameMarketplaceScreen.swift:
- Around line 1326-1333: Hoist the identity-credit affordability check currently
used by requestCostCard into the surrounding view state, and use it to disable
the “Request Username” confirmButton when available credits cannot cover fund
(available <= fund). Keep requestCostCard’s existing message behavior and allow
the button when the balance is sufficient.
---
Nitpick comments:
In `@DashWallet/Sources/UI/Explore` Dash/UsernameMarketplaceScreen.swift:
- Around line 1285-1286: Move the `@ViewBuilder` attribute onto the same line as
each computed property declaration for contestedContent and the other affected
property, resolving the SwiftLint attributes violations without changing their
implementations.
🪄 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: 7c17626f-3d37-4999-90da-d2f40bf363bb
📒 Files selected for processing (4)
DashWallet/Sources/Infrastructure/SwiftDashSDK/Identity/DWContestedNameStatusService.swiftDashWallet/Sources/Infrastructure/SwiftDashSDK/UsernameMarketplaceService.swiftDashWallet/Sources/UI/Explore Dash/UsernameMarketplaceScreen.swiftDashWallet/en.lproj/Localizable.strings
🚧 Files skipped from review as they are similar to previous changes (1)
- DashWallet/en.lproj/Localizable.strings
…etplace # Conflicts: # DashWallet/en.lproj/Localizable.strings
…etplace # Conflicts: # DashWallet.xcodeproj/project.pbxproj # DashWallet/en.lproj/Localizable.strings
- Map PIN-gate outcomes to typed service errors (authCancelled / authFailed) via an authorize() wrapper, same shape as SwiftDashSDKContactsService, so the UI no longer imports the authorizer's error type. - Guard setPrice against duffs→credits overflow with a typed invalidPrice error. - Capture the network before any async work in requestContestedName and use the network-explicit recordSubmission overload, so a mid-flight network switch can't mis-scope the contest bookmark. - Treat any resolved contest winner as .locked in contestPrecheck — a label another identity won was offered as requestable. - Disable Request Username when the identity balance can't cover the vote-resolution fund. - Restrict DPNS label validation to ASCII (isLetter admitted unregistrable labels like "café"). - Release isPerformingAction as soon as the trade completes; the success banner auto-hides in a detached task instead of holding the buttons disabled for 2.5 s. - Abort the Buy action instead of falling back to a 0-credit expected price; report a missing wallet in the transfer sheet; gate the search row's For sale badge on isForSale. - Positional specifiers in the two-argument balance string; SwiftLint cleanups (sorted imports, case let, @ViewBuilder attribute placement). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Merged Fixed
Skipped
Clean |
QuantumExplorer
left a comment
There was a problem hiding this comment.
Reviewed the latest head (1354a1147). The canonical dashpay arm64 simulator build succeeds. I found one blocking correctness issue in the contested-name flow.
| // Bookmark BEFORE the vote-state read — Platform can legitimately | ||
| // return nil until the contest is indexed, and the conservative | ||
| // fallback deadline written here is what reconciliation leans on. | ||
| DWContestedNameStatusService.shared.recordSubmission(label: label, network: network) |
There was a problem hiding this comment.
[P1] Preserve every in-flight contested-name bookmark
This screen allows a second contested request whenever it is for a different label, but recordSubmission stores only one pendingLabel, so requesting B overwrites the bookmark for still-voting A. registerDpnsName preregisters each contested document, while DWCurrentUserIdentityInfo filters only that single bookmark from getDpnsNames(); after the overwrite, A can therefore appear in Edit Profile, invitation links, and payment username memo as if it were already owned, and app reconciliation only follows B. Please either persist/filter/reconcile all in-flight labels, or prevent any new contested request while one is pending.
There was a problem hiding this comment.
Fixed in d45ce91 by preventing a second contested request while one is pending (the scoped option — the multi-label rework of the bookmark store, DWCurrentUserIdentityInfo's filter, and checkPendingContestResolution touches ~10 consumer files and is marked TODO(contest-multi) as a follow-up):
requestContestedNamenow throws a typedcontestInProgress(pendingLabel:)whenever any bookmark is pending for the captured network, before the PIN prompt — the overwrite can no longer happen at the service boundary regardless of caller.RegisterNameSheetshows the one-at-a-time explanation (naming the in-flight label) in place of the cost card and submit button when a different label's request is pending, andalreadyRequestednow also consults the bookmark so an unsynced SDK cache can't offer a duplicate same-label submit.
Clean dashpay arm64 simulator build.
…ot bookmark The contest reconciliation bookmark in DWContestedNameStatusService is single-slot per network: submitting a second contested request while one is still voting overwrote the first's bookmark, leaking the still-voting label into username surfaces (Edit Profile, invitations, payment memos read getDpnsNames() filtered only by that bookmark) and orphaning its reconciliation. Until the bookmark store, DWCurrentUserIdentityInfo's filter, and checkPendingContestResolution go multi-label (TODO(contest-multi)), refuse a new contested request while any bookmark is pending: - requestContestedName throws a typed contestInProgress(pendingLabel:) naming the in-flight label. - RegisterNameSheet explains the one-at-a-time limit in place of the cost card and submit button; alreadyRequested now also consults the bookmark so an unsynced SDK cache can't offer a duplicate submit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Issue being fixed or feature implemented
A username marketplace over the DPNS v2 contract's trade surface. The v2 contract (
packages/dpns-contract/schema/v2) enables it all:transferable = 1,tradeMode = 1(direct purchase),keepsTransferHistory/keepsPurchaseHistory/keepsPricingHistory.Layering (what lives where)
Already in rs-sdk / swift-sdk (used directly by this PR): the generic document trade transitions —
setDocumentPrice,purchaseDocument,transferDocumentonManagedPlatformWallet(FFI-wired) — plusSDK.documentListwith where-clauses,dpnsNormalizeLabel,resolveDpnsName,registerDpnsName.Consensus facts this PR is built on (verified in rs-drive source):
$pricepresent (DocumentNotForSaleError) and the transition's price must equal the listed price — built-in protection against seller-side races.$price(document_purchase/transfer_transition_action/v0/transformer.rs) — so delist = transfer to self (ownership unchanged, listing cleared). The contract has no dedicated remove-price transition (documentsMutable = false).Delegated to the platform repo (spawned as the "Username marketplace: platform-wallet orchestration + history" task): sale-state persistence through the changeset pipeline, wallet-level purchase orchestration with local reconciliation (including the sold-main-username case), per-name transfer/purchase/pricing history via a document-revision query, typed errors at the FFI, and the
$priceindex investigation — the contract has no price index, so a global "browse everything for sale" is not queryable today. This PR's marketplace is deliberately search-driven until that lands; the app-side service is a thin facade the wallet-level APIs can replace without UI changes.What was done (app)
UsernameMarketplaceService(stateless facade, no singleton) +UsernameMarketplaceScreenreached from a new Explore row:parentNameAndLabelindex — onedocumentListquery returns full domain documents, so every result carries live sale state and price. Exact-label unregistered queries (validated against DPNS label rules, compared via the SDK's own normalizer since DPNS folds look-alike characters) offer a Register row — non-contested labels register directly on the identity; contested-eligible labels are refused with a pointer to the voting flow.records.identityindex), with sale badges. Pull-to-refresh.$createdAt/$transferredAtwhen the document carries them — the full trade timeline explicitly waits for the SDK revision-history query rather than faking one.purchaseDocumentwith the exact confirmed price. Post-purchase the identity snapshot refreshes so the name shows everywhere.resolveDpnsName, unresolvable input is an error — never guessed) or raw base58 identity id.Update — wired to the wallet-level SDK (platform #4348)
The second commit consumes the merged wallet layer end to end: typed
searchDpnsMarketplace/myDpnsMarketplaceNames(local rows including retained sold/transferred departures, rendered in a "No longer yours" section) /dpnsMarketplaceNameState/dpnsNameHistory— the detail sheet now shows the real trade timeline (registered / listed / purchased with counterparties and price / transferred, delist rendered as "Removed from sale"). Trade ops use the orchestrated calls with typed errors surfaced in user terms; delist uses the dedicateddelistDpnsName; the app-side documentList parser, manual pre-flight, and balance pre-check are deleted (the SDK owns them). The recurring DPNS sync starts with the wallet, and My Names pull-to-refresh runs asyncDpnsMarketplacepass.Seller clarity: a listed name that isn't yours reads "For sale by an independent user" on the search row, the detail sheet carries a callout ("offered by an independent user on the Dash network — not by Dash or this app; the seller sets the price"), and the purchase confirmation repeats it before the PIN gate.
Update — contested short names are requestable in place
The register sheet no longer dead-ends on contested-eligible labels. It submits the request natively: an explanation of the masternode vote, a pre-submit check of the label's state (fresh vote / active vote you'd join as a contender / locked by a past vote — no submit button on locked), the protocol's 0.2 DASH vote-resolution fund shown as the request cost next to the identity balance, and the honest footnote that the fund isn't returned if another contender wins. Submission reuses the setup flow's
registerDpnsName+ bookmark + reconciliation machinery; My Names grows an "In network vote" section with the voting deadline.finalizeWonnow backfills the global username mirror only when it's empty, so a contested win on a second name can't displace the user's existing username.Not in this PR (tracked)
$priceis not an indexable system property on Dash Platform, so this is not buildable at any layer today (confirmed in #4348's API docs).How Has This Been Tested?
Clean
dashpaybuild (both targets compile the new files; pbxproj registration mirrors the existing pattern). Installed on the mainnet QA simulator: search returns real DPNS documents with sale state, My Names lists the identity's names, and the detail/action sheets render per state. Trade actions (list/buy/transfer/register) are real mainnet transactions and were NOT fired in this pass — the flows stop at the PIN gate; end-to-end trade verification is planned on testnet alongside the platform task's own testnet run. (Unit-test target pre-existing broken.)Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit