MessageSendEvent::PaymentFailureNetworkUpdate served as a hack to pass
an HTLCFailChannelUpdate from ChannelManager to NetGraphMsgHandler via
PeerManager. Instead, remove the event entirely and move the contained
data (renamed NetworkUpdate) to Event::PaymentFailed to be processed by
an event handler.
CounterpartyForwardingInfo is public (previously exposed with a
`pub use`), and used inside of ChannelCounterparty in
channelmanager.rs. However, it is defined in channel.rs, away from
where it is used.
This would be fine, except that the bindings generator is somewhat
confused by this - it doesn't currently support interpreting
`pub use` as a struct to expose, instead ignoring it.
Fixes https://github.com/lightningdevkit/ldk-garbagecollected/issues/44
Now that NetworkGraph uses interior mutability, the RwLock used around
it in NetGraphMsgHandler is no longer needed. This allows for shared
ownership without a lock.
This adds the new range-based closing_signed negotiation specified
in https://github.com/lightningnetwork/lightning-rfc/pull/847 as
well as cleans up the existing closing_signed negotiation to unify
the new codepaths and the old ones.
Note that because the new range-based closing_signed negotiation
allows the channel fundee to ultimately select the fee out of a
range specified by the funder, which we, of course, always select
the highest allowed amount from. Thus, we've added an extra round
of closing_signed in the common case as we will not simply accept
the first fee we see, always preferring to make the funder pay as
much as they're willing to.
When we added the support for external signing, many of the
signing functions were allowed to return an error, closing the
channel in such a case. `sign_closing_transaction` is one such
function which can now return an error, except instead of handling
it properly we'd simply never send a `closing_signed` message,
hanging the channel until users intervene and force-close it.
Piping the channel-closing error back through the various callsites
(several of which already have pending results by the time they
call `maybe_propose_first_closing_signed`) may be rather
complicated, so instead we simply attempt to propose the initial
`closing_signed` in `get_and_clear_pending_msg_events` like we do
for holding-cell freeing.
Further, since we now (possibly) generate a `ChannelMonitorUpdate`
on `shutdown`, we may need to wait for monitor updating to complete
before we can send a `closing_signed`, meaning we need to handle
the send asynchronously anyway.
This simplifies a few function interfaces and has no impact on
behavior, aside from a few message-ordering edge-cases, as seen in
the two small test changes required.
This makes it much simpler to deal with `MaybeReadable` types in
`Vec`s in TLVs as we can transparently deal with them as `vec`,
with the wrapper doing the Right Thing.
This requires we implement `MaybeReadable` for all `Readable` which
has some downstream implications, but nothing too bad.
We don't actually yet support `warning` messages as there are
issues left to resolve in the spec PR, but there's nothing to stop
us adding an internal enum variant for sending a warning message
before we actually support doing so.
Previously we'd been expecting to implement anchor outputs before
shipping 0.1, thus reworking our channel fee update process
entirely and leaving it as a future task. However, due to the
difficulty of working with on-chain anchor pools, we are now likely
to ship 0.1 without requiring anchor outputs.
In either case, there isn't a lot of reason to require that users
call an explicit "prevailing feerates have changed" function now
that we have a timer method which is called regularly. Further, we
really should be the ones deciding on the channel feerate in terms
of the users' FeeEstimator, instead of requiring users implement a
second fee-providing interface by calling an update_fee method.
Finally, there is no reason for an update_fee method to be
channel-specific, as we should be updating all (outbound) channel
fees at once.
Thus, we move the update_fee handling to the background, calling it
on the regular 1-minute timer. We also update the regular 1-minute
timer to fire on startup as well as every minute to ensure we get
fee updates even on mobile clients that are rarely, if ever, open
for more than one minute.
When handling shutdown messages, Channel cannot move to
ChannelState::ShutdownComplete. Remove the code in ChannelManager that
adds a MessageSendEvent::BroadcastChannelUpdate in this case since it is
unreachable.
When a shutdown script is omitted from open_channel or accept_channel,
it must be provided when sending shutdown. Generate the shutdown script
at channel closing time in this case rather at channel opening.
This requires producing a ChannelMonitorUpdate with the shutdown script
since it is no longer known at ChannelMonitor creation.
This is one of the riskiest parts of our API from the perspective
of accidental force-closes - if users delay persisting the
ChannelManager much at all after a ChannelMonitor we may hit a
force-close after restart.
The fact that we don't log at all when this happens is criminal.
While we should never reach `ClaimFundsFromHop::DuplicateClaim` in
most cases, if we do, it likely indicates the HTLC was timed out
some time ago and is no longer available to be claimed. Thus, it
does not make sense to imply that we `claimed_any_htlcs`.
It is useful for accounting and informational reasons for users to
be informed when a payment has been successfully forwarded. Thus,
when an HTLC which represents a forwarded leg is claimed, we
generate a new `PaymentForwarded` event.
This requires some additional plumbing to return HTLC values from
`OnchainEvent`s. Further, when we have to go on-chain to claim the
inbound side of the payment, we do not inform the user of the fee
reward, as we cannot calculate it until we see what is confirmed
on-chain.
Substantial code structure rewrites by:
Valentine Wallace <vwallace@protonmail.com>
Previously, we could fail to generate a new commitment transaction
but it simply indicated we had gone to doule-claim an HTLC. Now
that double-claims are returned instead as Ok(None), we should
handle the error case and fail the channel, as the only way to hit
the error case is if key derivation failed or the user refused to
sign the new commitment transaction.
This also resolves an issue where we wouldn't inform our
ChannelMonitor of the new payment preimage in case we failed to
fetch a signature for the new commitment transaction.
Private nodes should never wish to forward HTLCs at all, which we
support here by disabling forwards out over private channels by
default. As private nodes should not have any public channels, this
suffices, without allowing users to disable forwarding over
channels announced in the routing graph already.
Closes#969
