fix: monero silently ignoring wallet opens (#3621)
What the developer wrote
Authored by cyan
fix: monero silently ignoring wallet opens (#3621)
What changed, and why it matters
This patch fixes a bug where Cake Wallet's Monero module could skip actually opening a wallet and instead leave the user looking at the previously opened wallet without any warning. The old code tracked the 'last opened wallet' path and, if the requested path matched it, simply did nothing. That state tracking was unreliable and could become stale, so the app might appear to load a wallet while silently staying on the wrong one. The fix removes that shortcut and always performs the real open-wallet work, including error checks.
Treat as a functional bug fix with possible security-adjacent side effects. Review whether any user-facing flow relied on the `_lastOpenedWallet` shortcut and confirm that removing it does not regress performance or cause double-opens. Consider whether a stale `currentWallet` could have led to incorrect receive addresses or transaction signing in practice, and add regression tests for repeated wallet opens.
Security signals we found
State confusion / stale-cache bug: relying on `_lastOpenedWallet` could cause the app to use the wrong wallet object
Silent failure: a wallet open request could return successfully without actually opening the requested wallet
UI/app-layer integrity issue: user may believe they are operating wallet A while the backend still holds wallet B
No cryptographic weakness or memory-safety bug in the diff itself
Evidence from the diff
The change removes the _lastOpenedWallet global and all assignments to it. loadWallet previously short-circuited when path == _lastOpenedWallet, returning without verifying or reopening the wallet. The patch makes loadWallet always store the current wallet, clear txhistory, query the wallet device type, run WalletManager_openWallet in an isolate, check the returned wallet’s status, set up background sync, and update currentWallet/openedWalletsByPath. This prevents stale state from causing a silent no-op load. The commit title frames it as ‘silently ignoring wallet opens’.
Changed components
cw_monero/lib/api/wallet_manager.dartMonero wallet load/restore/create flowsIn-memory wallet state (`currentWallet`, `openedWalletsByPath`)Inspect captured patch +54 / −63
diff --git a/cw_monero/lib/api/wallet_manager.dart b/cw_monero/lib/api/wallet_manager.dart
index 85bc7729..9de58c68 100644
--- a/cw_monero/lib/api/wallet_manager.dart
+++ b/cw_monero/lib/api/wallet_manager.dart
@@ -101,7 +101,6 @@ void createWallet(
currentWallet!.setCacheAttribute(key: "cakewallet.passphrase", value: passphrase);
currentWallet!.store(path: path);
openedWalletsByPath[path] = currentWallet!;
- _lastOpenedWallet = path;
}
bool isWalletExist({required String path}) {
@@ -147,7 +146,6 @@ void restoreWalletFromSeedSync(
openedWalletsByPath[path] = currentWallet!;
currentWallet!.store(path: path);
- _lastOpenedWallet = path;
}
void storePassphrase({required String path, required String passphrase}) {
@@ -220,7 +218,6 @@ void restoreWalletFromKeys(
currentWallet = newW;
openedWalletsByPath[path] = currentWallet!;
- _lastOpenedWallet = path;
}
// English only, because normalization.
@@ -310,11 +307,8 @@ void restoreWalletFromSpendKeySync(
setupBackgroundSync(password, currentWallet!);
openedWalletsByPath[path] = currentWallet!;
- _lastOpenedWallet = path;
}
-String _lastOpenedWallet = "";
-
Future<void> restoreWalletFromHardwareWallet(
{required String path,
required String password,
@@ -339,7 +333,6 @@ Future<void> restoreWalletFromHardwareWallet(
currentWallet = newW;
currentWallet!.store(path: path);
- _lastOpenedWallet = path;
openedWalletsByPath[path] = currentWallet!;
}
@@ -351,72 +344,70 @@ Future<void> loadWallet({required String path, required String password, int net
currentWallet = openedWalletsByPath[path]!;
return;
}
- if (currentWallet == null || path != _lastOpenedWallet) {
- if (currentWallet != null) {
- final addr = currentWallet!.ffiAddress();
- Isolate.run(() {
- monero.Wallet_store(Pointer.fromAddress(addr));
- });
- }
- txhistory = null;
- /// Get the device type
- /// 0: Software Wallet
- /// 1: Ledger
- /// 2: Trezor
- var deviceType = 0;
+ if (currentWallet != null) {
+ final addr = currentWallet!.ffiAddress();
+ Isolate.run(() {
+ monero.Wallet_store(Pointer.fromAddress(addr));
+ });
+ }
+ txhistory = null;
- if (Platform.isAndroid || Platform.isIOS) {
- deviceType = wmPtr.queryWalletDevice(
- keysFileName: "$path.keys",
- password: password,
- kdfRounds: 1,
- );
- final status = wmPtr.errorString();
- if (status != "") {
- printV("loadWallet:" + status);
- // This is most likely closeWallet call leaking error. This is fine.
- if (status.contains("failed to save file")) {
- printV("loadWallet: error leaked: $status");
- deviceType = 0;
- } else {
- throw WalletOpeningException(message: status);
- }
+ /// Get the device type
+ /// 0: Software Wallet
+ /// 1: Ledger
+ /// 2: Trezor
+ var deviceType = 0;
+
+ if (Platform.isAndroid || Platform.isIOS) {
+ deviceType = wmPtr.queryWalletDevice(
+ keysFileName: "$path.keys",
+ password: password,
+ kdfRounds: 1,
+ );
+ final status = wmPtr.errorString();
+ if (status != "") {
+ printV("loadWallet:" + status);
+ // This is most likely closeWallet call leaking error. This is fine.
+ if (status.contains("failed to save file")) {
+ printV("loadWallet: error leaked: $status");
+ deviceType = 0;
+ } else {
+ throw WalletOpeningException(message: status);
}
- } else {
- deviceType = 0;
}
+ } else {
+ deviceType = 0;
+ }
- if (deviceType == 1) {
- if (gLedger == null) {
- throw Exception("Tried to open a ledger wallet with no ledger connected");
- }
- enableLedgerExchange(gLedger!);
+ if (deviceType == 1) {
+ if (gLedger == null) {
+ throw Exception("Tried to open a ledger wallet with no ledger connected");
}
+ enableLedgerExchange(gLedger!);
+ }
- final addr = wmPtr.ffiAddress();
- final newWptrAddr = await Isolate.run(() {
- return monero.WalletManager_openWallet(Pointer.fromAddress(addr),
- path: path, password: password)
- .address;
- });
-
- final newW = MoneroWallet(Pointer.fromAddress(newWptrAddr));
+ final addr = wmPtr.ffiAddress();
+ final newWptrAddr = await Isolate.run(() {
+ return monero.WalletManager_openWallet(Pointer.fromAddress(addr),
+ path: path, password: password)
+ .address;
+ });
- int status = newW.status();
- if (status != 0) {
- final err = newW.errorString();
- printV("loadWallet:" + err);
- throw WalletOpeningException(message: err);
- }
- if (deviceType == 0) {
- setupBackgroundSync(password, newW);
- }
+ final newW = MoneroWallet(Pointer.fromAddress(newWptrAddr));
- currentWallet = newW;
- _lastOpenedWallet = path;
- openedWalletsByPath[path] = currentWallet!;
+ int status = newW.status();
+ if (status != 0) {
+ final err = newW.errorString();
+ printV("loadWallet:" + err);
+ throw WalletOpeningException(message: err);
+ }
+ if (deviceType == 0) {
+ setupBackgroundSync(password, newW);
}
+
+ currentWallet = newW;
+ openedWalletsByPath[path] = currentWallet!;
}
void setupBackgroundSync(String password, Wallet2Wallet wallet) {
Why this scored 49/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.