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

test(electrum): test for checking that fee calculation is correct #1685

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

f3r10
Copy link

@f3r10 f3r10 commented Nov 13, 2024

Description

This PR adds a unit test for checking that the fee calculation of bdk_electrum is correct.

a "preliminary" tx to an address in Core's wallet such that the output created becomes the input of the next tx. This preliminary tx is the prev_tx that needs to be fetched in order to calculate the fee of the tx sent to the receiver. This way is better because the required outpoint is less determined. Then we assert we have the right (outpoint, txout) in the update TxGraph #1444 (comment)

Notes to the reviewers

fixes: #1444

Changelog notice

Checklists

All Submissions:

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

@f3r10 f3r10 marked this pull request as ready for review November 14, 2024 18:28
@notmandatory notmandatory added this to the 1.0.0-beta milestone Nov 15, 2024
@LagginTimes
Copy link
Contributor

LagginTimes commented Nov 15, 2024

Thank you for taking this on! It looks good to me so far.
Just a suggestion: squash your second commit into the first to keep the history cleaner.

Edit: Also just a reminder to address the clippy warning.

@notmandatory notmandatory removed this from the 1.0.0-beta milestone Nov 21, 2024
@f3r10 f3r10 force-pushed the test_electrum_fee_calculation branch from 5ed5266 to 99888fa Compare November 25, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Needs Review
Development

Successfully merging this pull request may close these issues.

More tests for bdk_electrum fee calculation
3 participants