From add5d262b6cb3b1e598c69796840a09307db890c Mon Sep 17 00:00:00 2001 From: Sergi Delgado Segura Date: Mon, 9 Dec 2019 14:28:00 +0100 Subject: [PATCH] Includes Appointment docstrings and redesigns triggered flag The triggered flag is only used to interact with the DB but it was kept as an Appointment attribute. Now it is only used when exporting to json, which is used to store data into the db. --- pisa/appointment.py | 75 +++++++++++++++++++++++++++++------- pisa/cleaner.py | 3 +- pisa/db_manager.py | 14 ++++--- test/unit/test_cleaner.py | 2 +- test/unit/test_db_manager.py | 22 +++++++++-- 5 files changed, 91 insertions(+), 25 deletions(-) diff --git a/pisa/appointment.py b/pisa/appointment.py index c881aa5..0a7af84 100644 --- a/pisa/appointment.py +++ b/pisa/appointment.py @@ -3,36 +3,69 @@ import json from pisa.encrypted_blob import EncryptedBlob -# Basic appointment structure class Appointment: + """ + The Appointment class contains the information regarding an appointment between a client and the Watchtower. + + Args: + locator (str): A 16-byte hex-encoded value used by the tower to detect channel breaches. It serves as a trigger + for the tower to decrypt and broadcast the penalty transaction. + start_time (int): The block height at which the tower is hired to start watching for breaches. + end_time (int): The block height at which the tower will stop watching for breaches. + dispute_delta (int): The `to_self_delay` encoded in the CSV of the HTLC that this appointment is covering. + encrypted_blob (EncryptedBlob): An `EncryptedBlob` object containing an encrypted penalty transaction. The tower + will decrypt it and broadcast the penalty transaction upon seeing a breach on the blockchain. + """ + # DISCUSS: 35-appointment-checks - def __init__(self, locator, start_time, end_time, dispute_delta, encrypted_blob, triggered=False): + def __init__(self, locator, start_time, end_time, dispute_delta, encrypted_blob): self.locator = locator self.start_time = start_time # ToDo: #4-standardize-appointment-fields self.end_time = end_time # ToDo: #4-standardize-appointment-fields self.dispute_delta = dispute_delta self.encrypted_blob = EncryptedBlob(encrypted_blob) - self.triggered = triggered @classmethod def from_dict(cls, appointment_data): + """ + Builds an appointment from a dictionary. + + This method is useful to load data from a database. + + Args: + appointment_data (dict): a dictionary containing the following keys: + `{locator, start_time, end_time, dispute_delta, encrypted_blob}` + + Returns: + Appointment: An appointment initialized using the provided data. + + Raises: + ValueError: If one of the mandatory keys is missing in `appointment_data`. + """ + locator = appointment_data.get("locator") start_time = appointment_data.get("start_time") # ToDo: #4-standardize-appointment-fields end_time = appointment_data.get("end_time") # ToDo: #4-standardize-appointment-fields dispute_delta = appointment_data.get("dispute_delta") encrypted_blob_data = appointment_data.get("encrypted_blob") - triggered = True if appointment_data.get("triggered") is True else False - - if any(v is None for v in [locator, start_time, end_time, dispute_delta, encrypted_blob_data, triggered]): + if any(v is None for v in [locator, start_time, end_time, dispute_delta, encrypted_blob_data]): raise ValueError("Wrong appointment data, some fields are missing") else: - appointment = cls(locator, start_time, end_time, dispute_delta, encrypted_blob_data, triggered) + appointment = cls(locator, start_time, end_time, dispute_delta, encrypted_blob_data) return appointment - def to_dict(self, include_triggered=True): + def to_dict(self): + """ + Exports an `Appointment` as a dictionary. + + Returns: + appointment (dict): A dictionary containing the `Appointment` attributes. + + """ + # ToDO: #3-improve-appointment-structure appointment = { "locator": self.locator, @@ -42,10 +75,26 @@ class Appointment: "encrypted_blob": self.encrypted_blob.data, } - if include_triggered: - appointment["triggered"] = self.triggered - return appointment - def to_json(self): - return json.dumps(self.to_dict(), sort_keys=True, separators=(",", ":")) + def to_json(self, triggered=False): + """ + Exports an `Appointment` as a deterministic json encoded string. + + This method ensures that multiple invocations with the same data yield the same value. This is the format used + to store appointments in the database. + + Args: + triggered (bool): Whether the dispute has been triggered or not. When an appointment passes from the + `Watcher` to the `Responder` it is not deleted straightaway. Instead, the appointment is stored in the DB + flagged as triggered. This aims to ease handling block reorgs in the future. + + Returns: + appointment (str): A json-encoded str representing the appointment. + """ + + appointment = self.to_dict() + + appointment["triggered"] = triggered + + return json.dumps(appointment, sort_keys=True, separators=(",", ":")) diff --git a/pisa/cleaner.py b/pisa/cleaner.py index 3de9693..d30e473 100644 --- a/pisa/cleaner.py +++ b/pisa/cleaner.py @@ -40,8 +40,7 @@ class Cleaner: # DISCUSS: instead of deleting the appointment, we will mark it as triggered and delete it from both # the watcher's and responder's db after fulfilled # Update appointment in the db - appointment.triggered = True - db_manager.store_watcher_appointment(uuid, appointment.to_json()) + db_manager.store_watcher_appointment(uuid, appointment.to_json(triggered=True)) @staticmethod def delete_completed_jobs(jobs, tx_job_map, completed_jobs, height, db_manager): diff --git a/pisa/db_manager.py b/pisa/db_manager.py index 0b3d40a..08e5f2c 100644 --- a/pisa/db_manager.py +++ b/pisa/db_manager.py @@ -71,13 +71,15 @@ class DBManager: def load_responder_job(self, key): return self.load_entry(RESPONDER_PREFIX + key) - def load_watcher_appointments(self): - all_appointments = self.load_appointments_db(prefix=WATCHER_PREFIX) - non_triggered_appointments = { - uuid: appointment for uuid, appointment in all_appointments.items() if appointment["triggered"] is False - } + def load_watcher_appointments(self, include_triggered=False): + appointments = self.load_appointments_db(prefix=WATCHER_PREFIX) - return non_triggered_appointments + if not include_triggered: + appointments = { + uuid: appointment for uuid, appointment in appointments.items() if appointment["triggered"] is False + } + + return appointments def load_responder_jobs(self): return self.load_appointments_db(prefix=RESPONDER_PREFIX) diff --git a/test/unit/test_cleaner.py b/test/unit/test_cleaner.py index b8d2238..0431e69 100644 --- a/test/unit/test_cleaner.py +++ b/test/unit/test_cleaner.py @@ -28,7 +28,7 @@ def set_up_appointments(db_manager, total_appointments): uuid = uuid4().hex locator = get_random_value_hex(LOCATOR_LEN_BYTES) - appointment = Appointment(locator, None, None, None, None, None) + appointment = Appointment(locator, None, None, None, None) appointments[uuid] = appointment locator_uuid_map[locator] = [uuid] diff --git a/test/unit/test_db_manager.py b/test/unit/test_db_manager.py index 64824e9..2f5c79b 100644 --- a/test/unit/test_db_manager.py +++ b/test/unit/test_db_manager.py @@ -209,10 +209,26 @@ def test_store_load_watcher_appointment(db_manager, watcher_appointments): assert watcher_appointments.keys() == db_watcher_appointments.keys() for uuid, appointment in watcher_appointments.items(): - assert db_watcher_appointments[uuid] == appointment.to_dict() + assert json.dumps(db_watcher_appointments[uuid], sort_keys=True, separators=(",", ":")) == appointment.to_json() -def test_store_load_appointment_jobs(db_manager, responder_jobs): +def test_store_load_triggered_appointment(db_manager): + db_watcher_appointments = db_manager.load_watcher_appointments() + db_watcher_appointments_with_triggered = db_manager.load_watcher_appointments(include_triggered=True) + + assert db_watcher_appointments == db_watcher_appointments_with_triggered + + # Create an appointment flagged as triggered + triggered_appointment, _ = generate_dummy_appointment(real_height=False) + uuid = uuid4().hex + db_manager.store_watcher_appointment(uuid, triggered_appointment.to_json(triggered=True)) + + # The new appointment is grabbed only if we set include_triggered + assert db_watcher_appointments == db_manager.load_watcher_appointments() + assert uuid in db_manager.load_watcher_appointments(include_triggered=True) + + +def test_store_load_responder_jobs(db_manager, responder_jobs): for key, value in responder_jobs.items(): db_manager.store_responder_job(key, json.dumps({"value": value})) @@ -226,7 +242,7 @@ def test_store_load_appointment_jobs(db_manager, responder_jobs): def test_delete_watcher_appointment(db_manager, watcher_appointments): # Let's delete all we added - db_watcher_appointments = db_manager.load_watcher_appointments() + db_watcher_appointments = db_manager.load_watcher_appointments(include_triggered=True) assert len(db_watcher_appointments) != 0 for key in watcher_appointments.keys():