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.
`ScorerAccountingForInFlightHtlcs` generally stores a `Score`
reference generated by calling `LockableScore::lock`, which
actually returns an arbitrary `Score`. Given `Score` is implemented
directly on lock types, it makes sense to simply hold a fully owned
`Score` in `ScorerAccountingForInFlightHtlcs` rather than a mutable
reference to one.
In c70bd1f, we implemented tracking HTLCs by adding path information
for pending HTLCs to `InvoicePayer`’s `payment_cache` when receiving
specific events.
Since we can now track inflight HTLCs entirely within ChannelManager,
there is no longer a need for this to exist.
We introduce a new sealed trait BaseEventHandler that has a blanket
implementation for any T. Since the trait cannot be implemented outside
of the crate, this allow us to expose specific implementations of
InvoicePayer that allow for synchronous and asynchronous event handling.
When a user attempts to send a payment but it fails due to
idempotency key violation, they need to know that this was the
reason as they need to handle the error programmatically
differently from other errors.
Here we simply add a new `PaymentSendFailure` enum variant for
`DuplicatePayment` to allow for that.
It was pointed out that its quite confusing that
`AllFailedRetrySafe` does not allow you to call `retry_payment`,
though the documentation on it does specify this. Instead, we
simply rename it to `AllFailedResendSafe` to indicate that the
action that is safe to take is *resending*, not *retrying*.
This is part of moving the Router trait into ChannelManager, which will help
allow ChannelManager to fetch routes on-the-fly as part of supporting
trampoline payments.
In order to allow users to pass a custom idempotency key to the
`send*` methods in `InvoicePayer`, we have to pipe the `PaymentId`
through to the `Payer` methods, which we do here.
By default, existing `InvoicePayer` methods use the `PaymentHash`
as the `PaymentId`, however we also add duplicate `send*_with_id`
methods which allow users to pass a custom `PaymentId`.
Finally, appropriate documentation updates are made to clarify
idempotency guarantees.
In c986e52ce8, an `MppId` was added
to `HTLCSource` objects as a way of correlating HTLCs which belong
to the same payment when the `ChannelManager` sees an HTLC
succeed/fail. This allows it to have awareness of the state of all
HTLCs in a payment when it generates the ultimate user-facing
payment success/failure events. This was used in the same PR to
avoid generating duplicative success/failure events for a single
payment.
Because the field was only used as an internal token to correlate
HTLCs, and retries were not supported, it was generated randomly by
calling the `KeysInterface`'s 32-byte random-fetching function.
This also provided a backwards-compatibility story as the existing
HTLC randomization key was re-used for older clients.
In 28eea12bbe `MppId` was renamed to
the current `PaymentId` which was then used expose the
`retry_payment` interface, allowing users to send new HTLCs which
are considered a part of an existing payment.
At no point has the payment-sending API seriously considered
idempotency, a major drawback which leaves the API unsafe in most
deployments. Luckily, there is a simple solution - because the
`PaymentId` must be unique, and because payment information for a
given payment is held for several blocks after a payment
completes/fails, it represents an obvious idempotency token.
Here we simply require the user provide the `PaymentId` directly in
`send_payment`, allowing them to use whatever token they may
already have for a payment's idempotency token.
Mobile clients often take a second or two before they reconnect to
their peers as its not always clear immediately that connections
have been killed (especially on iOS). This can cause us to
spuriously fail to include route hints in our invoices if we're on
mobile.
The fix is simple, if we're selecting channels to include in route
hints and we're not not connected to the peer for any of our
channels, we should simply include the hints for all channels, even
though we're disconencted.
Fixes#1768.
We had a user who was confused why an invoice failed to round-trip
(i.e. was not `PartialEq` with itself after write/read) when a
subsecond creation time was used (e.g. via the `from_system_time`
constructor).
This commit should address this confusion by dropping subsecond
parts in easily-accessible constructors when creating BOLT 11
invoices.
Fixes#1759.
