Skip to content

fix(contested-names): normalize DPNS label in vote poll construction + pre-flight existence check#835

Merged
lklimek merged 4 commits into
v1.0-devfrom
fix/contested-names-normalize-dpns-vote-poll
Apr 22, 2026
Merged

fix(contested-names): normalize DPNS label in vote poll construction + pre-flight existence check#835
lklimek merged 4 commits into
v1.0-devfrom
fix/contested-names-normalize-dpns-vote-poll

Conversation

@lklimek
Copy link
Copy Markdown
Contributor

@lklimek lklimek commented Apr 21, 2026

User story

Imagine you are a masternode operator using Dash Evo Tool. A new DPNS username contest opens — say alice — and you cast your Lock vote. Today, if the contested name contains any of i, l, or o (the overwhelming majority of real names), the app sits silently for about 70 seconds and then flashes an "An unexpected error occurred" banner with no actionable hint. Your vote never lands on Platform.

After this fix, the vote either succeeds on the first attempt or fails in well under a second with a clear, actionable message: "The contested name … is not currently open for voting. It may have been resolved or may not exist. Refresh the contested names list and try again."

Summary

  • Normalize the DPNS label via convert_to_homograph_safe_chars (maps i1, l1, o0, then lowercase) before building the vote poll's index_values. Platform stores contested polls under the normalized label; the old code used the raw label, so any name containing i/l/o silently mismatched the index.
  • Add a pre-flight existence check via ContestedResource::fetch_many before broadcasting any signed vote state transition. Short-circuits the DAPI retry chain (client-side timeout: 10s × retries: 6 × ~7 evonode attempts) that otherwise burns ~70 s on a poll that isn't there.
  • New typed error variant TaskError::VotePollNotFound { name } carrying an Everyday-User-persona message ("what happened + what to do"). Replaces the opaque timeout path.
  • Extract the index-values construction as a pure helper (dpns_vote_poll_index_values) with unit-test coverage pinning the normalization contract (alice → a11ce, bar22 → bar22).

Why

Drive-abci server logs from three separate PR 827 TC-090 runs (heights 318950, 319048, 319073) showed VotePollNotFoundError at PrepareProposal. The tx passed CheckTx because validates_full_state_on_check_tx() for MasternodeVote was only added to Platform in December 2025 (b44ccff5, v3.1.0-dev.1 onward). Testnet v3.0.x clients must therefore detect the missing poll themselves or pay the full retry timeout.

In the DET GUI, VoteOnDPNSNames happens to pass contested_name.normalized_contested_name — so the GUI worked. CLI, MCP, scheduled replay, and E2E tests passed raw labels, so they hit the bug. The fix is at the backend boundary so every caller is covered regardless of whether they pre-normalized.

Scope

  • src/backend_task/contested_names/vote_on_dpns_name.rs — normalization + pre-flight check + helper + unit tests.
  • src/backend_task/error.rsVotePollNotFound { name } variant.

Other ContestedDocumentResourceVotePoll construction sites were surveyed and left unchanged:

  • query_dpns_vote_contenders.rs — only called with labels fetched from Platform's index (already normalized).
  • query_ending_times.rs — only reads index_values from polls returned by Platform.

Relationship to PR #827

This fix was discovered during backend E2E test work on #827 (TC-090 masternode vote verification) and is being filed here as a standalone PR against v1.0-dev because the bug is orthogonal to the test-only scope of #827. #827 currently carries the same commit on its branch; on rebase after this merges, git will deduplicate it.

Test plan

  • cargo check --all-features --tests — clean
  • cargo clippy --all-features --all-targets -- -D warnings — clean (verified on source branch prior to cherry-pick)
  • cargo +nightly fmt --all — no changes
  • cargo test --all-features --workspace --lib --bins — 455 passed, 0 failed (includes two new unit tests: index_values_normalizes_homograph_chars, index_values_preserve_non_homograph_label)
  • TC-090 end-to-end against testnet with e0emnduubiwud (contains i) — PASS in 30.11 s, vote broadcast returned proof in 2.4 s, pre-flight confirmed the poll, VotePollNotFound never raised. (Previously: 3 consecutive failures at ~70 s each.)

🤖 Co-authored by Claudius the Magnificent AI Agent

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • DPNS voting now validates that an active vote poll exists before processing vote submissions.
    • Improved consistency in DPNS name normalization during voting operations.
    • Added clear error messaging when attempting to vote on a DPNS name with no active vote poll, instructing users to refresh the list and retry.

…+ pre-flight existence check

`vote_on_dpns_name` was constructing `ContestedDocumentResourceVotePoll`
with the raw DPNS label passed in by the caller. Platform indexes the
poll under the **normalized** label produced by
`convert_to_homograph_safe_chars` (`i`→`1`, `l`→`1`, `o`→`0`,
`to_lowercase`). Any label containing those characters — i.e. the common
case — produced a `(dash, <raw>)` lookup against an index keyed on
`(dash, <normalized>)`, triggering `VotePollNotFoundError` at
`PrepareProposal`. Client-side this surfaced as a ~70 s opaque
"An unexpected error occurred" timeout from the vote retry chain.

Confirmed against drive-abci logs at heights 319048, 319073, and 318950.

Changes:
* Normalize the label via `convert_to_homograph_safe_chars` before
  building `index_values`. Extracted as `dpns_vote_poll_index_values`,
  a pure helper with unit coverage.
* Add a pre-flight existence check that queries Platform for the vote
  poll before broadcasting any state transitions. Missing polls now
  return immediately instead of burning ~70 s on retries.
* New `TaskError::VotePollNotFound { name }` variant with a
  user-facing message (Everyday User persona, "what happened + what
  to do"): "The contested name \"{name}\" is not currently open for
  voting. It may have been resolved or may not exist. Refresh the
  contested names list and try again."
* Unit tests pin the normalization contract (`alice` → `a11ce`,
  `bar22` → `bar22`) so regressions in the helper fail in CI.

Other call-sites surveyed:
* `query_dpns_vote_contenders.rs` builds the same poll but is only
  called with labels already fetched from Platform (normalized), so
  is left unchanged per scope.
* `query_ending_times.rs` only **reads** `index_values` from polls
  returned by Platform — no construction, no normalization needed.

In the DET UI, `VoteOnDPNSNames` is dispatched with
`contested_name.normalized_contested_name`, so the user-facing path
happens to already supply a normalized string — but the backend must
be defensive regardless of caller (CLI, scheduled vote replay,
tests), hence the fix at the boundary.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 21, 2026

Warning

Rate limit exceeded

@lklimek has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 23 minutes and 11 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 23 minutes and 11 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6d1d6ddd-e902-43a6-acec-69e153e6355a

📥 Commits

Reviewing files that changed from the base of the PR and between 801cf07 and 1f50522.

📒 Files selected for processing (1)
  • src/backend_task/contested_names/vote_on_dpns_name.rs
📝 Walkthrough

Walkthrough

The DPNS voting process now applies homograph normalization to names before indexing and includes a pre-flight Platform query to verify an open vote poll exists before submitting votes. A new error variant communicates when the poll is not available.

Changes

Cohort / File(s) Summary
DPNS Vote Validation & Normalization
src/backend_task/contested_names/vote_on_dpns_name.rs
Added homograph-safe character normalization for DPNS names via convert_to_homograph_safe_chars. Implemented pre-flight VotePollsByDocumentTypeQuery to verify open vote poll exists before submission. Introduced helper dpns_vote_poll_index_values to standardize index values as ["dash", normalized_label]. Added unit tests for normalization behavior.
Error Handling
src/backend_task/error.rs
Added TaskError::VotePollNotFound { name: String } variant with user-facing error message directing users to refresh the contested names list and retry.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 A hop through the contested names so fine,
We normalize and verify, all in line,
Before each vote we cast with care,
Homographs balanced, fair and square!
The poll exists—now vote with cheer,

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and specifically describes the main changes: DPNS label normalization in vote poll construction and the addition of a pre-flight existence check, which are the core fixes implemented in this PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/contested-names-normalize-dpns-vote-poll

Warning

Review ran into problems

🔥 Problems

Timed out fetching pipeline failures after 30000ms


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes DPNS contested-name voting failures by ensuring vote polls are addressed using Platform’s homograph-safe normalized label, and by adding a fast pre-flight existence check to avoid long retry/timeout paths when the poll doesn’t exist.

Changes:

  • Normalize DPNS label via convert_to_homograph_safe_chars when building vote poll index_values.
  • Add a pre-flight ContestedResource::fetch_many existence check to fail fast with a typed error instead of timing out.
  • Introduce TaskError::VotePollNotFound { name } and add unit tests pinning the index-values normalization behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/backend_task/error.rs Adds a typed VotePollNotFound error variant with an actionable user-facing message.
src/backend_task/contested_names/vote_on_dpns_name.rs Normalizes vote poll index values, adds pre-flight poll existence check, and introduces unit tests for normalization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/backend_task/contested_names/vote_on_dpns_name.rs Outdated
…when/then

Address review feedback on #835: the `dpns_vote_poll_index_values` doc
comment and the pre-flight check inline comment were over-explanatory;
trimmed each to 2-3 lines with concrete examples. The two unit tests
now use inline Given/When/Then comments instead of a free-form doc
comment — easier to scan and matches project test-style preference.

No logic changes; assertions unchanged.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@thepastaclaw
Copy link
Copy Markdown
Collaborator

thepastaclaw commented Apr 21, 2026

⏳ Review in progress (commit 1f50522)

…re-flight comment

The ~70 s retry chain text is platform-version-bound and will age badly
once Platform's MasternodeVote picks up `validates_full_state_on_check_tx()`
(b44ccff5, v3.1.0-dev.1+) — CheckTx will start rejecting missing polls
and the client-visible cost drops. The pre-flight's durable value is
fail-fast with an actionable error regardless of server-side timing.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/backend_task/contested_names/vote_on_dpns_name.rs (1)

34-67: Optional: avoid normalizing the same label twice.

convert_to_homograph_safe_chars(name) is invoked on line 66 to build normalized_label, and again inside dpns_vote_poll_index_values(name) on line 67. Cheap, but makes it easy for future edits to the two call sites to drift apart. Consider accepting the already-normalized label in the helper:

♻️ Proposed refactor
-/// Returns `[Value::from("dash"), Value::Text(normalized_label)]`.
-fn dpns_vote_poll_index_values(name: &str) -> Vec<Value> {
-    vec![
-        Value::from("dash"),
-        Value::Text(convert_to_homograph_safe_chars(name)),
-    ]
-}
+/// Returns `[Value::from("dash"), Value::Text(normalized_label.to_owned())]`.
+///
+/// Caller is responsible for normalizing the label via
+/// `convert_to_homograph_safe_chars` before invoking this helper.
+fn dpns_vote_poll_index_values(normalized_label: &str) -> Vec<Value> {
+    vec![
+        Value::from("dash"),
+        Value::Text(normalized_label.to_owned()),
+    ]
+}
-        let normalized_label = convert_to_homograph_safe_chars(name);
-        let index_values = dpns_vote_poll_index_values(name);
+        let normalized_label = convert_to_homograph_safe_chars(name);
+        let index_values = dpns_vote_poll_index_values(&normalized_label);

Update the index_values_* tests to pass normalized inputs, and add one test that asserts convert_to_homograph_safe_chars("alice") == "a11ce" to keep the normalization coverage.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/backend_task/contested_names/vote_on_dpns_name.rs` around lines 34 - 67,
The helper dpns_vote_poll_index_values currently takes raw name and calls
convert_to_homograph_safe_chars again, causing double normalization; change
dpns_vote_poll_index_values to accept a pre-normalized label (e.g., fn
dpns_vote_poll_index_values(normalized_label: &str) -> Vec<Value>), update the
call site in AppContext::vote_on_dpns_name to pass normalized_label instead of
name, and update the index_values_* tests to pass normalized inputs; also add a
small unit test asserting convert_to_homograph_safe_chars("alice") == "a11ce" to
retain normalization coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/backend_task/contested_names/vote_on_dpns_name.rs`:
- Around line 34-67: The helper dpns_vote_poll_index_values currently takes raw
name and calls convert_to_homograph_safe_chars again, causing double
normalization; change dpns_vote_poll_index_values to accept a pre-normalized
label (e.g., fn dpns_vote_poll_index_values(normalized_label: &str) ->
Vec<Value>), update the call site in AppContext::vote_on_dpns_name to pass
normalized_label instead of name, and update the index_values_* tests to pass
normalized inputs; also add a small unit test asserting
convert_to_homograph_safe_chars("alice") == "a11ce" to retain normalization
coverage.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 05e2ac69-96c2-4519-8389-738d16790c48

📥 Commits

Reviewing files that changed from the base of the PR and between eb7c162 and 801cf07.

📒 Files selected for processing (2)
  • src/backend_task/contested_names/vote_on_dpns_name.rs
  • src/backend_task/error.rs

…l_index_values

Bot reviewers on #835 (copilot-pull-request-reviewer, coderabbitai)
flagged that convert_to_homograph_safe_chars ran twice per vote:
once by the caller to build normalized_label for the pre-flight query,
and once inside the helper. Harmless today, but if either call site
ever picks up a different normalization the pre-flight and the vote
poll would key off different strings — silently reproducing the very
bug this PR fixes.

Helper now takes a pre-normalized &str and returns it verbatim. The
caller's normalized_label is passed through once, used for both the
pre-flight query and the vote poll's index_values. A dedicated test
pins convert_to_homograph_safe_chars("alice") == "a11ce" so the
normalization contract itself stays covered after the move.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@lklimek lklimek merged commit cd4b52f into v1.0-dev Apr 22, 2026
5 checks passed
@lklimek lklimek deleted the fix/contested-names-normalize-dpns-vote-poll branch April 22, 2026 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants