What changed, and why it matters
This commit fixes how the Firo/Spark wallet identifies its own change address. Previously, code compared a transaction output's address against the raw 'sparkChangeAddress' object instead of its string value. The fix exposes the change address as a string getter and updates comparisons to use that string. If left unfixed, the wallet could fail to recognize its own change outputs, which might cause incorrect balance calculations or transaction history display, but it does not appear to let an outsider steal funds directly.
Review all remaining usages of Address objects versus their .value string across the Spark/Firo wallet code to ensure consistency. Verify that balance and transaction-history tests cover change outputs. Consider adding regression tests for Spark change detection. No immediate emergency patch is indicated, but wallet teams should validate balance accuracy.
Security signals we found
Incorrect change-address classification could lead to balance miscalculation
Type mismatch between Address object and its string value
Fix is partial: only two call sites updated; broader Spark address handling not audited
No explicit security disclosure or CVE referenced in commit
Evidence from the diff
In spark_interface.dart, sparkChangeAddress was a late Address object. In firo_wallet.dart, the code checked output.addresses.contains(sparkChangeAddress.value), but elsewhere sparkChangeAddress was used directly. The patch makes sparkChangeAddress a String? getter returning _sparkChangeAddress?.value and updates comparisons to use the string. This corrects type mismatches and ensures change outputs are properly classified. The change is localized to balance/transaction parsing logic for Spark/Firo wallets.
Changed components
lib/wallets/wallet/impl/firo_wallet.dartlib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dartFiro Spark change-address handlingInspect captured patch +6 / −5
diff --git a/lib/wallets/wallet/impl/firo_wallet.dart b/lib/wallets/wallet/impl/firo_wallet.dart
index 3973f60..8c69f79 100644
--- a/lib/wallets/wallet/impl/firo_wallet.dart
+++ b/lib/wallets/wallet/impl/firo_wallet.dart
@@ -339,7 +339,7 @@ class FiroWallet<T extends ElectrumXCurrencyInterface> extends Bip39HDWallet<T>
output = output.copyWith(walletOwns: true);
} else if (isSparkMint && isMySpark) {
wasReceivedInThisWallet = true;
- if (output.addresses.contains(sparkChangeAddress.value)) {
+ if (output.addresses.contains(sparkChangeAddress)) {
changeAmountReceivedInThisWallet += output.value;
} else {
amountReceivedInThisWallet += output.value;
diff --git a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart
index 860323f..5e1560f 100644
--- a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart
+++ b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart
@@ -112,8 +112,9 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
String? _viewKeyHex;
String? get sparkViewKey => _viewKeyHex!;
- // Really we should just send change back to the same address.
- late Address sparkChangeAddress;
+ Address? _sparkChangeAddress;
+
+ String? get sparkChangeAddress => _sparkChangeAddress?.value;
bool get isTestNet {
return cryptoCurrency.network.isTestNet;
@@ -331,7 +332,7 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
}
_currentSparkAddress = address;
- sparkChangeAddress = await _generateSparkAddress(libSpark.sparkChange);
+ _sparkChangeAddress = await _generateSparkAddress(libSpark.sparkChange);
} catch (e, s) {
// do nothing, still allow user into wallet
Logging.instance.e("$runtimeType init() failed", error: e, stackTrace: s);
@@ -654,7 +655,7 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
),
memo: txData.sparkRecipients![i].memo,
isChange:
- sparkChangeAddress.value == txData.sparkRecipients![i].address,
+ _sparkChangeAddress!.value == txData.sparkRecipients![i].address,
));
}
Why this scored 32/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.