From 2749ea4d495c704a861b34a3853975f281767ba2 Mon Sep 17 00:00:00 2001 From: bitromortac Date: Fri, 7 May 2021 11:21:58 +0200 Subject: [PATCH] lnrouter: add inflight htlcs to liquidity hints --- electrum/lnrouter.py | 53 ++++++++++++++++++++++++++++----- electrum/lnworker.py | 14 ++++++--- electrum/tests/test_lnrouter.py | 48 +++++++++++++++++++++++++++-- 3 files changed, 102 insertions(+), 13 deletions(-) diff --git a/electrum/lnrouter.py b/electrum/lnrouter.py index 0cb73b264..3400101a2 100644 --- a/electrum/lnrouter.py +++ b/electrum/lnrouter.py @@ -183,6 +183,8 @@ class LiquidityHint: self._cannot_send_backward = None self.blacklist_timestamp = 0 self.hint_timestamp = 0 + self._inflight_htlcs_forward = 0 + self._inflight_htlcs_backward = 0 def is_hint_invalid(self) -> bool: now = int(time.time()) @@ -273,10 +275,28 @@ class LiquidityHint: else: self.cannot_send_backward = amount + def inflight_htlcs(self, is_forward_direction: bool): + if is_forward_direction: + return self._inflight_htlcs_forward + else: + return self._inflight_htlcs_backward + + def add_htlc(self, is_forward_direction: bool): + if is_forward_direction: + self._inflight_htlcs_forward += 1 + else: + self._inflight_htlcs_backward += 1 + + def remove_htlc(self, is_forward_direction: bool): + if is_forward_direction: + self._inflight_htlcs_forward = max(0, self._inflight_htlcs_forward - 1) + else: + self._inflight_htlcs_backward = max(0, self._inflight_htlcs_forward - 1) + def __repr__(self): is_blacklisted = False if not self.blacklist_timestamp else int(time.time()) - self.blacklist_timestamp < BLACKLIST_DURATION - return f"forward: can send: {self._can_send_forward} msat, cannot send: {self._cannot_send_forward} msat, \n" \ - f"backward: can send: {self._can_send_backward} msat, cannot send: {self._cannot_send_backward} msat, \n" \ + return f"forward: can send: {self._can_send_forward} msat, cannot send: {self._cannot_send_forward} msat, htlcs: {self._inflight_htlcs_forward}\n" \ + f"backward: can send: {self._can_send_backward} msat, cannot send: {self._cannot_send_backward} msat, htlcs: {self._inflight_htlcs_backward}\n" \ f"blacklisted: {is_blacklisted}" @@ -288,15 +308,13 @@ class LiquidityHintMgr: algorithm that favors channels which can route payments and penalizes channels that cannot. """ - # TODO: incorporate in-flight htlcs - # TODO: use timestamps for can/not_send to make them None after some time? # TODO: hints based on node pairs only (shadow channels, non-strict forwarding)? def __init__(self): self.lock = RLock() self._liquidity_hints: Dict[ShortChannelID, LiquidityHint] = {} @with_lock - def get_hint(self, channel_id: ShortChannelID): + def get_hint(self, channel_id: ShortChannelID) -> LiquidityHint: hint = self._liquidity_hints.get(channel_id) if not hint: hint = LiquidityHint() @@ -313,6 +331,16 @@ class LiquidityHintMgr: hint = self.get_hint(channel_id) hint.update_cannot_send(node_from < node_to, amount) + @with_lock + def add_htlc(self, node_from: bytes, node_to: bytes, channel_id: ShortChannelID): + hint = self.get_hint(channel_id) + hint.add_htlc(node_from < node_to) + + @with_lock + def remove_htlc(self, node_from: bytes, node_to: bytes, channel_id: ShortChannelID): + hint = self.get_hint(channel_id) + hint.remove_htlc(node_from < node_to) + def penalty(self, node_from: bytes, node_to: bytes, channel_id: ShortChannelID, amount: int) -> float: """Gives a penalty when sending from node1 to node2 over channel_id with an amount in units of millisatoshi. @@ -337,16 +365,19 @@ class LiquidityHintMgr: # we only evaluate hints here, so use dict get (to not create many hints with self.get_hint) hint = self._liquidity_hints.get(channel_id) if not hint: - can_send, cannot_send = None, None + can_send, cannot_send, inflight_htlcs = None, None, 0 else: can_send = hint.can_send(node_from < node_to) cannot_send = hint.cannot_send(node_from < node_to) + inflight_htlcs = hint.inflight_htlcs(node_from < node_to) if cannot_send is not None and amount >= cannot_send: return inf if can_send is not None and amount <= can_send: return 0 - return fee_for_edge_msat(amount, DEFAULT_PENALTY_BASE_MSAT, DEFAULT_PENALTY_PROPORTIONAL_MILLIONTH) + success_fee = fee_for_edge_msat(amount, DEFAULT_PENALTY_BASE_MSAT, DEFAULT_PENALTY_PROPORTIONAL_MILLIONTH) + inflight_htlc_fee = inflight_htlcs * success_fee + return success_fee + inflight_htlc_fee @with_lock def add_to_blacklist(self, channel_id: ShortChannelID): @@ -403,6 +434,14 @@ class LNPathFinder(Logger): self.liquidity_hints.update_cannot_send(r.start_node, r.end_node, r.short_channel_id, amount_msat) break + def update_htlcs_liquidity_hints(self, route: LNPaymentRoute, add_htlcs: bool): + self.logger.info(f"{'Adding' if add_htlcs else 'Removing'} htlcs in liquidity hints.") + for r in route: + if add_htlcs: + self.liquidity_hints.add_htlc(r.start_node, r.end_node, r.short_channel_id) + else: + self.liquidity_hints.remove_htlc(r.start_node, r.end_node, r.short_channel_id) + def _edge_cost( self, *, diff --git a/electrum/lnworker.py b/electrum/lnworker.py index 2b453d7af..6f9c2c843 100644 --- a/electrum/lnworker.py +++ b/electrum/lnworker.py @@ -1196,12 +1196,14 @@ class LNWallet(LNWorker): raise Exception(f"amount_inflight={amount_inflight} < 0") log.append(htlc_log) if htlc_log.success: - # TODO: report every route to liquidity hints for mpp - # even in the case of success, we report channels of the - # route as being able to send the same amount in the future, - # as we assume to not know the capacity if self.network.path_finder: + # TODO: report every route to liquidity hints for mpp + # in the case of success, we report channels of the + # route as being able to send the same amount in the future, + # as we assume to not know the capacity self.network.path_finder.update_liquidity_hints(htlc_log.route, htlc_log.amount_msat) + # remove inflight htlcs from liquidity hints + self.network.path_finder.update_htlcs_liquidity_hints(htlc_log.route, add_htlcs=False) return # htlc failed if len(log) >= attempts: @@ -1268,6 +1270,8 @@ class LNWallet(LNWorker): amount_sent, amount_failed = self.sent_buckets[payment_secret] amount_sent += amount_receiver_msat self.sent_buckets[payment_secret] = amount_sent, amount_failed + # add inflight htlcs to liquidity hints + self.network.path_finder.update_htlcs_liquidity_hints(route, add_htlcs=True) util.trigger_callback('htlc_added', chan, htlc, SENT) def handle_error_code_from_failed_htlc( @@ -1333,6 +1337,8 @@ class LNWallet(LNWorker): # for errors that do not include a channel update else: self.network.path_finder.liquidity_hints.add_to_blacklist(fallback_channel) + # remove inflight htlcs from liquidity hints + self.network.path_finder.update_htlcs_liquidity_hints(route, add_htlcs=False) def _handle_chanupd_from_failed_htlc(self, payload, *, route, sender_idx) -> Tuple[bool, bool]: blacklist = False diff --git a/electrum/tests/test_lnrouter.py b/electrum/tests/test_lnrouter.py index 5382fe502..a292aadda 100644 --- a/electrum/tests/test_lnrouter.py +++ b/electrum/tests/test_lnrouter.py @@ -154,7 +154,7 @@ class Test_LNRouter(TestCaseForTestnet): self.cdb.stop() asyncio.run_coroutine_threadsafe(self.cdb.stopped_event.wait(), self.asyncio_loop).result() - def test_find_path_liquidity_hints_failure(self): + def test_find_path_liquidity_hints(self): self.prepare_graph() amount_to_send = 100000 @@ -197,7 +197,7 @@ class Test_LNRouter(TestCaseForTestnet): assume success over channel 4, D -> C A -3-> B |-2-> E A -6-> D |-5-> E - A -6-> D -4-> C -7-> E <= chosen path + A -6-> D -4-> C -7-> E <= smaller penalty: chosen path A -3-> B -1-> C -7-> E A -6-> D -4-> C -1-> B |-2-> E A -3-> B -1-> C -4-> D |-5-> E @@ -211,6 +211,44 @@ class Test_LNRouter(TestCaseForTestnet): self.assertEqual(channel(4), path[1].short_channel_id) self.assertEqual(channel(7), path[2].short_channel_id) + def test_find_path_liquidity_hints_inflight_htlcs(self): + self.prepare_graph() + amount_to_send = 100000 + + """ + add inflight htlc to channel 2, B -> E + A -3-> B -2(1)-> E + A -6-> D -5-> E <= chosen path + A -6-> D -4-> C -7-> E + A -3-> B -1-> C -7-> E + A -6-> D -4-> C -1-> B -2-> E + A -3-> B -1-> C -4-> D -5-> E + """ + self.path_finder.liquidity_hints.add_htlc(node('b'), node('e'), channel(2)) + path = self.path_finder.find_path_for_payment( + nodeA=node('a'), + nodeB=node('e'), + invoice_amount_msat=amount_to_send) + self.assertEqual(channel(6), path[0].short_channel_id) + self.assertEqual(channel(5), path[1].short_channel_id) + + """ + remove inflight htlc from channel 2, B -> E + A -3-> B -2(0)-> E <= chosen path + A -6-> D -5-> E + A -6-> D -4-> C -7-> E + A -3-> B -1-> C -7-> E + A -6-> D -4-> C -1-> B -2-> E + A -3-> B -1-> C -4-> D -5-> E + """ + self.path_finder.liquidity_hints.remove_htlc(node('b'), node('e'), channel(2)) + path = self.path_finder.find_path_for_payment( + nodeA=node('a'), + nodeB=node('e'), + invoice_amount_msat=amount_to_send) + self.assertEqual(channel(3), path[0].short_channel_id) + self.assertEqual(channel(2), path[1].short_channel_id) + self.cdb.stop() asyncio.run_coroutine_threadsafe(self.cdb.stopped_event.wait(), self.asyncio_loop).result() @@ -251,6 +289,12 @@ class Test_LNRouter(TestCaseForTestnet): self.assertEqual(3_000_000, hint.can_send(node_from < node_to)) self.assertEqual(None, hint.cannot_send(node_from < node_to)) + # test inflight htlc + liquidity_hints.reset_liquidity_hints() + liquidity_hints.add_htlc(node_from, node_to, channel_id) + liquidity_hints.get_hint(channel_id) + # we have got 600 (attempt) + 600 (inflight) penalty + self.assertEqual(1200, liquidity_hints.penalty(node_from, node_to, channel_id, 1_000_000)) @needs_test_with_all_chacha20_implementations def test_new_onion_packet_legacy(self):