Follow-up to 42e1b5d9797b65 (#12486).
round() with our utility function satoshi_round() to avoid intermittent test failuresfee_per_kB to feerate_BTC_kvB for precisionfeerate_BTC_vBPossibly resolves #19418.
782 | - assert_fee_amount(result3['fee'], count_bytes(result3['hex']), 10 * result_fee_rate) 783 | - assert_fee_amount(result4['fee'], count_bytes(result4['hex']), 10 * result_fee_rate) 784 | + assert_feerate_amount(result1['fee'], count_bytes(result1['hex']), 2 * result_fee_rate) 785 | + assert_feerate_amount(result2['fee'], count_bytes(result2['hex']), 2 * result_fee_rate) 786 | + assert_feerate_amount(result3['fee'], count_bytes(result3['hex']), 10 * result_fee_rate) 787 | + assert_feerate_amount(result4['fee'], count_bytes(result4['hex']), 10 * result_fee_rate)
the first arg is fee (absolute fee), not a feerate
oops. thanks, fixed, change is much smaller now
Can you explain the changes?
Thanks for the quick look. Fixed, simplified, and updated the description to explain the changes.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
33 | @@ -34,13 +34,13 @@ def assert_approx(v, vexp, vspan=0.00001): 34 | raise AssertionError("%s > [%s..%s]" % (str(v), str(vexp - vspan), str(vexp + vspan))) 35 | 36 | 37 | -def assert_fee_amount(fee, tx_size, fee_per_kB): 38 | - """Assert the fee was in range""" 39 | - target_fee = round(tx_size * fee_per_kB / 1000, 8) 40 | +def assert_fee_amount(fee, tx_size, feerate_per_kvB):
Wondering if feerate_per_kvB is better than fee_per_kvB. Technically _per_kvB indicates a rate, so feerate_per_kvB may sound like rate of a rate.
No hard opinion in either way.
Good point, updated to the more precise feerate_BTC_kvB. Let me know if that's better.
Sorry I am probably bikeshedding here, this one seems even more confusing. To me the original fee_per_kvB seems reasonable.
Yes, if you run something like git grep -ni "btc_kvb\|sat_vb" from repo root, you'll see that we have been working to upgrade and standardize how we describe fee rates
fee per (denominator without specifying the numerator) to fee rate in numerator/denominator
Concept ACK.
Just one totally ignore-able nit.
- update call to round() with satoshi_round() to avoid intermittent test failures
- rename fee_per_kB to feerate_BTC_kvB for precision
- store division result in feerate_BTC_vB
utACK b658d7d5c5339739dc19bf961d84186469a818d5
I think the test is correct and this is a bug that should be fixed: #22949 (comment)
Thanks everyone.
Sanity check if anyone passing by is curious:
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -38,6 +38,12 @@ def assert_fee_amount(fee, tx_size, feerate_BTC_kvB):
"""Assert the fee is in range."""
feerate_BTC_vB = feerate_BTC_kvB / 1000
target_fee = satoshi_round(tx_size * feerate_BTC_vB)
+ target_fee_old = round(tx_size * feerate_BTC_vB, 8)
+
+ import pprint
+ pprint.pprint(f"{target_fee_old} round()")
+ pprint.pprint(f"{target_fee} satoshi_round()")
+
if fee < target_fee:
raise AssertionError("Fee of %s BTC too low! (Should be %s BTC)" % (str(fee), str(target_fee)))
# allow the wallet's estimation to be at most 2 bytes off
$ test/functional/wallet_basic.py
2021-10-01T11:52:21.033000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_juu29c68
2021-10-01T11:52:22.323000Z TestFramework (INFO): Mining blocks...
2021-10-01T11:52:24.085000Z TestFramework (INFO): Test gettxout
2021-10-01T11:52:24.209000Z TestFramework (INFO): Test gettxout (second part)
'0.00014100 round()'
'0.00014100 satoshi_round()'
'0.00011000 round()'
'0.00011000 satoshi_round()'
2021-10-01T11:52:32.374000Z TestFramework (INFO): Test sendmany
'0.00014100 round()'
'0.00014100 satoshi_round()'
'0.00014100 round()'
'0.00014100 satoshi_round()'
2021-10-01T11:52:33.550000Z TestFramework (INFO): Test sendmany with fee_rate param (explicit fee rate in sat/vB)
'0.00000282 round()'
'0.00000282 satoshi_round()'
'0.00000282 round()'
'0.00000282 satoshi_round()'
2021-10-01T11:52:35.802000Z TestFramework (INFO): Test sendmany raises 'fee rate too low' if fee_rate of 0.99999999 is passed
2021-10-01T11:52:35.822000Z TestFramework (INFO): Test sendmany raises if an invalid fee_rate is passed
2021-10-01T11:52:35.926000Z TestFramework (INFO): Test sendmany raises if an invalid conf_target or estimate_mode is passed
2021-10-01T11:52:44.981000Z TestFramework (INFO): Test -walletbroadcast
2021-10-01T11:52:55.891000Z TestFramework (INFO): Test sendtoaddress with fee_rate param (explicit fee rate in sat/vB)
'0.00000282 round()'
'0.00000282 satoshi_round()'
'0.00000173 round()'
'0.00000173 satoshi_round()'
2021-10-01T11:53:03.407000Z TestFramework (INFO): Test sendtoaddress raises 'fee rate too low' if fee_rate of 0.99999999 is passed
2021-10-01T11:53:03.432000Z TestFramework (INFO): Test sendtoaddress raises if an invalid fee_rate is passed
2021-10-01T11:53:03.539000Z TestFramework (INFO): Test sendtoaddress raises if an invalid conf_target or estimate_mode is passed
2021-10-01T11:53:05.333000Z TestFramework (INFO): Test -reindex
2021-10-01T11:53:11.957000Z TestFramework (INFO): Testing gettransaction response with different arguments...
2021-10-01T11:53:11.976000Z TestFramework (INFO): Test send* RPCs with verbose=True
2021-10-01T11:53:12.210000Z TestFramework (INFO): Test send* RPCs with verbose=False
2021-10-01T11:53:12.462000Z TestFramework (INFO): Stopping nodes
2021-10-01T11:53:12.670000Z TestFramework (INFO): Cleaning up /tmp/bitcoin_func_test_juu29c68 on exit
2021-10-01T11:53:12.670000Z TestFramework (INFO): Tests successful