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.
In order to test Scorer hermetically, sleeps must be avoided. Add a
SinceEpoch abstraction for manually advancing time. Implement the Time
trait for SinceEpoch so that it can be used with ScorerUsingTime in
tests.
The bindings don't currently support passing `Vec`s of objects
which it mappes as "opaque types". This is because it will require
clones to convert its own list of references to Rust's list of
objects.
In the near future we should resolve this limitation, allowing us
to revert this (and make `find_route`'s method signature similarly
cleaner), but for now we must avoid `Vec<OpaqueType>`.
Scorer should be serialized to retain penalty data between restarts.
Implement (de)serialization for Scorer by serializing last failure times
as duration since the UNIX epoch. For no-std, the zero-Duration is used.
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.
Move channel failure penalty logic into a ChannelFailure abstraction.
This encapsulates the logic for accumulating penalties and decaying them
over time. It also is responsible for the no-std behavior. This cleans
up Scorer and will make it easier to serialize it.
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.
This rewrites a good chunk of the retry logic in `InvoicePayer` to
address two issues:
* it was not considering the return value of `send_payment` (and
`retry_payment`) may indicate a failure on some paths but not
others,
* it was not considering that more failures may still come later
when removing elements from the retry count map. This could
result in us seeing an MPP-partial-failure, failing to retry,
removing the retries count entry, and then retrying other parts,
potentially forever.
Users can provide anything they want as `RouteParameters` so we
shouldn't assume any fields are set any particular way, including
`expiry_time` set at all.
