aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMarcoFalke <falke.marco@gmail.com>2022-02-07 17:08:21 +0100
committerMarcoFalke <falke.marco@gmail.com>2022-02-07 17:08:26 +0100
commiteca694a4e78d54ce4e29b388b3e81b06e55c2293 (patch)
treee3168d060a878ad0d13d2837d7ad14916004fbfb
parent9392e1350cd8091677d728c72dd2ea842604227b (diff)
parentd1fab9d5d27a2db2546db0f610e0f6929ec4864e (diff)
downloadbitcoin-eca694a4e78d54ce4e29b388b3e81b06e55c2293.tar.xz
Merge bitcoin/bitcoin#24239: test: fix ceildiv division by using integers
d1fab9d5d27a2db2546db0f610e0f6929ec4864e test: Call ceildiv helper with integer (Martin Zumsande) Pull request description: On master, `assert_fee_amount(Decimal("0.00000993"), 217, Decimal("0.00004531"))` passes `assert_fee_amount(Decimal("0.00000993"), Decimal("217"), Decimal("0.00004531"))` fails. the reason is that the // operator in `ceildiv(a,b) = -(-a//b)` has a different behavior for Decimals, see [doc](https://docs.python.org/3/library/decimal.html#decimal-objects). `wallet_send.py` calls this function with Decimals, and I think this is the reason for the failure reported in the OP of #24151 (`wallet_send.py --legacy-wallet` line 332, the numbers used in the example above are from there). However, the other failures reported there cannot be explained by this, so this is just a partial fix. ACKs for top commit: ryanofsky: Code review ACK d1fab9d5d27a2db2546db0f610e0f6929ec4864e. Tracking down this problem was a good find, and code seems safer and easier to understand now Tree-SHA512: 5bf0568cd1a0824f6b1a15a03580b6e9391b4f51112a97c1d00469d255bf6dda45c49a36fa567a5ba9b9973efe1d9cdd480db91965c9f4c2aa963629a8a32cba
-rw-r--r--test/functional/test_framework/util.py11
-rwxr-xr-xtest/functional/wallet_send.py9
2 files changed, 14 insertions, 6 deletions
diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index dabde13bf1..210025104e 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -36,6 +36,7 @@ def assert_approx(v, vexp, vspan=0.00001):
def assert_fee_amount(fee, tx_size, feerate_BTC_kvB):
"""Assert the fee is in range."""
+ assert isinstance(tx_size, int)
target_fee = get_fee(tx_size, feerate_BTC_kvB)
if fee < target_fee:
raise AssertionError("Fee of %s BTC too low! (Should be %s BTC)" % (str(fee), str(target_fee)))
@@ -219,7 +220,13 @@ def str_to_b64str(string):
def ceildiv(a, b):
- """Divide 2 ints and round up to next int rather than round down"""
+ """
+ Divide 2 ints and round up to next int rather than round down
+ Implementation requires python integers, which have a // operator that does floor division.
+ Other types like decimal.Decimal whose // operator truncates towards 0 will not work.
+ """
+ assert isinstance(a, int)
+ assert isinstance(b, int)
return -(-a // b)
@@ -227,7 +234,7 @@ def get_fee(tx_size, feerate_btc_kvb):
"""Calculate the fee in BTC given a feerate is BTC/kvB. Reflects CFeeRate::GetFee"""
feerate_sat_kvb = int(feerate_btc_kvb * Decimal(1e8)) # Fee in sat/kvb as an int to avoid float precision errors
target_fee_sat = ceildiv(feerate_sat_kvb * tx_size, 1000) # Round calculated fee up to nearest sat
- return satoshi_round(target_fee_sat / Decimal(1e8)) # Truncate BTC result to nearest sat
+ return target_fee_sat / Decimal(1e8) # Return result in BTC
def satoshi_round(amount):
diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py
index 843a9f52b7..86e36be8f7 100755
--- a/test/functional/wallet_send.py
+++ b/test/functional/wallet_send.py
@@ -16,6 +16,7 @@ from test_framework.util import (
assert_fee_amount,
assert_greater_than,
assert_raises_rpc_error,
+ count_bytes,
)
from test_framework.wallet_util import bytes_to_wif
@@ -320,20 +321,20 @@ class WalletSendTest(BitcoinTestFramework):
res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=7, add_to_wallet=False)
fee = self.nodes[1].decodepsbt(res["psbt"])["fee"]
- assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00007"))
+ assert_fee_amount(fee, count_bytes(res["hex"]), Decimal("0.00007"))
# "unset" and None are treated the same for estimate_mode
res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=2, estimate_mode="unset", add_to_wallet=False)
fee = self.nodes[1].decodepsbt(res["psbt"])["fee"]
- assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00002"))
+ assert_fee_amount(fee, count_bytes(res["hex"]), Decimal("0.00002"))
res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=4.531, add_to_wallet=False)
fee = self.nodes[1].decodepsbt(res["psbt"])["fee"]
- assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00004531"))
+ assert_fee_amount(fee, count_bytes(res["hex"]), Decimal("0.00004531"))
res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=3, add_to_wallet=False)
fee = self.nodes[1].decodepsbt(res["psbt"])["fee"]
- assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00003"))
+ assert_fee_amount(fee, count_bytes(res["hex"]), Decimal("0.00003"))
# Test that passing fee_rate as both an argument and an option raises.
self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=1, fee_rate=1, add_to_wallet=False,