Skip to content

Commit 7311cbc

Browse files
MarcoFalkeClaude Code
authored andcommitted
Merge bitcoin#24239: test: fix ceildiv division by using integers
d1fab9d 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 bitcoin#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 d1fab9d. Tracking down this problem was a good find, and code seems safer and easier to understand now Tree-SHA512: 5bf0568cd1a0824f6b1a15a03580b6e9391b4f51112a97c1d00469d255bf6dda45c49a36fa567a5ba9b9973efe1d9cdd480db91965c9f4c2aa963629a8a32cba
1 parent c33122d commit 7311cbc

File tree

2 files changed

+27
-7
lines changed

2 files changed

+27
-7
lines changed

test/functional/test_framework/util.py

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,13 @@ def assert_approx(v, vexp, vspan=0.00001):
3838

3939
def assert_fee_amount(fee, tx_size, feerate_DASH_kvB):
4040
"""Assert the fee is in range."""
41-
feerate_DASH_vB = feerate_DASH_kvB / 1000
42-
target_fee = satoshi_round(tx_size * feerate_DASH_vB)
41+
assert isinstance(tx_size, int)
42+
target_fee = get_fee(tx_size, feerate_DASH_kvB)
4343
if fee < target_fee:
4444
raise AssertionError("Fee of %s DASH too low! (Should be %s DASH)" % (str(fee), str(target_fee)))
4545
# allow the wallet's estimation to be at most 2 bytes off
46-
if fee > (tx_size + 2) * feerate_DASH_vB:
46+
high_fee = get_fee(tx_size + 2, feerate_DASH_kvB)
47+
if fee > high_fee:
4748
raise AssertionError("Fee of %s DASH too high! (Should be %s DASH)" % (str(fee), str(target_fee)))
4849

4950

@@ -243,6 +244,24 @@ def random_bitflip(data):
243244
return bytes(data)
244245

245246

247+
def ceildiv(a, b):
248+
"""
249+
Divide 2 ints and round up to next int rather than round down
250+
Implementation requires python integers, which have a // operator that does floor division.
251+
Other types like decimal.Decimal whose // operator truncates towards 0 will not work.
252+
"""
253+
assert isinstance(a, int)
254+
assert isinstance(b, int)
255+
return -(-a // b)
256+
257+
258+
def get_fee(tx_size, feerate_dash_kvb):
259+
"""Calculate the fee in DASH given a feerate is DASH/kvB. Reflects CFeeRate::GetFee"""
260+
feerate_duff_kvb = int(feerate_dash_kvb * Decimal(1e8)) # Fee in duff/kvb as an int to avoid float precision errors
261+
target_fee_duff = ceildiv(feerate_duff_kvb * tx_size, 1000) # Round calculated fee up to nearest duff
262+
return target_fee_duff / Decimal(1e8) # Return result in DASH
263+
264+
246265
def satoshi_round(amount):
247266
return Decimal(amount).quantize(Decimal('0.00000001'), rounding=ROUND_DOWN)
248267

test/functional/wallet_send.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
assert_fee_amount,
1616
assert_greater_than,
1717
assert_raises_rpc_error,
18+
count_bytes,
1819
)
1920

2021
class WalletSendTest(BitcoinTestFramework):
@@ -309,20 +310,20 @@ def run_test(self):
309310

310311
res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=7, add_to_wallet=False)
311312
fee = self.nodes[1].decodepsbt(res["psbt"])["fee"]
312-
assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00007"))
313+
assert_fee_amount(fee, count_bytes(res["hex"]), Decimal("0.00007"))
313314

314315
# "unset" and None are treated the same for estimate_mode
315316
res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=2, estimate_mode="unset", add_to_wallet=False)
316317
fee = self.nodes[1].decodepsbt(res["psbt"])["fee"]
317-
assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00002"))
318+
assert_fee_amount(fee, count_bytes(res["hex"]), Decimal("0.00002"))
318319

319320
res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=4.531, add_to_wallet=False)
320321
fee = self.nodes[1].decodepsbt(res["psbt"])["fee"]
321-
assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00004531"))
322+
assert_fee_amount(fee, count_bytes(res["hex"]), Decimal("0.00004531"))
322323

323324
res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=3, add_to_wallet=False)
324325
fee = self.nodes[1].decodepsbt(res["psbt"])["fee"]
325-
assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00003"))
326+
assert_fee_amount(fee, count_bytes(res["hex"]), Decimal("0.00003"))
326327

327328
# Test that passing fee_rate as both an argument and an option raises.
328329
self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=1, fee_rate=1, add_to_wallet=False,

0 commit comments

Comments
 (0)