Commit graph

5104 commits

Author SHA1 Message Date
Matt Corallo
67ad6c40f0 Don't remove nodes if there's no channel_update for a temp failure
Previously, we were requiring any `UPDATE` onion errors to include
a `channel_update`, as the spec mandates[1]. If we see an onion
error which is missing one we treat it as a misbehaving node that
isn't behaving according to the spec and simply remove the node.

Sadly, it appears at least some versions of CLN are such nodes, and
opt to not include `channel_update` at all if they're returning a
`temporary_channel_failure`. This causes us to completely remove
CLN nodes from our graph after they fail to forward our HTLC.

While CLN is violating the spec here, there's not a lot of reason
to not allow it, so we go ahead and do so here, treating it simply
as any other failure by letting the scorer handle it.

[1] The spec says `Please note that the channel_update field is
mandatory in messages whose failure_code includes the UPDATE flag`
however doesn't repeat it in the requirements section so its not
crazy that someone missed it when implementing.
2023-04-24 18:52:05 +00:00
Matt Corallo
3dcd490bbf Remove unused import in functional_test_utils 2023-04-24 17:58:06 +00:00
Matt Corallo
3dab242f08 Fix unused Secp256k1 context in monitor_tests 2023-04-24 17:58:06 +00:00
Matt Corallo
bc54441424
Merge pull request #2212 from wpaulino/off-by-one-locktime
Fix off-by-one finalized transaction locktime
2023-04-22 21:54:06 +00:00
Wilmer Paulino
97e4344bea
Fix off-by-one finalized transaction locktime
While these transactions were still valid, we incorrectly assumed that
they would propagate with a locktime of `current_height + 1`, when in
reality, only those with a locktime strictly lower than the next height
in the chain are allowed to enter the mempool.
2023-04-22 11:16:32 -07:00
Wilmer Paulino
e904d68fa8
Catch up test nodes to latest block height
In a future commit, we plan to correctly enforce that the spending
transaction has a valid locktime relative to the chain for the node
broascasting it in `TestBroadcaster::broadcast_transaction` to. We catch
up these test node instances to their expected height, such that we do
not fail said enforcement.
2023-04-22 11:16:32 -07:00
Wilmer Paulino
69d0bfacd9
Use current height when generating claims on block_disconnected
The `height` argument passed to `OnchainTxHandler::block_disconnected`
represents the height being disconnected, and not the current height.
Due to the incorrect assumption, we'd generate a claim with a locktime
in the future.

Ultimately, we shouldn't be generating claims within
`block_disconnected`. Rather, we should retry the claim at a later block
height, since the bitcoin blockchain does not ever roll back without
connecting a new block. Addressing this is left for future work.
2023-04-22 11:16:28 -07:00
Matt Corallo
5f96d13344
Merge pull request #2199 from tnull/2023-04-fix-async-event-processing
Allow async events processing without holding `total_consistency_lock`
2023-04-22 01:45:06 +00:00
Matt Corallo
02ae5cb9a7
Merge pull request #2208 from wpaulino/monitor-rebroadcast-pending-claims
Implement pending claim rebroadcast on force-closed channels
2023-04-21 23:33:52 +00:00
Wilmer Paulino
453b3a1055
Call ChainMonitor::rebroadcast_pending_claims on REBROADCAST_TIMER tick
This will prompt monitors to rebroadcast/fee-bump their pending claims
on a force-closed channel once on startup and every 30 seconds after.
2023-04-21 14:34:42 -07:00
Wilmer Paulino
db123f74be
Implement pending claim rebroadcast on force-closed channels
This attempts to rebroadcast/fee-bump each pending claim a monitor is
tracking for a force-closed channel. This is crucial in preventing
certain classes of pinning attacks and ensures reliability if
broadcasting fails. For implementations of `FeeEstimator` that also
support mempool fee estimation, we may broadcast a fee-bumped claim
instead, ensuring we can also react to mempool fee spikes between
blocks.
2023-04-21 14:34:41 -07:00
Wilmer Paulino
e496d62b98
Extend OnchainTxHandler::generate_claim to optionally force feerate bump
In the next commit, we plan to extend the `OnchainTxHandler` to retry
pending claims on a timer. This timer may fire with much more frequency
than incoming blocks, so we want to avoid manually bumping feerates
(currently by 25%) each time our fee estimator provides a lower feerate
than before.
2023-04-21 14:34:40 -07:00
Matt Corallo
7a07049c7a
Merge pull request #2210 from alandotcom/patch-1
Fix README typo
2023-04-21 19:01:39 +00:00
Alan Cohen
fcba0193ee
Fix README typo 2023-04-21 13:20:35 -04:00
Elias Rohrer
f2453b7fff
Allow events processing without holding total_consistency_lock
Unfortunately, the RAII types used by `RwLock` are not `Send`, which is
why they can't be held over `await` boundaries. In order to allow
asynchronous events processing in multi-threaded environments, we here
allow to process events without holding the `total_consistency_lock`.
2023-04-21 18:02:54 +02:00
Elias Rohrer
b5468226cc
Fix BP prune timer and don't panic on persistence notification failure 2023-04-21 11:44:36 +02:00
Matt Corallo
9d5adfcd10
Merge pull request #2205 from wpaulino/sign-ecdsa-with-noncedata
Generate local signatures with additional randomness
2023-04-20 21:53:13 +00:00
Wilmer Paulino
86531e5ceb
Use new feature to gate test vectors behind
To match the local signatures found in test vectors, we must make sure
we don't use any additional randomess when generating signatures, as
we'll arrive at a different signature otherwise.
2023-04-20 12:14:28 -07:00
Wilmer Paulino
16edbd4196
Sign gossip messages without grinding low R signatures
Gossip messages always use signatures in their compact form, so grinding
for low R signatures is unnecessary.
2023-04-20 12:14:24 -07:00
Wilmer Paulino
78b967f5b0
Generate local signatures with additional randomness
Previously, our local signatures would always be deterministic, whether
we'd grind for low R value signatures or not. For peers supporting
SegWit, Bitcoin Core will generally use a transaction's witness-txid, as
opposed to its txid, to advertise transactions. Therefore, to ensure a
transaction has the best chance to propagate across node mempools in the
network, each of its broadcast attempts should have a unique/distinct
witness-txid, which we can achieve by introducing random nonce data when
generating local signatures, such that they are no longer deterministic.
2023-04-20 12:14:21 -07:00
Wilmer Paulino
2c5bd1c56f
Implement EntropySource for InMemorySigner
This allows the `InMemorySigner` to produce its own randomness, which we
plan to use when generating signatures in future work.

We can no longer derive `Clone` due to the `AtomicCounter`, so we opt to
implement it manually.
2023-04-20 12:14:19 -07:00
Matt Corallo
a7600dcd58
Merge pull request #2206 from jkczyz/2023-04-invoice-description
Expose description from Invoice's offer/refund
2023-04-20 18:18:28 +00:00
Jeffrey Czyz
42a772d675
Expose description from Invoice's offer/refund
This will allow clients like VLS to check the description for an invoice
as part of verification.
2023-04-20 08:41:07 -05:00
Matt Corallo
b8ed4d2608
Merge pull request #1989 from jkczyz/2023-01-stateless-offers
Stateless BOLT 12 message verification
2023-04-20 04:25:21 +00:00
Jeffrey Czyz
8afe694020
DRY up InvoiceFields construction 2023-04-19 21:31:08 -05:00
Jeffrey Czyz
c8a847ae11
Support responding to refunds with transient keys 2023-04-19 21:31:07 -05:00
Jeffrey Czyz
e1a6bc3cad
Extract keys from Offer::metadata to sign Invoice
For offers where the signing pubkey is derived, the keys need to be
extracted from the Offer::metadata in order to sign an invoice.
Parameterize InvoiceBuilder such that a build_and_sign method is
available for this situation.
2023-04-19 21:31:07 -05:00
Jeffrey Czyz
259aa9aac3
DRY up verification of invreq TLV records 2023-04-19 21:31:07 -05:00
Jeffrey Czyz
2298af4d0b
Stateless verification of Invoice for Refund
Stateless verification of Invoice for Offer

Verify that an Invoice was produced from a Refund constructed by the
payer using the payer metadata reflected in the Invoice. The payer
metadata consists of a 128-bit encrypted nonce and possibly a 256-bit
HMAC over the nonce and Refund TLV records (excluding the payer id)
using an ExpandedKey.

Thus, the HMAC can be reproduced from the refund bytes using the nonce
and the original ExpandedKey, and then checked against the metadata. If
metadata does not contain an HMAC, then the reproduced HMAC was used to
form the signing keys, and thus can be checked against the payer id.
2023-04-19 21:31:07 -05:00
Jeffrey Czyz
1a7540f2c9
Refund metadata and payer id derivation
Add support for deriving a transient payer id for each Refund from an
ExpandedKey and a nonce. This facilitates payer privacy by not tying any
Refund to any other nor to the payer's node id.

Additionally, support stateless Invoice verification by setting payer
metadata using an HMAC over the nonce and the remaining TLV records,
which will be later verified when receiving an Invoice response.
2023-04-19 21:31:07 -05:00
Jeffrey Czyz
022eadc4db
Stateless verification of Invoice for Offer
Verify that an Invoice was produced from an InvoiceRequest constructed
by the payer using the payer metadata reflected in the Invoice. The
payer metadata consists of a 128-bit encrypted nonce and possibly a
256-bit HMAC over the nonce and InvoiceRequest TLV records (excluding
the payer id) using an ExpandedKey.

Thus, the HMAC can be reproduced from the invoice request bytes using
the nonce and the original ExpandedKey, and then checked against the
metadata. If metadata does not contain an HMAC, then the reproduced HMAC
was used to form the signing keys, and thus can be checked against the
payer id.
2023-04-19 21:31:07 -05:00
Jeffrey Czyz
9bd43e077f
Fix builder docs in offers module 2023-04-19 21:31:06 -05:00
Jeffrey Czyz
3880e69237
InvoiceRequest metadata and payer id derivation
Add support for deriving a transient payer id for each InvoiceRequest
from an ExpandedKey and a nonce. This facilitates payer privacy by not
tying any InvoiceRequest to any other nor to the payer's node id.

Additionally, support stateless Invoice verification by setting payer
metadata using an HMAC over the nonce and the remaining TLV records,
which will be later verified when receiving an Invoice response.
2023-04-19 21:31:06 -05:00
Jeffrey Czyz
e15044b889
Refactor InvoiceRequestContents fields into a sub-struct
InvoiceRequestBuilder has a field containing InvoiceRequestContents.
When deriving the payer_id from the remaining fields, a struct is needed
without payer_id as it not optional. Refactor InvoiceRequestContents to
have an inner struct without the payer_id such that
InvoiceRequestBuilder can use it instead.
2023-04-19 21:31:06 -05:00
Jeffrey Czyz
dd2ccd2322
Stateless verification of InvoiceRequest
Verify that an InvoiceRequest was produced from an Offer constructed by
the recipient using the Offer metadata reflected in the InvoiceRequest.
The Offer metadata consists of a 128-bit encrypted nonce and possibly a
256-bit HMAC over the nonce and Offer TLV records (excluding the signing
pubkey) using an ExpandedKey.

Thus, the HMAC can be reproduced from the offer bytes using the nonce
and the original ExpandedKey, and then checked against the metadata. If
metadata does not contain an HMAC, then the reproduced HMAC was used to
form the signing keys, and thus can be checked against the signing
pubkey.
2023-04-19 21:30:40 -05:00
Matt Corallo
8d50c919cf
Merge pull request #2203 from wpaulino/retry-untractable-packages
Retry untractable packages
2023-04-20 02:20:34 +00:00
Jeffrey Czyz
b2e87ff25b
TlvStream range iterator
Add an iterator that yields TlvRecords over a range of a TlvStream.
Useful for verifying that, e.g., an InvoiceRequest was sent in response
to an Offer constructed by the intended recipient.
2023-04-19 21:08:05 -05:00
Jeffrey Czyz
1cad430e14
Offer metadata and signing pubkey derivation
Add support for deriving a transient signing pubkey for each Offer from
an ExpandedKey and a nonce. This facilitates recipient privacy by not
tying any Offer to any other nor to the recipient's node id.

Additionally, support stateless Offer verification by setting its
metadata using an HMAC over the nonce and the remaining TLV records,
which will be later verified when receiving an InvoiceRequest.
2023-04-19 21:06:37 -05:00
Wilmer Paulino
a3b416a32c
Make PackageTemplate::height_timer non-optional
Now that we leverage a package's `height_timer` even for untractable
packages, there's no need to have it be an `Option` anymore. We aim to
not break compatibility by keeping the deserialization of such as an
`option`, and use the package's `height_original` when not present. This
allows us to retry packages from older `ChannelMonitor` versions that
have had a failed initial package broadcast.
2023-04-19 16:49:37 -07:00
Wilmer Paulino
4828817f3f
Use existing height timer to retry untractable packages
Untractable packages are those which cannot have their fees updated once
signed, hence why they weren't retried. There's no harm in retrying
these packages by simply re-broadcasting them though, as the fee market
could have spontaneously spiked when we first broadcast it, leading to
our transaction not propagating throughout node mempools unless
broadcast manually.
2023-04-19 16:49:35 -07:00
Matt Corallo
2e15df730f
Merge pull request #2127 from TheBlueMatt/2023-03-payment-metadata
Support sending `PaymentMetadata` in HTLCs
2023-04-19 17:17:49 +00:00
Matt Corallo
ef8e3770a9 Fix variable name typo 2023-04-19 14:55:48 +00:00
Matt Corallo
934a5349f8 Update and clarify the reasons for HTLCDestination::FailedPayment 2023-04-19 14:55:48 +00:00
Matt Corallo
a41d75fb08 Add some tests of payment metadata being sent and received 2023-04-19 14:55:48 +00:00
Matt Corallo
e1e79031be Expose the RecipientOnionFields in Event::PaymentClaimable
This finally completes the piping of the `payment_metadata` from
from the BOLT11 invoice on the sending side all the way through the
onion sending + receiving ends to the user on the receive events.
2023-04-19 14:55:42 +00:00
Matt Corallo
9c55adaa4a Pipe received payment_metadata through the HTLC receipt pipeline
When we receive an HTLC, we want to pass the `payment_metadata`
through to the `PaymentClaimable` event. This does most of the
internal refactoring required to do so - storing a
`RecipientOnionFields` in the inbound HTLC tracking structs,
including the `payment_metadata`.

In the future this struct will allow us to do MPP keysend receipts
(as it now stores an Optional `payment_secret` for all inbound
payments) as well as custom TLV receipts (as the struct is
extensible to store additional fields and the internal API supports
filtering for fields which are consistent across HTLCs).
2023-04-19 14:55:42 +00:00
Matt Corallo
3dd05ab261 continue automatically after fail_htlc in receiving an HTLC
If we receive an HTLC and are processing it a potential MPP part,
we always continue in the per-HTLC loop if we call the `fail_htlc`
macro, thus its nice to actually do the `continue` therein rather
than at the callsites.
2023-04-19 14:55:42 +00:00
Matt Corallo
c1e6a74e0b Add a debug_assert the newly-documented (but existing) requirement
If we add an entry to `claimable_payments` we have to ensure we
actually accept the HTLC we're considering, otherwise we'll end up
with an empty `claimable_payments` entry.
2023-04-19 14:55:02 +00:00
Elias Rohrer
36bf817ec7
Actually test futures builds
Currently the BP `futures` tests rely on `std`. In order to actually
have them run, we should enable `std`, i.e., remove
`--no-default-features`.
2023-04-19 10:25:04 +02:00
Elias Rohrer
0d59417473
Fix unrelated warnings
Just two trivial compiler warnings that are unrelated to the changes
made here.
2023-04-19 10:25:04 +02:00