rust-lightning

Add clarifying comment to signer_maybe_unblocked

Include fees in SpliceContribution docs

How fees are paid for in a SpliceContribution depends on whether it is a
SpliceIn or SpliceOut. Include this in its docs for clarification.

Fix debug_assert on our_funding_contribution

When processing a splice_ack, the debug_assert on the range of
our_funding_contribution should account for values that are for both
positive (splice-in) and negative (splice-out).

Add splice-out support

Update SpliceContribution with a variant used to support splice-out
(i.e., removing funds from a channel). The TxOut values must not exceed
the users channel balance after accounting for fees and the reserve
requirement.

Support accepting splice-out

When a counterparty sends splice_init with a negative contribution, they
are requesting to remove funds from a channel. Remove conditions
guarding against this and check that they have enough channel balance to
cover the removed funds.

Use a SpliceContribution enum for passing splice-in params

ChannelManager::splice_channel takes individual parameters to support
splice-in. Change these to an enum such that it can be used for
splice-out as well.

Replace funding input tuple with struct

The funding inputs used for splicing and v2 channel establishment are
passed as a tuple of txin, prevtx, and witness weight. Add a struct so
that the items included can be better documented.

Rename ChannelContext::counterparty_prev_commitment_point

To align with the "current" and "next" nomenclature used by
HolderCommitmentPoint, update the naming of the counterparty commitment
point field to use "current" instead of "previous".

Rename ChannelContext::counterparty_cur_commitment_point

To align with the "current" and "next" nomenclature used by
HolderCommitmentPoint, update the naming of the counterparty commitment
point field to use "next" instead of "current".

Rename ChannelContext::cur_counterparty_commitment_transaction_number

To align with the "current" and "next" nomenclature used by
HolderCommitmentPoint, update the naming of the counterparty commitment
transaction number field to use "next" instead of "current".

Return ChannelError instead of calling expect

Delete dead `next_{local, remote}_commitment_tx_fee_info_cached`

The cached fee is never checked in the current test suite.

Add validation of the fees predicted by `next_commitment_stats`

Anytime we build a (feerate, nondust-htlc-count, fee) pair, cache it,
and check that the fee matches if the feerate and nondust-htlc-count
match when building a commitment transaction.

Improve prediction of commitment stats in `can_accept_incoming_htlc`

`ChannelContext::get_pending_htlc_stats` predicts that the set of HTLCs
on the next commitment will be all the HTLCs in
`ChannelContext.pending_inbound_htlcs`, and
`ChannelContext.pending_outbound_htlcs`, as well as all the outbound
HTLC adds in the holding cell.

This is an overestimate:

* Outbound HTLC removals which have been ACK'ed by the counterparty will
certainly not be present in any *next* commitment, even though they
remain in `pending_outbound_htlcs`.

* Outbound HTLCs in the `RemoteRemoved` state, will not be present in
the next *local* commitment.

* Outbound HTLCs in the `LocalAnnounced` state have no guarantee that
they were yet received by the counterparty.

* Outbound `update_add_htlc`'s in the holding cell are certainly not
known by the counterparty, and we will reevaluate their addition to
the channel when freeing the holding cell.

* Inbound HTLCs in the `LocalRemoved` state will not be present in the
next *remote* commitment.

This commit stops using `get_pending_htlc_stats` in favor of the newly
added `ChannelContext::get_next_{local, remote}_commitment_stats`
methods, and fixes the issues described above.

`ChannelContext::next_remote_commit_tx_fee_msat` counts inbound HTLCs in
the `LocalRemoved` state, as well as outbound HTLCs in the
`LocalAnnounced` state. We now do not count them for the same reasons
described above.

Inbound `LocalRemoved` HTLCs that were **not** successful are now
credited to `remote_balance_before_fee_msat` as they will certainly not
be on the next remote commitment. We previously debited these from the
remote balance to arrive at `remote_balance_before_fee_msat`.

We now always check holder dust exposure, whereas we previously would
only do it if the incoming HTLC was dust on our own commitment
transaction.

Furthermore, dust exposure calculations now take a buffer from the
currently committed feerate, and ignore any fee updates in
`ChannelContext.pending_update_fee`.

Add `ChannelContext::get_next_{local, remote}_commitment_stats`

In upcoming commits, these methods will serve as proxies to
`SpecTxBuilder::get_next_commitment_stats` in all validation of channel
updates in `ChannelContext`.

Eventually, these methods will completely replace
`get_pending_htlc_stats`, and
`get_next_{local, remote}_commit_tx_fee_msat`.

When predicting the HTLCs on next commitment, we take the conservative
approach and only assume that a HTLC will not be in the next commitment
when it is guaranteed that it won't be.

Improve prediction of commitment stats in `can_send_update_fee`

`ChannelContext::get_pending_htlc_stats` predicts that the set of HTLCs
on the next commitment will be all the HTLCs in
`ChannelContext.pending_inbound_htlcs`, and
`ChannelContext.pending_outbound_htlcs`, as well as all the outbound
HTLC adds in the holding cell.

This is an overestimate:

* Outbound HTLC removals which have been ACK'ed by the counterparty will
certainly not be present in any *next* commitment, even though they
remain in `pending_outbound_htlcs` (I refer to states
`AwaitingRemoteRevokeToRemove` and `AwaitingRemovedRemoteRevoke`).

* Outbound HTLCs in the `RemoteRemoved` state, will not be present in
the next *local* commitment.

* Inbound HTLCs in the `LocalRemoved` state will not be present in the
next *remote* commitment.

`ChannelContext::build_commitment_stats(funding, true, true, ..)` makes
these errors when predicting the HTLC count on the remote commitment:

* Inbound HTLCs in the state `RemoteAnnounced` are not included, but
they will be in the next remote commitment transaction if the local
ACK's the addition before producing the next remote commitment.

* Inbound HTLCs in the state `AwaitingRemoteRevokeToAnnounce` are not
included, even though the local has ACK'ed the addition.

* Outbound HTLCs in the state `AwaitingRemoteRevokeToRemove` are
counted, even though the local party has ACK'ed the removal.

This commit replaces these functions in favor of the newly added
`ChannelContext::get_next_{local, remote}_commitment_stats` methods,
and fixes the issues described above.

We now always calculate dust exposure using a buffer from
`msg.feerate_per_kw`, and not from
`max(feerate_per_kw, self.feerate_per_kw, self.pending_update_fee)`.

Improve prediction of commitment stats in `validate_update_add_htlc`

`ChannelContext::get_pending_htlc_stats` predicts that the set of HTLCs
on the next commitment will be all the HTLCs in
`ChannelContext.pending_inbound_htlcs`, and
`ChannelContext.pending_outbound_htlcs`, as well as all the outbound
HTLC adds in the holding cell.

This is an overestimate:

* Outbound HTLC removals which have been ACK'ed by the counterparty will
certainly not be present in any *next* commitment, even though they
remain in `pending_outbound_htlcs`.

* Outbound HTLCs in the `RemoteRemoved` state, will not be present in
the next *local* commitment.

* Outbound HTLCs in the `LocalAnnounced` state have no guarantee that
they were received by the counterparty before she sent the
`update_fee`.

* Outbound `update_add_htlc`'s in the holding cell are certainly not
known by the counterparty, and we will reevaluate their addition to
the channel when freeing the holding cell.

* Inbound HTLCs in the `LocalRemoved` state will not be present in the
next *remote* commitment.

`ChannelContext::next_local_commit_tx_fee_msat` over-counts outbound
HTLCs in the `LocalAnnounced` and `RemoteRemoved` states, as well as
outbound `update_add_htlc`'s in the holding cell.

`ChannelContext::next_remote_commit_tx_fee_msat` over-counts inbound
HTLCs in the `LocalRemoved` state, as well as outbound HTLCs in the
`LocalAnnounced` state.

This commit stops using these functions in favor of the newly added
`ChannelContext::get_next_{local, remote}_commitment_stats` methods,
and fixes the issues described above.

If we are the funder, we also check that adding this inbound HTLC
doesn't increase the commitment transaction fee to the point of
exhausting our balance on the local commitment. Previously, we would
only subtract the anchors from `funding.value_to_self_msat`; we now
also subtract the outbound HTLCs on the next local commitment from
`funding.value_to_self_msat` before checking if we can afford the
additional transaction fees.

Inbound `LocalRemoved` HTLCs that were **not** successful are now
credited to `remote_balance_before_fee_msat` as they will certainly not
be on the next remote commitment. We previously debited these from the
remote balance to arrive at `remote_balance_before_fee_msat`.

When calculating dust exposure, we now take a buffer from the currently
committed feerate, and ignore any fee updates in
`ChannelContext.pending_update_fee`.

Add `TxBuilder::get_next_commitment_stats`

Given a snapshot of the lightning state machine,
`TxBuilder::get_next_commitment_stats` calculates the transaction fees,
the dust exposure, and the holder and counterparty balances
(the balances themselves do *not* account for the transaction fee).

Adjust dust exposure due to excess fees for clarity

Improve prediction of commitment stats in `validate_update_fee`

`ChannelContext::get_pending_htlc_stats` predicts that the set of HTLCs
on the next commitment will be all the HTLCs in
`ChannelContext.pending_inbound_htlcs`, and
`ChannelContext.pending_outbound_htlcs`, as well as all the outbound
HTLC adds in the holding cell.

This is an overestimate:

* Outbound HTLC removals which have been ACK'ed by the counterparty will
certainly not be present in any *next* commitment, even though they
remain in `pending_outbound_htlcs`.

* Outbound HTLCs in the `RemoteRemoved` state, will not be present in
the next *local* commitment.

* Outbound HTLCs in the `LocalAnnounced` state have no guarantee that
they were received by the counterparty before she sent the
`update_fee`.

* Outbound `update_add_htlc`'s in the holding cell are certainly not
known by the counterparty, and we will reevaluate their addition to
the channel when freeing the holding cell.

* Inbound HTLCs in the `LocalRemoved` state will not be present in the
next *remote* commitment.

This commit stops using `get_pending_htlc_stats` in favor of the newly
added `ChannelContext::get_next_{local, remote}_commitment_stats`
methods, and fixes the issues described above.

We now always calculate dust exposure using a buffer from
`msg.feerate_per_kw`, and not from
`max(self.feerate_per_kw, msg.feerate_per_kw)`.

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.