Matt Corallo [Sun, 5 Mar 2023 20:38:42 +0000 (20:38 +0000)]
Avoid enums containing references with lifetimes
Having struct fields with references to other structs is tough in
our bindings logic, but even worse if the fields are in an enum.
Its simplest to just take the clone penalty here.
Matt Corallo [Tue, 28 Feb 2023 21:45:14 +0000 (21:45 +0000)]
Export `outbound_payment` structs in their respective modules
Re-exports in Rust make `use` statements a little shorter, but for
otherwise don't materially change a crate's API. Sadly, the C
bindings generator currently can't figure out re-exports, but it
also exports everything into one global namespace, so it doesn't
matter much anyway.
Matt Corallo [Tue, 23 Jan 2024 19:55:24 +0000 (19:55 +0000)]
Stop relying on a `Clone`able `NetworkGraph` ref in `DefaultRouter`
While there's not really much harm in requiring a `Clone`able
reference (they almost always are), it does make our bindings
struggle a bit as they don't support multi-trait bounds (as it
would require synthesizing a new C trait, which the bindings don't
do automatically). Luckily, there's really no reason for it, and we
can just call the `DefaultMessageRouter` directly when we want to
route a message.
Matt Corallo [Mon, 13 May 2024 14:36:01 +0000 (14:36 +0000)]
[UPSTREAM] impl `Clone` on unsigned BOLT12 types
This is required for bindings as passing types from Rust to GC'd
languages can't map the concept of a type that has a lifetime of
the called function but instead needs to clone for safety.
Matt Corallo [Sat, 11 May 2024 18:51:48 +0000 (18:51 +0000)]
[UPSTREAM] Make `offers::Amount` `Copy` and export it in bindings
`Amount` is less than two pointers long, so there's no reason it
shouldn't be `Copy`. Further, because its an enum, bindings can't
map a reference to it in an `Option`. Thus, here, we simply make it
`Copy` and return it in `Option`s rather than a reference to it.
Matt Corallo [Tue, 30 Apr 2024 17:11:54 +0000 (17:11 +0000)]
Use consistent byte/char offsets when parsing invoice HRPs
When parsing lightning-invoice HRPs we want to read them
char-by-char, tracking at which offset different fields were. Prior
to this commit this was done first by reading char-by-char and then
by indexing using the byte offset which works for ASCII strings but
fails on multi-byte characters.
This commit fixes this issue by simply always walking byte-by-byte
and rejecting multi-byte characters which don't belong in HRPs.
Matt Corallo [Tue, 7 May 2024 16:34:36 +0000 (16:34 +0000)]
Fix the `full_stack_target` breakage test and doc feerate requests
When we added the additional deust exposure checks in 702196819e6445048b803574fcacef77d5ce8c9c we added several
additional feerate fetches which broke the `full_stack_target`
change-detection test.
This updates the hard-coded test to support the new feerate fetches
and also includes a comment on `FeeEstimator` to indicate that
users really need to be caching feerates as otherwise they'll slow
us down.
Matt Corallo [Sun, 5 May 2024 00:44:07 +0000 (00:44 +0000)]
Update default dust exposure limit and the documentation thereof
Now that we're including excess counterparty commitment transaction
fees in our dust calculation, we need to update the docs
accordingly. We do so here, describing some of the considerations
and risks that come with the new changes.
We also take this opportunity to double the default value, as users
have regularly complained that non-anchor channels fail to send
HTLCs with the default settings with some feerates.
Matt Corallo [Tue, 30 Apr 2024 20:42:36 +0000 (20:42 +0000)]
Include excess commitment transaction fees in dust exposure
Transaction fees on counterparty commitment transactions are
ultimately not our money and thus are really "dust" from our PoV -
they're funds that may be ours during off-chain updates but are not
ours once we go on-chain.
Thus, here, we count any such fees in excess of our own fee
estimates towards dust exposure. We don't bother to make an
inbound/outbound channel distinction here as in most cases users
will use `MaxDustExposure::FeeRateMultiplier` which will scale
with the fee we set on outbound channels anyway.
Note that this also enables the dust exposure checks on anchor
channels during feerate updates. We'd previously elided these as
increases in the channel feerates do not change the HTLC dust
exposure, but now do for the fee dust exposure.
Jeffrey Czyz [Thu, 2 May 2024 16:43:04 +0000 (11:43 -0500)]
Only require description when offer has an amount
The spec was changed to allow excluding an offer description if the
offer doesn't have an amount. However, it is still required when the
amount is set.
Matt Corallo [Wed, 1 May 2024 21:56:04 +0000 (21:56 +0000)]
Swap `UserConfig::default()` for `test_default_channel_config`
As LDK changes, `UserConfig::default()` may imply marginally
different behavior, whereas `test_default_channel_config` is
intended to tweak defaults to provide a stable behavior for test
contexts.
This commit changes a few uses of `UserConfig::default()` to
`test_default_channel_config` in cases that will fail over the
coming commits due to dust changes.
Matt Corallo [Wed, 27 Mar 2024 02:14:50 +0000 (02:14 +0000)]
Unify `get_{inbound,outbound}_pending_htlc_stats`
In most cases we already call both in a pair, and in fact always
consolidate some of the returned values across both accessors, so
there's not much reason to have them be separate methods.
Matt Corallo [Wed, 1 May 2024 14:32:47 +0000 (14:32 +0000)]
Don't attempt to resume channels if we already exchanged funding
ddf75afd16 introduced the ability to re-exchange our `ChannelOpen`
after a peer disconnects if we didn't complete funding on our end.
It did not implement nor consider what would happen if we
re-connected after we created our own funding transactions, and
currently it panics (and even if it did not it would replay the
`FundingTransactionGenerated` event to users).
This addresses this in a temporary fashion to fix the immediate
panic for 0.0.123, allowing us to fix the issue more fully later.
Matt Corallo [Tue, 30 Apr 2024 21:30:26 +0000 (21:30 +0000)]
Wake `background-processor` from `ChainMonitor` on new blocks
When we receive a new block we may generate
`Event::SpendableOutputs` in `ChannelMonitor`s which then need to
be processed by the background processor. While it will do so
eventually when its normal loop goes around, this may cause user
tests to be delayed in finding events, so we should notify the BP
immediately to wake it on new blocks.
We implement that here, unconditionally notifying the
`background-processor` whenever we receive a new block or confirmed
transactions.
Matt Corallo [Wed, 1 May 2024 15:49:59 +0000 (15:49 +0000)]
Move `[u8; 32]` wrapper types to a common module
The `PaymentHash`, `PaymentSecret`, `PaymentPreimage`, and
`ChannelId` types are all small wrappers around `[u8; 32]` and are
used throughout the codebase but were defined in the top-level
`ln/mod.rs` file and the relatively sparsely-populated
`ln/channel_id.rs` file.
Here we move them to a common `types` module and go ahead and
update all our in-crate `use` statements to refer to the new
module for bindings. We do, however, leave a `pub use` alias for
the old paths to avoid upgrade hassle for users.
Matt Corallo [Wed, 1 May 2024 15:19:20 +0000 (15:19 +0000)]
Add bindings no-export tags to doc test types
We don't actually intend these to be public as they're just for
docs but the bindings don't currently parse `#[doc(hidden)]` as
"no-export" so we add manual no-export tags as well.
Matt Corallo [Tue, 23 Apr 2024 15:56:13 +0000 (15:56 +0000)]
Rename and expose message-specific `NextHop`
`onion::message::messenger::PeeledOnion` is a public enum which
included the private enum `NextHop`, which is not acceptable. Thus,
we here expose `NextHop` but rename it `NextMessageHop` to make
clear that it is specific to messages.
Matt Corallo [Tue, 30 Apr 2024 13:45:45 +0000 (13:45 +0000)]
Fix new rustc `#[macro_export]` warning in doctests
rustc now warns any time a `#[macro_export]` is used inside a
function as it generates surprising results. Because doctests are
run inside implicit test functions this means any use of
`#[macro_export]` inside a doctest will now warn. We do this in
`lightning-custom-message` to demonstrated how to use the crate,
which now fails to compile.
Here we fix this by adding an `fn main() {}` to the doctest, which
causes doctests to be compiled as freestanding code rather than
inside a test function.
Note that while discussing this upstream it came up that rustc is
also planning on changing the way doctests are compiled to compile
an entire crate's doctests in one go rather than in separate
crates, causing doctests to have a shared namespace which may
generate future surprising outcomes.
Matt Corallo [Mon, 29 Apr 2024 21:05:52 +0000 (21:05 +0000)]
Send peers error messages for failures due to invalid batch funding
If we fail to fund a batch open we'll force-close all channels in
the batch, however would previously fail to send error messages to
our peers for any channels we were due to test after the one that
failed.
This commit fixes that issue, sending the required error messages
to allow our peers to clean up their state.
Matt Corallo [Mon, 29 Apr 2024 18:12:59 +0000 (18:12 +0000)]
Close channels when `find_funding_output` fails to find an output
In `funding_transaction_generated_intern`, if `find_funding_output`
fails (e.g. due to the require output not being present in the
provided funding transaction) we'd previously not generated a
`ChannelClosed` event which leaves users possibly in a confused
state.
Here we fix this, also fixing the relevant tests to check for the
new event.
InvoiceRequestFeatures may contain a large, odd bit. Including this in
InvoiceRequestFields, which is in each BlindedPath of a Bolt12Invoice,
could cause the invoice's onion message to exceed the maximum size. The
features are already checked before sending an invoice.
Event::PaymentClaimable and Event::PaymentClaimed already contain the
paid amount, so there's no need to included the requested amount in
InvoiceRequestFields.
PaymentContext is already stored in ClaimablePayment via PaymentPurpose,
so there is no need to repeat it in each ClaimableHTLC via OnionPayload.
This avoids cloning the PaymentContext each time an HTLC is received,
other than for the first HTLC for a payment.
Refunds currently require a description, though this may change to be
optional. Remove the description requirement from the API, setting and
empty string by default.
Offers currently require a description, though this may change to be
optional. Remove the description requirement from the API, setting and
empty string by default.
When parsing a Bolt12Invoice use both the Offer's signing_pubkey and
paths to determine if it is for an Offer or a Refund. Previously, an
Offer was required to have a signing_pubkey. But now that it is
optional, the Offers paths can be used to make the determination.
Additionally, check that the invoice matches one of the blinded node ids
from the paths' last hops.
Instead of reusing OfferTlvStream::paths, add a dedicated paths TLV to
InvoiceRequestTlvStream such that it can be used in Refund. This allows
for an Offer without a signing_pubkey and still be able to differentiate
whether an invoice is for an offer or a refund.
If an offer has at least one path, it may omit the signing pubkey and
use the blinded node id of the last hop of a path to sign an invoice.
Allow parsing such offers but not yet creating them.
If an Offer contains a path, the blinded_node_id of the path's final hop
can be used as the signing pubkey. Make Offer::signing_pubkey and
OfferContents::signing_pubkey return an Option to support this. Upcoming
commits will implement this behavior.
Matt Corallo [Thu, 25 Apr 2024 14:30:05 +0000 (14:30 +0000)]
Drop completed blocked `ChannelMonitorUpdate`s on startup
If a user receives a payment preimage for an outbound payment, the
`PaymentSent` event will block any eventual RAA
`ChannelMonitorUpdate` from the same channel, assuming it comes in
before the event can be processed. If this blocking kicks in, but
the flow eventually completes with the RAA `ChannelMonitorUpdate`
being persisted, but the `ChannelManager` is only persisted prior
to the event being handled, on startup we'll have a fully
up-to-date `ChannelMonitor` but a pending, blocked
`ChannelMonitorUpdate`. When the `PaymentSent` event is replayed
we'll end up trying to apply a redundant `ChannelMonitorUpdate`
which will panic.
See the test added in this commit for an implementation of this
situation.
In this commit we fix this issue by simply dropping blocked
`ChannelMonitorUpdate`s the same as we do pending ones.
Matt Corallo [Mon, 22 Apr 2024 13:03:14 +0000 (13:03 +0000)]
Drop the rustup-specific calling in `ci/rustfmt.sh`
The +rustversion call semantics are specific to rustup and do not
work with standard rust toolchains. However, because rustfmt
formatting differs slightly between stable and our 1.63 target, we
need to keep the +1.63.0 for rustup users.
Thus, here, we check for the presence of a `rustup` command and
pass the "+1.63.0" argument if we find one.
Matt Corallo [Mon, 22 Apr 2024 12:50:58 +0000 (12:50 +0000)]
Only include `derive_add_tweak` for base key types that use it
Specifically `RevocationBasepoint` has a different derivation, so
shouldn't have a `derive_add_tweak` at all. We also use this
opportunity to link to the `from_basepoint` function in the
`derive_add_tweak` docs.
Matt Corallo [Fri, 19 Apr 2024 00:36:52 +0000 (00:36 +0000)]
Use `Sha256`s for tweaks in `sign` to enforce randomness
We assume that tweaks are the output of a SHA-256 hash function
(and thus that failing to create a private key from the has
negligible probability) in `add_public_key_tweak` and elsewhere.
Thus, we really shouldn't be taking byte arrays in the public API
but rather `Sha256` objects, and communicating in the docs for
`add_public_key_tweak` that we can panic if its not the output of
a hash function, both of which we do here.
Matt Corallo [Thu, 11 Apr 2024 21:53:44 +0000 (21:53 +0000)]
Debug-log monitor update starts and completions in `ChainMonitor`
When a `ChannelMonitor[Update]` persistence completes, we rely on
logging in `ChannelManager` to hear about it. However, this won't
happen at all if there's still pending updates as no `MonitorEvent`
will be generated.
Thus, here, we add logging directly in `ChainMonitor`, ensuring we
can deduce when individual updates completed from debug logs.