These were added to help debug an encoding issue. However, the encoding
code was moved to the blinded_path module. Additionally, the test vector
used an old TLV encoding.
It is annoying to have to match across all the enums of `Balance` to
just pull out the `claimable_amount_satoshis` value. This helper makes
it easier if you just want to amount.
We currently just print "with non-ASCII error message"
to log when we see non-ASCII chars, but should instead
use our fancy PrintableString type to display the
untrusted string and ignore control chars.
Now that the `get_available_balances` min/max bounds are exact, we
can stop doing all the explicit checks in `send_htlc` entirely,
instead comparing against the `get_available_balances` bounds and
failing if the amount is out of those bounds.
This breaks support for sending amounts below the dust limit if
there is some amount of dust exposure remaining before we hit our
cap, however we will no longer generate such routes anyway.
Now that the value available to send is expected to match the
success or failure of sending exactly, we should assert this in the
`chanmon_consistency` fuzzer.
In the next commit we'll actually rip the checks out of `send_htlc`
which will make this a somewhat less useful test, however fuzzing
on this specific commit can help to reveal bugs.
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 how much adding one additional (dust) HTLC would
impact our total dust exposure, setting the new next-HTLC-minimum
field to require HTLCs be non-dust if required or set our next-HTLC
maximum if we cannot send a dust HTLC but do have some additional
exposure remaining.
We also add some testing when sending to ensure that send failures
are accounted for in our balance calculations.
Fixes#2252.
In the coming commits, in order to ensure all routes we generate
are usable, we'll start calculating the next-HTLC minimum for our
channels and using it in the router. Here we set this up by adding
an always-0 field for it in `ChannelDetails` and use it when
routing.
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 whether one additional HTLC's commitment tx fees
would result in the counterparty's commitment tx fees being greater
than the reserve we've picked for them and, if so, limit our next
HTLC value to only include dust HTLCs.
We also add some testing when sending to ensure that send failures
are accounted for in our balance calculations.
This, and the previous few commits, fixes#1126.
If the `networks` field is present in a received `Init` message, then
we need to make sure our genesis chain hash matches one of those, otherwise
we should disconnect the peer.
We now also always send our genesis chain hash in `Init` messages to
our peers.
This was a fairly old introduction to the spec to allow nodes to indicate
to their peers what chains they are interested in (i.e. will open channels
and gossip for).
We don't do any of the handling of this message in this commit and leave
that to the very next commit, so the behaviour is effectively the same
(ignore networks preference).
When we forward a payment and receive an `update_fulfill_htlc`
message from the downstream channel, we immediately claim the HTLC
on the upstream channel, before even doing a `commitment_signed`
dance on the downstream channel. This implies that our
`ChannelMonitorUpdate`s "go out" in the right order - first we
ensure we'll get our money by writing the preimage down, then we
write the update that resolves giving money on the downstream node.
This is safe as long as `ChannelMonitorUpdate`s complete in the
order in which they are generated, but of course looking forward we
want to support asynchronous updates, which may complete in any
order.
Here we add infrastructure to handle downstream
`ChannelMonitorUpdate`s which are blocked on an upstream
preimage-containing one. We don't yet actually do the blocking which
will come in a future commit.
If a `ChannelMonitorUpdate` was created and given to the user but
left uncompleted when the `ChannelManager` is persisted prior to a
restart, the user likely lost the `ChannelMonitorUpdate`(s). Thus,
we need to replay them for the user, which we do here using the
new `BackgroundEvent::MonitorUpdateRegeneratedOnStartup` variant.
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.
