Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

commands: add weights before converting to vbytes #734

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

jp1ac4
Copy link
Collaborator

@jp1ac4 jp1ac4 commented Oct 20, 2023

The sanity_check_psbt() function checks, among other things, that the feerate in sats/vbytes is in the required range. When estimating the transaction size, converting each part to vbytes first can lead to a larger result due to rounding multiple times and therefore a lower feerate, which could fall below 1 and fail the sanity check.

This issue has arisen in #560 when creating a transaction using coin selection.

Converting to vbytes first can lead to a larger result and
therefore a lower feerate, which could fall below 1 and fail the
sanity check.
Copy link
Member

@darosior darosior left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 176859e -- great catch

@@ -17,7 +17,7 @@ pub use keys::*;
pub mod analysis;
pub use analysis::*;

const WITNESS_FACTOR: usize = 4;
pub const WITNESS_FACTOR: usize = 4;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future we should avoid rolling our own constant and instead use rust-bitcoin's.

@darosior darosior merged commit a4183dd into wizardsardine:master Oct 20, 2023
18 checks passed
@jp1ac4 jp1ac4 deleted the sanity-check-wu-vb-conversion branch October 20, 2023 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants