Skip to content

Commit cf8525d

Browse files
reubenyapclaude
andcommitted
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) <noreply@anthropic.com>
1 parent 243015b commit cf8525d

1 file changed

Lines changed: 83 additions & 24 deletions

File tree

lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart

Lines changed: 83 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import 'dart:isolate';
44
import 'dart:math';
55

66
import 'package:bitcoindart/bitcoindart.dart' as btc;
7+
import 'package:coinlib_flutter/coinlib_flutter.dart' as coinlib;
78
import 'package:decimal/decimal.dart';
89
import 'package:flutter/foundation.dart';
910
import 'package:isar_community/isar.dart';
@@ -1550,6 +1551,55 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
15501551
final random = Random.secure();
15511552
final List<TxData> results = [];
15521553

1554+
// Pre-compute signing keys for all UTXOs to avoid repeated calls to
1555+
// getRootHDNode() (which re-derives from mnemonic seed each time) and
1556+
// individual DB lookups inside the hot loop.
1557+
final root = await getRootHDNode();
1558+
final Map<String, ({DerivePathType derivePathType, coinlib.HDPrivateKey key})>
1559+
signingKeyCache = {};
1560+
for (final utxo in availableUtxos) {
1561+
final address = utxo.address!;
1562+
if (!signingKeyCache.containsKey(address)) {
1563+
final derivePathType = cryptoCurrency.addressType(address: address);
1564+
final dbAddress = await mainDB.getAddress(walletId, address);
1565+
if (dbAddress?.derivationPath != null) {
1566+
final key = root.derivePath(dbAddress!.derivationPath!.value);
1567+
signingKeyCache[address] =
1568+
(derivePathType: derivePathType, key: key);
1569+
}
1570+
}
1571+
}
1572+
1573+
// Cache addresses used repeatedly inside the loop.
1574+
final sparkAddress = (await getCurrentReceivingSparkAddress())!.value;
1575+
final changeAddress = await getCurrentChangeAddress();
1576+
1577+
// Pre-cache the change address signing key so change UTXOs that get
1578+
// recycled back into valueAndUTXOs can be signed without re-deriving.
1579+
if (changeAddress != null &&
1580+
!signingKeyCache.containsKey(changeAddress.value)) {
1581+
final derivePathType = cryptoCurrency.addressType(
1582+
address: changeAddress.value,
1583+
);
1584+
final dbAddress = await mainDB.getAddress(
1585+
walletId,
1586+
changeAddress.value,
1587+
);
1588+
if (dbAddress?.derivationPath != null) {
1589+
final key = root.derivePath(dbAddress!.derivationPath!.value);
1590+
signingKeyCache[changeAddress.value] =
1591+
(derivePathType: derivePathType, key: key);
1592+
}
1593+
}
1594+
1595+
// Pre-fetch wallet-owned addresses for output ownership checks.
1596+
final walletAddresses = await mainDB.isar.addresses
1597+
.where()
1598+
.walletIdEqualTo(walletId)
1599+
.valueProperty()
1600+
.findAll();
1601+
final walletAddressSet = walletAddresses.toSet();
1602+
15531603
valueAndUTXOs.shuffle(random);
15541604

15551605
while (valueAndUTXOs.isNotEmpty) {
@@ -1590,7 +1640,7 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
15901640
}
15911641

15921642
// if (!MoneyRange(mintedValue) || mintedValue == 0) {
1593-
if (mintedValue == BigInt.zero) {
1643+
if (mintedValue <= BigInt.zero) {
15941644
valueAndUTXOs.remove(itr);
15951645
skipCoin = true;
15961646
break;
@@ -1610,7 +1660,7 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
16101660
if (autoMintAll) {
16111661
singleTxOutputs.add(
16121662
MutableSparkRecipient(
1613-
(await getCurrentReceivingSparkAddress())!.value,
1663+
sparkAddress,
16141664
mintedValue,
16151665
"",
16161666
),
@@ -1694,11 +1744,19 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
16941744
BigInt nValueIn = BigInt.zero;
16951745
for (final utxo in itr) {
16961746
if (nValueToSelect > nValueIn) {
1697-
setCoins.add(
1698-
(await addSigningKeys([
1699-
StandardInput(utxo),
1700-
])).whereType<StandardInput>().first,
1747+
final cached = signingKeyCache[utxo.address!];
1748+
if (cached == null) {
1749+
throw Exception(
1750+
"Signing key not found for address ${utxo.address}. "
1751+
"Local db may be corrupt. Rescan wallet.",
1752+
);
1753+
}
1754+
final input = StandardInput(
1755+
utxo,
1756+
derivePathType: cached.derivePathType,
17011757
);
1758+
input.key = cached.key;
1759+
setCoins.add(input);
17021760
nValueIn += BigInt.from(utxo.value);
17031761
}
17041762
}
@@ -1720,7 +1778,6 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
17201778
throw Exception("Change index out of range");
17211779
}
17221780

1723-
final changeAddress = await getCurrentChangeAddress();
17241781
vout.insert(nChangePosInOut, (
17251782
changeAddress!.value,
17261783
nChange.toInt(),
@@ -1817,13 +1874,19 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
18171874
throw Exception("Transaction too large");
18181875
}
18191876

1820-
const nBytesBuffer = 10;
1877+
// ECDSA DER signature lengths vary by up to ~4 bytes per input
1878+
// (r randomly flips the 0x80 bit → 32 vs 33 bytes; s varies similarly
1879+
// within low-S bounds). The dummy tx above is signed with real keys
1880+
// over different data than the final real tx, so their vSizes differ
1881+
// by up to ~4 bytes per input. Scale the safety buffer with input
1882+
// count so the estimated fee always covers the final signed tx.
1883+
final nBytesBuffer = 10 + 4 * setCoins.length;
18211884
final nFeeNeeded = BigInt.from(
18221885
estimateTxFee(
18231886
vSize: nBytes + nBytesBuffer,
18241887
feeRatePerKB: feesObject.medium,
18251888
),
1826-
); // One day we'll do this properly
1889+
);
18271890

18281891
if (nFeeRet >= nFeeNeeded) {
18291892
for (final usedCoin in setCoins) {
@@ -1984,19 +2047,11 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
19842047
addresses: [
19852048
if (addressOrScript is String) addressOrScript.toString(),
19862049
],
1987-
walletOwns:
1988-
(await mainDB.isar.addresses
1989-
.where()
1990-
.walletIdEqualTo(walletId)
1991-
.filter()
1992-
.valueEqualTo(
1993-
addressOrScript is Uint8List
1994-
? output.$3!
1995-
: addressOrScript as String,
1996-
)
1997-
.valueProperty()
1998-
.findFirst()) !=
1999-
null,
2050+
walletOwns: walletAddressSet.contains(
2051+
addressOrScript is Uint8List
2052+
? output.$3!
2053+
: addressOrScript as String,
2054+
),
20002055
),
20012056
);
20022057
}
@@ -2076,11 +2131,15 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
20762131
);
20772132

20782133
Logging.instance.i("nFeeRet=$nFeeRet, vSize=${data.vSize}");
2134+
// fee_sats < vSize_bytes ⟺ feeRate < 1 sat/byte (the standard minimum
2135+
// relay fee). Firing here means feesObject.medium came back below that
2136+
// threshold, not that the buffer underestimated the real tx size.
20792137
if (nFeeRet.toInt() < data.vSize!) {
20802138
Logging.instance.w(
2081-
"Spark mint transaction failed: $nFeeRet is less than ${data.vSize}",
2139+
"Fee rate below 1 sat/byte minimum relay fee: "
2140+
"fee=$nFeeRet sats, vSize=${data.vSize} bytes",
20822141
);
2083-
throw Exception("fee is less than vSize");
2142+
throw Exception("Fee rate below 1 sat/byte minimum relay fee");
20842143
}
20852144

20862145
results.add(data);

0 commit comments

Comments
 (0)