lightning icon indicating copy to clipboard operation
lightning copied to clipboard

Add strict-forwarding option

Open Lagrang3 opened this issue 1 year ago • 2 comments

Add strict forwarding option

Description

When receiving a forward request lightningd selects the best channel with the next peer based on its ability to add an HTLC and its spendable amount. If two forwarding request arrive at the same time pointing to the same peer we can have a race condition in which both HTLCs compete concurrently for the same spendable amount.

There shouldn't be a race condition if both requests are supposed to be allocated on different parallel channels. However, non-strict forwarding is allowed in the protocol and it is reasonable that node operators can decide which channel to use when forwarding to a peer.

Therefore we add configuration option dev-strict-forwarding default to false, to enforce strict forwarding. With that flag we no longer need to skip parallel channel tests in renepay (9d88ce3).

Related Issues

  • Closes #7605

Changes Made

  • [x] Feature: the ability to turn off non-strict forwarding as a configuration option.

Checklist

Ensure the following tasks are completed before submitting the PR:

  • [x] Changelog has been added in relevant commit/s.
  • [x] Tests have been added or updated to cover the changes.
  • [x] Documentation has been updated as needed.
  • [x] Any relevant comments or TODOs have been addressed or removed.

Additional Notes

This is not a solution to the race condition, it only avoids it by removing the freedom to choose the forward channel.

Lagrang3 avatar Aug 24 '24 10:08 Lagrang3

OK, so 495403d, breaks the test_zeroconf_multichan_forward on tests/test_opening.py, because the test expects a different forward channel than the one chosen by the payer...

Shall we add a configuration option to enable/disable non-strict forwarding?

Lagrang3 avatar Aug 24 '24 13:08 Lagrang3

If the channels have any fee differences, they will also not be considered equivalent. So, varying feerates serves to override, too. This will affect renepay's algorithm too, unfortunately :(

rustyrussell avatar Oct 21 '24 23:10 rustyrussell

Trivial rebase for silly conflict in test_pay.py.

rustyrussell avatar Nov 11 '24 23:11 rustyrussell