Often when we call `compute_fees` we really just want it to
saturate and we deal with `u64::max_value` later. In that case,
we're much better off doing the saturating in the `compute_fees` as
it can use CMOVs rather than branching at each step and then
`unwrap_or`ing at the callsite.
Our network graph has to be iterable in a deterministic order and
with the ability to iterate over a specific range. Thus,
historically, we've used a `BTreeMap` to do the iteration. This is
fine, except our map needs to also provide high performance lookups
in order to make route-finding fast. Sadly, `BTreeMap`s are quite
slow due to the branching penalty.
Here we replace the `BTreeMap`s in the scorer with a dummy wrapper.
In the next commit the internals thereof will be replaced with a
`HashMap`-based implementation.
As evidenced by the previous commit, it appears our A* router
does worse than a more naive approach. This isn't super surpsising,
as the A* heuristic calculation requires a map lookup, which is
relatively expensive.
```
test routing::router::benches::generate_mpp_routes_with_probabilistic_scorer ... bench: 169,991,943 ns/iter (+/- 30,838,048)
test routing::router::benches::generate_mpp_routes_with_zero_penalty_scorer ... bench: 122,144,987 ns/iter (+/- 61,708,911)
test routing::router::benches::generate_routes_with_probabilistic_scorer ... bench: 48,546,068 ns/iter (+/- 10,379,642)
test routing::router::benches::generate_routes_with_zero_penalty_scorer ... bench: 32,898,557 ns/iter (+/- 14,157,641)
```
Historically we've had various bugs in keeping the
`lowest_inbound_channel_fees` field in `NodeInfo` up-to-date as we
go. This leaves the A* routing less efficient as it can't prune
hops as aggressively.
In order to get accurate benchmarks, this commit updates the
minimum-inbound-fees field on load. This is not the most efficient
way of doing so, but suffices for fetching benchmarks and will be
removed in the coming commits.
Note that this is *slower* than the non-updating version in the
previous commit. While I haven't dug into this incredibly deeply,
the graph snapshot in use has min-fee info for only 9,618 of
20,818 nodes. Thus, it is my guess that with the graph snapshot
as-is the branch predictor is able to largely remove the A*
heuristic lookups, but with this change it is forced to wait for
A* heuristic map lookups to complete, causing a performance
regression.
```
test routing::router::benches::generate_mpp_routes_with_probabilistic_scorer ... bench: 182,980,059 ns/iter (+/- 32,662,047)
test routing::router::benches::generate_mpp_routes_with_zero_penalty_scorer ... bench: 151,170,457 ns/iter (+/- 75,351,011)
test routing::router::benches::generate_routes_with_probabilistic_scorer ... bench: 58,187,277 ns/iter (+/- 11,606,440)
test routing::router::benches::generate_routes_with_zero_penalty_scorer ... bench: 41,210,193 ns/iter (+/- 18,103,320)
```
The previous copy was more than one and a half years old, the
lightning network has changed a lot since!
As of this commit, performance on my Xeon W-10885M with a
SK hynix Gold P31 storing a BTRFS volume is as follows:
```
test ln::channelmanager::bench::bench_sends ... bench: 5,896,492 ns/iter (+/- 512,421)
test routing::gossip::benches::read_network_graph ... bench: 1,645,740,604 ns/iter (+/- 47,611,514)
test routing::gossip::benches::write_network_graph ... bench: 234,870,775 ns/iter (+/- 8,301,775)
test routing::router::benches::generate_mpp_routes_with_probabilistic_scorer ... bench: 166,155,032 ns/iter (+/- 30,206,162)
test routing::router::benches::generate_mpp_routes_with_zero_penalty_scorer ... bench: 136,843,661 ns/iter (+/- 67,111,218)
test routing::router::benches::generate_routes_with_probabilistic_scorer ... bench: 52,954,598 ns/iter (+/- 11,360,547)
test routing::router::benches::generate_routes_with_zero_penalty_scorer ... bench: 37,598,126 ns/iter (+/- 17,262,519)
test bench::bench_sends ... bench: 37,760,922 ns/iter (+/- 5,179,123)
test bench::bench_reading_full_graph_from_file ... bench: 25,615 ns/iter (+/- 1,149)
```
This is purely a refactor that does not change the InitFeatures
advertised by a ChannelManager. This allows users to configure which
features should be advertised based on the values of `UserConfig`. While
there aren't any existing features currently leveraging this behavior,
it will be used by the upcoming anchors_zero_fee_htlc_tx feature.
The UserConfig dependency on provided_init_features caused most
callsites of the main test methods responsible for opening channels to
be updated. This commit foregos that completely by no longer requiring
the InitFeatures of each side to be provided to these methods. The
methods already require a reference to each node's ChannelManager to
open the channel, so we use that same reference to obtain their
InitFeatures. A way to override such features was required for some
tests, so a new `override_init_features` config option now exists on
the test harness.
Every exported macro needed to have all the macros used inside it:
1- to be exported as well.
2- be called from the `$crate` namespace so it works in other crates.
Some structs in `lightning::util::ser` needed to be made public as they were used inside the exported macros.
Use the macros like this:
```Rust
lightning::impl_writeable_tlv_based!(...)
```
`ScorerAccountingForInFlightHtlcs` generally stores a `Score`
reference generated by calling `LockableScore::lock`, which
actually returns an arbitrary `Score`. Given `Score` is implemented
directly on lock types, it makes sense to simply hold a fully owned
`Score` in `ScorerAccountingForInFlightHtlcs` rather than a mutable
reference to one.
Strings defined by third parties may contain control characters. Provide
a wrapper such that these are replaced when displayed. Useful in node
aliases and offer fields.
This is part of moving the Router trait into ChannelManager, which will help
allow ChannelManager to fetch routes on-the-fly as part of supporting
trampoline payments.
If we send payments over a path where a channel ended up being
closed, we'll remove it before we call
`ProbabilisticPaymentScorer::payment_path_failed`. This should be
fine, except that `payment_path_failed` does not break out of its
scoring loop if a channel is missing, causing it to assign a
minimum available-liquidity of the payment amount even to channels
which our attempt never arrived at.
The fix is simple - add the missing check and break.
Because we now never generate an `EffectiveCapacity` with an
`htlc_maximum_msat` set to `None`, making it non-`Option`al
effectively removes dead code, which we do here.
We currently construct `DirectedChannelInfo`s for routing before
checking if the given direction has its directional info filled in.
We then always check for directional info before actually deciding
to route over a channel, as otherwise we assume the channel is not
online.
This makes for somewhat redundant checks, and `DirectedCHannelInfo`
isn't, by itself, a very useful API. Because fetching the HTLC-max
or effective channel capacity gives spurious data if no directional
info is available, there's little reason to have that data
available, and so we here check for directional info first. This
effectively merges `DirectionalChannelInfo` and
`DirectionalChannelInfoWithUpdate`.
In 56b07e52aa we made
`MultiThreadedLockableScore` fully bindings-compatible. However, it
did not add a `WriteableScore` implementation for it. This was an
oversight as it is a `WriteableScore` in Rust and needs to be for
use in other parts of the API.
Here we add the required impl in a way that the bindings generator
is able to handle it and add conversion utilities.
While applying gossip info from RGS-server, number of harmless
errors might arise which should be ignored. E.g. client should not
fail if there is a duplicate gossip for same channel or duplicate
update.
Even at relatively high payment volumes, decaying knowledge of each
individual channel every hour causes aggressive retrying of
channels as we quickly forget the state of a channel. Even with the
historical tracker, this isn't fully remedied, as we'll track the
history bounds with the decayed value.
Instead, we decay every six hours here, reducing how often we'll
retry a channel due to decay.
In addition to this, the decay likely needs to be substantially
more linear, as tracked in #1752.
We had some user confusion on how the probabilistic scorer works,
especially in reference to the half-life parameter. This attempts
to clarify how the bounds work, and how they are decayed.
