From 4586e34ef53d0ad4af017066c69b7afce20ac5e8 Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Mon, 9 Aug 2021 11:41:45 +0100 Subject: [PATCH] Prevent crash if negative change in sweep Fixes #976. Joinmarket will allow, but warn, when non-zero change values (which are not included in the transaction) result from sweeps (due to the impossibility of knowing the exact value in advance). However, prior to this commit, if that estimation inaccuracy resulted in a negative change value that was to be ignored, there was a crash due to the fact that `jmbitcoin.amount_to_str` does not allow negative values. Hence we instead print the number in the warning without going through this formatting function. Adds tests case for negative sweep change. --- jmclient/jmclient/taker.py | 2 +- jmclient/test/test_taker.py | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/jmclient/jmclient/taker.py b/jmclient/jmclient/taker.py index b76db18..916322f 100644 --- a/jmclient/jmclient/taker.py +++ b/jmclient/jmclient/taker.py @@ -475,7 +475,7 @@ class Taker(object): # fees is acceptable jlog.info( ('WARNING CHANGE NOT BEING USED\nCHANGEVALUE = {}').format( - btc.amount_to_str(my_change_value))) + my_change_value)) # we need to check whether the *achieved* txfee-rate is outside # the range allowed by the user in config; if not, abort the tx. # this is done with using the same estimate fee function and comparing diff --git a/jmclient/test/test_taker.py b/jmclient/test/test_taker.py index 4af07e0..f8ed283 100644 --- a/jmclient/test/test_taker.py +++ b/jmclient/test/test_taker.py @@ -338,7 +338,9 @@ def test_auth_pub_not_found(setup_taker): 2, False, None, None), #tumble style non-int amounts #edge case triggers that don't fail ([(0, 0, 4, "mxeLuX8PP7qLkcM8uarHmdZyvP1b5e1Ynf", 0, NO_ROUNDING)], False, False, - 2, False, None, None), #sweep rounding error case + 2, False, None, None), #sweep rounding error case 1 + ([(0, 0, 4, "mteaYsGsLCL9a4cftZFTpGEWXNwZyDt5KS", 0, NO_ROUNDING)], False, False, + 2, False, None, None), #sweep rounding error case 2 ([(0, 199856001, 3, "mnsquzxrHXpFsZeL42qwbKdCP2y1esN3qw", 0, NO_ROUNDING)], False, False, 2, False, None, None), #trigger sub dust change for taker #edge case triggers that do fail @@ -448,7 +450,7 @@ def test_taker_init(setup_taker, schedule, highfee, toomuchcoins, minmakers, if schedule[0][3] == "mxeLuX8PP7qLkcM8uarHmdZyvP1b5e1Ynf": #to trigger rounding error for sweep (change non-zero), #modify the total_input via the values in self.input_utxos; - #the amount to trigger a 2 satoshi change is found by trial-error. + #the amount to trigger a small + satoshi change is found by trial-error. #TODO note this test is not adequate, because the code is not; #the code does not *DO* anything if a condition is unexpected. taker.input_utxos = copy.deepcopy(t_utxos_by_mixdepth)[0] @@ -457,6 +459,14 @@ def test_taker_init(setup_taker, schedule, highfee, toomuchcoins, minmakers, res = taker.receive_utxos(maker_response) assert res[0] return clean_up() + if schedule[0][3] == "mteaYsGsLCL9a4cftZFTpGEWXNwZyDt5KS": + # as above, but small -ve change instead of +ve. + taker.input_utxos = copy.deepcopy(t_utxos_by_mixdepth)[0] + for k,v in iteritems(taker.input_utxos): + v["value"] = int(0.999805028 * v["value"]) + res = taker.receive_utxos(maker_response) + assert res[0] + return clean_up() res = taker.receive_utxos(maker_response) if minmakers != 2: