This may avoid risk of bugs in the future as it requires the caller
to think about the fee being used, not just blindly use the current
(committed) channel feerate.
In upcoming commits, we'll be making the payment secret and payment hash/preimage
derivable from info about the payment + a node secret. This means we don't
need to store any info about incoming payments and can eventually get rid of the
channelmanager::pending_inbound_payments map.
Traits in top-level modules is somewhat confusing - generally
top-level modules are just organizational modules and don't contain
things themselves, instead placing traits and structs in
sub-modules. Further, its incredibly awkward to have a `scorer`
sub-module, but only have a single struct in it, with the relevant
trait it is the only implementation of somewhere else. Not having
`Score` in the `scorer` sub-module is further confusing because
it's the only module anywhere that references scoring at all.
Currently, we write out the Channel's `ChannelTypeFeatures` as an
odd type, implying clients which don't understand the
`ChannelTypeFeatures` field can simply ignore it. This is obviously
nonsense if the channel type is some future version - the client
needs to fail to deserialize as it doesn't understand the channel's
type.
We adapt the serialization logic here to only write out the
`ChannelTypeFeatures` field if it is something other than
only-static-remote-key, and simply consider that "default" (as it
is the only supported type today). Then, we write out the channel
type as an even TLV, implying clients which do not understand it
must fail to read the `Channel`.
Note that we do not need to bother reserving the TLV type no longer
written as it never appeared in a release (merged post-0.0.103).
We currently assume our counterparty is naive and misconfigured and
may force-close a channel to get an HTLC we just forwarded them.
There shouldn't be any reason to do this - we don't have any such
bug, and we shouldn't start by assuming our counterparties are
buggy. Worse, this results in refusing to forward payments today,
failing HTLCs for largely no reason.
Instead, we keep a fairly conservative check, but not one which
will fail HTLC forwarding spuriously - testing only that the HTLC
doesn't expire for a few blocks from now.
Fixes#1114.
If we send a payment and fail to update the first-hop channel state
with a `PermanentFailure` ChannelMonitorUpdateErr, we would have an
entry in our pending payments map, but possibly not return the
PaymentId back to the user to retry the payment, leading to a (rare
and relatively minor) memory leak.
I realized on my own node that I don't have any visibility into how
long a monitor or manager persistence call takes, potentially
blocking other operations. This makes it much more clear by adding
a relevant log_trace!() print immediately before and immediately
after persistence.
Scorer uses time to determine how much to penalize a channel after a
failure occurs. Parameterizing it by time cleans up the code such that
no-std support is in a single AlwaysPresent struct, which implements the
Time trait. Time is implemented for std::time::Instant when std is
available.
This parameterization also allows for deterministic testing since a
clock could be devised to advance forward as needed.
NetworkGraph is owned by NetGraphMsgHandler, but DefaultRouter requires
a reference to it. Introduce shared ownership to NetGraphMsgHandler so
that both can use the same NetworkGraph.
As payments fail, the channel responsible for the failure may be
penalized. Implement Scorer::payment_path_failed to penalize the failed
channel using a configured penalty. As time passes, the penalty is
reduced using exponential decay, though penalties will accumulate if the
channel continues to fail. The decay interval is also configurable.
This ensures we don't let a hung connection stick around forever if
the peer never completes the initial handshake.
This also resolves a race where, on receiving a second connection
from a peer, we may reset their_node_id to None to prevent sending
messages even though the `channel_encryptor`
`is_ready_for_encryption()`. Sending pings only checks the
`channel_encryptor` status, not `their_node_id` resulting in an
`unwrap` on `None` in `enqueue_message`.
An upcoming Router interface will be used for finding a Route both when
initially sending a payment and also when retrying failed payment paths.
Unify the three varieties of get_route so the interface can consist of a
single method implemented by the new `find_route` method. Give get_route
pub(crate) visibility so it can still be used in tests.
