Secrets should not be exposed in-memory at the interface level as it
would be impossible the implement it against a hardware security
module/secure element.
We have a number of debug assertions which are expected to never
fire when running in a single thread. This is just fine in tests,
and gives us good coverage of our lockorder requirements, but is
not-irregularly surprising to users, who may run with their own
debug assertions in test environments.
Instead, we gate these checks by the `cfg(test)` setting as well as
the `_test_utils` feature, ensuring they run in our own tests, but
not downstream tests.
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.
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.
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.
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.
...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.
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)`.
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.
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:#️⃣:map::HashMap<lightning::chain::transaction::OutPoint,lightning::chain::chainmonitor::MonitorHolder<lightning::util::enforcing_trait_impls::EnforcingSigner>,std::collections:#️⃣:map::RandomState> >::`
