Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always pick the best paths, rather than (poorly) attempting to randomize #1610

Merged

Commits on Jul 19, 2022

  1. Fix tracking of collected value across pathfinding iterations

    If we end up "paying" for an `htlc_minimum_msat` with fees, we
    increment `already_collected_value_msat` by more than the amount
    of the path that we collected (who's `value_contribution_msat` is
    higher than the total payment amount, despite having been reduced
    down to the payment amount).
    
    This throws off our total value collection target, though in the
    coming commit(s) it would also throw off our path selection
    calculations.
    TheBlueMatt committed Jul 19, 2022
    Copy the full SHA
    0c3a47c View commit details
    Browse the repository at this point in the history
  2. Make route path selection optimize strictly for cost / amount

    Currently, after we've selected a number of candidate paths, we
    construct a route from a random set of paths repeatedly, and then
    select the route with the lowest total cost. In the vast majority
    of cases this ends up doing a bunch of additional work in order to
    select the path(s) with the total lowest cost, with some vague
    attempt at randomization that doesn't actually work.
    
    Instead, here, we simply sort available paths by `cost / amount`
    and select the top paths. This ends up in practice having the same
    end result with substantially less complexity. In some rare cases
    it gets a better result, which also would have been achieved
    through more random trials. This implies there may in such cases be
    a potential privacy loss, but not a substantial one, given our path
    selection is ultimately mostly deterministic in many cases (or, if
    it is not, then privacy is achieved through randomization at the
    scorer level).
    TheBlueMatt committed Jul 19, 2022
    Copy the full SHA
    985153e View commit details
    Browse the repository at this point in the history
  3. Reduce default max_channel_saturation_power_of_half to 2 (max 1/4)

    Saturating a channel beyond 1/4 of its capacity seems like a more
    reasonable threshold for avoiding a path than 1/2, especially given
    we should still be willing to send a payment with a lower
    saturation limit if it comes to that.
    
    This requires an (obvious) change to some router tests, but also
    requires a change to the `fake_network_test`, opting to simply
    remove some over-limit test code there - `fake_network_test` was
    our first ever functional test, and while it worked great to ensure
    LDK worked at all on day one, we now have a rather large breadth
    of functional tests, and a broad "does it work at all" test is no
    longer all that useful.
    TheBlueMatt committed Jul 19, 2022
    Copy the full SHA
    ff8d3f7 View commit details
    Browse the repository at this point in the history