Skip to content

feat(identity): pending DPNS registration indicator, hint-text sizing, and social-profile fixes#918

Merged
lklimek merged 15 commits into
v1.0-devfrom
feat/dpns-pending-pill-and-hint-typography
Jul 23, 2026
Merged

feat(identity): pending DPNS registration indicator, hint-text sizing, and social-profile fixes#918
lklimek merged 15 commits into
v1.0-devfrom
feat/dpns-pending-pill-and-hint-typography

Conversation

@Claudius-Maginificent

@Claudius-Maginificent Claudius-Maginificent commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

TL;DR: Show a "Pending" indicator for a requested-but-not-yet-awarded DPNS username, fix illegibly small hint text, clean up a confusing Contacts CTA with a dead button, and give social-profile saves real progress/success/error feedback — plus fix two review-found bugs in that new feedback (a banner that never went away, and a false success toast). Further fixes: the DPNS registration screen now tells you correctly whether your username was registered outright or is still awaiting a community vote, the Pending tooltip explains that Dash masternodes decide contested names, a pending request now counts toward the "Pick a username" onboarding step, and the identity badge on the Settings tab is properly aligned. Also fixes two wallet bugs found while live-testing this PR: a "Create Asset Lock" button that sat in the wrong place on the Wallets screen, and a Fund Platform Address dialog (plus Receive and Mine) that could silently vanish and reset when picking certain addresses from its dropdown.

User story

As a user who requested a username and its contest hasn't resolved yet, I want to see that my request is pending rather than looking like I never requested one, so I know it wasn't lost.

As a user saving my social profile, I want an honest, correctly-timed progress/success/error banner, so I can trust whether my changes actually saved.

As a user who registers a DPNS username that turns out to be contested, I want to be told honestly that it's pending a community vote rather than being told it's registered, so I don't believe I own a name I might still lose.

As a user funding a Platform address from an existing asset lock, I want the dialog to stay open no matter which address I pick from the list, so a stray click doesn't silently lose my in-progress attempt.

Scenario

Base flow

A user requests a DPNS username while its contest is still open; separately, a user opens Contacts to set up a display profile and saves it; separately, a user registers a DPNS username that other people also want; separately, a user opens the Wallets screen to fund a Platform address from an existing asset lock.

Actual behavior

A pending username looked identical to never having requested one. Onboarding hint text rendered at ~9px, hard to read. The Contacts setup card had a mis-worded CTA and a "Why?" button that did nothing when clicked. Saving a social profile gave no feedback even though it succeeded on Platform — and in review, that turned out to be worse than just "no feedback": the "Saving…" banner could stay on screen forever alongside the outcome banner, and a stale save result could pop a false "saved" toast after switching identities mid-save. Separately, registering a contested username showed "DPNS Name Registered!" even though the name was only submitted for a community vote, with no mention that masternodes decide the outcome, and the onboarding checklist kept showing "Pick a username" as incomplete despite the pending request. The identity badge on the Settings tab also sat awkwardly in its own row, misaligned with the rest of the column. Separately, on the Wallets screen, the "Create Asset Lock" button sat in the Asset Locks card's header instead of below the list like every other action button on that screen, and picking certain addresses from the Fund Platform Address dialog's dropdown — particularly ones near the bottom of a longer list — silently closed and reset the whole dialog, discarding the in-progress funding attempt with no error shown. The Receive and Mine dialogs had the same latent flaw with their own popups.

Expected behavior

A "Pending" pill (with an ETA tooltip) shows on the Identity Home hero card, Identities list, and Settings tab. Hint text is legible. The Contacts CTA is clear with no dead controls. Saving a profile shows progress, then exactly one accurate success/error banner that actually clears when the operation finishes — no lingering banners, no false positives. Registering a contested username now shows honest, distinct copy for "registered outright" vs. "submitted, pending a community vote"; the Pending tooltip explains that Dash masternodes vote on the outcome; a pending request counts toward the "Pick a username" checklist step; and the Settings identity badge sits properly aligned beside its heading. The "Create Asset Lock" button now sits below the asset-lock list in every state (loading, empty, and populated), consistent with the rest of the screen. The Fund Platform Address, Receive, and Mine dialogs stay open through address selection regardless of which row is picked, closing only via an explicit Cancel/Close control or the window's own close button.

Detailed discussion

What was done

Feature (first 3 commits on this branch):

  • model/contested_name.rs gains pure pending_username_for/pending_username_in decision logic + a human ETA formatter, surfaced as a "Pending" pill on the Identity Home hero card, Identities list, and Settings tab.
  • Typography::hint(): a new, documented 14px token for instructional hint text, replacing raw .small() (~9px) at the 5 genuine instructional-hint call sites (the other 67 unrelated .small() sites were deliberately left alone).
  • Contacts gate card: CTA reworded; the non-functional "Why?" button and its dead expand/collapse scaffolding removed (confirmed via investigation that it could never actually open its panel).
  • Social-profile save: added progress/success/error banners (previously silent); fixed ProfileCache to update from a save's result, not just a load; added Avatar-URL format/size guidance.
  • Settings identity-type badge: now a read-only accent pill instead of a Button-styled fake pill.

Review fixes (4th commit, addressing claudius:grumpy-review consolidation):

  • SEC-001 (blocking): the PROFILE_SAVING progress banner was never cleared on success or error — it sat on screen forever next to the outcome banner, contradicting it. Now cleared on both paths via a shared clear_profile_saving_banner() helper wired into AppState's generic success/error dispatch.
  • CODE-007 (blocking): the "Your social profile is saved." toast fired on identity-ID match alone, even when on_profile_saved() correctly returned None for a stale result (e.g. switching away from and back to an identity mid-save) — showing a false success toast. Moved inside the on_profile_saved() guard so it only fires for a genuine, just-completed save.
  • CODE-005 (medium): the pending-username lookup rescanned and re-deserialized the entire cached-contest store on every egui repaint across 3 render paths. Added an in-memory AppContext::pending_dpns_usernames cache, refreshed once per contest-cache update, instead of scanning per frame.
  • CODE-006 (medium): an identity with multiple simultaneous pending contests silently showed only whichever one the cache iterator returned first, with no indication another existed. Replaced first-match with a deterministic priority (earliest known decision time, then name as a stable tiebreaker).
  • SEC-002 (low, defense-in-depth): a network-sourced pending username was rendered with no neutralization of Unicode bidi/control characters. Added a sanitizer (strips control + bidi-override chars, clamps to 63 chars) at every render site.
  • PROJ-002/PROJ-003/PROJ-004 (low, copy/catalog): fixed the Contacts CTA grammar/terminology drift, added the missing pill.rs catalog entry to ui/components/README.md, and rewrote the pending-username ETA phrase as complete, i18n-clean sentences instead of a spliced fragment.
  • CALL-001 (info): call-tree walk confirmed the Why? teardown left no orphaned callers — no action needed.

Further fixes (5th–6th commits, addressing additional user-reported issues):

  • Identity Settings badge alignment: the "User identity" pill (already using the shared accent_pill) sat in its own row above "Username," throwing off the Settings columns. It now sits right-aligned beside the "Username" heading, sharing the Home hero's label, tooltip, and accent source.
  • DPNS registration outcome: the registration screen always said "DPNS Name Registered!" even when the submitted name entered contested voting. It now classifies the outcome via the DPNS contract's own contested-vote-poll predicate at submission time (not the separately-refreshed contest cache, which could look stale) and shows outcome-specific copy — "Your username is registered. You can use it now." or "Your username request was submitted. Other people can also request this name, so the community will vote on who receives it. Check the Pending label on your identity for updates."
  • Pending-pill tooltip clarity: every ETA bucket (and the unknown-ETA fallback) now explicitly states that Dash masternodes vote on who receives the contested name, instead of a vague "being confirmed" message.
  • Onboarding checklist credit: a pending (submitted but not yet decided) username registration now completes the "Pick a username" step, with subtext explaining the request is pending while masternodes vote; if the pending request later disappears without a win, the step correctly reverts to incomplete.

Wallet dialog fixes (7th commit, addressing further issues found in live QA of this PR):

  • Fund Platform Address / Receive / Mine dialogs silently closing: each dialog's address selector (a ComboBox popup, or AddressInput's autocomplete overlay) renders as its own floating layer that can extend past the modal window's edge for rows further down the list. The outside-click-to-close check only tested whether a click landed inside the window's own rectangle, so clicking a row that rendered outside it (most likely the last row in a longer list) registered as an "outside click" and force-closed and fully reset the dialog — losing the in-progress selection with no error shown. All three dialogs now rely solely on an explicit Cancel/Close control and the window's native close button; Receive gained a Close button it was missing. selection_dialog.rs, which already handled this correctly, was left untouched and used as the reference pattern. Audited all 14 call sites of this outside-click helper across the app; only these three had a floating popup and needed the change.
  • "Create Asset Lock" button placement: it sat in the Asset Locks card's header row, inconsistent with every other section on this screen (e.g. "+ Add Receiving Address," which sits below its table). Moved below the asset-lock list/empty-state in every state, and switched from a bare button to the project's standard StyledButton.
  • New kittest regressions confirm the last dropdown row keeps the Fund dialog open and selects that address, and that the asset-lock action renders below the empty state.

Testing

  • New/updated unit + kittest coverage across the feature, each review fix, and the further fixes — all green via the narrowest relevant scope, with new/changed test names individually confirmed passing in the verification logs (not just an aggregate count).
  • cargo clippy --all-features --all-targets -- -D warnings clean (and scoped --lib --test kittest re-checks for the further fixes), cargo fmt --all clean.
  • Independently re-verified by security-engineer-smythe and qa-engineer-marvin during grumpy-review consolidation.
  • Wallet dialog fixes: cargo test --test kittest --all-features and cargo clippy --bin dash-evo-tool --all-features -- -D warnings both clean; 260 kittest cases passed including the two new regressions, individually confirmed in the log.

Breaking changes

None.

Checklist

  • Tests added (TDD)
  • i18n-ready, no jargon in user-facing strings
  • No secrets in diff

Prior work

Attribution

🤖 Co-authored by Claudius the Magnificent AI Agent

Summary by CodeRabbit

  • New Features

    • DPNS username registration now distinguishes between immediate availability and a pending community-voting state.
    • Pending DPNS usernames now show a “Pending” badge across identity views (hero, identity list/name cell, identity cards, settings, and onboarding checklist), including an estimated readiness time when available.
    • Social profile saving now shows clearer progress and success feedback, and saved profile details update immediately.
  • Bug Fixes

    • Pending username UI is safer and correctly formatted (with improved badge tooltip messaging).
    • Profile-saving notifications/banners are dismissed correctly after both success and failure, scoped to the matching identity.

lklimek and others added 4 commits July 21, 2026 09:35
Instructional hint text — the short "what to do / why" line under a
primary label — was rendered with egui's raw RichText::small(), which
resolves to egui's ~9px default and reads too small. This was not using
the app's centralized Typography scale at all.

Add a dedicated Typography::hint() token (SCALE_SM, 14px) so this text
class is pinned to the centralized scale, and migrate the Add-contact
error tips to it. Other .small() call sites (timestamps, tags, incidental
labels) are intentionally left untouched — only genuine instructional
hint text moves to the new token.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A contested DPNS name that was requested but not yet awarded leaves an
identity with empty dpns_names (owned) while its alias is optimistically
set to "name.dash". The Identity Home hero card therefore showed
"No username yet — Pick a username" even though the user had already
chosen a name — indistinguishable from never having requested one.

Distinguish requested-but-unawarded from never-requested and owned:

- Pure detection in model/contested_name.rs: pending_username_for /
  pending_username_in (a contender is pending when the contest is not
  WonBy/Locked) plus an ETA humanizer, approximate_time_until.
- Read-only AppContext wrappers pending_dpns_username_for /
  pending_dpns_usernames over the ongoing-contest cache.
- Reusable "Pending" pill in ui/components/pill.rs (accent_pill extracted
  from the hero's own pill helper), shown on the hero card next to the
  requested @name and in the Identities list Name cell. Tooltip states the
  name is being confirmed and gives an estimated ready time when known.
- Onboarding checklist reflects the pending state — a "being confirmed"
  subtext and no "pick a name" nag button — instead of implying nothing
  was done. Its instructional subtext also adopts Typography::hint().
- New user story DPN-010.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…utton

On the Contacts gate card:

- The primary CTA "Add a display name" is reworded to "Setup display
  profile" — it opens the profile editor, not just a name field.
- The "Why?" button was non-functional: `render_gated` received its
  `why_toggled` response but only had a TODO that never persisted the
  `expanded` flag, so the explanation panel could never open. Rather
  than wire dead UI, remove the button, its panel, and all the now-unused
  scaffolding (WHY_* copy, `expanded`/`with_expanded`, `resolved_why_label`,
  `GateCardAction::WhyToggled`, the `why_toggled` response field).

Tests updated: gate-card unit tests drop the Why?-toggle assertions and
lock the new CTA copy; the IT-CONTACTS-01 integration test asserts the
reworded CTA renders and the "Why?" button no longer does.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…d page polish

Fixes the "Save social profile" flow on the Identity Hub Settings tab and
adds the requested guidance/indicators. Log evidence showed the backend
save genuinely succeeds ("Profile created: doc_id=…, revision=1") but the
app gave no feedback and looked unsaved on revisit. Root causes and fixes:

- No confirmation: app.rs routes DashPayProfileUpdated to the screen with
  no generic success banner, and the hub's arm set none. Now it shows a
  success banner. A progress banner is shown on dispatch (no auto-dismiss,
  since the save can take minutes) and is replaced by the success/error
  banner when the task finishes.
- Looked unsaved app-wide / on revisit: the profile cache's record_result
  only consumes LoadProfile results; a save arrives as
  DashPayProfileUpdated(id) with no fields, so the cache kept the pre-save
  profile. `on_profile_saved` now returns the committed fields and the hub
  refreshes the cache via new `ProfileCache::record_saved`, so the hero,
  Contacts gate, and this tab on re-entry all reflect the save. (There is
  no stuck guard: the Save button is purely !invalid && dirty; the "stuck"
  perception was the missing feedback plus the correct post-save disable.)
- Avatar URL field: adds accurate guidance (public square image; JPEG/PNG/
  WebP/GIF; 256×256+ recommended) using the Typography::hint() token.
- Identity-type badge: rendered as a read-only accent pill (matching the
  Home hero card) instead of an input-looking button that read as editable.
- Pending username: when the identity owns no name but has a requested-but-
  unawarded one, shows the requested @name with a "Pending" pill (reusing
  the shared pending-DPNS detection) instead of the register CTA.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@lklimek, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 11 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c3e28c5a-faa7-46e9-83c5-4e04b2913ae4

📥 Commits

Reviewing files that changed from the base of the PR and between e914b5b and 95cb1fa.

📒 Files selected for processing (2)
  • src/backend_task/migration/finish_unwire.rs
  • src/backend_task/migration/v093_upgrade.rs
📝 Walkthrough

Walkthrough

The PR adds DPNS registration outcome classification and pending username tracking, exposes pending status throughout identity UI, updates onboarding and tooltips, and improves DashPay profile-save feedback. It also simplifies the social-profile gate, changes wallet dialog interactions, and updates related tests and documentation.

Changes

DPNS registration and pending-state pipeline

Layer / File(s) Summary
DPNS outcome and pending-state model
src/model/dpns.rs, src/model/contested_name.rs
DPNS registrations now report Registered or PendingCommunityVote; contested-name data produces prioritized, sanitized pending username records with coarse ETA values.
Pending-state cache and backend wiring
src/context/..., src/backend_task/...
AppContext caches pending usernames, refreshes them after contested-resource queries, and exposes filtered lookup methods. Registration results now carry the outcome in a struct-shaped backend result.
Registration validation
tests/backend-e2e/...
Backend tests match the new structured RegisteredDpnsName result shape.

Pending username presentation

Layer / File(s) Summary
Shared pending pill
src/ui/components/pill.rs, src/ui/components/mod.rs, src/ui/components/README.md
Adds reusable accent and pending pill renderers with tooltip text and ETA handling.
Identity and username surfaces
src/ui/identities/..., src/ui/identity/{home.rs,identity_hero_card.rs,settings.rs}
Pending usernames appear in identity lists, hero cards, and Settings; owned handles take precedence over pending requests.
Onboarding and registration results
src/ui/identity/onboarding_checklist.rs, src/ui/identities/register_dpns_name_screen.rs, tests/kittest/register_dpns_name_screen.rs
Pending requests complete the username checklist step without an action button, while registration success copy reflects the classified outcome.

DashPay profile-save feedback

Layer / File(s) Summary
Profile update lifecycle
src/backend_task/mod.rs, src/ui/identity/settings.rs, src/ui/identity/profile_cache.rs, src/ui/identity/hub_screen.rs, src/app.rs
Profile updates receive a dedicated backend context, show a persistent saving banner, clear it on success or error, and immediately update the profile cache after confirmed saves.
Profile update tests
src/app.rs, src/ui/identity/{hub_screen.rs,settings.rs}
Tests cover banner cleanup, stale results, committed profile fields, and cache refresh behavior.

Social-profile gate and instructional UI

Layer / File(s) Summary
Single-action social-profile gate
src/ui/identity/{contacts.rs,social_profile_gate_card.rs}, tests/kittest/identity_hub_contacts.rs
The gate now exposes only “Set up your social profile”; the “Why?” interaction and related state are removed.
Hint typography
src/ui/theme.rs, src/ui/dashpay/add_contact_screen.rs, src/ui/identity/onboarding_checklist.rs
Adds Typography::hint() and applies it to instructional and error guidance text.

Wallet dialog interactions

Layer / File(s) Summary
Explicit wallet dialog controls
src/ui/wallets/wallets_screen/{asset_locks.rs,dialogs.rs}
Asset-lock actions use shared styling, and wallet dialogs now close through explicit controls rather than outside-click handling.
Wallet UI tests
tests/kittest/wallets_screen.rs
Wallet fixtures use randomized zeroized seeds, and funding-dialog tests verify selection remains open until the explicit fund action.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Suggested labels: claudius-review

Suggested reviewers: lklimek

🚥 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 clearly captures the main identity-focused changes: pending DPNS indicators, hint-text sizing, and social-profile fixes.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/dpns-pending-pill-and-hint-typography

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.

@lklimek
lklimek marked this pull request as ready for review July 21, 2026 10:11
@thepastaclaw

thepastaclaw commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

🕓 Ready for review — 11 ahead in queue (commit e914b5b)
Queue position: 12/23 · 3 reviews active
ETA: start ~14:30 UTC · complete ~14:50 UTC (median 20m across 30 recent reviews; 3 slots)
Queued 20h 36m ago · Last checked: 2026-07-23 13:10 UTC

Ensure social-profile save banners finish cleanly, memoize pending DPNS names, and sanitize pending-name display text.

Co-Authored-By: Codex GPT-5 <noreply@openai.com>
lklimek and others added 2 commits July 21, 2026 13:28
Co-Authored-By: Codex GPT-5 <noreply@openai.com>
Classify the DPNS registration outcome (Registered vs PendingCommunityVote)
via the document type's own contested-vote-poll predicate, replacing the
misleading unconditional "DPNS Name Registered!" success panel with
outcome-specific copy. Explain that Dash masternodes vote on contested
names in the Pending pill's tooltip. Credit a pending (submitted but still
contested) registration as completing the "Pick a username" onboarding
step, with voting-status subtext.

Co-Authored-By: Codex GPT-5 <noreply@openai.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/ui/identity/settings.rs (1)

412-427: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate "owns name → gate pending lookup" logic.

Identical to the block in src/ui/identity/home.rs (lines 282-297). Worth extracting to a shared helper — see the consolidated comment below.

🤖 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 `@src/ui/identity/settings.rs` around lines 412 - 427, Extract the duplicated
pending DPNS username gating and lookup from the current settings flow and the
corresponding home flow into a shared helper. Reuse that helper where
pending_username is computed, preserving the existing behavior of skipping the
lookup when any non-empty DPNS name exists and returning None on lookup failure
or absence.
src/ui/identity/home.rs (1)

282-297: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate "owns name → gate pending lookup" logic.

This block is identical to the one in src/ui/identity/settings.rs (lines 416-427). Extracting a shared helper would remove the duplication and also close a gap in identities_screen.rs, which currently has no equivalent guard.

See the consolidated comment below.

🤖 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 `@src/ui/identity/home.rs` around lines 282 - 297, Extract the shared “identity
owns a non-empty DPNS name, otherwise read pending username” logic into a
reusable helper, then replace the duplicated block in the home and settings
flows with that helper. Reuse the helper in identities_screen.rs so pending
lookup is also gated there, preserving best-effort failure handling and
returning None when a name is already owned.
🤖 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 `@src/ui/identities/identities_screen.rs`:
- Around line 531-539: Centralize the ownership check in a shared AppContext
helper, such as pending_dpns_username_for_identity, that returns no pending
username when the QualifiedIdentity already has a non-blank dpns_names entry.
Update src/ui/identities/identities_screen.rs#L531-L539 to apply this guard to
batch results, and replace the duplicated inline lookup logic with the helper in
src/ui/identity/settings.rs#L412-L427 and src/ui/identity/home.rs#L282-L297.

---

Nitpick comments:
In `@src/ui/identity/home.rs`:
- Around line 282-297: Extract the shared “identity owns a non-empty DPNS name,
otherwise read pending username” logic into a reusable helper, then replace the
duplicated block in the home and settings flows with that helper. Reuse the
helper in identities_screen.rs so pending lookup is also gated there, preserving
best-effort failure handling and returning None when a name is already owned.

In `@src/ui/identity/settings.rs`:
- Around line 412-427: Extract the duplicated pending DPNS username gating and
lookup from the current settings flow and the corresponding home flow into a
shared helper. Reuse that helper where pending_username is computed, preserving
the existing behavior of skipping the lookup when any non-empty DPNS name exists
and returning None on lookup failure or absence.
🪄 Autofix (Beta)

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

Run ID: ea1a1775-0faa-411c-9dad-9c80637e0bfa

📥 Commits

Reviewing files that changed from the base of the PR and between 0812761 and 5e97410.

📒 Files selected for processing (29)
  • docs/user-stories.md
  • src/app.rs
  • src/backend_task/contested_names/query_dpns_contested_resources.rs
  • src/backend_task/identity/register_dpns_name.rs
  • src/backend_task/mod.rs
  • src/context/contested_names_db.rs
  • src/context/mod.rs
  • src/model/contested_name.rs
  • src/model/dpns.rs
  • src/ui/components/README.md
  • src/ui/components/mod.rs
  • src/ui/components/pill.rs
  • src/ui/dashpay/add_contact_screen.rs
  • src/ui/identities/identities_screen.rs
  • src/ui/identities/register_dpns_name_screen.rs
  • src/ui/identity/contacts.rs
  • src/ui/identity/home.rs
  • src/ui/identity/hub_screen.rs
  • src/ui/identity/identity_hero_card.rs
  • src/ui/identity/onboarding_checklist.rs
  • src/ui/identity/profile_cache.rs
  • src/ui/identity/settings.rs
  • src/ui/identity/social_profile_gate_card.rs
  • src/ui/theme.rs
  • tests/backend-e2e/dashpay_tasks.rs
  • tests/backend-e2e/framework/fixtures.rs
  • tests/backend-e2e/register_dpns.rs
  • tests/kittest/identity_hub_contacts.rs
  • tests/kittest/register_dpns_name_screen.rs

Comment thread src/ui/identities/identities_screen.rs Outdated
@lklimek lklimek added the claudius-review Triggers automated code review using claudius plugin, runs as a CI job label Jul 21, 2026
lklimek and others added 2 commits July 21, 2026 16:19
…e test fixtures

- Add a short comment at the preorder-document put call site explaining why
  its conflicts intentionally stay generic (its only unique index,
  saltedDomainHash, is unrelated to usernames).
- Replace four copy-pasted DuplicateUniqueIndexError test fixtures (three in
  error.rs, one in register_dpns_name.rs) with a single shared helper in the
  existing src/test_support.rs module.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claudius has reviewed PR #918 — and deigns to be mostly impressed

A tidy, genuinely well-tested piece of work: priority determinism, tie-breaking, bidi/control sanitization, and outcome-copy all have real assertions behind them, the "Why?" teardown left zero orphaned identifiers or duplicated render blocks (a low bar this repo has tripped over before — noted and appreciated), and every changed signature has its callers updated. Four independent reviewers — security, project-consistency, adversarial QA, and docs — converged on the same short list.

2 MEDIUM findings (both inline above, both living in the very feedback/indicator code this PR set out to improve — so worth closing before merge):

  • 🟠 Identities list shows a stale "Pending" pill for a name the identity already owns. Home and Settings guard on ownership; the batch path doesn't. One shared AppContext helper fixes the gap and the copy-paste. (Also the still-open CodeRabbit thread.)
  • 🟠 "Saving…" banner clears on the wrong save — the clear matches on task type, not dispatch_id/identity, so an identity switch mid-save strips the banner owed to an in-flight save.

7 LOW/INFO (optional polish, full detail in the report): a ~230-char success sentence rendered at ui.heading() scale; a Cf zero-width-char gap in the username sanitizer (defense-in-depth, native renderer so no injection surface); a six-fold-duplicated copy sentence; a single-variant action enum left after the "Why?" removal; a public pending_username_in() exercised only by its own tests while production uses a different accumulator; and a README catalog row that omits the Identities-list consumer.

No security blockers, no correctness landmines, no secrets. Address the two ambers and this is ready. The empire approves of the trajectory. 🏆

🤖 Consolidated from a 4-agent grumpy-review. Full severity-ranked report (HTML + JSON) generated for the maintainers.
📊 View full HTML review report

Comment thread src/ui/identities/identities_screen.rs Outdated
Comment thread src/app.rs
@github-actions github-actions Bot removed the claudius-review Triggers automated code review using claudius plugin, runs as a CI job label Jul 21, 2026
lklimek added 4 commits July 22, 2026 07:58
Two review findings from PR #918 confirmed still open at HEAD:

- The Identities list's batch pending-DPNS lookup had no ownership guard,
  unlike Home and Settings, so it could show a stale "Pending" pill for a
  name that had already been awarded. Added
  `AppContext::pending_dpns_username_for_identity` and a batch
  `pending_dpns_usernames_for_identities`, and switched all three call
  sites to the shared, ownership-aware helper.

- The "Saving your social profile…" progress banner cleared on *any*
  DashPay profile-update result, so two saves in flight for different
  identities could have one identity's completion silently swallow the
  other's feedback. `BackendTaskContext::DashPayProfileUpdate` now
  carries the identity id; the clear helpers and
  `SettingsTab::clear_pending_save` only act when the result's identity
  matches the banner's/snapshot's owner.

Regression tests added for both: ownership-filtered batch lookup, and
cross-identity banner/pending-save isolation on both the success and
error paths.

Verified via cargo-cached.sh: `test --lib --all-features` (7 named
tests passing), `clippy --all-features --all-targets -- -D warnings`
(clean).

Resolves review threads:
#918 (comment)
#918 (comment)
#918 (comment)

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Keep Receive, Fund Platform Address, and Mine dialogs open while their floating child widgets are in use. Move the styled Create Asset Lock action below the asset-lock content in every state and add UI regressions.\n\nAddresses user-reported issues on PR #918.\n\nCo-Authored-By: OpenAI Codex <noreply@openai.com>
@lklimek
lklimek enabled auto-merge (squash) July 22, 2026 15:33
@lklimek
lklimek disabled auto-merge July 22, 2026 15:33
@lklimek lklimek added the claudius-review Triggers automated code review using claudius plugin, runs as a CI job label Jul 22, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claudius has re-reviewed PR #918 — and pronounces it ready

The empire is pleased. This pass (HEAD 44485830) confirms the two MEDIUM findings from the previous round are genuinely fixed, not merely papered over:

  • 🟢 Stale "Pending" pill — the batch path now routes through the ownership-aware pending_dpns_username_for_identity / pending_dpns_usernames_for_identities helpers, and the regression test exercises a mixed owned/unowned batch rather than a vacuous single-identity case. Verified.
  • 🟢 "Saving…" banner clearing on the wrong saveBackendTaskContext::DashPayProfileUpdate now carries the identity id, and the clear helpers key on it. The new tests stand up two concurrent per-identity banners and prove clearing A's leaves B's alone. A proper race-shaped test, not a happy path.

The newest commit — the wallet-dialog popup-preservation fix — is a clean, complete fix across all three dialogs (Receive / Fund Platform / Mine): outside-click-to-close removed only where a floating popup layer could extend past the modal rect, every dialog keeps an explicit Close/Cancel/native path, and the shared guard stays live for the ~10 modals that legitimately want it. No dead code, no stranded dialogs, no fund-path logic touched.

Three independent reviewers (security · project-consistency · adversarial-QA) converged. No security, correctness, or fund-safety blockers. No secrets.

6 LOW findings — optional polish, none blocking

  • 🔵 Profile-save confirmation gap (hub_screen.rs:784) — handle_profile_updated gates the "saved" toast + cache write on the currently-selected identity rather than the task's own carried id. Switch identities mid-save and the completed save gets no confirmation (self-heals on cache-reset nav paths). The banner-clear fix already threads the carried id — the cache write could ride the same rail (it's keyed by saved_id anyway), leaving only the on-screen banner gated to selection.
  • 🔵 Receive/Mine dialogs lack the popup regression test — the fix covers all three, but only Fund Platform got a test. A future one-dialog regression would keep CI green.
  • 🔵 pending_username_in (contested_name.rs:89) — public, zero production callers, exercised only by its own tests; production runs the plural accumulator. (Recurring — flagged last round.)
  • 🔵 PendingCommunityVote success copy (register_dpns_name_screen.rs:326) — a ~185-char paragraph handed to ui.heading(). Short title in the heading, paragraph in the body slot. (Recurring.)
  • 🔵 Duplicated "Create Asset Lock" button block (asset_locks.rs:92/186) — byte-identical across two branches; extract a closure before it drifts.
  • 🔵 pill.rs catalog row (components/README.md:33) — still omits the Identities-list consumer, and disagrees with user-stories.md DPN-010 on which surfaces use the pill. (Recurring.)

None of these gate merge. Three of them are prior LOW items that survived — worth a mop-up commit if the mood strikes, but the empire will not withhold its blessing over them.

Approved. Ship it. 🏆

🤖 Consolidated from a 3-agent grumpy-review (0 security, 4 project, 2 QA → 6 LOW after dedup/severity re-assessment). Full HTML + JSON report generated for maintainers.
📊 View full HTML review report

@github-actions github-actions Bot removed the claudius-review Triggers automated code review using claudius plugin, runs as a CI job label Jul 22, 2026
…og row

The `pending_username_pill` renderer gained a third consumer in this PR —
`identities_screen.rs` — alongside `IdentityHeroCard` and Settings, but the
components catalog row still named only the first two. pill.rs's own module
doc and user-stories DPN-010 already count the Identities list, so the catalog
was the odd one out. Bring it in line with the code it describes.

Co-Authored-By: Claude Opus <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/backend_task/mod.rs (1)

1320-1327: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not commit a plaintext HD seed in the test.

[0x7b; 64] is a seed literal committed to source. Generate test-only bytes at runtime and retain them in Zeroizing.

Proposed fix
+        let hd_seed = Zeroizing::new(rand::random::<[u8; 64]>());
         secret_access.remember_session(
             &scope,
-            SecretPlaintext::HdSeed(&Zeroizing::new([0x7b; 64])),
+            SecretPlaintext::HdSeed(&hd_seed),
             RememberPolicy::UntilAppClose,
         );

As per coding guidelines, “Never commit plaintext recovery phrases, private keys, passwords, seeds, or API tokens in source, tests, fixtures, or documentation.”

🤖 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 `@src/backend_task/mod.rs` around lines 1320 - 1327, Replace the hard-coded
`[0x7b; 64]` value in the `secret_access.remember_session` test with bytes
generated at runtime using an appropriate test-only generator, and keep the
generated seed wrapped in `Zeroizing`. Preserve the existing
`SecretPlaintext::HdSeed` and `RememberPolicy::UntilAppClose` behavior.

Source: Coding guidelines

🤖 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 `@src/context/contested_names_db.rs`:
- Around line 260-286: Update the primary DPNS name selection in the identity
settings flow to skip entries whose name is blank after trimming whitespace.
Replace the direct first-entry lookup on identity.dpns_names with a
nonblank-aware search, preserving the existing pending-username fallback when no
displayed awarded name exists.

---

Outside diff comments:
In `@src/backend_task/mod.rs`:
- Around line 1320-1327: Replace the hard-coded `[0x7b; 64]` value in the
`secret_access.remember_session` test with bytes generated at runtime using an
appropriate test-only generator, and keep the generated seed wrapped in
`Zeroizing`. Preserve the existing `SecretPlaintext::HdSeed` and
`RememberPolicy::UntilAppClose` behavior.
🪄 Autofix (Beta)

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

Run ID: ebf4c64e-e458-445f-878c-6a3f21453d17

📥 Commits

Reviewing files that changed from the base of the PR and between 5e97410 and e914b5b.

📒 Files selected for processing (15)
  • docs/user-stories.md
  • src/app.rs
  • src/backend_task/error.rs
  • src/backend_task/identity/register_dpns_name.rs
  • src/backend_task/mod.rs
  • src/context/contested_names_db.rs
  • src/test_support.rs
  • src/ui/components/README.md
  • src/ui/identities/identities_screen.rs
  • src/ui/identity/home.rs
  • src/ui/identity/hub_screen.rs
  • src/ui/identity/settings.rs
  • src/ui/wallets/wallets_screen/asset_locks.rs
  • src/ui/wallets/wallets_screen/dialogs.rs
  • tests/kittest/wallets_screen.rs
🚧 Files skipped from review as they are similar to previous changes (8)
  • src/ui/components/README.md
  • docs/user-stories.md
  • src/backend_task/identity/register_dpns_name.rs
  • src/app.rs
  • src/ui/identities/identities_screen.rs
  • src/ui/identity/home.rs
  • src/ui/identity/hub_screen.rs
  • src/ui/identity/settings.rs

Comment on lines +260 to +286
/// Return a pending DPNS username only while `identity` owns no awarded name.
pub fn pending_dpns_username_for_identity(
&self,
identity: &QualifiedIdentity,
) -> Option<PendingUsername> {
if identity_owns_dpns_name(identity) {
None
} else {
self.pending_dpns_username_for(&identity.identity.id())
.ok()
.flatten()
}
}

/// Map identities without an awarded name to their pending DPNS usernames.
pub fn pending_dpns_usernames_for_identities(
&self,
identities: &[QualifiedIdentity],
) -> HashMap<Identifier, PendingUsername> {
let identity_ids = identities
.iter()
.filter(|identity| !identity_owns_dpns_name(identity))
.map(|identity| identity.identity.id())
.collect::<Vec<_>>();
self.pending_dpns_usernames(&identity_ids)
.unwrap_or_default()
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Ignore blank DPNS entries when selecting the displayed primary name.

These wrappers correctly return pending data for whitespace-only names, but src/ui/identity/settings.rs:414-558 checks identity.dpns_names.first() without the same nonblank filter. It renders an empty primary username instead of reaching the pending branch.

Update that primary-name selection to filter blank names, e.g. iter().find(|name| !name.name.trim().is_empty()).

🤖 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 `@src/context/contested_names_db.rs` around lines 260 - 286, Update the primary
DPNS name selection in the identity settings flow to skip entries whose name is
blank after trimming whitespace. Replace the direct first-entry lookup on
identity.dpns_names with a nonblank-aware search, preserving the existing
pending-username fallback when no displayed awarded name exists.

`a_second_launch_after_an_unreadable_identity_preserves_user_edits_and_deletions`
called `delete_local_qualified_identity` immediately after the first
migration `run()` returned. `run()` detaches its best-effort DAPI refresh
onto a spawned task that queues for the `migration_run` guard right behind
the caller (see `spawn_dapi_refresh`), and `delete_local_qualified_identity`
claims that same guard via `try_lock`. Under CI's parallel/loaded test
execution the detached task sometimes won the race, so the delete call
transiently failed with `WalletStorageNotReady` — this PR's Test Suite job
went red on it (2079 passed, 1 failed), while the test passed reliably in
isolation.

Promote `finish_unwire`'s existing `wait_for_dapi_refresh` test helper
(already used to close this exact race in two other tests in the same
file) from private-to-`mod tests` to `pub(crate)` at module scope, and call
it from the v093_upgrade regression before the delete — same fix pattern,
reused instead of re-derived.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@lklimek
lklimek enabled auto-merge (squash) July 23, 2026 09:00
@lklimek
lklimek merged commit 7e80e60 into v1.0-dev Jul 23, 2026
5 checks passed
@lklimek
lklimek deleted the feat/dpns-pending-pill-and-hint-typography branch July 23, 2026 09:06
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