Skip to content

Use mathematical notation for semantic policy Display #914

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.

Sign up for GitHub

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

Open
febyeji wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use mathematical notation for semantic policy Display #914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 examples/htlc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn main() {
// Lift the descriptor into an abstract policy.
assert_eq!(
format!("{}", htlc_descriptor.lift().unwrap()),
"or(and(pk(022222222222222222222222222222222222222222222222222222222222222222),sha256(1111111111111111111111111111111111111111111111111111111111111111)),and(pk(020202020202020202020202020202020202020202020202020202020202020202),older(4444)))"
"((pk(022222222222222222222222222222222222222222222222222222222222222222) ∧ sha256(1111111111111111111111111111111111111111111111111111111111111111)) ∨ (pk(020202020202020202020202020202020202020202020202020202020202020202) ∧ older(4444)))"
);

// Get the scriptPubkey for this Wsh descriptor.
Expand Down
34 changes: 16 additions & 18 deletions fuzz/fuzz_targets/regression_descriptor_parse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,31 +14,29 @@ fn do_test(data: &[u8]) {
(Ok(x), Err(e)) => panic!("new logic parses {} as {:?}, old fails with {}", data_str, x, e),
(Err(e), Ok(x)) => panic!("old logic parses {} as {:?}, new fails with {}", data_str, x, e),
(Ok(new), Ok(old)) => {
use miniscript::policy::Liftable as _;
use old_miniscript::policy::Liftable as _;

assert_eq!(
old.to_string(),
new.to_string(),
"input {} (left is old, right is new)",
data_str
);

match (new.lift(), old.lift()) {
(Err(_), Err(_)) => {}
(Ok(x), Err(e)) => {
panic!("new logic lifts {} as {:?}, old fails with {}", data_str, x, e)
}
(Err(e), Ok(x)) => {
panic!("old logic lifts {} as {:?}, new fails with {}", data_str, x, e)
}
(Ok(new), Ok(old)) => {
assert_eq!(
old.to_string(),
new.to_string(),
"lifted input {} (left is old, right is new)",
data_str
)
// Lifted semantic policy Display format has intentionally changed
// to mathematical notation. Cross-crate structural comparison is not
// possible, so we only verify both crates agree on liftability.
Copy link
Copy Markdown
Member

@apoelstra apoelstra Apr 21, 2026

Choose a reason for hiding this comment

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

In 73ee818:

Ouch. This is actually a critical check and removing it completely guts our regression testing.

I think we need to preserve the old serialization logic somewhere. Maybe add a new to_policy_like_string() or something and document that it's only there for legacy usage.

{
use miniscript::policy::Liftable as _;
use old_miniscript::policy::Liftable as _;

match (new.lift(), old.lift()) {
(Err(_), Err(_)) => {}
(Ok(x), Err(e)) => {
panic!("new logic lifts {} as {:?}, old fails with {}", data_str, x, e)
}
(Err(e), Ok(x)) => {
panic!("old logic lifts {} as {:?}, new fails with {}", data_str, x, e)
}
(Ok(_), Ok(_)) => {}
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/descriptor/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2175,15 +2175,15 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
// Taproot structure is erased but key order preserved..
let desc = Descriptor::<String>::from_str("tr(ROOT,{pk(A1),{pk(B1),pk(B2)}})").unwrap();
let lift = desc.lift().unwrap();
assert_eq!(lift.to_string(), "or(pk(ROOT),or(pk(A1),pk(B1),pk(B2)))",);
assert_eq!(lift.to_string(), "(pk(ROOT) ∨ (pk(A1) ∨ pk(B1) ∨ pk(B2)))");
let desc = Descriptor::<String>::from_str("tr(ROOT,{{pk(A1),pk(B1)},pk(B2)})").unwrap();
let lift = desc.lift().unwrap();
assert_eq!(lift.to_string(), "or(pk(ROOT),or(pk(A1),pk(B1),pk(B2)))",);
assert_eq!(lift.to_string(), "(pk(ROOT) ∨ (pk(A1) ∨ pk(B1) ∨ pk(B2)))");

// And normalization happens
let desc = Descriptor::<String>::from_str("tr(ROOT,{0,{0,0}})").unwrap();
let lift = desc.lift().unwrap();
assert_eq!(lift.to_string(), "or(pk(ROOT),UNSATISFIABLE)",);
assert_eq!(lift.to_string(), "(pk(ROOT) ∨ UNSATISFIABLE)");
}

#[test]
Expand Down
22 changes: 12 additions & 10 deletions src/policy/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,12 +251,6 @@ mod tests {
assert_eq!(s.to_lowercase(), output.to_lowercase());
}

fn semantic_policy_rtt(s: &str) {
let sem = SemanticPol::from_str(s).unwrap();
let output = sem.normalized().to_string();
assert_eq!(s.to_lowercase(), output.to_lowercase());
}

#[test]
fn test_timelock_validity() {
// only height
Expand All @@ -279,17 +273,25 @@ mod tests {
concrete_policy_rtt("or(99@pk(X),1@pk(Y))");
concrete_policy_rtt("and(pk(X),or(99@pk(Y),1@older(12960)))");

semantic_policy_rtt("pk()");
semantic_policy_rtt("or(pk(X),pk(Y))");
semantic_policy_rtt("and(pk(X),pk(Y))");

//fuzzer crashes
assert!(ConcretePol::from_str("thresh()").is_err());
assert!(SemanticPol::from_str("thresh(0)").is_err());
assert!(SemanticPol::from_str("thresh()").is_err());
concrete_policy_rtt("ripemd160()");
}

#[test]
fn semantic_display_uses_mathematical_notation() {
let pol = SemanticPol::from_str("and(pk(A),pk(B))").unwrap();
assert_eq!(pol.normalized().to_string(), "(pk(A) ∧ pk(B))");

let pol = SemanticPol::from_str("or(pk(A),pk(B))").unwrap();
assert_eq!(pol.normalized().to_string(), "(pk(A) ∨ pk(B))");

let pol = SemanticPol::from_str("thresh(2,pk(A),pk(B),pk(C))").unwrap();
assert_eq!(pol.normalized().to_string(), "#{pk(A), pk(B), pk(C)} = 2");
}

#[test]
fn compile_invalid() {
// Since the root Error does not support Eq type, we have to
Expand Down
29 changes: 24 additions & 5 deletions src/policy/semantic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,13 @@ impl<Pk: MiniscriptKey> fmt::Debug for Policy<Pk> {
}
}

/// Displays the policy using mathematical notation for readability.
///
/// - `and(a, b)` is displayed as `(a ∧ b)`
/// - `or(a, b)` is displayed as `(a ∨ b)`
/// - `thresh(k, a, b, c)` is displayed as `#{a, b, c} = k`
///
/// Note: this format is not parseable.
impl<Pk: MiniscriptKey> fmt::Display for Policy<Pk> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
Expand All @@ -261,12 +268,26 @@ impl<Pk: MiniscriptKey> fmt::Display for Policy<Pk> {
Policy::Ripemd160(ref h) => write!(f, "ripemd160({})", h),
Policy::Hash160(ref h) => write!(f, "hash160({})", h),
Policy::Thresh(ref thresh) => {
let mut iter = thresh.iter();
let first = iter.next().expect("thresholds are never empty");
if thresh.k() == thresh.n() {
thresh.display("and", false).fmt(f)
write!(f, "({}", first)?;
for sub in iter {
write!(f, " ∧ {}", sub)?;
}
f.write_str(")")
} else if thresh.k() == 1 {
thresh.display("or", false).fmt(f)
write!(f, "({}", first)?;
for sub in iter {
write!(f, " ∨ {}", sub)?;
}
f.write_str(")")
} else {
thresh.display("thresh", true).fmt(f)
write!(f, "#{{{}", first)?;
for sub in iter {
write!(f, ", {}", sub)?;
}
write!(f, "}} = {}", thresh.k())
}
}
}
Expand All @@ -281,8 +302,6 @@ impl<Pk: FromStrKey> str::FromStr for Policy<Pk> {
}
}

serde_string_impl_pk!(Policy, "a miniscript semantic policy");

impl<Pk: FromStrKey> expression::FromTree for Policy<Pk> {
fn from_tree(root: expression::TreeIterItem) -> Result<Policy<Pk>, Error> {
root.verify_no_curly_braces()
Expand Down
Loading