electrum

commit c95c0dcb8090e7e99a5706c3232077de7051db9b
parent 05a191cc6a544adf625c70454540c8b79ea5ed7c
Author: SomberNight <somber.night@protonmail.com>
Date:   Mon,  9 Mar 2020 20:39:13 +0100

lnrouter: add comments about path-finding blocking the asyncio loop

Diffstat:
M
electrum/lnrouter.py
 | 
7
++++++-
M
electrum/lnworker.py
 | 
7
+++++--
M
electrum/tests/test_lnpeer.py
 | 
6
+++---

3 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/electrum/lnrouter.py b/electrum/lnrouter.py
@@ -146,7 +146,6 @@ class LNPathFinder(Logger):
             return float('inf'), 0
         if channel_policy.is_disabled():
             return float('inf'), 0
-        route_edge = RouteEdge.from_channel_policy(channel_policy, short_channel_id, end_node)
         if payment_amt_msat < channel_policy.htlc_minimum_msat:
             return float('inf'), 0  # payment amount too little
         if channel_info.capacity_sat is not None and \
@@ -155,6 +154,7 @@ class LNPathFinder(Logger):
         if channel_policy.htlc_maximum_msat is not None and \
                 payment_amt_msat > channel_policy.htlc_maximum_msat:
             return float('inf'), 0  # payment amount too large
+        route_edge = RouteEdge.from_channel_policy(channel_policy, short_channel_id, end_node)
         if not route_edge.is_sane_to_use(payment_amt_msat):
             return float('inf'), 0  # thanks but no thanks
 
@@ -187,6 +187,11 @@ class LNPathFinder(Logger):
         assert type(nodeB) is bytes
         assert type(invoice_amount_msat) is int
         if my_channels is None: my_channels = {}
+        # note: we don't lock self.channel_db, so while the path finding runs,
+        #       the underlying graph could potentially change... (not good but maybe ~OK?)
+        #       (but at the time of writing, we are called on the asyncio event loop,
+        #        and the graph is also only updated from the event loop, so it will
+        #        not change)
 
         # FIXME paths cannot be longer than 20 edges (onion packet)...
 
diff --git a/electrum/lnworker.py b/electrum/lnworker.py
@@ -922,7 +922,10 @@ class LNWallet(LNWorker):
         success = False
         for i in range(attempts):
             try:
-                route = await self._create_route_from_invoice(decoded_invoice=lnaddr)
+                # note: this call does path-finding which takes ~1 second
+                #       -> we will BLOCK the asyncio loop... (could just run in a thread and await,
+                #       but then the graph could change while the path-finding runs on it)
+                route = self._create_route_from_invoice(decoded_invoice=lnaddr)
                 self.set_payment_status(payment_hash, PR_INFLIGHT)
                 self.network.trigger_callback('invoice_status', key)
                 payment_attempt_log = await self._pay_to_route(route, lnaddr)
@@ -1035,7 +1038,7 @@ class LNWallet(LNWorker):
                 f"min_final_cltv_expiry: {addr.get_min_final_cltv_expiry()}"))
         return addr
 
-    async def _create_route_from_invoice(self, decoded_invoice) -> LNPaymentRoute:
+    def _create_route_from_invoice(self, decoded_invoice) -> LNPaymentRoute:
         amount_msat = int(decoded_invoice.amount * COIN * 1000)
         invoice_pubkey = decoded_invoice.pubkey.serialize()
         # use 'r' field from invoice
diff --git a/electrum/tests/test_lnpeer.py b/electrum/tests/test_lnpeer.py
@@ -350,7 +350,7 @@ class TestPeer(ElectrumTestCase):
             await asyncio.wait_for(p1.initialized, 1)
             await asyncio.wait_for(p2.initialized, 1)
             # alice sends htlc
-            route = await w1._create_route_from_invoice(decoded_invoice=lnaddr)
+            route = w1._create_route_from_invoice(decoded_invoice=lnaddr)
             htlc = p1.pay(route, alice_channel, int(lnaddr.amount * COIN * 1000), lnaddr.paymenthash, lnaddr.get_min_final_cltv_expiry())
             # alice closes
             await p1.close_channel(alice_channel.channel_id)
@@ -370,14 +370,14 @@ class TestPeer(ElectrumTestCase):
         pay_req = self.prepare_invoice(w2)
 
         addr = w1._check_invoice(pay_req)
-        route = run(w1._create_route_from_invoice(decoded_invoice=addr))
+        route = w1._create_route_from_invoice(decoded_invoice=addr)
 
         run(w1.force_close_channel(alice_channel.channel_id))
         # check if a tx (commitment transaction) was broadcasted:
         assert q1.qsize() == 1
 
         with self.assertRaises(NoPathFound) as e:
-            run(w1._create_route_from_invoice(decoded_invoice=addr))
+            w1._create_route_from_invoice(decoded_invoice=addr)
 
         peer = w1.peers[route[0].node_id]
         # AssertionError is ok since we shouldn't use old routes, and the