It currently reads "disconnected from peer which hasn't completed
handshake due to ping timeout", which is confusing.
Instead, it will now read "disconnected from peer which hasn't
completed handshake due to ping/handshake timeout"
Unfortunately, the RAII types used by `RwLock` are not `Send`, which is
why they can't be held over `await` boundaries. In order to allow
asynchronous events processing in multi-threaded environments, we here
allow to process events without holding the `total_consistency_lock`.
We very regularly receive confusion over the super generic
"Peer sent invalid data or we decided to disconnect due to a
protocol error" message, which doesn't say very much. Usually, we
end up disconnecting because we have a duplicate connection with a
peer, which doesn't merit such a scary message.
Instead, here we clarify the error message to just refer to the
fact that we're disconnecting, and note that its usually a dup
connection in a parenthetical.
To match the local signatures found in test vectors, we must make sure
we don't use any additional randomess when generating signatures, as
we'll arrive at a different signature otherwise.
Previously, our local signatures would always be deterministic, whether
we'd grind for low R value signatures or not. For peers supporting
SegWit, Bitcoin Core will generally use a transaction's witness-txid, as
opposed to its txid, to advertise transactions. Therefore, to ensure a
transaction has the best chance to propagate across node mempools in the
network, each of its broadcast attempts should have a unique/distinct
witness-txid, which we can achieve by introducing random nonce data when
generating local signatures, such that they are no longer deterministic.
This allows the `InMemorySigner` to produce its own randomness, which we
plan to use when generating signatures in future work.
We can no longer derive `Clone` due to the `AtomicCounter`, so we opt to
implement it manually.
Verify that an InvoiceRequest was produced from an Offer constructed by
the recipient using the Offer metadata reflected in the InvoiceRequest.
The Offer metadata consists of a 128-bit encrypted nonce and possibly a
256-bit HMAC over the nonce and Offer TLV records (excluding the signing
pubkey) using an ExpandedKey.
Thus, the HMAC can be reproduced from the offer bytes using the nonce
and the original ExpandedKey, and then checked against the metadata. If
metadata does not contain an HMAC, then the reproduced HMAC was used to
form the signing keys, and thus can be checked against the signing
pubkey.
Add support for deriving a transient signing pubkey for each Offer from
an ExpandedKey and a nonce. This facilitates recipient privacy by not
tying any Offer to any other nor to the recipient's node id.
Additionally, support stateless Offer verification by setting its
metadata using an HMAC over the nonce and the remaining TLV records,
which will be later verified when receiving an InvoiceRequest.
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.
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.
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.
