Currently the channel shutdown sequence has a number of steps which
all the shutdown callsites have to call. Because many shutdown
cases are rare error cases, its relatively easy to miss a call and
leave users without `Event`s or miss some important cleanup.
One of those steps, calling `issue_channel_close_events`, is rather
easy to remove, as it only generates two events, which can simply
be moved to another shutdown step.
Here we remove `issue_channel_close_events` by moving
`ChannelClosed` event generation into `finish_force_close_channel`.
Currently the channel shutdown sequence has a number of steps which
all the shutdown callsites have to call. Because many shutdown
cases are rare error cases, its relatively easy to miss a call and
leave users without `Event`s or miss some important cleanup.
One of those steps, calling `issue_channel_close_events`, is rather
easy to remove, as it only generates two events, which can simply
be moved to another shutdown step.
Here we move the first of the two events, `DiscardFunding`, into
`finish_force_close_channel`.
If we promote our channel to `AwaitingChannelReady` after adding
funding info, but still have `MONITOR_UPDATE_IN_PROGRESS` set, we
haven't broadcasted the funding transaction yet and thus should
return values from `unbroadcasted_funding[_txid]` and generate a
`DiscardFunding` event.
If a peer provides a feerate which nears `u32::MAX`, we may
overflow calculating the dust buffer feerate, leading to spuriously
keeping non-anchor channels open when they should be force-closed.
If we try to open a channel with a peer that is disconnected (but
with which we have some other channels), we'll end up with an
unfunded channel which will lead to a panic when the peer
reconnects. Here we drop this debug assertion without bother to add
a new test, given this behavior will change in a PR very soon.
When contest delays are >= 0x8000, script pushes require an extra
byte to avoid being interpreted as a negative int. Thus, for
channels with CSV delays longer than ~7.5 months we may generate
transactions with slightly too little fee. This isn't really a huge
deal, but we should prefer to be conservative here, and slightly
too high fee in the general case is better than slightly too little
fee in other cases.
When we or our counterparty are updating the fees on the channel,
we currently check that the resulting balance is sufficient not
only to meet the reserve threshold, but also not push it below
dust. This isn't required in the BOLTs and may lead to spurious
force-closures (which would be a bit safer, but reserve should
always exceed the dust threshold).
Worse, the current logic is broken - it compares the output value
in *billionths of satoshis* to the dust limit in satoshis. Thus,
the code is borderline dead anyway, but can overflow for channels
with several million Bitcoin, causing the fuzzer to get mad (and
lead to spurious force-closures for few-billion-dollar channels).
If we get a `Feature` object which has excess zero bytes, we
shouldn't consider it a different `Feature` from another with the
same bits set, but no excess zero bytes. Here we fix both the
`Hash` and `PartialEq` implementation for `Features` to ignore
excess zero bytes.
62f8669654 added two
`htlc_maximum_msat.unwrap_or`s, but used a default value of 0,
spuriously causing all HTLCs to fail if we don't have an htlc
maximum value. This should be mostly harmless, but we should fix it
anyway.
In 4b5db8c3ce, `channelmanager::PendingHTLCRouting` was made
public, exposing a `FinalOnionHopData` field to the world. However,
`FinalOnionHopData` was left crate-private, making the enum
impossible to construct.
There isn't a strong reason for this (even though the
`FinalOnionHopData` API is somewhat confusing, being separated from
the rest of the onion structs), so we expose it here.
Since `lightning-invoice` now depends on the `bitcoin` crate
directly, also depending on the `bitcoin_hashes` crate is redundant
and just means we confuse users by setting the `std` flag only on
`bitcoin`. Thus, we drop the explicit dependency here and replace
it with `bitcoin::hashes`.
When we make the `PrivateRoute` inner `RouteHint` `pub`, we failed
to note that the `PrivateRoute::new` constructor actually verifies
a length invariant. Thus, we un-export the inner field and force
users to go back through the `new` fn.
To avoid exposing a node's identity in a blinded path, only create
one-hop blinded paths if the node has been announced, and thus has
public channels. Otherwise, there is no way to route a payment to the
node, exposing its identity needlessly.
When constructing blinded payment paths for Bolt12Invoice, delegate to
Router::create_blinded_payment_paths which may produce multi-hop blinded
paths. Fallback to one-hop blinded paths if the Router fails or returns
no paths.
The Router trait is used to find a Route for paying a node. Expand the
interface with a create_blinded_payment paths method for creating such
paths to a recipient node.
Provide an implementation for DefaultRouter that creates two-hop
blinded paths where the recipient's peers serve as the introduction
nodes.
When constructing blinded paths for Offer and Refund, delegate to
MessageRouter::create_blinded_paths which may produce multi-hop blinded
paths. Fallback to one-hop blinded paths if the MessageRouter fails or
returns no paths.
Likewise, do the same for InvoiceRequest and Bolt12Invoice reply paths.
When finding a route through a blinded path, a random CLTV offset may be
added to the path in order to preserve privacy. This needs to be
accounted for in the blinded path's PaymentConstraints. Add
CLTV_FAR_FAR_AWAY to the max_cltv_expiry constraint to allow for such
offsets.
When we fail to accept a counterparty's funding for various
reasons, we should ensure we call the correct cleanup methods in
`internal_funding_created` to remove the temporary data for the
channel in our various internal structs (primarily the SCID alias
map).
This adds the missing cleanup, using `convert_chan_phase_err`
consistently in all the error paths.
This also ensures we get a `ChannelClosed` event when relevant.
ChannelManager is parameterized by a Router in order to find routes when
sending and retrying payments. For the offers flow, it needs to be able
to construct blinded paths (e.g., in the offer and in reply paths).
Instead of adding yet another parameter to ChannelManager, require that
any Router also implements MessageRouter. Implement this for
DefaultRouter by delegating to a DefaultMessageRouter.
The MessageRouter trait is used to find an OnionMessagePath to a
Destination (e.g., to a BlindedPath). Expand the interface with a
create_blinded_paths method for creating such paths to a recipient.
Provide a default implementation creating two-hop blinded paths where
the recipient's peers serve as introduction nodes.
The RouteBlinding feature flag is signals support for relaying payments
over blinded paths. It is used for paying BOLT 12 invoices, which are
required to included at least one blinded path.
