Matt Corallo [Sun, 15 Jan 2023 23:37:00 +0000 (23:37 +0000)]
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.
Matt Corallo [Fri, 6 Jan 2023 20:05:07 +0000 (20:05 +0000)]
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.
Matt Corallo [Fri, 6 Jan 2023 19:53:47 +0000 (19:53 +0000)]
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.
Wilmer Paulino [Wed, 11 Jan 2023 18:21:29 +0000 (10:21 -0800)]
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.
Matt Corallo [Tue, 10 Jan 2023 06:37:39 +0000 (06:37 +0000)]
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:
Omer Yacine [Fri, 6 Jan 2023 08:18:26 +0000 (10:18 +0200)]
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!(...)
```
Jeffrey Czyz [Fri, 16 Dec 2022 20:06:33 +0000 (14:06 -0600)]
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.
Jeffrey Czyz [Fri, 16 Dec 2022 19:35:50 +0000 (13:35 -0600)]
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.
Matt Corallo [Wed, 28 Dec 2022 18:12:29 +0000 (18:12 +0000)]
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.
Matt Corallo [Wed, 28 Dec 2022 17:44:33 +0000 (17:44 +0000)]
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.
Matt Corallo [Fri, 23 Dec 2022 20:44:24 +0000 (20:44 +0000)]
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.
Matt Corallo [Thu, 22 Dec 2022 21:58:53 +0000 (21:58 +0000)]
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.
9d7bb73b599a7a9d8468a2f0c54d28f0ce6cf543 broke some capitalization
in docs for `sign_invoice`, which we fix here as well as taking
this opportunity to clean up the `sign_invoice` docs more
generally.
Once ChannelManager supports payment retries, it will make more sense for its
current send_payment method to be named send_payment_with_route because
retrying should be the default. Here we get a head start on this by making the
rename in outbound_payment, but not changing the public interface yet.
Swap pending_outbound_payments for OutboundPayments struct
This allows us to move a lot of outbound payment logic out of ChannelManager
and into the new outbound_payment module, and helps avoid growing
ChannelManager when we add retry logic to it in upcoming work.