Vincenzo Palazzo [Tue, 27 Aug 2024 08:18:28 +0000 (10:18 +0200)]
event: store the outpoint when is_manual_broadcast
With [1], it's possible to specify `manual_broadcast` for
the channel funding transaction. When `is_manual_broadcast` is
set to true, the transaction in the `DiscardFunding` event is
replaced with a dummy empty transaction.
This commit checks if `is_manual_broadcast` is true and
stores the funding OutPoint in the DiscardFunding event instead.
This test was added some time ago in 0c034e9a82e4339fb32af9da63832ac2a64abb0b, but never made any sense.
`PeerManager::process_events` will go around its loop as many times
is required to ensure we've always processed all events which were
pending prior to a `process_events` call, so having a test that
checks that we never go around more than twice is obviously broken.
And, indeed, in CI this tests fails with some regularity.
Instead, the test here is changed to ensure that we detectably go
around the loop again at least once.
Support next_blinding_override in blinded payment paths.
This allow us to forward blinded payments where the blinded path that we are
forwarding within was concatenated to another blinded path that starts at the
next hop.
Also allows constructing blinded paths using this override.
Matt Corallo [Sat, 17 Aug 2024 21:13:08 +0000 (21:13 +0000)]
Remove the `std` feature implications from the `lightning` crate
Now that we don't have to have everything in our entire ecosystem
use the same `std`/`no-std` feature combinations we should start by
untangling our own features a bit.
This takes one last step, removing the implications of the `std`
feature from the `lightning` crate.
Matt Corallo [Fri, 16 Aug 2024 20:32:58 +0000 (20:32 +0000)]
Drop the `no-std` feature from `lightning-rapid-gossip-sync`
Now that we don't have to have everything in our entire ecosystem
use the same `std`/`no-std` feature combinations we should start by
untangling our own features a bit.
This takes another step by removing the `no-std` feature entirely
from the `lightning-rapid-gossip-sync` crate and removing all
feature implications on dependencies from the remaining `std`
feature.
Matt Corallo [Fri, 16 Aug 2024 19:17:42 +0000 (19:17 +0000)]
Drop the `no-std` feature from `lightning-invoice`
Now that we don't have to have everything in our entire ecosystem
use the same `std`/`no-std` feature combinations we should start by
untangling our own features a bit.
This takes another step by removing the `no-std` feature entirely
from the `lightning-invoice` crate and removing all feature
implications on dependencies from the remaining `std` feature.
Matt Corallo [Fri, 16 Aug 2024 19:07:05 +0000 (19:07 +0000)]
Drop the `no-std` feature from BP and drop feature implications
Now that we don't have to have everything in our entire ecosystem
use the same `std`/`no-std` feature combinations we should start by
untangling our own features a bit.
This takes the first step by removing the `no-std` feature entirely
from the `lightning-background-processor` crate and removing most
feature implications on dependencies from the remaining `std`
feature.
It also addresses a CI oversight where we were not testing
`lightning-background-processor` without the `std` feature in CI at
all.
Matt Corallo [Fri, 16 Aug 2024 20:13:18 +0000 (20:13 +0000)]
Ensure we always pass a `path` for in-workspace dependencies
In order to ensure our crates depend on the workspace copies of
each other in test builds we need to override the crates.io
dependency with a local `path`.
We can do this in one of two ways - either specify the `path` in
the dependency listing in each crate's `Cargo.toml` or use the
workspace `Cargo.toml` to `patch` all dependencies. The first is
tedious while the second lets us have it all in one place. However,
the second option does break `cargo *` in individual crate
directories (forcing the use of `cargo -p crate *` instead) and
makes it rather difficult to depend on local versions of workspace
crates.
Thus, here we drop the `patch.crates-io` from our top-level
`Cargo.toml` entirely.
Still, we do update the `ci/ci-tests.sh` script here to use
`cargo -p crate` instead of switching to each crate's directory as
it allows `cargo` to use a shared `target` and may speed up tests.
Matt Corallo [Fri, 16 Aug 2024 18:47:38 +0000 (18:47 +0000)]
Optimize `BufReader` somewhat for the only used case
`rust-bitcoin` doesn't ever actually *use* its `BufRead`
requirement when deserializing objects, and forcing it is somewhat
inefficient, so we optimize the only (actual) case here by passing
reads straight through to the backing stream.
Matt Corallo [Mon, 19 Aug 2024 14:37:03 +0000 (14:37 +0000)]
Re-export `lightning_types` in top-level `lightning` modules
Since we now have many types in one place, it makes sense to export
them in that place. Further, doing so finally somewhat starts to
reduce our `lightning::ln` module size, which historically is the
dumping ground for everything when most things really should be
top-level modules in `lightning`.
Here we take a step in the right direction by exporting
`lightning_types` as `lightning::types` and encouraging users to
use those paths directly rather than the ones in `lightning::ln`.
Matt Corallo [Mon, 11 Dec 2023 02:10:51 +0000 (02:10 +0000)]
Tweak alignment and memory order in the `ProbabilisticScorer`
During routing, the majority of our time is spent in the scorer.
Given the scorer isn't actually doing all that much computation,
this means we're quite sensitive to memory latency. Thus, the cache
lines our data sits on is incredibly important.
Here, we manually lay out the `ChannelLiquidity` and
`HistoricalLiquidityTracker` structs to ensure that we can do the
non-historical scoring and skip historical scoring for channels
with insufficient data by just looking at the same cache line the
channel's SCID is on.
Sadly, to do the full historical scoring we need to load a second
128-byte cache line pair, but we have some time to get there. We
might consider issuing a preload instruction in the future.
Matt Corallo [Sat, 9 Dec 2023 04:18:46 +0000 (04:18 +0000)]
Cache the total points tracked in our historical liquidity data
When we go to score a channel using the historical liquidity data,
the first thing we do is step through all the valid bucket
combinations, multiply the min and max bucket, and then add them
together to calculate the total number of points tracked. This
isn't a free operation, and for scorers without much data it
represents a large part of the total time spent scoring during
routefinding.
Thus, here we cache this value, updating it every time the buckets
are updated.
Matt Corallo [Sat, 9 Dec 2023 04:30:08 +0000 (04:30 +0000)]
Change the directed history tracker's storage of its direction
Rather than storing the two direction's buckets in
`HistoricalMinMaxBuckets` (renamed
`DirectedHistoricalLiquidityTracker`), we store a single reference
to the `HistoricalLiquidityTracker` as well as the direction bool.
This will allow us in the next commit to reference fields in the
`HistoricalLiquidityTracker` aside from the two directions.
Matt Corallo [Sat, 9 Dec 2023 04:29:12 +0000 (04:29 +0000)]
Make the historical bucket data private to `bucketed_history`
In a comming commit we'll cache some additional data in the
historical bucket tracker. In order to do so, here we isolate the
buckets themselves into the `bucketed_history` module, reducing
the possibility of accidentally updating them directly without
updating caches.
Matt Corallo [Sat, 9 Dec 2023 03:32:40 +0000 (03:32 +0000)]
Store both min and max historical buckets in one, new, struct
In the coming commits we'll isolate historical bucket logic slightly
further, allowing us to cache some state. This is the first step
towards that, storing the historical liquidity information in a new
`HistoricalLiquidityTracker` rather than in the general
`ChannelLiquidity`.
Jeffrey Czyz [Wed, 14 Aug 2024 22:39:13 +0000 (17:39 -0500)]
Add PaymentId authentication to public API
When receiving an InvoiceError message, it should be authenticated
before using it to abandon the payment. Add methods to PaymentId's
public API for constructing and verifying an HMAC for use in
OffersContext::OutboundPayment. This allows other implementations of
OffersMessageHandler to construct the HMAC and authenticate the message.
Arik Sosman [Thu, 15 Aug 2024 06:57:14 +0000 (23:57 -0700)]
Fix fuzz warnings.
Version 0.32.2 of `rust-bitcoin` deprecates a number of methods that
are commonly used in this project, most visibly `txid()`, which is
now called `compute_txid()`. This resulted in a lot of warnings, and
this commit is part of a series that seeks to address that.
Arik Sosman [Thu, 15 Aug 2024 02:52:01 +0000 (19:52 -0700)]
Fix lightning-transaction sync warnings.
Version 0.32.2 of `rust-bitcoin` deprecates a number of methods that
are commonly used in this project, most visibly `txid()`, which is
now called `compute_txid()`. This resulted in a lot of warnings, and
this commit is part of a series that seeks to address that.
Arik Sosman [Wed, 14 Aug 2024 20:31:40 +0000 (13:31 -0700)]
Fix lightning-block-sync warnings.
Version 0.32.2 of `rust-bitcoin` deprecates a number of methods that
are commonly used in this project, most visibly `txid()`, which is
now called `compute_txid()`. This resulted in a lot of warnings, and
this commit is part of a series that seeks to address that.
Arik Sosman [Wed, 14 Aug 2024 20:41:55 +0000 (13:41 -0700)]
Fix lightning-background-processor warnings.
Version 0.32.2 of `rust-bitcoin` deprecates a number of methods that
are commonly used in this project, most visibly `txid()`, which is
now called `compute_txid()`. This resulted in a lot of warnings, and
this commit is part of a series that seeks to address that.
Arik Sosman [Fri, 16 Aug 2024 15:45:27 +0000 (08:45 -0700)]
Fix lightning warnings.
Version 0.32.2 of `rust-bitcoin` deprecates a number of methods that
are commonly used in this project, most visibly `txid()`, which is
now called `compute_txid()`. This resulted in a lot of warnings, and
this commit is part of a series that seeks to address that.
Arik Sosman [Thu, 15 Aug 2024 01:49:22 +0000 (18:49 -0700)]
Introduce io module.
The rust-bitcoin upgrade will introduce `bitcoin::io` module, which
will be missing a necessary subset of traits.
To accommodate those traits' future implementations, we move the
`lightning::io` module to its own file, where we will be able to
implement the missing trait subset in the next commit.
Arik Sosman [Wed, 14 Aug 2024 21:25:56 +0000 (14:25 -0700)]
Simplify Readable mutability.
In anticipation of the rust-bitcoin upgrade, which incorporates its
own `io::Read` implementation, we need to make our usage compatible
with dropping `std::io` and `core2::io`.
Notably, in version 0.32.2, `bitcoin::io`'s `Read` is no longer
implemented for `&mut R where R: Read + ?Sized`, which results in
errors anytime `&mut &mut Readable` is passed instead of
`&mut Readable`.
Elias Rohrer [Wed, 14 Aug 2024 07:22:13 +0000 (09:22 +0200)]
Disallow skipping tx-sync tests in CI
Previously, we'd always skip tx-sync tests if the
`BITCOIND_EXE`/`ELECTRS_EXE` environment variables would be unset. While
this is especially fine for local testing, we still want to enforce
tests failing if somehow the `bitcoind`/`electrs` downloading or caching
in CI stops working. Here, we therefore add a `CI_ENV` variable that
indicates we're indeed running in CI, and only skip if it's unset.
Matt Corallo [Wed, 14 Aug 2024 19:36:46 +0000 (19:36 +0000)]
Return slices, rather than `Vec` references, in addresses
Its a bit strange to return a reference to a `Vec` in Rust, when a
slice is really intended as the way to do so. Worse, the bindings
don't know how to map a reference to a `Vec` (but do have code to
map a slice of `Clone`able objects).
Here, we move `NodeAnnouncementInfo::addresses` to return a slice,
though to do so we have to adapt the `WithoutLength` `Writeable`
impl to support slices as well.
Jeffrey Czyz [Fri, 9 Aug 2024 16:07:35 +0000 (11:07 -0500)]
Make PaymentFailureReason downgradable
The PaymentFailureReason variants for invoice request failures will
cause downgrades to break. Instead, use a new TLV for the reason and
continue to write the old TLV, only use None for the new reasons.
Jeffrey Czyz [Tue, 6 Aug 2024 23:33:51 +0000 (18:33 -0500)]
Add PaymentFailureReason::InvoiceRequestRejected
Instead of re-using PaymentFailureReason::RecipientRejected, define a
new InvoiceRequestRejected variant for when an InvoiceError is received
instead of a Bolt12Invoice. This allows user to differentiate the cause
of the failure.
Jeffrey Czyz [Tue, 6 Aug 2024 22:51:16 +0000 (17:51 -0500)]
Remove Event::InvoiceRequestFailed
Now that Event::PaymentFailed has an option payment_hash, it can be used
in replace of Event::InvoiceRequestFailed. This allows for including a
reason when abandoning a payment before an invoice is received.
Jeffrey Czyz [Tue, 6 Aug 2024 22:17:52 +0000 (17:17 -0500)]
Make payment_hash optional in Event::PaymentFailed
When abandoning a BOLT12 payment before a Bolt12Invoice is received, an
Event::InvoiceRequestFailed is generated and the abandonment reason is
lost. Make payment_hash optional in Event::PaymentFailed so that
Event::InvoiceRequestFailed can be removed in favor of it.
When testing Bolt12Invoice unknown require feature handling, a large
feature bit can cause SendError::TooBigPacket when creating an onion
message. Use a smaller feature bit for UnknownFeature, which also has
the added benefit of reducing test output.
When handling a BOLT12 invoice, and invoice error is sent if the invoice
contains unknown required features. However, since the payment is still
in state AwaitingInvoice, abandoning it results in losing the reason
since an InvoiceRequestFailed event would be generated. Move the check
to PendingOutboundPayments such that the payment is first moved to state
InvoiceReceived so that a PaymentFailed event is generated instead.
Jeffrey Czyz [Fri, 2 Aug 2024 14:39:52 +0000 (09:39 -0500)]
Don't include HMAC in Refund paths
Refunds are typically communicated via QR code, where a smaller size is
desirable. Make the HMAC in OutboundPayment data optional such that it
is elided from blinded paths used in refunds. This prevents abandoning
refunds if the reader sends an invoice_error instead of an invoice
message. However, this use case isn't necessary as the corresponding
outbound payment will either timeout when the refund expires or can be
explicitly abandoned by the creator.
A BOLT12 payment may be abandoned when handling the invoice or when
receiving an InvoiceError message. When abandoning the payment, don't
use UserAbandoned as the reason since that is meant for when the user
calls ChannelManager::abandon_payment.
When making an outbound BOLT12 payment, multiple invoices may be
received for the same payment id. Instead of abandoning the payment when
a duplicate invoice received, simply ignore it without responding with
an InvoiceError. This prevents abandoning in-progress payments and
sending unnecessary onion messages.
Before abandoning a payment when receiving an InvoiceError, verify that
the PaymentId included in the OffersContext with the included HMAC. This
prevents a malicious actor sending an InvoiceError with a known payment
id from abandoning our payment.
When receiving an InvoiceError in response to an InvoiceRequest, the
corresponding payment should be abandoned. Add an HMAC to
OffersContext::OutboundPayment such that the payment ID can be
authenticated prior to abandoning the payment.
An HMAC needs to be included in OffersContext::OutboundPayment to
authenticate the included PaymentId. Implement Readable and Writeable to
allow for this.
When receiving an InvoiceError in response to an InvoiceRequest, the
corresponding payment should be abandoned. Add functions for
constructing and verifying an HMAC over a Payment ID to allow for this.
Matt Corallo [Fri, 9 Aug 2024 15:27:38 +0000 (15:27 +0000)]
`rustfmt` new files added in the past few commits
The past handful of commits were mostly moving code around, so to
aid reviewers violated our `rustfmt` rules. Here we rectify that by
`rustfmt`'ing the newly-added files.
Matt Corallo [Fri, 9 Aug 2024 15:42:48 +0000 (15:42 +0000)]
Prepare to `rustfmt` newly added files
In the next commit we'll `rustfmt` newly-added files, but before
we do so we clean up some code so that the resulting files won't be
quite as absurd. We also exclude the new `invoice_utils.rs` file,
as it needs quite substantial cleanups.
Matt Corallo [Fri, 9 Aug 2024 02:45:55 +0000 (02:45 +0000)]
Provide the signer with a full `RawBolt11Invoice` to sign
Now that the `lightning` crate depends on the `lightning-invoice`
crate, there's no reason to have the `sign_invoice` method take raw
base32 field elements as we can now give it a real
`RawBolt11Invoice`, which we do here.
This simplifies the interface and avoids a
serialization-deserialization roundtrip when signing invoices in a
validating signer.
Matt Corallo [Fri, 9 Aug 2024 01:29:48 +0000 (01:29 +0000)]
Swap the dep order between `lightning` and `lightning-invoice`
`lightning-invoice` previously had a dependency on the entire
`lightning` crate just because it wants to use some of the useful
types from it. This is obviously backwards and leads to some
awkwardness like the BOLT 11 invoice signing API in the `lightning`
crate taking a `[u5]` rather than a `Bolt11Invoice`.
Here we finally rectify this issue, swapping the dependency order
and making `lightning` depend on `lightning-invoice` rather than
the other way around.
This moves various utilities which were in `lightning-invoice` but
relied on `lightning` payment types to make payments to where they
belong (the `lightning` crate), but doesn't bother with integrating
them well in their new home.
Matt Corallo [Fri, 9 Aug 2024 01:13:25 +0000 (01:13 +0000)]
Add a `lightning-types` dependency to `lightning-invoice`
`lightning-invoice` currently has a dependency on the entire
`lightning` crate just because it wants to use some of the useful
types from it. This is obviously backwards and leads to some
awkwardness like the BOLT 11 invoice signing API in the `lightning`
crate taking a `[u5]` rather than a `Bolt11Invoice`.
This takes tees us up for the final step, adding a
`lightning-types` dependency to `lightning-invoice` and using it
for imports rather than the `lightning` crate.
Matt Corallo [Fri, 9 Aug 2024 13:20:35 +0000 (13:20 +0000)]
Use `check_added_monitors` test utility in invoice utils tests
In a coming commit, the `lightning-invoice::utils` module will move
to the `lightning` crate, causing its tests to be included in the
global lockorder tests done in that crate. This should be fine,
except that the `lightning-invoice::utils` module currently holds
the `added_monitors` lock too long causing lockorder violations.
Instead, this commit replaces the legacy monitors-added test with
the `check_added_monitors` test utility.