Alec Chen [Thu, 8 Jun 2023 17:08:25 +0000 (12:08 -0500)]
Enforce explicit claims on payments with even custom TLVs
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.
Alec Chen [Thu, 8 Jun 2023 04:17:09 +0000 (23:17 -0500)]
Add `FailureCode::InvalidOnionPayload` variant
When a user decodes custom TLVs, if they fail to recognize even type
numbers they should fail back with the correct failure code and fail
data. This new variant adds the proper failure variant for the user to
pass into `ChannelManager::fail_htlc_backwards_with_reason`.
Note that the enum discriminants were removed because when adding a
struct variant we can no longer make use of the discriminant through
casting like we previously did, and instead have to manually define the
associated failure code anyway.
Alec Chen [Fri, 19 May 2023 20:37:47 +0000 (15:37 -0500)]
Drop non-matching custom TLVs when receiving MPP
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.
Alec Chen [Fri, 19 May 2023 04:16:29 +0000 (23:16 -0500)]
De/serialize custom TLVs on `{Inbound,Outbound}OnionPayload`
When serialized, the TLVs in `OutboundOnionPayload`, unlike a normal
TLV stream, are prefixed with the length of the stream. To allow a user
to add arbitrary custom TLVs, we aren't able to communicate to our
serialization macros exactly which fields to expect, so this commit
adds new macro variants to allow appending an extra set of bytes (and
modifying the prefixed length accordingly).
Because the keysend preimage TLV has a type number in the custom type
range, and a user's TLVs may have type numbers above and/or below
keysend's type number, and because TLV streams must be serialized in
increasing order by type number, this commit also ensures the keysend
TLV is properly sorted/serialized amongst the custom TLVs.
Alec Chen [Tue, 16 May 2023 22:56:28 +0000 (17:56 -0500)]
Allow users to provide custom TLVs through `RecipientOnionFields`
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.
Receive payment onions as new InboundPayload instead of OnionHopData
To support route blinding, we want to split OnionHopData into two separate
structs, one for inbound onions and one for outbound onions. This is because
blinded payloads change the fields present in the onion hop data struct based
on whether we're sending vs receiving (outbound onions include encrypted blobs,
inbound onions can decrypt those blobs and contain the decrypted fields
themselves).
In upcoming commits, we'll add variants for blinded payloads to the new
InboundPayload enum.
Matt Corallo [Sun, 30 Jul 2023 17:10:38 +0000 (17:10 +0000)]
Drop unicode in documentation
Javadocs refuse unicode and as our rustdocs get copied over to Java
bindings (and thus get run through javadocs) we can't have unicode
in our rustdocs.
Matt Corallo [Fri, 28 Jul 2023 06:20:43 +0000 (06:20 +0000)]
Drop `claimable` from `Balance::claimable_amount_satoshis` fields
In Java/TypeScript, we map enums as a base class and each variant
as a class which extends the base. In Java/TypeScript, functions
and fields share the same namespace, which means we cannot have
functions on an enum which have the same name as any fields in any
enum variants.
`Balance`'s `claimable_amount_satoshis` method aliases with fields
in each variant, and thus ultimately doesn't compile in TypeScript.
Because `Balance::claimable_amount_satoshis` has the same name as
the fields, it's also a bit confusing, as it doesn't return the
field for each variant, but sometimes returns zero if we're not
sure we can claim the balance.
Instead, we rename the fields in each enum variant to simply
`amount_satoshis`, to avoid implying that we can definitely claim
the balance.
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.
benthecarman [Fri, 2 Jun 2023 16:27:51 +0000 (11:27 -0500)]
Impl clone for ChannelMonitor
This gives people more freedom with the channel monitors. For Mutiny
this would be nice for us to be able to create copies of them and pass
aorund in memory without having to serialize until we actually want to.
Originally by benthecarman <benthecarman@live.com>
Small bugfix from Matt Corallo <git@bluematt.me>
Matt Corallo [Mon, 24 Jul 2023 22:03:15 +0000 (22:03 +0000)]
Run all tests first before testing more esoteric flags in CI
This should at least marginally more aggressively target things
which are more likely to have changed in CI, making `ci-tests.sh`
more useful as a "default" script for developers to run locally.
While this is generally uneccessary as users set the `no-std` or
`std` features on the `lightning` crate directly, having this
allows `lightning-background-processor` to be built by itself
without extra dep lines. Specifically, the bindings are moving to
using the `-Z avoid-dev-deps` option, which now causes
`lightning-background-processor` to fail to build directly.
Elias Rohrer [Tue, 18 Jul 2023 11:23:20 +0000 (13:23 +0200)]
Improve logging for ignored candiate hops
Previously, we barely gave any hints why we excluded certain hops during
pathfinding. Here, we introduce more verbose logging by a) accounting
how much candidates we ignored for which reasons and b) logging any
first/last/blinded hops we end up ignoring.
Elias Rohrer [Tue, 18 Jul 2023 10:41:07 +0000 (12:41 +0200)]
Update outdated `router.rs` docs
As `RouteParameters` are not included anymore in
`Event::PaymentPathFailed` since 0.0.115, and we don't give value/payee
as immediate arguments to `find_route` anymore.
Elias Rohrer [Sat, 17 Jun 2023 11:16:52 +0000 (13:16 +0200)]
Avoid panic when 0conf channel's ann. sigs race on-chain confirmation
A channel's `short_channel_id` is currently only set when the funding
transaction is confirmed via `transactions_confirmed`, which might be
well after the channel initally becomes usable, e.g., in the 0conf case.
Previously we would panic due to a reachable `unwrap` when receiving a
counterparty's `announcement_signatures` message for a 0conf channel
pending confirmation on-chain.
Here we fix this bug by avoiding unsafe `unwrap`s and just erroring out
and ignoring the announcement_signatures message if the `short_channel_id`
hasn't been set yet.
Matt Corallo [Wed, 19 Jul 2023 20:11:35 +0000 (20:11 +0000)]
Make `requires_unknown_bits_from` param type bindings compatible
While bindings should probably be able to figure out that this is
the same type as `Self`, for now we simply swap the type to make
the bindings generator have an easier go of it.
Matt Corallo [Wed, 19 Jul 2023 20:04:24 +0000 (20:04 +0000)]
Mark the `Duration` serialization implementation no-bindings-export
The bindings are being updated to consider all traits even if the
trait itself is no-export, which causes issues generating code
around the `Duration` impl here.
Matt Corallo [Wed, 19 Jul 2023 20:02:10 +0000 (20:02 +0000)]
No-export un-exportable types in BOLT12 module
We missed one method that now cannot be bindings exported - the
`payment_paths` method, as it returns a slice of objects, which
cannot be supported in bindings.
Matt Corallo [Wed, 19 Jul 2023 19:37:21 +0000 (19:37 +0000)]
Tweak PSBT signing for bindings compatibility
In bindings we can't practically pass a mutable PSBT, and instead
need to pass an owned transaction and have the sign method return a
signed copy. We do this here for all build modes as its not a
material API change for Rust users.
Matt Corallo [Tue, 18 Jul 2023 19:52:26 +0000 (19:52 +0000)]
Drop `c_bindings` implementation of scoring on `Mutex`/`RefCell`
This code was always effectively dead - we have a special
`MultiThreadedLockableScore` type which wraps a `Mutex` for
bindings users, so there's no need to implement any
bindings-specific scoring logic for them.
Matt Corallo [Tue, 18 Jul 2023 19:41:07 +0000 (19:41 +0000)]
Pass `InFlightHltcs` to the scorer by ownership rather than ref
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.
Matt Corallo [Tue, 18 Jul 2023 19:34:00 +0000 (19:34 +0000)]
Pass must-spend inputs to users by ownership
We already hold them in a vec, so there's no cost to passing them
by ownership vs making it a slice. Further, this helps bindings as
we can't represent slices to non-pointers in a sensible way.
Matt Corallo [Tue, 18 Jul 2023 19:13:19 +0000 (19:13 +0000)]
Tweak transaction bumping `sign_tx` types for bindings
In bindings we can't practically pass a mutable transaction, and
instead need to pass an owned transaction and have the sign method
return a signed copy. We do this here for all build modes as the
API is roughly equivalent also to Rust users.
Duncan Dean [Fri, 14 Jul 2023 14:59:29 +0000 (16:59 +0200)]
Close and remove unfunded inbound/outbound channels that are older than an hour
We introduce a `UnfundedChannelContext` which contains a counter for the
current age of an unfunded channel in timer ticks. This age is incremented
for every `ChannelManager::timer_tick_ocurred` and the unfunded channel
is removed if it exceeds `UNFUNDED_CHANNEL_AGE_LIMIT_TICKS`.
The value will not be persisted as unfunded channels themselves are not
persisted.
Duncan Dean [Mon, 26 Jun 2023 14:06:50 +0000 (10:06 -0400)]
Add missing unfunded channel maps checks in `ChannelManager`
One of a series of follow-up commits to address some issues found
in PR 2077, where we split channels up into different maps and structs
depending on phase in their life.
Duncan Dean [Mon, 17 Jul 2023 13:52:44 +0000 (15:52 +0200)]
Refer to channels busy with funding tx negotiation as "unfunded"
We had some inconsistencies so far in referring to channels such as
`OutboundV1Channel` and `InboundV1Channel` as pending and unfunded.
From here we refer to these kinds of channels only as "unfunded".
This is a slight conflation with the term "unfunded" in the contexts
of denial of service mitigation. There, "unfunded" actually refers to
non-0conf, inbound channels that have not had their funding transaction
confirmed. This might warrant changing that usage to "unconfirmed inbound".
Matt Corallo [Tue, 18 Jul 2023 19:04:56 +0000 (19:04 +0000)]
Tweak generics on `derive_channel_signer` to be bindings-compatible
The C bindings generation currently has issues looking through a
generic associated type. While this should be fixed in the bindings
generator, its easy to fix here for now and we can revisit it
later.
Matt Corallo [Mon, 17 Jul 2023 21:01:02 +0000 (21:01 +0000)]
Drop `tokio/macros` dependency in `lightning-net-tokio`, fix MSRV
The `tokio` `macros` feature depends on `proc-macro2`, which
recently broke its MSRV in a patch version. Such crates aren't
reasonable for us to have as dependencies, so instead we replace
the one trivial use we have of `tokio::select!()` with our own
manual future.
The `RefCell` was necessary in a previous iteration of the code in which
the iterator was not `Clone` so we needed interior mutability in order
to consume the iterator. Now that it is `Clone`, we can drop it, as
we're no longer mutating the original iterator.
Use min mempool feerate for outbound updates on anchor channels
As done with inbound feerate updates, we can afford to commit less in
fees, as long as we still may the minimum mempool feerate. This enables
users to spend a bit more of their balance, as less funds are being
committed to transaction fees.
Relax constraints for inbound feerate updates on anchor channels
Channels supporting anchors outputs no longer require their feerate
updates to target a prompt confirmation since commitment transactions
can be bumped when broadcasting. Commitment transactions must now at
least meet the minimum mempool feerate, until package relay is deployed,
such that they can propagate across node mempools in the network by
themselves.
The existing higher bound no longer applies to channels supporting
anchor outputs since their HTLC transactions don't have any fees
committed, which directly impact the available balance users can send.
Add new ConfirmationTarget variant for min mempool feerates
Now that we support channels with anchor outputs, we add a new
ConfirmationTarget variant that, for now, will only apply to such
channels. This new variant should target estimating the minimum feerate
required to be accepted into most node mempools across the network.
Consider existing commitment transaction feerate when bumping
With anchors, we've yet to change the frequency or aggressiveness of
feerate updates, so it's likely that commitment transactions have a
good enough feerate to confirm on its own. In any case, when producing a
child anchor transaction, we should already take into account the fees
paid by the commitment transaction itself, allowing the user to save
some satoshis. Unfortunately, in its current form, this will still
result in overpaying by a small margin at the expense of making the coin
selection API more complex.
Avoid yielding ChannelClose bump events with sufficient feerate
There's no need to yield such an event when the commitment transaction
already meets the target feerate on its own, so we can simply broadcast
it without an anchor child transaction. This may be a common occurrence
until we are less aggressive about feerate updates.