Skip to content

Commit 51d4357

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 51d4357

1 file changed

Lines changed: 72 additions & 24 deletions

File tree

lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart

Lines changed: 72 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,44 @@ 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+
Future<void> cacheSigningKey(String address) async {
1561+
if (signingKeyCache.containsKey(address)) return;
1562+
final derivePathType = cryptoCurrency.addressType(address: address);
1563+
final dbAddress = await mainDB.getAddress(walletId, address);
1564+
if (dbAddress?.derivationPath != null) {
1565+
final key = root.derivePath(dbAddress!.derivationPath!.value);
1566+
signingKeyCache[address] = (derivePathType: derivePathType, key: key);
1567+
}
1568+
}
1569+
1570+
for (final utxo in availableUtxos) {
1571+
await cacheSigningKey(utxo.address!);
1572+
}
1573+
1574+
// Cache addresses used repeatedly inside the loop.
1575+
final sparkAddress = (await getCurrentReceivingSparkAddress())!.value;
1576+
final changeAddress = await getCurrentChangeAddress();
1577+
1578+
// Pre-cache the change address signing key so change UTXOs that get
1579+
// recycled back into valueAndUTXOs can be signed without re-deriving.
1580+
if (changeAddress != null) {
1581+
await cacheSigningKey(changeAddress.value);
1582+
}
1583+
1584+
// Pre-fetch wallet-owned addresses for output ownership checks.
1585+
final walletAddresses = await mainDB.isar.addresses
1586+
.where()
1587+
.walletIdEqualTo(walletId)
1588+
.valueProperty()
1589+
.findAll();
1590+
final walletAddressSet = walletAddresses.toSet();
1591+
15531592
valueAndUTXOs.shuffle(random);
15541593

15551594
while (valueAndUTXOs.isNotEmpty) {
@@ -1590,7 +1629,7 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
15901629
}
15911630

15921631
// if (!MoneyRange(mintedValue) || mintedValue == 0) {
1593-
if (mintedValue == BigInt.zero) {
1632+
if (mintedValue <= BigInt.zero) {
15941633
valueAndUTXOs.remove(itr);
15951634
skipCoin = true;
15961635
break;
@@ -1610,7 +1649,7 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
16101649
if (autoMintAll) {
16111650
singleTxOutputs.add(
16121651
MutableSparkRecipient(
1613-
(await getCurrentReceivingSparkAddress())!.value,
1652+
sparkAddress,
16141653
mintedValue,
16151654
"",
16161655
),
@@ -1694,11 +1733,19 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
16941733
BigInt nValueIn = BigInt.zero;
16951734
for (final utxo in itr) {
16961735
if (nValueToSelect > nValueIn) {
1697-
setCoins.add(
1698-
(await addSigningKeys([
1699-
StandardInput(utxo),
1700-
])).whereType<StandardInput>().first,
1736+
final cached = signingKeyCache[utxo.address!];
1737+
if (cached == null) {
1738+
throw Exception(
1739+
"Signing key not found for address ${utxo.address}. "
1740+
"Local db may be corrupt. Rescan wallet.",
1741+
);
1742+
}
1743+
final input = StandardInput(
1744+
utxo,
1745+
derivePathType: cached.derivePathType,
17011746
);
1747+
input.key = cached.key;
1748+
setCoins.add(input);
17021749
nValueIn += BigInt.from(utxo.value);
17031750
}
17041751
}
@@ -1720,7 +1767,6 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
17201767
throw Exception("Change index out of range");
17211768
}
17221769

1723-
final changeAddress = await getCurrentChangeAddress();
17241770
vout.insert(nChangePosInOut, (
17251771
changeAddress!.value,
17261772
nChange.toInt(),
@@ -1817,13 +1863,19 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
18171863
throw Exception("Transaction too large");
18181864
}
18191865

1820-
const nBytesBuffer = 10;
1866+
// ECDSA DER signature lengths vary by up to ~4 bytes per input
1867+
// (r randomly flips the 0x80 bit → 32 vs 33 bytes; s varies similarly
1868+
// within low-S bounds). The dummy tx above is signed with real keys
1869+
// over different data than the final real tx, so their vSizes differ
1870+
// by up to ~4 bytes per input. Scale the safety buffer with input
1871+
// count so the estimated fee always covers the final signed tx.
1872+
final nBytesBuffer = 10 + 4 * setCoins.length;
18211873
final nFeeNeeded = BigInt.from(
18221874
estimateTxFee(
18231875
vSize: nBytes + nBytesBuffer,
18241876
feeRatePerKB: feesObject.medium,
18251877
),
1826-
); // One day we'll do this properly
1878+
);
18271879

18281880
if (nFeeRet >= nFeeNeeded) {
18291881
for (final usedCoin in setCoins) {
@@ -1984,19 +2036,11 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
19842036
addresses: [
19852037
if (addressOrScript is String) addressOrScript.toString(),
19862038
],
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,
2039+
walletOwns: walletAddressSet.contains(
2040+
addressOrScript is Uint8List
2041+
? output.$3!
2042+
: addressOrScript as String,
2043+
),
20002044
),
20012045
);
20022046
}
@@ -2076,11 +2120,15 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
20762120
);
20772121

20782122
Logging.instance.i("nFeeRet=$nFeeRet, vSize=${data.vSize}");
2123+
// fee_sats < vSize_bytes ⟺ feeRate < 1 sat/byte (the standard minimum
2124+
// relay fee). Firing here means feesObject.medium came back below that
2125+
// threshold, not that the buffer underestimated the real tx size.
20792126
if (nFeeRet.toInt() < data.vSize!) {
20802127
Logging.instance.w(
2081-
"Spark mint transaction failed: $nFeeRet is less than ${data.vSize}",
2128+
"Fee rate below 1 sat/byte minimum relay fee: "
2129+
"fee=$nFeeRet sats, vSize=${data.vSize} bytes",
20822130
);
2083-
throw Exception("fee is less than vSize");
2131+
throw Exception("Fee rate below 1 sat/byte minimum relay fee");
20842132
}
20852133

20862134
results.add(data);

0 commit comments

Comments
 (0)