Skip to content

[wallet] Fix details.fees being wrong when change is dust#255

Merged
notmandatory merged 1 commit intobitcoindevkit:masterfrom
LLFourn:dust_change_fix
Jan 4, 2021
Merged

[wallet] Fix details.fees being wrong when change is dust#255
notmandatory merged 1 commit intobitcoindevkit:masterfrom
LLFourn:dust_change_fix

Conversation

@LLFourn
Copy link
Copy Markdown
Collaborator

@LLFourn LLFourn commented Dec 29, 2020

This is a bug I found while doing other things. The TransactionDetails fee field will be wrong if the change is omitted because it's below the dust threshold.

All Submissions:

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

Bugfixes:

  • I've added tests to reproduce the issue which are now passing

@notmandatory
Copy link
Copy Markdown
Member

ACK 0c9c071

nice catch, I double checked that fee_amount is 141 in this test scenario, so change_val is 59 (50,000-49,800-141) and thus correct final fee amount is 141 + 59, not 141 + 200 as it was calculating before.

@afilini
Copy link
Copy Markdown
Member

afilini commented Jan 4, 2021

ACK 0c9c071

Sorry it took me a while to answer, I decided to take a break after the release to relax and distract myself a little bit.. Over the next few days I'll start working regularly like I was doing before

@notmandatory notmandatory merged commit 0c9c071 into bitcoindevkit:master Jan 4, 2021
@LLFourn LLFourn deleted the dust_change_fix branch January 4, 2021 21:20
@LLFourn
Copy link
Copy Markdown
Collaborator Author

LLFourn commented Jan 4, 2021

Thanks. @afilini No rush whatsoever. I hope you all had a great break.

nickfarrow pushed a commit to nickfarrow/bdk that referenced this pull request Feb 21, 2022
…(Pr 2 of 3)

42e4c50 Cleanup context code with SigType enum (sanket1729)
e70e6a3 Fix fuzzer crash while allocating keys for multi_a (sanket1729)
f6b2536 Add satisfaction tests (sanket1729)
1a7e779 Change satisfier to support xonly sigs (sanket1729)
2c32c03 Use bitcoin::SchnorrSig type from rust-bitcoin (sanket1729)
e26d4e6 Use bitcoin::EcdsaSig from rust-bitcoin (sanket1729)
89e7cb3 add multi_a (sanket1729)

Pull request description:

  Adds support for MultiA script fragment. Builds on top of bitcoindevkit#255 .

  Do not merge this until we have a rust-bitcoin release

ACKs for top commit:
  apoelstra:
    ACK 42e4c50

Tree-SHA512: 10701266cf6fe436fe07359e67d16bc925ebbcd767d4e5a8d325db61b979bd76b1e0291dd9eae5b7d58f6a385f8f2e16c2657957a597ce18736382b776e20502
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.

3 participants