feat(platform-wallet): wallet-level DPNS username marketplace with FFI and Swift wrappers - #4348
Merged
Merged
Conversation
Wallet-level orchestration for the DPNS marketplace, composing the existing generic document-trade transitions with the DPNS specifics the app layer should not own: - DpnsDomainState queries keeping $id and $price (search by prefix with cursor pagination, exact-name state, per-identity states via the records.identity index) - set_dpns_name_price / delist_dpns_name (transfer-to-self, verified to clear $price on the confirmed document) / transfer_dpns_name / purchase_dpns_name with typed pre-flight checks and automatic AUTHENTICATION+ECDSA signing-key selection - typed errors (DpnsNameNotFound, DocumentNotForSale, DocumentPriceChanged, InsufficientIdentityCredits, ContestedNameNotTradable) incl. consensus-error downcasts (40108 / 40109 / IdentityInsufficientBalanceError) wired into the generic set-price/purchase/transfer paths - dpns_name_history: per-name Registered/PriceSet/Purchased/Transferred timeline from the Document History system contract (byDocument index) - DpnsNameStateEntry persistence: new changeset + capability bit DPNS_NAME_STATES + sqlite table (V005) + in-memory working set on PlatformWalletInfo; sold/transferred rows retained with counterparty - dpns_names label-list merge/apply switched from append-only-by-label to LWW wholesale (every emitter snapshots the full list) so sold names can leave; set_dpns_names/remove_dpns_name mutations added - DpnsSyncManager: periodic marketplace sweep (60s default) detecting price changes, acquisitions, and departures (sold vs transferred classified through the history contract), with quiesce/shutdown parity with the sibling coordinators and a completion event Design record + browse-for-sale ($price index) investigation: docs/DPNS_MARKETPLACE.md — a global browse-by-price query is not buildable at any layer today (protocol-upgrade path documented). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…etplace; self-registering contract caches
examples/dpns_marketplace_testnet.rs — discover phase (HD identity map,
key-layout probe for an external identity id) and run phase exercising
the full wallet-level flow against real testnet: register -> list ->
re-price -> typed stale-price rejection -> purchase by a second identity
(owner/records/label reconciliation) -> history timeline -> typed
not-for-sale -> re-list -> delist via transfer-to-self with $price
verified cleared. All checks green 2026-08-09; transcript recorded in
docs/DPNS_MARKETPLACE.md §9.
Verification-driven fixes:
- register_name_with_external_signer now fetches and registers the DPNS
contract with the context provider BEFORE broadcasting — without it,
hosts that never seed known contracts fail post-broadcast proof
verification ("unknown contract ... in document verification") even
though the registration landed on-chain.
- the marketplace DPNS / Document History contract caches now hold the
on-chain FETCHED contract (via fetch_contract_arc_for_document_op,
which also registers it with the provider) instead of the bundled
system contract, so query proof verification matches the network's
active contract version and works on unseeded hosts.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… Swift wrappers FFI (rs-platform-wallet-ffi): - error codes 37-40 (DocumentNotForSale, DocumentPriceChanged, InsufficientIdentityCredits, ContestedNameNotTradable) with stable JSON detail payloads in the result message so hosts recover typed values; DpnsNameNotFound maps to NotFound (98) - dpns_marketplace.rs: search / name-state / my-names / set-price / delist / transfer / purchase / history / sync entry points over the wallet layer, with pointer-only repr(C) rows and paired destructors - dpns_sync.rs: manager-handle start/stop/sync-now/interval wrappers for the DpnsSyncManager coordinator - persistence: on_persist_dpns_name_states_fn vtable slot (appended, ABI-additive) + DpnsNameStateFFI mirror rows + DPNS_NAME_STATES capability dispatch swift-sdk: - PlatformWalletResultCode/PlatformWalletError mirrors incl. typed priceChanged/insufficientIdentityCredits/contestedNameNotTradable cases decoded from the JSON detail - DpnsMarketplace.swift: DpnsMarketplaceName / DpnsNameStateRow / DpnsNameHistoryEvent value types + ManagedPlatformWallet methods (searchDpnsMarketplace, dpnsMarketplaceNameState, myDpnsMarketplaceNames, setDpnsNamePrice, delistDpnsName, transferDpnsName, purchaseDpnsName, dpnsNameHistory, syncDpnsMarketplace) - DpnsSyncManager Swift wrappers; PersistentDPNSName grows marketplace columns (documentId, price, sale status, counterparty) fed by the new persister callback; marketplace columns are meaningful only while documentIdBase58 != nil (clear-don't-delete: the row is shared with the identity label cache) Verified: cargo test -p platform-wallet-ffi (290 tests) and -p platform-wallet (752) green; build_ios.sh --target mac + swift build clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QuantumExplorer
requested review from
ZocoLini,
lklimek,
llbartekll and
shumkov
as code owners
August 9, 2026 08:19
Contributor
|
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:
📝 WalkthroughWalkthroughAdds end-to-end DPNS marketplace support across Rust wallet logic, persistence, FFI, JNI, Swift, and Kotlin. The change includes trading, history, synchronization, typed errors, versioned callbacks, database state, and example interfaces. ChangesDPNS marketplace and wallet coordination
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Collaborator
|
🕓 Ready for review — next in queue (commit 0dd004c) |
QuantumExplorer
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4348 +/- ##
============================================
- Coverage 87.62% 86.12% -1.50%
============================================
Files 2668 2706 +38
Lines 339345 345369 +6024
============================================
+ Hits 297362 297466 +104
- Misses 41983 47903 +5920
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (6)
packages/rs-platform-wallet-ffi/src/dpns_sync.rs (1)
73-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap the raw-pointer writes in explicit
unsafeblocks.This file writes through
out_running,out_syncing,out_last_sync_unix, and the threesync_nowout-params without anunsafe { }block. The sibling moduledpns_marketplace.rswraps every equivalent write. Explicit blocks keep the two modules consistent and make the file safe underunsafe_op_in_unsafe_fn, which becomes the default in edition 2024.♻️ Proposed change for one site
check_ptr!(out_running); // Define the out-slot before the stale-handle early return below can // fire, so the caller never reads uninitialized stack contents. - *out_running = false; + unsafe { *out_running = false }; let option = PLATFORM_WALLET_MANAGER_STORAGE .with_item(handle, |manager| manager.dpns_sync().is_running()); let running = unwrap_option_or_return!(option); - *out_running = running; + unsafe { *out_running = running };Also applies to: 89-89, 109-109, 170-178
🤖 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 `@packages/rs-platform-wallet-ffi/src/dpns_sync.rs` at line 73, Wrap every raw-pointer dereference and write through out_running, out_syncing, out_last_sync_unix, and the three sync_now output parameters in explicit unsafe blocks within the affected functions, matching the established pattern in dpns_marketplace.rs and preserving the existing assignments.packages/rs-platform-wallet-ffi/src/persistence.rs (1)
6213-6225: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a round-level test for the DPNS projection.
store_projects_dashpay_payments_overlay_onlypins the payments fan-out through a sink callback. The new DPNS fan-out has no equivalent test. A mirrored test would pin three contracts that the unit tests indpns_name_state_persistence.rscannot cover: the callback fires only whenchangeset.dpns_name_statescarries rows or tombstones, the tombstone array projects the 32-byte document ids, and a nonzero callback return flipsround_success.🤖 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 `@packages/rs-platform-wallet-ffi/src/persistence.rs` around lines 6213 - 6225, Add a round-level test adjacent to store_projects_dashpay_payments_overlay_only for the DPNS projection, covering callback invocation only when changeset.dpns_name_states contains rows or tombstones, projection of tombstones as 32-byte document IDs, and propagation of a nonzero callback result to round_success. Mirror the existing sink-callback test setup without changing the production fan-out behavior.packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs (1)
1098-1137: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPaginate the history query or report truncation.
fetch_history_documentssends one query withlimit: HISTORY_QUERY_LIMIT(100) andstart: None. A name with more than 100 events of one type returns a silently truncated timeline, anddpns_name_historypresents it as complete. A frequently re-priced name reaches this bound throughpriceUpdatedocuments alone.Loop with
Start::StartAfter(last_id)until a page returns fewer thanHISTORY_QUERY_LIMITrows, or return a "truncated" flag so the caller can render it.🤖 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 `@packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs` around lines 1098 - 1137, Update fetch_history_documents to paginate through all history documents using the query’s start cursor, continuing with Start::StartAfter(last_id) until a page contains fewer than HISTORY_QUERY_LIMIT rows; preserve ordering and accumulate results across pages so dpns_name_history does not receive a silently truncated timeline.packages/rs-platform-wallet/src/wallet/identity/network/dpns.rs (1)
135-154: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winApply the same contract pre-registration to
register_name_with_signer.
register_name_with_external_signernow callsself.dpns_contract().await?at Line 195 so the post-broadcast proof can resolve the DPNS contract.register_name_with_signerreaches the sameself.sdk.register_dpns_name(input)broadcast at Line 152 without that step, so it stays exposed to the "unknown contract … in document verification" failure on a host that does not pre-seed known contracts.
dpns_contract()returnsPlatformWalletError, while this method returnsdash_sdk::Error, so the call needs a mapping or an ignore-on-failure.🤖 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 `@packages/rs-platform-wallet/src/wallet/identity/network/dpns.rs` around lines 135 - 154, Update register_name_with_signer to call self.dpns_contract().await before constructing or submitting RegisterDpnsNameInput, matching register_name_with_external_signer’s pre-registration contract step. Convert the PlatformWalletError into dash_sdk::Error, or explicitly ignore the lookup failure as appropriate, while preserving the existing registration and full_domain_name return flow.packages/rs-platform-wallet/src/manager/mod.rs (1)
798-800: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
DpnsSyncManagerto the shutdown doc list.The doc comment enumerates the quiesced coordinators.
DpnsSyncManageris now quiesced at Line 883 and Line 896 but is absent from the list.📝 Proposed doc fix
- /// Stops SPV and **quiesces** the periodic coordinators - /// (`PlatformAddressSyncManager`, `IdentitySyncManager`, - /// `DashPaySyncManager`, `ShieldedSyncManager`) — cancelling each loop + /// Stops SPV and **quiesces** the periodic coordinators + /// (`PlatformAddressSyncManager`, `IdentitySyncManager`, + /// `DashPaySyncManager`, `DpnsSyncManager`, `ShieldedSyncManager`) — cancelling each loop🤖 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 `@packages/rs-platform-wallet/src/manager/mod.rs` around lines 798 - 800, Update the shutdown documentation comment near the coordinator list to include DpnsSyncManager alongside PlatformAddressSyncManager, IdentitySyncManager, DashPaySyncManager, and ShieldedSyncManager, matching the coordinators quiesced by the shutdown implementation.packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDPNSName.swift (1)
100-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare the defaults that the comments promise.
Line 101 states that
saleStatusRaw"Defaults to 0", and Line 111 states thatmarketplaceUpdatedAtis0when never written. Neither property declares a default value. Both are non-optional, so the documented default exists only becauseinitassigns it at Line 153 and Line 155. Add the defaults to the declarations so the stated contract holds for every construction path.♻️ Proposed change
- public var saleStatusRaw: Int16 + public var saleStatusRaw: Int16 = 0 @@ - public var marketplaceUpdatedAt: UInt64 + public var marketplaceUpdatedAt: UInt64 = 0🤖 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 `@packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDPNSName.swift` around lines 100 - 112, Declare inline default values of 0 for saleStatusRaw and marketplaceUpdatedAt in the PersistentDPNSName model, while preserving the existing initializer behavior and types.
🤖 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 `@packages/rs-platform-wallet-ffi/src/dpns_marketplace.rs`:
- Around line 179-181: Run cargo fmt --all to apply rustfmt-default formatting
across both new FFI modules. In
packages/rs-platform-wallet-ffi/src/dpns_marketplace.rs lines 179-181, reformat
owned_c_string and all reported chains in lines 177-917, including the iterator
collects at 384-385 and 457-458; in
packages/rs-platform-wallet-ffi/src/dpns_sync.rs lines 75-76, also reformat the
PLATFORM_WALLET_MANAGER_STORAGE.with_item chains at 75-76, 91-92, and 111-112
using rustfmt’s leading .with_item continuation form.
In `@packages/rs-platform-wallet-ffi/src/error.rs`:
- Around line 1405-1413: Move the marketplace helper `message_of` and its
associated tests so the documentation block for
`message_signing_failed_falls_through_to_unknown` directly precedes that test.
Ensure the helper’s own documentation remains attached only to `message_of`, and
preserve the existing test behavior.
In `@packages/rs-platform-wallet-storage/migrations/V005__dpns_name_states.rs`:
- Line 23: Update the migration schema near the price column in V005 to enforce
CHECK (price IS NULL OR price >= 0), preserving NULL as valid while rejecting
negative prices. Add or verify the migration test covering price = -1 and assert
that inserting such a row fails.
In `@packages/rs-platform-wallet-storage/src/sqlite/schema/dpns_name_states.rs`:
- Around line 16-61: Run rustfmt with workspace defaults on both affected files:
reformat the params! argument list in apply within dpns_name_states.rs,
including the inline map/transpose expressions, and reformat the
DpnsNameStateFFI struct literal in build_dpns_name_state_entries within
dpns_name_state_persistence.rs; preserve all behavior and ensure cargo fmt
--check --all passes.
In `@packages/rs-platform-wallet/examples/dpns_marketplace_testnet.rs`:
- Around line 116-132: Run cargo fmt on
packages/rs-platform-wallet/examples/dpns_marketplace_testnet.rs and accept
rustfmt’s formatting throughout the file, including collapsing
private_key_matches’s signature and wrapping the over-width println! calls.
- Around line 120-124: Update the SecretKey construction in the secp256k1
key-generation flow to pass the byte array by value: dereference sk_bytes when
calling SecretKey::from_byte_array, while preserving the existing failure return
and public-key derivation.
In `@packages/rs-platform-wallet/src/manager/load.rs`:
- Line 102: Update the wallet load restore path to retrieve persisted DPNS rows
despite the load contract lacking a DPNS field, convert them into the expected
state map, and assign that map to dpns_name_states instead of initializing an
empty BTreeMap. Preserve the local_dpns_name_states behavior after restart
without requiring synchronization.
In `@packages/rs-platform-wallet/src/test_support.rs`:
- Line 257: Adjust the indentation of dpns_name_states in all five
PlatformWalletInfo initializers to match the surrounding fields, especially
tracked_asset_locks, using standard rustfmt formatting.
In `@packages/rs-platform-wallet/src/wallet/apply.rs`:
- Around line 165-176: Update DpnsNameStateChangeSet::merge so it preserves the
chronological order between names upserts and removed tombstones, preventing
stale tombstones from deleting newer upserts when apply_changeset processes
removed after names. Ensure both upsert-then-remove and remove-then-upsert
sequences behave correctly, and add tests covering both orders plus repeated
replay.
In `@packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs`:
- Around line 45-49: Run cargo fmt on the entire Rust file and apply all
rustfmt-generated formatting changes, including reordering the dash_sdk imports
around Start and any differences through the rest of the file. Do not make
unrelated code changes.
- Around line 903-913: Update the documentation comment for purchase_dpns_name
to remove the claim that a wallet-owned seller’s row transitions to Sold; state
that only the seller label and balance are reconciled, and that no separate Sold
row is persisted when buyer and seller share a wallet due to document_id-only
row keying.
- Around line 301-341: Scope the caches used by dpns_contract and
document_history_contract to the wallet manager’s network rather than
process-wide function-local OnceLock values. Key each cache by a stable network
identifier, or move the caches onto the network-bound SDK/wallet instance, so
wallets on different networks cannot share contract definitions. Preserve
concurrent initialization, cache-hit provider registration, and returning the
contract for the caller’s network.
In
`@packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDPNSName.swift`:
- Around line 177-187: Update the sale-status mapping in PersistentDPNSName to
match raw value 0 explicitly to .owned, while preserving the existing cases for
sold and transferred; return nil for any other unrecognized value so unreliable
rows are not reported as owned.
---
Nitpick comments:
In `@packages/rs-platform-wallet-ffi/src/dpns_sync.rs`:
- Line 73: Wrap every raw-pointer dereference and write through out_running,
out_syncing, out_last_sync_unix, and the three sync_now output parameters in
explicit unsafe blocks within the affected functions, matching the established
pattern in dpns_marketplace.rs and preserving the existing assignments.
In `@packages/rs-platform-wallet-ffi/src/persistence.rs`:
- Around line 6213-6225: Add a round-level test adjacent to
store_projects_dashpay_payments_overlay_only for the DPNS projection, covering
callback invocation only when changeset.dpns_name_states contains rows or
tombstones, projection of tombstones as 32-byte document IDs, and propagation of
a nonzero callback result to round_success. Mirror the existing sink-callback
test setup without changing the production fan-out behavior.
In `@packages/rs-platform-wallet/src/manager/mod.rs`:
- Around line 798-800: Update the shutdown documentation comment near the
coordinator list to include DpnsSyncManager alongside
PlatformAddressSyncManager, IdentitySyncManager, DashPaySyncManager, and
ShieldedSyncManager, matching the coordinators quiesced by the shutdown
implementation.
In `@packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs`:
- Around line 1098-1137: Update fetch_history_documents to paginate through all
history documents using the query’s start cursor, continuing with
Start::StartAfter(last_id) until a page contains fewer than HISTORY_QUERY_LIMIT
rows; preserve ordering and accumulate results across pages so dpns_name_history
does not receive a silently truncated timeline.
In `@packages/rs-platform-wallet/src/wallet/identity/network/dpns.rs`:
- Around line 135-154: Update register_name_with_signer to call
self.dpns_contract().await before constructing or submitting
RegisterDpnsNameInput, matching register_name_with_external_signer’s
pre-registration contract step. Convert the PlatformWalletError into
dash_sdk::Error, or explicitly ignore the lookup failure as appropriate, while
preserving the existing registration and full_domain_name return flow.
In
`@packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDPNSName.swift`:
- Around line 100-112: Declare inline default values of 0 for saleStatusRaw and
marketplaceUpdatedAt in the PersistentDPNSName model, while preserving the
existing initializer behavior and types.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c7a22db-8b5c-4f8d-9c93-657f7dcaf5e6
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (42)
packages/rs-platform-wallet-ffi/src/dpns_marketplace.rspackages/rs-platform-wallet-ffi/src/dpns_name_state_persistence.rspackages/rs-platform-wallet-ffi/src/dpns_sync.rspackages/rs-platform-wallet-ffi/src/error.rspackages/rs-platform-wallet-ffi/src/lib.rspackages/rs-platform-wallet-ffi/src/persistence.rspackages/rs-platform-wallet-storage/migrations/V005__dpns_name_states.rspackages/rs-platform-wallet-storage/src/sqlite/persister.rspackages/rs-platform-wallet-storage/src/sqlite/schema/dpns_name_states.rspackages/rs-platform-wallet-storage/src/sqlite/schema/mod.rspackages/rs-platform-wallet/Cargo.tomlpackages/rs-platform-wallet/docs/DPNS_MARKETPLACE.mdpackages/rs-platform-wallet/examples/dpns_marketplace_testnet.rspackages/rs-platform-wallet/src/changeset/changeset.rspackages/rs-platform-wallet/src/changeset/mod.rspackages/rs-platform-wallet/src/changeset/persistence_capabilities.rspackages/rs-platform-wallet/src/error.rspackages/rs-platform-wallet/src/events.rspackages/rs-platform-wallet/src/manager/accessors.rspackages/rs-platform-wallet/src/manager/dpns_sync.rspackages/rs-platform-wallet/src/manager/load.rspackages/rs-platform-wallet/src/manager/mod.rspackages/rs-platform-wallet/src/manager/wallet_lifecycle.rspackages/rs-platform-wallet/src/test_support.rspackages/rs-platform-wallet/src/wallet/apply.rspackages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rspackages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rspackages/rs-platform-wallet/src/wallet/identity/network/document.rspackages/rs-platform-wallet/src/wallet/identity/network/dpns.rspackages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rspackages/rs-platform-wallet/src/wallet/identity/network/mod.rspackages/rs-platform-wallet/src/wallet/identity/state/managed_identity/identity_ops.rspackages/rs-platform-wallet/src/wallet/identity/state/manager/apply.rspackages/rs-platform-wallet/src/wallet/platform_wallet.rspackages/rs-platform-wallet/src/wallet/platform_wallet_traits.rspackages/swift-sdk/Sources/SwiftDashSDK/Persistence/DashModelContainer.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDPNSName.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/DpnsMarketplace.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerDpnsSync.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift
…n the JNI vtable `PersistenceCallbacks` is constructed exhaustively in rs-unified-sdk-jni's `build_vtable`, so adding `on_persist_dpns_name_states_fn` broke the Android build — a break no local Rust or Swift verification compiles (only CI's Kotlin job does). Android has no marketplace surface and no Kotlin mirror for the name-state rows, so the slot is an explicit `None` with the consequence documented: the FFI persister only attests `DPNS_NAME_STATES` when the callback is wired, so the capability stays unattested and marketplace sale state is session-scoped on Android rather than reported as durable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CI's Rust workspace job runs `cargo fmt --all --check` before the test run; the new marketplace files (wallet, storage, FFI, example) were not rustfmt-clean. Formatting only — no behavioural change; tests re-run green (752 + 268 + 132). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… merge ordering, honest unknowns CI (secrets scan): - rename the `seed` fixture discriminator in the dpns_name_states test; `seed` is a FORBIDDEN substring in schema files (secrets_scan guard, see SECRETS.md). The guard is a deliberate blunt scan — rename rather than weaken it. Review fixes: - **contract caches are now keyed by network.** Switching these to on-chain FETCHED contracts (4adfca7) made a process-global OnceLock wrong: a host can hold managers on several networks at once, and the first caller's network would decide the contract definition every later caller used for queries and proof verification. Networks run different protocol versions, so the DPNS / Document History contracts can differ in schema. Now a (network, contract id)-keyed cache, with no lock held across the await. - `DpnsNameStateChangeSet::merge` is last-OPERATION-wins per document id: each side evicts the key from the other, so a stale tombstone can no longer swallow a newer upsert (the sqlite writer applies inserts before deletes, and a marketplace row can legitimately come back when a name is re-acquired). Tests cover both orders plus replay. - `purchase_dpns_name` doc no longer claims the seller gets a `Sold` row — the buyer's `Owned` row already occupies that document_id key, so the seller's departure is the label removal; `Sold` rows come from the sync pass. - Swift `PersistentDPNSName.saleStatus` returns nil for an unknown discriminant instead of `.owned`, so an older build reports a departed name as unreadable rather than still-owned. - V005 `price` column gains CHECK (price IS NULL OR price >= 0) as a corruption backstop, with a test. - move the FFI `message_of` helper / `MessageSigningFailed` doc block so the doc block attaches to its own test again. Verified: 752 + 268 + 133 tests, secrets_scan, cargo fmt --all --check, cargo check --workspace --all-features, swift build — all clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…scoped `local_dpns_name_states` reads an in-memory map that starts empty on every process start and is repopulated by the first sync pass — the wallet load path does not rehydrate it, matching the invitations store (SqlitePersister does not attest WALLET_RESTORE). Say so on the method and record the restore work as a known follow-up in the design doc, so no caller reads an empty result as "no names". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… alias CI runs `cargo clippy --workspace --all-targets --all-features --locked -- --no-deps -D warnings`, which denies `clippy::type_complexity` on the inline `OnceLock<RwLock<BTreeMap<(Network, Identifier), Arc<DataContract>>>>` introduced with the per-network cache. Extract `SystemContractCache`. Verified with CI's exact clippy invocation across the workspace. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ead-only
TC-P1-003 requires every `.prepare(` in `src/sqlite/schema/*.rs` to be a
writer using `prepare_cached`, unless the SELECT is listed in
`READ_ONLY_PREPARE_ALLOWED`. `dpns_name_states::read_all` is a
test-gated one-shot reader — same shape as `invitations::read_all` —
so list it rather than caching a statement no production path runs.
Verified: 1393 tests across platform-wallet{,-ffi,-storage}, 0 failures.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
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 `@packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs`:
- Line 1006: Update the required-amount calculation near expected_price to
detect overflow when adding DOCUMENT_TRANSITION_FEE_RESERVE_CREDITS and return
InvalidParameter before broadcast instead of using saturating_add; preserve the
existing availability validation for non-overflowing sums.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: be07844c-79a7-4621-bfd1-264866e9472a
📒 Files selected for processing (15)
packages/rs-platform-wallet-ffi/src/dpns_marketplace.rspackages/rs-platform-wallet-ffi/src/dpns_name_state_persistence.rspackages/rs-platform-wallet-ffi/src/dpns_sync.rspackages/rs-platform-wallet-ffi/src/error.rspackages/rs-platform-wallet-storage/migrations/V005__dpns_name_states.rspackages/rs-platform-wallet-storage/src/sqlite/schema/dpns_name_states.rspackages/rs-platform-wallet-storage/tests/sqlite_compile_time.rspackages/rs-platform-wallet/docs/DPNS_MARKETPLACE.mdpackages/rs-platform-wallet/examples/dpns_marketplace_testnet.rspackages/rs-platform-wallet/src/changeset/changeset.rspackages/rs-platform-wallet/src/changeset/mod.rspackages/rs-platform-wallet/src/test_support.rspackages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rspackages/rs-unified-sdk-jni/src/persistence.rspackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDPNSName.swift
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/rs-platform-wallet/src/test_support.rs
- packages/rs-platform-wallet/src/changeset/mod.rs
- packages/rs-platform-wallet-storage/migrations/V005__dpns_name_states.rs
- packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDPNSName.swift
- packages/rs-platform-wallet-ffi/src/error.rs
- packages/rs-platform-wallet-ffi/src/dpns_marketplace.rs
- packages/rs-platform-wallet/src/changeset/changeset.rs
- packages/rs-platform-wallet-ffi/src/dpns_sync.rs
- packages/rs-platform-wallet/docs/DPNS_MARKETPLACE.md
- packages/rs-platform-wallet/examples/dpns_marketplace_testnet.rs
- packages/rs-platform-wallet-ffi/src/dpns_name_state_persistence.rs
…ts Swift call sites type-check Three layers, all masked by an incremental `swift build` reusing a stale generated header: - `PersistenceCallbacksExtension.on_persist_dpns_name_states_fn` was declared as `Option<PersistDpnsNameStatesFn>`. cbindgen does not expand a named fn-pointer alias inside `Option`: it emitted an opaque `struct Option_PersistDpnsNameStatesFn` forward declaration and then used it BY VALUE, an incomplete type that made the whole DashSDKFFI clang module unbuildable. Declared inline like every sibling callback, with the reason recorded so the next one stays inline. - With the struct finally generated correctly, its `struct_size` / `version` fields are `uintptr_t` / `uint32_t`, but the Swift setup assigned `Int` / `Int32` — code that had never type-checked because the module never built. Added explicit UInt / UInt32 conversions at both call sites. - `InvitationPersistenceTests` pins the exact attested capability set; add `dpnsNameStates`, which the handler genuinely attests now that `on_persist_dpns_name_states_fn` is wired to PersistentDPNSName. Verified after a real `build_ios.sh --target mac` regeneration (not an incremental cache hit): header emits a proper function pointer, `swift test` 326 passed / 0 failed, `cargo test -p platform-wallet-ffi` 292 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DpnsMarketplacePersistenceTests.swift (1)
84-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
removed:path ofpersistDpnsNameStates.Both tests pass
removed: []. The removal branch is untested. That branch enforces a specific contract: it clears only the marketplace columns and keeps the label-cache row. If a future change deletes the row instead, it destroys state that the identity snapshot owns, and no test fails.The setup helpers in this file already cover everything the test needs.
💚 Proposed test for the removal path
+ func testRemovalClearsOnlyTheMarketplaceSection() throws { + let context = ModelContext(container) + let owner = PersistentIdentity(identityId: ownerId, isLocal: false, network: .testnet) + let alice = PersistentDPNSName(identity: owner, label: "Alice", acquiredAt: 10) + let documentId = Data(repeating: 0x33, count: 32).toBase58String() + alice.documentIdBase58 = documentId + alice.priceCredits = 5_000 + alice.saleStatusRaw = 1 + alice.counterpartyIdBase58 = Data(repeating: 0x44, count: 32).toBase58String() + alice.marketplaceUpdatedAt = 100 + context.insert(owner) + context.insert(alice) + try context.save() + + handler.beginChangeset(walletId: walletId) + XCTAssertTrue(handler.persistDpnsNameStates( + walletId: walletId, + upserts: [], + removed: [documentId] + )) + XCTAssertTrue(handler.endChangeset(walletId: walletId, success: true)) + + let readContext = ModelContext(container) + let rows = try readContext.fetch(FetchDescriptor<PersistentDPNSName>()) + XCTAssertEqual(rows.count, 1, "removal must clear the marketplace section, not delete the row") + XCTAssertEqual(rows.first?.label, "Alice") + XCTAssertNil(rows.first?.documentIdBase58) + XCTAssertNil(rows.first?.priceCredits) + XCTAssertEqual(rows.first?.saleStatusRaw, 0) + XCTAssertNil(rows.first?.counterpartyIdBase58) + XCTAssertEqual(rows.first?.marketplaceUpdatedAt, 0) + }🤖 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 `@packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DpnsMarketplacePersistenceTests.swift` around lines 84 - 100, Add a test in the existing DPNS marketplace persistence test suite covering persistDpnsNameStates with a non-empty removed argument. Assert that removal clears only marketplace-owned columns while preserving the label-cache row and identity-snapshot state, using the file’s existing setup helpers and test symbols.
🤖 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
`@packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDPNSName.swift`:
- Around line 74-78: Make the canonical identity snapshot the sole authority for
PersistentDPNSName.isOwned, preventing later marketplace Owned updates from
restoring true after an identity snapshot sets false. Reconcile marketplace
persistence with the stored identity state, and add a regression test covering
the identity-then-marketplace update sequence.
In
`@packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentIdentity.swift`:
- Around line 87-90: Update upsertDPNSNames in PlatformWalletPersistenceHandler
so a sold or transferred DPNS row is not reassigned to the current identity when
the previous identity remains stored; preserve the previous-owner relationship
and retained history, using separate history storage or narrowing the
PersistentIdentity documentation contract accordingly. Add a regression test
covering same-store transfers with both identities present.
---
Nitpick comments:
In
`@packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DpnsMarketplacePersistenceTests.swift`:
- Around line 84-100: Add a test in the existing DPNS marketplace persistence
test suite covering persistDpnsNameStates with a non-empty removed argument.
Assert that removal clears only marketplace-owned columns while preserving the
label-cache row and identity-snapshot state, using the file’s existing setup
helpers and test symbols.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b7d9fcf2-27a4-4794-8113-1f332795638d
📒 Files selected for processing (10)
packages/rs-platform-wallet-ffi/src/manager.rspackages/rs-platform-wallet-ffi/src/persistence.rspackages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rspackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDPNSName.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentIdentity.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/DpnsMarketplace.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swiftpackages/swift-sdk/SwiftTests/SwiftDashSDKTests/DpnsMarketplacePersistenceTests.swiftpackages/swift-sdk/SwiftTests/SwiftDashSDKTests/InvitationPersistenceTests.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/DpnsMarketplace.swift
thepastaclaw
suggested changes
Aug 9, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Preliminary review — Codex only
The marketplace ownership, synchronization, and cache boundaries still permit incorrect wallet state, stale proof-verification context, and remotely amplified recurring work; six in-scope blocking findings require changes. The Rust DPNS completion event also stops at the FFI boundary, leaving recurring Swift clients without the documented refresh signal.
Source: reviewer backends codex-general=gpt-5.6-sol, codex-security-auditor=gpt-5.6-sol, codex-rust-quality=gpt-5.6-sol, codex-ffi-engineer=gpt-5.6-sol; final verifier backend codex-verifier=gpt-5.6-sol. openclaw-agent/cliproxy/gpt-5.6-sol is orchestration-only and not reviewer evidence.
Validated blockers were found in the Codex precheck. Opus is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— security-auditor (completed),gpt-5.6-sol— rust-quality (completed),gpt-5.6-sol— ffi-engineer (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 6 blocking | 🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs:1297-1304: Marketplace sync treats observed identities as wallet-owned
`IdentityManager::identity_ids()` deliberately includes both signing-capable identities in `wallet_identities` and observed, read-only identities in `out_of_wallet_identities`. This pass consequently queries `records.identity` for every observed contact, persists their documents as this wallet's `Owned` names, and can mutate their legacy `dpns_names` lists. It also discloses the wallet's observed/contact identity set through periodic DAPI queries. The same ownership erasure occurs in `is_wallet_identity` at lines 798-802 because `IdentityManager::identity()` searches both buckets; transferring a name to an observed contact is therefore treated as an intra-wallet transfer and replaces the sender's departure row with an `Owned` row for that contact. Build the sync snapshot and ownership predicates from `wallet_identities[&self.wallet_id]` only, and apply the same restriction before signing, balance, and label mutations.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs:1427-1447: Clear or reclassify the persisted row when a DPNS document was deleted
`Ok(None)` is a proof-verified result that the domain document no longer exists, but it is handled together with a transport or proof failure. The caller removes the identity's label and writes no row or tombstone, leaving the previous `DpnsNameStateEntry` intact—often with `status == Owned` and a stale sale price. The Rust `my names` API and Swift marketplace fields can therefore continue reporting a deleted name as owned indefinitely. Split `Ok(None)` from `Err(_)`: a confirmed absence must remove or explicitly reclassify the tracked row, while a failed lookup may retain it for retry.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs:1402-1403: Serialize sync reconciliation with marketplace trade operations
The sync pass performs its network reads before taking wallet-state locks and later writes every fetched row unconditionally. The manager's `is_syncing` flag only serializes manager-level sync passes; the public per-wallet sync entry point and `set_dpns_name_price`, `delist_dpns_name`, `transfer_dpns_name`, and `purchase_dpns_name` bypass it. A sync can fetch an old owned or unlisted document, a concurrent trade can confirm and persist a newer listed or transferred state, and this final write can then overwrite the confirmed result and re-add a departed label. Add a per-wallet DPNS operation gate shared by sync and trade reconciliation, or reject stale reconciliation with an authoritative freshness check covering both rows and label mutations.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs:334-345: Do not reuse a system contract solely by Network enum
The process-wide cache now prevents mainnet/testnet collisions, but `(dashcore::Network, contract_id)` still does not identify an SDK trust context, a particular devnet/regtest deployment, or the active contract version. System-contract upgrades retain the contract ID, and `Sdk::set_context_provider` explicitly permits provider replacement. A cache hit after a provider/chain replacement or contract upgrade therefore registers the old contract into the new provider without fetching or proving it against that context. Queries, transition construction, and post-broadcast proof verification can then use a stale or foreign schema until process restart. Scope the cache to the SDK/provider/chain lifetime and invalidate it when that context or active contract version changes, rather than using a permanent process-global map.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs:1314-1317: Unsolicited name transfers can make every sync pass unbounded
Every recurring pass calls `dpns_domain_states_for_identity(..., None)`, whose pagination loop drains all pages and accumulates every document in one `Vec`. A DPNS transfer requires the current owner's authorization but no recipient consent, and the protocol imposes no per-identity name-count bound. An attacker can therefore transfer many names to a known victim identity and force that wallet to repeat unbounded proof queries, allocations, conversions, and persistence work every 60 seconds. The attacker pays the on-chain cost once while the victim repeatedly pays the client cost. Process pages incrementally under a fixed per-pass budget and retain a cursor so injected names cannot create sustained network, CPU, and memory pressure or starve legitimate names.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs:1491-1499: Multi-hop departures are attributed to the latest owner instead of the actual counterparty
`classify_departure` does not receive the identity whose label departed. It only checks whether a purchase document's buyer and timestamp match the live domain document's latest owner and transfer timestamp. If `S` owned the name before the app went offline and ownership then moved `S → A → B`, the latest `A → B` purchase satisfies this predicate and records `S` as having sold directly to `B`; if `S → A` was a transfer, it can also refresh `S` as a seller even though `A` made the sale. Match history against the departing identity—`sellerId` for purchases and the history document's `$ownerId` for transfers—and persist the recipient from that matching departure event rather than the current live owner.
In `packages/rs-platform-wallet/src/events.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/events.rs:37-46: Forward DPNS sync completion through the C and Swift event bridge
`DpnsSyncManager::sync_now` dispatches this new event, but `FFIEventHandler` does not override `on_dpns_marketplace_sync_completed`, and Swift's `PlatformWalletEventHandler.makeCallbacks()` has no corresponding callback or trampoline. Recurring synchronization therefore silently discards the per-wallet summaries at the Rust/FFI boundary, so Swift clients cannot receive the departure signal that this method documents as the trigger for refreshing main-username and profile state. Add a C/Swift event bridge with owned summary values. Because the existing `EventHandlerCallbacks` is unsized and consumed via `ptr::read`, use a size/version-tagged extension or registration API rather than introducing another unsafe by-value growth for older callers.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (10)
packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/persistence/DashDatabaseMigrationTest.kt (1)
378-392: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert preservation of the legacy DPNS columns.
The fixture writes
label,normalizedLabel,parentDomainName,normalizedParentDomainName,acquiredAt, andidentityId. The query checks only the new marketplace columns. A migration that loses or changes the existing values would still pass.Extend the query and assertions to verify the inserted legacy values and row identity.
This follows the test's stated v9-to-v10 data-preservation objective.
🤖 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 `@packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/persistence/DashDatabaseMigrationTest.kt` around lines 378 - 392, Extend the query in DashDatabaseMigrationTest to include the legacy DPNS columns label, normalizedLabel, parentDomainName, normalizedParentDomainName, acquiredAt, and identityId, then assert their inserted values along with the existing marketplace-column checks. Also verify the expected row identity, preserving the v9-to-v10 data-preservation coverage.packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/DpnsMarketplacePresentationTests.swift (1)
25-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd an assertion for the
.notForSalemapping.
DpnsMarketplaceUI.error(_:)maps five marketplace cases explicitly. This test covers.priceChanged,.insufficientIdentityCredits,.contestedNameNotTradable, and.signingKeyUnavailable. It does not cover.notForSale. A regression that drops that case would fall through todefaultand emit the rawlocalizedDescriptionwithout detection.💚 Proposed additional assertion
let signing = DpnsMarketplaceUI.error( PlatformWalletError.signingKeyUnavailable("key 3 is watch-only") ) XCTAssertTrue(signing.contains("signing key is unavailable")) XCTAssertTrue(signing.contains("Unlock or repair")) + + let notListed = DpnsMarketplaceUI.error(PlatformWalletError.notForSale) + XCTAssertTrue(notListed.contains("no longer listed")) }Confirm the exact
notForSalecase signature before applying; it takes no associated value inDpnsMarketplaceView.swiftline 757.🤖 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 `@packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/DpnsMarketplacePresentationTests.swift` around lines 25 - 52, Extend testTypedTradeErrorsProduceActionableMessages to construct PlatformWalletError.notForSale with no associated value, pass it to DpnsMarketplaceUI.error(_:), and assert the result contains the expected not-for-sale mapping text. Preserve the existing assertions for the other typed marketplace errors.packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DpnsMarketplaceDecodingTests.swift (2)
251-257: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the fixed yield count with an explicit expectation.
The loop yields at most 10 times and then falls through. The bound is arbitrary. If the manager needs more than 10 suspension points to publish, the test fails intermittently instead of deterministically.
XCTAssertEqualdoes report the failure, so the test is not silently green, but the diagnostic points at the assertion rather than at the missed event.Use an
XCTestExpectationwith a timeout, or poll against a deadline and callXCTFailwhen the deadline passes.♻️ Proposed deadline-based wait
- for _ in 0..<10 where manager.lastDpnsSyncEvent == nil { - await Task.yield() - } - - let event = manager.lastDpnsSyncEvent + let deadline = Date().addingTimeInterval(2) + while manager.lastDpnsSyncEvent == nil, Date() < deadline { + await Task.yield() + } + + let event = try XCTUnwrap(manager.lastDpnsSyncEvent, "No DPNS sync event was published") XCTAssertEqual(event?.syncUnixSeconds, 123)
XCTUnwrapthrows, so mark the test methodasync throwsif you adopt this form.🤖 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 `@packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DpnsMarketplaceDecodingTests.swift` around lines 251 - 257, Replace the bounded Task.yield loop in the test with an explicit XCTestExpectation or deadline-based polling wait for manager.lastDpnsSyncEvent. Fail explicitly with XCTFail when the event is not published before the timeout, and mark the test async throws if using XCTUnwrap; keep the existing event assertions after successful retrieval.
72-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the unclassified-departure path.
This test exercises
DpnsNameDepartureonly withhas_status = trueandstatus = 1. The decoder atDpnsMarketplace.swiftlines 347-348 maps every other combination tostatus = nil, and that is the documented "the sweep could only establish that the name departed" case. That branch is reachable from the Rust sweep and is currently untested.Add a case with
has_status = falseand assertstatusis nil whileidentityIdandlabelstill decode.💚 Proposed additional coverage
let departed = DpnsNameDeparture(ffi: departedFFI) XCTAssertEqual(departed.identityId, identityId) XCTAssertEqual(departed.documentId, documentId) XCTAssertEqual(departed.status, .sold(to: counterparty)) + + var unclassifiedFFI = departedFFI + unclassifiedFFI.has_status = false + let unclassified = DpnsNameDeparture(ffi: unclassifiedFFI) + XCTAssertEqual(unclassified.identityId, identityId) + XCTAssertNil(unclassified.status)🤖 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 `@packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DpnsMarketplaceDecodingTests.swift` around lines 72 - 88, Add a test case alongside the existing DpnsNameDeparture decoding coverage that constructs DpnsNameDepartedFFI with has_status = false, then assert DpnsNameDeparture.status is nil while identityId and label still decode correctly. Reuse the existing departure fixture/setup patterns and keep the sold-status case unchanged.packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs (1)
1365-1365: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftA stuck departure blocks the ownership scan for that identity.
Line 1365 runs the ownership page scan only when
pending_departuresis empty. Lines 1462-1465 push the item back andbreakon a retryable failure, leaving the queue non-empty.If one departed name keeps failing its lookup, the identity never refreshes owned rows or price changes again for the process lifetime. Consider rotating the failed item to the back of the queue, or tracking an attempt count so a persistently failing item cannot starve the ownership scan.
Also applies to: 1454-1482
🤖 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 `@packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs` at line 1365, Update the departure-processing flow around the pending_departures check and the retryable-failure handling near the departure loop so one repeatedly failing item cannot prevent the ownership scan from running. Rotate failed departures behind other queued items or enforce an attempt limit, while preserving retries and allowing the ownership/price refresh path to execute even when a failure remains queued.packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/storage/StorageModels.kt (1)
84-92: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueHandle
saleStatusRaw == 0separately from an unclassified departure.
DpnsNameSaleStatus.OWNEDis raw value0. A row withisOwned == falseandsaleStatusRaw == 0is a name whose marketplace state was cleared or never synced. The currentelsebranch labels it "departed". Add an explicit0Lbranch so an uncategorised row reads as unknown instead of asserting a departure.♻️ Proposed change
val ownership = if (row.bool("isOwned")) "owned" else when (row.long("saleStatusRaw")) { 1L -> "sold" 2L -> "transferred" - else -> "departed" + 0L, null -> "not synced" + else -> "departed" }🤖 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 `@packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/storage/StorageModels.kt` around lines 84 - 92, Update the ownership label logic in the subtitle lambda to handle saleStatusRaw == 0L explicitly, returning "unknown" for uncategorised non-owned rows; retain the existing "sold", "transferred", and "departed" mappings for their respective values.packages/rs-unified-sdk-jni/src/dpns_marketplace.rs (1)
419-500: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConfirm the signed-to-unsigned price contract at the JNI boundary.
setPriceandpurchasecastprice as u64without a range check, whilesyncSetIntervalrejects negative values throughnonnegative_u64. The Kotlin wrapper passesULong.toLong(), so the bit-preserving cast is required there. A direct Java caller that passes-1silently requestsu64::MAXcredits. Document this bit-pattern contract in the two entry points so the asymmetry withnonnegative_u64is intentional and visible.🤖 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 `@packages/rs-unified-sdk-jni/src/dpns_marketplace.rs` around lines 419 - 500, Document the signed-to-unsigned bit-pattern contract at the JNI entry points Java_org_dashfoundation_dashsdk_ffi_DpnsMarketplaceNative_setPrice and Java_org_dashfoundation_dashsdk_ffi_DpnsMarketplaceNative_purchase, explicitly noting that price as u64 preserves Kotlin ULong.toLong() values and that negative Java inputs map to their corresponding u64 bit pattern. Keep the existing casts unchanged and make the intentional difference from syncSetInterval’s nonnegative_u64 validation visible.packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/dpns/DpnsMarketplace.kt (1)
10-20: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winOverride
equalsandhashCodeon the data classes that holdByteArrayfields.
DpnsMarketplaceName,DpnsNameState,DpnsNameHistoryEvent,DpnsNameAdded,DpnsNameDeparted, andDpnsPriceChangeall carryByteArrayproperties. The generatedequalscompares arrays by reference, so two decodes of the same row are unequal. This breaks set membership,distinctUntilChanged, and Compose recomposition keys. Other SDK models in this package, such asWalletSyncEvent, already override both methods usingcontentEqualsandcontentHashCode.Also applies to: 32-44, 56-78
🤖 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 `@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/dpns/DpnsMarketplace.kt` around lines 10 - 20, Override equals and hashCode for DpnsMarketplaceName, DpnsNameState, DpnsNameHistoryEvent, DpnsNameAdded, DpnsNameDeparted, and DpnsPriceChange so every ByteArray property is compared with contentEquals and hashed with contentHashCode, while other properties retain value-based comparison. Follow the existing WalletSyncEvent implementation pattern to ensure independently decoded equivalent models compare and hash identically.packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt (1)
482-511: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDocument the reserved marketplace codes in the Kotlin mapping.
Rust reserves codes 37–40 for these marketplace errors and pins them with a test. Add a short comment above this block that references
packages/rs-platform-wallet-ffi/src/error.rs.🤖 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 `@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt` around lines 482 - 511, The marketplace error mappings for codes 37–40 in the surrounding error-mapping function need documentation. Add a short comment immediately above the code 37 branch referencing packages/rs-platform-wallet-ffi/src/error.rs and noting that these codes are reserved for marketplace errors; leave the mappings unchanged.packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt (1)
763-810: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for removal on an owned label row.
This test removes the DPNS name state for a row whose identity snapshot already dropped the label, so full row deletion is correct. The Swift equivalent,
testMarketplaceRemovalClearsOnlyMarketplaceColumnsinpackages/swift-sdk/SwiftTests/SwiftDashSDKTests/DpnsMarketplacePersistenceTests.swift, covers the other case: removal on a still-owned row must clear only the marketplace columns and keep the label,acquiredAt, andisOwned.Android has no equivalent test. A regression that deletes the label cache on an owned row would pass this suite.
Also consider asserting
retained.priceCreditsis null, since the callback passedhasPrice = false.🤖 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 `@packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt` around lines 763 - 810, Add a second removal scenario to marketplaceStateRetainsDepartedNameAndCanRemoveIt, or a dedicated test, where the DPNS row remains owned with its label and acquiredAt populated; after onRemoveDpnsNameState, assert the row remains and only marketplace fields are cleared while label, acquiredAt, and isOwned are preserved. Also assert the existing retained row’s priceCredits is null when hasPrice is false.
🤖 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
`@packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/DpnsMarketplaceScreen.kt`:
- Around line 117-132: Capture the current manager and wallet into non-null
local values before invoking launch in the refresh action, then use those locals
inside the coroutine instead of manager!! and wallet!!. Apply the same
pre-launch capture pattern to the search action and history action, preserving
their existing operations and messages.
- Line 271: Update the result Card testTag in DpnsMarketplaceScreen to use the
dpnsMarketplace.search.${row.normalizedLabel} identifier, while leaving the
existing dpnsMarketplace.owned tag unchanged.
In
`@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt`:
- Around line 1169-1172: Update onRemoveDpnsNameState to match the Swift removal
semantics: preserve the dpns_names label-cache row and clear only the
marketplace-related columns instead of deleting the entire record. Reuse the
existing DAO update operation or add the corresponding targeted persistence
method, while keeping the guarded staging flow intact.
- Around line 1116-1167: Update onPersistDpnsNameState to check whether
walletIdentityId exists before staging and upserting the DpnsNameEntity. If the
identity is absent, skip the marketplace-row persistence and return success
without executing the upsert; otherwise preserve the existing flow.
In `@packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs`:
- Around line 1591-1633: Update classify_departure to return a result that
distinguishes successful classification with no matching event from
fetch_history_documents failure, preserving the failure state when either
history lookup fails. In resolve_departed_name, handle that failure outcome by
retaining the marketplace row and requesting retry, while keeping the existing
removal behavior only for a successful classification with no direct departure
event.
In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/SwiftExampleAppApp.swift`:
- Line 246: Separate the coordinator calls in SwiftExampleAppApp from the shared
do/catch by giving each DPNS and sync operation its own best-effort error
boundary, or route them through a helper that logs failures and continues.
Ensure a failure in an earlier stop, sync check, or start call does not prevent
stopDpnsSync at the wallet-removal path or the DPNS startup operations at the
wallet-load path. Add regression coverage for both continuation scenarios.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/IdentityDetailView.swift`:
- Around line 237-246: Update the marketplace NavigationLink condition in
IdentityDetailView so it requires both !identity.isLocal and
hasLoadedWallet(for: identity), matching the gating used by the Top Up,
Transfer, and Withdraw actions. Keep the surrounding DPNS section visibility
logic unchanged.
---
Nitpick comments:
In
`@packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/storage/StorageModels.kt`:
- Around line 84-92: Update the ownership label logic in the subtitle lambda to
handle saleStatusRaw == 0L explicitly, returning "unknown" for uncategorised
non-owned rows; retain the existing "sold", "transferred", and "departed"
mappings for their respective values.
In
`@packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/persistence/DashDatabaseMigrationTest.kt`:
- Around line 378-392: Extend the query in DashDatabaseMigrationTest to include
the legacy DPNS columns label, normalizedLabel, parentDomainName,
normalizedParentDomainName, acquiredAt, and identityId, then assert their
inserted values along with the existing marketplace-column checks. Also verify
the expected row identity, preserving the v9-to-v10 data-preservation coverage.
In
`@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/dpns/DpnsMarketplace.kt`:
- Around line 10-20: Override equals and hashCode for DpnsMarketplaceName,
DpnsNameState, DpnsNameHistoryEvent, DpnsNameAdded, DpnsNameDeparted, and
DpnsPriceChange so every ByteArray property is compared with contentEquals and
hashed with contentHashCode, while other properties retain value-based
comparison. Follow the existing WalletSyncEvent implementation pattern to ensure
independently decoded equivalent models compare and hash identically.
In
`@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt`:
- Around line 482-511: The marketplace error mappings for codes 37–40 in the
surrounding error-mapping function need documentation. Add a short comment
immediately above the code 37 branch referencing
packages/rs-platform-wallet-ffi/src/error.rs and noting that these codes are
reserved for marketplace errors; leave the mappings unchanged.
In
`@packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt`:
- Around line 763-810: Add a second removal scenario to
marketplaceStateRetainsDepartedNameAndCanRemoveIt, or a dedicated test, where
the DPNS row remains owned with its label and acquiredAt populated; after
onRemoveDpnsNameState, assert the row remains and only marketplace fields are
cleared while label, acquiredAt, and isOwned are preserved. Also assert the
existing retained row’s priceCredits is null when hasPrice is false.
In `@packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs`:
- Line 1365: Update the departure-processing flow around the pending_departures
check and the retryable-failure handling near the departure loop so one
repeatedly failing item cannot prevent the ownership scan from running. Rotate
failed departures behind other queued items or enforce an attempt limit, while
preserving retries and allowing the ownership/price refresh path to execute even
when a failure remains queued.
In `@packages/rs-unified-sdk-jni/src/dpns_marketplace.rs`:
- Around line 419-500: Document the signed-to-unsigned bit-pattern contract at
the JNI entry points
Java_org_dashfoundation_dashsdk_ffi_DpnsMarketplaceNative_setPrice and
Java_org_dashfoundation_dashsdk_ffi_DpnsMarketplaceNative_purchase, explicitly
noting that price as u64 preserves Kotlin ULong.toLong() values and that
negative Java inputs map to their corresponding u64 bit pattern. Keep the
existing casts unchanged and make the intentional difference from
syncSetInterval’s nonnegative_u64 validation visible.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/DpnsMarketplacePresentationTests.swift`:
- Around line 25-52: Extend testTypedTradeErrorsProduceActionableMessages to
construct PlatformWalletError.notForSale with no associated value, pass it to
DpnsMarketplaceUI.error(_:), and assert the result contains the expected
not-for-sale mapping text. Preserve the existing assertions for the other typed
marketplace errors.
In
`@packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DpnsMarketplaceDecodingTests.swift`:
- Around line 251-257: Replace the bounded Task.yield loop in the test with an
explicit XCTestExpectation or deadline-based polling wait for
manager.lastDpnsSyncEvent. Fail explicitly with XCTFail when the event is not
published before the timeout, and mark the test async throws if using XCTUnwrap;
keep the existing event assertions after successful retrieval.
- Around line 72-88: Add a test case alongside the existing DpnsNameDeparture
decoding coverage that constructs DpnsNameDepartedFFI with has_status = false,
then assert DpnsNameDeparture.status is nil while identityId and label still
decode correctly. Reuse the existing departure fixture/setup patterns and keep
the sold-status case unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 41df46bc-537c-4eb6-a49e-0340d198a74f
📒 Files selected for processing (55)
packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/di/AppContainer.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/navigation/AppNavHost.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/navigation/Routes.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/DpnsMarketplaceScreen.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/IdentityDetailScreen.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/storage/StorageModels.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/storage/StorageRecordDetailScreen.ktpackages/kotlin-sdk/sdk/schemas/org.dashfoundation.dashsdk.persistence.DashDatabase/10.jsonpackages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/persistence/DashDatabaseMigrationTest.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/dpns/DpnsMarketplace.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/DpnsMarketplaceNative.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/NativePersistenceBridge.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/NativeWalletEventBridge.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/DashDatabase.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/dao/DpnsNameDao.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/entities/DpnsNameEntity.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/WalletSyncEvent.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/dpns/DpnsMarketplaceTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DpnsMarketplaceErrorTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/WalletEventFanOutTest.ktpackages/rs-platform-wallet-ffi/src/dpns_marketplace.rspackages/rs-platform-wallet-ffi/src/event_handler.rspackages/rs-platform-wallet-ffi/src/manager.rspackages/rs-platform-wallet/examples/dpns_marketplace_testnet.rspackages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rspackages/rs-platform-wallet/src/wallet/identity/network/identity_handle.rspackages/rs-platform-wallet/src/wallet/identity/network/payments.rspackages/rs-platform-wallet/src/wallet/identity/state/manager/accessors.rspackages/rs-platform-wallet/src/wallet/identity/state/manager/mod.rspackages/rs-platform-wallet/src/wallet/platform_wallet.rspackages/rs-unified-sdk-jni/src/dpns_marketplace.rspackages/rs-unified-sdk-jni/src/events.rspackages/rs-unified-sdk-jni/src/lib.rspackages/rs-unified-sdk-jni/src/persistence.rspackages/rs-unified-sdk-jni/src/wallet_manager.rspackages/swift-sdk/Sources/SwiftDashSDK/Persistence/DashModelContainer.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDPNSName.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/DpnsMarketplace.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerAddressSync.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerDpnsSync.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/SwiftExampleAppApp.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DpnsMarketplaceView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/IdentityDetailView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageModelListViews.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/DpnsMarketplacePresentationTests.swiftpackages/swift-sdk/SwiftTests/SwiftDashSDKTests/DpnsMarketplaceDecodingTests.swiftpackages/swift-sdk/SwiftTests/SwiftDashSDKTests/DpnsMarketplacePersistenceTests.swift
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/rs-platform-wallet/src/wallet/platform_wallet.rs
- packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift
- packages/swift-sdk/Sources/SwiftDashSDK/Persistence/DashModelContainer.swift
- packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DpnsMarketplacePersistenceTests.swift
- packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDPNSName.swift
- packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift
- packages/rs-platform-wallet/examples/dpns_marketplace_testnet.rs
- packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift
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
2 participants
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.
What this is
Wallet-level orchestration for the DPNS username marketplace — the durable layer underneath the dashwallet-ios marketplace UI (which currently composes the generic
setDocumentPrice/purchaseDocument/transferDocumentprimitives directly), plus the pieces the app cannot do at all today (per-name trade history, typed trade errors, sale-state persistence). Design record + investigation results:packages/rs-platform-wallet/docs/DPNS_MARKETPLACE.md.Wallet layer (
rs-platform-wallet)dpns_marketplace.rs:search_dpns_names_with_state(cursor-paginated, keeps$id/$pricewhich the SDK'sDpnsUsernamedrops),dpns_name_state,set_dpns_name_price,delist_dpns_name(transfer-to-self — consensus clears$price; the confirmed document is verified),transfer_dpns_name,purchase_dpns_name,dpns_name_history,sync_dpns_marketplace. Signing keys are auto-selected (AUTHENTICATION/ECDSA at the doc type's required security level) — no hardcoded key ids.DpnsNameNotFound,DocumentNotForSale,DocumentPriceChanged{expected,actual},InsufficientIdentityCredits{required,available},ContestedNameNotTradable— raised pre-flight and recovered from consensus rejections (40108/40109/IdentityInsufficientBalanceErrordowncasts, wired into the generic trade paths too). A purchase always broadcasts the user-confirmed price; the consensus equality check is the backstop and a lost race surfaces as typedDocumentPriceChanged.ContestedNameNotTradablebefore broadcast.DpnsNameStateEntrystore (changeset +DPNS_NAME_STATEScapability bit + SQLiteV005+ FFI mirror) rather than mutating the bincode-positionalIdentityEntry. The legacydpns_nameslabel-list merge switches append-only → LWW wholesale (same policy ascontested_dpns_names) so sold names can leave an identity;set_dpns_names/remove_dpns_namemutations added.DpnsSyncManager: 60s coordinator (full quiesce/shutdown parity with siblings) that refreshes sale state, detects acquisitions and departures (sold-vs-transferred classified through the history contract), removes departed labels, and refreshes seller balances. Completion event onPlatformEventHandler.keeps*Historyflags writetransfer/purchase/priceUpdatedocuments into the Document History system contract (byDocumentindex) — per-name timelines are three indexed queries, merged and$createdAt-ordered. (ThegetDocumentHistoryendpoint serves the GroveDBdocumentsKeepHistorymechanism, which DPNS does not use — returns empty; documented.)FFI + swift-sdk
priceChanged(documentId:expected:actual:)etc.).platform_wallet_dpns_*entry points for all ops +DpnsSyncManagerstart/stop/sync-now; pointer-only repr(C) rows with paired destructors.on_persist_dpns_name_states_fnpersister vtable slot (ABI-additive) mirroring rows intoPersistentDPNSName, which grows marketplace columns (documentId, price, sale status, counterparty) — meaningful only whiledocumentIdBase58 != nil.ManagedPlatformWalletmethods matching the app's seven conceptual marketplace operations one-for-one, so the app swaps over without UI changes.Browse-for-sale investigation (doc §7)
A global "names for sale ordered by price" needs a
$priceindex, which is not buildable today at any layer:$priceis not in rs-dpp's indexable system-property set (contract parse fails withUndefinedIndexPropertyError), index definitions are immutable onDataContractUpdatefor all contracts, and the DPNS owner id ([0;32]) is unsignable. Only a protocol-version upgrade can add it (the$creatorIdfeature-gate precedent; new index trees start empty — no backfill machinery). The exact PV15 path is documented; until then the marketplace is search-driven.Testnet verification (doc §9)
examples/dpns_marketplace_testnet.rsran the full flow against real testnet (two identities of one wallet): register → list 1M credits → re-price 2M → typed stale-price rejection → purchase (ownership flip,$pricecleared,records.identityrewritten by the protocol) → history shows Registered + PriceSet×2 + Purchased with prices/parties/block heights → typed not-for-sale → delist-via-self-transfer clears$priceon-chain. All checks passed. Verification exposed and fixed a real bug:Sdk::register_dpns_namenever registers the DPNS contract with the context provider, so hosts that don't pre-seed known contracts failed post-broadcast proof verification — registration now self-heals, and the marketplace contract caches hold the on-chain-fetched, provider-registered contract.Tests
cargo test -p platform-wallet— 752 passedcargo test -p platform-wallet-ffi— 290 passedcargo test -p platform-wallet-storage— new round-trip coverage for the V005 tablebuild_ios.sh --target mac+swift build— clean🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes