Skip to content

fix: top up with qr code wasnt working #410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pauldelucia merged 3 commits into from
Jul 17, 2025
Merged

fix: top up with qr code wasnt working #410

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8ef61e2
fix: top up with qr code wasnt working
pauldelucia Jul 16, 2025
7be749b
fmt and clippy
pauldelucia Jul 16, 2025
7373c3f
coderabbit
pauldelucia Jul 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/backend_task/tokens/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,7 @@ impl AppContext {
0 => TokenTradeMode::NotTradeable,
_ => TokenTradeMode::NotTradeable, // Default to NotTradeable for any unknown value
};

token_config_v0.marketplace_rules = TokenMarketplaceRules::V0(TokenMarketplaceRulesV0 {
trade_mode,
trade_mode_change_rules: marketplace_rules,
Expand Down
92 changes: 38 additions & 54 deletions src/ui/identities/top_up_identity_screen/by_wallet_qr_code.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,9 @@ use std::sync::Arc;

impl TopUpIdentityScreen {
fn render_qr_code(&mut self, ui: &mut egui::Ui, amount: f64) -> Result<(), String> {
Comment thread
pauldelucia marked this conversation as resolved.
Resolved
let (address, _should_check_balance) = {
// Scope the write lock to ensure it's dropped before calling `start_balance_check`.

let address = {
if let Some(wallet_guard) = self.wallet.as_ref() {
// Get the receive address
// Get the receive address from the selected wallet
if self.funding_address.is_none() {
let mut wallet = wallet_guard.write().unwrap();
let receive_address = wallet.receive_address(
Expand All @@ -23,49 +21,33 @@ impl TopUpIdentityScreen {
Some(&self.app_context),
)?;

if let Some(has_address) = self.core_has_funding_address {
if !has_address {
self.app_context
.core_client
.read()
.expect("Core client lock was poisoned")
.import_address(
&receive_address,
Some("Managed by Dash Evo Tool"),
Some(false),
)
.map_err(|e| e.to_string())?;
}
self.funding_address = Some(receive_address);
} else {
let info = self
.app_context
.core_client
.read()
.expect("Core client lock was poisoned")
.get_address_info(&receive_address)
// Import address to Core if needed for monitoring
let core_client = self
.app_context
.core_client
.read()
.expect("Core client lock was poisoned");

Copilot AI Jul 16, 2025

Copy link

Choose a reason for hiding this comment

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

Using expect on a lock acquisition can cause a panic if the lock is poisoned. Consider propagating this error gracefully or handling lock poisoning to avoid unexpected crashes.

Suggested change
.expect("Core client lock was poisoned");
.map_err(|_| "Core client lock was poisoned".to_string())?;

Copilot uses AI. Check for mistakes.

let info = core_client
.get_address_info(&receive_address)
.map_err(|e| e.to_string())?;

if !(info.is_watchonly || info.is_mine) {
core_client
.import_address(
&receive_address,
Some("Managed by Dash Evo Tool"),
Some(false),
)
.map_err(|e| e.to_string())?;

if !(info.is_watchonly || info.is_mine) {
self.app_context
.core_client
.read()
.expect("Core client lock was poisoned")
.import_address(
&receive_address,
Some("Managed by Dash Evo Tool"),
Some(false),
)
.map_err(|e| e.to_string())?;
}
self.funding_address = Some(receive_address);
self.core_has_funding_address = Some(true);
}

// Extract the address to return it outside this scope
(self.funding_address.as_ref().unwrap().clone(), true)
drop(core_client);

self.funding_address = Some(receive_address.clone());
receive_address
Comment thread
pauldelucia marked this conversation as resolved.
Resolved
} else {
(self.funding_address.as_ref().unwrap().clone(), false)
self.funding_address.as_ref().unwrap().clone()
}
} else {
return Err("No wallet selected".to_string());
Expand Down Expand Up @@ -113,10 +95,6 @@ impl TopUpIdentityScreen {
// Extract the step from the RwLock to minimize borrow scope
let step = *self.step.read().unwrap();

let Ok(amount_dash) = self.funding_amount.parse::<f64>() else {
return AppAction::None;
};

ui.heading(
format!(
"{}. Select how much you would like to transfer?",
Expand All @@ -130,8 +108,17 @@ impl TopUpIdentityScreen {
self.top_up_funding_amount_input(ui);

let response = ui.vertical_centered(|ui| {
if let Err(e) = self.render_qr_code(ui, amount_dash) {
self.error_message = Some(e);
// Only try to render QR code if we have a valid amount
if let Ok(amount_dash) = self.funding_amount.parse::<f64>() {
if amount_dash > 0.0 {
if let Err(e) = self.render_qr_code(ui, amount_dash) {
self.error_message = Some(e);
}
} else {
ui.label("Please enter an amount greater than 0");
}
} else if !self.funding_amount.is_empty() {
ui.label("Please enter a valid amount");
}

ui.add_space(20.0);
Expand Down Expand Up @@ -162,7 +149,7 @@ impl TopUpIdentityScreen {
.unwrap_or_default();
let identity_input = IdentityTopUpInfo {
qualified_identity: self.identity.clone(),
wallet: Arc::clone(selected_wallet), // Clone the Arc reference
wallet: Arc::clone(selected_wallet),
identity_funding_method: TopUpIdentityFundingMethod::FundWithUtxo(
utxo,
tx_out,
Expand All @@ -175,17 +162,14 @@ impl TopUpIdentityScreen {
let mut step = self.step.write().unwrap();
*step = WalletFundedScreenStep::WaitingForAssetLock;

// Create the backend task to register the identity
return AppAction::BackendTask(BackendTask::IdentityTask(
IdentityTask::TopUpIdentity(identity_input),
));
}
}
WalletFundedScreenStep::ReadyToCreate => {}
WalletFundedScreenStep::WaitingForAssetLock => {
ui.heading(
"=> Waiting for Core Chain to produce proof of transfer of funds. <=",
);
ui.heading("=> Waiting for Core Chain to produce proof of transfer of funds. <=");
ui.add_space(20.0);
ui.label("NOTE: If this gets stuck, the funds were likely either transferred to the wallet or asset locked,\nand you can use the funding method selector in step 1 to change the method and use those funds to complete the process.");
}
Expand Down
25 changes: 19 additions & 6 deletions src/ui/identities/top_up_identity_screen/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ pub struct TopUpIdentityScreen {
step: Arc<RwLock<WalletFundedScreenStep>>,
funding_asset_lock: Option<(Transaction, AssetLockProof, Address)>,
wallet: Option<Arc<RwLock<Wallet>>>,
core_has_funding_address: Option<bool>,
funding_address: Option<Address>,
funding_method: Arc<RwLock<FundingMethod>>,
funding_amount: String,
Expand All @@ -55,7 +54,6 @@ impl TopUpIdentityScreen {
step: Arc::new(RwLock::new(WalletFundedScreenStep::ChooseFundingMethod)),
funding_asset_lock: None,
wallet: None,
core_has_funding_address: None,
funding_address: None,
funding_method: Arc::new(RwLock::new(FundingMethod::NoSelection)),
funding_amount: "".to_string(),
Expand Down Expand Up @@ -490,11 +488,26 @@ impl ScreenLike for TopUpIdentityScreen {

if funding_method == FundingMethod::UseWalletBalance
|| funding_method == FundingMethod::UseUnusedAssetLock
|| funding_method == FundingMethod::AddressWithQRCode
{
ui.heading(format!(
"{}. Choose the wallet to use to top up this identity.",
step_number
));
ui.horizontal(|ui| {
ui.heading(format!(
"{}. Choose the wallet to use to top up this identity.",
step_number
));
ui.add_space(10.0);

// Add info icon with hover tooltip
crate::ui::helpers::info_icon_button(
ui,
"The selected wallet will be used to:\n\n\
• Generate a receive address for the QR code\n\
• Hold the private keys needed to create the asset lock transaction\n\
• Sign and broadcast the transaction to top up your identity\n\n\
The wallet must have control over the funds to create the asset lock \
that credits your identity on the Dash Platform.",

Copilot AI Jul 16, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] The long tooltip string makes the UI code verbose and harder to read. Consider extracting this text into a named constant or external resource for clarity and easier localization.

Suggested change
"The selected wallet will be used to:\n\n\
• Generate a receive address for the QR code\n\
• Hold the private keys needed to create the asset lock transaction\n\
• Sign and broadcast the transaction to top up your identity\n\n\
The wallet must have control over the funds to create the asset lock \
that credits your identity on the Dash Platform.",
TOOLTIP_TEXT,

Copilot uses AI. Check for mistakes.
);
});
step_number += 1;

ui.add_space(10.0);
Expand Down