Skip to content

Commit

Permalink
expression: unify Taproot and non-Taproot parsing
Browse files Browse the repository at this point in the history 
The `expression::Tree` type now parses expression trees containing both
{} and () as children. It marks which is which, and it is the
responsibility of FromTree implementors to make sure that the correct
style is used.

This eliminates a ton of ad-hoc string parsing code from
descriptor/tr.rs, including 8 instances of Error::Unexpected. It is also
the first change that preserves (sorta) a pubkey parsing error rather
than converting it to a string.

Because of rust-bitcoin#734
this inserts a call to `Descriptor::sanity_check` when parsing a string,
specifically for Taproot descriptors. This is weird and wrong but we
want to address it separately from this PR, whose goal is to preserve
all behavior.
  • Loading branch information
@apoelstra
apoelstra committed Nov 13, 2024
1 parent beacc8d commit 7b6f49d
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 183 deletions.
38 changes: 27 additions & 11 deletions src/descriptor/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use bitcoin::{
};
use sync::Arc;

use crate::expression::FromTree as _;
use crate::miniscript::decode::Terminal;
use crate::miniscript::{satisfy, Legacy, Miniscript, Segwitv0};
use crate::plan::{AssetProvider, Plan};
Expand Down Expand Up @@ -981,17 +982,17 @@ impl<Pk: FromStrKey> crate::expression::FromTree for Descriptor<Pk> {
impl<Pk: FromStrKey> FromStr for Descriptor<Pk> {
type Err = Error;
fn from_str(s: &str) -> Result<Descriptor<Pk>, Error> {
// tr tree parsing has special code
// Tr::from_str will check the checksum
// match "tr(" to handle more extensibly
let desc = if s.starts_with("tr(") {
Ok(Descriptor::Tr(Tr::from_str(s)?))
} else {
let top = expression::Tree::from_str(s)?;
expression::FromTree::from_tree(&top)
}?;

Ok(desc)
let top = expression::Tree::from_str(s)?;
let ret = Self::from_tree(&top)?;
if let Descriptor::Tr(ref inner) = ret {
// FIXME preserve weird/broken behavior from 12.x.
// See https://github.com/rust-bitcoin/rust-miniscript/issues/734
ret.sanity_check()?;
for (_, ms) in inner.iter_scripts() {
ms.ext_check(&crate::miniscript::analyzable::ExtParams::sane())?;
}
}
Ok(ret)
}
}

Expand Down Expand Up @@ -1545,6 +1546,21 @@ mod tests {
)
}

#[test]
fn tr_named_branch() {
use crate::{ParseError, ParseTreeError};

assert!(matches!(
StdDescriptor::from_str(
"tr(0202d44008000010100000000084F0000000dd0dd00000000000201dceddd00d00,abc{0,0})"
),
Err(Error::Parse(ParseError::Tree(ParseTreeError::IncorrectName {
expected: "",
..
}))),
));
}

#[test]
fn roundtrip_tests() {
let descriptor = Descriptor::<bitcoin::PublicKey>::from_str("multi");
Expand Down
234 changes: 104 additions & 130 deletions src/descriptor/tr.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// SPDX-License-Identifier: CC0-1.0

use core::str::FromStr;
use core::{cmp, fmt, hash};

#[cfg(not(test))] // https://github.com/rust-lang/rust/issues/121684
Expand All @@ -12,9 +11,10 @@ use bitcoin::taproot::{
use bitcoin::{opcodes, Address, Network, ScriptBuf, Weight};
use sync::Arc;

use super::checksum::{self, verify_checksum};
use super::checksum;
use crate::descriptor::DefiniteDescriptorKey;
use crate::expression::{self, FromTree};
use crate::iter::TreeLike as _;
use crate::miniscript::satisfy::{Placeholder, Satisfaction, SchnorrSigType, Witness};
use crate::miniscript::Miniscript;
use crate::plan::AssetProvider;
Expand All @@ -23,8 +23,8 @@ use crate::policy::Liftable;
use crate::prelude::*;
use crate::util::{varint_len, witness_size};
use crate::{
Error, ForEachKey, FromStrKey, MiniscriptKey, Satisfier, ScriptContext, Tap, Threshold,
ToPublicKey, TranslateErr, Translator,
Error, ForEachKey, FromStrKey, MiniscriptKey, ParseError, Satisfier, ScriptContext, Tap,
Threshold, ToPublicKey, TranslateErr, Translator,
};

/// A Taproot Tree representation.
Expand Down Expand Up @@ -490,79 +490,114 @@ where
}
}

#[rustfmt::skip]
impl<Pk: FromStrKey> Tr<Pk> {
// Helper function to parse taproot script path
fn parse_tr_script_spend(tree: &expression::Tree,) -> Result<TapTree<Pk>, Error> {
match tree {
expression::Tree { name, args, .. } if !name.is_empty() && args.is_empty() => {
let script = Miniscript::<Pk, Tap>::from_str(name)?;
Ok(TapTree::Leaf(Arc::new(script)))
}
expression::Tree { name, args, .. } if name.is_empty() && args.len() == 2 => {
let left = Self::parse_tr_script_spend(&args[0])?;
let right = Self::parse_tr_script_spend(&args[1])?;
Ok(TapTree::combine(left, right))
}
_ => {
Err(Error::Unexpected(
"unknown format for script spending paths while parsing taproot descriptor"
.to_string(),
))},
}
impl<Pk: FromStrKey> core::str::FromStr for Tr<Pk> {
type Err = Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let expr_tree = expression::Tree::from_str(s)?;
Self::from_tree(&expr_tree)
}
}

impl<Pk: FromStrKey> crate::expression::FromTree for Tr<Pk> {
fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
if top.name == "tr" {
match top.args.len() {
1 => {
let key = &top.args[0];
if !key.args.is_empty() {
return Err(Error::Unexpected(format!(
"#{} script associated with `key-path` while parsing taproot descriptor",
key.args.len()
)));
fn from_tree(expr_tree: &expression::Tree) -> Result<Self, Error> {
use crate::expression::{Parens, ParseTreeError};

expr_tree
.verify_toplevel("tr", 1..=2)
.map_err(From::from)
.map_err(Error::Parse)?;

let mut round_paren_depth = 0;

let mut internal_key = None;
let mut tree_stack = vec![];

for item in expr_tree.verbose_pre_order_iter() {
// Top-level "tr" node.
if item.index == 0 {
if item.is_complete {
debug_assert!(
internal_key.is_some(),
"checked above that top-level 'tr' has children"
);
}
} else if item.index == 1 {
// First child of tr, which must be the internal key
if !item.node.args.is_empty() {
return Err(Error::Parse(ParseError::Tree(
ParseTreeError::IncorrectNumberOfChildren {
description: "internal key",
n_children: item.node.args.len(),
minimum: Some(0),
maximum: Some(0),
},
)));
}
internal_key = Some(
Pk::from_str(item.node.name)
.map_err(ParseError::box_from_str)
.map_err(Error::Parse)?,
);
} else {
// From here on we are into the taptree.
if item.n_children_yielded == 0 {
match item.node.parens {
Parens::Curly => {
if !item.node.name.is_empty() {
return Err(Error::Parse(ParseError::Tree(
ParseTreeError::IncorrectName {
actual: item.node.name.to_owned(),
expected: "",
},
)));
}
if round_paren_depth > 0 {
return Err(Error::Parse(ParseError::Tree(
ParseTreeError::IllegalCurlyBrace {
pos: item.node.children_pos,
},
)));
}
}
Parens::Round => round_paren_depth += 1,
_ => {}
}
Tr::new(expression::terminal(key, Pk::from_str)?, None)
}
2 => {
let key = &top.args[0];
if !key.args.is_empty() {
return Err(Error::Unexpected(format!(
"#{} script associated with `key-path` while parsing taproot descriptor",
key.args.len()
)));
if item.is_complete {
if item.node.parens == Parens::Curly {
if item.n_children_yielded == 2 {
let rchild = tree_stack.pop().unwrap();
let lchild = tree_stack.pop().unwrap();
tree_stack.push(TapTree::combine(lchild, rchild));
} else {
return Err(Error::Parse(ParseError::Tree(
ParseTreeError::IncorrectNumberOfChildren {
description: "Taptree node",
n_children: item.n_children_yielded,
minimum: Some(2),
maximum: Some(2),
},
)));
}
} else {
if item.node.parens == Parens::Round {
round_paren_depth -= 1;
}
if round_paren_depth == 0 {
let script = Miniscript::from_tree(item.node)?;
// FIXME hack for https://github.com/rust-bitcoin/rust-miniscript/issues/734
if script.ty.corr.base != crate::miniscript::types::Base::B {
return Err(Error::NonTopLevel(format!("{:?}", script)));
};
tree_stack.push(TapTree::Leaf(Arc::new(script)));
}
}
let tree = &top.args[1];
let ret = Self::parse_tr_script_spend(tree)?;
Tr::new(expression::terminal(key, Pk::from_str)?, Some(ret))
}
_ => Err(Error::Unexpected(format!(
"{}[#{} args] while parsing taproot descriptor",
top.name,
top.args.len()
))),
}
} else {
Err(Error::Unexpected(format!(
"{}[#{} args] while parsing taproot descriptor",
top.name,
top.args.len()
)))
}
}
}

impl<Pk: FromStrKey> core::str::FromStr for Tr<Pk> {
type Err = Error;
fn from_str(s: &str) -> Result<Self, Self::Err> {
let desc_str = verify_checksum(s)
.map_err(From::from)
.map_err(Error::ParseTree)?;
let top = parse_tr_tree(desc_str)?;
Self::from_tree(&top)
assert!(tree_stack.len() <= 1);
Tr::new(internal_key.unwrap(), tree_stack.pop())
}
}

Expand All @@ -588,69 +623,6 @@ impl<Pk: MiniscriptKey> fmt::Display for Tr<Pk> {
}
}

// Helper function to parse string into miniscript tree form
fn parse_tr_tree(s: &str) -> Result<expression::Tree, Error> {
if s.len() > 3 && &s[..3] == "tr(" && s.as_bytes()[s.len() - 1] == b')' {
let rest = &s[3..s.len() - 1];
if !rest.contains(',') {
let key = expression::Tree::from_str(rest)?;
if !key.args.is_empty() {
return Err(Error::Unexpected("invalid taproot internal key".to_string()));
}
let internal_key = expression::Tree {
name: key.name,
parens: expression::Parens::None,
children_pos: 0,
args: vec![],
};
return Ok(expression::Tree {
name: "tr",
parens: expression::Parens::Round,
children_pos: 0,
args: vec![internal_key],
});
}
// use str::split_once() method to refactor this when compiler version bumps up
let (key, script) = split_once(rest, ',')
.ok_or_else(|| Error::BadDescriptor("invalid taproot descriptor".to_string()))?;

let key = expression::Tree::from_str(key)?;
if !key.args.is_empty() {
return Err(Error::Unexpected("invalid taproot internal key".to_string()));
}
let internal_key = expression::Tree {
name: key.name,
parens: expression::Parens::None,
children_pos: 0,
args: vec![],
};
let tree = expression::Tree::from_slice_delim(script, expression::Deliminator::Taproot)
.map_err(Error::ParseTree)?;
Ok(expression::Tree {
name: "tr",
parens: expression::Parens::Round,
children_pos: 0,
args: vec![internal_key, tree],
})
} else {
Err(Error::Unexpected("invalid taproot descriptor".to_string()))
}
}

fn split_once(inp: &str, delim: char) -> Option<(&str, &str)> {
if inp.is_empty() {
None
} else {
// find the first character that matches delim
let res = inp
.chars()
.position(|ch| ch == delim)
.map(|idx| (&inp[..idx], &inp[idx + 1..]))
.unwrap_or((inp, ""));
Some(res)
}
}

impl<Pk: MiniscriptKey> Liftable<Pk> for TapTree<Pk> {
fn lift(&self) -> Result<Policy<Pk>, Error> {
fn lift_helper<Pk: MiniscriptKey>(s: &TapTree<Pk>) -> Result<Policy<Pk>, Error> {
Expand Down Expand Up @@ -767,6 +739,8 @@ where

#[cfg(test)]
mod tests {
use core::str::FromStr;

use super::*;

fn descriptor() -> String {
Expand Down
Loading

0 comments on commit 7b6f49d

Please sign in to comment.