Skip to content

Commit 5ee2efc

Browse files
committed
Fix error handling gaps in wallet type conversion and downgrade flow
Surface setWalletType errors to the user on both Android and iOS instead of silently swallowing them. Add navigation after xpub paste import on iOS. In Rust, propagate keychain and DB errors from downgrade check instead of swallowing them, add MissingMetadata error variant for the correct semantic, and propagate get_all errors in duplicate detection.
1 parent 4bab656 commit 5ee2efc

File tree

5 files changed

+53
-35
lines changed

5 files changed

+53
-35
lines changed

android/app/src/main/java/org/bitcoinppl/cove/MainActivity.kt

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,15 @@ private fun GlobalAlertDialog(
437437
onDismiss()
438438
try {
439439
app.rust.setWalletType(walletId, WalletType.COLD)
440-
} catch (_: Exception) {
440+
} catch (e: Exception) {
441+
Log.e("GlobalAlert", "Failed to set wallet type to cold", e)
442+
app.alertState =
443+
TaggedItem(
444+
AppAlertState.General(
445+
title = "Error",
446+
message = e.message ?: "Failed to convert wallet",
447+
),
448+
)
441449
}
442450
}) { Text("Use with Hardware Wallet") }
443451
}

ios/Cove/CoveApp.swift

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,18 @@ struct CoveApp: App {
8585
}
8686

8787
Button("Use with Hardware Wallet") {
88-
app.alertState = .none
89-
try? app.rust.setWalletType(id: walletId, walletType: .cold)
88+
do {
89+
try app.rust.setWalletType(id: walletId, walletType: .cold)
90+
app.alertState = .none
91+
} catch {
92+
Log.error("Failed to set wallet type to cold: \(error)")
93+
DispatchQueue.main.async {
94+
app.alertState = .init(.general(
95+
title: "Error",
96+
message: error.localizedDescription
97+
))
98+
}
99+
}
90100
}
91101

92102
Button("Use as Watch Only", role: .cancel) {
@@ -194,6 +204,7 @@ struct CoveApp: App {
194204
do {
195205
let wallet = try Wallet.newFromXpub(xpub: text)
196206
try app.rust.selectWallet(id: wallet.id())
207+
app.resetRoute(to: .selectedWallet(wallet.id()))
197208
} catch {
198209
DispatchQueue.main.async {
199210
app.alertState = .init(

rust/src/manager/import_wallet_manager.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ pub enum ImportWalletError {
6262
#[error("wallet already exists")]
6363
WalletAlreadyExists(WalletId),
6464

65+
#[error("wallet metadata missing for existing wallet")]
66+
MissingMetadata(WalletId),
67+
6568
#[error("failed to save wallet: {0}")]
6669
Database(#[from] database::Error),
6770

@@ -137,7 +140,7 @@ impl RustImportWalletManager {
137140
let mut metadata = Database::global()
138141
.wallets
139142
.get(&id, network, mode)?
140-
.ok_or_else(|| ImportWalletError::WalletAlreadyExists(id.clone()))?;
143+
.ok_or_else(|| ImportWalletError::MissingMetadata(id.clone()))?;
141144

142145
let descriptors =
143146
mnemonic.clone().into_descriptors(None, network, metadata.address_type);

rust/src/manager/wallet_manager.rs

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -258,11 +258,8 @@ impl RustWalletManager {
258258
let reconciler = MessageSender::new(sender.clone());
259259
let mut deferred = DeferredSender::new(reconciler.clone());
260260

261-
let wallet = {
262-
let mut wallet = Wallet::try_load_persisted(id.clone())?;
263-
downgrade_and_notify_if_needed(&mut wallet.metadata, &mut deferred);
264-
wallet
265-
};
261+
let mut wallet = Wallet::try_load_persisted(id.clone())?;
262+
wallet.metadata = downgrade_and_notify_if_needed(wallet.metadata, &mut deferred)?;
266263

267264
let metadata = Database::global()
268265
.wallets
@@ -1353,38 +1350,41 @@ impl Drop for RustWalletManager {
13531350
/// If a hot wallet's private key is missing from the keychain, downgrade it to
13541351
/// watch-only and queue a `HotWalletKeyMissing` notification so the UI can alert the user
13551352
fn downgrade_and_notify_if_needed(
1356-
metadata: &mut WalletMetadata,
1353+
metadata: WalletMetadata,
13571354
deferred: &mut DeferredSender<Message>,
1358-
) {
1355+
) -> Result<WalletMetadata, Error> {
13591356
if metadata.wallet_type != WalletType::Hot {
1360-
return;
1357+
return Ok(metadata);
13611358
}
13621359

13631360
let has_private_key = match Keychain::global().get_wallet_key(&metadata.id) {
13641361
Ok(Some(_)) => true,
13651362
Ok(None) => false,
13661363
Err(error) => {
1367-
warn!("failed to read hot wallet private key for {}: {error}", metadata.id);
1368-
false
1364+
return Err(Error::UnknownError(format!(
1365+
"failed to read keychain for {}: {error}",
1366+
metadata.id
1367+
)));
13691368
}
13701369
};
13711370

13721371
if has_private_key {
1373-
return;
1372+
return Ok(metadata);
13741373
}
13751374

1376-
warn!(
1377-
"hot wallet {} is missing private key in keychain, downgrading to watch-only",
1378-
metadata.id
1379-
);
1380-
metadata.wallet_type = WalletType::WatchOnly;
1381-
metadata.hardware_metadata = None;
1375+
let id = metadata.id.clone();
1376+
warn!("hot wallet {id} is missing private key in keychain, downgrading to watch-only",);
13821377

1383-
if let Err(error) = Database::global().wallets.update_wallet_metadata(metadata.clone()) {
1384-
error!("failed to persist watch-only downgrade for {}: {error}", metadata.id);
1385-
}
1378+
let mut updated = metadata;
1379+
updated.wallet_type = WalletType::WatchOnly;
1380+
updated.hardware_metadata = None;
1381+
1382+
Database::global().wallets.update_wallet_metadata(updated.clone()).map_err(|e| {
1383+
Error::UnknownError(format!("failed to persist watch-only downgrade for {id}: {e}",))
1384+
})?;
13861385

1387-
deferred.queue(Message::HotWalletKeyMissing(metadata.id.clone()));
1386+
deferred.queue(Message::HotWalletKeyMissing(updated.id.clone()));
1387+
Ok(updated)
13881388
}
13891389

13901390
/// Get the public descriptor content for export

rust/src/wallet.rs

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -297,17 +297,13 @@ impl Wallet {
297297

298298
let existing = database
299299
.wallets
300-
.get_all(network, mode)
301-
.map(|wallets| {
302-
wallets
303-
.into_iter()
304-
.filter_map(|wm| {
305-
let fp = wm.master_fingerprint.as_ref()?;
306-
(fp.as_ref() == &fingerprint).then_some(wm)
307-
})
308-
.next()
300+
.get_all(network, mode)?
301+
.into_iter()
302+
.filter_map(|wm| {
303+
let fp = wm.master_fingerprint.as_ref()?;
304+
(fp.as_ref() == &fingerprint).then_some(wm)
309305
})
310-
.unwrap_or(None);
306+
.next();
311307

312308
if let Some(mut existing_metadata) = existing {
313309
if existing_metadata.wallet_type != WalletType::WatchOnly {

0 commit comments

Comments
 (0)