From e848f829d52427975c900284764c606ec5b462fc Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 10 Mar 2026 20:33:03 +0000 Subject: [PATCH 1/4] Optimize Firo Spark mint tx generation and fix fee < vSize error Performance: Pre-compute signing keys, addresses, and wallet ownership data before the main loop instead of doing expensive async operations (getRootHDNode, DB lookups, key derivation) per-UTXO per-fee-iteration. With many inputs this eliminates N*M redundant mnemonic-to-seed derivations and database queries. Bug fix: When the real transaction (generate:true) is larger than the dummy (generate:false) used for fee estimation, retry with a fee based on the real vSize instead of throwing "fee is less than vSize". https://claude.ai/code/session_01YHRu2KAxZW26VnP1bZKZoX --- .../spark_interface.dart | 154 +++++++++++++++--- 1 file changed, 130 insertions(+), 24 deletions(-) diff --git a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart index 0dec8aab2..359257b33 100644 --- a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart +++ b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart @@ -4,6 +4,7 @@ import 'dart:isolate'; import 'dart:math'; import 'package:bitcoindart/bitcoindart.dart' as btc; +import 'package:coinlib_flutter/coinlib_flutter.dart' as coinlib; import 'package:decimal/decimal.dart'; import 'package:flutter/foundation.dart'; import 'package:isar_community/isar.dart'; @@ -1550,17 +1551,79 @@ mixin SparkInterface final random = Random.secure(); final List results = []; + // Pre-compute signing keys for all UTXOs to avoid repeated calls to + // getRootHDNode() (which re-derives from mnemonic seed each time) and + // individual DB lookups inside the hot loop. + final root = await getRootHDNode(); + final Map + signingKeyCache = {}; + for (final utxo in availableUtxos) { + final address = utxo.address!; + if (!signingKeyCache.containsKey(address)) { + final derivePathType = cryptoCurrency.addressType(address: address); + final dbAddress = await mainDB.getAddress(walletId, address); + if (dbAddress?.derivationPath != null) { + final key = root.derivePath(dbAddress!.derivationPath!.value); + signingKeyCache[address] = + (derivePathType: derivePathType, key: key); + } + } + } + + // Cache addresses used repeatedly inside the loop. + final sparkAddress = (await getCurrentReceivingSparkAddress())!.value; + final changeAddress = await getCurrentChangeAddress(); + + // Pre-cache the change address signing key so change UTXOs that get + // recycled back into valueAndUTXOs can be signed without re-deriving. + if (changeAddress != null && + !signingKeyCache.containsKey(changeAddress.value)) { + final derivePathType = cryptoCurrency.addressType( + address: changeAddress.value, + ); + final dbAddress = await mainDB.getAddress( + walletId, + changeAddress.value, + ); + if (dbAddress?.derivationPath != null) { + final key = root.derivePath(dbAddress!.derivationPath!.value); + signingKeyCache[changeAddress.value] = + (derivePathType: derivePathType, key: key); + } + } + + // Pre-fetch wallet-owned addresses for output ownership checks. + final walletAddresses = await mainDB.isar.addresses + .where() + .walletIdEqualTo(walletId) + .valueProperty() + .findAll(); + final walletAddressSet = walletAddresses.toSet(); + valueAndUTXOs.shuffle(random); + // Tracks the minimum fee for the current UTXO group across retries. + // When the real transaction turns out to be larger than the dummy estimate, + // this is raised and the group is retried. + BigInt minFeeForGroup = BigInt.zero; + int feeRetryCount = 0; + const maxFeeRetries = 3; + while (valueAndUTXOs.isNotEmpty) { final lockTime = random.nextInt(10) == 0 ? max(0, currentHeight - random.nextInt(100)) : currentHeight; const txVersion = 1; - final List vin = []; - final List<(dynamic, int, String?)> vout = []; + List vin = []; + List<(dynamic, int, String?)> vout = []; + + BigInt nFeeRet = minFeeForGroup; - BigInt nFeeRet = BigInt.zero; + // Save outputs_ state before this UTXO group so it can be restored + // if a fee retry is needed. + final outputsBeforeGroup = outputs_ + .map((e) => MutableSparkRecipient(e.address, e.value, e.memo)) + .toList(); final itr = valueAndUTXOs.first; BigInt valueToMintInTx = _sum(itr); @@ -1610,7 +1673,7 @@ mixin SparkInterface if (autoMintAll) { singleTxOutputs.add( MutableSparkRecipient( - (await getCurrentReceivingSparkAddress())!.value, + sparkAddress, mintedValue, "", ), @@ -1694,11 +1757,19 @@ mixin SparkInterface BigInt nValueIn = BigInt.zero; for (final utxo in itr) { if (nValueToSelect > nValueIn) { - setCoins.add( - (await addSigningKeys([ - StandardInput(utxo), - ])).whereType().first, + final cached = signingKeyCache[utxo.address!]; + if (cached == null) { + throw Exception( + "Signing key not found for address ${utxo.address}. " + "Local db may be corrupt. Rescan wallet.", + ); + } + final input = StandardInput( + utxo, + derivePathType: cached.derivePathType, ); + input.key = cached.key; + setCoins.add(input); nValueIn += BigInt.from(utxo.value); } } @@ -1720,7 +1791,6 @@ mixin SparkInterface throw Exception("Change index out of range"); } - final changeAddress = await getCurrentChangeAddress(); vout.insert(nChangePosInOut, ( changeAddress!.value, nChange.toInt(), @@ -1984,19 +2054,11 @@ mixin SparkInterface addresses: [ if (addressOrScript is String) addressOrScript.toString(), ], - walletOwns: - (await mainDB.isar.addresses - .where() - .walletIdEqualTo(walletId) - .filter() - .valueEqualTo( - addressOrScript is Uint8List - ? output.$3! - : addressOrScript as String, - ) - .valueProperty() - .findFirst()) != - null, + walletOwns: walletAddressSet.contains( + addressOrScript is Uint8List + ? output.$3! + : addressOrScript as String, + ), ), ); } @@ -2077,12 +2139,56 @@ mixin SparkInterface Logging.instance.i("nFeeRet=$nFeeRet, vSize=${data.vSize}"); if (nFeeRet.toInt() < data.vSize!) { + feeRetryCount++; + if (feeRetryCount > maxFeeRetries) { + throw Exception( + "fee is less than vSize after $maxFeeRetries retries " + "(fee=$nFeeRet, vSize=${data.vSize})", + ); + } + // The real transaction (with generate: true) can be larger than the + // dummy used for fee estimation (generate: false) because the real + // Spark mint proofs are larger than the dummy placeholders. When this + // happens, set a minimum fee based on the real vSize and retry the + // entire UTXO group from the top of the outer loop. + final realFeeNeeded = BigInt.from( + estimateTxFee( + vSize: data.vSize!, + feeRatePerKB: feesObject.medium, + ), + ); Logging.instance.w( - "Spark mint transaction failed: $nFeeRet is less than ${data.vSize}", + "Spark mint fee $nFeeRet < vSize ${data.vSize}. " + "Retrying ($feeRetryCount/$maxFeeRetries) with " + "realFeeNeeded=$realFeeNeeded", ); - throw Exception("fee is less than vSize"); + // Restore the UTXOs used in this attempt back to itr so they can be + // re-selected on the next try. + for (final usedCoin in vin) { + if (!itr.any( + (u) => + u.txid == usedCoin.utxo.txid && u.vout == usedCoin.utxo.vout, + )) { + itr.add(usedCoin.utxo); + } + } + // Ensure itr is back in valueAndUTXOs if it was removed. + if (!valueAndUTXOs.contains(itr)) { + valueAndUTXOs.insert(0, itr); + } + // Restore outputs_ to the state before the fee loop consumed it, + // so the retry processes the same mint amounts. + outputs_ = outputsBeforeGroup; + // Set the minimum fee for the retry and continue the outer loop, + // which will restart fee estimation for this same UTXO group. + minFeeForGroup = realFeeNeeded; + continue; } + // Successfully built transaction, reset for next UTXO group. + minFeeForGroup = BigInt.zero; + feeRetryCount = 0; + results.add(data); if (nChangePosInOut >= 0) { From 3069afaf5a5e9751045ebc8a51a84c3b38c25af6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 7 Apr 2026 10:57:55 +0000 Subject: [PATCH 2/4] Fix fee retry state leaking between UTXO groups in Spark mint When a fee retry caused a UTXO group to be skipped (mintedValue hit zero due to the higher fee floor), minFeeForGroup and feeRetryCount were not reset before moving to the next group. This could cause subsequent groups to start with an inflated fee floor from a previous larger group's transaction, potentially skipping groups unnecessarily or reducing their retry budget. https://claude.ai/code/session_01D9ssjMkQAMoCUcrPzVVtEq --- .../wallet/wallet_mixin_interfaces/spark_interface.dart | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart index 359257b33..bfb120ee0 100644 --- a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart +++ b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart @@ -1956,6 +1956,11 @@ mixin SparkInterface } if (skipCoin) { + // Reset fee retry state so the next UTXO group starts fresh. + // Without this, a fee floor computed for a larger group could + // cause smaller groups to be incorrectly skipped. + minFeeForGroup = BigInt.zero; + feeRetryCount = 0; continue; } From 3d0dec1a44a46a61cf3da48d6eaabab6346d1138 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 7 Apr 2026 18:11:13 +0000 Subject: [PATCH 3/4] Fix audit findings: add fee buffer to retry and restore final on vin/vout - Add 10-byte vSize buffer to retry fee calculation to match the nBytesBuffer used in the inner fee estimation loop, avoiding a potential unnecessary extra retry. - Restore `final` on vin/vout declarations since they are never reassigned (only cleared via .clear()). https://claude.ai/code/session_01D9ssjMkQAMoCUcrPzVVtEq --- .../wallet/wallet_mixin_interfaces/spark_interface.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart index bfb120ee0..5a29bb12e 100644 --- a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart +++ b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart @@ -1614,8 +1614,8 @@ mixin SparkInterface ? max(0, currentHeight - random.nextInt(100)) : currentHeight; const txVersion = 1; - List vin = []; - List<(dynamic, int, String?)> vout = []; + final List vin = []; + final List<(dynamic, int, String?)> vout = []; BigInt nFeeRet = minFeeForGroup; @@ -2158,7 +2158,7 @@ mixin SparkInterface // entire UTXO group from the top of the outer loop. final realFeeNeeded = BigInt.from( estimateTxFee( - vSize: data.vSize!, + vSize: data.vSize! + 10, feeRatePerKB: feesObject.medium, ), ); From 81a6f324b2084be745c3ce6847d76ac92933bf56 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 7 Apr 2026 19:35:12 +0000 Subject: [PATCH 4/4] Fix negative mintedValue passing through zero-check in fee loop The guard at the top of the inner fee loop only checked mintedValue == BigInt.zero, but mintedValue can go negative when nFeeRet (starting from minFeeForGroup on retry) exceeds the UTXO group's total value. A negative mintedValue would flow into createSparkMintRecipients causing invalid outputs. Change the check to <= BigInt.zero to match the intent of the commented-out MoneyRange check from the C++ reference implementation. https://claude.ai/code/session_01D9ssjMkQAMoCUcrPzVVtEq --- lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart index 5a29bb12e..e0f42f63d 100644 --- a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart +++ b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart @@ -1653,7 +1653,7 @@ mixin SparkInterface } // if (!MoneyRange(mintedValue) || mintedValue == 0) { - if (mintedValue == BigInt.zero) { + if (mintedValue <= BigInt.zero) { valueAndUTXOs.remove(itr); skipCoin = true; break;