added tests for calculate_fee(), calculate_waste() and effective_value() #25
Conversation
| .value | ||
| .saturating_sub(calculate_fee(output.weight, feerate)) | ||
| .saturating_sub(calculate_fee(output.weight, feerate)?)) |
There was a problem hiding this comment.
A better name for this could be output.spending_weight. Weight can imply two things, the weight of the output, or the weight of the spending transaction's input. We need the second weight here.
There was a problem hiding this comment.
I agree that spending_weight is a better name than weight. Perhaps input_weight or utxo_weight would be other options to consider. Anything to differentiate the two would improve the readability of the code.
There was a problem hiding this comment.
utACK 4ebff2c
The test structure looks good to me. Although I haven't ran or checked the calculations.
One small nit remaining.
ACK from me module @delcin-raj 's review.
| .value | ||
| .saturating_sub(calculate_fee(output.weight, feerate)) | ||
| .saturating_sub(calculate_fee(output.weight, feerate)?)) |
|
fmt failed because of an error in line 707. It reads |
|
Should fix, unless the number itself is wrong.. |
| @@ -75,6 +76,8 @@ pub enum ExcessStrategy { | |||
| pub enum SelectionError { | |||
| InsufficientFunds, | |||
| NoSolutionFound, | |||
| NegativeFeeRate, | |||
There was a problem hiding this comment.
NonPositiveFeeRate will be accurate
| @@ -75,6 +76,8 @@ pub enum ExcessStrategy { | |||
| pub enum SelectionError { | |||
| InsufficientFunds, | |||
| NoSolutionFound, | |||
| NegativeFeeRate, | |||
| AbnormallyHighFee, | |||
I added tests: test_calculate_fee(), test_effective_value_when_less_than_zero(), test_effective_value_when_greater_than_zero(), test_effective_value when_less_than_zero(), test_calculate_waste_to_drain(), and test_calculate_waste_to_miner() per issue number 24.