diff --git a/pisa/cleaner.py b/pisa/cleaner.py index d7808b9..b7c2947 100644 --- a/pisa/cleaner.py +++ b/pisa/cleaner.py @@ -1,14 +1,13 @@ -import pisa.conf as conf from pisa import logging +# Dictionaries in Python are "passed-by-reference", so no return is needed for the Cleaner" +# https://docs.python.org/3/faq/programming.html#how-do-i-write-a-function-with-output-parameters-call-by-reference + class Cleaner: @staticmethod - def delete_expired_appointment(block, appointments, locator_uuid_map): - to_delete = [uuid for uuid, appointment in appointments.items() - if block["height"] > appointment.end_time + conf.EXPIRY_DELTA] - - for uuid in to_delete: + def delete_expired_appointment(expired_appointments, appointments, locator_uuid_map): + for uuid in expired_appointments: locator = appointments[uuid].locator appointments.pop(uuid) @@ -22,13 +21,11 @@ class Cleaner: logging.info("[Cleaner] end time reached with no match! Deleting appointment {} (uuid: {})".format(locator, uuid)) - return appointments, locator_uuid_map - @staticmethod def delete_completed_jobs(jobs, tx_job_map, completed_jobs, height): - for uuid in completed_jobs: + for uuid, confirmations in completed_jobs: logging.info("[Cleaner] job completed (uuid = {}). Appointment ended at block {} after {} confirmations" - .format(uuid, jobs[uuid].justice_txid, height, jobs[uuid].confirmations)) + .format(uuid, height, confirmations)) # ToDo: #9-add-data-persistence justice_txid = jobs[uuid].justice_txid @@ -41,5 +38,3 @@ class Cleaner: else: tx_job_map[justice_txid].remove(uuid) - - return jobs, tx_job_map diff --git a/pisa/responder.py b/pisa/responder.py index 0f04662..7eabd9b 100644 --- a/pisa/responder.py +++ b/pisa/responder.py @@ -120,8 +120,7 @@ class Responder: txs, self.unconfirmed_txs, self.tx_job_map, self.missed_confirmations) txs_to_rebroadcast = self.get_txs_to_rebroadcast(txs) - self.jobs, self.tx_job_map = Cleaner.delete_completed_jobs(self.jobs, self.tx_job_map, - self.get_completed_jobs(height), height) + Cleaner.delete_completed_jobs(self.jobs, self.tx_job_map, self.get_completed_jobs(height), height) self.rebroadcast(txs_to_rebroadcast) @@ -157,9 +156,10 @@ class Responder: tx = Carrier.get_transaction(job.dispute_txid) # FIXME: Should be improved with the librarian - if tx is not None and tx.get('confirmations') > MIN_CONFIRMATIONS: + confirmations = tx.get('confirmations') + if tx is not None and confirmations > MIN_CONFIRMATIONS: # The end of the appointment has been reached - completed_jobs.append(uuid) + completed_jobs.append((uuid, confirmations)) return completed_jobs diff --git a/pisa/watcher.py b/pisa/watcher.py index bbf066c..f63e5ab 100644 --- a/pisa/watcher.py +++ b/pisa/watcher.py @@ -3,10 +3,11 @@ from queue import Queue from threading import Thread from pisa import logging +from pisa.cleaner import Cleaner +from pisa.conf import EXPIRY_DELTA from pisa.responder import Responder from pisa.conf import MAX_APPOINTMENTS from pisa.block_processor import BlockProcessor -from pisa.cleaner import Cleaner from pisa.utils.zmq_subscriber import ZMQHandler @@ -78,15 +79,17 @@ class Watcher: block_hash = self.block_queue.get() logging.info("[Watcher] new block received {}".format(block_hash)) - block = BlockProcessor.getblock(block_hash) + block = BlockProcessor.get_block(block_hash) if block is not None: txids = block.get('tx') logging.info("[Watcher] list of transactions: {}".format(txids)) - self.appointments, self.locator_uuid_map = Cleaner.delete_expired_appointment( - block, self.appointments, self.locator_uuid_map) + expired_appointments = [uuid for uuid, appointment in self.appointments.items() + if block["height"] > appointment.end_time + EXPIRY_DELTA] + + Cleaner.delete_expired_appointment(expired_appointments, self.appointments, self.locator_uuid_map) potential_matches = BlockProcessor.get_potential_matches(txids, self.locator_uuid_map) matches = BlockProcessor.get_matches(potential_matches, self.locator_uuid_map, self.appointments) diff --git a/tests/test_cleaner.py b/tests/test_cleaner.py new file mode 100644 index 0000000..5a3bbab --- /dev/null +++ b/tests/test_cleaner.py @@ -0,0 +1,87 @@ +import random +from os import urandom +from uuid import uuid4 +from binascii import hexlify + +from pisa import logging +from pisa.responder import Job +from pisa.cleaner import Cleaner +from pisa.appointment import Appointment + +CONFIRMATIONS = 6 +ITEMS = 10 +MAX_ITEMS = 100 +ITERATIONS = 1000 + + +def set_up_appointments(total_appointments): + appointments = dict() + locator_uuid_map = dict() + + for _ in range(total_appointments): + uuid = uuid4().hex + locator = hexlify(urandom(64)) + + appointments[uuid] = Appointment(locator, None, None, None, None, None, None) + locator_uuid_map[locator] = [uuid] + + # Each locator can have more than one uuid assigned to it. Do a coin toss to add multiple ones + while random.randint(0, 1): + uuid = uuid4().hex + + appointments[uuid] = Appointment(locator, None, None, None, None, None, None) + locator_uuid_map[locator].append(uuid) + + return appointments, locator_uuid_map + + +def set_up_jobs(total_jobs): + jobs = dict() + tx_job_map = dict() + + for _ in range(total_jobs): + uuid = uuid4().hex + txid = hexlify(urandom(64)) + + # Assign both justice_txid and dispute_txid the same id (it shouldn't matter) + jobs[uuid] = Job(txid, txid, None, None, None) + tx_job_map[txid] = [uuid] + + # Each justice_txid can have more than one uuid assigned to it. Do a coin toss to add multiple ones + while random.randint(0, 1): + uuid = uuid4().hex + + jobs[uuid] = Job(txid, txid, None, None, None) + tx_job_map[txid].append(uuid) + + return jobs, tx_job_map + + +def test_delete_expired_appointment(): + appointments, locator_uuid_map = set_up_appointments(MAX_ITEMS) + expired_appointments = random.sample(list(appointments.keys()), k=ITEMS) + + Cleaner.delete_expired_appointment(expired_appointments, appointments, locator_uuid_map) + + assert not set(expired_appointments).issubset(appointments.keys()) + + +def test_delete_completed_jobs(): + jobs, tx_job_map = set_up_jobs(MAX_ITEMS) + selected_jobs = random.sample(list(jobs.keys()), k=ITEMS) + + completed_jobs = [(job, 6) for job in selected_jobs] + + Cleaner.delete_completed_jobs(jobs, tx_job_map, completed_jobs, 0) + + assert not set(completed_jobs).issubset(jobs.keys()) + + +logging.getLogger().disabled = True + +for _ in range(ITERATIONS): + test_delete_expired_appointment() + +for _ in range(ITERATIONS): + test_delete_completed_jobs() +