Skip to content

Fix P2WPKH_SATISFACTION_SIZE in CS tests#723

Merged
afilini merged 1 commit intobitcoindevkit:masterfrom
danielabrozzoni:fix/coin_selection_fee
Aug 17, 2022
Merged

Fix P2WPKH_SATISFACTION_SIZE in CS tests#723
afilini merged 1 commit intobitcoindevkit:masterfrom
danielabrozzoni:fix/coin_selection_fee

Conversation

@danielabrozzoni
Copy link
Copy Markdown
Member

Our costant for the P2WPKH satisfaction size was wrong: in
7ac87b8 we added 1 WU for the script
sig len - but actually, that's 4WU! This resulted in
P2WPKH_SATISFACTION_SIZE being equal to 109 instead of 112.
This also adds a comment for better readability.

Description

Notes to the reviewers

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature
  • I've updated CHANGELOG.md

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@danielabrozzoni danielabrozzoni self-assigned this Aug 16, 2022
@danielabrozzoni danielabrozzoni added the bug Something isn't working label Aug 16, 2022
Our costant for the P2WPKH satisfaction size was wrong: in
7ac87b8 we added 1 WU for the script
sig len - but actually, that's 4WU! This resulted in
P2WPKH_SATISFACTION_SIZE being equal to 109 instead of 112.
This also adds a comment for better readability.
@csralvall
Copy link
Copy Markdown
Contributor

utACK cd07890

Copy link
Copy Markdown
Member

@afilini afilini left a comment

Choose a reason for hiding this comment

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

ACK cd07890

@afilini afilini merged commit c9b1b6d into bitcoindevkit:master Aug 17, 2022
@danielabrozzoni danielabrozzoni deleted the fix/coin_selection_fee branch October 11, 2023 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants