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.
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.
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.
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).
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.
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.
This adds the new `payment_metadata` to `RecipientOnionFields`,
passing the metadata from BOLT11 invoices through the send pipeline
and finally copying them info the onion when sending HTLCs.
This completes send-side support for the new payment metadata
feature.
To support transient signing pubkeys and payer ids for Offers, add
another key derivation to ExpandedKey. Also useful for constructing
metadata for stateless message authentication.
Move utility functions used across all offers modules into a common
module. Avoids duplicating larger utilities such as payment_path across
more than one module.
We correctly send out a gossip channel disable update after one
full time tick being down (1-2 minutes). This is pretty nice in
that it avoids nodes trying to route through our nodes too often
if they're down. Other nodes have a much longer time window,
causing them to have much less aggressive channel disables. Sadly,
at one minute it's not super uncommon for tor nodes to get disabled
(once a day or so on two nodes I looked at), and this causes the
lightning terminal scorer to consider the LDK node unstable (even
though it's the one doing the disabling - so is online). This
causes user frustration and makes LDK look bad (even though it's
probably failing fewer payments).
Given this, and future switches to block-based `channel_update`
timestamp fields, it makes sense to go ahead and switch to delaying
channel disable announcements for 10 minutes. This puts us more in
line with other implementations and reduces gossip spam, at the
cost of less reliable payments.
Fixes#2175, at least the currently visible parts.
When generating a `channel_update` either in response to a fee
configuration change or an HTLC failure, we currently poll the
channel to check if the peer's connected when setting the disabled
bit in the `channel_update`. This could cause cases where we set
the disable bit even though the peer *just* disconnected, and don't
generate a followup broadcast `channel_update` with the disabled
bit unset.
While a node generally shouldn't rebroadcast a `channel_update` it
received in an onion, there's nothing inherently stopping them from
doing so. Obviously in the fee-update case we expect the message to
propagate.
Luckily, since we already "stage" disable-changed updates, we can
check the staged state and use that to set the disabled bit in all
`channel_update` cases.
u16 arrays are used in the historical liquidity range tracker.
Previously, we read them without applying the stride multiple,
reading bytes repeatedly and at an offset, corrupting data as we
go.
This applies the correct stride multiplayer fixing the issue.
In our test utilities, we generally refer to a `Node` struct which
holds a `ChannelManager` and a number of other structs. However, we
use the same utilities in benchmarking, where we have a different
`Node`-like struct. This made moving from macros to functions
entirely impossible, as we end up needing multiple types in a given
context.
Thus, here, we take the pain and introduce some wrapper traits
which encapsulte what we need from `Node`, swapping some of our
macros to functions.
In 6090d9e6a8 we swapped out old
debug assertions that checked that a lock was `try_lock`able to
test that certain locks weren't held when we needed to be able to
take them in some near branch. However, another slipped in after in
the `ChannelMonitorUpdate` handling rework, which is replaced with
the new debug assertions here.
Now that we guarantee `claim_payment` will always succeed we have
to let the user know what the deadline is. We still fail payments
if they haven't been claimed in time, which we now expose in
`PaymentClaimable`.
There's no reason to hold a lock on `per_peer_state` while we're
claiming from a since-closed channel via a `ChannelMonitorUpdate`,
which we stop doing here.
`handle_error` must be called without `per_peer_state` mutex or
`pending_events` mutex locks held or we may risk deadlocks.
Previously we checked this in debug builds in the error path, but
not in the success path.
As it turns out, `funding_transaction_generated`'s error path does
hold a `per_peer_state` lock, which we fix here as well as move the
tests to happen unconditionally.
