rust-lightning
4 months agoAvoid excess multiplies by multiplying in `success_probability` 2023-12-cache-scoring-points
Matt Corallo [Mon, 11 Dec 2023 03:33:46 +0000 (03:33 +0000)]
Avoid excess multiplies by multiplying in `success_probability`

A substantial portion (~12%!) of our scoring time is spent dividing
the bucket pair probability by the `success_probability` divisor.

Here, we avoid this by multiplying the bucket pair probability
in floating point math and using a floating point divide (which can
be faster in some cases). This also avoids the 2^30 multiplies that
are used to avoid rounding errors when converting the float
numerator and denominator to ints.

4 months agoManually lay out scorer memory
Matt Corallo [Mon, 11 Dec 2023 02:10:51 +0000 (02:10 +0000)]
Manually lay out scorer memory

4 months agoCache the total points tracked in our historical liquidity data
Matt Corallo [Sat, 9 Dec 2023 04:18:46 +0000 (04:18 +0000)]
Cache the total points tracked in our historical liquidity data

When we go to score a channel using the historical liquidity data,
the first thing we do is step through all the valid bucket
combinations, multiply the min and max bucket, and then add them
together to calculate the total number of points tracked. This
isn't a free operation, and for scorers without much data it
represents a large part of the total time spent scoring during
routefinding.

Thus, here we cache this value, updating it every time the buckets
are updated.

4 months agoChange the directed history tracker's storage of its direction
Matt Corallo [Sat, 9 Dec 2023 04:30:08 +0000 (04:30 +0000)]
Change the directed history tracker's storage of its direction

Rather than storing the two direction's buckets in
`HistoricalMinMaxBuckets` (renamed
`DirectedHistoricalLiquidityTracker`), we store a single reference
to the `HistoricalLiquidityTracker` as well as the direction bool.

This will allow us in the next commit to reference fields in the
`HistoricalLiquidityTracker` aside from the two directions.

4 months agoMake the historical bucket data private to `bucketed_history`
Matt Corallo [Sat, 9 Dec 2023 04:29:12 +0000 (04:29 +0000)]
Make the historical bucket data private to `bucketed_history`

In a comming commit we'll cache some additional data in the
historical bucket tracker. In order to do so, here we isolate the
buckets themselves into the `bucketed_history` module, reducing
the possibility of accidentally updating them directly without
updating caches.

4 months agoStore both min and max historical buckets in one, new, struct
Matt Corallo [Sat, 9 Dec 2023 03:32:40 +0000 (03:32 +0000)]
Store both min and max historical buckets in one, new, struct

In the coming commits we'll isolate historical bucket logic slightly
further, allowing us to cache some state. This is the first step
towards that, storing the historical liquidity information in a new
`HistoricalLiquidityTracker` rather than in the general
`ChannelLiquidity`.

4 months agoMove log approximation from `scoring.rs` to its own file
Matt Corallo [Sat, 9 Dec 2023 00:36:47 +0000 (00:36 +0000)]
Move log approximation from `scoring.rs` to its own file

4 months agoUse a real (probing-generated) scorer in benchmarks
Matt Corallo [Sun, 10 Dec 2023 18:23:17 +0000 (18:23 +0000)]
Use a real (probing-generated) scorer in benchmarks

Until now, our routing benchmarks used a synthetic scorer,
generated by scoring random paths to build up some history. This is
pretty far removed from real-world routing conditions, as
alternative paths generally have no scoring information and even
the paths we do take have only one or two past scoring results.

Instead, we fetch a static serialized scorer, generated using
minutely probes. This means future changes to the scorer's data may
be harder to benchmark, but makes for substantially more realistic
benchmarks for changes which don't impact the serialized state.

4 months agoForce inlining source and target node id lookup in candidate hops
Matt Corallo [Sat, 9 Dec 2023 21:12:30 +0000 (21:12 +0000)]
Force inlining source and target node id lookup in candidate hops

Because our router is rather large, LLVM doesn't always decide to
inline small functions in it, opting instead for a call. This is
somewhat wasteful when we do repeated `match`es on the same data,
so here we force inlining of the `source` and `target` `NodeId`
lookups.

4 months agoPrefetch per-direction channel info before looking at the channel
Matt Corallo [Sun, 10 Dec 2023 04:30:09 +0000 (04:30 +0000)]
Prefetch per-direction channel info before looking at the channel

In the previous commit, we laid out `ChannelInfo` to ensure most of
the data we cared about was sitting on two adjacent cache lines.
However, this left off the per-direction `ChannelUpdateInfo`, which
is sitting elsewhere.

Here, we try to reduce the cost we pay for accessing those when we
call `ChannelInfo::as_directed` by prefetching them as soon as we
fetch the `ChannelInfo` from the per-channel `HashMap`. We then
check the features for unknown flags, giving the CPU a handful of
instructions to chew on before we actually need the
`ChannelUpdateInfo`.

Sadly, this currently requires unsafe Rust (and is currently only
available on stable for x86), even though the x86 ISA is explicit
that the instruction "does not affect program behavior". Still,
this optimization reduces time taken waiting for the
`ChannelUpdateInfo` to load from ~5% of our routefinding time to
~2.5%, for a net reduction of ~2.5% in routefinding time.

4 months agoLayout channel info to ensure routing uses cache lines well
Matt Corallo [Sat, 9 Dec 2023 21:22:52 +0000 (21:22 +0000)]
Layout channel info to ensure routing uses cache lines well

Because we scan per-channel information in the hot inner loop of
our routefinding immediately after looking a channel up in a
`HashMap`, we end up spending a nontrivial portion of our
routefinding time waiting on memory to be read in.

While there is only so much we can do about that, ensuring the
channel information that we care about is sitting on one or
adjacent cache lines avoids paying that penalty twice. Thus, here
we manually lay out `ChannelInfo` and `ChannelUpdateInfo` and set
them to 128b and 32b alignment, respectively. This wastes some
space in memory in our network graph, but improves routing
performance in return.

4 months agoPush the route benchmark results into a separate uninlined function
Matt Corallo [Sat, 9 Dec 2023 18:51:09 +0000 (18:51 +0000)]
Push the route benchmark results into a separate uninlined function

This ensures the route benchmarks themselves will appear with a
distinct callgraph, making router profiling somewhat easier.

4 months agoConsolidate `candidate` access in `add_entry` during routing
Matt Corallo [Fri, 8 Dec 2023 19:43:17 +0000 (19:43 +0000)]
Consolidate `candidate` access in `add_entry` during routing

Because fetching fields from the `$candidate` often implies an
indirect read, grouping them together may result in one or two
fewer memory loads, so we do so here.

4 months agoSomewhat optimize the generic `Features::requires_unknown_bits`
Matt Corallo [Fri, 8 Dec 2023 05:44:32 +0000 (05:44 +0000)]
Somewhat optimize the generic `Features::requires_unknown_bits`

It turns out we spend several percent of our routefinding time just
checking if nodes and channels require unknown features
byte-by-byte. While the cost is almost certainly dominated by the
memory read latency, avoiding doing the checks byte-by-byte should
reduce the branch count slightly, which may reduce the overhead.

4 months agoStore source/target `node_counter`s in `DirectionalChannelInfo`
Matt Corallo [Fri, 8 Dec 2023 01:49:20 +0000 (01:49 +0000)]
Store source/target `node_counter`s in `DirectionalChannelInfo`

Because we now have some slack space in `PathBuildingHop`, we can
use it to cache some additional hot values. Here we use it to
cache the source and target `node_counter`s for public channels,
effectively prefetching the values from the channel state.

4 months agoDRY redundant calls to `$candidate.htlc_minimum_msat()` in routing
Matt Corallo [Fri, 8 Dec 2023 01:49:08 +0000 (01:49 +0000)]
DRY redundant calls to `$candidate.htlc_minimum_msat()` in routing

While LLVM should inline and elide the redundant calls, because the
router is rather large LLVM can decide against inlining in some
cases where it would be an nice win.

Thus, its worth DRY'ing the redundant calls explicitly.

4 months agoAlign `PathBuildingHop` to 128b, now that we store them in a `Vec`
Matt Corallo [Thu, 7 Dec 2023 23:40:26 +0000 (23:40 +0000)]
Align `PathBuildingHop` to 128b, now that we store them in a `Vec`

Now that `PathBuildingHop` is stored in a `Vec` (as `Option`s),
rather than `HashMap` entries, they can grow to fill a full two
cache lines without a memory access performance cost. In the next
commit we'll take advantage of this somewhat, but here we update
the assertions and drop the `repr(C)`, allowing rust to lay the
memory out as it wishes.

4 months agoCache whether a node is a first-hop target in the per-node state
Matt Corallo [Sun, 10 Dec 2023 03:28:37 +0000 (03:28 +0000)]
Cache whether a node is a first-hop target in the per-node state

When processing the main loop during routefinding, for each node,
we check whether it happens to be our peer in one of our channels.
This ensures we never fail to find a route that takes a hop through
a private channel of ours, to a private node, then through
invoice-provided route hints to reach the ultimate payee.

Because this is incredibly hot code, doing a full `HashMap` lookup
to check if each node is a first-hop target ends up eating a good
chunk of time during routing. Luckily, we can trivially avoid this
cost.

Because we're already looking up the per-node state in the `dist`
map, we can store a bool in each first-hop target's state, avoiding
the lookup unless we know its going to succeed.

This requires storing a dummy entry in `dist`, which feels somewhat
strange, but is ultimately fine as we should never be looking at
per-node state unless we've already found a path to that node,
updating the fields in doign so.

4 months agoDrop the `dist` `HashMap` in routing, replacing it with a `Vec`.
Matt Corallo [Thu, 7 Dec 2023 20:59:02 +0000 (20:59 +0000)]
Drop the `dist` `HashMap` in routing, replacing it with a `Vec`.

Now that we have unique, dense, 32-bit identifiers for all the
nodes in our network graph, we can store the per-node information
when routing in a simple `Vec` rather than a `HashMap`. This avoids
the overhead of hashing and table scanning entirely, for a nice
"simple" optimization win.

Because we must ensure we have a 1-to-1 mapping from node ids to
`node_counter`s, even across first-hop and last-hop hints, we have
to be careful to check the network graph first, then a new
`private_node_id_to_node_counter` map to ensure we only ever end up
with one counter per node id.

4 months agoStore the source and destination `node_counter`s in `ChannelInfo`
Matt Corallo [Thu, 7 Dec 2023 04:32:06 +0000 (04:32 +0000)]
Store the source and destination `node_counter`s in `ChannelInfo`

In the next commit, we'll use the new `node_counter`s to remove a
`HashMap` from the router, using a `Vec` to store all our per-node
information. In order to make finding entries in that `Vec` cheap,
here we store the source and destintaion `node_counter`s in
`ChannelInfo`, givind us the counters for both ends of a channel
without doing a second `HashMap` lookup.

4 months agoTrack a `counter` for each node in our network graph
Matt Corallo [Thu, 7 Dec 2023 04:12:04 +0000 (04:12 +0000)]
Track a `counter` for each node in our network graph

These counters are simply a unique number describing each node.
They have no specific meaning, but start at 0 and count up, with
counters being reused after a node has been deleted.

4 months agoDrop fake time advancing in scoring tests
Matt Corallo [Tue, 5 Dec 2023 18:15:55 +0000 (18:15 +0000)]
Drop fake time advancing in scoring tests

Now that we use explicit times passed to decay methods, there's no
reason to make calls to `SinceEpoch::advance` in scoring tests.

4 months agof drop shift test
Matt Corallo [Tue, 5 Dec 2023 18:15:48 +0000 (18:15 +0000)]
f drop shift test

4 months agoDrop half-life-based bucket decay in `update_history_buckets`
Matt Corallo [Wed, 29 Nov 2023 00:33:16 +0000 (00:33 +0000)]
Drop half-life-based bucket decay in `update_history_buckets`

Because we decay the bucket information in the background, there's
not much reason to try to decay them immediately prior to updating,
and in removing that we can also clean up a good bit of dead code,
which we do here.

4 months agoMake scorer decay + persistence more frequent
Matt Corallo [Wed, 29 Nov 2023 00:31:00 +0000 (00:31 +0000)]
Make scorer decay + persistence more frequent

There's some edge cases in our scoring when the information really
should be decayed but hasn't yet been prior to an update. Rather
than try to fix them exactly, we instead decay the scorer a bit
more often, which largely solves them but also gives us a bit more
accurate bounds on our channels, allowing us to reuse channels at
a similar amount to what just failed immediately, but at a
substantial penalty.

4 months agoDrop warning about mixing `no-std` and `std` `ProbabilisticScorer`s
Matt Corallo [Thu, 12 Oct 2023 18:23:51 +0000 (18:23 +0000)]
Drop warning about mixing `no-std` and `std` `ProbabilisticScorer`s

Now that the serialization format of `no-std` and `std`
`ProbabilisticScorer`s both just use `Duration` since UNIX epoch
and don't care about time except when decaying, we don't need to
warn users to not mix the scorers across `no-std` and `std` flags.

Fixes #2539

4 months agoAdd a benchmark for decaying a 100k channel scorer's liquidity info
Matt Corallo [Mon, 9 Oct 2023 03:23:55 +0000 (03:23 +0000)]
Add a benchmark for decaying a 100k channel scorer's liquidity info

This is a good gut-check to ensure we don't end up taking a ton of
time decaying channel liquidity info.

It currently clocks in around 1.25ms on an i7-1360P.

4 months agoDrop now-trivial `decayed_offset_msat` helper utility
Matt Corallo [Mon, 9 Oct 2023 02:21:09 +0000 (02:21 +0000)]
Drop now-trivial `decayed_offset_msat` helper utility

As we now no longer decay bounds information when fetching them,
there is no need to have a decaying-fetching helper utility.

4 months agoDrop now-unused `T: Time` bound on `ProbabilisticScorer`
Matt Corallo [Mon, 9 Oct 2023 01:52:20 +0000 (01:52 +0000)]
Drop now-unused `T: Time` bound on `ProbabilisticScorer`

Now that we don't access time via the `Time` trait in
`ProbabilisticScorer`, we can finally drop the `Time` bound
entirely, removing the `ProbabilisticScorerUsingTime` and type
alias indirection and replacing it with a simple struct.

4 months agoUse `Duration` based time info in scoring rather than `Time`
Matt Corallo [Mon, 9 Oct 2023 01:44:33 +0000 (01:44 +0000)]
Use `Duration` based time info in scoring rather than `Time`

In the coming commits, the `T: Time` bound on `ProbabilisticScorer`
will be removed. In order to enable that, we need to switch over to
using the `ScoreUpdate`-provided current time (as a `Duration`
since the unix epoch), making the `T` bound entirely unused.

4 months agoPipe `Duration`-based time information through scoring pipeline
Matt Corallo [Mon, 9 Oct 2023 01:15:18 +0000 (01:15 +0000)]
Pipe `Duration`-based time information through scoring pipeline

In the coming commits, the `T: Time` bound on `ProbabilisticScorer`
will be removed. In order to enable that, we need to pass the
current time (as a `Duration` since the unix epoch) through the
score updating pipeline, allowing us to keep the
`*last_updated_time` fields up-to-date as we go.

4 months agoUpdate history bucket last_update time immediately on update
Matt Corallo [Wed, 29 Nov 2023 03:07:54 +0000 (03:07 +0000)]
Update history bucket last_update time immediately on update

Now that we aren't decaying during scoring, when we set the
last_updated time in the history bucket logic doesn't matter, so
we should just update it when we've just updated the history
buckets.

4 months agoStop decaying liquidity information during bounds-based scoring
Matt Corallo [Mon, 9 Oct 2023 01:11:10 +0000 (01:11 +0000)]
Stop decaying liquidity information during bounds-based scoring

Because scoring is an incredibly performance-sensitive operation,
doing liquidity information decay (and especially fetching the
current time!) during scoring isn't really a great idea. Now that
we decay liquidity information in the background, we don't have any
reason to decay during scoring, and we ultimately remove it
entirely here.

4 months agoStop decaying historical liquidity information during scoring
Matt Corallo [Mon, 9 Oct 2023 02:14:21 +0000 (02:14 +0000)]
Stop decaying historical liquidity information during scoring

Because scoring is an incredibly performance-sensitive operation,
doing liquidity information decay (and especially fetching the
current time!) during scoring isn't really a great idea. Now that
we decay liquidity information in the background, we don't have any
reason to decay during scoring, and we remove the historical bucket
liquidity decaying here.

4 months agof handle zero half life
Matt Corallo [Tue, 5 Dec 2023 18:15:34 +0000 (18:15 +0000)]
f handle zero half life

4 months agof include ms/ns time parts when decaying
Matt Corallo [Tue, 5 Dec 2023 17:51:31 +0000 (17:51 +0000)]
f include ms/ns time parts when decaying

4 months agoImpl decaying in `ProbabilisticScorer::decay_liquidity_certainty`
Matt Corallo [Mon, 2 Oct 2023 20:07:21 +0000 (20:07 +0000)]
Impl decaying in `ProbabilisticScorer::decay_liquidity_certainty`

This implements decaying in the `ProbabilisticScorer`'s
`ScoreLookup::decay_liquidity_certainty` implementation, using
floats for accuracy since we're no longer particularly
time-sensitive. Further, it (finally) removes score entries which
have decayed to zero.

4 months agof temp bug
Matt Corallo [Tue, 5 Dec 2023 18:10:28 +0000 (18:10 +0000)]
f temp bug

4 months agoTrack historical liquidity update time separately from the bounds
Matt Corallo [Mon, 2 Oct 2023 19:44:36 +0000 (19:44 +0000)]
Track historical liquidity update time separately from the bounds

In the next commit, we'll start to use the new
`ScoreUpdate::decay_liquidity_certainty` to decay our bounds in the
background. This will result in the `last_updated` field getting
updated regularly on decay, rather than only on update. While this
isn't an issue for the regular liquidity bounds, it poses a problem
for the historical liquidity buckets, which are decayed on a
separate (and by default much longer) timer. If we didn't move to
tracking their decays separately, we'd never let the `last_updated`
field get old enough for the historical buckets to decay at all.

Instead, here we introduce a new `Duration` in the
`ChannelLiquidity` which tracks the last time the historical
liquidity buckets were last updated. We initialize it to a copy of
`last_updated` on deserialization if it is missing.

4 months agof sp
Matt Corallo [Tue, 5 Dec 2023 17:50:22 +0000 (17:50 +0000)]
f sp

4 months agof correct decay+persist log line
Matt Corallo [Tue, 5 Dec 2023 17:50:13 +0000 (17:50 +0000)]
f correct decay+persist log line

4 months agof docs
Matt Corallo [Wed, 29 Nov 2023 00:32:59 +0000 (00:32 +0000)]
f docs

4 months agoAdd a scoring decay method to the `ScoreUpdate` trait
Matt Corallo [Mon, 2 Oct 2023 19:14:26 +0000 (19:14 +0000)]
Add a scoring decay method to the `ScoreUpdate` trait

Rather than relying on fetching the current time during
routefinding, here we introduce a new trait method to `ScoreUpdate`
to do so. This largely mirrors what we do with the `NetworkGraph`,
and allows us to take on much more expensive operations (floating
point exponentiation) in our decaying.

4 months agoPass the current time through `ScoreUpDate` methods
Matt Corallo [Mon, 2 Oct 2023 18:33:08 +0000 (18:33 +0000)]
Pass the current time through `ScoreUpDate` methods

In the coming commits, we'll stop relying on fetching the time
during routefetching, preferring to decay score data in the
background instead.

The first step towards this - passing the current time through into
the scorer when updating.

4 months agoDepend on `libm` in `no-std` for `powf`(64)
Matt Corallo [Wed, 1 Nov 2023 01:16:12 +0000 (01:16 +0000)]
Depend on `libm` in `no-std` for `powf`(64)

In the next commits we'll need `f64`'s `powf`, which is only
available in `std`. For `no-std`, here we depend on `libm` (a
`rust-lang` org project), which we can use for `powf`.

4 months agoMerge pull request #2752 from valentinewallace/2023-11-large-final-onion-payload...
Matt Corallo [Fri, 8 Dec 2023 23:53:27 +0000 (23:53 +0000)]
Merge pull request #2752 from valentinewallace/2023-11-large-final-onion-payload-fixes

Large final onion payload fixes

4 months agoMerge pull request #2774 from TheBlueMatt/2023-12-2551-followups
Elias Rohrer [Fri, 8 Dec 2023 22:46:43 +0000 (23:46 +0100)]
Merge pull request #2774 from TheBlueMatt/2023-12-2551-followups

Doc and performance followups to #2551

4 months agoError if onion payloads exceed max length on packet construction.
Valentine Wallace [Fri, 8 Dec 2023 22:23:01 +0000 (17:23 -0500)]
Error if onion payloads exceed max length on packet construction.

Ensure that if we call construct_onion_packet and friends where payloads are
too large for the allotted packet length, we'll fail to construct. Previously,
senders would happily construct invalid packets by array-shifting the final
node's HMAC out of the packet when adding an intermediate onion layer, causing
the receiver to error with "final payload provided for us as an intermediate
node."

4 months agoFix debug panic in onion utils on large custom TLVs or metadata.
Valentine Wallace [Wed, 22 Nov 2023 23:21:57 +0000 (18:21 -0500)]
Fix debug panic in onion utils on large custom TLVs or metadata.

We previously assumed that the final node's payload would be ~93 bytes, and had
code to ensure that the filler encoded after that payload is not all 0s. Now
with custom TLVs and metadata supported, the final node's payload may take up
the entire onion packet, so we can't assume that there are 64 bytes of filler
to check.

4 months agoPre-calculate heap element scores (retaining RouteGraphNode size) 2023-12-2551-followups
Matt Corallo [Wed, 6 Dec 2023 05:29:28 +0000 (05:29 +0000)]
Pre-calculate heap element scores (retaining RouteGraphNode size)

`RouteGraphNode` currently recalculates scores in its `Ord`
implementation, wasting time while sorting the main Dijkstra's
heap.

Further, some time ago, when implementing the `htlc_maximum_msat`
amount reduction while walking the graph, we added
`PathBuildingHop::was_processed`, looking up the source node in
`dist` each time we pop'ed an element off of the binary heap.
As a result, we now have a reference to our `PathBuildingHop` when
processing a best-node's channels, leading to several fields in
`RouteGraphNode` being entirely redundant.

Here we drop those fields, but add a pre-calculated score field,
as well as force a suboptimal `RouteGraphNode` layout, retaining
its existing 64 byte size.

Without the suboptimal layout, performance is very mixed, but with
it performance is mostly improved, by around 10% in most tests.

4 months agoReorder `PathBuildingHop` fields somewhat
Matt Corallo [Wed, 6 Dec 2023 05:02:07 +0000 (05:02 +0000)]
Reorder `PathBuildingHop` fields somewhat

Given `PathBuildingHop` is now an even multiple of cache lines, we
can pick which fields "fall off" the cache line we have visible
when dealing with hops, which we do here.

4 months agoMake `find_route`'s `dist` map elements fit in 128 bytes
Matt Corallo [Wed, 6 Dec 2023 06:02:37 +0000 (06:02 +0000)]
Make `find_route`'s `dist` map elements fit in 128 bytes

We'd previously aggressively cached elements in the
`PathBuildingHop` struct (and its sub-structs), which resulted in a
rather bloated size. This implied cache misses as we read from and
write to multiple cache lines during processing of a single
channel.

Here, we reduce caching in `DirectedChannelInfo`, fitting the
`(NodeId, PathBuildingHop)` tuple in exactly 128 bytes. While this
should fit in a single cache line, it sadly does not generally lie
in only two lines, as glibc returns large buffers from `malloc`
which are very well aligned, plus 16 bytes (for its own allocation
tracking). Thus, we try to avoid reading from the last 16 bytes of
a `PathBuildingHop`, but luckily that isn't super hard.

Note that here we make accessing
`DirectedChannelInfo::effective_capacity` somewhat slower, but
that's okay as its only ever done once per `DirectedChannelInfo`
anyway.

While our routing benchmarks are quite noisy, this appears to
result in between a 5% and 15% performance improvement in the
probabilistic scoring benchmarks.

4 months agoMake `CandidateRouteHop::PrivateHop::target_node_id` a reference
Matt Corallo [Wed, 6 Dec 2023 03:54:28 +0000 (03:54 +0000)]
Make `CandidateRouteHop::PrivateHop::target_node_id` a reference

This avoids bloating `CandidateRouteHop` with a full 33-byte
node_id (and avoids repeated public key serialization when we do
multiple pathfinding passes).

4 months agoSimplify and make scoring calls in `TestRouter` more complete
Matt Corallo [Wed, 6 Dec 2023 17:47:00 +0000 (17:47 +0000)]
Simplify and make scoring calls in `TestRouter` more complete

`TestRouter` tries to make scoring calls that mimic what an actual
router would do, but the changes in f0ecc3ec73dcdb9303b1bd5ac687a36
failed to make scoring calls for private hints or if we take a
public hop for the last hop.

This fixes those regressions, though no tests currently depend on
this behavior.

4 months agoMake `CandidateRouteHop` method docs somewhat more descriptive
Matt Corallo [Wed, 6 Dec 2023 02:23:30 +0000 (02:23 +0000)]
Make `CandidateRouteHop` method docs somewhat more descriptive

4 months agoFix indentation in `router.rs` broken in a1d15ac1926f70aa5ab4f6686f
Matt Corallo [Wed, 6 Dec 2023 01:19:17 +0000 (01:19 +0000)]
Fix indentation in `router.rs` broken in a1d15ac1926f70aa5ab4f6686f

4 months agoRename `CandidateRouteHop::FirstHop::node_id` and make it a ref
Matt Corallo [Wed, 6 Dec 2023 01:22:21 +0000 (01:22 +0000)]
Rename `CandidateRouteHop::FirstHop::node_id` and make it a ref

Rather than calling `CandidateRouteHop::FirstHop::node_id` just
`node_id`, we should call it `payer_node_id` to provide more
context.

We also take this opportunity to make it a reference, avoiding
bloating `CandidateRouteHop`.

4 months ago`#[inline]` `CandidateRouteHop` accessors
Matt Corallo [Wed, 6 Dec 2023 01:17:48 +0000 (01:17 +0000)]
`#[inline]` `CandidateRouteHop` accessors

These are used in the performance-critical routing and scoring
operations, which may happen outside of our crate. Thus, we really
need to allow downstream crates to inline these accessors into
their code, which we do here.

4 months agoFix new unused warnings in `scoring.rs`
Matt Corallo [Wed, 6 Dec 2023 17:17:27 +0000 (17:17 +0000)]
Fix new unused warnings in `scoring.rs`

4 months agoPrivatise `CandidateRouteHop::short_channel_id` as its a footgun
Matt Corallo [Wed, 6 Dec 2023 01:13:33 +0000 (01:13 +0000)]
Privatise `CandidateRouteHop::short_channel_id` as its a footgun

Short channel "ID"s are not globally unique when they come from a
BOLT 11 route hint or a first hop (which can be an outbound SCID
alias). In those cases, its rather confusing that we have a
`short_channel_id` method which mixes them all together, and even
more confusing that we have a `CandidateHopId` which is not, in
fact returning a unique identifier.

In our routing logic this is mostly fine - the cost of a collision
isn't super high and we should still do just fine finding a route,
however the same can't be true for downstream users, as they may or
may not rely on the apparent guarantees.

Thus, here, we privatise the SCID and id accessors.

4 months agoFix and re-enable the `remembers_historical_failures` test
Matt Corallo [Wed, 6 Dec 2023 17:48:51 +0000 (17:48 +0000)]
Fix and re-enable the `remembers_historical_failures` test

f0ecc3ec73dcdb9303b1bd5ac687a361decce2dd introduced a regression in
the `remembers_historical_failures` test, and disabled it by simply
removing the `#[test]` annotation. This fixes the test and marks it
as a test again.

4 months agoRename `DirectedChannelInfo::outbound` to `from_node_one`
Matt Corallo [Wed, 6 Dec 2023 17:12:28 +0000 (17:12 +0000)]
Rename `DirectedChannelInfo::outbound` to `from_node_one`

...to give a bit more readability on accessing sites.

4 months agoRewrite docs in `CandidateRouteHop` to be somewhat more descriptive
Matt Corallo [Wed, 6 Dec 2023 01:22:07 +0000 (01:22 +0000)]
Rewrite docs in `CandidateRouteHop` to be somewhat more descriptive

4 months agoMerge pull request #2760 from TheBlueMatt/2023-11-chan-close-loop
Wilmer Paulino [Fri, 8 Dec 2023 18:16:12 +0000 (10:16 -0800)]
Merge pull request #2760 from TheBlueMatt/2023-11-chan-close-loop

Fix infinite loop when closing a pre-funding channel

4 months agoMerge pull request #2776 from jkczyz/2023-12-direct-connect-follow-ups
Matt Corallo [Fri, 8 Dec 2023 17:43:05 +0000 (17:43 +0000)]
Merge pull request #2776 from jkczyz/2023-12-direct-connect-follow-ups

Folllow-ups to #2723

4 months agoReturn correct SendSuccess in OnionMessenger
Jeffrey Czyz [Fri, 8 Dec 2023 04:44:58 +0000 (22:44 -0600)]
Return correct SendSuccess in OnionMessenger

When enqueuing a message for a node already awaiting a connection,
BufferedAwaitingConnection should be returned when a node is not yet
connected as a peer. However, it was only returned when the first
message was enqueued. Any messages enqueued after but before a
connection was established incorrectly returned Buffered.

4 months agoRename OnionMessagePath::addresses
Jeffrey Czyz [Fri, 8 Dec 2023 04:38:00 +0000 (22:38 -0600)]
Rename OnionMessagePath::addresses

The name itself doesn't provide much meaning to what the addresses
correspond to.

4 months agoFix create_onion_message return type documentation
Jeffrey Czyz [Fri, 8 Dec 2023 04:25:25 +0000 (22:25 -0600)]
Fix create_onion_message return type documentation

4 months agoImmediately error in `close_channel_internal` if there is no chan 2023-11-chan-close-loop
Matt Corallo [Wed, 29 Nov 2023 06:02:46 +0000 (06:02 +0000)]
Immediately error in `close_channel_internal` if there is no chan

Previously, unfunded channels would be stored outside of
`PeerState::channel_by_id`, and thus if there is no channel when
we look in `PeerState::channel_by_id`, `close_channel_internal`
called `force_close_channel_with_peer` to hunt for unfunded
channels.

However, that is no longer the case, so the call is redundant, and
we can simply return an error instead.

4 months agoMove pre-funded-channel immediate shutdown logic to the right place
Matt Corallo [Wed, 29 Nov 2023 18:11:30 +0000 (18:11 +0000)]
Move pre-funded-channel immediate shutdown logic to the right place

Because a `Funded` `Channel` cannot possibly be pre-funding, the
logic in `ChannelManager::close_channel_internal` to handle
pre-funding channels is in the wrong place.

Rather than being handled inside the `Funded` branch, it should be
in an `else` following it, handling either of the two
`ChannelPhases` outside of `Funded`.

Sadly, because of a previous control flow management `loop {}`, the
existing code will infinite loop, which is fixed here.

4 months agoDrop unreachable shutdown code in `Channel::get_shutdown`
Matt Corallo [Wed, 29 Nov 2023 05:58:52 +0000 (05:58 +0000)]
Drop unreachable shutdown code in `Channel::get_shutdown`

`Channel` is only a thing for funded channels. Thus, checking if a
channel has not yet been funded is dead code and can simply be
elided.

4 months agoLimit the scope of `get_funding_created_msg` to outbound channels
Matt Corallo [Thu, 30 Nov 2023 00:48:37 +0000 (00:48 +0000)]
Limit the scope of `get_funding_created_msg` to outbound channels

Since we no longer use `ChannelContext::get_funding_created_msg`,
it can be moved back into `UnfundedOutboundV1` channels only,
where it realistically belongs.

4 months agoMove to `Funded` after `funding_signed` rather than on funding
Matt Corallo [Thu, 30 Nov 2023 00:36:16 +0000 (00:36 +0000)]
Move to `Funded` after `funding_signed` rather than on funding

Previously, channels were stored in different maps in `PeerState`
based on whether the funding had been set, keeping the keys across
the maps consistent (pre-funding temporary_channel_ids vs
funding-outpoint-based channel_ids). However, channels are now
stored in a single `channel_by_id` map, making that point moot.

Instead, here, we convert the `ChannelPhase` state transition
boundary to "once we have a `ChannelMonitor`", which makes more
sense now, and was actually the original proposed boundary.

This also requires calling `signer_maybe_unblocked` on a pre-funded
outbound channel, but that nicely also lets us limit the scope of
`FundingCreated` message generation, which we do in the next
commit.

4 months agoMerge pull request #2723 from jkczyz/2023-11-direct-connect
Matt Corallo [Fri, 8 Dec 2023 01:39:13 +0000 (01:39 +0000)]
Merge pull request #2723 from jkczyz/2023-11-direct-connect

Direct connect for `OnionMessage` sending

4 months agoMerge pull request #2691 from wpaulino/refactor-channel-state
Matt Corallo [Thu, 7 Dec 2023 23:59:13 +0000 (23:59 +0000)]
Merge pull request #2691 from wpaulino/refactor-channel-state

Refactor ChannelState to decouple state flags from states

4 months agoRename certain flags to align with dual funding
Wilmer Paulino [Tue, 5 Dec 2023 23:38:47 +0000 (15:38 -0800)]
Rename certain flags to align with dual funding

`FundingCreated` and `FundingSent` were mostly named after the
respective `funding_created` and `funding_sent` wire messages. They
include the signature for the initial commitment transaction when
opening a channel. With dual funding, these messages are no longer used,
and instead we rely on the existing `commitment_signed` to exchange
those signatures.

4 months agoRename OnionMessageBuffer to OnionMessageRecipient
Jeffrey Czyz [Fri, 1 Dec 2023 19:22:43 +0000 (13:22 -0600)]
Rename OnionMessageBuffer to OnionMessageRecipient

4 months agoRemove superfluous space from where clause
Jeffrey Czyz [Thu, 30 Nov 2023 03:42:48 +0000 (21:42 -0600)]
Remove superfluous space from where clause

4 months agoTest pending connection onion message buffering
Jeffrey Czyz [Thu, 30 Nov 2023 03:30:15 +0000 (21:30 -0600)]
Test pending connection onion message buffering

Add tests for onion message buffering checking that messages are cleared
upon disconnection and timed out after MAX_TIMER_TICKS. Also, checks
that ConnectionNeeded events are generated.

4 months agoReuse MessengerNode in spec_test_vector
Jeffrey Czyz [Wed, 29 Nov 2023 23:36:50 +0000 (17:36 -0600)]
Reuse MessengerNode in spec_test_vector

Additional tests will be added needing a similar node struct, so
consolidate its usage.

4 months agoCall OnionMessageHandler::timer_tick_occurred
Jeffrey Czyz [Thu, 16 Nov 2023 16:21:12 +0000 (10:21 -0600)]
Call OnionMessageHandler::timer_tick_occurred

lightning-background-processor processes events provided by the
PeerManager's OnionMessageHandler for when a connection is needed. If a
connection is not established in a reasonable amount of time, drop any
buffered onion messages by calling timer_tick_occurred.

4 months agoProcess OnionMessageHandler events in background
Jeffrey Czyz [Thu, 16 Nov 2023 16:07:12 +0000 (10:07 -0600)]
Process OnionMessageHandler events in background

OnionMessageHandler implementations now also implement EventsProvider.
Update lightning-background-processor to also process any events the
PeerManager's OnionMessageHandler provides.

4 months agoRe-order define_run_body macro parameters
Jeffrey Czyz [Thu, 16 Nov 2023 15:03:18 +0000 (09:03 -0600)]
Re-order define_run_body macro parameters

Simply to avoid excessive wrapping when possible.

4 months agoRe-wrap define_run_body macro parameters
Jeffrey Czyz [Thu, 16 Nov 2023 14:59:40 +0000 (08:59 -0600)]
Re-wrap define_run_body macro parameters

Some code hygiene before another parameter is added and rustfmt is
eventually used.

4 months agoRemove unnecessary BackgroundProcessor type param
Jeffrey Czyz [Thu, 16 Nov 2023 14:49:05 +0000 (08:49 -0600)]
Remove unnecessary BackgroundProcessor type param

4 months agoMerge pull request #2765 from TheBlueMatt/2023-12-2314-cleanups-1
Matt Corallo [Wed, 6 Dec 2023 20:37:06 +0000 (20:37 +0000)]
Merge pull request #2765 from TheBlueMatt/2023-12-2314-cleanups-1

Post-#2314 Cleanups

4 months agoDrop buffered messages for timed out nodes
Jeffrey Czyz [Thu, 9 Nov 2023 21:58:24 +0000 (15:58 -0600)]
Drop buffered messages for timed out nodes

OnionMessenger buffers onion messages for nodes that are pending a
connection. To prevent DoS concerns, add a timer_tick_occurred method to
OnionMessageHandler so that buffered messages can be dropped. This will
be called in lightning-background-processor every 10 seconds.

4 months agoMake OnionMessageHandler extend EventsProvider
Jeffrey Czyz [Thu, 9 Nov 2023 17:13:01 +0000 (11:13 -0600)]
Make OnionMessageHandler extend EventsProvider

An OnionMessageHandler may buffer messages that can't be sent because
the recipient is not a peer. Have the trait extend EventsProvider so
that implementation so that an Event::ConnectionNeeded can be generated
for any nodes that fall into this category. Also, implement
EventsProvider for OnionMessenger and IgnoringMessageHandler.

4 months agoAdd Event::ConnectionNeeded for onion messages
Jeffrey Czyz [Thu, 9 Nov 2023 17:10:23 +0000 (11:10 -0600)]
Add Event::ConnectionNeeded for onion messages

A MessageRouter may be unable to find a complete path to an onion
message's destination. This could because no such path exists or any
needs on a potential path don't support onion messages. Add an event
that indicates a connection with a node is needed in order to send the
message.

4 months agoReturn socket addresses from DefaultMessageRouter
Jeffrey Czyz [Tue, 14 Nov 2023 23:08:26 +0000 (17:08 -0600)]
Return socket addresses from DefaultMessageRouter

When there isn't a direct connection with the Destination of an
OnionMessage, look up socket addresses from the NetworkGraph. This is
used to signal to OnionMessenger that a direct connection is needed to
send the message.

4 months agoAdd Option<Vec<SocketAddress>> to OnionMessagePath
Jeffrey Czyz [Tue, 14 Nov 2023 21:30:17 +0000 (15:30 -0600)]
Add Option<Vec<SocketAddress>> to OnionMessagePath

MessageRouter::find_path is given a Destination to reach via a set of
peers. If a path cannot be found, it may return a partial path such that
OnionMessenger can signal a direct connection to the first node in the
path is needed. Include a list of socket addresses in the returned
OnionMessagePath to allow OnionMessenger to know how to connect to the
node.

This allows DefaultMessageRouter to use its NetworkGraph to return
socket addresses for gossiped nodes.

4 months agoAdd NetworkGraph reference to DefaultMessageRouter
Jeffrey Czyz [Tue, 14 Nov 2023 21:05:05 +0000 (15:05 -0600)]
Add NetworkGraph reference to DefaultMessageRouter

When buffering onion messages for a node that is not connected as a
peer, it's possible that the node does not exist. Include a NetworkGraph
reference in DefaultMessageRouter so that it can be used to check if the
node actually exists. Otherwise, an malicious node may send an onion
message where the reply path's introduction node doesn't exist. This
would result in buffering messages that may never be delivered.

4 months agoBuffer onion messages requiring a connection
Jeffrey Czyz [Tue, 7 Nov 2023 14:17:46 +0000 (08:17 -0600)]
Buffer onion messages requiring a connection

MessageRouter::find_path returns a path to use when sending an onion
message. If the first node on the path is not connected or does not
support onion messages, sending will fail with InvalidFirstHop. Instead
of failing outright, buffer the message for later sending once the first
node is a connected peer.

4 months agoDestination in OnionMessenger::send_onion_message
Jeffrey Czyz [Wed, 15 Nov 2023 23:26:45 +0000 (17:26 -0600)]
Destination in OnionMessenger::send_onion_message

OnionMessenger::send_onion_message takes an OnionMessagePath. This isn't
very useful as it requires finding a path manually. Instead, have the
method take a Destination and use OnionMessenger's MessageRouter to
construct the path. Later, this will allow for buffering messages where
the first node in the path isn't a direct connection.

4 months agoMerge pull request #2762 from TheBlueMatt/2023-11-htlc-docs
Elias Rohrer [Wed, 6 Dec 2023 11:28:22 +0000 (12:28 +0100)]
Merge pull request #2762 from TheBlueMatt/2023-11-htlc-docs

Improve docs on newly-public structs after  #2700

4 months agoUse a message buffer abstraction in OnionMessenger
Jeffrey Czyz [Mon, 6 Nov 2023 22:53:07 +0000 (16:53 -0600)]
Use a message buffer abstraction in OnionMessenger

Onion messages are buffered for sending to the next node. Since the
network has limited adoption, connecting directly to a peer may be
necessary. Add an OnionMessageBuffer abstraction that can differentiate
between connected peers and those are pending a connection. This allows
for buffering messages before a connection is established and applying
different buffer policies for peers yet to be connected.

5 months agoMerge pull request #2551 from jbesraa/expose-CandidateRouteHop-to-channel_penalty_msat
Matt Corallo [Wed, 6 Dec 2023 01:20:27 +0000 (01:20 +0000)]
Merge pull request #2551 from jbesraa/expose-CandidateRouteHop-to-channel_penalty_msat

Add CandidateRouteHop to channel_penalty_msat inputs

5 months agoAllow users to accept skimmed fees in calling `peel_payment_onion` 2023-11-htlc-docs
Matt Corallo [Wed, 29 Nov 2023 23:25:04 +0000 (23:25 +0000)]
Allow users to accept skimmed fees in calling `peel_payment_onion`

LSP users who wish to use `peel_payment_onion` to understand if
they'd accept an HTLC prior to receit should be able to check the
skimmed fees just like they would for full payment receipt. Thus,
we need to expose the fee-skimming acceptance bool to
`peel_payment_onion`, which we do here, in addition to some doc
cleanups.

5 months agoFlesh out docs on `PendingHTLCInfo`
Matt Corallo [Wed, 29 Nov 2023 23:23:34 +0000 (23:23 +0000)]
Flesh out docs on `PendingHTLCInfo`

Now that `PendingHTLCInfo` is public, its docs should be meaningful
to developers not working directly on LDK, and thus needs
substantially more information than it previously had.

This adds much of that information.

5 months agoFlesh out docs on `PendingHTLCRouting`
Matt Corallo [Wed, 29 Nov 2023 23:11:02 +0000 (23:11 +0000)]
Flesh out docs on `PendingHTLCRouting`

Now that `PendingHTLCRouting` is public, its docs should be
meaningful to developers not working directly on LDK, and thus
needs substantially more information than it previously had.

This adds much of that information.