Useful for using the payment_id within to look up the corresponding outbound
async payment so we know we can safely release the HTLCs to the now-onlinen
recipient.
In order to maintain interface consistency, we refactor all message
handler interfaces to take `PublicKey` rather than `&PublicKey`, as the
difference in efficiency should be negigible and the former is easier to
handle in binding languages.
Over time, we also want to move (no pun intended) towards all messaging
interfaces using move semantics, so dropping the reference for
`PublicKey` is the first step in this direction.
When we force-close a channel, occasionally its due to feerate
disagreements or other non-HTLC-related issues. In those cases,
there's no reason to use a very urgent feerate estimate - we don't
have any timers expiring soon.
Instead, we should give users the information they need to be more
economical on fees in this case, which we do here by splitting
`OnChainSweep` into `UrgentOnChainSweep` and
`NonUrgentOnChainSweep` `ConfirmationTarget`s.
When we broke `ConfirmationTarget` out into task-specific names, we
left `MaxDustHTLCExposure::FeeRateMultiplier` as using the "when we
broadcast feerate" as we were mostly concerned about the dust
thresholds on outbound channels where we pick the fee and drive our
own funds to dust.
In 51bf78d604, that changed to
include transaction fees on both inbound and outbound channels in
our dust exposure amount, but we continued to use
`ConfirmationTarget::OnChainSweep` for the fee estimator threshold.
While the `MaxDustHTLCExposure::FeeRateMultiplier` value is quite
conservative and shouldn't lead to force-closures unless feerate
estimates disagree by something like 500 sat/vB (with only one HTLC
active in a channel), this happened on Aug 22 when feerates spiked
from 4 sat/vB to over 1000 sat/vB in one block.
To avoid simple feerate estimate horizons causing this in the
future, here we add a new
`ConfirmationTarget::MaximumFeeEstimate` which is used for dust
calculations. This allows users to split out the estimates they use
for checking counterparty feerates from the estimates used for
actual broadcasting.
Now that the `MessageRouter` can `create_blinded_paths` forcing
callers of the `OnionMessenger` to provide it with a reply path up
front is unnecessary complexity, doubly so in message handlers.
Here we take the first step towards untangling that, moving from
`PendingOnionMessage` to `MessageSendInstructions` for the outbound
message queue in `CustomMessageHandler`. Better, we can also drop
the `c_bindings`-specific message queue variant, unifying the APIs.
Our onion message handlers generally have one or more methods which
return a `ResponseInstruction` parameterized with the expected
message type (enum) of the message handler.
Sadly, that restriction is impossible to represent in our bindings -
our bindings concretize all LDK structs, enums, and traits into a
single concrete instance with generics set to our concrete trait
instances (which hold a jump table). This prevents us from having
multiple instances of `ResponseInstruction` structs for different
message types.
Our bindings do, however, support different parameterizations of
standard enums, including `Option`s and tuples.
In order to support bindings for the onion message handlers, we
are thus forced into std types bound by expected message types,
which we do here by making `ResponseInstruction` contain only the
instructions and generally using it as a two-tuple of
`(message, ResponseInstruction)`.
We currently have two structs with identical names in our public
API - `blinded_path::message::ForwardNode` and
`blinded_path::payment::ForwardNode`. This makes the API somewhat
awkward to use - users have to try (and fail) to import
`ForwardNode` twice only to then have to change their imports.
More importantly, however, this makes the API very hard to use in
some bindings languages where rename-imports or module imports
aren't available.
Thus, here, we rename both to give them context.
Referring to announced/unannounced channels as 'public'/'private'
regularly leads to confusion on what they are and when which should be
used. To avoid perpetuating this confusion, we should avoid referring to
announced channels as 'public' in our API.
Here we rename `ChannelDetails::is_public` (which breaks
previously-released API) to align it with the changes in
`OpenChannelRequest`.
This allow us to forward blinded payments where the blinded path that we are
forwarding within was concatenated to another blinded path that starts at the
next hop.
Also allows constructing blinded paths using this override.
Version 0.32.2 of `rust-bitcoin` deprecates a number of methods that
are commonly used in this project, most visibly `txid()`, which is
now called `compute_txid()`. This resulted in a lot of warnings, and
this commit is part of a series that seeks to address that.
Now that the `lightning` crate depends on the `lightning-invoice`
crate, there's no reason to have the `sign_invoice` method take raw
base32 field elements as we can now give it a real
`RawBolt11Invoice`, which we do here.
This simplifies the interface and avoids a
serialization-deserialization roundtrip when signing invoices in a
validating signer.
FIxes#3227
`lightning-invoice` currently has a dependency on the entire
`lightning` crate just because it wants to use some of the useful
types from it. This is obviously backwards and leads to some
awkwardness like the BOLT 11 invoice signing API in the `lightning`
crate taking a `[u5]` rather than a `Bolt11Invoice`.
This is the first step towards fixing that - moving the common
types we need into a new `lightning-types` crate which both can
depend on.
Since we're using a new crate and can't depend on the existing
`lightning` hex utility to implement `Display`, we also take this
opportunity to switch to the new `Display` impl macro in
`hex_conservative`.
Use the `hex-conservative` crate directly from `bitcoin` instead of from
`hashes`. Although it makes no real difference it is slightly more clear
and more terse.
The `hex` crate is re-exported by `rust-bitcoin` so we can get it from
there instead of explicitly depending on it. Doing so reduces the
maintenance burden and helps reduce the likelyhood of getting two
versions in the dependency graph.
The `rust-bitcoin` project is working towards making the public API
separate from the directory structure; eventually the
`bitcoin::blockdata` will go away, to make maintenance easier here stop
using the `blockdata` module.
Do not run the formatter, so as to make review easier. This patch was
created mechanically using:
search-and-replace bitcoin::blockdata bitcoin
and having defined
```bash
search-and-replace () {
if (($# != 2))
then
echo "Usage: $0 <this> <that>"
return
fi
local this="$1"
local that="$2"
for file in $(git grep -l "$this")
do
perl -pi -e "s/$this/$that/g" "$file"
done
}
```
`ChannelId` is just a 32-byte array, so there's not a lot of value
in passing it by reference to `funding_transaction_generated`,
which we fix here.
This is also nice for bindings as languages like Java can better
analyze whether the `ChannelManager` ends up with a reference to
the `ChannelId`.
Now that `ChannelManager::send_payment_with_route` is deprecated,
we don't care too much about making it as effecient as possible, so
there's not much cost to making it take `Route` by value. This
avoids bindings being unsure if the by-reference `Route` passed
needs to outlive the `ChannelManager` itself or if it only needs to
outlive the method call, creating some call overhead by forcing a
`Route::clone`, but avoiding a memory leak.
Now that ChannelManager uses a known OffersContext when creating blinded
paths, OffersContext::Unknown is no longer needed. Remove it and update
OffersMessageHandler to us an Option, which is more idiomatic for
signifying whether a message was delivered with or without an
OffersContext.
When creating a Bolt12Invoice in fuzz tests, use
BlindedPath::new_for_payment instead of BlindedPath::new_for_message.
This way PaymentContext is used instead of MessageContext, as is more
realistic though should not affect the test. This allows us to remove
OffersContext::Unknown.
- Enabled `create_blinded_paths` to accept `MessageContext` TLVs as
an input field.
- `MessageContext` is intended to be sent along with the `reply_path`
to the counterparty.
- Added `MessageContext` in the `create_blinded_paths` flow, optionally
appending it within the `reply_path`.
- Updated tests to verify the new feature.
1. Handling Offers Data:
- Updated `handle_message` to accept `OffersContext` data as an input field.
- If it is present, it will be utilized by the handler to
abandon outbound payments that have failed for any reason.
2. Consistency in Custom Message Handling:
- Updated `handle_custom_message` to accept optional custom data.
for consistency.
- Note: `custom_data` will remain unused in this PR.
When the `htlc_maximum_msat` field was made mandatory in
`ChannelUpdate` (in b0e8b739b7) we
started ignoring the `message_flags` field entirely and always
writing `1`. The comment updates indicated that the `message_flags`
field was deprecated, but this is not true - only the
`htlc_maximum_msat` indicator bit was deprecated, requiring it to
be 1.
If a node creates a `channel_update` with `message_flags` bits set
other than the low bit, this will cause us to spuriously reject
the message with an invalid signature error as we will check the
message against the wrong hash.
With today's current spec this is totally fine - the only other bit
defined for `message_flags` is the `dont_forward` bit, which when
set indicates we shouldn't accept the message into our gossip store
anyway (though this may lead to spurious `warning` messages being
sent to peers). However, in the future this may mean we start
rejecting valid `channel_update`s if new bits are defiend.
Add a trait for handling async payments messages to OnionMessenger. This allows
users to either provide their own custom handling for async payments messages
or rely on a version provided by LDK.
This change implements non-strict forwarding, allowing the node to
forward an HTLC along a channel other than the one specified
by short_channel_id in the onion message, so long as the receiver has
the same node public key intended by short_channel_id
([BOLT](57ce4b1e05/04-onion-routing.md (non-strict-forwarding))).
This can improve payment reliability when there are multiple channels
with the same peer e.g. when outbound liquidity is replenished by
opening a new channel.
The implemented forwarding strategy now chooses the channel with the
lowest outbound liquidity that can forward an HTLC to maximize the
probability of being able to successfully forward a subsequent HTLC.
Fixes#1278.
