Optimize Firo Spark mint tx generation and fix fee < vSize error#1295
Open
reubenyap wants to merge 4 commits intocypherstack:stagingfrom
Open
Optimize Firo Spark mint tx generation and fix fee < vSize error#1295reubenyap wants to merge 4 commits intocypherstack:stagingfrom
reubenyap wants to merge 4 commits intocypherstack:stagingfrom
Conversation
Performance: Pre-compute signing keys, addresses, and wallet ownership data before the main loop instead of doing expensive async operations (getRootHDNode, DB lookups, key derivation) per-UTXO per-fee-iteration. With many inputs this eliminates N*M redundant mnemonic-to-seed derivations and database queries. Bug fix: When the real transaction (generate:true) is larger than the dummy (generate:false) used for fee estimation, retry with a fee based on the real vSize instead of throwing "fee is less than vSize". https://claude.ai/code/session_01YHRu2KAxZW26VnP1bZKZoX
When a fee retry caused a UTXO group to be skipped (mintedValue hit zero due to the higher fee floor), minFeeForGroup and feeRetryCount were not reset before moving to the next group. This could cause subsequent groups to start with an inflated fee floor from a previous larger group's transaction, potentially skipping groups unnecessarily or reducing their retry budget. https://claude.ai/code/session_01D9ssjMkQAMoCUcrPzVVtEq
…vout - Add 10-byte vSize buffer to retry fee calculation to match the nBytesBuffer used in the inner fee estimation loop, avoiding a potential unnecessary extra retry. - Restore `final` on vin/vout declarations since they are never reassigned (only cleared via .clear()). https://claude.ai/code/session_01D9ssjMkQAMoCUcrPzVVtEq
The guard at the top of the inner fee loop only checked mintedValue == BigInt.zero, but mintedValue can go negative when nFeeRet (starting from minFeeForGroup on retry) exceeds the UTXO group's total value. A negative mintedValue would flow into createSparkMintRecipients causing invalid outputs. Change the check to <= BigInt.zero to match the intent of the commented-out MoneyRange check from the C++ reference implementation. https://claude.ai/code/session_01D9ssjMkQAMoCUcrPzVVtEq
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Performance: Pre-compute signing keys, addresses, and wallet ownership data before the main loop instead of doing expensive async operations (getRootHDNode, DB lookups, key derivation) per-UTXO per-fee-iteration. With many inputs this eliminates N*M redundant mnemonic-to-seed derivations and database queries.
Bug fix: When the real transaction (generate:true) is larger than the dummy (generate:false) used for fee estimation, retry with a fee based on the real vSize instead of throwing "fee is less than vSize".