fix(resources): regression in address history endpoint

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

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])

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