From 51d43572e8e01540cca8bfebf6d7fba5e458acf3 Mon Sep 17 00:00:00 2001 From: Reuben Yap Date: Mon, 20 Apr 2026 10:08:29 +0800 Subject: [PATCH 1/3] Optimize Firo Spark mint tx generation and fix fee < vSize error ## Performance Pre-compute signing keys, addresses, and wallet-owned address set before the main loop. The original code called getRootHDNode() (expensive mnemonic-to-seed derivation), a per-UTXO DB lookup for derivationPath, and a per-output DB lookup for walletOwns, all inside nested loops. For N inputs across M fee-estimation iterations, this was O(N*M) redundant work. Also caches getCurrentReceivingSparkAddress() and getCurrentChangeAddress() since neither can change within the function. ## Fee-less-than-vSize bug fix The dummy transaction built for fee estimation is signed with real keys over different data than the final real transaction. bitcoindart's ECDSA signing (RFC 6979, low-S enforced, low-R not enforced) produces DER signatures whose length varies by up to ~4 bytes per input depending on the random r value's high bit. For P2PKH inputs (Firo's default), this variance counts at full weight, so with 10+ inputs the dummy vs real vSize can differ by more than the original 10-byte buffer, tripping the nFeeRet < data.vSize check. Scale the buffer linearly with input count: final nBytesBuffer = 10 + 4 * setCoins.length; This matches what Firo's own C++ wallet does in DummySignatureCreator (src/wallet/wallet.h:1436): "Helper for producing a bunch of max-sized low-S signatures (eg 72 bytes)". Extra fee cost: ~4 sats per input at 1 sat/byte. ## Subsidiary fixes - mintedValue <= BigInt.zero (was == BigInt.zero): catches negative mintedValue when a UTXO group's total is less than the computed fee and subtractFeeFromAmount=false. Matches the C++ reference !MoneyRange(mintedValue) || mintedValue == 0. - Clarified the fee < vSize error message: the check is effectively a min-relay-fee check (feeRate < 1 sat/byte), not a fee/size mismatch. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../spark_interface.dart | 96 ++++++++++++++----- 1 file changed, 72 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..03f4925a9 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,6 +1551,44 @@ 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 = {}; + Future cacheSigningKey(String address) async { + if (signingKeyCache.containsKey(address)) return; + 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); + } + } + + for (final utxo in availableUtxos) { + await cacheSigningKey(utxo.address!); + } + + // 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) { + await cacheSigningKey(changeAddress.value); + } + + // 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); while (valueAndUTXOs.isNotEmpty) { @@ -1590,7 +1629,7 @@ mixin SparkInterface } // if (!MoneyRange(mintedValue) || mintedValue == 0) { - if (mintedValue == BigInt.zero) { + if (mintedValue <= BigInt.zero) { valueAndUTXOs.remove(itr); skipCoin = true; break; @@ -1610,7 +1649,7 @@ mixin SparkInterface if (autoMintAll) { singleTxOutputs.add( MutableSparkRecipient( - (await getCurrentReceivingSparkAddress())!.value, + sparkAddress, mintedValue, "", ), @@ -1694,11 +1733,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 +1767,6 @@ mixin SparkInterface throw Exception("Change index out of range"); } - final changeAddress = await getCurrentChangeAddress(); vout.insert(nChangePosInOut, ( changeAddress!.value, nChange.toInt(), @@ -1817,13 +1863,19 @@ mixin SparkInterface throw Exception("Transaction too large"); } - const nBytesBuffer = 10; + // ECDSA DER signature lengths vary by up to ~4 bytes per input + // (r randomly flips the 0x80 bit → 32 vs 33 bytes; s varies similarly + // within low-S bounds). The dummy tx above is signed with real keys + // over different data than the final real tx, so their vSizes differ + // by up to ~4 bytes per input. Scale the safety buffer with input + // count so the estimated fee always covers the final signed tx. + final nBytesBuffer = 10 + 4 * setCoins.length; final nFeeNeeded = BigInt.from( estimateTxFee( vSize: nBytes + nBytesBuffer, feeRatePerKB: feesObject.medium, ), - ); // One day we'll do this properly + ); if (nFeeRet >= nFeeNeeded) { for (final usedCoin in setCoins) { @@ -1984,19 +2036,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, + ), ), ); } @@ -2076,11 +2120,15 @@ mixin SparkInterface ); Logging.instance.i("nFeeRet=$nFeeRet, vSize=${data.vSize}"); + // fee_sats < vSize_bytes ⟺ feeRate < 1 sat/byte (the standard minimum + // relay fee). Firing here means feesObject.medium came back below that + // threshold, not that the buffer underestimated the real tx size. if (nFeeRet.toInt() < data.vSize!) { Logging.instance.w( - "Spark mint transaction failed: $nFeeRet is less than ${data.vSize}", + "Fee rate below 1 sat/byte minimum relay fee: " + "fee=$nFeeRet sats, vSize=${data.vSize} bytes", ); - throw Exception("fee is less than vSize"); + throw Exception("Fee rate below 1 sat/byte minimum relay fee"); } results.add(data); From 653d11a281698d2e347a689e7997fc174bae5b40 Mon Sep 17 00:00:00 2001 From: Reuben Yap Date: Mon, 27 Apr 2026 17:03:00 +0800 Subject: [PATCH 2/3] Address Spark mint review feedback --- .../spark_interface.dart | 100 ++++++++++-------- 1 file changed, 58 insertions(+), 42 deletions(-) diff --git a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart index 03f4925a9..e3c02ee66 100644 --- a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart +++ b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart @@ -1547,38 +1547,54 @@ mixin SparkInterface .map((e) => MutableSparkRecipient(e.address, e.value, e.memo)) .toList(); // deep copy final feesObject = await fees; + final minRelayFeeRatePerKB = BigInt.from(1000); + final mintFeeRatePerKB = feesObject.medium < minRelayFeeRatePerKB + ? minRelayFeeRatePerKB + : feesObject.medium; final currentHeight = await chainHeight; 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 String? autoMintSparkAddress = autoMintAll + ? (await getCurrentReceivingSparkAddress())?.value + : null; + if (autoMintAll && autoMintSparkAddress == null) { + throw Exception("No current Spark receiving address found."); + } + + // Cache signing keys lazily for selected inputs. This mirrors the subset + // of addSigningKeys used by Firo Spark mints; Firo currently supports only + // BIP44 transparent inputs, so caching from the wallet root is valid here. final root = await getRootHDNode(); - final Map - signingKeyCache = {}; - Future cacheSigningKey(String address) async { - if (signingKeyCache.containsKey(address)) return; + final Map signingKeyCache = {}; + Future<_SparkMintSigningKey> getCachedSigningKey(String address) async { + final existing = signingKeyCache[address]; + if (existing != null) { + return existing; + } + 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); + if (dbAddress?.derivationPath == null) { + throw Exception( + "Signing key not found for address $address. " + "Local db may be corrupt. Rescan wallet.", + ); } - } - for (final utxo in availableUtxos) { - await cacheSigningKey(utxo.address!); + final key = root.derivePath(dbAddress!.derivationPath!.value); + final cached = (derivePathType: derivePathType, key: key); + signingKeyCache[address] = cached; + return cached; } - // 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) { - await cacheSigningKey(changeAddress.value); + Address? cachedChangeAddress; + Future
getMintChangeAddress() async { + cachedChangeAddress ??= await getCurrentChangeAddress(); + if (cachedChangeAddress == null) { + throw Exception("No current change address found."); + } + return cachedChangeAddress!; } // Pre-fetch wallet-owned addresses for output ownership checks. @@ -1649,7 +1665,7 @@ mixin SparkInterface if (autoMintAll) { singleTxOutputs.add( MutableSparkRecipient( - sparkAddress, + autoMintSparkAddress!, mintedValue, "", ), @@ -1687,9 +1703,10 @@ mixin SparkInterface for (int i = 0; i < singleTxOutputs.length; ++i) { if (singleTxOutputs[i].value <= singleFee) { - singleTxOutputs.removeAt(i); - remainder += singleTxOutputs[i].value - singleFee; + final removed = singleTxOutputs.removeAt(i); + remainder += removed.value - singleFee; --i; + continue; } singleTxOutputs[i].value -= singleFee; if (remainder > BigInt.zero && @@ -1733,13 +1750,7 @@ mixin SparkInterface BigInt nValueIn = BigInt.zero; for (final utxo in itr) { if (nValueToSelect > nValueIn) { - 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 cached = await getCachedSigningKey(utxo.address!); final input = StandardInput( utxo, derivePathType: cached.derivePathType, @@ -1767,8 +1778,9 @@ mixin SparkInterface throw Exception("Change index out of range"); } + final changeAddress = await getMintChangeAddress(); vout.insert(nChangePosInOut, ( - changeAddress!.value, + changeAddress.value, nChange.toInt(), null, )); @@ -1863,17 +1875,17 @@ mixin SparkInterface throw Exception("Transaction too large"); } - // ECDSA DER signature lengths vary by up to ~4 bytes per input - // (r randomly flips the 0x80 bit → 32 vs 33 bytes; s varies similarly - // within low-S bounds). The dummy tx above is signed with real keys - // over different data than the final real tx, so their vSizes differ - // by up to ~4 bytes per input. Scale the safety buffer with input - // count so the estimated fee always covers the final signed tx. + // ECDSA DER signatures are not fixed-size. Even with low-S + // normalization, the encoded signature length can vary across + // signatures, so the dummy signed transaction used for fee estimation + // can be smaller than the final signed transaction. Use a per-input + // safety margin so fee estimation remains an upper bound for many-input + // Spark mints. final nBytesBuffer = 10 + 4 * setCoins.length; final nFeeNeeded = BigInt.from( estimateTxFee( vSize: nBytes + nBytesBuffer, - feeRatePerKB: feesObject.medium, + feeRatePerKB: mintFeeRatePerKB, ), ); @@ -2120,9 +2132,8 @@ mixin SparkInterface ); Logging.instance.i("nFeeRet=$nFeeRet, vSize=${data.vSize}"); - // fee_sats < vSize_bytes ⟺ feeRate < 1 sat/byte (the standard minimum - // relay fee). Firing here means feesObject.medium came back below that - // threshold, not that the buffer underestimated the real tx size. + // Sanity check: with the fee rate clamped to at least 1 sat/vbyte, this + // should only fire if fee accounting or size estimation regresses. if (nFeeRet.toInt() < data.vSize!) { Logging.instance.w( "Fee rate below 1 sat/byte minimum relay fee: " @@ -2555,6 +2566,11 @@ BigInt _sum(List utxos) => utxos .map((e) => BigInt.from(e.value)) .fold(BigInt.zero, (previousValue, element) => previousValue + element); +typedef _SparkMintSigningKey = ({ + DerivePathType derivePathType, + coinlib.HDPrivateKey key, +}); + class MutableSparkRecipient { String address; BigInt value; From 9bcba7d7ca3cfc9e684e1fa2c134aa21d1f09878 Mon Sep 17 00:00:00 2001 From: Reuben Yap Date: Mon, 27 Apr 2026 23:58:11 +0800 Subject: [PATCH 3/3] Fix Spark mint fee subtraction edge case --- .../spark_interface.dart | 50 ++++++++++--------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart index e3c02ee66..96a63e088 100644 --- a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart +++ b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart @@ -1664,11 +1664,7 @@ mixin SparkInterface if (autoMintAll) { singleTxOutputs.add( - MutableSparkRecipient( - autoMintSparkAddress!, - mintedValue, - "", - ), + MutableSparkRecipient(autoMintSparkAddress!, mintedValue, ""), ); } else { BigInt remainingMintValue = BigInt.parse(mintedValue.toString()); @@ -1696,26 +1692,34 @@ mixin SparkInterface } } - if (subtractFeeFromAmount) { - final BigInt singleFee = - nFeeRet ~/ BigInt.from(singleTxOutputs.length); - BigInt remainder = nFeeRet % BigInt.from(singleTxOutputs.length); - - for (int i = 0; i < singleTxOutputs.length; ++i) { - if (singleTxOutputs[i].value <= singleFee) { - final removed = singleTxOutputs.removeAt(i); - remainder += removed.value - singleFee; - --i; - continue; + if (subtractFeeFromAmount && nFeeRet > BigInt.zero) { + var remainingFee = nFeeRet; + var outputIndex = 0; + while (outputIndex < singleTxOutputs.length && + remainingFee > BigInt.zero) { + final outputsLeft = BigInt.from( + singleTxOutputs.length - outputIndex, + ); + var feeShare = remainingFee ~/ outputsLeft; + if (remainingFee % outputsLeft != BigInt.zero) { + feeShare += BigInt.one; } - singleTxOutputs[i].value -= singleFee; - if (remainder > BigInt.zero && - singleTxOutputs[i].value > - nFeeRet % BigInt.from(singleTxOutputs.length)) { - // first receiver pays the remainder not divisible by output count - singleTxOutputs[i].value -= remainder; - remainder = BigInt.zero; + + if (singleTxOutputs[outputIndex].value <= feeShare) { + remainingFee -= singleTxOutputs[outputIndex].value; + singleTxOutputs.removeAt(outputIndex); + continue; } + + singleTxOutputs[outputIndex].value -= feeShare; + remainingFee -= feeShare; + ++outputIndex; + } + + if (singleTxOutputs.isEmpty) { + valueAndUTXOs.remove(itr); + skipCoin = true; + break; } }