Includes simple changes to test util signers and tests, as well as
handling the error case for get_per_commitment_point in
HolderCommitmentPoint. This leaves a couple `.expect`s in places
that will be addressed in a separate PR for handling funding.
99aa6e27f6 detected that we had an
undefined feature in `lightning-invoice` called `strict`, which was
used to turn on `deny(warnings)`. It resolved that by adding the
feature to the `Cargo.toml`, but we actually don't need it - our CI
already builds with `-Dwarnings`, so any warnings should be
rejected during CI and there's not a lot of value in having a
(public) feature to do the same.
We previously upstreamed the `validate_merkle_proof` utility method,
which shipped with `electrum-client` 0.19.0.
Since we upgraded to that version recently, we can now drop our local
code and use the upstreamed version.
This uses the newly introduced conditional configuration checks that are
now configurable withint Cargo (beta).
This allows us to get rid of our custom python script that checks for
expected features and cfgs.
This does introduce a warning regarding the unknown lint in Cargo
versions prior to the current beta, but since these are not rustc errors,
they won't break any builds with the "-D warnings" RUSTFLAG.
Moving to this lint actually exposed the "strict" feature not being
present in the lightning-invoice crate, as our python script didnt
correctly parse the cfg_attr where it appeared.
This will be used later in send_payment_for_bolt12_invoice instead of
find_route_and_send_payment as it will allow for returning
RetryableSendFailure when path finding fails.
Instead of returning Ok when path finding fails, allow returning a
RetryableSendFailure from send_payment_for_bolt12_invoice. Follow up
commits will return such failures.
- Previously, the `create_blinded_path` function was limited to
returning a single `BlindedPath`, which restricted the usage of
`blinded_paths`.
- This commit extends the `create_blinded_path` function to return
the entire blinded path vector generated by the `MessageRouter`'s
`create_blinded_paths`.
- The updated functionality is integrated across the codebase, enabling
the sending of Offers Response messages, such as `InvoiceRequest`
(in `pay_for_offer`) and `Invoice` (in `request_refund_payment`),
utilizing multiple reply paths.
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.
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.
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.
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.
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.
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.
This marginally reduces the size of `get_route` by moving a the
blinded path introduction point resolution and blinded path checks
into a helper method.
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.
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.
The router's `introduction_node_id_cache` is used to cache the
`NodeId`s of blinded path introduction points so that we don't
have to look them up every time we go around the main router loop.
When using it, if the introduction point isn't a public node we
then look up the introduction in our first-hops map.
In either case, we have to end up with a reference to a `NodeId`
that outlives our `dist` map.
Here we consolidate both the initial cache building and the
first-hops map lookup to one place, storing only a reference to a
`NodeId` either in the `NetworkGraph` or in the new `NodeCounters`
to get the required lifetime without needing to reference into the
first-hops map.
We then take this opportunity to avoid `clone`ing the first-hops
map entries as we now no longer reference into it.
With the new `NodeCounters` have have a all the `NodeId`s we'll
need during routing, so there's no need to keep the
`private_hop_key_cache` which existed to provide references to
`NodeId`s which are needed during routing.
In the next commit we'll stop using `NodeId`s to look up nodes when
routing, instead using the new per-node counters. Here we take the
first step, adding a local struct which tracks temporary counters
for route hints/source/destination nodes.
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.
