Create Taproot feature and introduce signer type enum #2289
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,7 +136,7 @@ mod sealed { | |
| // Byte 2 | ||
| BasicMPP | Wumbo | AnchorsZeroFeeHtlcTx, | ||
| // Byte 3 | ||
| ShutdownAnySegwit | Taproot, | ||
| // Byte 4 | ||
| OnionMessages, | ||
| // Byte 5 | ||
|
|
@@ -152,7 +152,7 @@ mod sealed { | |
| // Byte 2 | ||
| BasicMPP | Wumbo | AnchorsZeroFeeHtlcTx, | ||
| // Byte 3 | ||
| ShutdownAnySegwit | Taproot, | ||
| // Byte 4 | ||
| OnionMessages, | ||
| // Byte 5 | ||
|
|
@@ -198,7 +198,7 @@ mod sealed { | |
| // Byte 2 | ||
| AnchorsZeroFeeHtlcTx, | ||
| // Byte 3 | ||
| Taproot, | ||
| // Byte 4 | ||
| , | ||
| // Byte 5 | ||
|
|
@@ -384,6 +384,9 @@ mod sealed { | |
| define_feature!(27, ShutdownAnySegwit, [InitContext, NodeContext], | ||
| "Feature flags for `opt_shutdown_anysegwit`.", set_shutdown_any_segwit_optional, | ||
| set_shutdown_any_segwit_required, supports_shutdown_anysegwit, requires_shutdown_anysegwit); | ||
| define_feature!(31, Taproot, [InitContext, NodeContext, ChannelTypeContext], | ||
|
There was a problem hiding this comment. I think the extension-bolt should be updated to mark
wpaulino marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Should we add a |
||
| "Feature flags for `option_taproot`.", set_taproot_optional, | ||
| set_taproot_required, supports_taproot, requires_taproot); | ||
| define_feature!(39, OnionMessages, [InitContext, NodeContext], | ||
| "Feature flags for `option_onion_messages`.", set_onion_messages_optional, | ||
| set_onion_messages_required, supports_onion_messages, requires_onion_messages); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| use crate::sign::{ChannelSigner, EcdsaChannelSigner}; | ||
|
|
||
| pub(crate) enum ChannelSignerType<ECS: ChannelSigner> { | ||
| // in practice, this will only ever be an EcdsaChannelSigner | ||
| Ecdsa(ECS) | ||
| } | ||
|
|
||
| impl<ECS: EcdsaChannelSigner> ChannelSignerType<ECS> { | ||
| pub(crate) fn as_ref(&self) -> &dyn ChannelSigner { | ||
wpaulino marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| match self { | ||
| ChannelSignerType::Ecdsa(cs) => cs.as_channel_signer() | ||
| } | ||
| } | ||
|
|
||
| pub(crate) fn as_mut(&mut self) -> &mut dyn ChannelSigner { | ||
| match self { | ||
| ChannelSignerType::Ecdsa(cs) => cs.as_mut_channel_signer() | ||
| } | ||
| } | ||
|
|
||
| pub(crate) fn as_ecdsa(&self) -> Option<&ECS> { | ||
| match self { | ||
| ChannelSignerType::Ecdsa(ecs) => Some(ecs), | ||
| #[cfg(taproot)] | ||
| _ => None | ||
| } | ||
| } | ||
|
|
||
| pub(crate) fn as_mut_ecdsa(&mut self) -> Option<&mut ECS> { | ||
| match self { | ||
| ChannelSignerType::Ecdsa(ecs) => Some(ecs), | ||
| #[cfg(taproot)] | ||
| _ => None | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Helper trait for accessing common channel signer methods between different implementations | ||
| pub trait AsChannelSigner { | ||
|
There was a problem hiding this comment. I don't think this needs to be public, right? There was a problem hiding this comment. Oh I guess it doesn't matter since we make the module There was a problem hiding this comment. It actually does, unfortunately. Rust is complaining that ChannelSigner (public) inherits from a trait that would otherwise have a lower visibility, which is a no-no. Thankfully, the compiler doesn't yet detect that that's meaningless. So 🤫 There was a problem hiding this comment. I think this means a downstream crate is unable to implement There was a problem hiding this comment. Not inheriting will break the functionality, unfortunately. It's a crucial aspect. But I have a different idea. There was a problem hiding this comment. Why is the inheritance critical here? Maybe we don't land this PR until we get a few more commits so that reviewers can better understand where its going? There was a problem hiding this comment. Because without inheritance, this won't work: https://stackoverflow.com/questions/28632968/why-doesnt-rust-support-trait-object-upcasting/28664881#28664881 There was a problem hiding this comment. Huh, TIL, but you can take that sample and simply remove the extension and it still compiles? There was a problem hiding this comment. Here's a more self-contained example of what we're trying to achieve: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=4d9d295a47d6da11e2fd58c27ffc3fda There was a problem hiding this comment. The issue is the |
||
| fn as_channel_signer(&self) -> &dyn ChannelSigner; | ||
| fn as_mut_channel_signer(&mut self) -> &mut dyn ChannelSigner; | ||
| } | ||
|
|
||
| impl<CS: ChannelSigner> AsChannelSigner for CS { | ||
| fn as_channel_signer(&self) -> &dyn ChannelSigner { | ||
| self | ||
| } | ||
|
|
||
| fn as_mut_channel_signer(&mut self) -> &mut dyn ChannelSigner { | ||
| self | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While you're at it can we clean up a lot of the long lines in this file (that are now worse).