Merge pull request #1956 from TheBlueMatt/2023-01-ser-cleanups

Trivial Serialization Tweaks

Merge pull request #1968 from lexe-tech/max/net-tokio-no-arc

lightning-net-tokio: Don't require `Arc<PeerManager>`

lightning-net-tokio: Don't require `Arc<PeerManager>`

Merge pull request #1962 from TheBlueMatt/2023-01-bp-no-std

Use the user-provided `SleepFuture` for interval checks in `background-processor`

Merge pull request #1960 from tnull/2023-01-deref-router-in-invoicepayer

Make `InvoicePayer`'s `Router` `Deref`

Merge pull request #1957 from TheBlueMatt/2022-01-mon-ref-lockorder

Pass MonitorUpdates by ref and tweak manager lockorder

Use a variable-length integer for many serialization wrappers

The lightning protocol uses u16s for lengths in most cases. As our
serialization framework primarily targets that, we must as well.
However, because we may serialize objects that have  more than 65K
entries, we want to be able to store larger values. Thus, we define
a variable length integer here which is backwards-compatible but
treats 0xffff as "read eight more bytes".

This doesn't address any specific known issue, but feels like good
practice just in case.

Make `background-processor` no-std-friendly (ish)

This makes `background-processor` build without `std` at all. This
isn't particularly useful in the general no-std case as
`background-processor` is only useful with the `futures` feature,
and async will generally need `std` in some way or another. Still,
it ensures we don't end up reintroducing a dependency on the
current time, which breaks `wasm` use-cases.

Merge pull request #1955 from TheBlueMatt/2023-01-1950-message-text

Fix link text in `reply_short_channel_ids_end`

Merge pull request #1963 from lexe-tech/max/derive-debug

derive `Debug` for `ConfirmationTarget`

derive `Debug` for `ConfirmationTarget`

Merge pull request #1959 from danielgranhao/stop-passing-init-features-in-msg-handlers

Stop passing `InitFeatures` in msg handlers

Merge pull request #1939 from TheBlueMatt/2022-01-fuzz-hashbrown

Misc fuzzing tweaks

Use the user-provided `SleepFuture` for interval checks in BP

`background-processor` does a number of jobs on various timers.
Instead of doing those by interrogating `std::time::Instant`, this
change swaps to using the existing user-provided sleep future.

Fixes #1864.

Use flexible timer types in background processor's regular jobs

`background-processor` does a number of jobs on various timers.
Currently, those are all done by checking the timers every 100ms
by interrogating `std::time::Instant`. This is fine for the
threaded version, but we'd like more flexibility in the `futures`-
based `background-processor`.

Here we swap the `std::time::Instant` interrogation for a lambda
which we will switch out to the user-provided sleeper in the next
commit.

Make `InvoicePayer`'s `Router` `Deref`

As of HEAD the `ChannelManager` is parametrized by a `Router`, while
`InvoicePayer` also owns a `Router`. In order to allow for a single
object being reused, we make the `InvoicePayer` side `Deref`.

Stop passing `InitFeatures` in msg handlers

Swap per_peer_state lock order

Pass monitor updates by reference, not owned

In the next commit(s) we'll start holding `ChannelMonitorUpdate`s
that are being persisted in `Channel`s until they're done
persisting. In order to do that, switch to applying the updates by
reference instead of value.

Use `ChannelUnavailable` for a peer disconnecting not `MisuseError`

This fixes a crash in the `full_stack_target` fuzz test (found by
Chaincode's generous fuzzing infrastructure!) but ultimately is a
better error code - a peer disconnecting before we can fund a
channel isn't a "misuse error" its an unavailable channel.

Make fuzz assertions explicit in Cargo.toml

Do not rely on auto-deref'ing when aaccessing a `Hash{Map,Set}`

In newer versions of `hashbrown` this code would be broken. While
we aren't updating `hashbrown` any time soon (as it requires an
MSRV bump), it is useful to swap for a newer `hashbrown` when
fuzzing, which this makes easier.

Switch to hashbrown in fuzzing

hashbrown by default uses ahash, which may be a bit faster, but
more importantly, if we upgrade to hashbrown 0.13/ahash 0.8 we can
make it use a constant randomization factor, making fuzzers happier.

Bump fuzz `afl` dependency to latest upstream version

Convert `Vec` de/serialization impl to a macro and impl for tuples

...to make it easier to add new implementations and implement it
for all tuples which implement `Readabe` + `Writeable`. Note that
we don't want to just convert to a blanket implementation as we'd
really like to keep our optimized `Vec<u8>` wrapper or we'll end
up spinning way too much when writing vecs of bytes.

Add support for including `ignorable` types in enum de/ser macros

An enum implements de/serialization via
`impl_writeable_tlv_based_enum_upgradable` currently cannot contain
an object that only implements `MaybeReadable`.

This solves that by implementing the required blocks for
`ignorable`, opting to return `Ok(None)` in the top-level read in
cases where the inner field read returns `Ok(None)`.

Implement (de)serialization for BTreeMap same as HashMap

Fix link text in `reply_short_channel_ids_end`

Merge pull request #1950 from tnull/2023-01-fix-doc-warnings-and-nits

Fix doc warnings and doc cleanup in `msgs.rs`/`ser.rs`

Merge pull request #1946 from wpaulino/init-features-user-config

Use UserConfig to determine advertised InitFeatures by ChannelManager

Merge pull request #1953 from arik-so/2023-01-debug-gossip-heisenbug

Allow manually passing a timestamp to `channel_failed`. Fixes #1914.

Add error messages to stale gossip cleanup assertions.

Should help debug #1914.

Use UserConfig to determine advertised InitFeatures by ChannelManager

This is purely a refactor that does not change the InitFeatures
advertised by a ChannelManager. This allows users to configure which
features should be advertised based on the values of `UserConfig`. While
there aren't any existing features currently leveraging this behavior,
it will be used by the upcoming anchors_zero_fee_htlc_tx feature.

The UserConfig dependency on provided_init_features caused most
callsites of the main test methods responsible for opening channels to
be updated. This commit foregos that completely by no longer requiring
the InitFeatures of each side to be provided to these methods. The
methods already require a reference to each node's ChannelManager to
open the channel, so we use that same reference to obtain their
InitFeatures. A way to override such features was required for some
tests, so a new `override_init_features` config option now exists on
the test harness.

Merge pull request #1930 from arik-so/2022-12-remove-keysinterface

Remove KeysInterface

Fix misc. warnings from `--document-private-items`

Fix docs and export `impl_tlv_based_enum` variants

Fix misc doc warnings.

Fix doc warnings and cleanup in `msgs.rs`

Fix doc warnings and cleanup in `ser.rs`

Decouple lifetimes for trait implementations that used to comprise KeysInterface.

Split up generic parameters that used to comprise KeysInterface.

Remove KeysInterface trait.

Merge pull request #1823 from mariocynicys/expose-tlv-macros2

Expose `impl_writeable_tlv_based` macro

Merge pull request #1507 from ViktorTigerstrom/2022-05-store-channels-per-peer

Store `channels` per-peer

Merge pull request #1944 from TheBlueMatt/2022-01-lockorder-windows-robust

Make `debug_sync` regex more robust

Merge pull request #1943 from andrei-21/feature/handle-read-dir-errors

Handle `read_dir()` errors in persister

Make `debug_sync` regex more robust

On windows the symbol names appear to sometimes be truncated,
which causes the symbol name to not include the `::new` at the end.
This causes the regex to mis-match and track the wrong location
for the mutex construction, leading to bogus lockorder violations.

For example, in testing the following symbol name appeared on
Windows, without the function name itself:

`lightning::debug_sync::RwLock<std::collections::hash::map::HashMap<lightning::chain::transaction::OutPoint,lightning::chain::chainmonitor::MonitorHolder<lightning::util::enforcing_trait_impls::EnforcingSigner>,std::collections::hash::map::RandomState> >::`

Move tests from debug_sync to a new submodule

This will allow us to change the module regex match in debug_sync
to make it more robust.

Move `debug_sync` to the new `sync` folder

Move `no-std` sync implementations to a folder to clean up

Handle read_dir() errors in persister

Remove the `ChannelManager::channel_state`

Expect `pending_msg_events` to be in random peer order in tests

Store `pending_msg_events` per peer

Use correct node in `test_invalid_upfront_shutdown_script`

Avoid unnecessary looping over all peers' channels

Update `id_to_peer` docs with consistency guarantees info

Remove one tab level when accessing a `peer_state`

Remove unnecessary channel counterparty checks

Add handle unkown peer test

Add duplicate temporary_channel_id for 2 peers test

Unify failure to query `Channel` error messages

Avoid unnecessary immediate retake `per_peer_state` lock

Remove unnecessary `per_peer_state` branch

After `channels` are now stored in the `per_peer_state`, some logic can
be simplified and extra accessing of the `per_peer_state` can be
removed.

Store channels per peer

Merge pull request #1941 from andrei-21/feature/rework-unwrap

Rework `unwrap()` call in persistence

Fix an incorrect assertion in tlv stream encoding

Types must be unique and monotonically increasing (using < instead of <=)

Expose `impl_writeable_tlv_based` macro

Every exported macro needed to have all the macros used inside it:
1- to be exported as well.
2- be called from the `$crate` namespace so it works in other crates.

Some structs in `lightning::util::ser` needed to be made public as they were used inside the exported macros.

Use the macros like this:
```Rust
lightning::impl_writeable_tlv_based!(...)
```

Merge pull request #1940 from TheBlueMatt/2023-01-nostd-try-lock

Rework unwrap() call in persistence

Make the no-std `RwLockGuard` `try_lock` actually try

There doesn't appear to be any reason to have `try_lock` fail, and
future work shouldn't need to check for std to use `try_lock`.

Add `try_write` function to `FairRwLock`

Add `ChannelManager::PeerState::latest_features` docs

Add `counterparty_node` to test macros

Merge pull request #1927 from jkczyz/2022-12-invoice-rework

Pre-work for BOLT 12 invoices

Define blinded hop features for use in BOLT 12

BOLT 12 invoices may contain blinded_payinfo for each hop in a blinded
path. Each blinded_payinfo contains features, whose length must be
encoded since there may be multiple hops.

Note these features are also needed in the BOLT 4 encrypted_data_tlv
stream. But since they are a single TLV record, the length must *not* be
encoded there.

Define BOLT 12 invoice features with MPP support

Use explicit WithoutLength for BOLT 12 features

Most BOLT 12 features are used as the value of a TLV record and thus
don't use an explicit length. One exception is the features inside the
blinded payinfo subtype since the TLV record contains a list of them.
However, these features are also used in the BOLT 4 encrypted_data_tlv
TLV stream as a single record, where the length is implicit.

Implement Readable and Writeable for Features wrapped in WithoutLength
such that either serialization can be used where required.

Remove Option from InvoiceRequest::signature

Refunds don't have signatures and now use their own abstraction.
Therefore, signatures can be required in invoice requests as per the
spec.

Correct documentation about Refund::payer_id

The docs incorrectly stated that Refund::payer_id is for signing, where
it is only used for identifying a node if Refund::paths is not present.

Merge pull request #1812 from valentinewallace/2022-10-chanman-router-param

Parameterize `ChannelManager` by a `Router`

Test utils: allow queueing >2 persistence update results

ser_macros: rename check_tlv_order

.. to disamgibutate from check_encoded_tlv_order

Provide a `static_value` TLV field serialization type

This is useful in the type serialization definition macros to avoid
writing or reading a field at all, simply using a static value on
each reload.

Parameterize Simple*ChannelManager with DefaultRouter and ProbScorer

Take in-flight HTLCs by reference in Router::find_route

Useful in upcoming work when for payment retries.

Implement routing against the netgraph in tests

Merge pull request #1935 from TheBlueMatt/2022-12-no-non-time-panic

Ensure derive_channel_keys doesn't panic if per-run seed is high

Parameterize ChannelManager by a Router trait

This will be used in upcoming work to fetch routes on-the-fly for payment
retries, which will no longer be the responsibility of InvoicePayer.

Ensure the per-channel key derivation counter doesn't role over

Previously, the `derive_channel_keys` derivation ID asserted that
the high bit of the per-channel key derivation counter doesn't
role over as it checked the 31st bit was zero. As we no longer do
that, we should ensure the assertion in `generate_channel_keys_id`
asserts that we don't role over.

Merge pull request #1934 from TheBlueMatt/2022-12-113-bindings-upstream

Trivial Bindings Updates

Merge pull request #1936 from TheBlueMatt/2023-01-in-flight-clone

`#[derive(Clone)]` for `InFlightHtlcs`

`#[derive(Clone)]` for `InFlightHtlcs`

This is useful for bindings, and generally isn't a bad thing for
users to have access to.

Ensure `derive_channel_keys` doesn't panic if per-run seed is high

b04d1b868fe28bea2e4c711e6e6d2470d2b98d77 changed the way we
calculate the `channel_keys_id` to include the 128-bit
`user_channel_id` as well, shifting the counter up four bytes and
the `starting_time_nanos` field up into the second four bytes.

In `derive_channel_keys` we hash the full `channel_keys_id` with an
HD-derived key from our master seed. Previously, that key was
derived with an index of the per-restart counter, re-calculated by
pulling the second four bytes out of the `user_channel_id`. Because
the `channel_keys_id` fields were shifted up four bytes, that is
now a reference to the `starting_time_nanos` value. This should be
fine, the derivation doesn't really add any value here, its all
being hashed anyway, except that derivation IDs must be below 2^31.
This implies that we panic if the user passes a
`starting_time_nanos` which has the high bit set. For those using
the nanosecond part of the current time this isn't an issue - the
value cannot exceed 1_000_000, which does not have the high bit
set, however, some users may use some other per-run seed.

Thus, here we simply drop the high bit from the seed, ensuring we
don't panic. Note that this is backwards compatible as it only
changes the key derivation in cases where we previously panicked.

Ideally we'd drop the derivation entirely, but that would break
backwards compatibility of key derivation.

No-export `&self` methods on non-cloneable enum(s)

Specifically, `OnionMessageContents` is a non-cloneable enum, which
isn't stored opaque so we cannot call `&self` methods on it.
Because its methods aren't critical to the API for now, we simply
no-export them rather than trying to work out an alternative
approach.

Store an owned `Score` in `ScorerAccountingForInFlightHtlcs`

`ScorerAccountingForInFlightHtlcs` generally stores a `Score`
reference generated by calling `LockableScore::lock`, which
actually returns an arbitrary `Score`. Given `Score` is implemented
directly on lock types, it makes sense to simply hold a fully owned
`Score` in `ScorerAccountingForInFlightHtlcs` rather than a mutable
reference to one.

Merge pull request #1929 from valentinewallace/2022-12-outbound-payment-mod-followup

Outbound payment module follow-up

outbound_payment: put method signature closing paren on next line

in long method signatures

Merge pull request #1931 from TheBlueMatt/2022-12-1910-followups

Trivial #1910 Followups