AI-generated analysisPublished automatically and not human-verified. Validated context appears in community notes below.
← Watch feed
Low 44 Monero

Optimize Firo Spark mint tx generation and fix fee < vSize error

Public commit record

What the developer wrote

Authored by Reuben Yap

78/100 · Adequate
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>
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Names security-relevant behavior explicitly
The short version

What changed, and why it matters

This commit fixes a bug in Stack Wallet's Firo Spark coin-minting code where the estimated transaction fee could be too small because the temporary 'dummy' transaction used for fee estimation had shorter digital signatures than the final real transaction. With many inputs, the difference could exceed the old fixed 10-byte safety buffer, causing the wallet to create transactions that failed a minimum-fee check and could not be broadcast. The patch scales the buffer with the number of inputs, pre-computes expensive key derivations for performance, and also catches a negative-mint-amount edge case. There is no direct evidence this was exploited as an attack; it appears to be a reliability/DoS bug for the wallet user.

Recommended action

Review and merge the patch, then verify with multi-input Spark mint tests that nFeeRet consistently covers final vSize. Consider adding a regression test that deliberately produces high-r/high-s signatures to exercise the buffer. Audit other coin implementations for the same constant-buffer anti-pattern.

Security signals we found

01

Transaction fee underestimation due to variable-length ECDSA DER signatures between dummy and real signing rounds

02

Minimum relay fee check (nFeeRet < data.vSize) could reject user-created mint transactions

03

Negative mintedValue not caught by equality check, potentially allowing invalid transaction construction

04

Performance optimization removes repeated mnemonic-to-seed derivation and per-UTXO DB lookups from nested loops

Risk score

Why this scored 44/100

Our methodology →
Potential impact 12/30
Exploitability 8/25
Stealth signal 6/15
Affected reach 7/15
Confidence 7/10
Evidence quality 4/5
Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.