After we moved the ChannelMonitor creation later during Channel
init, we never went back and cleaned up ChannelMonitor to remove
a number of now-useless Option<>s, so we do that now.
This test tries the new lockdown logic in case of a signed-and-broadcast
local commitment transaction while a concurrent ChannelMonitorUpdate for
a next _local_ commitment is submitted from offchain. Update is rejected
as expected with a ChannelMonitorUpdateErr.
HTLC Transaction can't be bumped without sighash changes
so their gneeration is one-time for nwo. We move them in
OnchainTxHandler for simplifying ChannelMonitor and to prepare
storage of keys material behind one external signer interface.
Some tests break due to change in transaction broadcaster order.
Number of transactions may vary because of temporary anti-duplicata
tweak can't dissociate between 2- broadcast from different
origins (ChannelMonitor, ChannelManager) and 2-broadcast from same
component.
Local Commitment Transaction can't be bumped without anchor outputs
so their generation is one-time for now. We move them in
OnchainTxHandler for simplifying ChannelMonitor and to prepare
storage of keys material behind one external signer interface.
Some tests break due to change in transaction broadcast order but
number of transactions broadcast should stay the same.
Previously, we would regenerate this class of txn twice due to
block-rescan triggered by new watching outputs registered.
This commmit doesn't change behavior, it only tweaks TestBroadcaster
to ensure we modify cleanly tests anticipating next commit
refactor.
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.
