feat: DashPay contacts improvements#564
Merged
Merged
Conversation
Added local DB cache lookup and Platform profile fetching for identity IDs in contact requests, so incoming requests show display names/usernames instead of raw identity IDs, and outgoing requests show recipient names. Task: 7.2a Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Load contact data from local DB on screen creation (dashpay_contacts, dashpay_profiles, contact_private_info tables) - Trigger FetchContactProfile backend task on screen arrival for fresh Platform data - Save contact info edits to both local DB (immediately) and Platform (via DashPayTask::UpdateContactInfo with encrypted data) - Handle ContactProfile, ContactInfoUpdated, and ContactsWithInfo task results - Replace non-functional Remove/Block Contact buttons with informational text - Use DashColors semantic constants instead of inline Color32 values Task: 7.2b
Replace mock "alice.dash" contact name in SendPaymentScreen with actual DB lookup. Implement load_payment_history() to query the dashpay_payments table. Update LoadPaymentHistory backend handler to return real DB records with resolved contact names. Task: 7.2c
…fo label Task: 7.2d
Added created_at field to Contact struct, populated from DB StoredContact. Implemented SearchFilter::Recent (7-day window) and SortOrder::DateAdded (newest first) which previously had TODO stubs. Task: 7.2e Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ay backend Task: 7.2f
Previously, load_contacts() fetched each contact's DashPay profile and DPNS username sequentially (2N network round-trips for N contacts). Now processes contacts in chunks of 10 using futures::join_all, running all fetches within each chunk concurrently. Reduces total fetch time by roughly 10x for large contact lists. Task: 7.2g Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace DashPayResult sub-enum references with flat BackendTaskSuccessResult variants that exist on v1.0-dev. Replace DashColors::WARNING_ORANGE with inline Color32. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughLocal DashPay payment history and contact/profile loading were implemented and enriched; contact enrichment queries were parallelized in chunks; UI screens now load contact/profile/private info from the local DB, persist private contact edits locally, schedule backend profile fetches, and reconcile backend task results into in-memory state and the DB. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ui/dashpay/contacts_list.rs (1)
445-523:⚠️ Potential issue | 🟠 MajorExpose the new Recent filter and DateAdded sort options in the UI.
SearchFilter::RecentandSortOrder::DateAddedexist, but the combo boxes never present them, so the feature can’t be used.✅ Add the missing options
.show_ui(ui, |ui| { ui.selectable_value( &mut self.search_filter, SearchFilter::All, "All", ); ui.selectable_value( &mut self.search_filter, SearchFilter::WithUsernames, "With usernames", ); ui.selectable_value( &mut self.search_filter, SearchFilter::WithoutUsernames, "No usernames", ); ui.selectable_value( &mut self.search_filter, SearchFilter::WithBio, "With bio", ); + ui.selectable_value( + &mut self.search_filter, + SearchFilter::Recent, + "Recent", + ); ui.selectable_value( &mut self.search_filter, SearchFilter::Hidden, "Hidden", ); ui.selectable_value( &mut self.search_filter, SearchFilter::Visible, "Visible", ); }); }); @@ .show_ui(ui, |ui| { ui.selectable_value(&mut self.sort_order, SortOrder::Name, "Name"); ui.selectable_value( &mut self.sort_order, SortOrder::Username, "Username", ); + ui.selectable_value( + &mut self.sort_order, + SortOrder::DateAdded, + "Date added", + ); ui.selectable_value( &mut self.sort_order, SortOrder::AccountRef, "Account", ); });
🤖 Fix all issues with AI agents
In `@src/backend_task/dashpay/contacts.rs`:
- Around line 418-470: The async tasks inside the contact_list.chunks_mut loop
capture the shared sdk and call Document::fetch_many(sdk, ...) across await
points; to avoid holding internal SDK locks you must clone the SDK for async
use: clone the SDK once before entering the chunk loop and then use that clone
inside each async move closure (or clone again per-closure) so
Document::fetch_many is called with the cloned instance (e.g., replace uses of
sdk inside the closure with sdk.clone() or a locally bound sdk_clone), ensuring
the cloned variable is moved into the async block that contains the await
points.
In `@src/ui/dashpay/contact_requests.rs`:
- Around line 375-415: The code in update_names_from_profile uses doc.get(...)
which doesn't compile because Document fields must be accessed via
doc.properties(); update every Document access in update_names_from_profile (the
displayName, publicMessage, avatarUrl lookups) to call
doc.properties().get("fieldName") and then chain the same and_then(|v|
v.as_text()).map(...).filter(...)/as_deref() logic you already have; keep the
existing checks that only set from_display_name/to_display_name when None and
preserve the save via self.app_context.db.save_dashpay_profile(...) unchanged,
just passing the values derived from doc.properties().get(...) instead of
doc.get(...).
lklimek
reviewed
Feb 16, 2026
lklimek
requested changes
Feb 16, 2026
Contributor
lklimek
left a comment
lklimek
left a comment
There was a problem hiding this comment.
There are a few important bugs in the code, please check and address them.
The SearchFilter::Recent and SortOrder::DateAdded options existed in code but were missing from the combo box dropdowns, making them unreachable. Addresses CodeRabbit review feedback on PR #564. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
thepastaclaw
added a commit
to thepastaclaw/dash-evo-tool
that referenced
this pull request
Feb 17, 2026
1. CRITICAL: Fix BitOrAssign dropping tasks in fetch_unresolved_profiles()
- Changed from action |= AppAction::BackendTask(task) loop (which only
kept the last task) to collecting all tasks into a Vec and dispatching
via AppAction::BackendTasks with Concurrent execution mode.
2. HIGH: Remove dummy [0x02; 33] pubkey in load_payment_history()
- Changed PaymentRecord.to_address from Address to Option<Address>
- Historical records loaded from DB now use None instead of generating
a fake-but-valid P2PKH address from a dummy public key.
3. HIGH: Fix i64 as u64 wrapping for amounts (payments.rs and dashpay.rs)
- Added bounds checking with warning logs for negative amounts,
clamping to 0 instead of silently wrapping.
4. MEDIUM: Eliminate duplicate payment history logic in dashpay.rs
- LoadPaymentHistory task handler now calls the shared
payments::load_payment_history() and converts results, instead of
reimplementing the same logic with different return types.
5. MEDIUM: Fix N+1 database queries in resolve_names_from_local_cache()
- Pre-load all contacts once before the loop and build a HashMap
for O(1) lookups instead of calling load_dashpay_contacts() and
load_dashpay_profile() per request inside the loop.
6. MEDIUM: Fix created_at: None breaking filters in contacts_list.rs
- Contacts from DashPayContactsWithInfo results now get current
timestamp as fallback instead of None, so they work with
'Recent' filter and 'Date added' sort.
7. LOW: Fix 'failed' as failure reason in payment status
- Changed PaymentStatus::Failed to use descriptive 'Transaction failed'
string instead of echoing the literal status column value 'failed'.
8. LOW: Fix i64 as u64 for timestamps in payments.rs
- Added bounds checking with warning for negative timestamps,
consistent with the amount fix.
9. LOW: Both i64 as u64 locations fixed (dashpay.rs duplicate removed
entirely by fix dashpay#4, payments.rs fixed by fixes dashpay#3/dashpay#8).
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/ui/dashpay/contacts_list.rs (1)
1092-1097: In-memorycreated_atfallback to current time is acceptable but worth noting.When contacts arrive from
DashPayContactsWithInfo,created_atis set to "now" rather than a true creation timestamp from Platform. This acts as a "first seen locally" time. Sincesave_dashpay_contact(Line 1102) persists to the DB (which likely auto-generatescreated_at), subsequent loads from DB (Line 144) will use the stored value. The in-memory value is transient.If this "first seen" semantic is intentional, a brief comment would help future readers. If the
ContactDatafrom the backend carries a real creation timestamp, it would be better to use that instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/dashpay/contacts_list.rs` around lines 1092 - 1097, The in-memory mapping that sets ContactData.created_at to SystemTime::now() should either use the backend's real creation timestamp if DashPayContactsWithInfo/ContactData provides one, or be annotated to indicate it's a "first-seen locally" fallback; update the mapping code that constructs ContactData (where created_at is currently set using SystemTime::now()) to prefer the platform timestamp when present, otherwise set created_at to None or keep the now() fallback but add a concise comment explaining the "first-seen locally" semantics; also ensure this aligns with save_dashpay_contact behavior so DB persistence will supply the canonical created_at on subsequent loads.src/backend_task/dashpay.rs (1)
170-215: Payment history loading with contact name enrichment looks correct overall.Two observations:
- Line 178-179:
unwrap_or_default()silently swallows DB errors when loading contacts. If the DB call fails, every payment will show "Unknown (…)". Consider at least logging the failure:Proposed improvement
- let contacts = self - .db - .load_dashpay_contacts(&identity_id, &network_str) - .unwrap_or_default(); + let contacts = match self + .db + .load_dashpay_contacts(&identity_id, &network_str) + { + Ok(c) => c, + Err(e) => { + tracing::warn!("Failed to load contacts for name resolution: {}", e); + Vec::new() + } + };
- Lines 191-202: The contact lookup iterates all contacts for every payment record (O(records × contacts)) and calls
Identifier::from_byteson each contact per iteration. For small datasets this is fine, but if the list grows, pre-building aHashMap<Identifier, ...>fromcontactswould be more efficient.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend_task/dashpay.rs` around lines 170 - 215, The DB call in DashPayTask::LoadPaymentHistory uses self.db.load_dashpay_contacts(...).unwrap_or_default(), which swallows errors—replace that unwrap_or_default with proper error handling: call load_dashpay_contacts and on Err log the error (using the task’s logger) and fall back to an empty list so failures are visible; and optimize the per-record contact lookup by prebuilding a HashMap from Identifier (created once via Identifier::from_bytes on each contact) to the contact display string (username or display_name) before iterating records, then use that map to resolve contact_name when mapping records to produce BackendTaskSuccessResult::DashPayPaymentHistory.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/ui/dashpay/contact_requests.rs`:
- Around line 390-432: The code in update_names_from_profile uses doc.get(...)
which doesn't exist on Document; use the DocumentV0Getters trait's properties()
to access the underlying map and call .get(...) on that map instead. Replace
occurrences of doc.get("displayName"), doc.get("publicMessage"), and
doc.get("avatarUrl") with doc.properties().get("displayName") /
doc.properties().get("publicMessage") / doc.properties().get("avatarUrl") (and
keep the chain of .and_then(|v| v.as_text())/.filter(...) and cloning of
display_name unchanged), so incoming/outgoing request updates and the
save_dashpay_profile call use values pulled from doc.properties().get(...).
---
Nitpick comments:
In `@src/backend_task/dashpay.rs`:
- Around line 170-215: The DB call in DashPayTask::LoadPaymentHistory uses
self.db.load_dashpay_contacts(...).unwrap_or_default(), which swallows
errors—replace that unwrap_or_default with proper error handling: call
load_dashpay_contacts and on Err log the error (using the task’s logger) and
fall back to an empty list so failures are visible; and optimize the per-record
contact lookup by prebuilding a HashMap from Identifier (created once via
Identifier::from_bytes on each contact) to the contact display string (username
or display_name) before iterating records, then use that map to resolve
contact_name when mapping records to produce
BackendTaskSuccessResult::DashPayPaymentHistory.
In `@src/ui/dashpay/contacts_list.rs`:
- Around line 1092-1097: The in-memory mapping that sets ContactData.created_at
to SystemTime::now() should either use the backend's real creation timestamp if
DashPayContactsWithInfo/ContactData provides one, or be annotated to indicate
it's a "first-seen locally" fallback; update the mapping code that constructs
ContactData (where created_at is currently set using SystemTime::now()) to
prefer the platform timestamp when present, otherwise set created_at to None or
keep the now() fallback but add a concise comment explaining the "first-seen
locally" semantics; also ensure this aligns with save_dashpay_contact behavior
so DB persistence will supply the canonical created_at on subsequent loads.
Merges v1.0-dev into feat/dashpay-contacts-improvements. Conflict in contacts.rs: kept PR's parallelized profile fetching structure but applied the publicMessage field fix from #582 (publicMessage is the correct DashPay contract field, not bio).
merge: resolve v1.0-dev conflicts in #564
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/ui/dashpay/contact_requests.rs (2)
1113-1113:⚠️ Potential issue | 🟠 Major
unwrap()onselected_identitycan panic if the identity is cleared while the backend task is in flight.If the user switches or deselects their identity before the
DashPayContactRequestsresult arrives,self.selected_identitywill beNoneand this line will panic. Guard with an early return:Proposed fix
- let current_identity_id = self.selected_identity.as_ref().unwrap().identity.id(); + let Some(current_identity) = self.selected_identity.as_ref() else { + tracing::warn!("Received DashPayContactRequests result but no identity is selected"); + return; + }; + let current_identity_id = current_identity.identity.id();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/dashpay/contact_requests.rs` at line 1113, The code calls self.selected_identity.as_ref().unwrap().identity.id() (producing current_identity_id) and can panic if selected_identity becomes None while the backend task is in flight; update the DashPayContactRequests result handling to guard against None by returning early (or skipping processing) when self.selected_identity.is_none() or using if let Some(sel) = self.selected_identity.as_ref() to safely obtain sel.identity.id() before proceeding so no unwrap occurs.
467-488:⚠️ Potential issue | 🔴 CriticalMove
refresh()implementation into theScreenLikeimpl block.The
refresh()method at line 467 has a real implementation but is defined outside theimpl ScreenLikeblock. The framework dispatches refresh calls through the trait (seesrc/ui/mod.rslines 1030–1046), which invokes the empty default impl instead of your implementation. This makes the logic at line 467 unreachable dead code.Additionally,
refresh_on_arrival()currently bypasses therefresh()call entirely and only loads from the database. To follow the trait design, overriderefresh()within theimpl ScreenLikeblock with the implementation from line 467, and letrefresh_on_arrival()either call it or replicate its logic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/dashpay/contact_requests.rs` around lines 467 - 488, The concrete refresh implementation currently defined outside the impl ScreenLike block is never called; move the body of pub fn refresh(&mut self) (the logic that clears temporary state, auto-selects an identity using app_context.load_local_qualified_identities(), sets selected_identity and selected_identity_string, and calls load_requests_from_database() when selected_identity.is_some()) into the impl ScreenLike for this struct so it overrides the trait method; also update refresh_on_arrival() to call self.refresh() (or defer to the same logic) instead of directly loading from the DB to ensure the trait-dispatched refresh path is used.
🧹 Nitpick comments (4)
src/ui/wallets/wallets_screen/dialogs.rs (1)
171-171: Fix is correct; consider aligning withDashColorsfor consistency.The
egui::qualification is necessary sinceColor32has no directusein scope — this is the right fix.As a minor consistency note, this is the only remaining hardcoded
from_rgbcolor in a file that otherwise routes all error/warning colors through theDashColorstheme system (e.g.,DashColors::ERRORat line 196 for the general send error,DashColors::error_color(dark_mode)at line 925 for the private-key warning).🎨 Optional: use DashColors for consistency
- ui.colored_label(egui::Color32::from_rgb(255, 100, 100), error); + ui.colored_label(DashColors::error_color(dark_mode), error);Note:
dark_modeis already resolved inrender_send_dialog's caller context — if it isn't in scope at this point,DashColors::ERRORis the simplest drop-in.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/wallets/wallets_screen/dialogs.rs` at line 171, Replace the hardcoded egui::Color32::from_rgb usage in the ui.colored_label call with the project's DashColors theme to stay consistent: use DashColors::error_color(dark_mode) if dark_mode is available in this scope, otherwise use the simple drop-in DashColors::ERROR; update the ui.colored_label call (the call site using ui.colored_label(..., error)) to pass the selected DashColors value instead of egui::Color32::from_rgb.src/backend_task/dashpay/contacts.rs (2)
486-494: Result writeback is correct but could be simplified.Since
join_allpreserves future ordering (results[i] matches the i-th future, which corresponds tochunk[i]), you could zip results directly withchunk.iter_mut()instead of thefind-by-id scan. WithCHUNK_SIZE = 10this is not a performance concern — just a minor clarity improvement.Optional simplification
- for (contact_id, display_name, avatar_url, bio, username) in results { - if let Some(contact) = chunk.iter_mut().find(|c| c.identity_id == contact_id) { - contact.display_name = display_name; - contact.avatar_url = avatar_url; - contact.bio = bio; - contact.username = username; - } + for (contact, (_contact_id, display_name, avatar_url, bio, username)) in + chunk.iter_mut().zip(results) + { + contact.display_name = display_name; + contact.avatar_url = avatar_url; + contact.bio = bio; + contact.username = username; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend_task/dashpay/contacts.rs` around lines 486 - 494, The current loop re-finds each contact by identity_id when applying results; since join_all preserves order so results correspond to chunk order, replace the find-by-id scan in the loop that iterates over (contact_id, display_name, avatar_url, bio, username) in results with a zip over results and chunk.iter_mut() (referencing variables results, chunk and CHUNK_SIZE and the enclosing code that uses join_all), and assign display_name/avatar_url/bio/username directly to the paired mutable contact to simplify and clarify the writeback.
432-477: Enrichment failures are silently swallowed — add tracing for debuggability.Both the profile fetch (line 441) and the DPNS username fetch (line 469) discard errors via
if let Ok(...). If either query fails (network issues, malformed response, etc.), there's no log output, making it very hard to diagnose why a contact shows up without a display name or username in the UI.Consider adding
tracing::warn!(ordebug!) on the error branch so failures are observable:Proposed change (profile fetch example)
- if let Ok(results) = Document::fetch_many(sdk, profile_query).await + match Document::fetch_many(sdk, profile_query).await { + Ok(results) => { && let Some((_, Some(doc))) = results.into_iter().next() - { + { let props = doc.properties(); display_name = props .get("displayName") .and_then(|v| v.as_text()) .map(|s| s.to_string()); // ... avatar_url, bio + } + } + Err(e) => { + tracing::debug!("Failed to fetch profile for {}: {}", contact_id, e); + } }Apply the same pattern to the DPNS username fetch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend_task/dashpay/contacts.rs` around lines 432 - 477, The profile and DPNS document fetches currently swallow errors via the `if let Ok(...) = Document::fetch_many(sdk, ...)` pattern inside the `DocumentQuery::new(dashpay_contract, "profile")` and `DocumentQuery::new(dpns_contract, "domain")` blocks; change each to explicitly handle the `Err(e)` case and emit a `tracing::warn!` (or `tracing::debug!`) including context (e.g., contact_id, contract name like `dashpay_contract`/`dpns_contract`, and the error) when `Document::fetch_many` fails, and also log when `DocumentQuery::new(...)` itself returns `Err(e)` so both query-construction and fetch failures are visible (refer to `contact_id`, `sdk`, `profile_query`/`dpns_query`, `display_name`, and `username` to locate the relevant code).src/ui/dashpay/contact_requests.rs (1)
286-330: Profiles with nodisplayNamewill be re-fetched from Platform on every resolution cycle.When
load_dashpay_profilereturns a profile withdisplay_name: None,profiles_cachestoresSome(None). The check on line 311 (Some(Some(display_name))) won't match, and if the contacts cache also lacks the entry,foundstaysfalse, pushing the ID intounresolved_ids. After the Platform fetch returns the same profile (still nodisplayName),update_names_from_profile(line 402) won't setfrom_display_nameeither, so the nextresolve_names_from_local_cachecall repeats the cycle.Consider treating a successfully loaded (but empty) profile as "resolved" to avoid redundant backend fetches:
Suggested approach
for id in incoming_ids.iter().chain(outgoing_ids.iter()) { if !profiles_cache.contains_key(id) { - let display_name = self + let profile_loaded = self .app_context .db .load_dashpay_profile(id, &network_str) .ok() - .flatten() - .and_then(|p| p.display_name); - profiles_cache.insert(*id, display_name); + .flatten(); + // (profile_exists, display_name) + profiles_cache.insert(*id, (profile_loaded.is_some(), profile_loaded.and_then(|p| p.display_name))); } }Then adjust the resolution logic to treat
(true, None)as "found" (profile exists, no display name) vs(false, None)as truly unresolved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/dashpay/contact_requests.rs` around lines 286 - 330, profiles loaded via load_dashpay_profile are currently stored in profiles_cache as Option<String>, causing Some(None) to be treated as missing and re-fetched; change profiles_cache to record presence vs absence (e.g., HashMap<Identifier, Option<Option<String>>> or HashMap<Identifier, ProfilePresence> where ProfilePresence distinguishes "profile found but display_name None" from "no profile"). Populate it from the load_dashpay_profile result so a successful load becomes Some(Some(display_name)) or Some(None) (profile exists but no name), and then in the resolve loop in resolve_names_from_local_cache (the block iterating self.incoming_requests) treat a cached Some(None) as found (set found = true) so you do not push the id to unresolved_ids; also align update_names_from_profile to respect the same presence semantics when setting from_display_name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/ui/dashpay/contact_requests.rs`:
- Line 1113: The code calls
self.selected_identity.as_ref().unwrap().identity.id() (producing
current_identity_id) and can panic if selected_identity becomes None while the
backend task is in flight; update the DashPayContactRequests result handling to
guard against None by returning early (or skipping processing) when
self.selected_identity.is_none() or using if let Some(sel) =
self.selected_identity.as_ref() to safely obtain sel.identity.id() before
proceeding so no unwrap occurs.
- Around line 467-488: The concrete refresh implementation currently defined
outside the impl ScreenLike block is never called; move the body of pub fn
refresh(&mut self) (the logic that clears temporary state, auto-selects an
identity using app_context.load_local_qualified_identities(), sets
selected_identity and selected_identity_string, and calls
load_requests_from_database() when selected_identity.is_some()) into the impl
ScreenLike for this struct so it overrides the trait method; also update
refresh_on_arrival() to call self.refresh() (or defer to the same logic) instead
of directly loading from the DB to ensure the trait-dispatched refresh path is
used.
---
Duplicate comments:
In `@src/backend_task/dashpay/contacts.rs`:
- Around line 418-495: The async tasks capture a borrowed `sdk` reference which
can be used concurrently across awaits; clone the SDK before entering each async
block so each future owns its own SDK handle: inside the iterator closure that
creates the async move, create a local cloned SDK (e.g., `let sdk =
sdk.clone();` or `let sdk_clone = sdk.clone();`) and move that into the `async
move` block, then use that cloned variable in the `Document::fetch_many` calls;
update references in the async block to the cloned name so
`Document::fetch_many` no longer borrows the outer `sdk`.
In `@src/ui/dashpay/contact_requests.rs`:
- Around line 392-434: The method update_names_from_profile is calling
doc.get(...) which doesn't exist on Document; use the DocumentV0Getters-provided
properties() accessor before field lookup. Replace each
doc.get("...").and_then(|v| v.as_text()) usage (e.g., for "displayName",
"publicMessage", "avatarUrl") with doc.properties().get("...").and_then(|v|
v.as_text()) (and keep the same chaining and cloning logic), and update any
other occurrences in update_names_from_profile to use doc.properties()
consistently so the code compiles.
---
Nitpick comments:
In `@src/backend_task/dashpay/contacts.rs`:
- Around line 486-494: The current loop re-finds each contact by identity_id
when applying results; since join_all preserves order so results correspond to
chunk order, replace the find-by-id scan in the loop that iterates over
(contact_id, display_name, avatar_url, bio, username) in results with a zip over
results and chunk.iter_mut() (referencing variables results, chunk and
CHUNK_SIZE and the enclosing code that uses join_all), and assign
display_name/avatar_url/bio/username directly to the paired mutable contact to
simplify and clarify the writeback.
- Around line 432-477: The profile and DPNS document fetches currently swallow
errors via the `if let Ok(...) = Document::fetch_many(sdk, ...)` pattern inside
the `DocumentQuery::new(dashpay_contract, "profile")` and
`DocumentQuery::new(dpns_contract, "domain")` blocks; change each to explicitly
handle the `Err(e)` case and emit a `tracing::warn!` (or `tracing::debug!`)
including context (e.g., contact_id, contract name like
`dashpay_contract`/`dpns_contract`, and the error) when `Document::fetch_many`
fails, and also log when `DocumentQuery::new(...)` itself returns `Err(e)` so
both query-construction and fetch failures are visible (refer to `contact_id`,
`sdk`, `profile_query`/`dpns_query`, `display_name`, and `username` to locate
the relevant code).
In `@src/ui/dashpay/contact_requests.rs`:
- Around line 286-330: profiles loaded via load_dashpay_profile are currently
stored in profiles_cache as Option<String>, causing Some(None) to be treated as
missing and re-fetched; change profiles_cache to record presence vs absence
(e.g., HashMap<Identifier, Option<Option<String>>> or HashMap<Identifier,
ProfilePresence> where ProfilePresence distinguishes "profile found but
display_name None" from "no profile"). Populate it from the load_dashpay_profile
result so a successful load becomes Some(Some(display_name)) or Some(None)
(profile exists but no name), and then in the resolve loop in
resolve_names_from_local_cache (the block iterating self.incoming_requests)
treat a cached Some(None) as found (set found = true) so you do not push the id
to unresolved_ids; also align update_names_from_profile to respect the same
presence semantics when setting from_display_name.
In `@src/ui/wallets/wallets_screen/dialogs.rs`:
- Line 171: Replace the hardcoded egui::Color32::from_rgb usage in the
ui.colored_label call with the project's DashColors theme to stay consistent:
use DashColors::error_color(dark_mode) if dark_mode is available in this scope,
otherwise use the simple drop-in DashColors::ERROR; update the ui.colored_label
call (the call site using ui.colored_label(..., error)) to pass the selected
DashColors value instead of egui::Color32::from_rgb.
lklimek
approved these changes
Feb 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements