These claims will never be valid as a previous claim has already
confirmed. If a previous claim is reorged out of the chain, a new claim
will be generated bypassing the new behavior.
While this doesn't change much for our existing transaction-based
claims, as broadcasting an already confirmed transaction acts as a NOP,
it prevents us from yielding redundant event-based claims, which will be
introduced as part of the anchors patchset.
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.
When the `abandon_payment` flow was added there was some concern
that upgrading users may not migrate to the new flow, causing
memory leaks in the pending-payment tracking.
While this is true, now that we're relying on the
pending_outbound_payments map for `send_payment` idempotency, the
risk of removing a payment prematurely goes up from "spurious
retry failure" to "sending a duplicative payment", which is much
worse.
Thus, we simply remove the automated payment timeout here,
explicitly requiring that users call `abandon_payment` when they
give up retrying a payment.
Previously, once a fulfilled outbound payment completed and all
associated HTLCs were resolved, we'd immediately remove the payment
entry from the `pending_outbound_payments` map.
Now that we're using the `pending_outbound_payments` map for send
idempotency, this presents a race condition - if the user makes a
redundant `send_payment` call at the same time that the original
payment's last HTLC is resolved, the user would reasonably expect
the `send_payment` call to fail due to our idempotency guarantees.
However, because the `pending_outbound_payments` entry is being
removed, if it completes first the `send_payment` call will
succeed even though the user has not had a chance to see the
corresponding `Event::PaymentSent`.
Instead, here, we delay removal of `Fulfilled`
`pending_outbound_payments` entries until several timer ticks have
passed without any corresponding event or HTLC pending.
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.
If we're sending straight to a blinded route with no unblinded intermediate
hops, and we are the introduction node, we need to advance the blinded route by
one hop so that the second hop is the new introduction node.
Because we now never generate an `EffectiveCapacity` with an
`htlc_maximum_msat` set to `None`, making it non-`Option`al
effectively removes dead code, which we do here.
We currently construct `DirectedChannelInfo`s for routing before
checking if the given direction has its directional info filled in.
We then always check for directional info before actually deciding
to route over a channel, as otherwise we assume the channel is not
online.
This makes for somewhat redundant checks, and `DirectedCHannelInfo`
isn't, by itself, a very useful API. Because fetching the HTLC-max
or effective channel capacity gives spurious data if no directional
info is available, there's little reason to have that data
available, and so we here check for directional info first. This
effectively merges `DirectionalChannelInfo` and
`DirectionalChannelInfoWithUpdate`.
+ remove MaybeReadableArgs trait as it is now unused
+ remove onion_utils::DecodeInput as it would've now needed to be parameterized
by the CustomOnionMessageHandler trait, and we'd like to avoid either
implementing DecodeInput in messenger or having onion_utils depend on
onion_message::*
Co-authored-by: Matt Corallo <git@bluematt.me>
Co-authored-by: Valentine Wallace <vwallace@protonmail.com>
The bindings have exported `io::Error` as, basically,
`io::ErrorKind`, for quite some time, so there's little reason to
not just export `io::ErrorKind` as well.
Useful since we're working on getting rid of KeysInterface::get_node_secret to
complete support for remote signing.
Will be used in upcoming work to check whether an outbound onion message
blinded path has our node id as the introduction node id
In 56b07e52aa we made
`MultiThreadedLockableScore` fully bindings-compatible. However, it
did not add a `WriteableScore` implementation for it. This was an
oversight as it is a `WriteableScore` in Rust and needs to be for
use in other parts of the API.
Here we add the required impl in a way that the bindings generator
is able to handle it and add conversion utilities.
This uses the work done in the preceding commits to implement encoding a user's
custom TLV in outbound onion messages, and decoding custom TLVs in inbound
onion messages, to be provided to the new CustomOnionMessageHandler.
