fix: register dpns names screen

[pauldelucia]

Summary by CodeRabbit

Release Notes

• New Features

  • Enhanced wallet management in the registration screen with new fields for wallet selection, password handling, and error messaging.
  • Improved network status checks and user feedback in the network chooser interface.

• Bug Fixes

  • Streamlined rendering logic for identity selection to improve user experience.

• Refactor

  • Code organization improvements across multiple files for better readability and clarity without altering functionality.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The pull request introduces several changes across multiple files, primarily focusing on code organization, readability improvements, and enhancements to wallet-related functionalities. Modifications include refactoring the run_core_task method for better readability, reordering import statements, and adding new fields and methods to the RegisterDpnsNameScreen struct. Additionally, updates to the NetworkChooserScreen enhance the handling of network statuses. Overall, the changes aim to maintain existing logic while improving code structure and preparing for future functionality.

Changes

File Path	Change Summary
src/backend_task/core/mod.rs	Refactored run_core_task method for improved readability; no change in logic or functionality.
src/backend_task/core/start_dash_qt.rs	Reordered import statements for better organization; no change in functionality or logic of the start_dash_qt function.
src/backend_task/identity/load_identity.rs	Added import for WalletSeedHash and introduced a new variable wallet_seed_hash: Option<(WalletSeedHash, u32)> without further usage; no change in existing logic or control flow.
src/ui/components/top_panel.rs	Reordered import statements and reformatted code for better readability in add_connection_indicator and add_top_panel functions; no change in functionality.
src/ui/identities/register_dpns_name_screen.rs	Updated RegisterDpnsNameScreen struct with new fields for wallet management; modified UI rendering logic and added new methods for wallet unlock functionality; introduced is_contested_name function.
src/ui/network_chooser_screen.rs	Adjusted check_network_status method for online status checks; refined render_network_row method logic; clarified usage context for check_core_status function.

Possibly related PRs

• feat: register usernames #3: Changes in the QualifiedIdentity struct related to identity management.
• fix: active contest vote tallies were not being updated with refresh button #6: Modifications to the RegisterDpnsNameScreen, part of the identity management flow.
• feat: choose mn to vote with #8: Changes in the DPNSContestedNamesScreen, relevant to identity management context.
• feat: refresh identities #14: Introduction of a new task type related to identity refreshing.
• feat: dpns subscreens #18: Integration of the DPNSSubscreen enum into the DPNSContestedNamesScreen.
• feat: view more keys pop-up in identities screen #25: Enhancements to the IdentitiesScreen for managing identities.

Suggested reviewers

• QuantumExplorer
• ogabrielides

🐰 In the code we hop and play,
With imports neat, we clear the way.
Wallets now have room to grow,
In screens where identities flow.
A dash of change, a sprinkle bright,
Our code now shines, a pure delight! 🌟

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[pauldelucia]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (4)

src/ui/network_chooser_screen.rs (1)

Line range hint 162-169: Consider improving timestamp handling and constants.

A few suggestions to enhance maintainability and robustness:

1. Extract the timestamp calculation into a helper method to reduce code duplication
2. Use unwrap_or_default() instead of expect() for more graceful error handling
3. Define timing constants at the struct level

Here's a suggested implementation:

 pub struct NetworkChooserScreen {
     // ... existing fields ...
+    const QUICK_RECHECK_DELAY: Duration = Duration::from_secs(1);
+    const NORMAL_RECHECK_DELAY: Duration = Duration::from_secs(5);
 }

 impl NetworkChooserScreen {
+    fn calculate_next_check_time(&self, delay: Duration) -> TimestampMillis {
+        SystemTime::now()
+            .duration_since(UNIX_EPOCH)
+            .unwrap_or_default()
+            .checked_add(delay)
+            .unwrap_or_default()
+            .as_millis() as u64
+    }

     fn render_network_row(&mut self, ui: &mut Ui, network: Network, name: &str) -> AppAction {
         // ... in the network selection handler ...
-        self.recheck_time = Some(
-            (SystemTime::now()
-                .duration_since(UNIX_EPOCH)
-                .expect("Time went backwards")
-                + Duration::from_secs(1))
-            .as_millis() as u64,
-        );
+        self.recheck_time = Some(self.calculate_next_check_time(Self::QUICK_RECHECK_DELAY));

         // ... in the Start button handler ...
-        self.recheck_time = Some(
-            (SystemTime::now()
-                .duration_since(UNIX_EPOCH)
-                .expect("Time went backwards")
-                + Duration::from_secs(5))
-            .as_millis() as u64,
-        );
+        self.recheck_time = Some(self.calculate_next_check_time(Self::NORMAL_RECHECK_DELAY));
     }
 }

src/ui/identities/register_dpns_name_screen.rs (3)

Line range hint 172-176: Improve error handling by associating errors with their source

The current implementation in display_message updates register_dpns_name_status to an error state based on any error message, which may originate from unrelated processes. This could lead to incorrect UI behavior if other components trigger error messages.

Consider modifying the error handling to associate error messages with their originating tasks or actions to ensure that the UI reflects the correct status.

---

75-79: Simplify selected_wallet initialization

You can simplify the initialization of selected_wallet in the constructor by using the cloned() method directly.

Apply this diff to streamline the code:

-let selected_wallet = if let Some(wallet) = app_context.wallets.read().unwrap().first() {
-    Some(wallet.clone())
-} else {
-    None
-};
+let selected_wallet = app_context.wallets.read().unwrap().first().cloned();

---

357-385: Reduce code duplication in ScreenWithWalletUnlock methods

The implementation of ScreenWithWalletUnlock contains repetitive getter and setter methods. Consider using default method implementations in the trait or leveraging macros to eliminate duplication and improve maintainability.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between d099e04 and 5e19466.

📒 Files selected for processing (6)

• src/backend_task/core/mod.rs (1 hunks)
• src/backend_task/core/start_dash_qt.rs (2 hunks)
• src/backend_task/identity/load_identity.rs (1 hunks)
• src/ui/components/top_panel.rs (3 hunks)
• src/ui/identities/register_dpns_name_screen.rs (7 hunks)
• src/ui/network_chooser_screen.rs (1 hunks)

✅ Files skipped from review due to trivial changes (4)

• src/backend_task/core/mod.rs
• src/backend_task/core/start_dash_qt.rs
• src/backend_task/identity/load_identity.rs
• src/ui/components/top_panel.rs

🔇 Additional comments (1)

src/ui/network_chooser_screen.rs (1)

160-161: LGTM! The task construction is clear and well-structured.

The code change maintains the existing functionality while improving readability by using a more explicit construction of the StartDashQT task with the network parameter.

Let's verify the task handling in the core module:

✅ Verification successful

Let me gather more information about the start_dash_qt implementation to ensure it properly handles the network parameter.

---

Let me check the implementation details of the start_dash_qt function to verify it properly handles the network parameter.

---

The StartDashQT task construction and handling is properly implemented and verified

The code change is correct and well-integrated:

• The StartDashQT task correctly passes the network parameter from the UI
• The core task handler properly routes it to start_dash_qt implementation
• The start_dash_qt function fully supports the network parameter by:
  • Validating the network type (mainnet/testnet)
  • Using appropriate network-specific configuration files
  • Handling platform-specific paths for the Dash-Qt binary

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the CoreTask::StartDashQT handler implementation
# Expected: Find the corresponding task handler that processes this network parameter

# Search for StartDashQT variant definition and its handler
ast-grep --pattern 'enum CoreTask {
  $$$
  StartDashQT($_)
  $$$
}'

# Search for the handler implementation
rg -A 5 "CoreTask::StartDashQT"

Length of output: 1303

---

Script:

#!/bin/bash
# Search for the start_dash_qt implementation
ast-grep --pattern 'fn start_dash_qt($_) {
  $$$
}'

# Also search for any direct references to this method
rg "start_dash_qt"

Length of output: 305

---

Script:

#!/bin/bash
# Get the implementation of start_dash_qt
cat src/backend_task/core/start_dash_qt.rs

Length of output: 2273