mpp_split: use single nodes for mpp payments over trampoline

electrum/lnworker.py

@@ -1166,7 +1166,7 @@ async def pay_to_node(
             if code == OnionFailureCode.MPP_TIMEOUT:
                 raise PaymentFailure(failure_msg.code_name())
             # trampoline
-            if self.channel_db is None:
+            if not self.channel_db:
                 if code == OnionFailureCode.TRAMPOLINE_FEE_INSUFFICIENT:
                     # todo: parse the node parameters here (not returned by eclair yet)
                     trampoline_fee_level += 1
@@ -1415,21 +1415,24 @@ def create_routes_for_payment(
         except NoPathFound:
             if not invoice_features.supports(LnFeatures.BASIC_MPP_OPT):
                 raise
-            channels_with_funds = dict([
-                (cid, int(chan.available_to_spend(HTLCOwner.LOCAL)))
-                for cid, chan in self._channels.items() if not chan.is_frozen_for_sending()])
+
+            channels_with_funds = {(cid, chan.node_id): int(chan.available_to_spend(HTLCOwner.LOCAL))
+                for cid, chan in self._channels.items() if not chan.is_frozen_for_sending()}
             self.logger.info(f"channels_with_funds: {channels_with_funds}")
-            # Create split configurations that are rated according to our
-            # preference -funds = (low rating=high preference).
-            split_configurations = suggest_splits(amount_msat, channels_with_funds)
+            # for trampoline mpp payments we have to restrict ourselves to pay
+            # to a single node due to some incompatibility in Eclair, see:
+            # https://github.com/ACINQ/eclair/issues/1723
+            use_singe_node = not self.channel_db and constants.net is constants.BitcoinMainnet
+            split_configurations = suggest_splits(amount_msat, channels_with_funds, single_node=use_singe_node)
             self.logger.info(f'suggest_split {amount_msat} returned {len(split_configurations)} configurations')
+
             for s in split_configurations:
                 self.logger.info(f"trying split configuration: {s[0].values()} rating: {s[1]}")
                 routes = []
                 try:
                     if not self.channel_db:
                         buckets = defaultdict(list)
-                        for chan_id, part_amount_msat in s[0].items():
+                        for (chan_id, _), part_amount_msat in s[0].items():
                             chan = self.channels[chan_id]
                             if part_amount_msat:
                                 buckets[chan.node_id].append((chan_id, part_amount_msat))
@@ -1475,7 +1478,7 @@ def create_routes_for_payment(
                                 self.logger.info('not enough margin to pay trampoline fee')
                                 raise NoPathFound()
                     else:
-                        for chan_id, part_amount_msat in s[0].items():
+                        for (chan_id, _), part_amount_msat in s[0].items():
                             if part_amount_msat:
                                 channel = self.channels[chan_id]
                                 route = self.create_route_for_payment(
@@ -1765,7 +1768,7 @@ def htlc_failed(
             self.logger.info(f"htlc_failed {failure_message}")
 
             # check sent_buckets if we use trampoline
-            if self.channel_db is None and payment_secret in self.sent_buckets:
+            if not self.channel_db and payment_secret in self.sent_buckets:
                 amount_sent, amount_failed = self.sent_buckets[payment_secret]
                 amount_failed += amount_receiver_msat
                 self.sent_buckets[payment_secret] = amount_sent, amount_failed

electrum/mpp_split.py

@@ -1,6 +1,6 @@
 import random
 import math
-from typing import List, Tuple, Optional, Sequence, Dict
+from typing import List, Tuple, Optional, Sequence, Dict, TYPE_CHECKING
 from collections import defaultdict
 
 from .util import profiler
@@ -23,7 +23,7 @@
 MAX_PARTS = 5
 
 
-def unique_hierarchy(hierarchy: Dict[int, List[Dict[bytes, int]]]) -> Dict[int, List[Dict[bytes, int]]]:
+def unique_hierarchy(hierarchy: Dict[int, List[Dict[Tuple[bytes, bytes], int]]]) -> Dict[int, List[Dict[Tuple[bytes, bytes], int]]]:
     new_hierarchy = defaultdict(list)
     for number_parts, configs in hierarchy.items():
         unique_configs = set()
@@ -36,11 +36,26 @@ def unique_hierarchy(hierarchy: Dict[int, List[Dict[bytes, int]]]) -> Dict[int,
     return new_hierarchy
 
 
-def number_nonzero_parts(configuration: Dict[bytes, int]):
+def single_node_hierarchy(hierarchy: Dict[int, List[Dict[Tuple[bytes, bytes], int]]]) -> Dict[int, List[Dict[Tuple[bytes, bytes], int]]]:
+    new_hierarchy = defaultdict(list)
+    for number_parts, configs in hierarchy.items():
+        for config in configs:
+            # determine number of nodes in configuration
+            if number_nonzero_nodes(config) > 1:
+                continue
+            new_hierarchy[number_parts].append(config)
+    return new_hierarchy
+
+
+def number_nonzero_parts(configuration: Dict[Tuple[bytes, bytes], int]) -> int:
     return len([v for v in configuration.values() if v])
 
 
-def create_starting_split_hierarchy(amount_msat: int, channels_with_funds: Dict[bytes, int]):
+def number_nonzero_nodes(configuration: Dict[Tuple[bytes, bytes], int]) -> int:
+    return len({nodeid for (_, nodeid), amount in configuration.items() if amount > 0})
+
+
+def create_starting_split_hierarchy(amount_msat: int, channels_with_funds: Dict[Tuple[bytes, bytes], int]):
     """Distributes the amount to send to a single or more channels in several
     ways (randomly)."""
     # TODO: find all possible starting configurations deterministically
@@ -81,8 +96,8 @@ def balances_are_not_ok(proposed_balance_from, channel_from, proposed_balance_to
     return check
 
 
-def propose_new_configuration(channels_with_funds: Dict[bytes, int], configuration: Dict[bytes, int],
-                              amount_msat: int, preserve_number_parts=True) -> Dict[bytes, int]:
+def propose_new_configuration(channels_with_funds: Dict[Tuple[bytes, bytes], int], configuration: Dict[Tuple[bytes, bytes], int],
+                              amount_msat: int, preserve_number_parts=True) -> Dict[Tuple[bytes, bytes], int]:
     """Randomly alters a split configuration. If preserve_number_parts, the
     configuration stays within the same class of number of splits."""
 
@@ -162,9 +177,13 @@ def swap(config: dict):
 
 
 @profiler
-def suggest_splits(amount_msat: int, channels_with_funds, exclude_single_parts=True) -> Sequence[Tuple[Dict[bytes, int], float]]:
+def suggest_splits(amount_msat: int, channels_with_funds: Dict[Tuple[bytes, bytes], int],
+                   exclude_single_parts=True, single_node=False) \
+        -> Sequence[Tuple[Dict[Tuple[bytes, bytes], int], float]]:
     """Creates split configurations for a payment over channels. Single channel
-    payments are excluded by default."""
+    payments are excluded by default. channels_with_funds is keyed by
+    (channelid, nodeid)."""
+
     def rate_configuration(config: dict) -> float:
         """Defines an objective function to rate a split configuration.
 
@@ -185,7 +204,7 @@ def rate_configuration(config: dict) -> float:
 
         return F
 
-    def rated_sorted_configurations(hierarchy: dict) -> Sequence[Tuple[Dict[bytes, int], float]]:
+    def rated_sorted_configurations(hierarchy: dict) -> Sequence[Tuple[Dict[Tuple[bytes, bytes], int], float]]:
         """Cleans up duplicate splittings, rates and sorts them according to
         the rating. A lower rating is a better configuration."""
         hierarchy = unique_hierarchy(hierarchy)
@@ -233,4 +252,8 @@ def rated_sorted_configurations(hierarchy: dict) -> Sequence[Tuple[Dict[bytes, i
         except:
             pass
 
+    if single_node:
+        # we only take configurations that send to a single node
+        split_hierarchy = single_node_hierarchy(split_hierarchy)
+
     return rated_sorted_configurations(split_hierarchy)

electrum/tests/test_lnpeer.py

@@ -38,7 +38,7 @@
 
 from .test_lnchannel import create_test_channels
 from .test_bitcoin import needs_test_with_all_chacha20_implementations
-from . import ElectrumTestCase
+from . import TestCaseForTestnet
 
 def keypair():
     priv = ECPrivkey.generate_random_key().get_secret_bytes()
@@ -303,7 +303,7 @@ class PaymentDone(Exception): pass
 class TestSuccess(Exception): pass
 
 
-class TestPeer(ElectrumTestCase):
+class TestPeer(TestCaseForTestnet):
 
     @classmethod
     def setUpClass(cls):

electrum/tests/test_mpp_split.py

@@ -13,11 +13,12 @@ def setUp(self):
         super().setUp()
         # to make tests reproducible:
         random.seed(0)
+        # key tuple denotes (channel_id, node_id)
         self.channels_with_funds = {
-            0: 1_000_000_000,
-            1: 500_000_000,
-            2: 302_000_000,
-            3: 101_000_000,
+            (0, 0): 1_000_000_000,
+            (1, 1): 500_000_000,
+            (2, 0): 302_000_000,
+            (3, 2): 101_000_000,
         }
 
     def tearDown(self):
@@ -28,24 +29,30 @@ def tearDown(self):
     def test_suggest_splits(self):
         with self.subTest(msg="do a payment with the maximal amount spendable over a single channel"):
             splits = mpp_split.suggest_splits(1_000_000_000, self.channels_with_funds, exclude_single_parts=True)
-            self.assertEqual({0: 660_000_000, 1: 340_000_000, 2: 0, 3: 0}, splits[0][0])
+            self.assertEqual({(0, 0): 660_000_000, (1, 1): 340_000_000, (2, 0): 0, (3, 2): 0}, splits[0][0])
 
         with self.subTest(msg="do a payment with a larger amount than what is supported by a single channel"):
             splits = mpp_split.suggest_splits(1_100_000_000, self.channels_with_funds, exclude_single_parts=True)
             self.assertEqual(2, mpp_split.number_nonzero_parts(splits[0][0]))
 
         with self.subTest(msg="do a payment with the maximal amount spendable over all channels"):
             splits = mpp_split.suggest_splits(sum(self.channels_with_funds.values()), self.channels_with_funds, exclude_single_parts=True)
-            self.assertEqual({0: 1_000_000_000, 1: 500_000_000, 2: 302_000_000, 3: 101_000_000}, splits[0][0])
+            self.assertEqual({(0, 0): 1_000_000_000, (1, 1): 500_000_000, (2, 0): 302_000_000, (3, 2): 101_000_000}, splits[0][0])
 
         with self.subTest(msg="do a payment with the amount supported by all channels"):
             splits = mpp_split.suggest_splits(101_000_000, self.channels_with_funds, exclude_single_parts=False)
             for s in splits[:4]:
                 self.assertEqual(1, mpp_split.number_nonzero_parts(s[0]))
 
+    def test_send_to_single_node(self):
+        splits = mpp_split.suggest_splits(1_000_000_000, self.channels_with_funds, exclude_single_parts=True, single_node=True)
+        self.assertEqual({(0, 0): 738_000_000, (1, 1): 0, (2, 0): 262_000_000, (3, 2): 0}, splits[0][0])
+        for split in splits:
+            assert mpp_split.number_nonzero_nodes(split[0]) == 1
+
     def test_saturation(self):
         """Split configurations which spend the full amount in a channel should be avoided."""
-        channels_with_funds = {0: 159_799_733_076, 1: 499_986_152_000}
+        channels_with_funds = {(0, 0): 159_799_733_076, (1, 1): 499_986_152_000}
         splits = mpp_split.suggest_splits(600_000_000_000, channels_with_funds, exclude_single_parts=True)
 
         uses_full_amount = False