Matt Corallo [Wed, 5 Jul 2023 17:06:19 +0000 (17:06 +0000)]
Add comment describing when a completion action can be discarded
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 4041f0899f86eaf6a0a4576a91918fa54026ac46 the
handling of channel closes with pending monitor updates is now
correct, and so is the existing code block.
Matt Corallo [Wed, 5 Jul 2023 02:15:42 +0000 (02:15 +0000)]
Run monitor update completion actions for pre-startup completion
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.
Matt Corallo [Fri, 7 Jul 2023 21:05:29 +0000 (21:05 +0000)]
Drop `vec_type` TLV handling entirely
Historically, we used `vec_type` for all TLV Vec reads/writes, but
it is asymmetric and thus somewhat confusing - on the write side it
always writes a TLV entry, even if there are zero elements. On the
read side, it happily accepts a missing TLV, providing a
zero-length vector.
Matt Corallo [Fri, 7 Jul 2023 19:57:23 +0000 (19:57 +0000)]
Convert `channelmonitor` `vec_type` TLV writes to required/optional
* `HolderSignedTx::htlc_outputs` has always been written since it
was converted to TLVs in 86641ea68062388a01c99c6ed131f56477c8e4.
* `ChanelMonitorUpdateStep::*::htlc_outputs` have been written
since the enum was converted to TLVs in 86641ea68062388a01c99c6.
Matt Corallo [Tue, 4 Jul 2023 15:13:32 +0000 (15:13 +0000)]
Handle monitor completion actions for closed channels
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.
Matt Corallo [Tue, 20 Jun 2023 22:52:27 +0000 (22:52 +0000)]
Don't drop `ChannelMonitor` `Event`s until they're processed
We currently assume the owner of `ChannelMonitor`s won't persist
the `ChannelMonitor` while `Event`s are being processed. This is
fine, except (a) its generally hard to do so and (b) the
`ChainMonitor` doesn't even do this.
Thus, in rare cases, a user could begin processing events which
are, generated by connecting a transaction or a new best-block,
take some time to do so, and while doing so process a further chain
event, causing persistece. This could lose the event being
processed alltogether, which could lose the user funds.
This should be very rare, but may have been made slightly more
reachable with (a) the async event processing making it more
common to do networking in event handling, (b) the new future
generation in the `ChainMonitor`, which now wakes the
`background-processor` directly when chain actions happen on the
`ChainMonitor`.
Handle new event processing logic when enqueuing forward event
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.
benthecarman [Thu, 18 May 2023 20:43:56 +0000 (15:43 -0500)]
Add helper functions to verify node and channel annoucements
Right now the only real way to verify the node and channel
announcements is to call `update_node_from_announcement`/
`update_channel_from_announcement`. If you want to do some
processing before you add to your network graph then you need to
manually verify the signature. This adds some nice helper functions
to make it easier.
I tried to do the same for channel update but it did not seem as
easy so figured that is fine to punt on since I don't see many
people doing manual things with channel updates.
Matt Corallo [Thu, 4 May 2023 21:51:18 +0000 (21:51 +0000)]
Re-claim forwarded HTLCs on startup
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.
Matt Corallo [Fri, 7 Jul 2023 19:27:15 +0000 (19:27 +0000)]
Convert routing `vec_type `TLV writes to other TLV types
* `BlindedTail::hops` has always been written since it was
introduced in 64c26c8a793f35c55076a2188912e92106900bab.
* `PaymentParameters::clear_hints` has always been written since
it was introduced as `Payee::route_hitns` in 46b68c517dba57bde5.
Matt Corallo [Fri, 7 Jul 2023 20:55:12 +0000 (20:55 +0000)]
Migrate `chan_utils` `vec_type` TLVs to required/optional
* `CommitmentTransaction::htlcs` has always been written since the
struct was converted to TLVs in 66784e32fe454e9a5b2080b85fc4d881.
* `HolderCommitmentTransaction::counterparty_htlc_sigs` have always
been written since the struct was converted to TLVs in c8bc1b6d3de45aeddc755d0875b3dce8d86f79c1.
Matt Corallo [Fri, 7 Jul 2023 18:44:24 +0000 (18:44 +0000)]
Convert some `vec_type` TLVs to `required_vec`
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 66784e32fe454e9a5b2080b85fc4d8816ac5e436.
* `NodeInfo::channels` has been written since `NetworkGraph`
structs were converted to TLVs in 321b19c4d96a49b39b5583d0f12fea.
* Several test-only TLV writes were converted.
Matt Corallo [Fri, 7 Jul 2023 18:38:06 +0000 (18:38 +0000)]
Add a `required_vec` TLV deserialization type
Historically, we used `vec_type` for all TLV Vec reads/writes, but
it is asymmetric and thus somewhat confusing - on the write side it
always writes a TLV entry, even if there are zero elements. On the
read side, it happily accepts a missing TLV, providing a
zero-length vector.
In 85b573ddad70f3c5ee36e0992d587842af507a8d a new `optional_vec`
TLV format was added which was symmetric, but only supports
optional vecs. This adds the corresponding required form, always
writing a TLV and ensuring it is present.
Use multiplier in dust exposure threshold calculation
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.
Alec Chen [Mon, 19 Jun 2023 02:53:43 +0000 (21:53 -0500)]
Add max dust exposure multiplier config knob
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.
Matt Corallo [Fri, 7 Jul 2023 18:28:25 +0000 (18:28 +0000)]
Fix backwards compat for `blocked_monitor_updates`
In 1ce2beb77455a674888f3e3589723195eaa7b13c,
`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.
Matt Corallo [Wed, 5 Jul 2023 17:20:48 +0000 (17:20 +0000)]
Move Channel's blocked monitor updates vec to an even TLV
In 9dfe42cf8681afc9c3f84e0c85a4e2a30c1156a8,
`ChannelMonitorUpdate`s were stored in `Channel` while they were
being processed. Because it was possible (though highly unlikely,
due to various locking likely blocking persistence) an update was
in-flight (even synchronously) when a `ChannelManager` was
persisted, the new updates were persisted via an odd TLV.
However, in 4041f0899f86eaf6a0a4576a91918fa54026ac46 these pending
monitor updates were moved to `ChannelManager`, with appropriate
handling there. Now the only `ChannelMonitorUpdate`s which are
stored in `Channel` are those which are explicitly blocked, which
requires the async pipeline.
Because we don't support async monitor update users downgrading to
0.0.115 or lower, we move to persisting them via an even TLV. As
the odd TLV storage has not yet been released, we can do so
trivially.
Andrei [Thu, 29 Jun 2023 00:00:00 +0000 (00:00 +0000)]
Use `MonotonicTime` as `Instant` shifted by 10 years forward
Such implementation allows `MonotonicTime` to go backward up to 10
years on all platforms. On some platforms (e.g. iOS) `Instant` is
represented as `u64` of nanoseconds since the boot of the system.
Obviously such implementation does not allow to go backward before the
time of the boot.
Co-authored-by: Andrei <andrei.i@posteo.de> Co-authored-by: Jeffrey Czyz <jkczyz@gmail.com>
Wilmer Paulino [Thu, 22 Jun 2023 22:19:15 +0000 (15:19 -0700)]
Require best block timestamp within ChannelManager::new
This ensures freshly initialized nodes can proceed to create unexpired
invoices without a call to `best_block_updated`, since an invoice's
expiration delta is applied to `highest_seen_timestamp`.
Wilmer Paulino [Tue, 20 Jun 2023 19:14:48 +0000 (12:14 -0700)]
Require inbound channels with anchor outputs to be accepted manually
Since the use of channels with anchor outputs requires a reserve of
onchain funds to handle channel force closures, it would be
irresponsible to allow a node to accept inbound channel without first
consulting such reserves. To allow users to do so, we require such
channels be manually accepted.
Wilmer Paulino [Tue, 20 Jun 2023 18:29:00 +0000 (11:29 -0700)]
Remove anchors config flag
Now that all of the core functionality for anchor outputs has landed,
we're ready to remove the config flag that was temporarily hiding it
from our API.
Matt Corallo [Tue, 20 Jun 2023 02:16:03 +0000 (02:16 +0000)]
Rename Channel's latest-monitor-update fetch method for clarity
`Channel::get_latest_complete_monitor_update_id` no longer refers
to complete updates, but rather ones which were passed to the
`ChannelManager` and which the `CHannel` no longer knows about.
Thus, we rename it `get_latest_unblocked_monitor_update_id`.
Matt Corallo [Mon, 19 Jun 2023 06:26:39 +0000 (06:26 +0000)]
Move in-flight `ChannelMonitorUpdate`s to `ChannelManager`
Because `ChannelMonitorUpdate`s can be generated for a
channel which is already closed, and must still be tracked
through their completion, storing them in a `Channel`
doesn't make sense - we'd have to have a redundant place to
put them post-closure and handle both storage locations
equivalently.
Instead, here, we move to storing in-flight
`ChannelMonitorUpdate`s to the `ChannelManager`, leaving
blocked `ChannelMonitorUpdate`s in the `Channel` as they
were.
Arik Sosman [Mon, 19 Jun 2023 17:30:34 +0000 (10:30 -0700)]
Replace `opt_anchors` with `ChannelTypeFeatures`
This change modifies six structs that were keeping
track of anchors features with an `opt_anchors` field,
as well as another field keeping track of nonzero-fee-
anchor-support.
Arik Sosman [Mon, 19 Jun 2023 17:28:36 +0000 (10:28 -0700)]
Define `ChannelTypeFeatures` methods for anchors logic.
Specifically, introduce a new constructor for an anchors-
supporting feature set, as well as methods that will
maintain forwards-compatible deserialization in older
versions.
Matt Corallo [Sun, 18 Jun 2023 23:56:16 +0000 (23:56 +0000)]
Simplify cases in `handle_new_monitor_update` macro
By giving up on a tiny bit of parallelism and tweaking the return
types, we can make the `handle_new_monitor_update` macro a bit
clearer - now the only cases where its called after a monitor was
updated was when the monitor was initially committed.
Matt Corallo [Sun, 18 Jun 2023 21:55:30 +0000 (21:55 +0000)]
Move most `handle_new_monitor_update` calls to pass the update
Most of the calls to the `handle_new_monitor_update` macro had the
exact same pattern - calling `update_monitor` followed by the
macro. Given that common pattern will grow to first pushing the
new monitor onto an in-flight set and then calling `update_monitor`
unifying the pattern into a single macro now avoids more code churn
in the coming commits.
Matt Corallo [Sun, 18 Jun 2023 21:18:03 +0000 (21:18 +0000)]
Return owned `ChannelMonitorUpdate`s from `Channel`
In the coming commits we'll move to storing in-flight
`ChannelMonitorUpdate`s in the `ChannelManager` rather in the
`Channel` (which will then only retain `ChannelMonitorUpdate`s
which have not yet been released/are blocked.
This will simplify handling of pending `ChannelMonitorUpdate` after
a channel has closed by not having to move them into the
`ChannelManager`.
Matt Corallo [Tue, 20 Jun 2023 17:14:21 +0000 (17:14 +0000)]
Ensure we build if a downstream crate sets `--cfg=fuzzing`
Downstream crates building fur fuzzing will usually set
`--cfg=fuzzing` as a side-effect of the Rust fuzzing tooling. Thus,
we should ensure we build without failure in such cases.
We do this here by simply relying on the `_test_utils` feature,
rather than conditionally-compiling in modules based on the
`fuzzing` flag.
Make sure the penultimate hop took the amount of fee that they claimed to take.
Without checking this TLV, we're heavily relying on the receiving wallet code
to correctly implement logic to calculate that that the fee is as expected.