diff --git a/hathor/wallet/resources/thin_wallet/address_history.py b/hathor/wallet/resources/thin_wallet/address_history.py index e7e231e71..4f5608871 100644 --- a/hathor/wallet/resources/thin_wallet/address_history.py +++ b/hathor/wallet/resources/thin_wallet/address_history.py @@ -35,8 +35,11 @@ class AddressHistoryResource(Resource): isLeaf = True def __init__(self, manager): - self._settings = get_global_settings() self.manager = manager + settings = get_global_settings() + # XXX: copy the parameters that are needed so tests can more easily tweak them + self.max_tx_addresses_history = settings.MAX_TX_ADDRESSES_HISTORY + self.max_inputs_outputs_address_history = settings.MAX_INPUTS_OUTPUTS_ADDRESS_HISTORY # TODO add openapi docs for this API def render_POST(self, request: Request) -> bytes: @@ -166,21 +169,23 @@ def get_address_history(self, addresses: list[str], ref_hash: Optional[str]) -> 'message': 'The address {} is invalid'.format(address) }) - tx = None - if ref_hash_bytes: - try: - tx = self.manager.tx_storage.get_transaction(ref_hash_bytes) - except TransactionDoesNotExist: - return json_dumpb({ - 'success': False, - 'message': 'Hash {} is not a transaction hash.'.format(ref_hash) - }) - # The address index returns an iterable that starts at `tx`. - hashes = addresses_index.get_sorted_from_address(address, tx) + if idx == 0: + ref_tx = None + if ref_hash_bytes: + try: + ref_tx = self.manager.tx_storage.get_transaction(ref_hash_bytes) + except TransactionDoesNotExist: + return json_dumpb({ + 'success': False, + 'message': 'Hash {} is not a transaction hash.'.format(ref_hash) + }) + hashes = addresses_index.get_sorted_from_address(address, ref_tx) + else: + hashes = addresses_index.get_sorted_from_address(address) did_break = False for tx_hash in hashes: - if total_added == self._settings.MAX_TX_ADDRESSES_HISTORY: + if total_added == self.max_tx_addresses_history: # If already added the max number of elements possible, then break # I need to add this if at the beginning of the loop to handle the case # when the first tx of the address exceeds the limit, so we must return @@ -193,7 +198,7 @@ def get_address_history(self, addresses: list[str], ref_hash: Optional[str]) -> if tx_hash not in seen: tx = self.manager.tx_storage.get_transaction(tx_hash) tx_elements = len(tx.inputs) + len(tx.outputs) - if total_elements + tx_elements > self._settings.MAX_INPUTS_OUTPUTS_ADDRESS_HISTORY: + if total_elements + tx_elements > self.max_inputs_outputs_address_history: # If the adition of this tx overcomes the maximum number of inputs and outputs, then break # It's important to validate also the maximum number of inputs and outputs because some txs # are really big and the response payload becomes too big diff --git a/tests/resources/base_resource.py b/tests/resources/base_resource.py index ce9a84c07..96a0d4994 100644 --- a/tests/resources/base_resource.py +++ b/tests/resources/base_resource.py @@ -53,8 +53,17 @@ def __init__(self, method, url, args=None, headers=None): # Set request args args = args or {} - for k, v in args.items(): - self.addArg(k, v) + if isinstance(args, dict): + for k, v in args.items(): + self.addArg(k, v) + elif isinstance(args, list): + for k, v in args: + if k not in self.args: + self.args[k] = [v] + else: + self.args[k].append(v) + else: + raise TypeError(f'unsupported type {type(args)} for args') def json_value(self): return json_loadb(self.written[0]) diff --git a/tests/resources/wallet/test_thin_wallet.py b/tests/resources/wallet/test_thin_wallet.py index 4f01a739d..393599c13 100644 --- a/tests/resources/wallet/test_thin_wallet.py +++ b/tests/resources/wallet/test_thin_wallet.py @@ -266,6 +266,55 @@ def test_history_paginate(self): # The last big tx self.assertEqual(len(response_data['history']), 1) + @inlineCallbacks + def test_address_history_optimization_regression(self): + # setup phase1: create 3 addresses with 2 transactions each in a certain order + self.manager.wallet.unlock(b'MYPASS') + address1 = self.get_address(0) + address2 = self.get_address(1) + address3 = self.get_address(2) + baddress1 = decode_address(address1) + baddress2 = decode_address(address2) + baddress3 = decode_address(address3) + [b1] = add_new_blocks(self.manager, 1, advance_clock=1, address=baddress1) + [b2] = add_new_blocks(self.manager, 1, advance_clock=1, address=baddress3) + [b3] = add_new_blocks(self.manager, 1, advance_clock=1, address=baddress2) + [b4] = add_new_blocks(self.manager, 1, advance_clock=1, address=baddress1) + [b5] = add_new_blocks(self.manager, 1, advance_clock=1, address=baddress2) + [b6] = add_new_blocks(self.manager, 1, advance_clock=1, address=baddress3) + add_blocks_unlock_reward(self.manager) + + # setup phase2: make the first request without a `hash` argument + self.web_address_history.resource.max_tx_addresses_history = 3 + res = (yield self.web_address_history.get( + 'thin_wallet/address_history', [ + (b'paginate', True), # this isn't needed, but used to ensure compatibility is not removed + (b'addresses[]', address1.encode()), + (b'addresses[]', address3.encode()), + (b'addresses[]', address2.encode()), + ] + )).json_value() + self.assertTrue(res['success']) + self.assertEqual(len(res['history']), 3) + self.assertTrue(res['has_more']) + self.assertEqual(res['first_address'], address3) + self.assertEqual(res['first_hash'], b6.hash_hex) + self.assertEqual([t['tx_id'] for t in res['history']], [b1.hash_hex, b4.hash_hex, b2.hash_hex]) + + # actual test, this request will miss transactions when the regression is present + res = (yield self.web_address_history.get( + 'thin_wallet/address_history', [ + (b'paginate', True), # this isn't needed, but used to ensure compatibility is not removed + (b'addresses[]', address3.encode()), + (b'addresses[]', address2.encode()), + (b'hash', res['first_hash'].encode()), + ] + )).json_value() + self.assertTrue(res['success']) + self.assertEqual(len(res['history']), 3) + self.assertFalse(res['has_more']) + self.assertEqual([t['tx_id'] for t in res['history']], [b6.hash_hex, b3.hash_hex, b5.hash_hex]) + def test_error_request(self): from hathor.wallet.resources.thin_wallet.send_tokens import _Context