For quite some time, LDK has force-closed channels if the peer
sends us a feerate update which is below our `FeeEstimator`'s
concept of a channel lower-bound. This is intended to ensure that
channel feerates are always sufficient to get our commitment
transaction confirmed on-chain if we do need to force-close.
However, we've never checked our channel feerate regularly - if a
peer is offline (or just uninterested in updating the channel
feerate) and the prevailing feerates on-chain go up, we'll simply
ignore it and allow our commitment transaction to sit around with a
feerate too low to get confirmed.
Here we rectify this oversight by force-closing channels with stale
feerates, checking after each block. However, because fee
estimators are often buggy and force-closures piss off users, we
only do so rather conservatively. Specifically, we only force-close
if a channel's feerate is below the minimum `FeeEstimator`-provided
minimum across the last day.
Further, because fee estimators are often especially buggy on
startup (and because peers haven't had a chance to update the
channel feerates yet), we don't force-close channels until we have
a full day of feerate lower-bound history.
This should reduce the incidence of force-closures substantially,
but it is expected this will still increase force-closures somewhat
substantially depending on the users' `FeeEstimator`.
Fixes#993
When we connect 100 blocks in a row, requiring the fuzz input to
contain 100 fee estimator results is uneccessary, so add a bool
that lets us skip those reads.
Closure due to feerate disagreements are a specific closure reason
which admins can understand and tune their config (in the form of
their `FeeEstimator`) to avoid, so having a separate
`ClosureReason` for it is useful.
In the next commit we'll add a second field to
`ChannelError::Close` so here we prep by converting existing calls
to the constructor function, which is almost a full-file sed.
.. the bindings branch needed a small patch as it doesn't support `dyn`
traits. Here, we opt to use `Deref` in the hopes this works with
bindings, rendering the patch unnecessary.
When calling MessageRouter::create_blinded_path, ChannelManager was
including disconnected peers. Filter peers such that only connected ones
are included. Otherwise, the resulting BlindedPath may not work.
When an offer is short-lived, the likelihood of a channel used in a
compact blinded path going away is low. Require passing the absolute
expiry of an offer to ChannelManager::create_offer_builder so that it
can be used to determine whether or not compact blinded path should be
used.
Use the same criteria for creating blinded paths for refunds as well.
There's no need to save space when creating reply paths since they are
part of onion messages rather than in QR codes. Use normal blinded paths
for these instead as they are less likely to become invalid in case of
channel closure.
Using compact blinded paths isn't always necessary or desirable. For
instance, reply paths are communicated via onion messages where space
isn't a premium unlike in QR codes. Additionally, long-lived paths could
become invalid if the channel associated with the SCID is closed.
Refactor MessageRouter::create_blinded_paths into two methods: one for
compact blinded paths and one for normal blinded paths.
An upcoming change to the MessageRouter trait will require reusing
DefaultMessageRouter::create_blinded_paths. Move the code to a utility
function so facilitate this.
