This was just an oversight when route calculation was split up into
parts - it makes no sense for get_route to require that we have a
full route message handler, only a network graph (which can always
be accessed from a NetGraphMsgHandler anyway).
... instead of only the txid.
This is another instance of it not being possible to fully
re-implement SimpleManyChannelMonitor using only public methods. In
this case you couldn't properly register outpoints for monitoring
so that the funding transaction would be matched.
Tests use sources of randomness to produce seeds, preimages, secrets,
and ephemeral data. However, this makes comparing logs between different
test runs difficult. Remove uses of random number generators and the
current time in favor of fixed values in order to make the test output
deterministic.
When we receive an inbound HTLC from a peer on an inbound channel,
make sure the funder can still cover the additional on-chain cost
of the HTLC while maintaining their channel reserve.
When we're sending an outbound HTLC, make sure the funder can still
cover the additional on-chain cost of the HTLC while maintaining
their channel reserve.
+ implement fee spike buffer for channel initiators sending payments.
Also add an additional spec-deviating fee spike buffer on the
receiving side (but don't close the channel if this reserve is
violated, just fail the HTLC).
From lightning-rfc PR #740.
Co-authored-by: Matt Corallo <git@bluematt.me>
Co-authored-by: Valentine Wallace <vwallace@protonmail.com>
`to_local` output or remote output on remote commitment transaction
needs a channel keys to be spent. As per-channel keys are derived from
KeysManager seed and per-channel secrets those must be backed up by
any descriptor bookmarking for latter spend. We test that generating
a new KeysManager loaded with such backed-up seed/per-channel secrets
return the correct keys for spending a `to_local` output.
Add sign_payment_transaction in ChanSigner to be able to spend
SpendableOutputDescriptor in test framework
Extend ChannelKeys with remote_pubkeys to access remote revocation
basepoint for witnessScript construction.
This makes it easier for our automated bindings generator to
function as it tries to automatically create a ::new if the struct
contains only pub elements who's type is convertible.
This caused a bunch of cascading changes, including
passing loggers down to Channels in function calls
rather than having each Channel have a pointer to the
ChannelManager's Logger (which was a circular reference).
Other structs that the Channel had passed its Logger to also
had their loggers removed. Other newly unused Loggers were
also removed, especially when keeping them would've caused
a bunch of extra test changes to be necessary, e.g. with
the ChainWatchInterfaceUtil's Logger.
This simplifies channelmonitor quite nicely (as expected) as we
never have to be concerned with learning data in a DataLossProtect
which is require for us to claim our funds from the latest remote
commitment transaction.
Previous to this commit, variables such as their_channel_reserve
referred to the channel reserve that _we_ are required to keep,
(the value is initially set by the remote). Similarly,
variables such as our_channel_reserve referred to the channel
reserve that we require the remote to keep.
Change this to use local_channel_reserve / remote_channel_reserve
to refer to the the channel reserve that the local is required to keep
and the channel reserve that the remote is required to keep, respectively.
Converting from InitFeatures to other Features is accomplished using
Features::with_known_relevant_init_flags. Define a more general
to_context method which converts from Features of one Context to
another.
Additionally, ensure the source context only has known flags before
selecting flags for the target context.
Features for a given context are duplicated throughout the features
module. Use a macro for defining a Context and the applicable features
such that features only need to be defined for a Context in one place.
The Context provides bitmasks for selecting known and unknown feature
flags.
BOLT 1 and BOLT 9 refer to features as "known" if a peer understands
them. They also use the term "supported" to mean either optional or
required.
Update the features module to use similar terminology.
- Define contexts in terms of required and optional features rather than
just supported features
- Define known features as those that are optional or required
- Rename supported() constructor to known()
For completeness, clear_optional_bit for each feature is now called
clear_bits and clears both optional and required bits.
This is a key test for our automatic HTLC time-out logic, as it
ensures we don't allow an HTLC which indicates we should wait for
additional HTLCs before responding to cause us to force-close a
channel due to HTLC near-timeout.
expect_payment_failed!() was introduced after many of the tests
which could use it were written, so we take this opportunity to
switch them over now, increasing test coverage slightly by always
checking the payment hash expected.
We only do this for incoming HTLCs directly as we rely on channel
closure and HTLC-Timeout broadcast to fail any HTLCs which we
relayed onwards where our next-hop doesn't update_fail in time.
Previously, we created the initial ChannelMonitor on outbound
channels when we generated the funding_created message. This was
somewhat unnecessary as, at that time, we hadn't yet received
clearance to broadcast our initial funding transaction, and thus
there should never be any use for a ChannelMonitor. It also
complicated ChannelMonitor a bit as, at this point, we didn't have
an initial local commitment transaction.
By moving the creation of the initial ChannelMonitor to when we
receive our counterparty's funding_signed, we can ensure that any
ChannelMonitor will always have both a latest remote commitment tx
and a latest local commitment tx for broadcast.
This also fixes a strange API where we would close a channel
unceremoniously on peer-disconnection if we hadn't yet received the
funding_signed, but we'd already have a ChannelMonitor for that
channel. While it isn't strictly a bug (some potential DoS issues
aside), it is strange that these two definitions of a channel being
open were not in sync.
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.
