handle change addresses differently

lib/services/coins/bitcoincash/bitcoincash_wallet.dart

@@ -140,10 +140,7 @@ class BitcoinCashWallet extends CoinServiceAPI
       coin: coin,
       db: db,
       getWalletCachedElectrumX: () => cachedElectrumXClient,
-      getNextUnusedChangeAddress: () async {
+      getNextUnusedChangeAddress: _getUnusedChangeAddresses,
-        await checkCurrentChangeAddressesForTransactions();
-        return await _currentChangeAddress;
-      },
       getTxCountForAddress: getTxCount,
       getChainHeight: () async => chainHeight,
       mnemonic: mnemonicString,
@@ -208,16 +205,80 @@ class BitcoinCashWallet extends CoinServiceAPI
           .findFirst()) ??
       await _generateAddressForChain(0, 0, DerivePathTypeExt.primaryFor(coin));
 
-  Future<isar_models.Address> get _currentChangeAddress async =>
+  Future<List<isar_models.Address>> _getUnusedChangeAddresses({
-      (await db
+    int numberOfAddresses = 1,
+  }) async {
+    if (numberOfAddresses < 1) {
+      throw ArgumentError.value(
+        numberOfAddresses,
+        "numberOfAddresses",
+        "Must not be less than 1",
+      );
+    }
+
+    final changeAddresses = await db
         .getAddresses(walletId)
         .filter()
         .typeEqualTo(isar_models.AddressType.p2pkh)
         .subTypeEqualTo(isar_models.AddressSubType.change)
         .derivationPath((q) => q.not().valueStartsWith("m/44'/0'"))
-          .sortByDerivationIndexDesc()
+        .sortByDerivationIndex()
-          .findFirst()) ??
+        .findAll();
-      await _generateAddressForChain(1, 0, DerivePathTypeExt.primaryFor(coin));
+
+    final List<isar_models.Address> unused = [];
+
+    for (final addr in changeAddresses) {
+      if (await _isUnused(addr.value)) {
+        unused.add(addr);
+        if (unused.length == numberOfAddresses) {
+          return unused;
+        }
+      }
+    }
+
+    // if not returned by now, we need to create more addresses
+    int countMissing = numberOfAddresses - unused.length;
+
+    int nextIndex =
+        changeAddresses.isEmpty ? 0 : changeAddresses.last.derivationIndex + 1;
+
+    while (countMissing > 0) {
+      //   create a new address
+      final address = await _generateAddressForChain(
+        1,
+        nextIndex,
+        DerivePathTypeExt.primaryFor(coin),
+      );
+      nextIndex++;
+      await db.putAddress(address);
+
+      // check if it has been used before adding
+      if (await _isUnused(address.value)) {
+        unused.add(address);
+        countMissing--;
+      }
+    }
+
+    return unused;
+  }
+
+  Future<bool> _isUnused(String address) async {
+    final txCountInDB = await db
+        .getTransactions(_walletId)
+        .filter()
+        .address((q) => q.valueEqualTo(address))
+        .count();
+    if (txCountInDB == 0) {
+      // double check via electrumx
+      // _getTxCountForAddress can throw!
+      final count = await getTxCount(address: address);
+      if (count == 0) {
+        return true;
+      }
+    }
+
+    return false;
+  }
 
   @override
   Future<void> exit() async {
@@ -888,7 +949,6 @@ class BitcoinCashWallet extends CoinServiceAPI
 
       if (currentHeight != storedHeight) {
         GlobalEventBus.instance.fire(RefreshPercentChangedEvent(0.2, walletId));
-        await _checkChangeAddressForTransactions();
 
         GlobalEventBus.instance.fire(RefreshPercentChangedEvent(0.3, walletId));
         await _checkCurrentReceivingAddressesForTransactions();
@@ -1810,52 +1870,6 @@ class BitcoinCashWallet extends CoinServiceAPI
     }
   }
 
-  Future<void> _checkChangeAddressForTransactions() async {
-    try {
-      final currentChange = await _currentChangeAddress;
-      final int txCount = await getTxCount(address: currentChange.value);
-      Logging.instance.log(
-          'Number of txs for current change address $currentChange: $txCount',
-          level: LogLevel.Info);
-
-      if (txCount >= 1 || currentChange.derivationIndex < 0) {
-        // First increment the change index
-        final newChangeIndex = currentChange.derivationIndex + 1;
-
-        // Use new index to derive a new change address
-        final newChangeAddress = await _generateAddressForChain(
-            1, newChangeIndex, DerivePathTypeExt.primaryFor(coin));
-
-        final existing = await db
-            .getAddresses(walletId)
-            .filter()
-            .valueEqualTo(newChangeAddress.value)
-            .findFirst();
-        if (existing == null) {
-          // Add that new change address
-          await db.putAddress(newChangeAddress);
-        } else {
-          if (existing.otherData != kReservedFusionAddress) {
-            // we need to update the address
-            await db.updateAddress(existing, newChangeAddress);
-          }
-        }
-        // keep checking until address with no tx history is set as current
-        await _checkChangeAddressForTransactions();
-      }
-    } on SocketException catch (se, s) {
-      Logging.instance.log(
-          "SocketException caught in _checkReceivingAddressForTransactions(${DerivePathTypeExt.primaryFor(coin)}): $se\n$s",
-          level: LogLevel.Error);
-      return;
-    } catch (e, s) {
-      Logging.instance.log(
-          "Exception rethrown from _checkReceivingAddressForTransactions(${DerivePathTypeExt.primaryFor(coin)}): $e\n$s",
-          level: LogLevel.Error);
-      rethrow;
-    }
-  }
-
   Future<void> _checkCurrentReceivingAddressesForTransactions() async {
     try {
       // for (final type in DerivePathType.values) {
@@ -1880,30 +1894,6 @@ class BitcoinCashWallet extends CoinServiceAPI
     }
   }
 
-  Future<void> _checkCurrentChangeAddressesForTransactions() async {
-    try {
-      // for (final type in DerivePathType.values) {
-      await _checkChangeAddressForTransactions();
-      // }
-    } catch (e, s) {
-      Logging.instance.log(
-          "Exception rethrown from _checkCurrentChangeAddressesForTransactions(): $e\n$s",
-          level: LogLevel.Error);
-      rethrow;
-    }
-  }
-
-  /// public wrapper because dart can't test private...
-  Future<void> checkCurrentChangeAddressesForTransactions() async {
-    if (Platform.environment["FLUTTER_TEST"] == "true") {
-      try {
-        return _checkCurrentChangeAddressesForTransactions();
-      } catch (_) {
-        rethrow;
-      }
-    }
-  }
-
   /// attempts to convert a string to a valid scripthash
   ///
   /// Returns the scripthash or throws an exception on invalid bch address
@@ -2485,10 +2475,11 @@ class BitcoinCashWallet extends CoinServiceAPI
         if (changeOutputSize > DUST_LIMIT.raw.toInt() &&
             satoshisBeingUsed - satoshiAmountToSend - changeOutputSize ==
                 feeForTwoOutputs) {
-          // generate new change address if current change address has been used
+          // get the next unused change address
-          await _checkChangeAddressForTransactions();
+          final String newChangeAddress =
-          final String newChangeAddress = await _getCurrentAddressForChain(
+              (await _getUnusedChangeAddresses(numberOfAddresses: 1))
-              1, DerivePathTypeExt.primaryFor(coin));
+                  .first
+                  .value;
 
           int feeBeingPaid =
               satoshisBeingUsed - satoshiAmountToSend - changeOutputSize;

lib/services/mixins/fusion_wallet_interface.dart

@@ -47,7 +47,8 @@ mixin FusionWalletInterface {
   }
 
   // Passed in wallet functions.
-  late final Future<Address> Function() _getNextUnusedChangeAddress;
+  late final Future<List<Address>> Function({int numberOfAddresses})
+      _getNextUnusedChangeAddresses;
   late final CachedElectrumX Function() _getWalletCachedElectrumX;
   late final Future<int> Function({
     required String address,
@@ -61,7 +62,8 @@ mixin FusionWalletInterface {
     required String walletId,
     required Coin coin,
     required MainDB db,
-    required Future<Address> Function() getNextUnusedChangeAddress,
+    required Future<List<Address>> Function({int numberOfAddresses})
+        getNextUnusedChangeAddress,
     required CachedElectrumX Function() getWalletCachedElectrumX,
     required Future<int> Function({
       required String address,
@@ -75,7 +77,7 @@ mixin FusionWalletInterface {
     _walletId = walletId;
     _coin = coin;
     _db = db;
-    _getNextUnusedChangeAddress = getNextUnusedChangeAddress;
+    _getNextUnusedChangeAddresses = getNextUnusedChangeAddress;
     _torService = FusionTorService.sharedInstance;
     _getWalletCachedElectrumX = getWalletCachedElectrumX;
     _getTxCountForAddress = getTxCountForAddress;
@@ -176,15 +178,11 @@ mixin FusionWalletInterface {
     }
   }
 
-  /// Creates a new reserved change address.
+  /// Reserve an address for fusion.
-  Future<Address> _createNewReservedChangeAddress() async {
+  Future<Address> _reserveAddress(Address address) async {
-    // _getNextUnusedChangeAddress() grabs the latest unused change address
+    address = address.copyWith(otherData: kReservedFusionAddress);
-    // from the wallet.
-    // CopyWith to mark it as a fusion reserved change address
-    final address = (await _getNextUnusedChangeAddress())
-        .copyWith(otherData: kReservedFusionAddress);
 
-    // Make sure the address is in the database as reserved for Fusion.
+    // Make sure the address is updated in the database as reserved for Fusion.
     final _address = await _db.getAddress(_walletId, address.value);
     if (_address != null) {
       await _db.updateAddress(_address, address);
@@ -195,60 +193,50 @@ mixin FusionWalletInterface {
     return address;
   }
 
+  /// un reserve a fusion reserved address.
+  /// If [address] is not reserved nothing happens
+  Future<Address> _unReserveAddress(Address address) async {
+    if (address.otherData != kReservedFusionAddress) {
+      return address;
+    }
+
+    final updated = address.copyWith(otherData: null);
+
+    // Make sure the address is updated in the database.
+    await _db.updateAddress(address, updated);
+
+    return updated;
+  }
+
   /// Returns a list of unused reserved change addresses.
   ///
   /// If there are not enough unused reserved change addresses, new ones are created.
   Future<List<fusion.Address>> _getUnusedReservedChangeAddresses(
     int numberOfAddresses,
   ) async {
-    // Fetch all reserved change addresses.
+    final unusedChangeAddresses = await _getNextUnusedChangeAddresses(
-    final List<Address> reservedChangeAddresses = await _db
+      numberOfAddresses: numberOfAddresses,
-        .getAddresses(_walletId)
+    );
-        .filter()
-        .otherDataEqualTo(kReservedFusionAddress)
-        .and()
-        .subTypeEqualTo(AddressSubType.change)
-        .findAll();
 
     // Initialize a list of unused reserved change addresses.
-    final List<Address> unusedAddresses = [];
+    final List<Address> unusedReservedAddresses = [];
-
+    for (final address in unusedChangeAddresses) {
-    // check addresses for tx history
+      unusedReservedAddresses.add(await _reserveAddress(address));
-    for (final address in reservedChangeAddresses) {
-      // first check in db to avoid unnecessary network calls
-      final txCountInDB = await _db
-          .getTransactions(_walletId)
-          .filter()
-          .address((q) => q.valueEqualTo(address.value))
-          .count();
-      if (txCountInDB == 0) {
-        // double check via electrumx
-        // _getTxCountForAddress can throw!
-        final count = await _getTxCountForAddress(address: address.value);
-        if (count == 0) {
-          unusedAddresses.add(address);
-        }
-      }
-    }
-
-    // If there are not enough unused reserved change addresses, create new ones.
-    while (unusedAddresses.length < numberOfAddresses) {
-      unusedAddresses.add(await _createNewReservedChangeAddress());
     }
 
     // Return the list of unused reserved change addresses.
-    return unusedAddresses.sublist(0, numberOfAddresses).map((e) {
+    return unusedReservedAddresses
-      final bool fusionReserved = e.otherData == kReservedFusionAddress;
+        .map(
-
+          (e) => fusion.Address(
-      return fusion.Address(
             address: e.value,
             publicKey: e.publicKey,
-        fusionReserved: fusionReserved,
+            fusionReserved: true,
             derivationPath: fusion.DerivationPath(
               e.derivationPath!.value,
             ),
-      );
+          ),
-    }).toList();
+        )
+        .toList();
   }
 
   int _torStartCount = 0;