Commit graph

649 commits

Author SHA1 Message Date
Jeffrey Czyz
54f96ef944
Use ChainHash instead of BlockHash as applicable
ChainHash is more appropriate for places where an arbitrary BlockHash is
not desirable. This type was introduced in later versions of the bitcoin
crate, thus BlockHash was used instead.

Using ChainHash also makes it easier to check if ChannelManager is
compatible with an Offer.
2023-10-16 13:29:53 -05:00
Matt Corallo
c86cacd29d
Merge pull request #2625 from tnull/2023-09-moar-tests-n-fixes
Improve test coverage of #2575 router fixes
2023-10-15 20:18:56 +00:00
Matt Corallo
087c8f683a Avoid blanket impls in bindings builds
The C bindings implements `Deref` for all traits, so having a
blanket `Deref` implementation ends up conflicting with this.
2023-10-01 00:05:01 +00:00
Matt Corallo
0bc0de46fa Simplify score bounding with a unified type
In a few places we require a unified scorer, which implements both
`ScoreLookUp` and `ScoreUpdate`. Rather than double-bounding (which
the bindings generator can't handle directly), we use a top-level
`Score` trait which requires both and is implemented for all
implementers of both supertraits.
2023-10-01 00:05:01 +00:00
Matt Corallo
748bebadb4 Remove unnecessary bounds in scoring
In our scoring logic we have a handful of unnecessary bounds,
leading to extra diff in the bindings branch when those bounds have
to be removed as well as a few cases where bindings generation
simply gets confused.

Here we remove a number of bounds across the scoring traits and
impls, cleaning things up and simplifying bindings changes.
2023-10-01 00:05:01 +00:00
Matt Corallo
c7c4226e99 Revert "Remove AvailableBalances::balance_msat"
While removing the `balance_msat` field absolutely makes sense -
it is, at best, confusing - we really need a solid replacement for
it before we can do so. While one such replacement is in progress,
it is not complete and we'd like to not block our current release
on its completion.

This reverts commit ef5be580f5.
2023-09-29 18:32:25 +00:00
Elias Rohrer
3a8bf8975e
Test we consider route hints if we are the src of the first hop
Previously, we would only consider route hints if the entry point was
in our first hops or in the network graph. We fixed this by also
considering hints if our own node ID was the first src.

Here, we add test coverage for this behavior.
2023-09-29 20:22:05 +02:00
Elias Rohrer
154cd3c043
Test we prefer first hops over route hints
We previously added logic that would avoid adding superflous candidates
for route hints if we detect that we have a first hop for this channel.

Here we add test coverage that we actually prefer the first hop over the
route hint, but still consider the remaining hints.
2023-09-29 20:22:05 +02:00
Matt Corallo
fa48df6049 Log max routing fee before we start pathfinding
This may be useful in debugging routing failures in the future.
2023-09-28 20:39:36 +00:00
Matt Corallo
8effd86c21 Include any recipient overpayment amounts in the route fee limit
If the user told us to limit their total fee exposure, we should
do so including any potential overpayment to the recipient, which
is ultimately a part of the "fee" as far as the user is concerned.
2023-09-28 20:39:36 +00:00
Matt Corallo
f3e33f4074 Try to overpay the recipient if we fail to find a path at all
Previously we'd only try to overpay if we managed to find a path
to the recipient which was sufficient. However, if we fail to find
any path to the recipient at all we should still retry overpaying
the recipient. Ultimately we should be silling to pay whatever
reasonable performance penalty if the alternative is not finding a
path at all, which we do here.
2023-09-28 20:39:36 +00:00
Matt Corallo
5861dde692 Allow routefinding again due to HTLC limit based on >= not >
While this doesn't matter much in practice, if we go around again
when route-finding to try to meet an htlc_minimum_msat, we use the
`recommended_value_msat` which can work if we meet the
`htlc_minimum_msat` on a channel exactly, so using >= rather than >
can capture cases with 1msat more.
2023-09-28 20:39:36 +00:00
Elias Rohrer
39c5bbc0bb
Make ignored candidate counts u32
.. as a follow-up from #2417.
2023-09-28 19:45:30 +02:00
Elias Rohrer
c480b0c857
Avoid adding duplicate hint candidates if they are first hops
If we have a direct channel to a node generating an invoice with route
hints, we'd previously happily add multiple candidates that all refer to
the same channel. To keep our candidate set small and unify our tracking
where possible, we now check if its `short_channel_id` is an
`outbound_scid_alias` of any of our first hops and refrain from adding
another candidate if it's the case.
2023-09-28 19:45:30 +02:00
Elias Rohrer
7a6d3097a6
Also add route hints if we are the source
Previously, we would only consider route hints if we had a direct
channel to the first node in the hint or if the first node in the hint
was part of the public network graph.

However, this left out the possiblity of us being part of the first hop,
especially if our own node is not announced and part of the graph.
2023-09-28 19:45:29 +02:00
Elias Rohrer
a8eef297fc
Extend logging of ignored candidates 2023-09-28 10:14:35 +02:00
Elias Rohrer
391da3f3b2
Use saturating_add when adding inflight HTLCs values
Previously this calculation could overflow, leading to panicking in `debug`.
2023-09-28 10:14:35 +02:00
Valentine Wallace
f3857d0f53
get_route: fix path value contribution to include min htlc overpay
Previously, the fuzzer hit a debug panic because we wouldn't include the amount
overpaid to meet a last hop's min_htlc in the total collected paths value. We
now include this value and also penalize hops along the overpaying path to
ensure that it gets deprioritized in path selection.
2023-09-27 13:19:51 -04:00
Valentine Wallace
ea38b938bb
get_route: fix path_min for first_hop<>network_node candidates
Previously, we would add a first_hop<>network_node channel that did not have
enough contribution amount to cover the next channel's min htlc plus fees,
because we were storing the next hop as having a path_min that did not include
fees, and would add a connecting first_hop node that did not have enough
contribution amount, leading to a debug panic upon invalid path construction.
2023-09-27 13:19:51 -04:00
Valentine Wallace
d83295ff5c
get_route: fix path_min for first_hop<>blinded_hint candidates
See previous commit, but the bug where we would underestimate how much a first
hop candidate needed to be able to relay was also present in blinded paths.
2023-09-27 13:19:51 -04:00
Valentine Wallace
2aacbae67a
get_route: fix path_min when adding first_hop<>route_hint candidates
Previously, we would add a candidate hop to the list of potential hops even
though its available contribution wasn't sufficient to meet the next hop's
min_htlc. We'd subsequently build an invalid path using this hop and hit a
debug assertion.
2023-09-27 13:19:51 -04:00
Valentine Wallace
5263b07b55
get_route: fix outdated var name
Previously this variable was a bool, but has since been updated to be an
Option, so rename accordingly.
2023-09-27 13:19:51 -04:00
Valentine Wallace
29c67d5246
Pathfinding: ignore blinded route hints where we are the intro node
See tests, but the fuzzer found several panics from not fully ignoring these
hints.

We should support these route hints eventually, but it will involve some
reworking of the Path/BlindedTail structs.
2023-09-27 13:19:49 -04:00
Valentine Wallace
b16d6a1e11
Remove trailing whitespace in get_route
Because my text editor loves to do that.
2023-09-27 09:37:21 -04:00
Valentine Wallace
7632cfe517
Refuse to pathfind when provided our_node_id matches internal dummy pk
The fuzzer managed to hit this and it causes some invalid paths to be generated
internally.
2023-09-27 09:37:21 -04:00
Valentine Wallace
6cfb052d98
Pathfinding: log when we ignore one hop blinded route hints 2023-09-27 09:37:21 -04:00
Matt Corallo
0c31c6f162 Set a default max_total_routing_fee_msat of 1% + 50sats
When using the normal default constructors, we should have some
fee maximum to ensure our default behavior is safe. Here we pick
1% + 50 sats to ensure we're always willing to pay
reasonabl(y high) fees, but not anything too wild.
2023-09-26 22:03:59 +00:00
Matt Corallo
793a1bf679 Derive Debug and Display for Route 2023-09-26 21:59:48 +00:00
Matt Corallo
1ac53ed02b
Merge pull request #2417 from tnull/2023-07-max-total-fee
Add config option to set maximum total routing fee
2023-09-26 20:07:52 +00:00
Elias Rohrer
b1a878fe81
Test we adhere to max_total_routing_fee_msat 2023-09-26 09:44:05 +02:00
Elias Rohrer
ca40d8a6fb
Consider RouteParameters::max_total_routing_fee_msat in get_route
We exclude any candidate hops if we find that using them would let the
aggregated path routing fees exceed `max_total_routing_fee_msat`.

Moreover, we return an error if the aggregated fees over all paths of
the selected route would surpass `max_total_routing_fee_msat`.
2023-09-26 09:44:05 +02:00
Elias Rohrer
d7e2ff6220
Introduce RouteParameters::max_total_routing_fee_msat
Currently, users have no means to upper-bound the total fees accruing
when finding a route. Here, we add a corresponding field to
`RouteParameters` which will be used to limit the candidate set during
path finding in the following commits.
2023-09-26 09:44:04 +02:00
Matt Corallo
6b0d94a302 Use Default::default() to construct () as a test scoring param
In bindings, we can't use unbounded generic types, and thus have to
rip out the `ScoreParams` and replace them with static
`ProbabilisticScoringFeeParams` universally. To make this easier,
using `Default::default()` everywhere allows the type to change out
from under the test without the test needing to change.
2023-09-21 01:44:23 +00:00
Matt Corallo
f2fe95e565
Merge pull request #2547 from TheBlueMatt/2023-04-nonlinear-scoring
Add an option to make the success probability estimation nonlinear
2023-09-20 22:21:02 +00:00
Matt Corallo
f2b2920b13 Avoid unnecessary newline in middle of log statement 2023-09-20 18:32:21 +00:00
Matt Corallo
259ceb0ebf Add an option to make the success probability estimation nonlinear
Our "what is the success probability of paying over a channel with
the given liquidity bounds" calculation currently assumes the
probability of where the liquidity lies in a channel is constant
across the entire capacity of a channel. This is obviously a
somewhat dubious assumption given most nodes don't materially
rebalance and flows within the network often push liquidity
"towards the edges".

Here we add an option to consider this when scoring channels during
routefinding. Specifically, if a new `linear_success_probability`
flag is unset on `ProbabilisticScoringFeeParameters`, rather than
assuming a PDF of `1` (across the channel's capacity scaled from 0
to 1), we use `(x - 0.5)^2`.

This assumes liquidity is likely to be near the edges, which
matches experimental results. Further, calculating the CDF (i.e.
integral) between arbitrary points on the PDF is trivial, which we
do as our main scoring function.

While this (finally) introduces floats in our scoring, its not
practical to exponentiate using fixed-precision, and benchmarks
show this is a performance regression, but not a huge one, more
than made up for by the increase in payment success rates.
2023-09-20 18:32:21 +00:00
Matt Corallo
df52da7b31 Score in-flight amounts as amounts, not a capacity reduction
When we started considering the in-flight amounts when scoring, we
took the approach of considering the in-flight amount as an
effective reduction in the channel's total capacity. When we were
scoring using a flat success probability PDF, that was fine,
however in the next commit we'll move to a highly nonlinear one,
which makes this a pretty confusing heuristic.

Here, instead, we move to considering the in-flight amount as
simply an extension of the amount we're trying to send over the
channel, which is equivalent for the flat success probability PDF,
but makes much more sense in a nonlinear world.
2023-09-20 18:32:21 +00:00
Matt Corallo
5f98c39927 Scale the success probability of channels without info down by 75%
If we are examining a channel for which we have no information at
all, we traditionally assume the HTLC success probability is
proportional to the channel's capacity. While this may be the case,
it is not the case that a tiny payment over a huge channel is
guaranteed to succeed, as we assume. Rather, the probability of
such success is likely closer to 50% than 100%.

Here we try to capture this by simply scaling the success
probability for channels where we have no information down
linearly. We pick 75% as the upper bound rather arbitrarily - while
50% may be more accurate, its possible it would lead to an
over-reliance on channels which we have paid through in the past,
which aren't necessarily always the best candidates.

Note that we only do this scaling for the historical bucket
tracker, as there we can be confident we've never seen a successful
HTLC completion on the given channel. If we were to apply the same
scaling to the simple liquidity bounds based scoring we'd penalize
channels we've never tried over those we've only ever fails to pay
over, which is obviously not a good outcome.
2023-09-19 21:23:28 +00:00
Matt Corallo
75438900b2 Split out success probability calculation to allow for changes
Our "what is the success probability of paying over a channel with
the given liquidity bounds" calculation is reused in a few places,
and is a key assumption across our main score calculation and the
historical bucket score calculations.

Here we break it out into a function to make it easier to
experiment with different success probability calculations.

Note that this drops the numerator +1 in the liquidity scorer,
which was added to compensate for the divisor + 1 (which exists to
avoid divide-by-zero), making the new math slightly less correct
but not by any material amount.
2023-09-19 21:22:49 +00:00
Matt Corallo
36af1f06fa
Merge pull request #2534 from tnull/2023-08-upstream-preflight-probing
Upstream and fix preflight probing
2023-09-18 16:41:57 +00:00
Elias Rohrer
c6a1a12aca
Include maybe_announced field in RouteHop
When sending preflight probes, we want to exclude last hops that are
possibly announced. To this end, we here include a new field in
`RouteHop` that will be `true` when we either def. know the hop to be
announced, or, if there exist public channels between the hop's
counterparties that this hop might refer to (i.e., be an alias for).
2023-09-18 15:08:27 +02:00
Matt Corallo
6d5c5ba4bb
Merge pull request #2176 from TheBlueMatt/2023-04-expose-success-prob
Move the historical bucket tracker to 32 unequal sized buckets
2023-09-15 22:38:57 +00:00
Matt Corallo
94376424c0 Move to a constant for "bucket one" in the scoring buckets
Scoring buckets are stored as fixed point ints, with a 5-bit
fractional part (i.e. a value of 1.0 is stored as "32"). Now that
we also have 32 buckets, this leads to the codebase having many
references to 32 which could reasonably be confused for each other.

Thus, we add a constant here for the value 1.0 in our fixed-point
scheme.
2023-09-15 17:27:31 +00:00
Matt Corallo
f7f524f19a Decay historical_estimated_channel_liquidity_* result to None
`historical_estimated_channel_liquidity_probabilities` previously
decayed to `Some(([0; 8], [0; 8]))`. This was thought to be useful
in that it allowed identification of cases where data was previously
available but is now decayed away vs cases where data was never
available. However, with the introduction of
`historical_estimated_payment_success_probability` (which uses the
existing scoring routines so will decay to `None`) this is
unnecessarily confusing.

Given data which has decayed to zero will also not be used anyway,
there's little reason to keep the old behavior, and we now decay to
`None`.

We also take this opportunity to split the overloaded
`get_decayed_buckets`, removing uneccessary code during scoring.
2023-09-15 17:27:29 +00:00
Matt Corallo
b7d1e5f516 Special-case the 0th minimum bucket in historical scoring
Points in the 0th minimum bucket either indicate we sent a payment
which is < 1/16,384th of the channel's capacity or, more likely,
we failed to send a payment. In either case, averaging the success
probability across the full range of upper-bounds doesn't make a
whole lot of sense - if we've never managed to send a "real"
payment over a channel, we should be considering it quite poor.

To address this, we special-case the 0th minimum bucket and only
look at the largest-offset max bucket when calculating the success
probability.
2023-09-15 17:20:38 +00:00
Matt Corallo
2ed21b87fa Track "steady-state" channel balances in history buckets not live
The lower-bound of the scoring history buckets generally never get
used - if we try to send a payment and it fails, we don't learn
a new lower-bound for the liquidity of a channel, and if we
successfully send a payment we only learn a lower-bound that
applied *before* we sent the payment, not after it completed.

If we assume channels have some "steady-state" liquidity, then
tracking our liquidity estimates *after* a payment doesn't really
make sense - we're not super likely to make a second payment across
the same channel immediately (or, if we are, we can use our
un-decayed liquidity estimates for that). By the time we do go to
use the same channel again, we'd assume that its back at its
"steady-state" and the impacts of our payment have been lost.

To combat both of these effects, here we "subtract" the impact of
any just-successful payments from our liquidity estimates prior to
updating the historical buckets.
2023-09-15 17:20:38 +00:00
Matt Corallo
da127d3f5f Move the historical bucket tracker to 32 unequal sized buckets
Currently we store our historical estimates of channel liquidity in
eight evenly-sized buckets, each representing a full octile of the
channel's total capacity. This lacks precision, especially at the
edges of channels where liquidity is expected to lie.

To mitigate this, we'd originally checked if a payment lies within
a bucket by comparing it to a sliding scale of 64ths of the
channel's capacity. This allowed us to assign penalties to payments
that fall within any more than the bottom 64th or lower than the
top 64th of a channel.

However, this still lacks material precision - on a 1 BTC channel
we could only consider failures for HTLCs above 1.5 million sats.
With today's lightning usage often including 1-100 sat payments in
tips, this is a rather significant lack of precision.

Here we rip out the existing buckets and replace them with 32
*unequal* sized buckets. This allows us to focus our precision at
the edges of a channel (where the liquidity is likely to lie, and
where precision helps the most).

We set the size of the edge buckets to 1/16,384th of the channel,
with the size increasing exponentially until it approaches the
inner buckets. For backwards compatibility, the buckets divide
evenly into octets, allowing us to convert the existing buckets
into the new ones cleanly.

This allows us to consider HTLCs down to 6,000 sats for 1 BTC
channels. In order to avoid failing to penalize channels which have
always failed, we drop the sliding scale for comparisons and simply
check if the payment is above the minimum bucket we're analyzing and
below *or in* the maximum one. This generates somewhat more
pessimistic scores, but fixes the lower bound where we suddenly
assign a 0% failure probability.

While this does represent a regression in routing performance, in
some cases the impact of not having to examine as many nodes
dominates, leading to a performance increase.

On a Xeon E3-1220 v5, the `large_mpp_routes` benchmark shows a 15%
performance increase, while the more stable benchmarks show an 8%
and 15% performance regression.
2023-09-15 17:20:38 +00:00
Matt Corallo
c74a581127 Clarify some scoring documentation by removing extraneous info 2023-09-15 17:20:38 +00:00
Antonio Yang
b1bedcfbec Rename SocketAddress from NetAddress 2023-09-08 20:42:06 +08:00
Elias Rohrer
543674f1da
Improve Route::get_total_amount docs 2023-09-06 19:35:39 +02:00