1. Updated the Offers Test to check the reply paths in BOLT12 Invoices.
2. Changed the `extract_invoice` return type from `Option<BlindedMessagePath>`
to `BlindedMessagePath` since all BOLT12Invoices now have a corresponding
reply path by default.
1. Introduced reply_path in BOLT12Invoices to address a gap in error handling.
Previously, if a BOLT12Invoice sent in the offers flow generated an Invoice Error,
the payer had no way to send this error back to the payee.
2. By adding a reply_path to the Invoice Message, the payer can now communicate
any errors back to the payee, ensuring better error handling and communication
within the offers flow.
Introduce HMAC and nonce calculation when sending Invoice with
reply path, so that if we receive InvoiceError back for the
corresponding Invoice we can verify the payment hash before logging it.
- The trait defines the public method one may define for creating and
verifying the HMAC.
- Using a pub trait to define these method allows the flexibility for
other `OffersMessageHandler` construct to construct the HMAC and
authenticate the message.
When a InvoiceError is received for a sent BOLT12Invoice, the
corresponding PaymentHash is to be logged. Introduce hmac construction
and verification function for PaymentHash for this purpose.
With [1], it's possible to specify `manual_broadcast` for
the channel funding transaction. When `is_manual_broadcast` is
set to true, the transaction in the `DiscardFunding` event is
replaced with a dummy empty transaction.
This commit checks if `is_manual_broadcast` is true and
stores the funding OutPoint in the DiscardFunding event instead.
[1] https://github.com/lightningdevkit/rust-lightning/pull/3024
Link: https://github.com/lightningdevkit/rust-lightning/issues/3164
Suggested-by: TheBlueMatt
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Returning a reference from a trait method is relatively difficult
to map in bindings and is currently handled by storing the object
in the trait instance, returning a reference to the local field.
This is fine when the object we're returning only needs to live as
long as the trait, but when it needs to be `'static` (as is the
case for onion message `msg_type`s), there's not really a good way
to map them at all.
Instead, here, condition on `#[cfg(c_bindings)]` we return a fully
owned `String`. This is obviously relatively less effecient, but
the extra allocation and `memcpy` isn't the end of the world,
especially given it should be released relatively quickly.
Note that this breaks doctests in with `c_bindings`.
In order to allow onion message handlers to reply asynchronously
without introducing a circular dependency graph, the message
handlers need to be able to send replies described by
`MessageSendInstructions`. This allows them to send replies via the
normal message queuing (i.e. without making a function call to
`OnionMessenger`).
Here we enable that by adding a `MessageSendInstructions::ForReply`
variant which holds `ReplyInstruction`s.
Fixes#3178
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 next step towards untangling that, moving from
`PendingOnionMessage` to `MessageSendInstructions` for the outbound
message queue in `AsyncPaymentsMessageHandler`. Better, we can also
drop the `c_bindings`-specific message queue variant, unifying the
APIs.
Here we also drop `PendingOnionMessage` entirely.
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 next step towards untangling that, moving from
`PendingOnionMessage` to `MessageSendInstructions` for the outbound
message queue in `OffersMessageHandler`. Better, we can also drop
the `c_bindings`-specific message queue variant, unifying the APIs.
Because `ChannelManager` needs to actually control the reply path
set in individual messages, however, we have to halfway this patch,
adding a new `MessageSendInstructions` variant that allows
specifying the `reply_path` explicitly. Still, because other message
handlers are moving this way, its nice to be consistent.
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.
In the next commit we'll use `MessageSendInstructions` for all
messages, so it needs the ability to take any `Destination`, not
only a `Destination::Blinded`.
In the coming commits we'll use the `ResponseInstruction` enum's
contents for all messages, allowing message senders to rely on the
in-`OnionMessegner` reply path selection logic.
In order to do so and avoid users confusing the new
`MessageSendInstructions` for `ResponseInstruction`, we leave
`ResponseInstruction` as a now-unconstructible struct which wraps a
`MessageSendInstructions`.
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.
We never actually build with `#![no_std]` in tests as Rust does
not support it. Thus, many tests have spurious `std` feature gates
when they run just fine without them. Here we remove those gates,
though note that tests that depend on behavior elsewhere in the
codebase which is `std`-gated obviously need to retain their
feature gates.
Not sure why we ever really had this, no one really ever bounds
anything on `std::Error` and its kinda a dead type, so there's no
need for us to `impl` it for our types.
To handle `std` and `no-std` concepts of time in scoring, we'd
originally written a generic `Time` trait which we could use to
fetch the current time, observe real (not wall-clock) elapsed time,
and serialize the time.
Eventually, scoring stopped using this `Time` trait but outbound
payment retry expiry started using it instead to mock time in
tests.
Since scoring no longer uses the full features which required the
`Time` trait, we can substantially simplify by just having the
mocking option.
In 81389dee30 we removed a note about
mixing the `std` and `no-std` feature when de/serializing
`ProbabilisticScorer`s but forgot to note that there was a second
copy of that note in the module documentation.
This removes that note.
Instead of using separate iterators for pubkeys and TLVs, use an
iterator that yields a tuple of them. This allows for holding a mutable
reference to the TLVs such that they can be padded. With two iterators,
the pubkey iterator would have a reference to the ForwardNode slice when
constructing a payment path. However, this would prevent holding mutable
references in the TLVs iterator.
When constructing a blinded path, two iterators are used: one for the
pubkeys and another for Writeable TLVs. The first iterator is used in
the build_keys_helper utility function while the second is used inside
of a callback. Update this utility to work on any type that can be
borrowed as a PublicKey. This allows for using a single iterator of
tuples, which is necessary for padding the hops without additional
allocations and clones.
When constructing a BlindedPath, an Option<Destination> parameter to
construct_keys_callback is never passed. Make a separate utility
function that doesn't take this parameter. This allows for using the
build_keys_helper macro with a Iterator yielding items other than
PublicKey.
