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

@@ -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=(",", ":"))

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

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)

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]
 

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():