When we generated a `ChannelMonitorUpdate` during `ChannelManager`
deserialization, we must ensure that it gets processed before any
other `ChannelMonitorUpdate`s. The obvious hook for this is when
taking the `total_consistency_lock`, which makes it unlikely we'll
regress by forgetting this.
Here we add that call in the `PersistenceNotifierGuard`, with a
test-only atomic bool to test that this criteria is met.
`BackgroundEvent` was used to store `ChannelMonitorUpdate`s which
result in a channel force-close, avoiding relying on
`ChannelMonitor`s having been loaded while `ChannelManager`
block-connection methods are called during startup.
In the coming commit(s) we'll also generate non-channel-closing
`ChannelMonitorUpdate`s during startup, which will need to be
replayed prior to any other `ChannelMonitorUpdate`s generated from
normal operation.
In the next commit we'll handle that by handling `BackgroundEvent`s
immediately after locking the `total_consistency_lock`.
Rather than letting `AChannelManager` be bounded by all traits
being `Sized` we make them explicitly `?Sized`. We also make the
trait no longer test-only as it will be used in a coming commit.
In the coming commits we'll need the counterparty node_id when
handling a background monitor update as we may need to resume
normal channel operation as a result. Thus, we go ahead and pipe it
through from the shutdown end, as it makes the codepaths
consistent.
Sadly, the monitor-originated shutdown case doesn't allow for a
required counterparty node_id as some versions of LDK didn't have
it present in the ChannelMonitor.
Our `no-std` locks simply panic if a lock cannot be taken as there
should be no lock contention in a single-threaded environment.
However, the `held_by_thread` debug methods were delegating to the
lock methods which resulted in a panic when asserting that a lock
*is* held by the current thread.
Instead, they are updated here to call the relevant `RefCell`
testing methods.
To support receiving MPP keysends, we will add a new non-backward
compatible field to `PendingHTLCRouting::ReceiveKeysend`, which will
break deserialization of `ChannelManager` on downgrades, so we allow the
user choose whether they want to do this. Note that this commit only
adds the config flag, while full MPP support is added in later commits.
At times, we've noticed that channels with `lnd` counterparties do not
receive messages we expect to in a timely manner (or at all) after
sending them a `ChannelReestablish` upon reconnection, or a
`CommitmentSigned` message. This can block the channel state machine
from making progress, eventually leading to force closes, if any pending
HTLCs are committed and their expiration is met.
It seems common wisdom for `lnd` node operators to periodically restart
their node/reconnect to their peers, allowing them to start from a fresh
state such that the message we expect to receive hopefully gets sent. We
can achieve the same end result by disconnecting peers ourselves
(regardless of whether they're a `lnd` node), which we opt to implement
here by awaiting their response within two timer ticks.
`enqueue_message` simply adds the message to the outbound queue, it
still needs to be written to the socket with `do_attempt_write_data`.
However, since we immediately return an error causing the socket to be
closed, the message never actually gets sent.
If thre's a thread currently handling `PeerManager` events, the
next thread which attempts to handle events will block on the first
and then handle events after the first completes. (later threads
will return immediately to avoid blocking more than one thread).
This works fine as long as the user has a spare thread to leave
blocked, but if they don't (e.g. are running with a single-threaded
tokio runtime) this can lead to a full deadlock.
Instead, here, we never block waiting on another event processing
thread, returning immediately after signaling that the first thread
should start over once its complete to ensure all events are
handled.
While this could lead to starvation as we cause one thread to go
around and around and around again, the risk of that should be
relatively low as event handling should be pretty quick, and it's
certainly better than deadlocking.
Fixes https://github.com/lightningdevkit/rapid-gossip-sync-server/issues/32
Atomic lock simplification suggestion from @andrei-21
`NetworkGraph`'s `PartialEq` impl before this commit was deadlock-prone.
Similarly to `ChannelMonitor`'s, `PartialEq` impl, we use position in
memory for a total lockorder. This uses the assumption that the objects
cannot move within memory while the inner locks are held.
When calculating the amount available to send for the next HTLC, if
we over-count we may create routes which are not actually usable.
Historically this has been an issue, which we resolve over a few
commits.
Here we consider the number of in-flight HTLCs which we are allowed
to push towards a counterparty at once, setting the available
balance to zero if we cannot push any further HTLCs.
We also add some testing when sending to ensure that send failures
are accounted for in our balance calculations.
When calculating the amount available to send for the next HTLC, if
we over-count we may create routes which are not actually usable.
Historically this has been an issue, which we resolve over a few
commits.
Here we include the cost of the commitment transaction fee in our
calculation, subtracting the commitment tx fee cost from the
available as we do in `send_payment`.
We also add some testing when sending to ensure that send failures
are accounted for in our balance calculations.
This commit is based on original work by
Gleb Naumenko <naumenko.gs@gmail.com> and modified by
Matt Corallo <git@bluematt.me>.
In the coming commits we redo our next-HTLC-available logic which
requires some minor test changes for tests which relied on
calculating routes which were not usable.
Here we do a minor prefactor to simplify a test which now no longer
requires later changes.
While its nice to be able to push an HTLC which spends balance that
is removed in our local commitment transaction but awaiting an RAA
from our peer for final removal its by no means a critical feature.
Because peers should really be sending RAAs quickly after we send
a commitment, this should be an exceedingly rare case, and we
already don't expose this as available balance when routing, so
this isn't even made available when sending, only forwarding.
Note that `test_pending_claimed_htlc_no_balance_underflow` is
removed as it tested a case which was only possible because of this
and now is no longer possible.
Also get rid of some trailing whitespace because my text editor likes to do
that.
We'll next add a new variant for max_htlc provided in route hints, which will
be treated differently in scoring.
See https://github.com/lightning/bolts/pull/803
This protect the justice claim of counterparty revoked output. As
otherwise if the all the revoked outputs claims are batched in a
single transaction, low-feerate HTLCs transactions can delay our
honest justice claim transaction until BREAKDOWN_TIMEOUT expires.
Previously, we would panic when failing to construct onion messages in
certain circumstances. Here we opt to always rather error out and don't
panic if something goes wrong during OM packet construction.
Rather than using the std benchmark framework (which isn't
maintained and is unlikely to get any further maintenance), we swap
for criterion, which at least gets us a variable number of test
runs so our benchmarks don't take forever.
We also fix the RGS benchmark to pass now that the file in use is
stale compared to today's date.
When benchmarking our router, we previously only ever tested with
amounts under 1,000 sats, which is an incredibly small amount.
While this ensures we have the maximal number of available channels
to consider, it prevents our scorer from getting exercise across
its range. Further, we only score the immediate path we are
expecting to to send over, and not randomly but rather based on the
amount sent.
Here we try to make the benchmarks a bit more realistic by adding
a new benchmark which attempts to send around 100K sats, which is
a reasonable amount to send over a channel today. We also convert
the scoring data to be randomized based on the seed as well as
attempt to (possibly) find a new route for a much larger value and
score based on that. This potentially allows us to score multiple
potential paths between the source and destination as the large
route-find may return an MPP result.
There's a few route tests which do the same thing as the benchmarks
as they're also a good test. However, they didn't share code, which
is somewhat wasteful, so we fix that here.
This PR aims to create a "stateless" scorer. Instead of passing
in fee params at construction-time, we want to parametrize the
scorer with an associated "parameter" type, which is then
passed to the router function itself, and allows passing
different parameters per route-finding call.
