Not only was watchtower mode never implemented, but the bits that
we had were removed some time ago. It doesn't seem likely we'll
move forward with a "watchtower-mode" ChannelMonitor, instead
we'll likely have some other, separate struct for this.
`incorrect_or_unknown_payment_details` failure message,
`0x4000 (PERM) | 15`, should include the following data:
- [u64:htlc_msat]
- [u32:height]
This patches ensure that the height is included in all
the occurrences of this failure message.
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.
1107ab06c3 added a Vec of future
updates to apply during a loop, fixing a borrow checker issue that
didn't exist in the merged version of the patch. This simply reverts
that small part of the change.
The new OnchainDetection struct (which is the remnants of the old
KeyStorage enum, which was removed in 1dbda4faed)
doesn't really add any clarity to ChannelMonitor, so best to just
drop it and move its members into ChannelMonitor directly.
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.
Channel shouldn't send a ChannelForceClosed update followed by
a LatestLocalCommitmentTxInfo as it would be a programming error
leading to risk of money loss. Force-closing the channel will
broadcast the local commitment transaction, if the revocation
secret for this one is released after its broadcast, it would
allow remote party to claim outputs on this transaction using
the revocation path.
Local commitment transaction broadcast can be triggered by a)
a Channel force-close or b) reaching some block height implying
a onchain HTLC-timeout. If one of this condition is fulfilled,
commitment is signed and from then any state update would be
rejected.
ChannelMonitor init at Channel creation need to be refactored
before to make get_fully_signed_local_tx infaillible to avoid
choking in the test framework.
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.
In case of channel force-closure, access to local commitment
transactions and its dependent HTLCs is needed. Instead of using
broadcast_by_local_state which registers outpoint to claim and
outputs to watch which are going to be discarded in this case,
we simply ask OnchainTxHandler to build and sign HTLC transactions
through new API.
Splitting further parsing from transaction generation, we cache
transaction elements needed for local HTLC transaction inside
OnchainTxHandler. Duplicated data will be removed from ChannelMonitor
in future commits.
Splitting further parsing from transaction generation, we cache
transaction elements needed for local HTLC transaction inside
OnchainTxHandler. Duplicated data will be removed from ChannelMonitor
in future commits.
Extend external signer interface to sign HTLC transactions on its
behalf without seckey passing. This move will allow us to remove
key access access from ChannelMonitor hot memory in further work.
HTLC transactions should stay half-signed by remote until
we need to broadcast them for timing-out/claiming HTLCs onchain.
Caching of input material for HTLC transaction was introducted
prevently but since then API (InputMaterial) has changed
between ChannelMonitor and OnchainTxHandler
Implementing dynamic fee bumping implied to cache transaction material
including its witness, to generate a bumped version if needed.
ChannelMonitor is slowly rescoped to its parsing function with ongoing
patchset and data duplicata are removed. If signed local commitment tx
access is needed, it's done through OnchainTxHandler extended API
For test framework purpose, we use the test-only method
ChannelMonitor::unsafe_get_latest_local_commitment_txn to intentionally
generate unsafe local commitment to exerce revocation logic.
By caching current local commitment number instead of deciphering
it from local commitment tx, we may remove local commitment tx
from ChannelMonitor in next commit.
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.
To prevent any unsafe state discrepancy between offchain and onchain,
once local commitment transaction has been signed due to an event
(either block height for HTLC-timeout or channel force-closure), don't
allow any further update of local commitment transaction view
to avoid delivery of revocation secret to counterparty for the
aformentionned signed transaction.
As transaction generation and signature is headed to be moved
inside OnchainTxHandler, cache local_commitment_tx signed by remote.
If access to local commitment transaction is needed, we extend Onchain
TxHandler API to do so.
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.
Extend external signer interface to sign local commitment transactions
on its behalf without seckey passing. This move will allow us to remove
key access from ChannelMonitor hot memory in further work.
Local commitment transaction should stay half-signed by remote until
we need to broadcast for a channel force-close or a HTLC to timeout onchain.
Add an unsafe test-only version of sign_local_commitment to fulfill our
test_framework needs.
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.
