Commit graph

7122 commits

Author SHA1 Message Date
Matt Corallo
28d85af955 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.
2024-05-07 16:51:38 +00:00
Matt Corallo
d1ac0717fe
Merge pull request #3045 from TheBlueMatt/2024-03-fees-are-dust
Include excess counterparty commitment transaction fees in dust exposure
2024-05-07 08:50:46 -07:00
Matt Corallo
5091c1fd68 Add a simple test of the new excess-fees-are-dust behavior 2024-05-07 14:13:55 +00:00
Matt Corallo
f0b3708c20 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.

Fixes #2922
2024-05-07 14:13:55 +00:00
Matt Corallo
51bf78d604 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.
2024-05-07 14:13:55 +00:00
Matt Corallo
78ab54ff82
Merge pull request #3039 from valentinewallace/2024-04-invoice-amt-msats-overflow
Fix overflow in invoice amount setter.
2024-05-06 12:20:37 -07:00
Matt Corallo
b8d4ac1371
Merge pull request #2957 from G8XSU/pause-events
Don't pause events for chainsync persistence
2024-05-06 12:08:10 -07:00
Valentine Wallace
0ea58d0713
Fix overflow in invoice amount setter. 2024-05-06 11:20:04 -04:00
Matt Corallo
37c431ad68
Merge pull request #3035 from TheBlueMatt/2024-04-upstream-123-bindings 2024-05-03 17:55:25 -07:00
Matt Corallo
a27088d6aa
Merge pull request #3018 from jkczyz/2024-04-optional-description
Optional description for `Offer` and `Refund`
2024-05-03 12:55:17 -07:00
Matt Corallo
3d5a6913b5
Merge pull request #3029 from TheBlueMatt/2024-04-fix-batch-funding-failures
Add error handling for channels which fail to be created in `funding_transaction_generated_intern`
2024-05-03 12:17:58 -07:00
Jeffrey Czyz
e61001f60d
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.
2024-05-02 16:42:27 -05:00
Matt Corallo
11c3d7001b 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.
2024-05-02 15:39:03 +00:00
Matt Corallo
0e831b4bd0 Correct indentation in get_pending_htlc_stats
The previous commit left indentation in `get_pending_htlc_stats`
deliberately wrong, to ease reviewability. This commit fixes the
indentation.
2024-05-02 15:39:03 +00:00
Matt Corallo
2937000051 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.

Here we merge them.
2024-05-02 15:39:03 +00:00
Matt Corallo
bd3cc003bb
Merge pull request #3033 from TheBlueMatt/2024-04-notify-bp-on-blocks
Wake `background-processor` from `ChainMonitor` on new blocks
2024-05-01 14:21:28 -07:00
Matt Corallo
021979b5dd 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.
2024-05-01 19:42:24 +00:00
Matt Corallo
376df8770b Update documentation on [u8; 32] wrappers for clarity 2024-05-01 19:01:40 +00:00
Matt Corallo
e818c4b13f 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.
2024-05-01 19:01:40 +00:00
Matt Corallo
8084cec3e4 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.
2024-05-01 16:04:37 +00:00
Matt Corallo
84070e1138 Add derives where they make sense and the bindings require them 2024-05-01 16:04:37 +00:00
Matt Corallo
89a67e59ab 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.
2024-05-01 15:16:57 +00:00
Matt Corallo
b403411c24
Merge pull request #3032 from valentinewallace/2024-04-pico-btc-overflow
Fix overflow in lightning-invoice `amount_pico_btc`
2024-05-01 06:08:45 -07:00
valentinewallace
c90369897b
Merge pull request #3031 from TheBlueMatt/2024-04-new-rustc-warning
Fix new rustc `#[macro_export]` warning in doctests
2024-04-30 13:16:11 -04:00
Matt Corallo
c8ddf36f9c 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.
2024-04-30 14:13:12 +00:00
Valentine Wallace
c6ae9288b9
Fix overflow in lightning-invoice amount_pico_btc. 2024-04-30 10:05:35 -04:00
Matt Corallo
b811cba748 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.
2024-04-30 13:43:32 +00:00
Matt Corallo
6960210f60 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.

Fixes #2843.
2024-04-30 13:43:32 +00:00
Matt Corallo
2b14cc40a6
Merge pull request #3017 from jkczyz/2024-04-optional-pubkey
Sending to `Offer` without `signing_pubkey`
2024-04-29 12:05:30 -07:00
Matt Corallo
d00e55077a
Merge pull request #3028 from jkczyz/2024-04-offer-id-followups
Follow-ups to #2970
2024-04-29 11:26:44 -07:00
Jeffrey Czyz
33b6162fd2
Remove InvoiceRequestFields::features
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.
2024-04-29 12:27:39 -05:00
Jeffrey Czyz
7b864425ea
Remove InvoiceRequestFields::amount_msats
Event::PaymentClaimable and Event::PaymentClaimed already contain the
paid amount, so there's no need to included the requested amount in
InvoiceRequestFields.
2024-04-29 12:27:38 -05:00
Jeffrey Czyz
8e562be5d4
Remove PaymentContext from OnionPayload
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.
2024-04-29 12:27:34 -05:00
Gursharan Singh
4d5de1fe61
Remove ChainMonitor::sync_persistence_id
It was used earlier for generating unique MonitorUpdateId for
UpdateOrigin::ChainSync monitor updates.
2024-04-26 20:29:02 -07:00
Gursharan Singh
c1125f00b3
Remove MonitorUpdateId from persist trait
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.
2024-04-26 20:28:55 -07:00
Jeffrey Czyz
db7e696673
Don't require Refund::description in API
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.
2024-04-26 18:14:30 -05:00
Jeffrey Czyz
48cba2954b
Don't require Offer::description in API
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.
2024-04-26 18:14:30 -05:00
Jeffrey Czyz
b7635c4dc2
Bolt12Invoice for Offer without signing_pubkey
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.
2024-04-26 18:14:09 -05:00
Jeffrey Czyz
61e4fcea8e
Add InvoiceRequestTlvStream::paths
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.
2024-04-26 18:14:09 -05:00
Jeffrey Czyz
94d5af663d
Allow parsing Offer without signing_pubkey
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.
2024-04-26 18:14:09 -05:00
Jeffrey Czyz
520fecf8fc
Optional OfferContents::signing_pubkey
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.
2024-04-26 18:14:09 -05:00
Gursharan Singh
7de602a38a
Stop tracking MonitorUpdates from ChainSync in pending_monitor_updates
We no longer need to track them since we no longer hold events for
pending MonitorUpdates resulting from ChainSync.
2024-04-26 16:06:44 -07:00
Gursharan Singh
e084ab26e1
Stop storing last_chain_persist_height
We only used to store last_chain_persist_height to release
events held for more than LATENCY_GRACE_PERIOD_BLOCKS due to
pending monitor update with UpdateOrigin::ChainSync. Since we no
longer pause events for ChainSync persistence, we no longer need to
store last_chain_persist_height.
2024-04-26 16:02:20 -07:00
Gursharan Singh
cb86399f2e
Don't pause events for chainsync persistence
We used to wait on ChannelMonitor persistence to avoid
duplicate payment events. But this can still happen in cases where
ChannelMonitor handed the event to ChannelManager and we did not persist
ChannelManager after event handling.
It is expected to receive payment duplicate events and clients should handle these
events in an idempotent manner. Removing this hold-up of events simplifies
the logic and makes it easier to not persist ChannelMonitors on every block connect.
2024-04-26 14:37:41 -07:00
Gursharan Singh
8701b1bbe3
Merge pull request #3003 from TheBlueMatt/2024-04-2964-followups
#2964 Followups
2024-04-25 13:19:09 -07:00
Matt Corallo
dfb250b976
Merge pull request #3021 from TheBlueMatt/2024-04-drop-blocked-completed-updates
Drop completed blocked `ChannelMonitorUpdate`s on startup
2024-04-25 11:33:18 -07:00
Matt Corallo
eebab4015d
Merge pull request #3004 from TheBlueMatt/2024-04-2761-followups
#2761 Followups
2024-04-25 10:52:12 -07:00
Matt Corallo
c40504a0fc Drop completed blocked ChannelMonitorUpdates 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.
2024-04-25 15:13:56 +00:00
Elias Rohrer
9a438eea3b
Merge pull request #3015 from tnull/2024-04-sweeper-error-on-persistence-failure
Make `OutputSweeper::track_spendable_outputs` fallible
2024-04-24 16:35:51 +02:00
Elias Rohrer
afb452a813
Make OutputSweeper::track_spendable_outputs fallible
.. as otherwise we might only log an error and continue if we fail to
persist the sweeper state.
2024-04-24 09:46:34 +02:00