As required by the warning messages PR, we should simply warn our
counterparty in this case and let them try again, continuing to try
to use the channel until they tell us otherwise.
A peer providing a channel_reserve_satoshis of 0 (or less than our
dust limit) is insecure, but only for them. Because some LSPs do it
with some level of trust of the clients (for a substantial UX
improvement), we explicitly allow it. Because its unlikely to
happen often in normal testing, we test it explicitly here.
We update the `Channel::update_time_counter` field (which is copied
into `ChannelUpdate::timestamp`) only when the channel is
initialized or closes, and when a new block is connected. However,
if a peer disconnects or reconnects, we may wish to generate
`ChannelUpdate` updates in between new blocks. In such a case, we
need to make sure the `timestamp` field is newer than any previous
updates' `timestamp` fields, which we do here by simply
incrementing it when the channel status is changed.
As a side effect of this we have to update
`test_background_processor` to ensure it eventually succeeds even
if the serialization of the `ChannelManager` changes after the test
begins.
Previously, `holder_selected_channel_reserve_satoshis` and
`holder_max_htlc_value_in_flight_msat` were constant functions
of the channel value satoshis. However, in the future we may allow
allow users to specify it. In order to do so, we'll need to track
them explicitly, including serializing them as appropriate.
We go ahead and do so here, in part as it will make testing
different counterparty-selected channel reserve values easier.
Previously, we would reject inbound channels if the funder wasn't
able to meet our channel reserve on their first commitment
transaction only if they also failed to push enough to us for us
to not meet their initial channel reserve as well.
There's not a lot of reason to care about us meeting their reserve,
however - its largely expected that they may not push enough to us
in the initial open to meet it, and its not actually our problem if
they don't.
Further, we used our own fee, instead of the channel's actual fee,
to calculate fee affordability of the initial commitment
transaction.
We resolve both issues here, rewriting the combined affordability
check conditionals in inbound channel open handling and adding a
fee affordability check for outbound channels as well.
The prior code may have allowed a counterparty to start the channel
with "no punishment" states - violating the reason for the reserve
threshold.
Instead of magic hard-coded constants, its better for tests to
derive the values used so that they change if constants are changed
and so that it is easier to re-derive constants in the future as
needed.
This may avoid risk of bugs in the future as it requires the caller
to think about the fee being used, not just blindly use the current
(committed) channel feerate.
Currently, we write out the Channel's `ChannelTypeFeatures` as an
odd type, implying clients which don't understand the
`ChannelTypeFeatures` field can simply ignore it. This is obviously
nonsense if the channel type is some future version - the client
needs to fail to deserialize as it doesn't understand the channel's
type.
We adapt the serialization logic here to only write out the
`ChannelTypeFeatures` field if it is something other than
only-static-remote-key, and simply consider that "default" (as it
is the only supported type today). Then, we write out the channel
type as an even TLV, implying clients which do not understand it
must fail to read the `Channel`.
Note that we do not need to bother reserving the TLV type no longer
written as it never appeared in a release (merged post-0.0.103).
We currently assume our counterparty is naive and misconfigured and
may force-close a channel to get an HTLC we just forwarded them.
There shouldn't be any reason to do this - we don't have any such
bug, and we shouldn't start by assuming our counterparties are
buggy. Worse, this results in refusing to forward payments today,
failing HTLCs for largely no reason.
Instead, we keep a fairly conservative check, but not one which
will fail HTLC forwarding spuriously - testing only that the HTLC
doesn't expire for a few blocks from now.
Fixes#1114.
This stores and tracks HTLC payee information with HTLCSource info,
allowing us to provide it back to the user if the HTLC fails and
ensuring persistence by keeping it with the HTLC itself as it
passes between Channel and ChannelMonitor.
This implements the channel type negotiation, though as we currently
only support channels with only static_remotekey set, it doesn't
implement the negotiation explicitly.
When an HTLC has been failed, we track it up until the point there
exists no broadcastable commitment transaction which has the HTLC
present, at which point Channel returns the HTLCSource back to the
ChannelManager, which fails the HTLC backwards appropriately.
When an HTLC is fulfilled, however, we fulfill on the backwards path
immediately. This is great for claiming upstream HTLCs, but when we
want to track pending payments, we need to ensure we can check with
ChannelMonitor data to rebuild pending payments. In order to do so,
we need an event similar to the HTLC failure event, but for
fulfills instead.
Specifically, if we force-close a channel, we remove its off-chain
`Channel` object entirely, at which point, on reload, we may notice
HTLC(s) which are not present in our pending payments map (as they
may have received a payment preimage, but not fully committed to
it). Thus, we'd conclude we still have a retryable payment, which
is untrue.
This commit does so, informing the ChannelManager via a new return
element where appropriate of the HTLCSource corresponding to the
failed HTLC.
During the event of a channel close, if the funding transaction
is yet to be broadcasted then a DiscardFunding event is issued
along with the ChannelClose event.
If a counterparty (or an old channel of ours) uses a non-segwit
script for their cooperative close payout, they may include an
output which is unbroadcastable due to not meeting the network dust
limit.
Here we check for this condition, force-closing the channel instead
if we find an output in the closing transaction which does not meet
the limit.
