A channel's `short_channel_id` is currently only set when the funding
transaction is confirmed via `transactions_confirmed`, which might be
well after the channel initally becomes usable, e.g., in the 0conf case.
Previously we would panic due to a reachable `unwrap` when receiving a
counterparty's `announcement_signatures` message for a 0conf channel
pending confirmation on-chain.
Here we fix this bug by avoiding unsafe `unwrap`s and just erroring out
and ignoring the announcement_signatures message if the `short_channel_id`
hasn't been set yet.
While bindings should probably be able to figure out that this is
the same type as `Self`, for now we simply swap the type to make
the bindings generator have an easier go of it.
Given we build `InFlightHtlcs` per route-fetch call, there's no
reason to pass them out by reference rather than simply giving the
user the full object. This also allows them to tweak the in-flight
set before fetching a route.
We introduce a `UnfundedChannelContext` which contains a counter for the
current age of an unfunded channel in timer ticks. This age is incremented
for every `ChannelManager::timer_tick_ocurred` and the unfunded channel
is removed if it exceeds `UNFUNDED_CHANNEL_AGE_LIMIT_TICKS`.
The value will not be persisted as unfunded channels themselves are not
persisted.
One of a series of follow-up commits to address some issues found
in PR 2077, where we split channels up into different maps and structs
depending on phase in their life.
We had some inconsistencies so far in referring to channels such as
`OutboundV1Channel` and `InboundV1Channel` as pending and unfunded.
From here we refer to these kinds of channels only as "unfunded".
This is a slight conflation with the term "unfunded" in the contexts
of denial of service mitigation. There, "unfunded" actually refers to
non-0conf, inbound channels that have not had their funding transaction
confirmed. This might warrant changing that usage to "unconfirmed inbound".
As done with inbound feerate updates, we can afford to commit less in
fees, as long as we still may the minimum mempool feerate. This enables
users to spend a bit more of their balance, as less funds are being
committed to transaction fees.
Channels supporting anchors outputs no longer require their feerate
updates to target a prompt confirmation since commitment transactions
can be bumped when broadcasting. Commitment transactions must now at
least meet the minimum mempool feerate, until package relay is deployed,
such that they can propagate across node mempools in the network by
themselves.
The existing higher bound no longer applies to channels supporting
anchor outputs since their HTLC transactions don't have any fees
committed, which directly impact the available balance users can send.
There's no need to yield such an event when the commitment transaction
already meets the target feerate on its own, so we can simply broadcast
it without an anchor child transaction. This may be a common occurrence
until we are less aggressive about feerate updates.
In an older PR a reviewer had asked why the discarding of a channel
being blocked on another monitor update is okay if the blocked
channel has since closed. At the time, this was not actually okay -
the monitor updates in the channel weren't moved to the
`ChannelManager` on close so the whole pipeline was busted, but
with the changes in 4041f0899f the
handling of channel closes with pending monitor updates is now
correct, and so is the existing code block.
If a `ChannelMonitorUpdate` completes being persisted, but the
`ChannelManager` isn't informed thereof (or isn't persisted) before
shutdown, on startup we may still have it listed as in-flight. When
we compare the available `ChannelMonitor` with the in-flight set,
we'll notice it completed and remove it, however this may leave
some post-update actions dangling which need to complete.
Here we handle this with a new `BackgroundEvent` indicating we need
to handle any post-update action(s) for a given channel.
If a channel has been closed, there may still be some
`ChannelMonitorUpdate`(s) which are pending completion. These
in-flight updates may also be blocking another channel from letting
an update fly, e.g. for forwarded payments where the payment
preimage will be removed from the downstream channel after the
upstream channel has closed.
Luckily all the infrastructure to handle this case is already in
place - we just need to process the
`monitor_update_blocked_actions` for closed channels.
This was a regression resulting from f2453b7 since we now process events
in a loop until there aren't any left. Processing events is done in
batches and they are not removed until we're done processing each batch.
Since handling a `PendingHTLCsForwardable` event will call back into the
`ChannelManager`, we'll still see the original forwarding event not
removed. Phantom payments will need an additional forwarding event
before being claimed to make them look real by taking more time.
Because `ChannelMonitorUpdate`s can complete asynchronously and
out-of-order now, a `commitment_signed` `ChannelMonitorUpdate` from
a downstream channel could complete prior to the preimage
`ChannelMonitorUpdate` on the upstream channel. In that case, we may
not get a `update_fulfill_htlc` replay on startup. Thus, we have to
ensure any payment preimages contained in that downstream update are
re-claimed on startup.
Here we do this during the existing walk of the `ChannelMonitor`
preimages for closed channels.
* `PhantomRouteHints::channels` has been written since the struct
was added in 410eb05365.
* `HTLCSource::path_hops` has been written since the struct was
converted to TLVs in 66784e32fe.
* `CommitmentTransaction::htlcs` has always been written since the
struct was converted to TLVs in 66784e32fe.
* `HolderCommitmentTransaction::counterparty_htlc_sigs` have always
been written since the struct was converted to TLVs in
c8bc1b6d3d.
This converts some required TLVs to `required_vec` which are, in
fact, required (and have been written forever).
* `HTLCFailReason` hasn't changed since many structs were converted
to TLVs in 66784e32fe.
* `NodeInfo::channels` has been written since `NetworkGraph`
structs were converted to TLVs in 321b19c4d9.
* Several test-only TLV writes were converted.
This commit makes use of the added enum to calculate the dust
exposure threshold based on the current fee rate. This also updates
tests to ensure it works as intended.
With fee rates rising dramatically in mid-April 2023, thresholds for
what is considered dust have risen, often exceeding our previous dust
exposure threshold of 5k sats. This causes all payments and HTLC
forwards between 5k sats and new dust thresholds to fail.
This commit changes our max dust exposure config knob from a fixed
upper limit to a `MaxDustHTLCExposure` enum with an additional variant
to allow setting our max dust exposure to a multiplier on the current
high priority feerate.
To remain backwards compatible we'll always write the fixed limit if
it's set, or its default value in its currently reserved TLV.
We also now write an odd TLV for the new enum, so that previous
versions can safely ignore it upon downgrading, while allowing us to
make use of the new type when it's written.
In 1ce2beb774,
`Channel::blocked_monitor_updates` was moved to an even TLV to
ensure downgrades with vec entries are forbidden. However, the
serialized type remained `vec_type`, which is always written.
Instead, `optional_vec` must be used.
