rust-lightning

Remove now-unused ServeStaticInvoice::invoice_id

In the initially-merged version of the static invoice server protocol, the
static invoice server would sometimes have to find a specific static invoice
based on (recipient_id, invoice_slot) and sometimetimes based on (recipient_id,
invoice_id). This made the API harder to use in terms of how the server would
index into the KVStore.

Over the course of the previous commits we transitioned to the server always
finding a specific invoice based on (recipient_id, invoice_slot). We still have
a few dangling references to invoice_id in some messages and events though, so
remove those here.

Replace StaticInvoiceReq::invoice_id with ::inv_slot

In the initially-merged version of the static invoice server protocol, the
static invoice server would sometimes have to find a specific static invoice
based on (recipient_id, invoice_slot) and sometime based on (recipient_id,
invoice_id). This made the API harder to use in terms of how the server would
index into the KVStore.

We'd like to transition to the server always finding a specific invoice based on
(recipient_id, invoice_slot) and get rid of the invoice_id concept.

Previously, when an invoice request would come in for the server on behalf of
the often-offline recipient, they would need to find the static invoice based
on the recipient_id and invoice_id. However, previous commits have now led to
the server being able to use the invoice_slot in the initial offer paths that
they create, which we do here, obviating the need for them to create their own
randomly-generated invoice_id. The final dangling references to the invoice_id
will be removed in the next commit.

Remove now-unused ServeStaticInv::inv_slot from OM

In the initially-merged version of the static invoice server protocol, the
static invoice server would sometimes have to find a specific static invoice
based on (recipient_id, invoice_slot) and sometime based on (recipient_id,
invoice_id). This made the API harder to use in terms of how the server would
index into the KVStore.

We'd like to transition to the server always finding a specific invoice based on
(recipient_id, invoice_slot) and get rid of the invoice_id concept.

In the previous commit the server began including the invoice_slot in the
ServeStaticInvoice blinded path context that gets provided back to themselves.
Therefore there is no need for the recipient to redundantly include it in the
ServeStaticInvoice onion message itself.

Track invoice_slot in ServeStaticInvoice context

In the initially-merged version of the static invoice server protocol, the
static invoice server would sometimes have to find a specific static invoice
based on (recipient_id, invoice_slot) and sometime based on (recipient_id,
invoice_id). This made the API harder to use in terms of how the server would
index into the KVStore.

We'd like to transition to the server always finding a specific invoice based on
(recipient_id, invoice_slot) and get rid of the invoice_id concept.

As part of this series of commits, include the invoice_slot in the
ServeStaticInvoice blinded path context that the server creates when sending an
offer_paths message. This is possible due to a previous commit including the
invoice_slot in the initial offer_paths_request from the recipient, and lays
the groundwork for removing the invoice_id field from this blinded path
context.

Cache pending offer in specific invoice slot

When we as an async recipient receive offer paths from the static invoice server,
we create an offer and cache it, retrying persisting a corresponding invoice with
the server until it succeeds.

In the initially-merged version of this protocol, we would put this cached
offer in any slot in the cache that needed an offer at the time the offer paths
were received. However, in the last commit we started requesting offer paths
for a specific slot in the cache, as part of eliminating the use of the
invoice_id field in the overall protocol.

As a result, here we put the cached offer in the specific cache slot that the
original OfferPathsRequest indicated, rather than any slot that could use a new
offer.

Account for shared input EMPTY_SCRIPT_SIG_WEIGHT

When splicing a channel, the previous funding output is spent and fees
for it are paid by the splice initiator. However, the witness weight was
not including EMPTY_SCRIPT_SIG_WEIGHT. Fix this and update the variable
name to make clear the weight needed is the input satisfaction.

Include invoice_slot in OfferPathsRequest message

In the initially-merged version of the static invoice server protocol, the
static invoice server would sometimes have to find a specific static invoice
based on (recipient_id, invoice_slot) and sometimes based on (recipient_id,
invoice_id). This made the API harder to use in terms of how the server would
index into the KVStore.

We'd like to transition to the server always finding a specific invoice based on
(recipient_id, invoice_slot) and get rid of the invoice_id concept.

Now that the invoice_slot is in the initial paths request, the server will be
able to include the slot in the offer paths that they create in response,
allowing the slot to be surfaced instead of the invoice_id when the invoice
request comes in, in upcoming commits.

Remove TransactionU16LenLimited

TransactionU16LenLimited was used to limit Transaction serialization
size to u16::MAX. This was because messages can not be longer than
u16::MAX bytes when serialized for the transport layer. However, this
limit doesn't take into account other fields in a message containing a
Transaction, including the length of the transaction itself.

Remove TransactionU16LenLimited and instead check any user supplied
transactions in the context of the enclosing message (e.g. TxAddInput).

Remove their_funding_contribution_satoshis from FundingNegotiationContext

Once the counterparty supplies their funding contribution, there is no
longer a need to store it in FundingNegotiationContext as it will have
already been used to create a FundingScope.

Include witness weights in FundingNegotiationContext

ChannelManager::splice_channel takes witness weights with the funding
inputs. Storing these in FundingNegotiationContext allows us to use them
when calculating the change output and include them in a common struct
used for initiating a splice-in.

In preparation for having ChannelManager::splice_channel take
FundingTxContributions, add a weight to the
FundingTxContributions::InputsOnly, which supports the splice-in use
case.

Check splice contributions against SignedAmount::MAX_MONEY

Splice contributions should never exceed the total bitcoin supply. This
check prevents a potential overflow when converting the contribution
from sats to msats. The commit additionally begins to store the
contribution using SignedAmount.

Fetch HolderCommitmentPoint::current_point on read

When reading HolderCommitmentPoint, attempt to fetch the current point
if it wasn't serialized. This allows channels to be spliced without
first needing to have the HolderCommitmentPoint advanced. Don't fail if
it can't be fetch synchronously as the channel can still be spliced once
it is advanced.

Set HolderCommitmentPoint::current_point on read

When introducing HolderCommitmentPoint::current_point, the value was
mistakenly not set when read except in the legacy case where the next
point needed to be fetched. But in that case, it would have been read as
None given it is a new field.

Fix comment regarding holder commitment point

Use WarnAndDisconnect to fail a splice

When the current holder commitment point is unavailable for a channel,
we can't splice the channel. Make sure to disconnect so that the channel
is no longer quiescent. Otherwise, it cannot be used for payments.

Re-phrase splice_channel error message

Make `ChannelMonitor` round-trip tests more robust

During testsing, we check that a `ChannelMonitor` will round-trip
through serialization exactly. However, we recently added a fix to
change a value in `PackageTemplate` on reload to fix some issues in
the field in 0.1. This can cause the round-trip tests to fail as a
field is modified during read.

We fix it here by simply exempting the field from the equality test
in the condition where it would be updated on read.

We also make the `ChannelMonitor` `PartialEq` trait implementation
non-public as weird workarounds like this make clear that such a
comparison is a britle API at best.

Correct `test_dup_htlc_onchain_doesnt_fail_on_reload`

`test_dup_htlc_onchain_doesnt_fail_on_reload` made reference to
`ChainMonitor` persisting `ChannelMonitor`s on each new block,
which hasn't been the case in some time. Instead, we update the
comment and code to make explicit that it doesn't impact the test.

Allow quiescence-init while disconnected from peers

There are a number of things in LDK where we've been lazy and not
allowed the user to initiate an action while a peer is
disconnected. While it may be accurate in the sense that the action
cannot be started while the peer is disconnected, it is terrible
dev UX - these actions can fail without the developer being at
fault and the only way for them to address it is just try again.

Here we fix this dev UX shortcoming for splicing, keeping any
queued post-quiescent actions around when a peer disconnects and
retrying the action (and quiescence generally) when the peer
reconnects.

Stop skipping the line in quiescence if our peer speaks first

In the case where we prepare to initiate quiescence, but cannot yet
send our `stfu` because we're waiting on some channel operations to
settle, and our peer ultimately sends their `stfu` before we can,
we would detect this case and, if we were able, send an `stfu`
which would allow us to send "something fundamental" first.

While this is a nifty optimization, its a bit overkill - the chance
that both us and our peer decide to attempt something fundamental
at the same time is pretty low, and worse this required additional
state tracking.

We simply remove this optimization here, simplifying the quiescence
state machine a good bit.

Add a `QuiescentAction` to track why we're going quiescent

When we initiate quiescence, it should always be because we're
trying to accomplish something (in the short term only splicing).
In order to actually do that thing, we need to store the
instructions for that thing somewhere the splicing logic knows to
look at once we reach quiescence.

Here we add a simple enum which will eventually store such actions.

Update `test_peer_storage` style to match newer tests

...somewhat cleaning up rustfmt crust.

Move `test_peer_storage` to `reload_tests`

In general we shouldn't be adding new tests in `channelmanager.rs`

Correct comments and flow in `test_peer_storage`

This cleans up `test_peer_storage` a bit to clarify what messages
are actually being exchanged.

Add debug assertions on revoked counterparty tx number fetching

We actually store the latest counterparty revoked commitment
transaction number in two places in `Channel` - as
`context.cur_counterparty_commitment_transaction_number + 2` and
in `commitment_secrets`.

Here we add a debug assertion that both values are equal.