To separate out the logic in the `sign` module, which will start to be
convoluted with multiple signer types, we're splitting out each signer
type into its own submodule, following the taproot.rs example from a
previous commit.
ChannelSignerType is an enum that contains variants of all currently
supported signer types. Given that those signer types are enumerated
as associated types in multiple places, it is prudent to denote one
type as the authority on signer types.
SignerProvider seemed like the best option. Thus, instead of
ChannelSignerType declaring the associated types itself, it simply
uses their definitions from SignerProvider.
Previously, SignerProvider was not laid out to support multiple signer
types. However, with the distinction between ECDSA and Taproot signers,
we now need to account for SignerProviders needing to support both.
This approach does mean that if ever we introduced another signer type
in the future, all implementers of SignerProvider would need to add it
as an associated type, and would also need to write a set of dummy
implementations for any Signer trait they do not wish to support.
For the time being, the TaprootSigner associated type is cfg-gated.
For Taproot support, we need to define an alternative trait to
EcdsaChannelSigner. This trait will be implemented by all signers
that wish to support Taproot channels.
If we're behind exactly one commitment (which we've revoked), we'd
previously force-close the channel, guaranteeing we'll lose funds
as the counterparty has our latest local commitment state's
revocation secret.
While this shouldn't happen because users should never lose data,
sometimes issues happen, and we should ensure we always panic.
Further, `test_data_loss_protect` is updated to test this case.
When we reestablish there are generally always 4 conditions for
both local and remote commitment transactions:
* we're stale and have possibly lost data
* we're ahead and the peer has lost data
* we're caught up
* we're nearly caught up and need to retransmit one update.
In especially the local commitment case we had a mess of different
comparisons, which is improved here. Further, the error messages
are clarified and include more information.
In 7f0fd868ad, `channel_keys_id` was
added as an argument to `SignerProvider::get_destination_script`,
allowing implementors to generate a new script for each channel.
This is great, however users then have no way to re-derive the
corresponding private key when they ultimately receive a
`SpendableOutputDescriptor::StaticOutput`. Instead, they have to
track all the addresses as they derive them separately. In many
cases this is fine, but we should support both deployments, which
we do here by simply including the missing `channel_keys_id` for
the user.
Currently all channel keys and their basepoints exist uniformly as
`PublicKey` type, which not only makes in harder for a developer to
distinguish those entities, but also does not engage the language
type system to check if the correct key is being used in any
particular function.
Having struct wrappers around keys also enables more nuanced
semantics allowing to express Lightning Protocol rules in language.
For example, the code allows to derive `HtlcKey` from
`HtlcBasepoint` and not from `PaymentBasepoint`.
This change is transparent for channel monitors that will use the
internal public key of a wrapper.
Payment, DelayedPayment, HTLC and Revocation basepoints and their
derived keys are now wrapped into a specific struct that make it
distinguishable for the Rust type system. Functions that require a
specific key or basepoint should not use generic Public Key, but
require a specific key wrapper struct to engage Rust type
verification system and make it more clear for developers which
key is used.
Now that we use the `rust-bitcoin` `WitnessProgram` to check our
addresses, we can just rely on it, rather than checking the program
length and version.
`rust-bitcoin` 0.30 added `#[non_exhaustive]` to the `Network`
enum, allowing them to "add support" for a new network type without
a major version change in the future. When upgrading, we added a
simple `unreachable` for the general match arm, which would break
in a minor version change of `rust-bitcoin`.
While it seems [possible rust-bitcoin will change
this](https://github.com/rust-bitcoin/rust-bitcoin/issues/2225),
we still shouldn't ba panicking, which we drop here in favor of a
`debug_assert`ion, and a default value.
We give some more information while reducing the log levels to make the
logging less spammy.
We also convert one safe-to-unwrap case from returning an error to
unwrapping the value.
- If a path within a route passes through the same channelID twice,
that shows the path is looped and will be rejected by nodes.
- Add a check to explicitly reject such payment before trying to send
them.
In other languages (Java and C#, notably), overriding `Eq` without
overriding `Hash` can lead to surprising or broken behavior. Even
in Rust, its usually the case that you actually want both. Here we
add missing `Hash` derivations for P2P messages, to at least
address the first pile of warnings the C# compiler dumps.
Implementation of standard traits on arrays longer than 32 elements
was shipped in rustc 1.47, which is below our MSRV of 1.48 and we
can use to remove some unnecessary manual implementation of
`PartialEq` on `OnionPacket`.
