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.
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.
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.
Fixes#2922
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.
A few years ago we had repeated user confusion dealing with the
`Filter` methods and exactly how they differ. Here we try to add a
bit more color in several ways as suggested by users in a few
places - listing examples of why the methods are used as well as
ensuring its clearly communicated that not all parameters need be
used.
Fixes#1069
- Currently, handle_message (or handle_custom_message) only exposes
the generated response for an OnionMessage, lacking the necessary
reply_path for asynchronous responses.
- This commit introduces a new flow for OnionMessage handling.
- Instead of solely taking the message as input, handle_message now accepts
an Optional `responder`, returning a `ResponseInstruction` containing both
the response and the reply_path.
- `ResponseInstruction` utilizes different enum variants to indicate how
`handle_onion_message_response` should handle the response.
- This enhancement enables exposing the reply_path alongside the response
and allows for more complex response mechanisms, such as responding with
an added reply_path.
- The commit introduces the foundational framework (structs & enums) for this new flow.
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.
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.
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.
Here we merge them.
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.
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.
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.
`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.
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.
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.
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.
Fixes#2843.
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.
MonitorUpdateId was an opaque abstraction for id's generated by
UpdateOrigin:Offchain and UpdateOrigin::ChainSync monitor updates.
It was mainly needed to map calls made to
ChainMonitor::channel_monitor_updated. We no longer track
UpdateOrigin::ChainSync MonitorUpdates and can directly use
ChannelMonitor::get_latest_update_id() for tracking
UpdateOrigin::Offchain monitor updates.
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.
