diff --git a/electrum/lnworker.py b/electrum/lnworker.py index e063ddd0d..90f3297e8 100644 --- a/electrum/lnworker.py +++ b/electrum/lnworker.py @@ -1166,7 +1166,7 @@ class LNWallet(LNWorker): 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 @@ class LNWallet(LNWorker): 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 @@ class LNWallet(LNWorker): 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 @@ class LNWallet(LNWorker): 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 diff --git a/electrum/mpp_split.py b/electrum/mpp_split.py index c2ac8c182..36d6ffa0b 100644 --- a/electrum/mpp_split.py +++ b/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 @@ REDISTRIBUTE = 20 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 propose_new_configuration(channels_with_funds: Dict[bytes, int], configurati @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 suggest_splits(amount_msat: int, channels_with_funds, exclude_single_parts=T 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 suggest_splits(amount_msat: int, channels_with_funds, exclude_single_parts=T 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) diff --git a/electrum/tests/test_lnpeer.py b/electrum/tests/test_lnpeer.py index f7386fbbc..724a386bf 100644 --- a/electrum/tests/test_lnpeer.py +++ b/electrum/tests/test_lnpeer.py @@ -38,7 +38,7 @@ from electrum.invoices import PR_PAID, PR_UNPAID 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): diff --git a/electrum/tests/test_mpp_split.py b/electrum/tests/test_mpp_split.py index 86f72f423..79d6a85c6 100644 --- a/electrum/tests/test_mpp_split.py +++ b/electrum/tests/test_mpp_split.py @@ -13,11 +13,12 @@ class TestMppSplit(ElectrumTestCase): 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,7 +29,7 @@ class TestMppSplit(ElectrumTestCase): 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) @@ -36,16 +37,22 @@ class TestMppSplit(ElectrumTestCase): 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