From 1259375d7b7c91053e09d1cbe3db983612fe301c Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 31 Aug 2024 19:29:01 +0000 Subject: [PATCH] miniscript: make display prefer 'u' over 'l' in the fragment l:0 When fuzzing my new non-recursive tree parsing logic, I noticed that we were deviating from the released 12.0 in the way that we display l:0. This is an ambiguous fragment which can be displayed either as l:0 or u:0. In our released code we use u:0 so stick with that. This was unintentially changed as part of #722. Change it back. While we are at it add a regression test for https://github.com/rust-bitcoin/rust-miniscript/pull/735 --- src/descriptor/mod.rs | 15 +++++++++++++++ src/miniscript/display.rs | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index d6e706bb8..487bbcc45 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -1036,6 +1036,21 @@ mod tests { ); } + #[test] + fn display_prefers_u() { + // The fragments u:0 and l:0 are identical in terms of Script and + // in terms of the in-memory representation -- OrI(False, False). + // Test that the way we display the ambiguous fragment doesn't + // change, in case somebody somehow is depending on it. + let desc = StdDescriptor::from_str("sh(u:0)").unwrap(); + assert_eq!("sh(u:0)#ncq3yf9h", desc.to_string()); + + // This is a regression test for https://github.com/rust-bitcoin/rust-miniscript/pull/735 + // which was found at the same time. It's just a bug plain and simple. + let desc = StdDescriptor::from_str("sh(and_n(u:0,1))").unwrap(); + assert_eq!("sh(and_n(u:0,1))#5j5tw8nm", desc.to_string()); + } + #[test] fn desc_rtt_tests() { roundtrip_descriptor("c:pk_k()"); diff --git a/src/miniscript/display.rs b/src/miniscript/display.rs index 2a4e9f757..c6510b7d0 100644 --- a/src/miniscript/display.rs +++ b/src/miniscript/display.rs @@ -260,14 +260,14 @@ impl Terminal { Terminal::ZeroNotEqual(..) => "n", Terminal::AndV(_, ref r) if matches!(r.as_inner(), Terminal::True) => "t", Terminal::AndV(..) => "and_v", - Terminal::AndB(..) => "and_b", Terminal::AndOr(_, _, ref c) if matches!(c.as_inner(), Terminal::False) => "and_n", + Terminal::AndB(..) => "and_b", Terminal::AndOr(..) => "andor", Terminal::OrB(..) => "or_b", Terminal::OrD(..) => "or_d", Terminal::OrC(..) => "or_c", - Terminal::OrI(ref l, _) if matches!(l.as_inner(), Terminal::False) => "l", Terminal::OrI(_, ref r) if matches!(r.as_inner(), Terminal::False) => "u", + Terminal::OrI(ref l, _) if matches!(l.as_inner(), Terminal::False) => "l", Terminal::OrI(..) => "or_i", Terminal::Thresh(..) => "thresh", Terminal::Multi(..) => "multi",