Because we don't know which custom TLV type numbers the user is
expecting (and it would be cumbersome for them to tell us), instead of
failing unknown even custom TLVs on deserialization, we accept all
custom TLVs, and pass them to the user to check whether they recognize
them and choose to fail back if they don't. However, a user may not
check for custom TLVs, in which case we should reject any even custom
TLVs as unknown.
This commit makes sure a user must explicitly accept a payment with
even custom TLVs, by (1) making the default
`ChannelManager::claim_funds` fail if the payment had even custom TLVs
and (2) adding a new function
`ChannelManager::claim_funds_with_known_custom_tlvs` that accepts them.
This commit also refactors our custom TLVs test and updates various
documentation to account for this.
Upon receiving multiple payment parts with custom TLVs, we fail payments
if they have any non-matching or missing even TLVs, and otherwise just
drop non-matching TLVs if they're odd.
Custom TLVs allow users to send extra application-specific data with
a payment. These have the additional flexibility compared to
`payment_metadata` that they don't have to reflect recipient generated
data provided in an invoice, in which `payment_metadata` could be
reused.
We ensure provided type numbers are unique, increasing, and within the
experimental range with the `RecipientOnionFields::with_custom_tlvs`
method.
This begins sender-side support for custom TLVs.
b0d4ab8cf8 fixed a nasty bug where
we'd failed to include the payment preimage in keysend onions at
all. Ultimately, this was a test failure - the existing test suite
should which did keysend payments were not structured in a way that
would fail in this case, instead using the same preimage variable
both for sending and receiving.
Here we improve the main keysend test tweaked by b0d4ab8cf8
to make absolutely sure it cannot work if the preimage doesn't come
from the onion. We make the payment preimage on the sending side a
variable inside a scope which only exists for the send call. Once
that scope completes the payment preimage only exists in the
sending `ChannelManager`, which must have put it in the onion in
order for the receiving node to have it.
Fixes a bug where we wouldn't use the provided keysend preimage when
piping through OutboundPayment::pay_route_internal.
Also simplifies and refactors existing keysend tests to make sure this
gets hit.
Makes it easier to add new arguments without a ton of resulting test changes.
Useful for route blinding testing because we need to check for malformed HTLCs,
which is not currently supported by reconnect_nodes. It also makes it easier to
tell what is being checked in relevant tests.
Given we build `InFlightHtlcs` per route-fetch call, there's no
reason to pass them out by reference rather than simply giving the
user the full object. This also allows them to tweak the in-flight
set before fetching a route.
Useful for penultimate hops in routes to take an extra fee, if for example they
opened a JIT channel to the payee and want them to help bear the channel open
cost.
This is one of a series of commits to make sure methods are moved by
chunks so they are easily reviewable in diffs. Unfortunately they are
not purely move-only as fields need to be updated for things to
compile, but these should be quite clear.
This commit also uses the `context` field where needed for compilation
and tests to pass due to the above change.
This commit refactors a significant portion of the receive validation in
`ChannelManager::process_pending_htlc_forwards` now that we repurpose
previous MPP validation logic to accomodate keysends. This also removes
a previous restriction on claiming, as well as tests sending and
receiving MPP keysends.
Now that the `get_available_balances` min/max bounds are exact, we
can stop doing all the explicit checks in `send_htlc` entirely,
instead comparing against the `get_available_balances` bounds and
failing if the amount is out of those bounds.
This breaks support for sending amounts below the dust limit if
there is some amount of dust exposure remaining before we hit our
cap, however we will no longer generate such routes anyway.
This was a fairly old introduction to the spec to allow nodes to indicate
to their peers what chains they are interested in (i.e. will open channels
and gossip for).
We don't do any of the handling of this message in this commit and leave
that to the very next commit, so the behaviour is effectively the same
(ignore networks preference).
When we generated a `ChannelMonitorUpdate` during `ChannelManager`
deserialization, we must ensure that it gets processed before any
other `ChannelMonitorUpdate`s. The obvious hook for this is when
taking the `total_consistency_lock`, which makes it unlikely we'll
regress by forgetting this.
Here we add that call in the `PersistenceNotifierGuard`, with a
test-only atomic bool to test that this criteria is met.
This PR aims to create a "stateless" scorer. Instead of passing
in fee params at construction-time, we want to parametrize the
scorer with an associated "parameter" type, which is then
passed to the router function itself, and allows passing
different parameters per route-finding call.
`rust-bitcoin v0.30.0` introduces concrete variants for data members of
block `Header`s. To avoid having to update these across every use, we
introduce new helpers to create dummy blocks and headers, such that the
update process is a bit more straight-forward.
PaymentParameters already includes this value.
This set us up to better support route blinding, since there is no known
final_cltv_delta when paying to a blinded route.
While these transactions were still valid, we incorrectly assumed that
they would propagate with a locktime of `current_height + 1`, when in
reality, only those with a locktime strictly lower than the next height
in the chain are allowed to enter the mempool.
Previously, our local signatures would always be deterministic, whether
we'd grind for low R value signatures or not. For peers supporting
SegWit, Bitcoin Core will generally use a transaction's witness-txid, as
opposed to its txid, to advertise transactions. Therefore, to ensure a
transaction has the best chance to propagate across node mempools in the
network, each of its broadcast attempts should have a unique/distinct
witness-txid, which we can achieve by introducing random nonce data when
generating local signatures, such that they are no longer deterministic.
Untractable packages are those which cannot have their fees updated once
signed, hence why they weren't retried. There's no harm in retrying
these packages by simply re-broadcasting them though, as the fee market
could have spontaneously spiked when we first broadcast it, leading to
our transaction not propagating throughout node mempools unless
broadcast manually.
Now that we guarantee `claim_payment` will always succeed we have
to let the user know what the deadline is. We still fail payments
if they haven't been claimed in time, which we now expose in
`PaymentClaimable`.
While most lightning nodes don't (currently) support providing a
payment secret or payment metadata for spontaneous payments,
there's no specific technical reason why we shouldn't support
sending those fields to a recipient.
Further, when we eventually move to allowing custom TLV entries in
the recipient's onion TLV stream, we'll want to support it for
spontaneous payments as well.
Here we simply add the new `RecipientOnionFields` struct as an
argument to the spontaneous payment send methods. We don't yet
plumb it through the payment sending logic, which will come when we
plumb the new struct through the sending logic to replace the
existing payment secret arguments.
This moves the public payment sending API from passing an explicit
`PaymentSecret` to a new `RecipientOnionFields` struct (which
currently only contains the `PaymentSecret`). This gives us
substantial additional flexibility as we look at add both
`PaymentMetadata`, a new (well, year-or-two-old) BOLT11 invoice
extension to provide additional data sent to the recipient.
In the future, we should also add the ability to add custom TLV
entries in the `RecipientOnionFields` struct.
Previously, LDK would refuse to claim a payment if a channel on
which the payment was received had been closed between when the
HTLC was received and when we went to claim it. This makes sense in
the payment case - why pay an on-chain fee to claim the HTLC when
presumably the sender may retry later. Long ago it also reduced
total code in the claim pipeline.
However, this doesn't make sense if you're trying to do an atomic
swap or some other protocol that requires atomicity with some other
action - if your money got claimed elsewhere you need to be able to
claim the HTLC in lightning no matter what. Further, this is an
over-optimization - there should be a very, very low likelihood
that a channel closes between when we receive the last HTLC for a
payment and the user goes to claim the payment. Since we now have
code to handle this anyway we should allow it.
Fixes#2017.
This is largely motivated by some follow-up work for anchors that will
introduce an event handler for `BumpTransaction` events, which we can
now include in this new top-level `events` module.
This results in a new, potentially redundant, `ChannelMonitorUpdate`
that must be applied to `ChannelMonitor`s to broadcast the holder's
latest commitment transaction.
This is a behavior change for anchor channels since their commitments
may require additional fees to be attached through a child anchor
transaction. Recall that anchor transactions are only generated by the
event consumer after processing a `BumpTransactionEvent::ChannelClose`
event, which is yielded after applying a
`ChannelMonitorUpdateStep::ChannelForceClosed` monitor update. Assuming
the node operator is not watching the mempool to generate these anchor
transactions without LDK, an anchor channel which we had to fail when
deserializing our `ChannelManager` would have its commitment transaction
broadcast by itself, potentially exposing the node operator to loss of
funds if the commitment transaction's fee is not enough to be accepted
into the network's mempools.
