When we landed the initial in-`ChannelManager` payment retries, we
stored the `RouteParameters` in the payment info, and then re-use
it as-is for new payments. `RouteParameters` is intended to store
the information specific to the *route*, `PaymentParameters` exists
to store information specific to a payment.
Worse, because we don't recalculate the amount stored in the
`RouteParameters` before fetching a new route with it, we end up
attempting to retry the full payment amount, rather than only the
failed part.
This issue brought to you by having redundant data in
datastructures, part 5,001.
The documentation for `Retry` is very clear that it counts the
number of failed paths, not discrete retries. When we added
retries internally in `ChannelManager`, we switched to counting
the number if discrete retries, even if multiple paths failed and
were replace with multiple MPP HTLCs.
Because we are now rewriting retries, we take this opportunity to
reduce the places where retries are analyzed, specifically a good
chunk of code is removed from `pay_internal`.
Because we now retry multiple failed paths with one single retry,
we keep the new behavior, updating the docs on `Retry` to describe
the new behavior.
`PaymentParams` is all about the parameters for a payment, i.e. the
parameters which are static across all the paths of a paymet.
`RouteParameters` is about the information specific to a given
`Route` (i.e. a set of paths, among multiple potential sets of
paths for a payment). The CLTV delta thus doesn't belong in
`RouterParameters` but instead in `PaymentParameters`.
Worse, because `RouteParameters` is built from the information in
the last hops of a `Route`, when we deliberately inflate the CLTV
delta in path-finding, retries of the payment will have the final
CLTV delta double-inflated as it inflates starting from the final
CLTV delta used in the last attempt.
When we calculate the `final_cltv_expiry_delta` to put in the
`RouteParameters` returned via events after a payment failure, we
should re-use the new one in the `PaymentParameters`, rather than
the one that was in the route itself.
`PaymentParams` is all about the parameters for a payment, i.e. the
parameters which are static across all the paths of a paymet.
`RouteParameters` is about the information specific to a given
`Route` (i.e. a set of paths, among multiple potential sets of
paths for a payment). The CLTV delta thus doesn't belong in
`RouterParameters` but instead in `PaymentParameters`.
Worse, because `RouteParameters` is built from the information in
the last hops of a `Route`, when we deliberately inflate the CLTV
delta in path-finding, retries of the payment will have the final
CLTV delta double-inflated as it inflates starting from the final
CLTV delta used in the last attempt.
By moving the CLTV delta to `PaymentParameters` we avoid this
issue, leaving only the sought amount in the `RouteParameters`.
As of now the `Confirm::get_relevant_txids()` docs state that it won't
return any transactions for which we hadn't previously seen a
confirmation. To align its functionality a bit more with the docs, at
least for `ChannelManager`, we only return values for which we had
registered a confirmation block hash before.
FailureCode is used to specify which error code and data to send
to peers when failing back an HTLC.
ChannelManager::fail_htlc_backwards_with_reason
allows a user to specify the error code and
corresponding data to send to peers when failing back an HTLC.
This function is mentioned in Event::PaymentClaimable docs.
ChannelManager::get_htlc_fail_reason_from_failure_code was also
added to assist with this function.
Adds two new payment `Method`s for identifying payments with custom
`min_final_cltv_expiry_delta` as payments with LDK or user payment
hashes.
The `min_final_cltv_expiry_delta` value is packed into the first 2
bytes of the expiry timestamp in the payment secret metadata.
All utility functions for invoice construction will now also accept an
Option<>al `min_final_cltv_expiry_delta` which is useful for things like
swaps etc. The `min_final_cltv_expiry_delta` will default back to
`MIN_FINAL_CLTV_EXPIRY_DELTA` if `None` is provided.
This matches the spec and helps avoid any confusion around
naming. We're also then consistent with `cltv_expiry` in an HTLC being
the actual block height value for the CLTV and not a delta.
Secrets should not be exposed in-memory at the interface level as it
would be impossible the implement it against a hardware security
module/secure element.
We have a number of debug assertions which are expected to never
fire when running in a single thread. This is just fine in tests,
and gives us good coverage of our lockorder requirements, but is
not-irregularly surprising to users, who may run with their own
debug assertions in test environments.
Instead, we gate these checks by the `cfg(test)` setting as well as
the `_test_utils` feature, ensuring they run in our own tests, but
not downstream tests.
In the next commit(s) we'll start holding `ChannelMonitorUpdate`s
that are being persisted in `Channel`s until they're done
persisting. In order to do that, switch to applying the updates by
reference instead of value.
This fixes a crash in the `full_stack_target` fuzz test (found by
Chaincode's generous fuzzing infrastructure!) but ultimately is a
better error code - a peer disconnecting before we can fund a
channel isn't a "misuse error" its an unavailable channel.
In newer versions of `hashbrown` this code would be broken. While
we aren't updating `hashbrown` any time soon (as it requires an
MSRV bump), it is useful to swap for a newer `hashbrown` when
fuzzing, which this makes easier.
This is purely a refactor that does not change the InitFeatures
advertised by a ChannelManager. This allows users to configure which
features should be advertised based on the values of `UserConfig`. While
there aren't any existing features currently leveraging this behavior,
it will be used by the upcoming anchors_zero_fee_htlc_tx feature.
The UserConfig dependency on provided_init_features caused most
callsites of the main test methods responsible for opening channels to
be updated. This commit foregos that completely by no longer requiring
the InitFeatures of each side to be provided to these methods. The
methods already require a reference to each node's ChannelManager to
open the channel, so we use that same reference to obtain their
InitFeatures. A way to override such features was required for some
tests, so a new `override_init_features` config option now exists on
the test harness.
