fix balance being stale cuz it's overriden by an old value
What changed, and why it matters
This commit fixes a bug where a Bitcoin wallet's displayed balance could become stale or be overwritten with an outdated value. The changes make balance updates copy the new value instead of sharing a reference, recalculate balances per account, avoid updating balances when the network connection is lost, and make address validation non-blocking so it doesn't freeze the app. There is no direct evidence this was exploited or treated as a security vulnerability by the vendor.
Review whether stale or incorrect balances could mislead users during transactions; consider whether the fix should be backported and whether a CVE or security advisory is warranted if balance manipulation could affect user funds. No immediate exploit path is evident from the diff alone.
Security signals we found
Balance display correctness bug fixed
Reference sharing replaced with explicit copy to avoid stale shared-mutable state
Network disconnect guard added before persisting fetched balance
Per-account balance isolation introduced
No explicit security framing in commit message or diff
Evidence from the diff
The patch addresses balance staleness in cw_bitcoin’s Electrum wallet. Key changes: (1) adds ElectrumBalance.copy() so balance[currency] receives a distinct instance rather than a shared reference that could be mutated elsewhere; (2) reorders updateBalance()/updateAllUnspents() calls so balance is computed after UTXO state is refreshed; (3) builds per-account balances and only updates the current account’s balance when hasAccountsSupport is true; (4) skips balance updates if the Electrum client disconnected during fetch; (5) converts _validateAddresses() from a synchronous async-foreach to an async time-sliced loop to reduce UI jank. The commit message frames this as a stale-balance fix, not a security fix.
Changed components
cw_bitcoin/lib/electrum_balance.dartcw_bitcoin/lib/electrum_wallet.dartcw_bitcoin/lib/electrum_wallet_addresses.dartInspect captured patch +84 / −15
### cw_bitcoin/lib/electrum_balance.dart
@@ -36,6 +36,14 @@ class ElectrumBalance extends Balance {
@override
Money frozen;
+ ElectrumBalance copy() => ElectrumBalance(
+ confirmed: confirmed,
+ unconfirmed: unconfirmed,
+ frozen: frozen,
+ secondConfirmed: secondConfirmed,
+ secondUnconfirmed: secondUnconfirmed,
+ );
+
static ElectrumBalance? fromJSON(String? jsonSource, Currency currency) {
if (jsonSource == null) return null;
### cw_bitcoin/lib/electrum_wallet.dart
@@ -428,7 +428,7 @@ abstract class ElectrumWalletBase
return;
}
- balance[currency] = newBalance;
+ balance[currency] = newBalance.copy();
}
Map<int, Set<String>> get addressesSetByAccount {
@@ -1845,8 +1845,8 @@ abstract class ElectrumWalletBase
isViewOnly: false,
)..addListener((transaction) async {
transactionHistory.addOne(transaction);
- await updateBalance();
await updateAllUnspents();
+ await updateBalance();
});
}
@@ -1950,8 +1950,8 @@ abstract class ElectrumWalletBase
unspentCoins
.removeWhere((utxo) => estimatedTx.utxos.any((e) => e.utxo.txHash == utxo.hash));
- await updateBalance();
await updateAllUnspents();
+ await updateBalance();
});
} catch (e) {
throw e;
@@ -2736,8 +2736,8 @@ abstract class ElectrumWalletBase
}
});
transactionHistory.addOne(transaction);
- await updateBalance();
await updateAllUnspents();
+ await updateBalance();
});
} catch (e) {
throw e;
@@ -4037,15 +4037,24 @@ abstract class ElectrumWalletBase
var totalConfirmed = 0;
var totalUnconfirmed = 0;
+ final confirmedByAccount = <int, int>{};
+ final unconfirmedByAccount = <int, int>{};
+ final frozenByAccount = <int, int>{};
+
if (hasSilentPaymentsScanning) {
// Add values from unspent coins that are not fetched by the address list
// i.e. scanned silent payments
transactionHistory.transactions.values.forEach((tx) {
if (tx.unspents != null) {
tx.unspents!.forEach((unspent) {
if (unspent.bitcoinAddressRecord is BitcoinSilentPaymentAddressRecord) {
- if (unspent.isFrozen) totalFrozen += unspent.value;
+ final account = unspent.bitcoinAddressRecord.accountIndex;
+ if (unspent.isFrozen) {
+ totalFrozen += unspent.value;
+ frozenByAccount[account] = (frozenByAccount[account] ?? 0) + unspent.value;
+ }
totalConfirmed += unspent.value;
+ confirmedByAccount[account] = (confirmedByAccount[account] ?? 0) + unspent.value;
}
});
}
@@ -4062,6 +4071,8 @@ abstract class ElectrumWalletBase
element.value == info.value) {
if (info.isFrozen) {
totalFrozen += element.value;
+ final account = element.bitcoinAddressRecord.accountIndex;
+ frozenByAccount[account] = (frozenByAccount[account] ?? 0) + element.value;
}
}
});
@@ -4089,13 +4100,37 @@ abstract class ElectrumWalletBase
totalConfirmed += confirmed;
totalUnconfirmed += unconfirmed;
+ final account = addressRecord.accountIndex;
+ confirmedByAccount[account] = (confirmedByAccount[account] ?? 0) + confirmed;
+ unconfirmedByAccount[account] = (unconfirmedByAccount[account] ?? 0) + unconfirmed;
+
addressRecord.balance = confirmed + unconfirmed;
if (confirmed > 0 || unconfirmed > 0) {
addressRecord.setAsUsed();
walletAddresses.clearLockIfMatches(addressRecord.type, addressRecord.address);
}
}
+ if (hasAccountsSupport) {
+ final perAccount = <int, ElectrumBalance>{};
+ final accounts = <int>{
+ ...confirmedByAccount.keys,
+ ...unconfirmedByAccount.keys,
+ ...frozenByAccount.keys,
+ ...walletAddresses.accountIndexes,
+ };
+
+ for (final account in accounts) {
+ perAccount[account] = ElectrumBalance(
+ confirmed: Money.fromInt(confirmedByAccount[account] ?? 0, currency),
+ unconfirmed: Money.fromInt(unconfirmedByAccount[account] ?? 0, currency),
+ frozen: Money.fromInt(frozenByAccount[account] ?? 0, currency),
+ );
+ }
+
+ accountBalances = ObservableMap<int, ElectrumBalance>.of(perAccount);
+ }
+
return ElectrumBalance(
confirmed: Money.fromInt(totalConfirmed, currency),
unconfirmed: Money.fromInt(totalUnconfirmed, currency),
@@ -4138,13 +4173,19 @@ abstract class ElectrumWalletBase
try {
final fetchedTotal = await fetchBalances();
+ if (!electrumClient.isConnected || syncStatus is LostConnectionSyncStatus) {
+ printV("updateBalance: connection lost during fetch, keeping existing balance");
+ return;
+ }
+
if (type == WalletType.bitcoin && hasAccountsSupport) {
// If the wallet has accounts support, we only want to update the balance for the current account.
final accountBalance = accountBalances[currentAccountIndex];
- if (accountBalance == null) {
- return;
+ if (accountBalance != null) {
+ balance[currency] = accountBalance.copy();
+ } else {
+ printV("updateBalance: no balance for account $currentAccountIndex, keeping existing");
}
- balance[currency] = accountBalance;
} else {
balance[currency] = fetchedTotal;
}
### cw_bitcoin/lib/electrum_wallet_addresses.dart
@@ -1,3 +1,4 @@
+import 'dart:async' show Zone;
import 'dart:io' show Platform;
import 'dart:math';
import "package:collection/collection.dart";
@@ -425,7 +426,7 @@ abstract class ElectrumWalletAddressesBase extends WalletAddresses with Store {
updateAddressesByMatch();
updateReceiveAddresses();
updateChangeAddresses();
- _validateAddresses();
+ await _validateAddresses();
await updateAddressesInBox();
if (currentReceiveAddressIndex >= receiveAddresses.length) {
@@ -506,7 +507,7 @@ abstract class ElectrumWalletAddressesBase extends WalletAddresses with Store {
if (shouldSkipHardwareWalletType) continue;
await _generateInitialAddresses(accountIndex: accountIndex, type: type);
-
+
// Legacy derivation for these types is identical to the standard one.
if (includeLegacy && !LEGACY_DUPLICATE_ADDRESS_TYPES.contains(type)) {
await _generateInitialAddresses(
@@ -1001,12 +1002,31 @@ abstract class ElectrumWalletAddressesBase extends WalletAddresses with Store {
updateAddressesByMatch();
}
- void _validateAddresses() {
- _addresses.forEach((element) async {
- if (element.type == SegwitAddresType.mweb) {
- // this would add a ton of startup lag for mweb addresses since we have 1000 of them
- return;
+ static const _validationTimeSlice = Duration(milliseconds: 16);
+
+ Future<void> _validateAddresses() async {
+ final addresses = _addresses.toList();
+ final slice = Stopwatch()..start();
+
+ for (final element in addresses) {
+ try {
+ await _validateAddress(element);
+ } catch (e, s) {
+ Zone.current.handleUncaughtError(e, s);
+ }
+
+ if (slice.elapsed >= _validationTimeSlice) {
+ await Future<void>.delayed(Duration.zero);
+ slice.reset();
}
+ }
+ }
+
+ Future<void> _validateAddress(BitcoinAddressRecord element) async {
+ if (element.type == SegwitAddresType.mweb) {
+ // this would add a ton of startup lag for mweb addresses since we have 1000 of them
+ return;
+ }
try {
final mainHd = _hdForAddressGeneration(Why this scored 33/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.