What changed, and why it matters
This commit revises how Stack Wallet builds Spark mint transactions. The visible changes fix a fee-rate floor bug, correct a loop that could remove the wrong recipient when outputs are too small, and tighten error handling when signing keys or change addresses are missing. It also replaces an eager key-caching approach with lazy caching. The commit is framed as responding to an external review, but no security incident or CVE is mentioned in the materials provided.
Treat this as a routine hardening/review-response patch. Users who build Spark mint transactions should update to the patched version. A deeper audit of the Spark mint fee-estimation and coin-selection logic is advisable because the commit is framed as review feedback and may be partial. No emergency response is warranted based solely on the diff.
Security signals we found
Fee-rate clamped to minimum relay fee to prevent transactions that would not propagate
Fixed list-remove-then-index bug that could mis-account recipient values during output pruning
Added explicit exceptions for missing signing keys and missing change addresses instead of silent null handling
Lazy signing-key derivation reduces exposure of derived keys but is a code-quality/performance change, not a vulnerability fix by itself
Commentary explicitly references ECDSA DER signature length variance and fee-estimation safety margin
Evidence from the diff
The patch modifies lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart. Key technical changes: (1) introduces minRelayFeeRatePerKB = 1000 and clamps the mint fee rate to at least that value, fixing a path where feesObject.medium below 1 sat/vbyte could produce a sub-relay transaction; (2) fixes an off-by-one/use-after-remove bug in the recipient-pruning loop where singleTxOutputs[i].value was read after removeAt(i) shifted the list; (3) makes signing-key lookup lazy and throws explicitly if a derivation path is missing, rather than silently leaving a null cache entry; (4) defers and caches the change address lookup; (5) updates comments to explain the ECDSA DER signature size variance buffer and the fee sanity check. The commit title/message only say ‘Address Spark mint review feedback’ and do not claim a security fix.
Changed components
Spark mint transaction construction in Stack Walletlib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dartFiro Spark transparent-input signing pathInspect captured patch +58 / −42
diff --git a/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart b/lib/wallets/wallet/wallet_mixin_interfaces/spark_interface.dart
index 03f4925..e3c02ee 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<T extends ElectrumXCurrencyInterface>
.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<TxData> 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<String, ({DerivePathType derivePathType, coinlib.HDPrivateKey key})>
- signingKeyCache = {};
- Future<void> cacheSigningKey(String address) async {
- if (signingKeyCache.containsKey(address)) return;
+ final Map<String, _SparkMintSigningKey> 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<Address> 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<T extends ElectrumXCurrencyInterface>
if (autoMintAll) {
singleTxOutputs.add(
MutableSparkRecipient(
- sparkAddress,
+ autoMintSparkAddress!,
mintedValue,
"",
),
@@ -1687,9 +1703,10 @@ mixin SparkInterface<T extends ElectrumXCurrencyInterface>
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<T extends ElectrumXCurrencyInterface>
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<T extends ElectrumXCurrencyInterface>
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<T extends ElectrumXCurrencyInterface>
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<T extends ElectrumXCurrencyInterface>
);
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<UTXO> 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;
Why this scored 41/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.