-
Notifications
You must be signed in to change notification settings - Fork 92
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
change(mempool): Refactor the ZIP-317 minimum fee rate calculation to use usize #6585
Conversation
let min_fee = (MIN_MEMPOOL_TX_FEE_RATE * transaction_size / KILOBYTE) | ||
.clamp(MIN_MEMPOOL_TX_FEE_RATE, MEMPOOL_TX_FEE_REQUIREMENT_CAP); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seemed easier to do this calculation in usize
then convert to Amount
, which is what zcashd
does.
Otherwise every calculation has to be checked for overflow, even though we clamp to [100, 1000]
at the end anyway.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## issue5336 #6585 +/- ##
=============================================
+ Coverage 77.90% 77.99% +0.09%
=============================================
Files 308 308
Lines 40583 40594 +11
=============================================
+ Hits 31615 31662 +47
+ Misses 8968 8932 -36 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
* add ZIP-317 rules to mempool * fix some docs * rustfmt * fix import * typo * fix tests * fix tests 2 * fix tests 3 * fix tests 4 * fix tests 5 * move constant * fix constant for feature * document/quote zip rules * add Minimum Fee Rate rule * change(mempool): Refactor the ZIP-317 minimum fee rate calculation to use usize (#6585) * Refactor the minimum fee rate calculation to use usize * Check for overflow if constants change * remove 1 rule check, fix docs --------- Co-authored-by: teor <teor@riseup.net>
Motivation
@oxarbitrage asked for some help refactoring this.
Specifications
See the links in the code.
Solution
zcashd
code, but make it simplerusize
for all the calculationsThis might break the tests, I'll let you update them all at once in whatever way you'd like, to avoid merge issues.
Review
Alfredo asked for this PR.
Feel free to merge or copy-paste.