Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

descriptor: introduce several Taproot accessors #751

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/descriptor/checksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ impl<'f, 'a> Formatter<'f, 'a> {
}
}

impl<'f, 'a> fmt::Write for Formatter<'f, 'a> {
impl fmt::Write for Formatter<'_, '_> {
fn write_str(&mut self, s: &str) -> fmt::Result {
self.fmt.write_str(s)?;
self.eng.input(s).map_err(|_| fmt::Error)
Expand Down
40 changes: 37 additions & 3 deletions src/descriptor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,40 @@ impl<Pk: MiniscriptKey> Descriptor<Pk> {
Ok(Descriptor::Tr(Tr::new(key, script)?))
}

/// For a Taproot descriptor, returns the internal key.
pub fn internal_key(&self) -> Option<&Pk> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps name it tap_internal_key() to keep it consistent with the other methods and to make it clearer that its taproot-related and taproot-only?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other methods yield a TapTree and are named as such, with no prefix. It's just more obvious that a "taptree" is a taproot object while an "internal key" is less obvious.

I can stick a taproot_ prefix onto here if you want, though that makes the method name longer and won't make it look consistent with the other methods. Since Taproot is the "default" output type for the forseeable future, I would almost rather stick non_taproot_ onto everything else..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Taproot is considered the 'default' then not having a prefix definitely makes sense. I didn't think of it as such.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, well, today it's definitely not the default (as evidenced by you, and me, and miketwenty1, running into a tons of API issues just in the last couple months). But things are trending that way and all the extensions to Bitcoin I see on the horizon work within Taproot, so I think we should assume (for API purposes) that this is the way things are going.

Having said that, maybe the way to do this is to foreground Tr, as you say, and let Taproot-only users use that in place of Descriptor? Not sure.

I can add a taproot_ prefix here if you really want. But I weakly prefer not to.

if let Descriptor::Tr(ref tr) = self {
Some(tr.internal_key())
} else {
None
}
}

/// For a Taproot descriptor, returns the [`TapTree`] describing the Taproot tree.
///
/// To obtain the individual leaves of the tree, call [`TapTree::iter`] on the
/// returned value.
pub fn tap_tree(&self) -> Option<&TapTree<Pk>> {
if let Descriptor::Tr(ref tr) = self {
tr.tap_tree().as_ref()
} else {
None
}
}

/// For a Taproot descriptor, returns an iterator over the scripts in the Taptree.
///
/// If the descriptor is not a Taproot descriptor, **or** if the descriptor is a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this kind of strange, I would prefer to differentiate these two cases so I can handle the 'not Taproot descriptor' as an error case if I was expecting it to be a Taproot.

But I guess that if the distinction is important, one could also explicitly use (my suggested) tap(), handle the None case, then iter.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can differentiate them by calling .tap_tree(), dealing with the Option, then calling iter_scripts. I added this method specifically to get rid of the extra unwrap that would require.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, actually to distinguish a non-taproot descriptor from a keyspend-only descriptor, but without matching and without being able to distinguish further, you would need to call .internal_key().is_some().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this method specifically to get rid of the extra unwrap that would require.

In what scenarios would it make sense to have a code path that handles both taproot and non-taproot descriptors in that way? It seems to me that using iter_tap_tree on non-taproot descriptors would typically be a coding mistake, can't really think of a case where I'd want to do it and have it return a empty iterator

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see from your other comment that you were thinking about already known-taproot descriptors, say if the same code initializes the tr descriptor and calls methods on it. It can make sense for such cases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if you are trying to "just sign a transaction" you might take an arbitrary descriptor, grab its scriptcode to produce a pre-Taproot sighash, then also iterate through all the Taproot branches (if any) to produce Taproot sighashes. In this case you could use the same code for both Taproot and non-Taproot descriptors.

/// Taproot descriptor containing only a keyspend, returns an empty iterator.
pub fn tap_tree_iter(&self) -> tr::TapTreeIter<Pk> {
if let Descriptor::Tr(ref tr) = self {
if let Some(ref tree) = tr.tap_tree() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use tr.iter_scripts()?

return tree.iter();
}
}
tr::TapTreeIter::empty()
}

/// Get the [DescriptorType] of [Descriptor]
pub fn desc_type(&self) -> DescriptorType {
match *self {
Expand Down Expand Up @@ -696,7 +730,7 @@ impl Descriptor<DescriptorPublicKey> {

struct KeyMapWrapper<'a, C: secp256k1::Signing>(KeyMap, &'a secp256k1::Secp256k1<C>);

impl<'a, C: secp256k1::Signing> Translator<String> for KeyMapWrapper<'a, C> {
impl<C: secp256k1::Signing> Translator<String> for KeyMapWrapper<'_, C> {
type TargetPk = DescriptorPublicKey;
type Error = Error;

Expand Down Expand Up @@ -744,7 +778,7 @@ impl Descriptor<DescriptorPublicKey> {
pub fn to_string_with_secret(&self, key_map: &KeyMap) -> String {
struct KeyMapLookUp<'a>(&'a KeyMap);

impl<'a> Translator<DescriptorPublicKey> for KeyMapLookUp<'a> {
impl Translator<DescriptorPublicKey> for KeyMapLookUp<'_> {
type TargetPk = String;
type Error = core::convert::Infallible;

Expand Down Expand Up @@ -907,7 +941,7 @@ impl Descriptor<DefiniteDescriptorKey> {
) -> Result<Descriptor<bitcoin::PublicKey>, ConversionError> {
struct Derivator<'a, C: secp256k1::Verification>(&'a secp256k1::Secp256k1<C>);

impl<'a, C: secp256k1::Verification> Translator<DefiniteDescriptorKey> for Derivator<'a, C> {
impl<C: secp256k1::Verification> Translator<DefiniteDescriptorKey> for Derivator<'_, C> {
type TargetPk = bitcoin::PublicKey;
type Error = ConversionError;

Expand Down
5 changes: 5 additions & 0 deletions src/descriptor/tr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,11 @@ pub struct TapTreeIter<'a, Pk: MiniscriptKey> {
stack: Vec<(u8, &'a TapTree<Pk>)>,
}

impl<Pk: MiniscriptKey> TapTreeIter<'_, Pk> {
/// Helper function to return an empty iterator from Descriptor::tap_tree_iter.
pub(super) fn empty() -> Self { Self { stack: vec![] } }
}

impl<'a, Pk> Iterator for TapTreeIter<'a, Pk>
where
Pk: MiniscriptKey + 'a,
Expand Down
10 changes: 7 additions & 3 deletions src/miniscript/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> PkIter<'a, Pk, Ctx> {
}
}

impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> Iterator for PkIter<'a, Pk, Ctx> {
impl<Pk: MiniscriptKey, Ctx: ScriptContext> Iterator for PkIter<'_, Pk, Ctx> {
type Item = Pk;

fn next(&mut self) -> Option<Self::Item> {
Expand All @@ -199,22 +199,24 @@ impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> Iterator for PkIter<'a, Pk, Ctx>
}
}

// Module is public since it export testcase generation which may be used in
// dependent libraries for their own tasts based on Miniscript AST
/// Module is public since it export testcase generation which may be used in
/// dependent libraries for their own tasts based on Miniscript AST
#[cfg(test)]
pub mod test {
use bitcoin::hashes::{hash160, ripemd160, sha256, sha256d, Hash};

use super::Miniscript;
use crate::miniscript::context::Segwitv0;

/// Test case.
pub type TestData = (
Miniscript<bitcoin::PublicKey, Segwitv0>,
Vec<bitcoin::PublicKey>,
Vec<hash160::Hash>,
bool, // Indicates that the top-level contains public key or hashes
);

/// Generate a deterministic list of public keys of the given length.
pub fn gen_secp_pubkeys(n: usize) -> Vec<secp256k1::PublicKey> {
let mut ret = Vec::with_capacity(n);
let secp = secp256k1::Secp256k1::new();
Expand All @@ -233,13 +235,15 @@ pub mod test {
ret
}

/// Generate a deterministic list of Bitcoin public keys of the given length.
pub fn gen_bitcoin_pubkeys(n: usize, compressed: bool) -> Vec<bitcoin::PublicKey> {
gen_secp_pubkeys(n)
.into_iter()
.map(|inner| bitcoin::PublicKey { inner, compressed })
.collect()
}

/// Generate a deterministic list of test cases of the given length.
pub fn gen_testcases() -> Vec<TestData> {
let k = gen_bitcoin_pubkeys(10, true);
let _h: Vec<hash160::Hash> = k
Expand Down
2 changes: 1 addition & 1 deletion src/miniscript/lex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub enum Token<'s> {
Bytes65(&'s [u8]),
}

impl<'s> fmt::Display for Token<'s> {
impl fmt::Display for Token<'_> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
Token::Num(n) => write!(f, "#{}", n),
Expand Down
40 changes: 30 additions & 10 deletions src/miniscript/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: CC0-1.0

//! # Abstract Syntax Tree
//! Abstract Syntax Tree
//!
//! Defines a variety of data structures for describing Miniscript, a subset of
//! Bitcoin Script which can be efficiently parsed and serialized from Script,
Expand Down Expand Up @@ -289,16 +289,27 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
Ctx::max_satisfaction_size(self).ok_or(Error::ImpossibleSatisfaction)
}

/// Helper function to produce Taproot leaf hashes
fn leaf_hash_internal(&self) -> TapLeafHash
where
Pk: ToPublicKey,
{
TapLeafHash::from_script(&self.encode(), LeafVersion::TapScript)
}

/// Attempt to produce non-malleable satisfying witness for the
/// witness script represented by the parse tree
pub fn satisfy<S: satisfy::Satisfier<Pk>>(&self, satisfier: S) -> Result<Vec<Vec<u8>>, Error>
where
Pk: ToPublicKey,
{
// Only satisfactions for default versions (0xc0) are allowed.
let leaf_hash = TapLeafHash::from_script(&self.encode(), LeafVersion::TapScript);
let satisfaction =
satisfy::Satisfaction::satisfy(&self.node, &satisfier, self.ty.mall.safe, &leaf_hash);
let satisfaction = satisfy::Satisfaction::satisfy(
&self.node,
&satisfier,
self.ty.mall.safe,
&self.leaf_hash_internal(),
);
self._satisfy(satisfaction)
}

Expand All @@ -311,12 +322,11 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
where
Pk: ToPublicKey,
{
let leaf_hash = TapLeafHash::from_script(&self.encode(), LeafVersion::TapScript);
let satisfaction = satisfy::Satisfaction::satisfy_mall(
&self.node,
&satisfier,
self.ty.mall.safe,
&leaf_hash,
&self.leaf_hash_internal(),
);
self._satisfy(satisfaction)
}
Expand Down Expand Up @@ -344,8 +354,12 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
where
Pk: ToPublicKey,
{
let leaf_hash = TapLeafHash::from_script(&self.encode(), LeafVersion::TapScript);
satisfy::Satisfaction::build_template(&self.node, provider, self.ty.mall.safe, &leaf_hash)
satisfy::Satisfaction::build_template(
&self.node,
provider,
self.ty.mall.safe,
&self.leaf_hash_internal(),
)
}

/// Attempt to produce a malleable witness template given the assets available
Expand All @@ -356,16 +370,22 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
where
Pk: ToPublicKey,
{
let leaf_hash = TapLeafHash::from_script(&self.encode(), LeafVersion::TapScript);
satisfy::Satisfaction::build_template_mall(
&self.node,
provider,
self.ty.mall.safe,
&leaf_hash,
&self.leaf_hash_internal(),
)
}
}

impl Miniscript<<Tap as ScriptContext>::Key, Tap> {
/// Returns the leaf hash used within a Taproot signature for this script.
///
/// Note that this method is only implemented for Taproot Miniscripts.
pub fn leaf_hash(&self) -> TapLeafHash { self.leaf_hash_internal() }
}

impl<Ctx: ScriptContext> Miniscript<Ctx::Key, Ctx> {
/// Attempt to parse an insane(scripts don't clear sanity checks)
/// script into a Miniscript representation.
Expand Down
4 changes: 2 additions & 2 deletions src/miniscript/satisfy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ impl_satisfier_for_map_hash_tapleafhash_to_key_taproot_sig! {
impl Satisfier<Pk> for HashMap<(hash160::Hash, TapLeafHash), (Pk, bitcoin::taproot::Signature)>
}

impl<'a, Pk: MiniscriptKey + ToPublicKey, S: Satisfier<Pk>> Satisfier<Pk> for &'a S {
impl<Pk: MiniscriptKey + ToPublicKey, S: Satisfier<Pk>> Satisfier<Pk> for &S {
fn lookup_ecdsa_sig(&self, p: &Pk) -> Option<bitcoin::ecdsa::Signature> {
(**self).lookup_ecdsa_sig(p)
}
Expand Down Expand Up @@ -322,7 +322,7 @@ impl<'a, Pk: MiniscriptKey + ToPublicKey, S: Satisfier<Pk>> Satisfier<Pk> for &'
fn check_after(&self, n: absolute::LockTime) -> bool { (**self).check_after(n) }
}

impl<'a, Pk: MiniscriptKey + ToPublicKey, S: Satisfier<Pk>> Satisfier<Pk> for &'a mut S {
impl<Pk: MiniscriptKey + ToPublicKey, S: Satisfier<Pk>> Satisfier<Pk> for &mut S {
fn lookup_ecdsa_sig(&self, p: &Pk) -> Option<bitcoin::ecdsa::Signature> {
(**self).lookup_ecdsa_sig(p)
}
Expand Down
2 changes: 1 addition & 1 deletion src/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ macro_rules! impl_log_method {
}

#[cfg(feature = "std")]
impl<'a> AssetProvider<DefiniteDescriptorKey> for LoggerAssetProvider<'a> {
impl AssetProvider<DefiniteDescriptorKey> for LoggerAssetProvider<'_> {
impl_log_method!(provider_lookup_ecdsa_sig, pk: &DefiniteDescriptorKey, -> bool);
impl_log_method!(provider_lookup_tap_key_spend_sig, pk: &DefiniteDescriptorKey, -> Option<usize>);
impl_log_method!(provider_lookup_tap_leaf_script_sig, pk: &DefiniteDescriptorKey, leaf_hash: &TapLeafHash, -> Option<usize>);
Expand Down
4 changes: 2 additions & 2 deletions src/primitives/threshold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ struct ThreshDisplay<'t, 's, T, const MAX: usize> {
show_k: bool,
}

impl<'t, 's, T, const MAX: usize> fmt::Display for ThreshDisplay<'t, 's, T, MAX>
impl<T, const MAX: usize> fmt::Display for ThreshDisplay<'_, '_, T, MAX>
where
T: fmt::Display,
{
Expand All @@ -286,7 +286,7 @@ where
}
}

impl<'t, 's, T, const MAX: usize> fmt::Debug for ThreshDisplay<'t, 's, T, MAX>
impl<T, const MAX: usize> fmt::Debug for ThreshDisplay<'_, '_, T, MAX>
where
T: fmt::Debug,
{
Expand Down
2 changes: 1 addition & 1 deletion src/psbt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ impl<'psbt> PsbtInputSatisfier<'psbt> {
pub fn new(psbt: &'psbt Psbt, index: usize) -> Self { Self { psbt, index } }
}

impl<'psbt, Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk> for PsbtInputSatisfier<'psbt> {
impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk> for PsbtInputSatisfier<'_> {
fn lookup_tap_key_spend_sig(&self) -> Option<bitcoin::taproot::Signature> {
self.psbt.inputs[self.index].tap_key_sig
}
Expand Down
Loading