From d3a171e63821d2a210c2816562b652b626ea7b6e Mon Sep 17 00:00:00 2001 From: Lex Berezhny Date: Fri, 3 Aug 2018 10:41:40 -0400 Subject: [PATCH] + closest_match step between branch_and_bound and random_draw coin selections, + Transaction.create() single method instead of .pay()/.liquidate() also its more powerful --- tests/integration/test_transactions.py | 9 +- tests/unit/test_coinselection.py | 14 ++- tests/unit/test_transaction.py | 136 +++++++++++++++++++++++-- torba/baseledger.py | 12 +-- torba/basetransaction.py | 124 +++++++++++----------- torba/coinselection.py | 24 ++++- 6 files changed, 232 insertions(+), 87 deletions(-) diff --git a/tests/integration/test_transactions.py b/tests/integration/test_transactions.py index 6b44a90d1..5eb29284b 100644 --- a/tests/integration/test_transactions.py +++ b/tests/integration/test_transactions.py @@ -29,7 +29,8 @@ class BasicTransactionTests(IntegrationTestCase): address2 = await d2f(account2.receiving.get_or_create_usable_address()) hash2 = self.ledger.address_to_hash160(address2) - tx = await d2f(self.ledger.transaction_class.pay( + tx = await d2f(self.ledger.transaction_class.create( + [], [self.ledger.transaction_class.output_class.pay_pubkey_hash(2*COIN, hash2)], [account1], account1 )) @@ -42,8 +43,10 @@ class BasicTransactionTests(IntegrationTestCase): self.assertEqual(round(await self.get_balance(account2)/COIN, 1), 2.0) utxos = await d2f(self.account.get_unspent_outputs()) - tx = await d2f(self.ledger.transaction_class.liquidate( - [utxos[0]], [account1], account1 + tx = await d2f(self.ledger.transaction_class.create( + [self.ledger.transaction_class.input_class.spend(utxos[0])], + [], + [account1], account1 )) await self.broadcast(tx) await self.on_transaction(tx) # mempool diff --git a/tests/unit/test_coinselection.py b/tests/unit/test_coinselection.py index e607fc3ef..a8d11880f 100644 --- a/tests/unit/test_coinselection.py +++ b/tests/unit/test_coinselection.py @@ -66,6 +66,18 @@ class TestCoinSelectionTests(BaseSelectionTestCase): self.assertEqual([2 * CENT], [c.txo.amount for c in match]) self.assertFalse(selector.exact_match) + def test_pick(self): + utxo_pool = self.estimates( + utxo(1*CENT), + utxo(1*CENT), + utxo(3*CENT), + utxo(5*CENT), + utxo(10*CENT), + ) + selector = CoinSelector(utxo_pool, 3*CENT, 0) + match = selector.select() + self.assertEqual([5*CENT], [c.txo.amount for c in match]) + class TestOfficialBitcoinCoinSelectionTests(BaseSelectionTestCase): @@ -108,7 +120,7 @@ class TestOfficialBitcoinCoinSelectionTests(BaseSelectionTestCase): self.assertEqual([3 * CENT, 2 * CENT], search(utxo_pool, 5 * CENT, 0.5 * CENT)) # Select 11 Cent, not possible - self.assertEqual(search(utxo_pool, 11 * CENT, 0.5 * CENT), []) + self.assertEqual([], search(utxo_pool, 11 * CENT, 0.5 * CENT)) # Select 10 Cent utxo_pool += self.estimates(utxo(5 * CENT)) diff --git a/tests/unit/test_transaction.py b/tests/unit/test_transaction.py index 47af97eb7..61b6b4187 100644 --- a/tests/unit/test_transaction.py +++ b/tests/unit/test_transaction.py @@ -1,4 +1,5 @@ from binascii import hexlify, unhexlify +from itertools import cycle from twisted.trial import unittest from twisted.internet import defer @@ -17,8 +18,8 @@ def get_output(amount=CENT, pubkey_hash=NULL_HASH): .outputs[0] -def get_input(): - return ledger_class.transaction_class.input_class.spend(get_output()) +def get_input(amount=CENT, pubkey_hash=NULL_HASH): + return ledger_class.transaction_class.input_class.spend(get_output(amount, pubkey_hash)) def get_transaction(txo=None): @@ -31,27 +32,22 @@ class TestSizeAndFeeEstimation(unittest.TestCase): def setUp(self): self.ledger = ledger_class({'db': ledger_class.database_class(':memory:')}) - return self.ledger.db.start() - - def io_fee(self, io): - return self.ledger.get_input_output_fee(io) def test_output_size_and_fee(self): txo = get_output() self.assertEqual(txo.size, 46) - self.assertEqual(self.io_fee(txo), 46 * FEE_PER_BYTE) + self.assertEqual(txo.get_fee(self.ledger), 46 * FEE_PER_BYTE) def test_input_size_and_fee(self): txi = get_input() self.assertEqual(txi.size, 148) - self.assertEqual(self.io_fee(txi), 148 * FEE_PER_BYTE) + self.assertEqual(txi.get_fee(self.ledger), 148 * FEE_PER_BYTE) def test_transaction_size_and_fee(self): tx = get_transaction() - base_size = tx.size - 1 - tx.inputs[0].size self.assertEqual(tx.size, 204) - self.assertEqual(tx.base_size, base_size) - self.assertEqual(self.ledger.get_transaction_base_fee(tx), FEE_PER_BYTE * base_size) + self.assertEqual(tx.base_size, tx.size - tx.inputs[0].size - tx.outputs[0].size) + self.assertEqual(tx.get_base_fee(self.ledger), FEE_PER_BYTE * tx.base_size) class TestTransactionSerialization(unittest.TestCase): @@ -169,3 +165,121 @@ class TestTransactionSigning(unittest.TestCase): b'304402203d463519290d06891e461ea5256c56097ccdad53379b1bb4e51ec5abc6e9fd02022034ed15b9' b'd7c678716c4aa7c0fd26c688e8f9db8075838f2839ab55d551b62c0a01' ) + + +class TransactionIOBalancing(unittest.TestCase): + + @defer.inlineCallbacks + def setUp(self): + self.ledger = ledger_class({'db': ledger_class.database_class(':memory:')}) + yield self.ledger.db.start() + self.account = self.ledger.account_class.from_seed( + self.ledger, + u"carbon smart garage balance margin twelve chest sword toast envelope bottom stomach ab" + u"sent", u"torba", {} + ) + + addresses = yield self.account.ensure_address_gap() + self.pubkey_hash = [self.ledger.address_to_hash160(a) for a in addresses] + self.hash_cycler = cycle(self.pubkey_hash) + + def txo(self, amount, address=None): + return get_output(int(amount*COIN), address or next(self.hash_cycler)) + + def txi(self, txo): + return ledger_class.transaction_class.input_class.spend(txo) + + def tx(self, inputs, outputs): + return ledger_class.transaction_class.create(inputs, outputs, [self.account], self.account) + + @defer.inlineCallbacks + def create_utxos(self, amounts): + utxos = [self.txo(amount) for amount in amounts] + + self.funding_tx = ledger_class.transaction_class() \ + .add_inputs([self.txi(self.txo(sum(amounts)+0.1))]) \ + .add_outputs(utxos) + + save_tx = 'insert' + for utxo in utxos: + yield self.ledger.db.save_transaction_io( + save_tx, self.funding_tx, 1, True, + self.ledger.hash160_to_address(utxo.script.values['pubkey_hash']), + utxo.script.values['pubkey_hash'], '' + ) + save_tx = 'update' + + defer.returnValue(utxos) + + @staticmethod + def inputs(tx): + return [round(i.amount/COIN, 2) for i in tx.inputs] + + @staticmethod + def outputs(tx): + return [round(o.amount/COIN, 2) for o in tx.outputs] + + @defer.inlineCallbacks + def test_basic_use_cases(self): + self.ledger.fee_per_byte = int(.01*CENT) + + # available UTXOs for filling missing inputs + utxos = yield self.create_utxos([ + 1, 1, 3, 5, 10 + ]) + + # pay 3 coins (3.02 w/ fees) + tx = yield self.tx( + [], # inputs + [self.txo(3)] # outputs + ) + # best UTXO match is 5 (as UTXO 3 will be short 0.02 to cover fees) + self.assertEqual(self.inputs(tx), [5]) + # a change of 1.98 is added to reach balance + self.assertEqual(self.outputs(tx), [3, 1.98]) + + yield self.ledger.release_outputs(utxos) + + # pay 2.98 coins (3.00 w/ fees) + tx = yield self.tx( + [], # inputs + [self.txo(2.98)] # outputs + ) + # best UTXO match is 3 and no change is needed + self.assertEqual(self.inputs(tx), [3]) + self.assertEqual(self.outputs(tx), [2.98]) + + yield self.ledger.release_outputs(utxos) + + # supplied input and output, but input is not enough to cover output + tx = yield self.tx( + [self.txi(self.txo(10))], # inputs + [self.txo(11)] # outputs + ) + # additional input is chosen (UTXO 3) + self.assertEqual([10, 3], self.inputs(tx)) + # change is now needed to consume extra input + self.assertEqual([11, 1.96], self.outputs(tx)) + + yield self.ledger.release_outputs(utxos) + + # liquidating a UTXO + tx = yield self.tx( + [self.txi(self.txo(10))], # inputs + [] # outputs + ) + self.assertEqual([10], self.inputs(tx)) + # missing change added to consume the amount + self.assertEqual([9.98], self.outputs(tx)) + + yield self.ledger.release_outputs(utxos) + + # liquidating at a loss, requires adding extra inputs + tx = yield self.tx( + [self.txi(self.txo(0.01))], # inputs + [] # outputs + ) + # UTXO 1 is added to cover some of the fee + self.assertEqual([0.01, 1], self.inputs(tx)) + # change is now needed to consume extra input + self.assertEqual([0.97], self.outputs(tx)) diff --git a/torba/baseledger.py b/torba/baseledger.py index bf64bfd01..3cbbf0b72 100644 --- a/torba/baseledger.py +++ b/torba/baseledger.py @@ -117,14 +117,6 @@ class BaseLedger(metaclass=LedgerRegistry): def path(self): return os.path.join(self.config['data_path'], self.get_id()) - def get_input_output_fee(self, io: basetransaction.InputOutput) -> int: - """ Fee based on size of the input / output. """ - return self.fee_per_byte * io.size - - def get_transaction_base_fee(self, tx): - """ Fee for the transaction header and all outputs; without inputs. """ - return self.fee_per_byte * tx.base_size - @defer.inlineCallbacks def add_account(self, account: baseaccount.BaseAccount) -> defer.Deferred: self.accounts.append(account) @@ -161,9 +153,7 @@ class BaseLedger(metaclass=LedgerRegistry): txos = yield self.get_effective_amount_estimators(funding_accounts) selector = CoinSelector( txos, amount, - self.get_input_output_fee( - self.transaction_class.output_class.pay_pubkey_hash(COIN, NULL_HASH32) - ) + self.transaction_class.output_class.pay_pubkey_hash(COIN, NULL_HASH32).get_fee(self) ) spendables = selector.select() if spendables: diff --git a/torba/basetransaction.py b/torba/basetransaction.py index e7dc1b4a7..b74a49fdd 100644 --- a/torba/basetransaction.py +++ b/torba/basetransaction.py @@ -95,6 +95,9 @@ class InputOutput: self.serialize_to(stream) return len(stream.get_bytes()) + def get_fee(self, ledger): + return self.size * ledger.fee_per_byte + def serialize_to(self, stream, alternate_script=None): raise NotImplementedError @@ -166,7 +169,7 @@ class BaseOutputEffectiveAmountEstimator: def __init__(self, ledger: 'baseledger.BaseLedger', txo: 'BaseOutput') -> None: self.txo = txo self.txi = ledger.transaction_class.input_class.spend(txo) - self.fee: int = ledger.get_input_output_fee(self.txi) + self.fee: int = self.txi.get_fee(ledger) self.effective_amount: int = txo.amount - self.fee def __lt__(self, other): @@ -268,11 +271,6 @@ class BaseTransaction: def add_outputs(self, outputs: Iterable[BaseOutput]) -> 'BaseTransaction': return self._add(outputs, self._outputs) - @property - def fee(self) -> int: - """ Fee that will actually be paid.""" - return self.input_sum - self.output_sum - @property def size(self) -> int: """ Size in bytes of the entire transaction. """ @@ -280,8 +278,32 @@ class BaseTransaction: @property def base_size(self) -> int: - """ Size in bytes of transaction meta data and all outputs; without inputs. """ - return len(self._serialize(with_inputs=False)) + """ Size of transaction without inputs or outputs in bytes. """ + return ( + self.size + - sum(txi.size for txi in self._inputs) + - sum(txo.size for txo in self._outputs) + ) + + @property + def input_sum(self): + return sum(i.amount for i in self.inputs) + + @property + def output_sum(self): + return sum(o.amount for o in self.outputs) + + def get_base_fee(self, ledger): + """ Fee for base tx excluding inputs and outputs. """ + return self.base_size * ledger.fee_per_byte + + def get_effective_input_sum(self, ledger): + """ Sum of input values *minus* the cost involved to spend them. """ + return sum(txi.amount - txi.get_fee(ledger) for txi in self._inputs) + + def get_total_output_sum(self, ledger): + """ Sum of output values *plus* the cost involved to spend them. """ + return sum(txo.amount + txo.get_fee(ledger) for txo in self._outputs) def _serialize(self, with_inputs: bool = True) -> bytes: stream = BCDataStream() @@ -346,62 +368,56 @@ class BaseTransaction: @classmethod @defer.inlineCallbacks - def pay(cls, outputs: Iterable[BaseOutput], funding_accounts: Iterable[BaseAccount], - change_account: BaseAccount): - """ Efficiently spend utxos from funding_accounts to cover the new outputs. """ + def create(cls, inputs: Iterable[BaseInput], outputs: Iterable[BaseOutput], + funding_accounts: Iterable[BaseAccount], change_account: BaseAccount): + """ Find optimal set of inputs when only outputs are provided; add change + outputs if only inputs are provided or if inputs are greater than outputs. """ + + tx = cls() \ + .add_inputs(inputs) \ + .add_outputs(outputs) - tx = cls().add_outputs(outputs) ledger = cls.ensure_all_have_same_ledger(funding_accounts, change_account) - amount = tx.output_sum + ledger.get_transaction_base_fee(tx) - spendables = yield ledger.get_spendable_utxos(amount, funding_accounts) - if not spendables: - raise ValueError('Not enough funds to cover this transaction.') + # value of the outputs plus associated fees + cost = ( + tx.get_base_fee(ledger) + + tx.get_total_output_sum(ledger) + ) + # value of the inputs less the cost to spend those inputs + payment = tx.get_effective_input_sum(ledger) try: - spent_sum = sum(s.effective_amount for s in spendables) - if spent_sum > amount: - change_address = yield change_account.change.get_or_create_usable_address() - change_hash160 = change_account.ledger.address_to_hash160(change_address) - change_amount = spent_sum - amount - tx.add_outputs([cls.output_class.pay_pubkey_hash(change_amount, change_hash160)]) - tx.add_inputs(s.txi for s in spendables) + if payment < cost: + deficit = cost - payment + spendables = yield ledger.get_spendable_utxos(deficit, funding_accounts) + if not spendables: + raise ValueError('Not enough funds to cover this transaction.') + payment += sum(s.effective_amount for s in spendables) + tx.add_inputs(s.txi for s in spendables) + + if payment > cost: + cost_of_change = ( + tx.get_base_fee(ledger) + + cls.output_class.pay_pubkey_hash(COIN, NULL_HASH32).get_fee(ledger) + ) + change = payment - cost + if change > cost_of_change: + change_address = yield change_account.change.get_or_create_usable_address() + change_hash160 = change_account.ledger.address_to_hash160(change_address) + change_amount = change - cost_of_change + tx.add_outputs([cls.output_class.pay_pubkey_hash(change_amount, change_hash160)]) + yield tx.sign(funding_accounts) except Exception as e: log.exception('Failed to synchronize transaction:') - yield ledger.release_outputs(s.txo for s in spendables) + yield ledger.release_outputs(tx.outputs) raise e defer.returnValue(tx) - @classmethod - @defer.inlineCallbacks - def liquidate(cls, assets, funding_accounts, change_account): - """ Spend assets (utxos) supplementing with funding_accounts if fee is higher than asset value. """ - tx = cls().add_inputs([ - cls.input_class.spend(utxo) for utxo in assets - ]) - ledger = cls.ensure_all_have_same_ledger(funding_accounts, change_account) - yield ledger.reserve_outputs(assets) - try: - cost_of_change = ( - ledger.get_transaction_base_fee(tx) + - ledger.get_input_output_fee(cls.output_class.pay_pubkey_hash(COIN, NULL_HASH32)) - ) - liquidated_total = sum(utxo.amount for utxo in assets) - if liquidated_total > cost_of_change: - change_address = yield change_account.change.get_or_create_usable_address() - change_hash160 = change_account.ledger.address_to_hash160(change_address) - change_amount = liquidated_total - cost_of_change - tx.add_outputs([cls.output_class.pay_pubkey_hash(change_amount, change_hash160)]) - yield tx.sign(funding_accounts) - except Exception: - yield ledger.release_outputs(assets) - raise - defer.returnValue(tx) - @staticmethod def signature_hash_type(hash_type): return hash_type @@ -425,14 +441,6 @@ class BaseTransaction: raise NotImplementedError("Don't know how to spend this output.") self._reset() - @property - def input_sum(self): - return sum(i.amount for i in self.inputs) - - @property - def output_sum(self): - return sum(o.amount for o in self.outputs) - @defer.inlineCallbacks def get_my_addresses(self, ledger): addresses = set() diff --git a/torba/coinselection.py b/torba/coinselection.py index bf3709636..440383230 100644 --- a/torba/coinselection.py +++ b/torba/coinselection.py @@ -25,7 +25,11 @@ class CoinSelector: return [] if self.target > self.available: return [] - return self.branch_and_bound() or self.single_random_draw() + return ( + self.branch_and_bound() or + self.closest_match() or + self.random_draw() + ) def branch_and_bound(self) -> List[basetransaction.BaseOutputEffectiveAmountEstimator]: # see bitcoin implementation for more info: @@ -85,13 +89,27 @@ class CoinSelector: return [] - def single_random_draw(self) -> List[basetransaction.BaseOutputEffectiveAmountEstimator]: + def closest_match(self) -> List[basetransaction.BaseOutputEffectiveAmountEstimator]: + """ Pick one UTXOs that is larger than the target but with the smallest change. """ + target = self.target + self.cost_of_change + smallest_change = None + best_match = None + for txo in self.txos: + if txo.effective_amount >= target: + change = txo.effective_amount - target + if smallest_change is None or change < smallest_change: + smallest_change, best_match = change, txo + return [best_match] if best_match else [] + + def random_draw(self) -> List[basetransaction.BaseOutputEffectiveAmountEstimator]: + """ Accumulate UTXOs at random until there is enough to cover the target. """ + target = self.target + self.cost_of_change self.random.shuffle(self.txos, self.random.random) selection = [] amount = 0 for coin in self.txos: selection.append(coin) amount += coin.effective_amount - if amount >= self.target+self.cost_of_change: + if amount >= target: return selection return []