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.
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.
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.
is_paying_spendable_output
Add ChannelMonitor::broadcasted_local_revokable_script to detect
onchain local txn paying back to us.
Fix tests in consequence
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.
Fix tests in consequence
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.
Fix#411
Small fixes by Matt Corallo <git@bluematt.me>
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.
Failing this requirement at sending means a strict receiver would
fail our channel while processing a HTLC routed from a third-party.
Fix by enforcing check on both sender and receiver side.
This reintroduces a check_spends!() removed in 3d640da5c3
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!().
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.
Encapsulates tracking and bumping of in-flight transactions in
its own component. This component may be latter abstracted
to reuse tracking and RBF for new features (e.g dual-funding,
splicing)
Build all transactions generation in one place. Also as fees
and signatures are closely tied, what keys do you have determine
what bumping mode you can use.
This removes the somewhat-easy-to-misuse Clone from ChannelMonitors,
opening us up to being able to track Events in ChannelMonitors with
less risk of misuse.
Sadly it doesn't remove the Clone requirement for ChannelKeys,
though gets us much closer - we now just need to request a second
copy once when we go to create the ChannelMonitors.
This removes the ability to merge ChannelMonitors in favor of
explicit ChannelMonitorUpdates. It further removes
ChannelManager::test_restore_channel_monitor in favor of the new
ChannelManager::channel_monitor_updated method, which explicitly
confirms a set of updates instead of providing the latest copy of
each ChannelMonitor to the user.
This removes almost all need for Channels to have the latest
channel_monitor, except for broadcasting the latest local state.
Previously, if we have a live ChannelManager (that has seen blocks)
and we open a new Channel, if we serialize that ChannelManager
before a new block comes in, we'll fail to deserialize it. This is
the result of an overly-ambigious last_block_connected check which
would see 0s for the new channel but the previous block for the
ChannelManager as a whole.
We add a new test which catches this error as well as hopefully
getting some test coverage for other similar issues in the future.
Previously, if new ouputs were found to be watched as part
of channel operations, the block was rescan which triggers
again parser and generation of transactions already issued.
This commit first modifies the test framework without
altering further ChannelMonitor.
ChannelMonitor refactoring is introduced in a latter commit.
This, as it should be, restricts OnionHopData to only being able to
represent valid states, while still allowing for tests to generate
bogus hop data fields to test deserialization.
Previously OnionHopData contained a OnionRealm0HopData field however
instead of bumping the realm number, it has been replaced with a
length, used to indicte the length of a TLV-formatted object.
Because a TLV-formatted hop data can contain the same information as
a realm-0 hop data, we flatten the field and simply keep track of
what format it was in.
Additional changes:
* Update fuzz crate to match ChannelManager's new API
* Update lightning-net-tokio library to match ChannelManager's new ChannelMonitor Deref API
* Update tests to match ChannelManager's new ChannelMonitor Deref API
This exposes the latest Init-context features in the ChannelDetails
passed to the Router during route calculation, which combines those
with the Node-context features tracked from node_announcements to
provide the latest Node-context features in RouteHop structs.
Fields are also added for Channel-context features, though those are
only partially used since no such features are defined today anyway.
These will be useful when determining whether to use new
TLV-formatted onion hop datas when generating onions for peers.
Fix a crash where previously we weren't able to detect any accepted
HTLC if its witness-encoded cltv expiry was different from expected
ACCEPTED_HTLC_SCRIPT_WEIGHT. This should work for any cltv expiry
included between 0 and 16777216 on mainnet, testnet and regtest.
Accessing a struct through an std::syn::MutexGuard using implicit
dereferencing can confuse the borrow checker. This situation arises when
obtaining mutable references to more than one field of the struct, which
is normally allowed.
https://doc.rust-lang.org/nomicon/borrow-splitting.html
However, when using implicit dereferencing, a mutable reference to the
the entire struct is taken. Thus, attempting to access another field in
this manner will lead to a compilation error.
https://doc.rust-lang.org/error-index.html#E0499
A simple way to avoid this is to first obtain a mutable reference to the
struct using explicit dereferencing.
