From 21556811b38b2e88c80075963384a55240f81181 Mon Sep 17 00:00:00 2001 From: Jonathan Moody <103143855+moodyjon@users.noreply.github.com> Date: Fri, 9 Dec 2022 14:49:41 -0500 Subject: [PATCH 1/5] Submit test_liquidate_at_loss() to repro bad TX missing output. --- tests/unit/wallet/test_transaction.py | 51 +++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 6 deletions(-) diff --git a/tests/unit/wallet/test_transaction.py b/tests/unit/wallet/test_transaction.py index b12acc645e..c63bf536aa 100644 --- a/tests/unit/wallet/test_transaction.py +++ b/tests/unit/wallet/test_transaction.py @@ -5,11 +5,11 @@ from binascii import hexlify, unhexlify from itertools import cycle +from lbry.error import InsufficientFundsError from lbry.testcase import AsyncioTestCase -from lbry.wallet.constants import CENT, COIN, NULL_HASH32 +from lbry.wallet.constants import CENT, COIN, DUST, NULL_HASH32 from lbry.wallet import Wallet, Account, Ledger, Database, Headers, Transaction, Output, Input - NULL_HASH = b'\x00'*32 FEE_PER_BYTE = 50 FEE_PER_CHAR = 200000 @@ -395,12 +395,12 @@ async def create_utxos(self, amounts): return utxos @staticmethod - def inputs(tx): - return [round(i.amount/COIN, 2) for i in tx.inputs] + def inputs(tx, precision=2): + return [round(i.amount/COIN, precision) for i in tx.inputs] @staticmethod - def outputs(tx): - return [round(o.amount/COIN, 2) for o in tx.outputs] + def outputs(tx, precision=2): + return [round(o.amount/COIN, precision) for o in tx.outputs] async def test_basic_use_cases(self): self.ledger.fee_per_byte = int(.01*CENT) @@ -532,3 +532,42 @@ async def test_basic_use_cases_sqlite(self): self.assertListEqual([0.01, 1], self.inputs(tx)) # change is now needed to consume extra input self.assertListEqual([0.97], self.outputs(tx)) + + async def test_liquidate_at_loss(self): + #self.ledger.coin_selection_strategy = 'sqlite' + self.ledger.fee_per_byte = int(0.01*CENT) + + # Create UTXOs with values large enough that they can be spent. + utxos = await self.create_utxos([a/COIN for a in range(1490*DUST, 1510*DUST, int(DUST/10))]) + + tx = await self.tx( + [self.txi(self.txo(0.01))], # inputs + [] # outputs + ) + + # A very tiny amount of change is generated as the only output. + self.assertListEqual([1100], [o.amount for o in tx.outputs]) + # A large number of additional UTXOs are added to cover fees. + self.assertListEqual([ + 1000000, 1509900, 1509800, 1509700, 1509600, 1509500, 1509400, 1509300, 1509200, 1509100, + 1509000, 1508900, 1508800, 1508700, 1508600, 1508500, 1508400, 1508300, 1508200, 1508100, + 1494600, 1494400, 1508000, 1507900, 1507800, 1507700, 1507600, 1507500, 1507400, 1507300, + 1507200, 1507100, 1507000, 1506900, 1506800, 1506700, 1506600, 1506300, 1492700, 1492600], + [i.amount for i in tx.inputs]) + self.assertIn(tx.size, range(5920, 5970)) + self.assertEqual(59760000, tx.fee) + await self.ledger.release_outputs(utxos) + + async def test_liquidate_at_loss_tiny_utxos(self): + #self.ledger.coin_selection_strategy = 'sqlite' + self.ledger.fee_per_byte = int(0.01*CENT) + + # Create UTXOs with values so tiny that they cannot be spent. + utxos = await self.create_utxos([a/COIN for a in range(1460*DUST, 1490*DUST, int(DUST/10))]) + + with self.assertRaises(InsufficientFundsError): + tx = await self.tx( + [self.txi(self.txo(0.01))], # inputs + [] # outputs + ) + self.assertFalse([i.amount for i in tx.inputs]) \ No newline at end of file From 54932e43e820081c54b23020cd2ee769d2be522c Mon Sep 17 00:00:00 2001 From: Jonathan Moody <103143855+moodyjon@users.noreply.github.com> Date: Fri, 9 Dec 2022 14:51:21 -0500 Subject: [PATCH 2/5] Ensure 2nd pass through loop will generate change > DUST. Drop double-count of Tx base fee from cost_of_change. --- lbry/wallet/transaction.py | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/lbry/wallet/transaction.py b/lbry/wallet/transaction.py index 7b81661b96..b4a2879a30 100644 --- a/lbry/wallet/transaction.py +++ b/lbry/wallet/transaction.py @@ -808,25 +808,33 @@ async def create(cls, inputs: Iterable[Input], outputs: Iterable[Output], tx.get_base_fee(ledger) + tx.get_total_output_sum(ledger) ) + cost_of_change = ( + Output.pay_pubkey_hash(COIN, NULL_HASH32).get_fee(ledger) + ) # value of the inputs less the cost to spend those inputs payment = tx.get_effective_input_sum(ledger) try: - for _ in range(5): + for i in range(2): - if payment < cost: + if payment < cost or (i > 0 and not tx._outputs): deficit = cost - payment + # this condition and the outer range(2) loop cover an edge case + # whereby a single input is just enough to cover the fee and + # has some change left over, but the change left over is less + # than the cost_of_change: thus the input is completely + # consumed and no output is added, which is an invalid tx. + # to be able to spend this input we must increase the cost + # in order to make a change output > DUST. + if i > 0 and not tx._outputs: + deficit += (cost_of_change + DUST + 1) spendables = await ledger.get_spendable_utxos(deficit, funding_accounts) if not spendables: raise InsufficientFundsError() payment += sum(s.effective_amount for s in spendables) tx.add_inputs(s.txi for s in spendables) - cost_of_change = ( - tx.get_base_fee(ledger) + - Output.pay_pubkey_hash(COIN, NULL_HASH32).get_fee(ledger) - ) if payment > cost: change = payment - cost change_amount = change - cost_of_change @@ -839,17 +847,11 @@ async def create(cls, inputs: Iterable[Input], outputs: Iterable[Output], if tx._outputs: break - # this condition and the outer range(5) loop cover an edge case - # whereby a single input is just enough to cover the fee and - # has some change left over, but the change left over is less - # than the cost_of_change: thus the input is completely - # consumed and no output is added, which is an invalid tx. - # to be able to spend this input we must increase the cost - # of the TX and run through the balance algorithm a second time - # adding an extra input and change output, making tx valid. - # we do this 5 times in case the other UTXOs added are also - # less than the fee, after 5 attempts we give up and go home - cost += cost_of_change + 1 + # We need to run through the balance algorithm a second time + # adding extra inputs and change output, making tx valid. + + if not tx._outputs: + raise InsufficientFundsError() if sign: await tx.sign(funding_accounts) From 2d1ef85d9b80b4ad18299f4b9af0f16483f139f6 Mon Sep 17 00:00:00 2001 From: Jonathan Moody <103143855+moodyjon@users.noreply.github.com> Date: Mon, 12 Dec 2022 11:22:27 -0600 Subject: [PATCH 3/5] Restore get_base_fee(ledger) in cost_of_change. --- lbry/wallet/transaction.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lbry/wallet/transaction.py b/lbry/wallet/transaction.py index b4a2879a30..5bff617d60 100644 --- a/lbry/wallet/transaction.py +++ b/lbry/wallet/transaction.py @@ -809,6 +809,7 @@ async def create(cls, inputs: Iterable[Input], outputs: Iterable[Output], tx.get_total_output_sum(ledger) ) cost_of_change = ( + tx.get_base_fee(ledger) + Output.pay_pubkey_hash(COIN, NULL_HASH32).get_fee(ledger) ) # value of the inputs less the cost to spend those inputs From 6e76fa207fc775dbded3b0467ae6a22666691d17 Mon Sep 17 00:00:00 2001 From: Jonathan Moody <103143855+moodyjon@users.noreply.github.com> Date: Mon, 12 Dec 2022 11:50:08 -0600 Subject: [PATCH 4/5] New logic for transaction create spends slightly less. --- tests/integration/claims/test_claim_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/claims/test_claim_commands.py b/tests/integration/claims/test_claim_commands.py index d3fbda0436..7dd4fbcb3e 100644 --- a/tests/integration/claims/test_claim_commands.py +++ b/tests/integration/claims/test_claim_commands.py @@ -2119,7 +2119,7 @@ async def test_abandoning_stream_at_loss(self): tx = await self.stream_create(bid='0.0001') await self.assertBalance(self.account, '9.979793') await self.stream_abandon(self.get_claim_id(tx)) - await self.assertBalance(self.account, '9.97968399') + await self.assertBalance(self.account, '9.979712') async def test_publish(self): From bea20892f734aa7937d741907afccffc19842bcf Mon Sep 17 00:00:00 2001 From: Jonathan Moody <103143855+moodyjon@users.noreply.github.com> Date: Mon, 12 Dec 2022 12:11:02 -0600 Subject: [PATCH 5/5] Fixup test after restoring get_base_fee(ledger) in cost_of_change. --- tests/unit/wallet/test_transaction.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/unit/wallet/test_transaction.py b/tests/unit/wallet/test_transaction.py index c63bf536aa..3b805f39de 100644 --- a/tests/unit/wallet/test_transaction.py +++ b/tests/unit/wallet/test_transaction.py @@ -552,10 +552,12 @@ async def test_liquidate_at_loss(self): 1000000, 1509900, 1509800, 1509700, 1509600, 1509500, 1509400, 1509300, 1509200, 1509100, 1509000, 1508900, 1508800, 1508700, 1508600, 1508500, 1508400, 1508300, 1508200, 1508100, 1494600, 1494400, 1508000, 1507900, 1507800, 1507700, 1507600, 1507500, 1507400, 1507300, - 1507200, 1507100, 1507000, 1506900, 1506800, 1506700, 1506600, 1506300, 1492700, 1492600], + 1507200, 1507100, 1507000, 1506900, 1506800, 1506700, 1506600, 1506500, 1506400, 1506300, + 1506200, 1505200, 1501000], [i.amount for i in tx.inputs]) - self.assertIn(tx.size, range(5920, 5970)) - self.assertEqual(59760000, tx.fee) + self.assertIn(tx.size, range(6350, 6430)) + self.assertEqual(64300000, tx.fee) + await self.ledger.release_outputs(utxos) async def test_liquidate_at_loss_tiny_utxos(self):