Verify channel-monitor processes blocks with skipped best_block
This can happen due to races b/w client's call to block_connect
and adding newly created channel-monitor to chain-monitor using
watch_channel in funding_created.
Duncan Dean [Wed, 26 Jul 2023 09:20:08 +0000 (11:20 +0200)]
Introduce `ChannelPhase` enum
We introduce the `ChannelPhase` enum which will contain the different
channel structs wrapped by each of its variants so that we can place
these within a single `channel_by_id` map in `peer_state` in the
following commits. This will reduce the number of map lookup operations
we need to do in `ChannelManager`'s various methods. It will also make
certain channel counting logic easier to reason about with less risk of
forgetting to modify logic when new channels structs are introduced for
V2 channel establishment.
Replace a constant three retry attempts for BOLT 12 invoice payments
with a retry strategy specified when creating a pending outbound
payment. This is configured by users in a later commit when constructing
an InvoiceRequest or a Refund.
An upcoming commit requires serializing Retry, so use a type with a
fixed byte length. Otherwise, using eight bytes to serialize a usize
would fail to read on 32-bit machines.
Add a send_payment_for_bolt12_invoice method to OutboundPayments for
initiating payment of a BOLT 12 invoice. This will be called from an
OffersMessageHandler, after which any retries are handled using the
Retryable logic.
Consolidate the creation and insertion of onion_session_privs to the
PendingOutboundPayment::Retryable arm. In an upcoming commit, this
method will be reused for an initial BOLT 12 invoice payment. However,
onion_session_privs are created using a different helper.
Jeffrey Czyz [Wed, 30 Aug 2023 17:01:15 +0000 (12:01 -0500)]
Add PendingOutboundPayment::InvoiceReceived
When a BOLT 12 invoice has been received, a payment attempt is made and
any errors result in abandoning the PendingOutboundPayment. This results
in generating at PaymentFailed event, which has a PaymentHash. Thus,
when receiving an invoice, transition from AwaitingInvoice to a new
InvoiceReceived state, the latter of which contains a PaymentHash such
the abandon_payment helper can still be used.
When a BOLT 12 invoice has been requested, in order to guarantee
invoice payment idempotency the future payment needs to be tracked. Add
an AwaitingInvoice variant to PendingOutboundPayment such that only
requested invoices are paid only once. Timeout after a few timer ticks
if a request has not been received.
When an invoice is requested but either receives an error or never
receives a response, surface an event to indicate to the user that the
corresponding future payment has failed.
When receiving a BOLT 12 invoice originating from either an invoice
request or a refund, the invoice should only be paid once. To accomplish
this, require that the invoice includes an encrypted payment id in the
payer metadata. This allows ChannelManager to track a payment when
requesting but prior to receiving the invoice. Thus, it can determine if
the invoice has already been paid.
Jeffrey Czyz [Thu, 24 Aug 2023 20:16:53 +0000 (15:16 -0500)]
Add an encryption key to ExpandedKey for Offers
Metadata such as the PaymentId should be encrypted when included in an
InvoiceRequest or a Refund, as it is user data and is exposed to the
payment recipient. Add an encryption key to ExpandedKey for this purpose
instead of reusing offers_base_key.
InvoiceRequest::verify_and_respond_using_derived_keys takes a payment
hash. To avoid generating one for invoice requests that ultimately
cannot be verified, split the method into one for verifying and another
for responding.
Matt Corallo [Mon, 28 Aug 2023 18:39:04 +0000 (18:39 +0000)]
Drop dep `tokio`'s `io-util` feat as it broke MSRV and isn't useful
We use `tokio`'s `io-util` feature to provide the
`Async{Read,Write}Ext` traits, which allow us to simply launch a
read future or `poll_write` directly as well as `split` the
`TcpStream` into a read/write half. However, these traits aren't
actually doing much for us - they are really just wrapping the
`readable` future (which we can trivially use ourselves) and
`poll_write` isn't doing anything for us that `poll_write_ready`
can't.
Similarly, the split logic is actually just `Arc`ing the
`TcpStream` and busy-waiting when an operation is busy to prevent
concurrent reads/writes. However, there's no reason to prevent
concurrent access at the stream level - we aren't ever concurrently
writing or reading (though we may concurrently read and write,
which is fine).
Worse, the `io-util` feature broke MSRV (though they're likely to
fix this upstream) and carries two additional dependencies (only
one on the latest upstream tokio).
Arik Sosman [Sat, 26 Aug 2023 00:34:10 +0000 (17:34 -0700)]
Fix flaky aggregated HTLC revocation test.
Releasing write locks in between monitor updates
requires storing a set of cloned keys to iterate
over. For efficiency purposes, that set of keys
is an actual set, as opposed to array, which means
that the iteration order may not be consistent.
The test was relying on an event array index to
access the revocation transaction. We change that
to accessing a hash map keyed by the txid, fixing
the test.
Arik Sosman [Fri, 25 Aug 2023 19:31:33 +0000 (12:31 -0700)]
Release write lock between monitor update iterations.
Previously, updating block data on a chain monitor
would acquire a write lock on all of its associated
channel monitors and not release it until the loop
completed.
Now, we instead acquire it on each iteration,
fixing #2470.
jbesraa [Tue, 22 Aug 2023 15:57:06 +0000 (18:57 +0300)]
Split LockableScore responsibilities between read & write operations
- Split Score from LockableScore to ScoreLookUp to handle read
operations and ScoreUpdate to handle write operations
- Change all struct that implemented Score to implement ScoreLookUp
and/or ScoreUpdate
- Change Mutex's to RwLocks to allow multiple data readers
- Change LockableScore to Deref in ScorerAccountingForInFlightHtlcs
as we only need to read
- Add ScoreLookUp and ScoreUpdate docs
- Remove reference(&'a) and Sized from Score in ScorerAccountingForInFlightHtlcs
as Score implements Deref
- Split MultiThreadedScoreLock into MultiThreadedScoreLockWrite and MultiThreadedScoreLockRead.
After splitting LockableScore, we split MultiThreadedScoreLock following
the same way, splitting a single score into two srtucts, one for read and
other for write.
MultiThreadedScoreLock is used in c_bindings.
Router: account for blinded path fee, etc on first_hop<>intro hop add
This previously led to a debug panic in the router because we wouldn't account
for the blinded path fee when calculating first_hop<>intro_node hop's available
liquidity and construct an invalid path that forwarded more over said hop than
was actually available.
This also led to us hitting unreachable code, see direct_to_matching_intro_nodes
test description.