This refactors ShutdownResult as follows:
- Makes ShutdownResult a struct instead of a tuple to represent
individual results that need to be handled. This recently also
includes funding batch closure propagations.
- Makes Channel solely responsible for constructing ShutdownResult as
it should own all channel-specific logic.
The check_closed_event function verified closure events against multiple
counterparty nodes, but only a single closure reason and channel
capacity. This commit introduces a check_closed_events function to
verify events against descriptions of each expected event, and refactors
check_closed_event in function of check_closed_events.
Add utility functions to ChannelManager for creating OfferBuilder,
and RefundBuilder such that derived keys are used for the signing
pubkey and payer id, respectively. This allows for stateless
verification of any InvoiceRequest and Invoice messages.
Later, blinded paths can be included in the returned builders.
Also tracks future payments using the given PaymentId such that the
corresponding Invoice is paid only once.
PendingOutboundPayment::AwaitingInvoice counts the number of timer ticks
that have passed awaiting a Bolt12Invoice for an InvoiceRequest. When a
constant INVOICE_REQUEST_TIMEOUT_TICKS has passed, the payment is
forgotten. However, this mechanism is insufficient for the Refund
scenario, where the Refund's expiration should be used instead.
Change AwaitingInvoice to store an absolute expiry instead. When
removing stale payments, pass the `SystemTime` in `std` and the highest
block time minus two hours in `no-std`.
When constructing onion messages to send initially (opposed to replying
to one from a handler), the user must construct an OnionMessagePath first
before calling OnionMessener::send_onion_message. Additionally, having a
reference to OnionMessener isn't always desirable. For instance, in an
upcoming commit, ChannelManager will implement OffersMessageHandler,
which OnionMessenger needs a reference to. If ChannelManager had a
reference to OnionMessenger, too, there would be a dependency cycle.
Instead, modify OffersMessageHandler and CustomOnionMessageHandler's
interfaces to include a method for releasing pending onion messages.
That way, ChannelManager may, for instance, construct and enqueue an
InvoiceRequest for sending without needing a reference to
OnionMessenger.
Additionally, OnionMessenger has responsibility for path finding just as
it does when replying to messages from a handler. It performs this when
extracting messages from the handlers before returning the next message
to send to a peer.
Rename CustomOnionMessageContents to OnionMessageContents and use it as
a trait bound on messages passed to OnionMessenger methods. This allows
using the trait in an upcoming commit as a bound on the contents of
PendingOnionMessage.
Also, make ParsedOnionMessageContent implement OnionMessageContents so
that Payload can be bounded by OnionMessageContents directly, but used
when either reading a ParsedOnionMessageContent or writing a specific
type of OnionMessageContents (e.g., OffersMessage).
OnionMessageProvider is a super-trait of OnionMessageHandler, but they
don't need to be used separately. Additionally, the former is misplaced
in the events module. Remove OnionMessageProvider and add it's only
method, next_onion_message_for_peer, into OnionMessageHandler.
Previously, outbound messages held in `process_events` could race
with peer disconnection, allowing a message intended for a peer
before disconnection to be sent to the same peer after
disconnection.
The fix is simple - hold the peers read lock while we fetch
pending messages from peers (as we disconnect with the write lock).
Not doing so caused a lock order inversion between the locks
`ChannelManager::best_block` and `ChannelManager::short_to_chan_info`
after the addition of `test_trigger_lnd_force_close`.
It turns out that we were holding the `short_to_chan_info` for longer
than needed when processing HTLC forwards. We only need to acquire it to
quickly obtain channel info, and there aren't any other locks within
`forwarding_channel_not_found` that depend on it being held.
We do this to ensure that the counterparty will always broadcast their
latest state when we broadcast ours. Usually, they'll do this with the
`error` message alone, but if they don't receive it or ignore it, then
we'll force them to broadcast by sending them a bogus
`channel_reestablish` upon reconnecting. Note that this doesn't apply to
unfunded channels as there is no commitment transaction to broadcast.
Unfortunately, lnd doesn't force close on errors
(abb1e3463f/htlcswitch/link.go (L2119)).
One of the few ways to get an lnd counterparty to force close is by
replicating what they do when restoring static channel backups (SCBs).
They send an invalid `ChannelReestablish` with `0` commitment numbers
and an invalid `your_last_per_commitment_secret`.
Since we received a `ChannelReestablish` for a channel that doesn't
exist, we can assume it's likely the channel closed from our point of
view, but it remains open on the counterparty's side. By sending this
bogus `ChannelReestablish` message now as a response to theirs, we
trigger them to force close broadcasting their latest state. If the
closing transaction from our point of view remains unconfirmed, it'll
enter a race with the counterparty's to-be-broadcast latest commitment
transaction.
ChainHash is more appropriate for places where an arbitrary BlockHash is
not desirable. This type was introduced in later versions of the bitcoin
crate, thus BlockHash was used instead.
Using ChainHash also makes it easier to check if ChannelManager is
compatible with an Offer.
As noted in BOLT 4, we should be using the update_add_htlc's cltv_expiry,
not the CLTV expiry set by the sender in the onion for this comparison.
See here: 4dcc377209/04-onion-routing.md (L334)
The trait itself has no purpose for bindings, as all structs are
concretized anyway. Further, the bindings have specific handling
for generic bounding traits like this.
The reason for having a separate `parse_onion_address` from
`FromStr` is to have an onion parsing function in `no-std`, but
when we added it we forgot to make it public. We do this here, as
well as fix a few compilation warnings in `no-std`.
`UtxoLookup` doesn't strictly need to be referenced from the
`PeerManager`, and in fact the new `GossipVerifier` in
`lightning-block-sync` requires itself to be owned by the
`PeerManager` (for circular type reasons).
This allows us to use `lightning-block-sync`'s `GossipVerifier`
with `SimpleArcPeerManager` in ldk-sample.
