Matt Corallo [Sat, 23 Nov 2024 20:57:17 +0000 (20:57 +0000)]
Add `probing_diversity_penalty_msat` to the scorer parameters
When doing background probing, its important to get a diverse view
of the network graph to ensure you have as many options when
pathfinding as possible.
Sadly, the naive probing we currently recommend users do does not
accomplish that - using the same success-optimized pathfinder when
sending as when probing results in always testing the same (good)
path over and over and over again.
Instead, here, we add a `probing_diversity_penalty_msat` parameter
to the scorer, allowing us to penalize channels for which we
already have recent data. It applies a penalty which scales with
the square of the inverse time since the channel was last updated,
up to one day.
Matt Corallo [Sat, 23 Nov 2024 20:48:34 +0000 (20:48 +0000)]
Track the last time we received new information about a channel
In the next commit we'll enable users to assign a penalty to
channels which we don't have any recent information for to better
enable probing diversity. In order to do so, we need to actually
track the last time we received new information for a channel,
which we do here.
Matt Corallo [Sat, 23 Nov 2024 20:36:06 +0000 (20:36 +0000)]
Calculate the padding required on `ChannelLiquidity` dynamically
We expect `ChannelLiquidity` to be exactly three cache lines to
ensure the first bytes we need are all one one cache line, but in
practice its a bit more ideal for `ChannelLiquidity`s to always
start on an even cache line as x86 CPUs will often load the
neighboring cache line automatically.
Further, it looks like some versions of `rustc` on some platforms
don't pack `ChannelLiquidity` as well (in #3415) and the next
commit is going to push us over three cache lines anyway.
Instead, here we calculate out the proper padding for
`ChannelLiquidity` to make it align to four 64-byte cache lines.
Duncan Dean [Fri, 25 Oct 2024 08:53:30 +0000 (10:53 +0200)]
Add features in InteractiveTxConstructor requried for V2 chan accept
1. InteractiveTxConstructorArgs is introduced to act as a single, more
readable input to InteractiveTxConstructor::new().
2. Various documentation updates.
For now this is unneeded as we do not provide any inputs as channel
acceptor and we do not allow creating outbound channels yet. It will
be re-added when that functionality is introduced.
Matt Corallo [Thu, 10 Oct 2024 19:42:16 +0000 (19:42 +0000)]
Avoid startup `PeerState` entries for peers with unfunded channels
If a peer creates a channel with us which never reaches the funding
stage (or never gets any commitment updates after creation), we'll
avoid inserting the `update_id` into
`closed_channel_monitor_update_ids` at runtime to avoid keeping a
`PeerState` entry around for no reason. However, on startup we
still create a `ChannelMonitorUpdate` with a `ChannelForceClosed`
update step to ensure the `ChannelMonitor` is locked and shut down.
This is pretty redundant, and results in a bunch of on-startup
`ChannelMonitorUpdate`s for any old but non-archived
`ChannelMonitor`s. Instead, here, we check if a `ChannelMonitor`
already saw a `ChannelForceClosed` update step before we generate
the on-startup `ChannelMonitorUpdate`.
This also allows us to skip the `closed_channel_monitor_update_ids`
insertion as we can be confident we'll never have a
`ChannelMonitorUpdate` for this channel at all.
Matt Corallo [Sun, 6 Oct 2024 19:58:29 +0000 (19:58 +0000)]
Stop using a constant for monitor `update_id`s after closure
Because `ChannelManager` doesn't have a corresponding `Channel`
after the channels are closed, we'd always used an `update_id` of
`u64::MAX` for any `ChannelMonitorUpdate`s we need to build after
the channel is closed.
This completely breaks the abstraction of `update_id`s and leaks
into persistence logic - because we might have more than one
`ChannelMonitorUpdate` with the same (`u64::MAX`) value, suddenly
instead of being able to safely use `update_id` as IDs, the
`MonitorUpdatingPersister` has to have special logic to handle
this.
Worse, because we don't have a unique ID with which to refer to
post-close `ChannelMonitorUpdate`s we cannot track when they
complete async persistence. This means we cannot properly support
async persist for forwarded payments where the inbound edge has hit
the chain prior to the preimage coming to us.
Here we rectify this by using consistent `update_id`s even after a
channel has closed. In order to do so we have to keep some state
for all channels for which the `ChannelMonitor` has not been
archived (after which point we can be confident we will not need to
update them). While this violates our long-standing policy of
having no state at all in `ChannelManager`s for closed channels,
its only a `(ChannelId, u64)` pair per channel, so shouldn't be
problematic for any of our users (as they already store a whole
honkin `ChannelMonitor` for these channels anyway).
While limited changes are made to the connection-count-limiting
logic, reviewers should carefully analyze the interactions the new
map created here has with that logic.
Matt Corallo [Sun, 6 Oct 2024 19:54:32 +0000 (19:54 +0000)]
Prefer to use `MonitorUpdateRegeneratedOnStartup` where possible
In the next commit we'll drop the magic `u64::MAX`
`ChannelMonitorUpdate::update_id` value used when we don't know the
`ChannelMonitor`'s `latest_update_id` (i.e. when the channel is
closed). In order to do so, we will store further information about
`ChannelMonitor`s in the per-peer structure, keyed by the
counterparty's node ID, which will be used when applying
`ChannelMonitorUpdate`s to closed channels.
By taking advantage of the change in the previous commit, that
information is now reliably available when we generate the
`ChannelMonitorUpdate` (when claiming HTLCs), but in order to
ensure it is available when applying the `ChannelMonitorUpdate` we
need to use `BackgroundEvent::MonitorUpdateRegeneratedOnStartup`
instead of
`BackgroundEvent::ClosedMonitorUpdateRegeneratedOnStartup` where
possible.
Here we do this, leaving `ClosedMonitorUpdateRegeneratedOnStartup`
only used to ensure very old channels (created in 0.0.118 or
earlier) which are not in the `ChannelManager` are force-closed on
startup.
Matt Corallo [Wed, 9 Oct 2024 19:05:18 +0000 (19:05 +0000)]
Req the counterparty node id when claiming against a closed chan
Currently we store in-flight `ChannelMonitorUpdate`s in the
per-peer structure in `ChannelManager`. This is nice and simple as
we're generally updating it when we're updating other per-peer
data, so we already have the relevant lock(s) and map entries.
Sadly, when we're claiming an HTLC against a closed channel, we
didn't have the `counterparty_node_id` available until it was
added in 0.0.124 (and now we only have it for HTLCs which were
forwarded in 0.0.124). This means we can't look up the per-peer
structure when claiming old HTLCs, making it difficult to track the
new `ChannelMonitorUpdate` as in-flight.
While we could transition the in-flight `ChannelMonitorUpdate`
tracking to a new global map indexed by `OutPoint`, doing so would
result in a major lock which would be highly contended across
channels with different peers.
Instead, as we move towards tracking in-flight
`ChannelMonitorUpdate`s for closed channels we'll keep our existing
storage, leaving only the `counterparty_node_id` issue to contend
with.
Here we simply accept the issue, requiring that
`counterparty_node_id` be available when claiming HTLCs against a
closed channel. On startup, we explicitly check for any forwarded
HTLCs which came from a closed channel where the forward happened
prior to 0.0.124, failing to deserialize, or logging an warning if
the channel is still open (implying things may work out, but panics
may occur if the channel closes prior to HTLC resolution).
While this is a somewhat dissapointing resolution, LDK nodes which
forward HTLCs are generally fairly well-upgraded, so it is not
anticipated to be an issue in practice.
Matt Corallo [Sun, 14 Jul 2024 13:08:04 +0000 (13:08 +0000)]
Add a `lightning-dns-resolver` crate which answers bLIP 32 queries
When a lightning node wishes to send payments to a BIP 353 human
readable name (using BOLT 12), it first has to resolve that name to
a DNS TXT record. bLIP 32 defines a way to do so over onion
messages, and this completes our implementation thereof by adding
the server side.
It operates by simply accepting new messages and spawning tokio
tasks to do DNS lookups using the `dnsse_prover` crate. It also
contains full end-to-end tests of the BIP 353 -> BOLT 12 -> payment
logic using the new server code to do the resolution.
Note that because we now have a workspace crate which sets the
"lightning/dnssec" feature in its `dev-dependencies`, a naive
`cargo test` will test the "dnssec" feature.
Matt Corallo [Sun, 14 Jul 2024 01:17:03 +0000 (01:17 +0000)]
Use `ChannelManager` as `DNSResolverMessageHandler` by default
Now that `ChannelManager` supports using bLIP 32 to resolve BIP 353
Human Readable Names we should encourage users to use that feature
by making the "default" (in various type aliases) to use
`ChannelManager` as the `DNSResolverMessageHandler`.
Matt Corallo [Thu, 7 Nov 2024 15:05:26 +0000 (15:05 +0000)]
Support paying Human Readable Names directly from `ChannelManager`
Now that we have the ability to resolve BIP 353 Human Readable
Names directly and have tracking for outbound payments waiting on
an offer resolution, we can implement full BIP 353 support in
`ChannelManager`.
Users will need one or more known nodes which offer DNS resolution
service over onion messages using bLIP 32, which they pass to
`ChannelManager::pay_for_offer_from_human_readable_name`, as well
as the `HumanReadableName` itself.
From there, `ChannelManager` asks the DNS resolver to provide a
DNSSEC proof, which it verifies, parses into an `Offer`, and then
pays.
For those who wish to support on-chain fallbacks, sadly, this will
not work, and they'll still have to use `OMNameResolver` directly
in order to use their existing `bitcoin:` URI parsing.
Matt Corallo [Mon, 30 Sep 2024 18:18:38 +0000 (18:18 +0000)]
Store the source `HumanReadableName` in `InvoiceRequestFields`
When we receive a payment to an offer we issued resolved with a
human readable name, it may have been resolved using a wildcard
DNS entry which we want to map to a specific recipient account
locally. To do this, we need the human readable name from the
`InvoiceRequest` in the `PaymentClaim{able,ed}`, which we pipe
through here using `InvoiceRequestFields`.
Matt Corallo [Sat, 13 Jul 2024 14:04:39 +0000 (14:04 +0000)]
Add support for storing a source HRN in BOLT 12 `invoice_request`s
When we resolve a Human Readable Name to a BOLT 12 `offer`, we may
end up resolving to a wildcard DNS name covering all possible
`user` parts. In that case, if we just blindly pay the `offer`, the
recipient would have no way to tell which `user` we paid.
Instead, BOLT 12 defines a field to include the HRN resolved in the
`invoice_request`, which we implement here.
We also take this opportunity to remove constant parameters from
the `outbound_payment.rs` interface to `channelmanager.rs`
Matt Corallo [Tue, 1 Oct 2024 18:06:19 +0000 (18:06 +0000)]
Skip the implicit trailing `.` in `HumanReadableName`'s domain
Domain names implicitly have a trailing `.`, which we require in
bLIP 32 but generally shouldn't be exposing to the user in
`HumanReadableName`s (after all, they're human-readable). Here we
make sure the trailing `.` is dropped in `HumanReadableName`s
before we re-add them when building the bLIP 32 messages.
Jeffrey Czyz [Tue, 20 Aug 2024 20:52:17 +0000 (15:52 -0500)]
Disallow user-provided payer_signing_pubkey
When creating an InvoiceRequests, users may choose to either use a
transient signing pubkey generated by LDK or provide a static one.
Disallow the latter as it allows users to reuse the same pubkey, which
results in poor sender privacy.
If we're receiving a keysend to a blinded path, then we created the payment
secret within. Using our inbound_payment_key, we can decrypt the payment secret
bytes to get the payment's min_cltv_expiry_delta and min amount, to verify the
payment is valid. However, if we're receiving an MPP keysend *not* to a blinded
path, then we did not create the payment secret and shouldn't verify it since
it's only used to correlate MPP parts.
Therefore, store whether the payment secret is recipient-generated in our pending
inbound payment data so we know whether to verify it or not.
Jeffrey Czyz [Tue, 29 Oct 2024 14:58:37 +0000 (09:58 -0500)]
Test new behavior in create_bolt11_invoice
Bolt11InvoiceParameters allows for setting currency and
duration_since_epoch. If currency is not set, test that the one
corresponding to ChannelManager's chain hash is usd. If
duration_since_epoch, is not set then highest seen timestamp is used in
non-std compilations.
Jeffrey Czyz [Wed, 6 Nov 2024 22:53:07 +0000 (16:53 -0600)]
Deprecate functions in invoice_utils module
ChannelManager::create_bolt11_invoice is a simpler and more flexible way
of creating a BOLT11 invoice, so deprecate the corresponding functions
in the invoice_utils module.
Jeffrey Czyz [Mon, 28 Oct 2024 22:28:10 +0000 (17:28 -0500)]
Add a BOLT11 invoice utility to ChannelManager
Now that the lightning crate depends on the lightning_invoice crate, the
utility functions previously living in the latter can be implemented on
ChannelManager. Additionally, the parameters are now moved to a struct
in order to remove the increasingly combinatorial blow-up of methods.
The new Bolt11InvoiceParameters is used to determine what values to set
in the invoice. Using None for any given parameter results in a
reasonable the default or a behavior determined by the ChannelManager as
detailed in the documentation.
Jeffrey Czyz [Fri, 8 Nov 2024 17:44:00 +0000 (11:44 -0600)]
Remove currency support from invoice_utils
When creating an invoice using a ChannelManager, payments for a specific
ChainHash / Network are only valid. Use the one from the ChannelManager
instead of allowing arbitrary ones in the form of a Currency.
Support verifying payment_secrets for inbound static invoice payments
Add a new payment type for this, because normally the payment hash is factored
into the payment secrets we create for invoices, but static invoices don't have
a payment hash since they are paid via keysend.
Add new inbound payment key for spontaneous payments
This key will be used in upcoming commits for encrypting metadata bytes for
spontaneous payments' payment secrets, to be included in the blinded paths of
static invoices for async payments. We need a new type of payment secret for
these payments because they don't have an a prior known payment hash, see the
next commit.
Remove pending_inbound_payments map from ChannelManager
LDK versions prior to 0.0.104 had stateful inbound payments written in this
map. In 0.0.104, we added support for stateless inbound payments with
deterministically generated payment secrets, and maintained deprecated support
for stateful inbound payments until 0.0.116. After 0.0.116, no further inbound
payments could have been written into this map.
Jeffrey Czyz [Thu, 7 Nov 2024 00:53:18 +0000 (18:53 -0600)]
Remove no-std support from invoice_utils functions
The upcoming ChannelManager::create_bolt11_invoice will not support
setting a specific creation time, so remove that functionality from the
invoice_utils functions. This will avoid duplicate code when
deprecating.
Elias Rohrer [Thu, 7 Nov 2024 13:29:09 +0000 (14:29 +0100)]
Pin `indexmap` back to account for `hashbrown` MSRV bump
`indexmap` 2.6.0 upgraded to `hashbrown` 0.15, which unfortunately
bumped their MSRV to rustc 1.65 with the 0.15.1 release. So we pin
`indexmap` to 2.5.0 to fix our MSRV CI.
Matthew Rheaume [Tue, 5 Nov 2024 00:11:37 +0000 (16:11 -0800)]
Updated docs on `PeerManager::process_events`.
Try to make it a bit more clear that there are downsides to solely
relying on `lightning-net-tokio`, and it's still recommended to
occasionally call this function in a separate loop.
Matt Corallo [Fri, 4 Oct 2024 17:54:00 +0000 (17:54 +0000)]
Build `per_peer_state` immediately in `ChannelManager` deser
Instead of first building a map from peers to a list of channels
then pulling out of that to build the `per_peer_state`, we build
`per_peer_state` immediately and store channels in it immediately.
This avoids an unnecessary map indirection but also gives us
access to the new fields in `per_peer_state` when reading
`Channel`s which we'll need in a coming commit.
Matt Corallo [Sat, 21 Sep 2024 04:23:09 +0000 (04:23 +0000)]
Avoid a `short_to_chan_info` read lock in `claim_funds_from_hop`
In 453ed11f80b40f28b6e95a74b1f7ed2cd7f012ad we started tracking the
counterparty's `node_id` in `HTLCPreviousHopData`, however we were
still trying to look it up using `prev_short_channel_id` in
`claim_funds_from_hop`.
Because we now usually have the counterparty's `node_id` directly
accessible, we should skip the `prev_short_channel_id` lookup.
This will also be more important in the next commit where we need
to look up state for our counterparty to generate
`ChannelMonitorUpdate`s whether we have a live channel or not.
Matt Corallo [Sun, 29 Sep 2024 19:30:48 +0000 (19:30 +0000)]
Add missing `update_maps_on_chan_removal` call in signer restore
When a channel is closed, we have to call
`update_maps_on_chan_removal` in the same per-peer-state lock as
the removal of the `ChannelPhase` object. We forgot to do so in
`ChannelManager::signer_unblocked` leaving dangling references to
the channel.
We also take this opportunity to include more context in the
channel-closure log in `ChannelManager::signer_unblocked` and add
documentation to `update_maps_on_chan_removal` and
`finish_close_channel` to hopefully avoid this issue in the future.
Matt Corallo [Sun, 29 Sep 2024 15:22:29 +0000 (15:22 +0000)]
Pass the `peer_state` lock through to `update_maps_on_chan_removal`
`update_maps_on_chan_removal` is used to perform `ChannelManager`
state updates when a channel is being removed, prior to dropping
the `peer_state` lock. In a future commit we'll use it to update
fields in the `per_peer_state`, but in order to do so we'll need to
have access to that state in the macro.
Here we get set up for this by passing the per-peer state to
`update_maps_on_chan_removal`, which is sadly a fairly large patch.
Matt Corallo [Tue, 5 Nov 2024 16:04:08 +0000 (16:04 +0000)]
Drop broken debug assertion on peer buffer lengths
1b711ed15f426ce40f1aa6be83d930a01ab530ba changed it so that we can
enqueue broadcast gossip messages to peers even though their buffer
is full as long as its our `ChannelMessageHandler` doing it. This
broke a debug assertion that the buffer isn't too large when
appending the broadcast message which is simply dropped here.
Jeffrey Czyz [Mon, 12 Aug 2024 21:54:55 +0000 (16:54 -0500)]
Parse experimental invoice TLV records
The BOLT12 spec defines an experimental TLV range that is allowed in
offer and invoice_request messages. The remaining TLV-space is for
experimental use in invoice messages. Allow this range when parsing an
invoice and include it when signing one.
Jeffrey Czyz [Thu, 8 Aug 2024 21:50:26 +0000 (16:50 -0500)]
Test verification with experimental invreq TLVs
Payer metadata is generated from the invreq TLVs and should included
those in the experimental range. When verifying invoice messages, these
TLVs must be included. Modify the BOLT12 verification tests to cover
them.
Jeffrey Czyz [Thu, 8 Aug 2024 16:44:03 +0000 (11:44 -0500)]
Parse experimental invreq TLV records
The BOLT12 spec defines an experimental TLV range that are allowed in
invoice_request messages. Allow this range when parsing an invoice
request and include those bytes in any invoice. Also include those bytes
when verifying that a Bolt12Invoice is for a valid InvoiceRequest.