Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(resources): regression in address history endpoint #1042

Merged
merged 1 commit into from
May 28, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 19 additions & 14 deletions hathor/wallet/resources/thin_wallet/address_history.py
Original file line number Diff line number Diff line change
@@ -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:
msbrogli marked this conversation as resolved.
Show resolved Hide resolved
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
13 changes: 11 additions & 2 deletions tests/resources/base_resource.py
Original file line number Diff line number Diff line change
@@ -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])
49 changes: 49 additions & 0 deletions tests/resources/wallet/test_thin_wallet.py
Original file line number Diff line number Diff line change
@@ -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