From c61f3ca94b59284d47a2093f736b91266af4f399 Mon Sep 17 00:00:00 2001 From: julian Date: Fri, 13 Oct 2023 12:17:59 -0600 Subject: [PATCH] handle change addresses differently --- .../coins/bitcoincash/bitcoincash_wallet.dart | 169 +++++++++--------- .../mixins/fusion_wallet_interface.dart | 96 +++++----- 2 files changed, 122 insertions(+), 143 deletions(-) diff --git a/lib/services/coins/bitcoincash/bitcoincash_wallet.dart b/lib/services/coins/bitcoincash/bitcoincash_wallet.dart index 22282e2bf..5a2abfe9b 100644 --- a/lib/services/coins/bitcoincash/bitcoincash_wallet.dart +++ b/lib/services/coins/bitcoincash/bitcoincash_wallet.dart @@ -140,10 +140,7 @@ class BitcoinCashWallet extends CoinServiceAPI coin: coin, db: db, getWalletCachedElectrumX: () => cachedElectrumXClient, - getNextUnusedChangeAddress: () async { - await checkCurrentChangeAddressesForTransactions(); - return await _currentChangeAddress; - }, + getNextUnusedChangeAddress: _getUnusedChangeAddresses, getTxCountForAddress: getTxCount, getChainHeight: () async => chainHeight, mnemonic: mnemonicString, @@ -208,16 +205,80 @@ class BitcoinCashWallet extends CoinServiceAPI .findFirst()) ?? await _generateAddressForChain(0, 0, DerivePathTypeExt.primaryFor(coin)); - Future get _currentChangeAddress async => - (await db - .getAddresses(walletId) - .filter() - .typeEqualTo(isar_models.AddressType.p2pkh) - .subTypeEqualTo(isar_models.AddressSubType.change) - .derivationPath((q) => q.not().valueStartsWith("m/44'/0'")) - .sortByDerivationIndexDesc() - .findFirst()) ?? - await _generateAddressForChain(1, 0, DerivePathTypeExt.primaryFor(coin)); + Future> _getUnusedChangeAddresses({ + 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'")) + .sortByDerivationIndex() + .findAll(); + + final List 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 _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 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 _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 _checkCurrentReceivingAddressesForTransactions() async { try { // for (final type in DerivePathType.values) { @@ -1880,30 +1894,6 @@ class BitcoinCashWallet extends CoinServiceAPI } } - Future _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 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 - await _checkChangeAddressForTransactions(); - final String newChangeAddress = await _getCurrentAddressForChain( - 1, DerivePathTypeExt.primaryFor(coin)); + // get the next unused change address + final String newChangeAddress = + (await _getUnusedChangeAddresses(numberOfAddresses: 1)) + .first + .value; int feeBeingPaid = satoshisBeingUsed - satoshiAmountToSend - changeOutputSize; diff --git a/lib/services/mixins/fusion_wallet_interface.dart b/lib/services/mixins/fusion_wallet_interface.dart index fea80bf25..c4c8d027f 100644 --- a/lib/services/mixins/fusion_wallet_interface.dart +++ b/lib/services/mixins/fusion_wallet_interface.dart @@ -47,7 +47,8 @@ mixin FusionWalletInterface { } // Passed in wallet functions. - late final Future
Function() _getNextUnusedChangeAddress; + late final Future> Function({int numberOfAddresses}) + _getNextUnusedChangeAddresses; late final CachedElectrumX Function() _getWalletCachedElectrumX; late final Future Function({ required String address, @@ -61,7 +62,8 @@ mixin FusionWalletInterface { required String walletId, required Coin coin, required MainDB db, - required Future
Function() getNextUnusedChangeAddress, + required Future> Function({int numberOfAddresses}) + getNextUnusedChangeAddress, required CachedElectrumX Function() getWalletCachedElectrumX, required Future 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. - Future
_createNewReservedChangeAddress() async { - // _getNextUnusedChangeAddress() grabs the latest unused change address - // from the wallet. - // CopyWith to mark it as a fusion reserved change address - final address = (await _getNextUnusedChangeAddress()) - .copyWith(otherData: kReservedFusionAddress); + /// Reserve an address for fusion. + Future
_reserveAddress(Address address) async { + address = address.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
_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> _getUnusedReservedChangeAddresses( int numberOfAddresses, ) async { - // Fetch all reserved change addresses. - final List
reservedChangeAddresses = await _db - .getAddresses(_walletId) - .filter() - .otherDataEqualTo(kReservedFusionAddress) - .and() - .subTypeEqualTo(AddressSubType.change) - .findAll(); + final unusedChangeAddresses = await _getNextUnusedChangeAddresses( + numberOfAddresses: numberOfAddresses, + ); // Initialize a list of unused reserved change addresses. - final List
unusedAddresses = []; - - // check addresses for tx history - 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()); + final List
unusedReservedAddresses = []; + for (final address in unusedChangeAddresses) { + unusedReservedAddresses.add(await _reserveAddress(address)); } // Return the list of unused reserved change addresses. - return unusedAddresses.sublist(0, numberOfAddresses).map((e) { - final bool fusionReserved = e.otherData == kReservedFusionAddress; - - return fusion.Address( - address: e.value, - publicKey: e.publicKey, - fusionReserved: fusionReserved, - derivationPath: fusion.DerivationPath( - e.derivationPath!.value, - ), - ); - }).toList(); + return unusedReservedAddresses + .map( + (e) => fusion.Address( + address: e.value, + publicKey: e.publicKey, + fusionReserved: true, + derivationPath: fusion.DerivationPath( + e.derivationPath!.value, + ), + ), + ) + .toList(); } int _torStartCount = 0;