From 4a17d5a31668dc8e9f156bbb0515d472060d3b07 Mon Sep 17 00:00:00 2001 From: ThomasV Date: Sat, 17 May 2025 15:29:14 +0200 Subject: [PATCH 1/2] pass number of htlc_slots_left to suggest_splits --- electrum/lnchannel.py | 27 +++++++++++++-------------- electrum/lnworker.py | 2 +- electrum/mpp_split.py | 30 +++++++++++++++++++----------- tests/test_mpp_split.py | 22 ++++++++++++++-------- 4 files changed, 47 insertions(+), 34 deletions(-) diff --git a/electrum/lnchannel.py b/electrum/lnchannel.py index 9336be484..2848cf6fd 100644 --- a/electrum/lnchannel.py +++ b/electrum/lnchannel.py @@ -1095,7 +1095,7 @@ class Channel(AbstractChannel): if amount_msat < chan_config.htlc_minimum_msat: raise PaymentFailure(f'HTLC value too small: {amount_msat} msat') - if self.too_many_htlcs(htlc_proposer): + if self.htlc_slots_left(htlc_proposer) == 0: raise PaymentFailure('Too many HTLCs already in channel') if amount_msat > self.remaining_max_inflight(htlc_receiver): @@ -1108,7 +1108,7 @@ class Channel(AbstractChannel): if max_can_send_msat < amount_msat: raise PaymentFailure(f'Not enough balance. can send: {max_can_send_msat}, tried: {amount_msat}') - def too_many_htlcs(self, htlc_proposer: HTLCOwner) -> bool: + def htlc_slots_left(self, htlc_proposer: HTLCOwner) -> int: # check "max_accepted_htlcs" htlc_receiver = htlc_proposer.inverted() ctn = self.get_next_ctn(htlc_receiver) @@ -1116,17 +1116,16 @@ class Channel(AbstractChannel): # If proposer is LOCAL we apply stricter checks as that is behaviour we can control. # This should lead to fewer disagreements (i.e. channels failing). strict = (htlc_proposer == LOCAL) - # this is the loose check BOLT-02 specifies: - if len(self.hm.htlcs_by_direction(htlc_receiver, direction=RECEIVED, ctn=ctn)) + 1 > chan_config.max_accepted_htlcs: - return True - # however, c-lightning is a lot stricter, so extra checks: - # https://github.com/ElementsProject/lightning/blob/4dcd4ca1556b13b6964a10040ba1d5ef82de4788/channeld/full_channel.c#L581 - if strict: - max_concurrent_htlcs = min(self.config[htlc_proposer].max_accepted_htlcs, - self.config[htlc_receiver].max_accepted_htlcs) - if len(self.hm.htlcs(htlc_receiver, ctn=ctn)) + 1 > max_concurrent_htlcs: - return True - return False + if not strict: + # this is the loose check BOLT-02 specifies: + return chan_config.max_accepted_htlcs - len(self.hm.htlcs_by_direction(htlc_receiver, direction=RECEIVED, ctn=ctn)) + else: + # however, c-lightning is a lot stricter, so extra checks: + # https://github.com/ElementsProject/lightning/blob/4dcd4ca1556b13b6964a10040ba1d5ef82de4788/channeld/full_channel.c#L581 + max_concurrent_htlcs = min( + self.config[htlc_proposer].max_accepted_htlcs, + self.config[htlc_receiver].max_accepted_htlcs) + return max_concurrent_htlcs - len(self.hm.htlcs(htlc_receiver, ctn=ctn)) def remaining_max_inflight(self, htlc_receiver: HTLCOwner) -> int: # check "max_htlc_value_in_flight_msat" @@ -1553,7 +1552,7 @@ class Channel(AbstractChannel): ) max_send_msat = min(max_send_msat, self.remaining_max_inflight(receiver)) - if self.too_many_htlcs(sender): + if self.htlc_slots_left(sender) == 0: max_send_msat = 0 max_send_msat = max(max_send_msat, 0) diff --git a/electrum/lnworker.py b/electrum/lnworker.py index 77e390af4..fe3ef9d45 100644 --- a/electrum/lnworker.py +++ b/electrum/lnworker.py @@ -1927,7 +1927,7 @@ class LNWallet(LNWorker): receiver_pubkey: bytes, ) -> List['SplitConfigRating']: channels_with_funds = { - (chan.channel_id, chan.node_id): int(chan.available_to_spend(HTLCOwner.LOCAL)) + (chan.channel_id, chan.node_id): ( int(chan.available_to_spend(HTLCOwner.LOCAL)), chan.htlc_slots_left(HTLCOwner.LOCAL)) for chan in my_active_channels } # if we have a direct channel it's preferrable to send a single part directly through this diff --git a/electrum/mpp_split.py b/electrum/mpp_split.py index c7ea3663a..3b0aa6417 100644 --- a/electrum/mpp_split.py +++ b/electrum/mpp_split.py @@ -15,7 +15,7 @@ MAX_PARTS = 5 # maximum number of parts for splitting # maps a channel (channel_id, node_id) to the funds it has available -ChannelsFundsInfo = Dict[Tuple[bytes, bytes], int] +ChannelsFundsInfo = Dict[Tuple[bytes, bytes], Tuple[int, int]] class SplitConfig(dict, Dict[Tuple[bytes, bytes], List[int]]): @@ -105,7 +105,7 @@ def rate_config( total_amount = config.total_config_amount() for channel, amounts in config.items(): - funds = channels_with_funds[channel] + funds, slots = channels_with_funds[channel] if amounts: for amount in amounts: rating += amount * amount / (total_amount * total_amount) # penalty to favor equal distribution of amounts @@ -116,7 +116,8 @@ def rate_config( def suggest_splits( - amount_msat: int, channels_with_funds: ChannelsFundsInfo, + amount_msat: int, + channels_with_funds: ChannelsFundsInfo, exclude_single_part_payments=False, exclude_multinode_payments=False, exclude_single_channel_splits=False @@ -132,32 +133,37 @@ def suggest_splits( """ configs = [] - channels_order = list(channels_with_funds.keys()) + channel_keys = list(channels_with_funds.keys()) # generate multiple configurations to get more configurations (there is randomness in this loop) for _ in range(CANDIDATES_PER_LEVEL): # we want to have configurations with no splitting to many splittings for target_parts in range(1, MAX_PARTS): config = SplitConfig() - # randomly split amount into target_parts chunks split_amounts = split_amount_normal(amount_msat, target_parts) # randomly distribute amounts over channels for amount in split_amounts: - random.shuffle(channels_order) + random.shuffle(channel_keys) # we check each channel and try to put the funds inside, break if we succeed - for c in channels_order: + for c in channel_keys: if c not in config: config[c] = [] - if sum(config[c]) + amount <= channels_with_funds[c]: + channel_funds, channel_slots = channels_with_funds[c] + if sum(config[c]) + amount <= channel_funds and len(config[c]) < channel_slots: config[c].append(amount) break # if we don't succeed to put the amount anywhere, # we try to fill up channels and put the rest somewhere else else: distribute_amount = amount - for c in channels_order: - funds_left = channels_with_funds[c] - sum(config[c]) + for c in channel_keys: + channel_funds, channel_slots = channels_with_funds[c] + slots_left = channel_slots - len(config[c]) + if slots_left == 0: + # no slot left in that channel + continue + funds_left = channel_funds - sum(config[c]) # it would be good to not fill the full channel if possible add_amount = min(funds_left, distribute_amount) config[c].append(add_amount) @@ -165,7 +171,7 @@ def suggest_splits( if distribute_amount == 0: break if config.total_config_amount() != amount_msat: - raise NoPathFound('Cannot distribute payment over channels.') + continue if target_parts > 1 and config.is_any_amount_smaller_than_min_part_size(): if target_parts == 2: # if there are already too small parts at the first split excluding single @@ -175,6 +181,8 @@ def suggest_splits( continue assert config.total_config_amount() == amount_msat configs.append(config) + if not configs: + raise NoPathFound('Cannot distribute payment over channels.') configs = remove_duplicates(configs) diff --git a/tests/test_mpp_split.py b/tests/test_mpp_split.py index 77c5e08b8..9efab660e 100644 --- a/tests/test_mpp_split.py +++ b/tests/test_mpp_split.py @@ -15,10 +15,10 @@ class TestMppSplit(ElectrumTestCase): random.seed(0) # key tuple denotes (channel_id, node_id) self.channels_with_funds = { - (b"0", b"0"): 1_000_000_000, - (b"1", b"1"): 500_000_000, - (b"2", b"0"): 302_000_000, - (b"3", b"2"): 101_000_000, + (b"0", b"0"): (1_000_000_000, 3), + (b"1", b"1"): (500_000_000, 2), + (b"2", b"0"): (302_000_000, 2), + (b"3", b"2"): (101_000_000, 1), } def tearDown(self): @@ -51,7 +51,7 @@ 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_part_payments=True) + sum([x[0] for x in self.channels_with_funds.values()]), self.channels_with_funds, exclude_single_part_payments=True) self.assertEqual({ (b"0", b"0"): [1_000_000_000], (b"1", b"1"): [500_000_000], @@ -75,7 +75,10 @@ class TestMppSplit(ElectrumTestCase): def test_saturation(self): """Split configurations which spend the full amount in a channel should be avoided.""" - channels_with_funds = {(b"0", b"0"): 159_799_733_076, (b"1", b"1"): 499_986_152_000} + channels_with_funds = { + (b"0", b"0"): (159_799_733_076, 1), + (b"1", b"1"): (499_986_152_000, 1) + } splits = mpp_split.suggest_splits(600_000_000_000, channels_with_funds, exclude_single_part_payments=True) uses_full_amount = False @@ -114,18 +117,21 @@ class TestMppSplit(ElectrumTestCase): def test_suggest_splits_single_channel(self): channels_with_funds = { - (b"0", b"0"): 1_000_000_000, + (b"0", b"0"): (1_000_000_000, 3), } - with self.subTest(msg="do a payment with the maximal amount spendable on a single channel"): splits = mpp_split.suggest_splits(1_000_000_000, channels_with_funds, exclude_single_part_payments=False) + self.assertEqual(1, len(splits[0].config[(b"0", b"0")])) self.assertEqual({(b"0", b"0"): [1_000_000_000]}, splits[0].config) + with self.subTest(msg="test sending an amount greater than what we have available"): self.assertRaises(NoPathFound, mpp_split.suggest_splits, *(1_100_000_000, channels_with_funds)) + with self.subTest(msg="test sending a large amount over a single channel in chunks"): mpp_split.PART_PENALTY = 0.5 splits = mpp_split.suggest_splits(1_000_000_000, channels_with_funds, exclude_single_part_payments=False) self.assertEqual(2, len(splits[0].config[(b"0", b"0")])) + with self.subTest(msg="test sending a large amount over a single channel in chunks"): mpp_split.PART_PENALTY = 0.3 splits = mpp_split.suggest_splits(1_000_000_000, channels_with_funds, exclude_single_part_payments=False) From e433b8d5bfbb45eb24fbe10fcd93e15b628f3d8d Mon Sep 17 00:00:00 2001 From: f321x Date: Tue, 20 May 2025 12:24:53 +0200 Subject: [PATCH 2/2] explicitly test the htlc slot limit in TestMppSplit --- tests/test_mpp_split.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_mpp_split.py b/tests/test_mpp_split.py index 9efab660e..2c6bfe244 100644 --- a/tests/test_mpp_split.py +++ b/tests/test_mpp_split.py @@ -68,6 +68,25 @@ class TestMppSplit(ElectrumTestCase): # a splitting of the parts into two self.assertEqual(2, splits[4].config.number_parts()) + with self.subTest(msg="no htlc slots available"): + channels = self.channels_with_funds.copy() + # set all available slots to 0 + for chan, (amount, _slots) in channels.items(): + channels[chan] = (amount, 0) + with self.assertRaises(NoPathFound): + mpp_split.suggest_splits(20_000_000, channels, exclude_single_part_payments=False) + + with self.subTest(msg="only one channel can add htlcs"): + channels = self.channels_with_funds.copy() + # set all available slots to 0 except for the first channel + for chan, (amount, _slots) in channels.items(): + if chan != (b"0", b"0"): + channels[chan] = (amount, 0) + splits = mpp_split.suggest_splits(1_000_000_000, channels, exclude_single_part_payments=True) + for split in splits: + # check that the whole amount has been split on this channel + self.assertEqual(sum(split.config[(b"0", b"0")]), 1_000_000_000) + def test_send_to_single_node(self): splits = mpp_split.suggest_splits(1_000_000_000, self.channels_with_funds, exclude_single_part_payments=False, exclude_multinode_payments=True) for split in splits: