From 25c1d13cfa22fb08b7fdfbec85327b7afb3dbd74 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Sat, 23 May 2026 11:24:13 -0500 Subject: [PATCH 1/3] descriptor: fix taproot tree formatting Commit 552e8440 ("tr: flatten TapTree type") changed TapTree from a recursive enum into a depth-preorder leaf list. The formatter only tracked depth changes between adjacent leaves, which loses binary subtree boundaries for some unbalanced trees and can render descriptors that do not parse back. Format the flat representation with an explicit child-count stack so completed binary subtrees are closed at the right time. --- src/descriptor/tr/taptree.rs | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/descriptor/tr/taptree.rs b/src/descriptor/tr/taptree.rs index 27e860390..9043253e6 100644 --- a/src/descriptor/tr/taptree.rs +++ b/src/descriptor/tr/taptree.rs @@ -89,27 +89,34 @@ fn fmt_helper( f: &mut fmt::Formatter, mut fmt_ms: impl FnMut(&mut fmt::Formatter, &Miniscript) -> fmt::Result, ) -> fmt::Result { - let mut last_depth = 0; + let mut child_counts = Vec::::with_capacity(TAPROOT_CONTROL_MAX_NODE_COUNT); + for item in view.leaves() { - if last_depth > 0 { + let depth = usize::from(item.depth()); + + if !child_counts.is_empty() { f.write_str(",")?; } - while last_depth < item.depth() { + while child_counts.len() < depth { f.write_str("{")?; - last_depth += 1; + child_counts.push(0); } + fmt_ms(f, item.miniscript())?; - while last_depth > item.depth() { + + if let Some(child_count) = child_counts.last_mut() { + *child_count += 1; + } + while let Some(2) = child_counts.last() { f.write_str("}")?; - last_depth -= 1; + child_counts.pop(); + if let Some(child_count) = child_counts.last_mut() { + *child_count += 1; + } } } - while last_depth > 0 { - f.write_str("}")?; - last_depth -= 1; - } Ok(()) } From c8bb341c058e2bac2f604b33f8e912565bc0f475 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Sat, 23 May 2026 11:24:56 -0500 Subject: [PATCH 2/3] descriptor: tr tree formatting regression tests Previous implementation was broken for these two cases: {{A,B},C} was formatted as {{A,B,C}} {{A,B},{C,D}} was formatted as {{A,B,C,D}} These cases are now covered by the test. Also this case was already correct, but was also covered: {A,{B,C}}. Also tested TapTree's own Display and Debug formatting, not as a part of Tr descriptor. --- src/descriptor/tr/mod.rs | 31 ++++++++++++++++++++++++------- src/descriptor/tr/taptree.rs | 25 +++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/src/descriptor/tr/mod.rs b/src/descriptor/tr/mod.rs index 47a72cfff..87484e8f1 100644 --- a/src/descriptor/tr/mod.rs +++ b/src/descriptor/tr/mod.rs @@ -519,20 +519,30 @@ mod tests { use super::*; + fn strip_ws(desc: &str) -> String { desc.replace(&[' ', '\n'][..], "") } + fn descriptor() -> String { let desc = "tr(acc0, { - multi_a(3, acc10, acc11, acc12), { + { + multi_a(3, acc10, acc11, acc12), and_v( v:multi_a(2, acc10, acc11, acc12), after(10) - ), - and_v( - v:multi_a(1, acc10, acc11, ac12), - after(100) ) - } + }, + and_v( + v:multi_a(1, acc10, acc11, ac12), + after(100) + ) })"; - desc.replace(&[' ', '\n'][..], "") + strip_ws(desc) + } + + fn assert_display_roundtrip(desc: &str) { + let desc = strip_ws(desc); + let tr = Tr::::from_str(&desc).unwrap(); + assert_eq!(format!("{:#}", tr), desc); + Tr::::from_str(&tr.to_string()).unwrap(); } #[test] @@ -543,6 +553,13 @@ mod tests { assert!(!tr.for_each_key(|k| k.starts_with("acc"))); } + #[test] + fn display_roundtrips_unbalanced_taptree() { + assert_display_roundtrip(&descriptor()); + assert_display_roundtrip("tr(acc0,{pk(acc1),{pk(acc2),pk(acc3)}})"); + assert_display_roundtrip("tr(acc0,{{pk(acc1),pk(acc2)},{pk(acc3),pk(acc4)}})"); + } + #[test] fn tr_maximum_depth() { // Copied from integration tests diff --git a/src/descriptor/tr/taptree.rs b/src/descriptor/tr/taptree.rs index 9043253e6..cf955083f 100644 --- a/src/descriptor/tr/taptree.rs +++ b/src/descriptor/tr/taptree.rs @@ -291,3 +291,28 @@ impl TapTreeBuilder { #[inline] pub(super) fn finalize(self) -> TapTree { TapTree { depths_leaves: self.depths_leaves } } } + +#[cfg(test)] +mod tests { + use core::str::FromStr; + + use super::*; + + fn leaf(ms: &str) -> TapTree { + TapTree::leaf(Arc::new(Miniscript::::from_str(ms).unwrap())) + } + + #[test] + fn display_single_leaf_taptree() { + let tree = leaf("pk(A)"); + + assert_eq!(format!("{}", tree), "pk(A)"); + } + + #[test] + fn debug_binary_taptree() { + let tree = TapTree::combine(leaf("pk(A)"), leaf("pk(B)")).unwrap(); + + assert_eq!(format!("{:?}", tree), "{[B/onduesm]pk(\"A\"),[B/onduesm]pk(\"B\")}"); + } +} From 870c1f5bc72c03827a67f9c089e7470e81e76a6c Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Sat, 23 May 2026 11:25:05 -0500 Subject: [PATCH 3/3] descriptor: assert non-empty TapTreeBuilder finalization --- src/descriptor/tr/taptree.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/descriptor/tr/taptree.rs b/src/descriptor/tr/taptree.rs index cf955083f..5f38ac766 100644 --- a/src/descriptor/tr/taptree.rs +++ b/src/descriptor/tr/taptree.rs @@ -289,7 +289,10 @@ impl TapTreeBuilder { } #[inline] - pub(super) fn finalize(self) -> TapTree { TapTree { depths_leaves: self.depths_leaves } } + pub(super) fn finalize(self) -> TapTree { + assert!(!self.depths_leaves.is_empty(), "TapTreeBuilder must not finalize an empty tree"); + TapTree { depths_leaves: self.depths_leaves } + } } #[cfg(test)]