Fix fee rate calculation and add fee display to verify-psbt#3936
Merged
Conversation
- Fix inaccurate vsize estimation in build-tx when using --max-inputs - Replace hardcoded formula with accurate TransactionBuilder::estimate_vbytes_with() - Remove unnecessary 1000 satoshi deduction from fee calculation - Add fee, fee rate, and vsize display to verify-psbt command This fixes the issue where transactions built with --max-inputs had only 19 satoshis fee instead of the expected ~19,000 satoshis for 1 sat/vbyte rate. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Bitcoin transaction fee calculation issues in the build-tx and verify-psbt commands. The vsize estimation was previously using an inaccurate hardcoded formula, causing severely underestimated fees (e.g., 19 satoshis instead of ~19,000 satoshis for a 114-input transaction). The PR replaces the hardcoded calculation with the accurate TransactionBuilder::estimate_vbytes_with() method and adds fee display functionality to verify-psbt.
Changes:
- Fixed vsize estimation in build-tx by replacing hardcoded formula with TransactionBuilder's accurate estimation method
- Made estimate_vbytes_with() public to enable reuse across modules
- Added fee, fee rate, and vsize display to verify-psbt command
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| crates/rooch/src/commands/bitcoin/transaction_builder.rs | Made estimate_vbytes_with() method public and added documentation |
| crates/rooch/src/commands/bitcoin/build_tx.rs | Replaced hardcoded vsize formula with TransactionBuilder::estimate_vbytes_with() and removed unnecessary 1000 satoshi deduction |
| crates/rooch/src/commands/bitcoin/verify_psbt.rs | Added total_input_sat and total_output_sat fields to calculate and display fee information |
- Use saturating_add to prevent overflow on large transaction amounts - Fix fee display condition to only show when input data is available - Add documentation explaining conservative vsize estimation (assumes 2 outputs) - Add support for non_witness_utxo in fee calculation These changes improve the robustness and accuracy of the fee calculation and display in verify-psbt command. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
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
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.
Summary
Fixes two issues related to Bitcoin transaction fee calculation:
build-txwhen using--max-inputsverify-psbtcommandProblem
When using
--max-inputsparameter inrooch bitcoin build-tx, the fee calculation used a simplified hardcoded formula:This resulted in severely underestimated vsize (e.g., 7,026 bytes vs actual 18,990 bytes for 114 inputs), causing transactions to have only 19 satoshis fee instead of the expected ~19,000 satoshis for a 1 sat/vbyte rate.
Changes
build_tx.rs
TransactionBuilder::estimate_vbytes_with()methodtransaction_builder.rs
estimate_vbytes_with()public to allow reuse in build_tx.rsverify_psbt.rs
total_input_satandtotal_output_satfields to track transaction amountsTesting
Verified the fix by:
verify-psbtnow displays fee information:Fixes #XXX