ChannelManager::send_payment stopped utilizing its ownership of the
Route with MPP (which, for readability, now clone()s the individual
paths when creating HTLCSource::OutboundRoute objects). While this
isn't ideal, it likely also makes sense to ensure that the user has
access to the Route after sending to correlate individual path
failures with the paths in the route or, in the future, retry
individual paths.
Thus, the easiest solution is to just take the Route by reference,
allowing the user to retain ownership.
This rather dramatically changes the return type of send_payment
making it much clearer when resending is safe and allowing us to
return a list of Results since different paths may have different
return values.
Previously if we claimed an MPP where a previous-hop channel was
closed while we were waitng for the user to provide us the preimage
we'd simply skip claiming that HTLC without letting the user know.
This refactors the claim logic to first check that all the channels
are still available (which is actually all we need - we really
mostly care about updating the channel monitors, not the channels
themselves) and then claim the HTLCs in the same lock, ensuring
atomicity.
Base AMP is centered around the concept of a 'payment_secret` - an
opaque 32-byte random string which is used to authenticate the
sender to the recipient as well as tie the various HTLCs which
make up one payment together. This new field gets exposed in a
number of places, though sadly only as an Option for backwards
compatibility when sending to a receiver/receiving from a sender
which does not support Base AMP.
Sadly a huge diff here, but almost all of it is changing the method
signatures for sending/receiving/failing HTLCs and the
PaymentReceived event, which all now need to expose an
Option<[u8; 32]> for the payment_secret.
It doesn't yet properly fail back pending HTLCs when the full AMP
payment is never received (which should result in accidental
channel force-closures). Further, as sending AMP payments is not
yet supported, the only test here is a simple single-path payment
with a payment_secret in it.
Upon channel failure, any pending HTLCs in a channel's holding cell must
be failed backward. The added test exercises this behavior and
demonstrates a deadlock triggered within the handle_error!() macro. The
deadlock occurs when the channel_state lock is already held and then
reacquired when finish_force_close_channel() is called.
This partially reverts 933ae34703,
though note that 933ae34703 fixed a
similar deadlock while introducing this one.
If we have HTLCs to fail backwards, handle_error!() will call
finish_force_close_channel() which will attempt to lock channel_state
while it is locked at the original caller. Instead, hold the lock for
shorter scopes such that it is not held upon entering handle_error!().
Co-authored-by: Matt Corallo <git@bluematt.me>
Co-authored-by: Jeffrey Czyz <jkczyz@gmail.com>
check_spend_local_transaction is tasked with detection of
onchain local commitment transaction and generate HTLC transaction.
Signing an already onchain tx isn't necessary.
Rename ChannelMonitor::Storage to OnchainDetection,
holder of channel state (base_key+per_commitment_point)
to detect onchain transactions accordingly.
Going further between splitting detection and transaction
generation, we endow OnchainTxHandler with keys access.
That way, in latter commits, we may remove secret keys entirely
from ChannelMonitor.
Watchtower will be supported through external signer interface
where a watchtower implementation may differ from a local one
by the scope of key access and pre-signed datas.
If any monitors are out of sync with the Channel, we previously
closed the channel, but we should really only do that if the
monitor is ahead of the channel, opting to call the whole thing
invalid if the channel is ahead of the monitor.
We delay SpendableOutputDescriptor until reaching ANTI_REORG_DELAY
to avoid misleading user wallet in case of reorg and alternative
settlement on a channel output.
Fix tests in consequence.
