Matt Corallo [Thu, 21 Mar 2024 15:18:22 +0000 (15:18 +0000)]
Allow(unused_imports) on prelude imports
New rustc now warns on duplicate imports when one of the imports
is from a wildcard import or the default prelude. Thus, because we
often don't actually use the imports from our prelude (as they
exist to duplicate the `std` default prelude), we have to mark most
of our `crate::prelude` imports with `#[allow(unused_imports)]`,
which we do here.
shaavan [Wed, 27 Mar 2024 13:03:42 +0000 (18:33 +0530)]
Delay broadcasting Channel Updates until connected to peers
- We might generate channel updates to be broadcast when
we are not connected to any peers to broadcast them to.
- This PR ensures to cache them and broadcast them only when
we are connected to some peers.
Other Changes:
1. Introduce a test.
2. Update the relevant current tests affected by this change.
3. Fix a typo.
4. Introduce two functions in functional_utils that optionally
connect and disconnect a dummy node during broadcast testing.
Wilmer Paulino [Fri, 29 Mar 2024 18:50:26 +0000 (11:50 -0700)]
Reserve async signing related channel TLV types
A LDK user deployed to production a WIP version of the async signing
branch in which two new TLVs were added to channel. To prevent them from
needing to perform a migration, we can just new types for TLVs that have
yet to be included in a release. A note has been added to ensure types
45 and 47 are not used for another purpose.
benthecarman [Wed, 27 Mar 2024 18:42:19 +0000 (13:42 -0500)]
Add DecodeError::DangerousValue for decoding invalid channel managers
This would help distinguish different types of errors when deserialzing
a channel manager. InvalidValue was used previously but this could be
because it is an old serialization format, whereas DangerousValue is a
lot more clear on why the deserialization failed.
Wilmer Paulino [Sat, 9 Mar 2024 08:53:48 +0000 (00:53 -0800)]
Decode update_add_htlc onions before forwarding HTLCs
This commit completes all of the groundwork necessary to decode incoming
`update_add_htlc` onions once they're fully committed to by both sides.
HTLCs are tracked in batches per-channel on the channel they were
received on. While this path is unreachable for now, until
`InboundHTLCResolution::Resolved` is replaced with
`InboundHTLCResolution::Pending`, it will allow us to obtain
`HTLCHandlingFailed` events for _any_ failed HTLC that comes across a
channel.
Wilmer Paulino [Fri, 8 Mar 2024 07:30:18 +0000 (23:30 -0800)]
Refactor forward_htlcs to return whether to push a forward event
When decoding pending `update_add_htlc` onions, we may need to forward
HTLCs using `ChannelManager::forward_htlcs`. This may end up queueing a
`PendingHTLCsForwardable` event, but we're only decoding these pending
onions as a result of handling a `PendingHTLCsForwardable`, so we
shouldn't have to queue another one and wait for it to be handled. By
having a `forward_htlcs` variant that does not push the forward event,
we can ignore the forward event push when forwarding HTLCs which we just
decoded the onion for.
Wilmer Paulino [Fri, 8 Mar 2024 07:35:30 +0000 (23:35 -0800)]
Consider pending decode_update_add_htlcs when pushing forward event
Since decoding pending `update_add_htlc` onions will go through the HTLC
forwarding path, we'll want to make sure we don't queue more events than
necessary if we have both HTLCs to forward/fail and pending
`update_add_htlc` onions to decode.
Wilmer Paulino [Fri, 8 Mar 2024 08:19:40 +0000 (00:19 -0800)]
Refactor incoming HTLC accept checks out from Channel::update_add_htlc
In the future, we plan to completely remove
`decode_update_add_htlc_onion` and replace it with a batched variant.
This refactor, while improving readability in its current form, does not
feature any functional changes and allows us to reuse the incoming HTLC
acceptance checks in the batched variant.
Wilmer Paulino [Fri, 8 Mar 2024 07:31:14 +0000 (23:31 -0800)]
Only include channel_update in failure if needed by error code
This simplifies the failure path by allowing us to return the general
error code for a failure, which we can then amend based on whether it
was for a phantom forward.
Wilmer Paulino [Wed, 24 Jan 2024 01:14:14 +0000 (17:14 -0800)]
Refactor outgoing channel lookup out from decode_update_add_htlc_onion
In the future, we plan to complete remove `decode_update_add_htlc_onion`
and replace it with a batched variant. This refactor, while improving
readability in its current form, does not feature any functional changes
and allows us to reuse most of the logic in the batched variant.
Wilmer Paulino [Wed, 24 Jan 2024 01:08:17 +0000 (17:08 -0800)]
Refactor outgoing HTLC checks out from decode_update_add_htlc_onion
In the future, we plan to complete remove `decode_update_add_htlc_onion`
and replace it with a batched variant. This refactor, while improving
readability in its current form, does not feature any functional changes
and allows us to reuse most of the logic in the batched variant.
Wilmer Paulino [Wed, 24 Jan 2024 00:55:40 +0000 (16:55 -0800)]
Refactor HTLCFailureMsg generation out from decode_update_add_htlc_onion
In the future, we plan to complete remove `decode_update_add_htlc_onion`
and replace it with a batched variant. This refactor, while improving
readability in its current form, does not feature any functional changes
and allows us to reuse most of the logic in the batched variant.
Wilmer Paulino [Tue, 23 Jan 2024 00:29:41 +0000 (16:29 -0800)]
Add new HTLCDestination variant for invalid onion
The existing variants do not cover such case as we previously never
surfaced `HTLCHandlingFailed` events for HTLCs that we failed back with
`UpdateFailMalformedHTLC` due to an invalid onion packet.
Wilmer Paulino [Fri, 8 Mar 2024 07:03:25 +0000 (23:03 -0800)]
Track pending update_add_htlcs in ChannelManager for later processing
We plan to decode the onions of these `update_add_htlc`s as part of the
HTLC forwarding flow (i.e., `process_pending_htlc_forwards`), so we'll
need to track them per-channel at the `ChannelManager` level.
Wilmer Paulino [Mon, 22 Jan 2024 23:28:59 +0000 (15:28 -0800)]
Remove unreachable handling of htlc_forwards upon channel_reestablish
`htlc_forwards` only returns a `Some` value from
`handle_channel_resumption` if we provide it a non-empty
`pending_forwards`. Since we don't, we'll never have a value to handle.
Wilmer Paulino [Mon, 22 Jan 2024 23:24:51 +0000 (15:24 -0800)]
Track incoming UpdateAddHTLC until HTLC resolution
This commit serves as a stepping stone to moving towards resolving HTLCs
once the HTLC has been fully committed to by both sides.
Currently, we decode HTLC onions immediately upon receiving an
`update_add_htlc`. Doing so determines what we should do with the HTLC:
forward it, or immediately fail it back if it cannot be accepted. This
action is tracked until the HTLC is fully committed to by both sides,
and a new commitment in the latter case is proposed to fully remove the
HTLC. While this has worked so far, it has some minor privacy
implications, as forwarding/failing back do not go through the usual
`PendingHTLCsForwardable` flow. It also presents issues with the
quiescence handshake, as failures through this path do not go through
the holding cell abstraction, leading to a potential violation of the
handshake by sending an `update_fail_*` after already having sent
`stfu`.
Since `pending_inbound_htlcs` are written pre-TLVs, we introduce a new
serialization version in which we change the `PendingHTLCStatus`
serialization of
`InboundHTLC::AwaitingRemoteRevokeToRemove/AwaitingRemovedRemoteRevoke`
to be an option instead. We'll still write it as the current version
(`MIN_SERIALIZATION_VERSION`), but we'll support reading the new version
to allow users to downgrade back to this commit.
Wilmer Paulino [Mon, 11 Mar 2024 17:21:43 +0000 (10:21 -0700)]
Support Vec serialization that include element length prefix
We add new macro alternatives to
impl_writeable_for_vec/impl_readable_for_vec that add a length prefix to
each element in the `Vec`. This is intended to be used over the existing
macros when attempting to serialize a `Vec` with elements of variable
lengths.
Wilmer Paulino [Thu, 7 Mar 2024 22:37:58 +0000 (14:37 -0800)]
Don't consume readers with FixedLengthReader
We can't always assume that we're done reading after using a
FixedLengthReader. In some cases, we may need to read a set of
length-prefixed objects one at a time, and we'd like to do so without
incurring an additional allocation by reading a number of bytes first to
then deserialize them.
Jeffrey Czyz [Sat, 23 Mar 2024 21:28:54 +0000 (16:28 -0500)]
Use AChannelManager in BackgroundProcessor
Replace instance of ChannelManager in BackgroundProcessor and in
Persister with AChannelManager. This reduces the number of type
parameters need in those types, which would need to be repeated in an
async version of Persister.
Elias Rohrer [Tue, 19 Mar 2024 14:30:16 +0000 (15:30 +0100)]
Dedup `confirmed_txs` `Vec`
Previously, we would just push to the `confirmed_txs` `Vec`, leading to
redundant `Confirm::transactions_confirmed` calls, especially now that
we re-confirm previously disconnected spends.
Here, we ensure that we don't push additional `ConfirmedTx` entries if
already one with matching `Txid` is present. This not only gets rid of
the spurious `transactions_confirmed` calls (which are harmless), but
more importantly saves us from issuing unnecessary network calls, which
improves latency.
Elias Rohrer [Thu, 21 Mar 2024 07:51:10 +0000 (08:51 +0100)]
Signal `GossipQuery` support when using `IgnoringMessagHandler`
With its v24.02 release CLN made `GossipQueries` a required feature,
leading to a incompatibility between LDK and CLN when using
`IgnoringMessagHandler` as a `RoutingMessageHandler`, which is usually
the case when a node uses RGS.
To fix this issue, we let `IgnoringMessagHandler` signal `GossipQuery`
support, just to go ahead and ignore every gossip message the peer will
send us. While this is nonsensical and still might result in some
unnecessary bandwidth wasted, we have to do something to fix the
incompatibility.
benthecarman [Tue, 20 Feb 2024 18:47:50 +0000 (18:47 +0000)]
Add HTLCsTimedOut closing reason
Before a force closure from timed out HTLCs was treated the same as when
the user manually force closed the channel. This leads to various UX
issues. This adds a new `ClosureReason` called `HTLCsTimedOut` that
signifies that the closure was caused because the HTLCs timed out. To go
along with this, previously we'd always send "Channel force-closed" when
force closing the channel in the error message which was ambigous, now
we send the force closure reason so the peer can know why the channel
was closed.
Jeffrey Czyz [Tue, 19 Mar 2024 22:04:01 +0000 (17:04 -0500)]
Use OnionMessenger::send_onion_message in tests
Use OnionMessenger's public interface in tests whenever possible (i.e.,
when not using any intermediate_nodes in an OnionMessagePath. This
allows us to exercise DefaultMessageRouter, and, in particular that a
path can be found for an unannounced sender when its in the introduction
node.
Jeffrey Czyz [Tue, 19 Mar 2024 21:04:43 +0000 (16:04 -0500)]
Fix sender is the introduction node onion messages
DefaultMessageRouter will form an OnionMessagePath from a BlindedPath
where the sender is the introduction node but only if the sender is
announced. If the sender is unannounced, then DefaultMessageRouter will
fail. While DefaultMessageRouter will only create a blinded path with an
announced introduction node, it may receive one where the introduction
node is unannounced. Don't return an error in this case, as the
OnionMessenger can advance the blinded path by one hop.
This may occur when two nodes have an unannounced channel and one (the
offer creator) wants to use it for payments without an intermediary node
and without putting its node id in the offer.
Elias Rohrer [Tue, 19 Mar 2024 14:07:27 +0000 (15:07 +0100)]
Track spent `WatchedOutput`s and re-add if unconfirmed
Previously, we would track a spending transaction but wouldn't account
for it being reorged out of the chain, in which case we wouldn't monitor
the `WatchedOutput`s until they'd be reloaded on restart.
Here, we keep any `WatchedOutput`s around until their spends are
sufficiently confirmed and only prune them after `ANTI_REORG_DELAY`.
Elias Rohrer [Thu, 7 Mar 2024 09:37:08 +0000 (10:37 +0100)]
Expose `{prev,next}_user_channel_id` fields in `PaymentForwarded`
This is useful for users that track channels by `user_channel_id`.
For example, in `lightning-liquidity` we currently keep a full
`HashMap<ChanelId, u128>` around *just* to be able to associate
`PaymentForwarded` events with the channels otherwise tracked by
`user_channel_id`.
Matt Corallo [Thu, 14 Mar 2024 19:47:07 +0000 (19:47 +0000)]
Avoid writing `ChannelManager` when hitting lnd bug 6039
When we hit lnd bug 6039, we end up sending error messages to peers
in a loop. This should be fine, but because we used the generic
`PersistenceNotifierGuard::notify_on_drop` lock above the specific
handling, we end up writing `ChannelManager` every time we manage a
round-trip to our peer.
This can add up quite quickly, and isn't actually changing, so we
really need to avoid writing the `ChannelManager` in this case.
Duncan Dean [Tue, 21 Mar 2023 19:12:53 +0000 (21:12 +0200)]
Implement interactive tx construction protocol via `InteractiveTxConstructor`
This implements the interactive construction protocol described at
https://github.com/lightning/bolts/blob/78e5a6b066d3a8e235931dfc06aa325337874749/02-peer-protocol.md?plain=1#L92.
Our implementation includes a state machine with typed states and transitions
to ensure consumers have compile-time assurances that the protocol is upheld.
States are tracked as in the `StateMachine` enum and can take on all
possible states during the negotiation.
The states are further divided into two categories, namely by the two traits
they implement, either `ReceivedMsgState` or `SentMsgState`.
The defined `StateTransitions` enforce the transitions that `ReceivedMsgState`,
`SentMsgState`, and the `_TxComplete`s can go through.
Co-authored-by: Wilmer Paulino <9447167+wpaulino@users.noreply.github.com> Co-authored-by: Duncan Dean <git@dunxen.dev> Co-authored-by: Jurvis Tan <5944973+jurvis@users.noreply.github.com>
Matt Corallo [Wed, 13 Mar 2024 19:18:20 +0000 (19:18 +0000)]
Replace the generic `parse_int_be` with a macro called twice
`parse_int_be` is generic across integer types and also input
types, but to do so it relies on the `num-traits` crate. There's
not a lot of reason for this now that std has `from_be_bytes`, so
we drop the generic now and replace it with a macro which is called
twice to create two functions, both only supporting conversion from
`u5` arrays.
Matt Corallo [Wed, 13 Mar 2024 19:16:20 +0000 (19:16 +0000)]
Use std's `from_be_bytes` rather than our `to_int_be` for int conv
`lightning-invoice` was mostly written before std's `from_be_bytes`
was stabilized, so used its own `to_int_be` utility to do int
conversions from `u8` arrays. Now that the std option has been
stable for quite some time, we should juse use it instead.
Matt Corallo [Tue, 6 Feb 2024 23:17:50 +0000 (23:17 +0000)]
Add `PersistenceNotifierGuard` take to offer/refund payments
This resolves an issue where offer and refund payments get delayed
while we wait for the `invoice_request`/`invoice` onion messages to
get sent. It further ensures we're likely to have the
`ChannelManager` persisted with the new payment info after
initiating the send/receive.
Matt Corallo [Tue, 6 Feb 2024 23:13:08 +0000 (23:13 +0000)]
Use `{}`, not `{:?}` for `PublicKey` logging
The `Debug` serialization of `PublicKey`s includes both the X and Y
coordinate, which isn't something most of our users deal with.
Instead, logging using `Display` gives users the keys they're used
to.
Matt Corallo [Tue, 12 Mar 2024 15:19:51 +0000 (15:19 +0000)]
Avoid new "out of disk space" issues in CI
Our 1.63 build on Ubuntu has been failing for quite some time
because it runs out of disk space trying to build tests in the last
cfg-flag steps. Thus, we add a few new `cargo clean`s here to fix
it.
Jeffrey Czyz [Thu, 29 Feb 2024 21:21:03 +0000 (15:21 -0600)]
Fail request_refund_payment for unsupported chain
If a Refund has an unsupported chain, ChannelManager should not send an
invoice as it can't be paid on that chain. Instead, return an error when
calling ChannelManager::request_refund_payment for such refunds.