Repository navigation
feat(swift-sdk): expose when each DPNS contender submitted its request - #5718
Conversation
Each contender of a contested DPNS name carries its serialized domain document, and the FFI already decoded it for the requested label. It now also returns the document's $createdAt: DashSDKContender gains created_at (ms) and has_created_at at its end, the vote-state JSON gains an optional "created_at" key, and DPNSContender gains createdAt: Date?. The two list queries share one contest_info_to_ffi, and both readers use one contender_document_fields decode. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Decode and serialize contender data packages/rs-sdk/src/platform/dpns_usernames/*, packages/rs-sdk-ffi/src/contested_resource/*, packages/rs-sdk-ffi/src/types.rs |
The shared decoder reads non-empty labels and creation timestamps, preserving present zero values. The FFI represents timestamp presence with a flag and serializes available fields into vote-state JSON. |
DPNS contest query conversion packages/rs-sdk-ffi/src/dpns/queries/contested.rs |
Both non-resolved contest queries use shared conversion for contender data and requested labels. The conversion preserves contest tallies, end time, and winner presence. |
Swift contender date conversion packages/swift-sdk/Sources/SwiftDashSDK/FFI/PlatformQueryExtensions.swift, packages/swift-sdk/Sources/SwiftDashSDK/Voting/*, packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DPNSContestDecoderTests.swift |
Swift query results and vote-state JSON convert valid timestamps to DPNSContender.createdAt. Tests cover missing and malformed values, present zero timestamps, and missing identity IDs. |
Estimated code review effort: 3 (Moderate) | ~25 minutes
Sequence Diagram(s)
sequenceDiagram
participant DPNSContestQuery
participant contest_info_to_ffi
participant ContenderRequest
participant DashSDKContender
participant PlatformQueryExtensions
participant DPNSContender
DPNSContestQuery->>contest_info_to_ffi: Convert contest data
contest_info_to_ffi->>ContenderRequest: Decode contender document fields
ContenderRequest-->>contest_info_to_ffi: Return label and optional creation timestamp
contest_info_to_ffi->>DashSDKContender: Set timestamp and presence flag
DashSDKContender-->>PlatformQueryExtensions: Return contender result
PlatformQueryExtensions->>DPNSContender: Convert present timestamp to createdAt
Possibly related PRs
- dashpay/platform#5337: Introduces the DPNS v3 domain contract used by the contender document decoding in this PR.
Suggested reviewers: quantumexplorer, zocolini
Merge Risk: 
083ad
-
Contender creation dates are exposed across the SDK paths. No merge-blocking issue was established.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title names the change: the Swift SDK now exposes when each DPNS contender submitted its request. The changeset matches this. It adds $createdAtto the FFI contender, the vote-state JSON, and `D…Docstring Coverage Docstring coverage is 93.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentati… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title names the change: the Swift SDK now exposes when each DPNS contender submitted its request. The changeset matches this. It adds
$createdAtto the FFI contender, the vote-state JSON, andDPNSContender.createdAt: Date?in Swift. The title names one scope (swift-sdk) and the main user-facing effect, but the change also spans rs-sdk and rs-sdk-ffi. That is expected for a title.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 93.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 15 functions across 11 files.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
|
Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
the
waiting-bots
|
✅ Final review complete — no blockers (commit 083ad6d) · triage: normal |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
Verified the supplied findings against head 331c745 using static source inspection. The timestamp plumbing and allocation ownership are sound, but the refactored FFI converter retains display-policy decisions that the repository explicitly assigns to a native Rust library; the new timestamp filter also changes native availability semantics. Current Rust, Swift, and Kotlin checks pass; the PR Hygiene error is a policy-check failure rather than evidence of a code defect.
🔴 1 blocking | 🟡 1 suggestion(s)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
normalbygpt-6.1-sol(effort low) — This is a cross-language behavioral change involving shared Rust decoding, contender conversion refactoring, C struct layout, and Swift date handling, but it does not modify a critical surface. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity quota lookup failed),glm-5.3-flash(not used above high effort; tier asks max) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort high); agentphase2-reviewer
🤖 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-sdk-ffi/src/dpns/queries/contested.rs`:
- [BLOCKING] packages/rs-sdk-ffi/src/dpns/queries/contested.rs:663-670: Move requested-label display policy into the Rust SDK
The shared converter derives the ordered, de-duplicated set of spellings callers should display instead of merely marshalling an SDK result. Its own documentation identifies this as display policy. The book's "FFI and mobile SDKs hold no business logic" rule and packages/swift-sdk/CLAUDE.md require these decisions to live in a native Rust library, with FFI limited to calling and marshalling. The existing SDK ContestInfo exposes contenders and end_time, but no requested-label result. Although the two previous blocks already contained this policy, this PR moves and rewrites those blocks into the new helper, so the ownership violation is within scope. Add the requested-label derivation to an SDK-owned DPNS helper or result alongside ContestInfo, and have this converter allocate C strings from that result without selecting or de-duplicating labels.
In `packages/rs-sdk-ffi/src/contested_resource/contender_document.rs`:
- [SUGGESTION] packages/rs-sdk-ffi/src/contested_resource/contender_document.rs:99: Preserve the native timestamp's availability semantics
This filter changes a present native timestamp of Some(0) into unavailable metadata. DPP's document serializers and deserializers distinguish presence through timestamp flags and preserve a present zero, while DocumentV0Getters::created_at() returns that option unchanged. The new C field already has an explicit presence flag, and the JSON field can express absence by omission, so neither representation needs a zero sentinel. Marshal the native option unchanged and let the Swift timestamp reader preserve the same distinction. If suppressing zero is an intentional DPNS interpretation, define that rule once in the native SDK rather than in this FFI helper, which also serves the generic contested-resource query, and independently in Swift. This is a verified availability-semantics mismatch, not a claim that live DPNS contenders normally contain epoch-zero dates.
|
Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix; thepastaclaw left review threads unresolved; resolve them. |
github-actions
Bot
added
waiting-self-review
The requester's own spelling, the `$createdAt` of each contender's document and the ordered, de-duplicated list of requested labels were derived in rs-sdk-ffi. They now come from `dpns_usernames::ContenderRequest` and `ContestRequests` in rs-sdk; the list queries and the vote-state query only marshal the results into their C and JSON shapes. A present `$createdAt` of 0 is passed through as the document holds it instead of being treated as absent, in the C contender (`has_created_at`), the vote-state JSON and the Swift readers. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Your move: thepastaclaw left review threads unresolved; resolve them. |
|
Your move: thepastaclaw left review threads unresolved; resolve them. |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Verified the complete diff at head 083ad6d: both prior findings are fixed, and no new actionable in-scope defects were identified. Request decoding and ordered label de-duplication now live in rs-sdk, while C, JSON, and Swift preserve timestamp presence, including a present zero. This was a static review without builds or test execution; current CI shows the JS build and Swift frozen-schema check passing, with Rust workspace tests, Kotlin build/tests, and JavaScript tests still pending.
🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
normalbygpt-6.1-sol(effort low) — The change spans Rust SDK document decoding, FFI representations, and Swift date handling with meaningful refactoring and tests, but does not change a critical surface sufficiently to qualify as critical. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity quota lookup failed),glm-5.3-flash(not used above high effort; tier asks max) - Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort high); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.
No unresolved findings remain from the prior review on this head.
|
Your move: thepastaclaw left review threads unresolved; resolve them. |
|
/self-reviewed |
|
Missing human approval. Automatic reviewer requests are paused and this PR is excluded from reviewers' queues. Merge, close, or draft another active PR by this author to free a review slot. Required human review can be satisfied without a slot; once all merge requirements are met, PR Hygiene passes. |
github-actions
Bot
added
too-many-open-prs
lklimek
left a comment
lklimek
left a comment
There was a problem hiding this comment.
Reviewed rs-sdk-ffi, approved with minor comment.
|
Bots are done — your move: address lklimek left a review thread unresolved, then post |
github-actions
Bot
added
waiting-self-review
|
/self-reviewed |
|
Policy satisfied — this can merge. |
github-actions
Bot
removed
the
waiting-self-review
Brings #5636, #5681, #5715, #5718, #5747 and #5748. Conflicts: - book/src/sdk/protocol-version-gate.md: #5636 rewrote the page and moved the decision rules to protocol-versions.md. The merge takes #5636's page and keeps #5615's getIdentityByNonUniquePublicKeyHash row, whose declaration in rs-dapi-client is still there. #5615's paragraph about other protocol 14 additions goes: #5636's getDocuments row now lists the document features nodes at 13 refuse. - rs-dapi-client/tests/request_inventory.rs: #5636 makes document requests declare nothing and #5615 adds the V1 non-unique lookup's Gate(14). The spelled-out test keeps both: it is now should_need_protocol_14_only_for_token_pool_and_non_unique_v1_requests. - rs-sdk/src/sdk/protocol_gate_tests.rs: imports. Both sides' imports are kept, without SdkInstance and MockDapiClient, which #5755 replaced with Sdk::mock_dapi. Semantic: #5755 removed the file's dapi() helper, and two tests #5636 added call it (fetch_fee_pots_together and should_let_waiting_calls_ask_when_the_asking_call_is_dropped). They now use sdk.mock_dapi(). Co-Authored-By: Claude Opus 5.5 <[email protected]>
Basic explanation
What this does: When a username is contested, each competing request comes with the DPNS document it submitted. The FFI already decoded that document to read the requested spelling, then threw the rest away. Now it also returns the document's creation time (
$createdAt), so the Swift SDK can tell callers when each request was made.Value: Wallets can order contested requests by date, show each request's date, and work out which one was submitted first. Dash Wallet iOS needs this for its username-voting redesign, where "Vote for all" approves the first-submitted request under each name. Android already does this through dashj.
Risks: Low. It is read-only and consensus is untouched. It adds no network call: the date comes from the same decode as the label. The C struct gains two fields at its end and no new memory to free. The vote-state JSON gains a key that existing parsers (Swift, and Kotlin's lenient one) ignore or read.
Issue being fixed or feature implemented
$createdAtis required on every DPNSdomaindocument (contract v1–v3), and every contender of a contested name carries its serialized document, but neither contender reader inrs-sdk-ffipassed the date on. SoDPNSContenderin the Swift SDK had no way to say when a request was made. That date is what a voting UI needs to sort by date, show per-request dates, find the first-submitted request, and derive when the poll opened.What was done?
rs-sdk-ffi/src/contested_resource/contender_document.rs(new):contender_document_fields()decodes a contender's serialized document once and returnslabelandcreated_at_ms. It is best-effort: with no document type, no document, or a failed decode, both fields areNoneand the contender is still returned. Both readers now use it instead of their own inline label decode (previously written out twice incontested.rsand once invote_state.rs).DashSDKContender(types.rs) gainscreated_at: u64(ms since epoch) andhas_created_at: bool, appended afterlabel. A flag rather than a 0 sentinel, followingDashSDKContestInfo.has_winner. The fields are plain values, so the free functions are unchanged.dpns/queries/contested.rs: both list queries (dash_sdk_dpns_get_non_resolved_contests_for_identity,dash_sdk_dpns_get_contested_non_resolved_usernames) now share onecontest_info_to_ffiinstead of two hand-copied ~80-line blocks, and fill the new fields; requested labels are collected from the decoded label rather than read back out of the C string.contested_resource/queries/vote_state.rs: each contender object gains"created_at": <ms>. Likelabel, the key is omitted when the document did not decode.DPNSContender.createdAt: Date?. The new init parameter defaults tonil, so existing callers compile unchanged.SDK.contender(from:), which can be tested without a live list.decodeVoteStatereadscreated_at.nil.rs-unified-sdk-jnipasses the vote-state JSON through as a string, andKotlinExampleAppparses it by key, so the new key is ignored there. Exposing the date in a Kotlin model is left to the Kotlin SDK's parity work.How Has This Been Tested?
rs-sdk-ffi/src/contested_resource/contender_document.rs: real DPNSdomaindocuments serialized throughContender::try_into_contender_with_serialized_documentagainst the baked-in system contract — label and date read; no$createdAtand a zero one read as absent; undecodable bytes, no document and no document type give no fields; the C contender and the JSON members built from decoded fields.rs-sdk-ffi/src/dpns/queries/contested.rs:contest_info_to_fficarries each contender's label and date, handles an undecodable contender, lists requested labels once in contender order, and frees throughdash_sdk_contest_info_free.DPNSContestDecoderTests:created_atfrom ms, absent, 0, a string, a negative and a boolean; the list path'scontender(from:)with the flag on and off; a contender without an id is dropped.CARGO_INCREMENTAL=0 cargo test -p rs-sdk-ffi --all-features --lib: 404 passed.cargo clippy -p rs-sdk-ffi --all-targets --all-features -- -D warningsandcargo fmt --check: clean../build_ios.sh --target sim, thenxcodebuild test -scheme SwiftDashSDK -only-testing:SwiftDashSDKTests/DPNSContestDecoderTestson an iPhone 17 / iOS 26.5 simulator: 29 passed.Breaking Changes
None. The C struct grows at its end, and its header is generated with the framework in the same build. The Swift initializer gains a defaulted parameter. The JSON gains an optional key.
Checklist:
structure.rs, regeneratedgrovedb-structure.json, and checked the structure viewer link posted on this pull requestFor repository code-owners and collaborators only
🤖 Generated with Claude Code
PR Hygiene ·
083ad6drust-sdk-ffi(packages/rs-sdk-ffi/src/contested_resource/contender_document.rs,packages/rs-sdk-ffi/src/contested_resource/mod.rs,packages/rs-sdk-ffi/src/contested_resource/queries/vote_state.rsand 2 more) — approved by lklimekrust-sdk(packages/rs-sdk/src/platform/dpns_usernames/contender_request.rs,packages/rs-sdk/src/platform/dpns_usernames/mod.rs) — approved by lklimekswift-sdk— you own itWhen every merge requirement is met, the
PR Hygienecheck passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.Summary by CodeRabbit