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

Fix for change and tiny fee calculations #112

Merged
merged 4 commits into from May 5, 2022
Merged

Conversation

theflyingcodr
Copy link
Contributor

With the change to tiny dust limits and 0.05 sat / byte tx fee rates some of the rounding needed re-visited as off by one bugs were noticed when calculating change.

This has been remedied by using floating point as much as possible and using an int conversion as late as possible to ensure accurate figures.

This has resulted in some tests that had questionable results now becoming more accurate also.

… some of the rounding needed re-visted as off by one bugs were noticed when calculating change. This has been remedied by using floating point as much as possible and using an int conversion as late as possible to ensure accurate figures. THis has resulted in some tests that had questionable results now becoming more accurate also.
@theflyingcodr theflyingcodr self-assigned this May 4, 2022
@mergify mergify bot added the bug-P3 label May 4, 2022
@codecov-commenter
Copy link

codecov-commenter commented May 4, 2022

Codecov Report

Merging #112 (f13149f) into master (0d68685) will decrease coverage by 0.01%.
The diff coverage is 88.00%.

@@            Coverage Diff             @@
##           master     #112      +/-   ##
==========================================
- Coverage   83.84%   83.82%   -0.02%     
==========================================
  Files          34       34              
  Lines        3676     3672       -4     
==========================================
- Hits         3082     3078       -4     
  Misses        427      427              
  Partials      167      167              
Impacted Files Coverage Δ
txchange.go 80.76% <85.00%> (-1.99%) ⬇️
tx.go 78.68% <100.00%> (+0.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d68685...f13149f. Read the comment docs.

Copy link
Member

@jadwahab jadwahab left a comment

Choose a reason for hiding this comment

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

doesn't feel right to me to use floats, let's discuss more

@mergify mergify bot merged commit 841602a into master May 5, 2022
@mergify mergify bot deleted the fix/change_rounding branch May 5, 2022 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants