Matt Corallo [Tue, 7 Jan 2020 01:29:33 +0000 (20:29 -0500)]
Implement multipath sends using payment_secret.
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.
Matt Corallo [Mon, 30 Mar 2020 20:24:19 +0000 (16:24 -0400)]
Refactor payment-claim logic to ensure MPP-claim atomicity
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.
Matt Corallo [Thu, 2 Jan 2020 06:23:48 +0000 (01:23 -0500)]
Impl Base AMP in the receive pipeline and expose payment_secret
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.
Jeffrey Czyz [Fri, 27 Mar 2020 23:46:57 +0000 (16:46 -0700)]
Test failing backward any pending HTLCs
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.
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>
Antoine Riard [Fri, 20 Mar 2020 20:58:13 +0000 (16:58 -0400)]
Remove useless local commitment txn signatures
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.
Antoine Riard [Wed, 18 Mar 2020 04:29:26 +0000 (00:29 -0400)]
Add ChanSigner in OnchainTxHandler
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.
Antoine Riard [Wed, 18 Mar 2020 05:15:28 +0000 (01:15 -0400)]
Remove Watchtower mode from Storage enum and make it a struct
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.
Matt Corallo [Thu, 19 Mar 2020 23:15:06 +0000 (19:15 -0400)]
Fail to deserialize ChannelManager if it is ahead of any monitor(s)
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.
Antoine Riard [Thu, 19 Mar 2020 00:58:05 +0000 (20:58 -0400)]
Implement reorg-safety for SpendableOutputDescriptor detection
We delay SpendableOutputDescriptor until reaching ANTI_REORG_DELAY
to avoid misleading user wallet in case of reorg and alternative
settlement on a channel output.
Previously, we would generate SpendableOutputDescriptor::StaticOutput
in OnchainTxHandler even if our claiming transaction wouldn't confirm
onchain, misbehaving user wallet to think it receives more funds than
in reality.
Matt Corallo [Thu, 19 Mar 2020 01:37:09 +0000 (21:37 -0400)]
Fetch latest commitment txn via monitor, not channel in test
Eventually, we want to remove the Channel's copy of its own
ChannelMonitor, reducing memory footprint and complexity of
ChannelManager greatly.
This removes the last uses of said ChannelMonitor for latest
local commitment transactions (though it is still used for
would_broadcast_at_height(), which is the last remaining use).
If we call get_update_fulfill_htlc (in this case via
ChannelManager::claim_funds_internal ->
Channel::get_update_fulfill_htlc_and_commit) and it finds that we
already have a holding-cell pending HTLC claim, it will return no
monitor update but leave latest_monitor_update_id incremented.
If we later go and add a new monitor update we'll panic as the
updates appear to have been applied out-of-order.
Antoine Riard [Wed, 26 Feb 2020 23:18:27 +0000 (18:18 -0500)]
Watch outputs of revoked HTLC-transactions
Bumping of justice txn on revoked HTLC-Success/HTLC-timeout is triggered
until our claim is confirmed onchain with at least
ANTI_REORG_DELAY_SAFE. Before this patch, we weren't tracking them in
check_spend_remote_htlc, leading us to infinite bumps.
Antoine Riard [Wed, 11 Mar 2020 19:10:29 +0000 (15:10 -0400)]
Fix duplicata of adjusted justice tx generation in OnchainTxHandler
Adjusted tx occurs when a previous aggregated claim tx has
seen one of its outpoint being partially claimed by a remote tx.
To pursue claiming of the remaining outpoint a adjusted claim tx
is generated with leftover of claimable outpoints.
Previously, in case of block-rescan where a partial claim occurs,
we would generate duplicated adjusted tx, wrongly inflating feerate
for next bumps. At rescan, if input has already been dropped from
outpoints map from a claiming request, don't regenerate again
a adjuste tx.
3d640da5c343111f538f006996c13c9a98e0d9e6 changed the indexes for
some enums in ChannelMonitor deserialization but not serialization.
Thus, the chanmon_deser_target fuzz target failed on travis on at
least one PR.
Antoine Riard [Tue, 10 Mar 2020 17:03:10 +0000 (13:03 -0400)]
Make htlc_minimum_msat configurable
Enforce a minimum htlc_minimum_msat of 1.
Instead of computing dynamically htlc_minimum_msat based on feerate,
relies on user-provided configuration value. This let user compute
an economical-driven channel parameter according to network dynamics.
Matt Corallo [Mon, 24 Feb 2020 19:17:04 +0000 (14:17 -0500)]
Fix long-standing race in net-tokio reading after a disconnect event
If rust-lightning tells us to disconnect a socket after we read
some bytes from the socket, but before we actually give those bytes
to rust-lightning, we may end up calling rust-lightning with a
Descriptor that isn't registered anymore.
Sadly, there really isn't a good way to solve this, and it should
be a pretty quick event, so we just busy-wait.
Matt Corallo [Sat, 15 Feb 2020 03:32:30 +0000 (22:32 -0500)]
Update pre-HTLC DataLossProtect to match new spec changes
This was the way DataLossProtect was originally written, however it
didn't match other implementations at the time during testing. It
turns out, other implementations didn't agree with each other
anyway (depending on the exact timeline), so the spec was clarified
somewhat in https://github.com/lightningnetwork/lightning-rfc/pull/550
. This updates us to be in line with the new guidance and appears
to solve out-of-sync issues in testing.
Matt Corallo [Thu, 5 Mar 2020 23:01:06 +0000 (18:01 -0500)]
Use block timestamps as the min for generated update messages.
Fixes issue #493 and should resolve some issues where other nodes
(incorrectly) reject channel_update/node_announcement messages
which have a serial number that is not a relatively recent
timestamp.
Matt Corallo [Fri, 3 Jan 2020 01:32:37 +0000 (20:32 -0500)]
Add ability to broadcast our own node_announcement.
This is a somewhat-obvious oversight in the capabilities of
rust-lightning, though not a particularly interesting one until we
start relying on node_features (eg for variable-length-onions and
Base AMP).
Sadly its not fully automated as we don't really want to store the
list of available addresses from the user. However, with a simple
call to ChannelManager::broadcast_node_announcement and a sensible
peer_handler, the announcement is made.
Matt Corallo [Wed, 4 Mar 2020 22:45:27 +0000 (17:45 -0500)]
Take multiple spent-txn to check_spends! in functional_tests
This reintroduces a check_spends!() removed in 3d640da5c343111f538f
due to check_spends not being able to check a transaction which
spends multiple other transactions.
It also simplifies a few calls in claim_htlc_outputs_single_tx by
using check_spends!().
Matt Corallo [Wed, 4 Mar 2020 22:36:12 +0000 (17:36 -0500)]
Drop redundant .clone() in check_spends calls.
The API to rust-bitcoin to check a transaction correctly spends
another changed some time ago, but we still have a lot of needless
.clone()s in our tests.
Matt Corallo [Wed, 4 Mar 2020 22:27:03 +0000 (17:27 -0500)]
Flatten Vec passed from channelmonitor to onchaintx block_connected
Instead of passing a Vec of Vecs drop them into one as we go in
ChannelMonitor, hopefully avoiding a bit of memory fragmentation
and improving readability.