Refactor transactions into readable code

handler/tests/test_transactions.py

@@ -45,6 +45,7 @@ class TestTransactions(TestCase):
 
         # Mock the rates
         self.transactions.money = MagicMock()
+        self.transactions.money.to_usd = self.mock_to_usd
         self.transactions.money.get_rates_all = MagicMock()
         self.transactions.money.get_rates_all.return_value = {"GBP": 0.8}
 
@@ -119,6 +120,17 @@ class TestTransactions(TestCase):
             if trade["id"] == string:
                 return trade["reference"]
 
+    def mock_to_usd(self, amount, currency):
+        if currency == "GBP":
+            return amount * 1.3
+        elif currency == "USD":
+            return amount
+        # fuck it who cares
+        elif currency == "SEK":
+            return 100
+        elif currency == "EUR":
+            return 10
+
     def test_transaction(self):
         self.transactions.transaction(self.test_data)
         self.transactions.release_funds.assert_called_once_with("uuid1", "TEST-1")
@@ -154,7 +166,7 @@ class TestTransactions(TestCase):
 
     def test_transaction_no_reference_pass(self):
         no_reference_pass = self.data_custom(1, "GBP", "none")
-        no_reference_pass["meta"]["provider_reference"] = "none"
+        no_reference_pass["meta"]["provider_reference"] = "THIS_ONE_FAILS"
         self.return_trades = [1]
 
         self.transactions.transaction(no_reference_pass)

handler/transactions.py

@@ -67,49 +67,64 @@ class Transactions(util.Base):
             self.lc_es_checks.start(delay)
             self.agora.es = self.es
 
-    # TODO: write tests then refactor, this is terribly complicated!
+    def valid_transaction(self, data):
-    def transaction(self, data):
         """
-        Store details of transaction and post notifications to IRC.
+        Determine if a given transaction object is valid.
-        Matches it up with data stored in Redis to attempt to reconcile with an Agora trade.
+        :param data: a transaction cast
-        :param data: details of transaction
         :type data: dict
+        :return: whether the transaction is valid
+        :rtype: bool
         """
-        print(f"Raw transaction data: {data}")
-        ts = data["timestamp"]
         txid = data["transaction_id"]
         if "amount" not in data:
-            return
+            return False
         if "currency" not in data:
-            return
+            return False
         amount = data["amount"]
         if amount <= 0:
             self.log.info(f"Ignoring transaction with negative/zero amount: {txid}")
-            return
+            return False
-        currency = data["currency"]
+        return True
 
+    def extract_reference(self, data):
+        """
+        Extract a reference from the transaction cast.
+        :param data: a transaction cast
+        :type data: dict
+        :return: the extracted reference or not_set
+        :rtype: str
+        """
         if "reference" in data:
-            reference = data["reference"]
+            return data["reference"]
         elif "meta" in data:
             if "provider_reference" in data["meta"]:
-                reference = data["meta"]["provider_reference"]
+                return data["meta"]["provider_reference"]
-            else:
+        return "not_set"
-                reference = "not_set"
-        else:
-            reference = "not_set"
 
-        subclass = data["subclass"]
+    def extract_sender(self, data):
-        to_store = {
+        """
-            "trade_id": "",
+        Extract a sender name from the transaction cast.
-            "subclass": subclass,
+        :param data: a transaction cast
-            "ts": ts,
+        :type data: dict
-            "txid": txid,
+        :return: the sender name or not_set
-            "reference": reference,
+        :rtype: str
-            "amount": amount,
+        """
-            "currency": currency,
+        if "debtorName" in data:
-        }
+            return data["debtorName"]
-        self.log.info(f"Transaction processed: {dumps(to_store, indent=2)}")
+        elif "meta" in data:
-        self.irc.sendmsg(f"AUTO Incoming transaction on {subclass}: {amount}{currency} ({reference})")
+            if "debtor_account_name" in data["meta"]:
+                return data["meta"]["debtor_account_name"]
+        return "not_set"
+
+    def reference_partial_check(self, reference, txid, currency, amount):
+        """
+        Perform a partial check by intersecting all parts of the split of the
+        reference against the existing references, and returning a set of the matches.
+        :param reference: the reference to check
+        :type reference: str
+        :return: matching trade ID string
+        :rtype: str
+        """
         # Partial reference implementation
         # Account for silly people not removing the default string
         # Split the reference into parts
@@ -122,13 +137,23 @@ class Transactions(util.Base):
             self.log.error(f"Multiple references valid for TXID {txid}: {reference}")
             self.irc.sendmsg(f"Multiple references valid for TXID {txid}: {reference}")
             self.ux.notify.notify_tx_lookup_failed(currency, amount, reference, "MULTIPLE_REFS_MATCH")
-            return
+            return False
+        if len(stored_trade_reference) == 0:
+            return None
+        return stored_trade_reference.pop()
 
-        stored_trade = False
+    def can_alt_lookup(self, amount, currency, reference):
-        looked_up_without_reference = False
+        amount_usd = self.money.to_usd(amount, currency)
+        # Amount is reliable here as it is checked by find_trade, so no need for stored_trade["amount"]
+        if float(amount_usd) > float(settings.Agora.AcceptableAltLookupUSD):
+            self.log.info("Not checking against amount and currency as amount exceeds MAX")
+            self.irc.sendmsg(f"Not checking against amount and currency as amount exceeds MAX")
+            # Close here if the amount exceeds the allowable limit for no reference
+            self.ux.notify.notify_tx_lookup_failed(currency, amount, reference, "EXCEEDS_MAX")
+            return False
+        return True
 
-        # Amount/currency lookup implementation
+    def amount_currency_lookup(self, amount, currency, txid, reference):
-        if not stored_trade_reference:
         self.log.info(f"No reference in DB refs for {reference}")
         self.irc.sendmsg(f"No reference in DB refs for {reference}")
         # Try checking just amount and currency, as some people (usually people buying small amounts)
@@ -136,52 +161,36 @@ class Transactions(util.Base):
 
         self.log.info(f"Checking against amount and currency for TXID {txid}")
         self.irc.sendmsg(f"Checking against amount and currency for TXID {txid}")
+        if not self.can_alt_lookup(amount, currency, reference):
+            return False
         stored_trade = self.find_trade(txid, currency, amount)
         if not stored_trade:
             self.log.info(f"Failed to get reference by amount and currency: {txid} {currency} {amount}")
             self.irc.sendmsg(f"Failed to get reference by amount and currency: {txid} {currency} {amount}")
             self.ux.notify.notify_tx_lookup_failed(currency, amount, reference, "ALT_LOOKUP_FAILED")
-                return
+            return None
-            if currency == "USD":
+        stored_trade["amount"] = float(stored_trade["amount"])  # convert to float
-                amount_usd = amount
+        return stored_trade
-            else:
+
-                rates = self.money.get_rates_all()
+    def normal_lookup(self, stored_trade_reference, reference, currency, amount):
-                amount_usd = amount / rates[currency]
+        stored_trade = self.get_ref(stored_trade_reference)
-            # Amount is reliable here as it is checked by find_trade, so no need for stored_trade["amount"]
-            if float(amount_usd) > float(settings.Agora.AcceptableAltLookupUSD):
-                self.log.info("Not checking against amount and currency as amount exceeds MAX")
-                self.irc.sendmsg(f"Not checking against amount and currency as amount exceeds MAX")
-                # Close here if the amount exceeds the allowable limit for no reference
-                if len(stored_trade_reference) == 1:  # better safe than sorry
-                    self.ux.notify.notify_tx_lookup_failed(currency, amount, reference, "EXCEEDS_MAX", stored_trade_reference[0])
-                else:
-                    self.ux.notify.notify_tx_lookup_failed(currency, amount, reference, "EXCEEDS_MAX")
-                return
-            # Note that we have looked it up without reference so we don't use +- below
-            # This might be redundant given the amount checks in find_trade, but better safe than sorry!
-            looked_up_without_reference = True
-        if not stored_trade:
-            stored_trade = self.get_ref(stored_trade_reference.pop())
         if not stored_trade:
             self.log.info(f"No reference in DB for {reference}")
             self.irc.sendmsg(f"No reference in DB for {reference}")
-            self.ux.notify.notify_tx_lookup_failed(currency, amount, reference, "NOREF", stored_trade["id"])
+            self.ux.notify.notify_tx_lookup_failed(currency, amount, reference, "NOREF", stored_trade_reference)
-            return
+            return False
+        stored_trade["amount"] = float(stored_trade["amount"])  # convert to float
+        return stored_trade
 
-        amount = float(amount)
+    def currency_check(self, currency, amount, reference, stored_trade):
-        stored_trade["amount"] = float(stored_trade["amount"])
-
-        # Make sure it was sent in the expected currency
         if not stored_trade["currency"] == currency:
             self.log.info(f"Currency mismatch, Agora: {stored_trade['currency']} / Sink: {currency}")
             self.irc.sendmsg(f"Currency mismatch, Agora: {stored_trade['currency']} / Sink: {currency}")
             self.ux.notify.notify_tx_lookup_failed(currency, amount, reference, "CURRENCY_MISMATCH", stored_trade["id"])
-            return
+            return False
+        return True
 
-        # Make sure the expected amount was sent
+    def alt_amount_check(self, amount, currency, reference, stored_trade):
-        if not stored_trade["amount"] == amount:
-            if looked_up_without_reference:
-                return
         # If the amount does not match exactly, get the min and max values for our given acceptable margins for trades
         min_amount, max_amount = self.money.get_acceptable_margins(currency, stored_trade["amount"])
         self.log.info(f"Amount does not match exactly, trying with margins: min: {min_amount} / max: {max_amount}")
@@ -190,6 +199,80 @@ class Transactions(util.Base):
             self.log.info("Amount mismatch - not in margins: {stored_trade['amount']} (min: {min_amount} / max: {max_amount}")
             self.irc.sendmsg(f"Amount mismatch - not in margins: {stored_trade['amount']} (min: {min_amount} / max: {max_amount}")
             self.ux.notify.notify_tx_lookup_failed(currency, amount, reference, "AMOUNT_MARGIN_MISMATCH", stored_trade["id"])
+            return False
+        return True
+
+    def transaction(self, data):
+        """
+        Store details of transaction and post notifications to IRC.
+        Matches it up with data stored in Redis to attempt to reconcile with an Agora trade.
+        :param data: details of transaction
+        :type data: dict
+        """
+        valid = self.valid_transaction(data)
+        if not valid:
+            return False
+        print(f"Raw transaction data: {data}")
+        ts = data["timestamp"]
+        txid = data["transaction_id"]
+        amount = float(data["amount"])
+        currency = data["currency"]
+
+        reference = self.extract_reference(data)
+        sender = self.extract_sender(data)
+
+        subclass = data["subclass"]
+        to_store = {
+            "trade_id": "",
+            "subclass": subclass,
+            "ts": ts,
+            "txid": txid,
+            "reference": reference,
+            "amount": amount,
+            "currency": currency,
+            "sender": sender,
+        }
+        self.log.info(f"Transaction processed: {dumps(to_store, indent=2)}")
+        self.irc.sendmsg(f"AUTO Incoming transaction on {subclass}: {amount}{currency} ({reference})")
+
+        stored_trade_reference = self.reference_partial_check(reference, txid, currency, amount)
+        if stored_trade_reference is False:  # can be None though
+            return
+
+        stored_trade = False
+        looked_up_without_reference = False
+
+        # Normal implementation for when we have a reference
+        if stored_trade_reference:
+            stored_trade = self.normal_lookup(stored_trade_reference, reference, currency, amount)
+            # if not stored_trade:
+            #    return
+
+        # Amount/currency lookup implementation for when we have no reference
+        else:
+            if not stored_trade:  # check we don't overwrite the lookup above
+                stored_trade = self.amount_currency_lookup(amount, currency, txid, reference)
+                if stored_trade is False:
+                    return
+                if stored_trade:
+                    # Note that we have looked it up without reference so we don't use +- below
+                    # This might be redundant given the amount checks in find_trade, but better safe than sorry!
+                    looked_up_without_reference = True
+                else:
+                    return
+            else:
+                # Stored trade reference is none, the checks below will do nothing at all
+                return
+
+        # Make sure it was sent in the expected currency
+        if not self.currency_check(currency, amount, reference, stored_trade):
+            return
+
+        # Make sure the expected amount was sent
+        if not stored_trade["amount"] == amount:
+            if looked_up_without_reference:
+                return
+            if not self.alt_amount_check(amount, currency, reference, stored_trade):
                 return
 
         r.hmset(f"tx.{txid}", to_store)
@@ -263,11 +346,8 @@ class Transactions(util.Base):
         matching_refs = []
         # TODO: use get_ref_map in this function instead of calling get_ref multiple times
         for ref in refs:
-            print(f"ITER REF {ref}")
             stored_trade = self.get_ref(ref)
-            print(f"ITER REF STORED TRADE {stored_trade}")
             if stored_trade["currency"] == currency and float(stored_trade["amount"]) == float(amount):
-                print(f"APPENDING STORED TRADE AS MATCH {stored_trade}")
                 matching_refs.append(stored_trade)
         if len(matching_refs) != 1:
             self.log.error(f"Find trade returned multiple results for TXID {txid}: {matching_refs}")