What the developer wrote
Authored by julian
fix: firo related addressbook bug
What changed, and why it matters
This commit fixes a bug in Stack Wallet's address book related to Firo's Spark privacy addresses. Previously, the code could try to use or generate Spark addresses for certain view-only wallets that don't actually support Spark, and it could also crash or behave incorrectly when no current Spark address existed yet. The patch adds a guard to skip non-Spark view-only wallets and makes the internal current-address field optional so the wallet can recover gracefully when it isn't initialized.
Treat as a routine bug-fix patch. Reviewers should verify that generateNextSparkAddress now derives correct diversifiers when _currentSparkAddress is null and that the view-only guard covers all incompatible wallet modes. No immediate security response appears necessary, but regression tests for address-book behavior with view-only Firo wallets and fresh wallets without an initialized Spark address are warranted.
Security signals we found
Null-safety / uninitialized-state handling change
View-only wallet mode guard added
Potential address derivation/indexing bug fixed
Crash/DoS surface reduced by removing late-initialized field assumption
Evidence from the diff
The change modifies two Dart files. In address_book_view.dart, the loop that collects current receiving addresses now skips wallets that implement SparkInterface but are view-only with a viewOnlyType other than spark, preventing incorrect Spark address generation/retrieval for incompatible wallet modes. In spark_interface.dart, _currentSparkAddress is changed from a non-nullable late Address to nullable Address?, and callers are updated: getCurrentReceivingSparkAddress() now returns the nullable value directly instead of throwing when uninitialized; generateNextSparkAddress() now derives the next diversifier from getCurrentReceivingAddress() rather than from the possibly-uninitialized field. A later use of _currentSparkAddress! in address scanning is kept with a force-unwrap and a comment that it should be safe in that context.
Changed components
lib/pages/address_book_views/address_book_view.dartlib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dartFiro Spark address handlingAddress book contact/wallet address collectionInspect captured patch +11 / −5
diff --git a/lib/pages/address_book_views/address_book_view.dart b/lib/pages/address_book_views/address_book_view.dart
index d2fb198..7f15268 100644
--- a/lib/pages/address_book_views/address_book_view.dart
+++ b/lib/pages/address_book_views/address_book_view.dart
@@ -84,7 +84,8 @@ class _AddressBookViewState extends ConsumerState<AddressBookView> {
final wallets = ref.read(pWallets).wallets;
for (final wallet in wallets) {
final String addressString;
- if (wallet is SparkInterface) {
+ if (wallet is SparkInterface &&
+ !(wallet.isViewOnly && wallet.viewOnlyType != .spark)) {
Address? address = await wallet.getCurrentReceivingSparkAddress();
address ??= await wallet.generateNextSparkAddress(saveToDB: true);
addressString = address.value;
diff --git a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart
index 37e8506..ecf7c94 100644
--- a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart
+++ b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart
@@ -108,7 +108,7 @@ Future<R> computeWithLibSparkLogging<M, R>(
mixin SparkInterface<T extends ElectrumXCurrencyInterface>
on Bip39HDWallet<T>, ElectrumXInterface<T> {
- late Address _currentSparkAddress;
+ Address? _currentSparkAddress;
String? _viewKeyHex;
String? get sparkViewKey => _viewKeyHex!;
@@ -367,7 +367,7 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
Future<Address?> getCurrentReceivingSparkAddress() async {
try {
// if _currentSparkAddress is not initialized, this will throw.
- return _currentSparkAddress;
+ return _currentSparkAddress!;
} catch (e) {
return await mainDB.isar.addresses
.where()
@@ -380,7 +380,10 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
}
Future<Address> generateNextSparkAddress({required bool saveToDB}) async {
- int diversifier = _currentSparkAddress.derivationIndex + 1;
+ final currentDiversifier =
+ (await getCurrentReceivingAddress())?.derivationIndex;
+ // if current is null, start at index 1
+ int diversifier = (currentDiversifier ?? 0) + 1;
if (diversifier == libSpark.sparkChange) {
diversifier++; // ensure only receiving addresses are shown
}
@@ -1423,7 +1426,9 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
// generating every time) arbitrary number of addresses
const lookAheadCount = 100;
- int diversifier = _currentSparkAddress.derivationIndex;
+ // force unwrap optional should be fine here. If not then the
+ // eclosing function is being called somewhere it probably shouldn't be.
+ int diversifier = _currentSparkAddress!.derivationIndex;
final maxDiversifier = diversifier + lookAheadCount;
while (diversifier < maxDiversifier) {
Why this scored 29/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.