In an upcoming commit, we will eliminate various invalid state
combinations between `Hop` and `InboundOnionPayload` enums. To do so,
rather than nesting one within the other, we will instead have them
both referring to the same structs, with variant-dependent supplemental
data. This requires pulling each variant's data into its own type.
d4bd56fc41 changed the logic for calling
unset_funding_info such that it may be called on a channel that was
already in ChannelPhase::Funded when handling funding_signed. This
caused a debug panic in the full_stack fuzz test when calling
FundedChannel::unset_funding_info. Fix this by only calling
unset_funding_info on watch_channel error, as was previously the case.
This also reverts moving the channel back into
ChannelPhase::UnfundedOutboundV1, which should be fine since the channel
is about to be removed.
Forwarding Trampoline packets requires storing their shared secrets on
top of the outer onion's shared secrets, as well as referencing the
next hop by its node ID as opposed to by an SCID. We modify
PendingHTLCRouting to adequately represent this information.
`LightningError` is an error type for returning errors back to the
`PeerHandler` when handling P2P messages. However, it used to be
more broadly used, in a way that never made any sense.
Here we remove on vestige of this, using a `&'static str` for
router errors rather than `LightningError` with a constant
`action`.
When we see a channel come into the router as a route-hint, but its
for a direct channel of ours, we'd like to ignore the route-hint as
we have more information in the first-hop channel info. We do this
by matching SCIDs, but only considered outbound SCID aliases.
Here we change to consider both outbound SCID aliases and the full
channel SCID, which some nodes may use in their invoices.
The router is a somewhat complicated beast, and though the last few
commits removed some code from it, a complicated beast it remains.
Thus, having `expect`s in it is somewhat risky, so we take this
opportunity to replace some of them with `debug_assert!(false)`s
and an `Err`-return.
When we process a path backwards from a node during pathfinding, we
implicitly commit to the path up to that node. Any changes to the
preferred path up to that node will make the newly processed path's
state invalid.
In the previous few commits we fixed cases for this in last-hop
paths (both blinded and unblinded).
Here we add assertions to enforce this, tracked in a new bool in
`PathBuildingHop`.
When we do pathfinding with blinded paths, we start each
pathfinding iteration by inserting all the blinded paths into our
nodes map as last-hops to the destination. As we do that, we check
if any of the introduction points happen to be nodes we have direct
chanels with, as we want to use the local info for such channels
and support finding a path even if that channel is not publicly
announced.
However, as we iterate the blinded paths, we may find a second
blinded path from the same introduction point which we prefer over
the first. If this happens, we would already have added info from
us over the local channel to that intro point and end up with
calculations for the first hop to a blinded path that we no longer
prefer.
This is ultimately fixed here in two ways:
(a) we process the first-hop channels to blinded path introduction
points in a separate loop after we've processed all blinded
paths, ensuring we only ever consider a channel to the blinded
path we will ultimately prefer.
(b) In the next commit, we add we add a new tracking bool in
`PathBuildingHop` called `best_path_from_hop_selected` which we
set when we process a channel backwards from a node, indicating
that we've committed to the best path to the node and check when
we add a new path to a node. This would have resulted in a much
earlier debug-assertion in fuzzing or several tests.
When we handle the unblinded last-hop route hints from an invoice,
we had a good bit of code dedicated to handling fee propagation
through the (potentially) multiple last-hops and connecting them to
potentially directly-connected first-hops.
This was a good bit of code that was almost never used, and it
turns out was also buggy - we could process a route hint with
multiple hops, committing to one path through nodes A, B, to C,
then process another route hint (or public channel) which changes
our best path from B to C, making the A entry invalid.
Here we remove the whole maze, utilizing the normal hop-processing
logic in `add_entries_to_cheapest_to_target_node` for last-hops as
well. It requires tracking which nodes connect to last-hop hints
similar to the way we do with `is_first_hop_target` in
`PathBuildingHop`, storing the `CandidateRouteHop`s in a new map,
and always calling `add_entries_to_cheapest_to_target_node` on the
payee node, whether its public or not.
This likely only impacts very rare edge cases, but if we have two
equal-cost paths, we should likely prefer ones which contribute
more value (avoiding cases where we use paths which are
amount-limited but equal fee to higher-amount paths) and then paths
with fewer hops (which may complete faster).
It does make test behavior more robust against router changes,
which comes in handy over the coming commits.
This method does not take into the intended funding amount, and it's
not currently used, therefore it's removed now. Its fee estimation
part is kept (estimate_v2_funding_transaction_fee).
This commit expands on the previously introduced merge method by
offering a way to simply replace the local scores by the liquidity
information that is obtained from an external source.
In a subsequent commit, we will be storing `TrampolineOnionPacket`s
within `PendingHTLCRouting`, requiring that they be serialized for
storage. To do so, `RequiredWrapper`'s requirements must be loosened to
only require `LengthReadable` instead of `Readable`.
In the previous commit we completed support for async receive from an
always-online sender to an often-offline receiver, minus support for acting as
the async receiver's always-online channel counterparty.
Previously, this test util did not account for config overrides supplied at
node creation time. Uncovered because it caused test nodes to be unable to
forward over private channels created with this util because that is not the
default.
