In `update_claims_view_from_matched_txn` we have two different
tx-equivalence checks which do the same thing - both check that the
tx which appeared on chain spent all of the outpoints which we
intended to spend in a given package. While one is more effecient
than the other (but only usable in a subset of cases), the
difference between O(N) and O(N^2) when N is 1-5 is trivial.
Still, it is possible we hit this code with just shy of 900 HTLC
outputs in a channel, and a transaction with a ton of inputs.
While having to spin through a few million entries if our
counterparty wastes a full block isn't really a big deal, we go
ahead and use a sorted vec and binary searches because its trivial.
In 19daccf7fb5ea81c8d235c1628a91efe0aa07b96, a `PackageId` type was
added to differentiate between an opaque Id for packages and the
`Txid` type which was being used for that purpose. It, however,
failed to also replace the single inner field in
`OnchainEvent::Claim` which was also a package ID. We do so here.
This prevents downgrading to older versions of LDK that are not capable
of supporting anchor channels when the field is serialized (i.e.,
opt_anchors is `Some`).
Now that our txids will no longer be stable for package claims that
require external funds to be allocated, we transition to a 32-byte array
identifier to remain compatible with them.
Previously, this method assumed that all HTLC transactions have 1 input
and 1 output, with the sole input having a witness of 5 elements. This
will no longer be the case for HTLC transactions on channels with
anchors outputs since additional inputs and outputs can be attached to
them to allow fee bumping.
This expands the outbound-HTLC-listing support in `ChannelMonitor`
to include not only the set of outbound HTLCs which have not yet
been resolved but to also include the full set of HTLCs which the
`ChannelMonitor` is currently able to to or has already finalized.
This will be used in the next commit to fail-back HTLCs which were
removed from a channel in the ChannelMonitor but not in a Channel.
Using the existing `get_pending_outbound_htlcs` for this purpose is
subtly broken - if the channel is already closed, an HTLC fail may
have completed on chain and is no longer "pending" to the monitor,
but the fail event is still in the monitor waiting to be handed
back to the `ChannelMonitor` when polled.
Similar to the previous commit, we introduce a new serialization version
that doesn't store a monitor's signer. Since the monitor already knows
of a channel's `channel_keys_id`, there's no need to store any new data
to re-derive all private key material for said channel.
Now that ready_channel is also called on startup upon deserializing
channels, we opt to rename it to a more indicative name.
We also derive `PartialEq` on ChannelTransactionParameters to allow
implementations to determine whether `provide_channel_parameters` calls
are idempotent after the channel parameters have already been provided.
`get_channel_signer` previously had two different responsibilites:
generating unique `channel_keys_id` and using said ID to derive channel
keys. We decide to split it into two methods `generate_channel_keys_id`
and `derive_channel_signer`, such that we can use the latter to fulfill
our goal of re-deriving signers instead of persisting them. There's no
point in storing data that can be easily re-derived.
The `derive_{public,private}_revocation_key` methods hash the two
input keys and then multiply the two input keys by hashed values
before adding them together. Because addition can fail if the tweak
is the inverse of the secret key this method currently returns a
`Result`.
However, it is not cryptographically possible to reach the error
case - in order to create an issue, the point-multiplied-by-hash
values must be the inverse of each other, however each point
commits the SHA-256 hash of both keys together. Thus, because
changing either key changes the hashes (and the ultimate points
added together) in an unpredictable way, there should be no way to
construct such points.
The `derive_{public,private}_key` methods hash the two input keys
and then add them to the input public key. Because addition can
fail if the tweak is the inverse of the secret key this method
currently returns a `Result`.
However, it is not cryptographically possible to reach the error
case - in order to create an issue, the SHA-256 hash of the
`base_point` (and other data) must be the inverse of the
`base_point`('s secret key). Because changing the `base_point`
changes the hash in an unpredictable way, there should be no way to
construct such a `base_point`.
In many complexity-reduced implementations of chain syncing using
esplora `transactions_confirmed` may be called redundantly for
transactions which were already confirmed. To ensure this is
idempotent we add two new `ConnectionStyle`s in our tests which
(a) call `transactions_confirmed` twice for each call, ensuring
simple idempotency is ensured and (b) call `transactions_confirmed`
once for each historical block every time we're connecting a new
block, ensuring we're fully idempotent even if every call is
repeated constantly.
In order to actually behave correctly this requires a simple
already-confirmed check in `ChannelMonitor`, which is included.
In `ChannelMonitor`s, when a transaction containing a spend of a
revoked remote output reaches 6 confs, we may have no other
tracking of that txid remaining. Thus, if we see that transaction
again (because a user duplicatively confirms it), we'll generate a
redundant spendable output event for it.
Here we simply explicitly track all txids of transactions which
confirm with a spendable output, allowing us to check this
condition in the next commit.
In `ChannelMonitor`, if we see a `transaction_unconfirmed` for a
transaction we last saw in a block at height X, we shouldn't
*only* remove the `onchain_events_awaiting_threshold_conf` entry
for the given tx but rather for all transactions that we last saw
at height >= X.
This avoids any potential `onchain_events_awaiting_threshold_conf`
inconsistencies due to the order in whcih users mark transactions
unconfirmed (which the `chain::Confirm` docs do not currently set
any requirements on).
This also matches the `OnchainTxHandler` behavior, which does the
same lookup.
Previously, `Confirm::get_relevant_txids()` only returned a list of
transactions that have to be monitored for reorganization out of the
chain. This interface however required double bookkeeping: while we
internally keep track of the best block, height, etc, it would also
require the user to keep track which transaction was previously
confirmed in which block and to take actions based on any change, e.g,
to reconfirm them when the block would be reorged-out and the
transactions had been reconfirmed in another block.
Here, we track the confirmation block hash internally and return it via
`Confirm::get_relevant_txids()` to the user, which alleviates the
requirement for double bookkeeping: the user can now simply check
whether the given transaction is still confirmed and in the given block,
and take action if not.
We also split `update_claims_view`: Previously it was one, now it's two
methods: `update_claims_view_from_matched_txn` and
`update_claims_view_from_requests`.
These claims will never be valid as a previous claim has already
confirmed. If a previous claim is reorged out of the chain, a new claim
will be generated bypassing the new behavior.
While this doesn't change much for our existing transaction-based
claims, as broadcasting an already confirmed transaction acts as a NOP,
it prevents us from yielding redundant event-based claims, which will be
introduced as part of the anchors patchset.
In c986e52ce8, an `MppId` was added
to `HTLCSource` objects as a way of correlating HTLCs which belong
to the same payment when the `ChannelManager` sees an HTLC
succeed/fail. This allows it to have awareness of the state of all
HTLCs in a payment when it generates the ultimate user-facing
payment success/failure events. This was used in the same PR to
avoid generating duplicative success/failure events for a single
payment.
Because the field was only used as an internal token to correlate
HTLCs, and retries were not supported, it was generated randomly by
calling the `KeysInterface`'s 32-byte random-fetching function.
This also provided a backwards-compatibility story as the existing
HTLC randomization key was re-used for older clients.
In 28eea12bbe `MppId` was renamed to
the current `PaymentId` which was then used expose the
`retry_payment` interface, allowing users to send new HTLCs which
are considered a part of an existing payment.
At no point has the payment-sending API seriously considered
idempotency, a major drawback which leaves the API unsafe in most
deployments. Luckily, there is a simple solution - because the
`PaymentId` must be unique, and because payment information for a
given payment is held for several blocks after a payment
completes/fails, it represents an obvious idempotency token.
Here we simply require the user provide the `PaymentId` directly in
`send_payment`, allowing them to use whatever token they may
already have for a payment's idempotency token.
