Repository navigation
fix(sdk): read contested DPNS names as the bare labels Drive returns - #5482
Conversation
`get_contested_dpns_normalized_usernames` queries the `parentNameAndLabel`
contested index with the parent domain fixed to "dash". Drive answers each
contest with the value of the next index property alone, `Text(label)`, but
the helper only accepted a `[parent, label]` array, so it dropped every name
and returned an empty list (0 of 1,033 on testnet).
`start_after` had the same mismatch: it sent `[Text("dash"), Text(name)]`
as the start value, which Drive refuses as the wrong type for the label
property. A `limit` over 100 was also refused, since Drive answers at most
100 contests per query.
The helper now reads text labels, starts after a text label, and pages 100
at a time until `limit` names are read or none are left (`None` reads every
name). A value that is not text is an error instead of a silently dropped
name. `get_contested_dpns_usernames_by_identity`, which now sees every
contested name, reads their vote states 8 at a time, as
`get_contested_non_resolved_usernames` already does.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 82.61% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 1 files. |
| 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. |
| Description Check | |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly identifies the main change: the SDK now reads contested DPNS names as the bare labels returned by Drive. It is concise and specific. |
✨ 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.
Comment @coderabbitai help to get the list of available commands.
|
Waiting for bot review — coderabbitai ✓ · 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 5a8a547) · triage: normal |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
The text-label decoding, exclusive cursor, and bounded name pagination match Drive's implementation. One blocking issue remains in the reactivated identity lookup: a valid large contest can fail proof verification and abort the entire lookup because contender requests omit their count and are not paginated. This was a static review; current CI shows passing Rust workspace tests, an unrelated Swift simulator test-runner launch failure, and pending PR Hygiene.
🔴 1 blocking
Review provenance
Source: reviewer 1: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 2: gemini-3.8-flash-high (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: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
normalbygpt-6.1-sol(effort low) — The contained SDK change adds pagination, bounded concurrent lookups, and stricter response handling, requiring ordinary logic review without changing a critical surface. - Phase 1 reviewers:
gemini-3.8-flash-high— general (completed, effort high); agentphase1-reviewer,gemini-3.8-flash-high— rust-quality (completed, effort high); agentphase1-reviewer - Phase 1 model:
gemini-3.8-flash-high— antigravity quota: weekly 71% left, 5h 100% left - 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— 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/src/platform/dpns_usernames/contested_queries.rs`:
- [BLOCKING] packages/rs-sdk/src/platform/dpns_usernames/contested_queries.rs:263-267: Page contender states before filtering contested names by identity
This loop now receives real contested names, but `get_contested_dpns_vote_state(label, None)` sends an omitted contender count. The server's `query_contested_resource_vote_state_v0` replaces that omission with its default of 100, while `ContestedDocumentVotePollDriveQuery::try_from_request` reconstructs the proof-verification query with `limit: None`. The contender `FetchMany` implementation does not normalize the default. For a sufficiently large contest, such as one with 250 contenders, the server therefore produces a bounded proof for an unbounded verification query. The pinned GroveDB verifier rejects the missing requested contender subtrees, and `vote_state?` aborts the entire identity lookup—even if the searched identity only participates in another contest. Such contests are valid; the current system limit is 1,000 contenders.
The PR reactivates this path and expands the unlimited lookup to every contested name, so its correctness depends on handling these valid contests. Fetch contender states with explicit page counts and exclusive contender-ID cursors, accumulating all contenders before filtering by identity and constructing the returned list. `platform/documents/contest_fund.rs` already demonstrates this pagination pattern. Add coverage that exercises proof verification for a contest exceeding 100 contenders; the new small, already-decoded mock responses bypass the count/proof mismatch.
|
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
… by identity lookup `get_contested_dpns_usernames_by_identity` read each contest with `get_contested_dpns_vote_state(label, None)`: one request with no count. Drive answers that with its default page of 100 contenders and proves that page, so for a contest of more than 100 contenders (a contest takes up to 1,000) the identity could sit past the page, and the proof was checked against no limit and refused, failing the whole lookup. The lookup now reads every contender of each contest in pages of 100, each request naming its count and starting after the last contender read (exclusive), as `contest_fund_to_join` already does, and checks the identity against all of them. It reads vote tallies only, since it needs the contender identities and not their documents. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Waiting for bot review — coderabbitai ✓ · thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
waiting-bots
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 5a8a547 and found no actionable in-scope defects. The text labels and cursors match Drive's index query, and explicit contender pagination resolves the prior blocker without bypassing proof verification. Validation was static only; current CI has no failed code checks, with Rust workspace tests and Evo SDK tests still pending.
🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)
Review provenance
Source: reviewer 1: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 2: gemini-3.8-flash-high (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: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 8: 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 SDK change introduces pagination, cursor handling, response validation, and bounded concurrent lookups with substantial tests, but does not change a critical surface. - Phase 1 reviewers:
gemini-3.8-flash-high— general (completed, effort high); agentphase1-reviewer,gemini-3.8-flash-high— rust-quality (completed, effort high); agentphase1-reviewer - Phase 1 model:
gemini-3.8-flash-high— antigravity quota: weekly 71% left, 5h 100% left - 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— 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— 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.
|
Bots are done — your move: post |
github-actions
Bot
added
waiting-self-review
2 participants
Basic explanation
What this does: The Rust SDK has a helper that lists every DPNS name currently under a vote (a "contested" username, where several identities asked for the same name and masternodes decide who gets it). Platform answers that question correctly, but the helper expected each answer in the wrong shape, threw every one away, and returned an empty list. This PR reads the answers in the shape Platform actually sends, fixes the "continue after this name" option the same way, and lets the helper fetch more than 100 names by asking in pages of 100.
Value: Apps built on the Rust SDK or the C FFI see the real list again. On testnet on 2026-10-17 the helper returns all 1,033 contested names instead of 0. Paging with
start_afterand alimitabove 100 now work too; before, both failed with an error from Platform.Risks: Low. Only the client SDK changes, with no Platform, consensus or protocol change. Two behaviours change that callers could notice: with no
limitthe helper now reads every contested name, in several requests (11 on testnet) instead of one; andget_contested_dpns_usernames_by_identitywith nolimitnow reads the contenders of every contested name (at least one query per name, 1,033 on testnet, 8 names at a time), so it takes noticeably longer. Before this PR it checked none, because the list was empty.Issue being fixed or feature implemented
Sdk::get_contested_dpns_normalized_usernamesreturns an empty list on a live network. On testnet (Drive 5.0.0-rc.4, protocol version 14), it returned 0 names while directContestedResource::fetch_manypages of the sameVotePollsByDocumentTypeQueryreturned 1,033 resources.The query fixes the parent domain in
start_index_values: [Text("dash")], so it ranges over the next property of theparentNameAndLabelindex,normalizedLabel. Drive answers each contest with that property's value alone (ResolvedVotePollsByDocumentTypeQuery::execute_no_proofand theContestedResourcesproof verifier both yield oneValueper contest), so the SDK getsText("000"),Text("0000"), and so on. The privateextract_label_from_contested_resourceonly accepted aValue::Arrayof at least two elements, so it dropped every resource.start_afterhad the same mismatch. Drive encodesstart_at_valueas a key of that same next property (serialize_value_for_vote_poll_key(normalizedLabel, value)), but the helper sentArray([Text("dash"), Text(name)]), which Drive refuses as the wrong type. Andlimitwas passed straight through as one request, so anything over 100 was refused (limit 250 out of bounds of [1, 100]).I found no open PR or issue for this (searched for "contested usernames", "extract_label", "contested dpns", "parentNameAndLabel", "get_contested_dpns_normalized_usernames", "get_contested_dpns_usernames_by_identity").
What was done?
In
packages/rs-sdk/src/platform/dpns_usernames/contested_queries.rs:extract_label_from_contested_resourcereadsValue::Text(label). Any other value is now anError::InvalidProvedResponseinstead of a silently dropped name, so a future mismatch between the query and the index shows up as an error rather than an empty list. The array form is gone: nothing produces it for this query, and no caller relied on it.start_afteris sent as(Value::Text(name), false), the next property's value, exclusive.get_contested_dpns_normalized_usernamespages: each request asks for at mostDEFAULT_QUERY_LIMIT(100, what Drive answers per query) and starts after the last label read, untillimitnames are read or a short page ends the list.limit: Nonereads every name. Every request now carries an explicit limit.get_contested_dpns_usernames_by_identityreads every contender of each name it checks, in pages of 100 that each name their count and start after the last contender read (exclusive), ascontest_fund_to_joinalready does. A contest holds up to 1,000 contenders, so an identity past the first 100 is now found, and each proof is checked against the page Drive proved (a request with no count was checked against no limit and refused for a contest of more than 100). It reads vote tallies only, since it needs the contender identities and not their documents. It checks names 8 at a time withbuffered, the same bounded fan-outget_contested_non_resolved_usernamesuses; results stay in name order, and the first failed query still fails the call. Itslimitstill bounds how many names it checks, as before.Affected callers: the C FFI exports
dash_sdk_dpns_get_all_contested_usernamesanddash_sdk_dpns_get_contested_usernames_by_identityinpackages/rs-sdk-ffi/src/dpns/queries/contested.rscall these two helpers and need no change (signatures are unchanged). No Swift or Kotlin wrapper in this repo calls those two exports today.packages/rs-sdk/tests/dpns_usernames.rsis left as is: it is#[ignore], needs a live network plus Core RPC, and only prints what it gets, so it passed with the empty list. The new unit tests cover the logic without a network.Before and after
The same calls against testnet on 2026-10-17 (DPNS
GWRSAVFMjXx8HpQFaNJMqBV7MBgMK4br5UESsB4S31Ec):How Has This Been Tested?
Unit tests (new,
contested_queries::tests, mock SDK, no network). Each page's expected query is built in the test rather than by the code under test, so a request shaped differently (an array cursor, a limit over 100) matches no expectation and fails:should_read_every_contested_name_once_across_pages: pages of 100, 100 and 50 give 250 names, each once, in order.should_stop_at_an_empty_page_after_a_full_oneshould_read_a_limit_over_100_in_pages_of_at_most_100:Some(150)asks for 100, then 50 after the 100th name.should_start_after_the_given_name:start_afteris sent asText(name), exclusive.should_extract_a_label_answered_as_textandshould_refuse_a_contested_resource_that_is_not_textshould_list_the_contested_names_an_identity_contends_for: the by-identity helper over three names, with the identity contending in two.should_find_an_identity_past_the_first_100_contenders: a contest of 160 contenders with the identity on the second page is found and lists all 160; a contest of exactly 100 ends at an empty second page.With the old array cursor put back, the four paging tests fail; with the fix they pass. With the contender count dropped, both by-identity tests fail; with contenders read from the first page only, the 160-contender test fails.
Live check against testnet (a temporary example using
TrustedHttpContextProvider, not committed), 2026-10-17:ContestedResource::fetch_manypages of 100 with a text cursorText("000")get_contested_dpns_normalized_usernames(None, None)(Some(100), start_after = last name)(Some(250), None)The same check run against the code before this PR gave
Ok(0 names), the type error and the limit error shown in the before/after above.Breaking Changes
None. No public signature changes. The helpers now return the names they were documented to return;
limit: Nonereads every name instead of sending one unlimited request.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 ·
5a8a547/self-reviewedrust-sdk(packages/rs-sdk/src/platform/dpns_usernames/contested_queries.rs) — lklimek or shumkovWhen 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