Use mathematical notation for semantic policy Display by febyeji · Pull Request #914 · rust-bitcoin/rust-miniscript

febyeji · 2026-04-01T04:40:38Z

Summary

Change semantic::Policy Display impl to use mathematical notation:
- and(a,b) → (a ∧ b)
- or(a,b) → (a ∨ b)
- thresh(k,a,b,c) → #{a, b, c} = k

Close #926

apoelstra · 2026-04-02T16:17:01Z

thresh(k,a,b,c) → #{a, b, c} ≥ k

The correct inequality is =, not ≥. If you provide too many satisfactions your witness will be invalid. But concept ACK using this brace/comma notation. I think it will be easier to read in a large expression than my notation was.

thresh requires exactly k satisfactions to be valid, not k or more. Providing too many satisfactions makes the witness invalid. Addresses apoelstra's review feedback on PR rust-bitcoin#914.

febyeji · 2026-04-02T17:31:38Z

Thanks for the review! Changed ≥ to = and added a doc comment to Display noting the notation and that it's not parseable.

apoelstra · 2026-04-02T17:48:27Z

Can you squash these commits, so that there isn't one modifying code introduced by the other?

tcharding · 2026-04-21T05:35:36Z

Do we want to close the issue if this merges?

tcharding

ACK 73ee818

trevarj · 2026-04-21T13:49:13Z

Do we want to close the issue if this merges?

@tcharding Don't close #885 until issues are made for these at least

We rename policy::Concrete to policy::Policy, leaving a type alias to minimize code breakage.

We move policy::Semantic into its own module so it becomes semantic::Semantic (we can also leave a type alias at the old location but we should deprecate it). We also move the Lift trait, which I think is in policy/mod.rs, to this own module (we can re-export it at the old location, but sadly we cannot deprecate re-exports).

We stop referring to policy::Semantic as a "semantic policy" or a "policy" at all anywhere in our docs

We change the Display impl for Semantic to output something goofy, like instead of and(a, b, c) it does (a ∧ b ∧ c). Whoever PRs to do this should provide several examples so we can debate this, especially the threshold notation. (Maybe we want (2 = a + b + c) for thresh(2, a, b, c)?)

Furthermore, I propose adding a method to "uncompile" from a Miniscript to a Policy (or even a Semantic to a Policy), but we should move that to a separate issue because I'm quite conflicted about what to do about percentages.

apoelstra · 2026-04-21T17:00:34Z

No, to close the issue we've also got to do the renames and doc changes to stop referring to semantic policies as "policies".

tcharding · 2026-04-21T22:46:18Z

@febyeji I've changed your PR description to close the new #926. Please check you are happy with that.

febyeji · 2026-04-22T09:53:56Z

@tcharding Looks good, thanks for updating the PR body. Happy with that.

apoelstra · 2026-04-22T20:30:06Z

In eb6011e:

This parser is gratuitously inefficient. You should be able to just parse the new syntax directly. It is a simple CFG and I don't think it even needs lookahead.

You definitely do not need to rewrite the string into the old syntax, and even if you did, this manages to be both recursive (we should not be introducing any new recursive code) and still manages to rescan the entire string on every iteration.

…ss parser Replace the recursive `rewrite_math_notation` that rescanned the input at every depth with a single linear pass that builds `Policy<Pk>` directly using an explicit frame stack. Matches `expression::Tree`'s non-recursive conventions. Addresses apoelstra's review on rust-bitcoin#914.

febyeji · 2026-04-24T05:29:25Z

I simplified the parser and added hybrid rejection. The math parser now refuses atoms whose root name is and/or/thresh, so strings like (a ∧ or(b, c)) and similar won't parse.

apoelstra · 2026-04-24T16:43:24Z

That solution is insane. I feel like I'm just arguing with a LLM by proxy here. I am not going to provide detailed feedback anymore.

febyeji · 2026-04-27T05:30:03Z

@apoelstra Apologies for the back-and-forth. I wrote the parser myself, but without carefully understanding the codebase. I was trying to be conservative and reuse expression::Tree, which compounded the mess since I had also misunderstood your intent and feedback.

I've rewritten the code. If the result isn't what you had in mind, please feel free to close the PR. I will just leave one note in case it helps resolving the issue later.

In the current code, FromStr accepts the legacy form as well to avoid breaking existing tests and users, but I'm not sure if FromStr should accept only the new format.

trevarj

Leaving some starting feedback on the parser. If the suggestions get implemented then I will do another review and see if we can refine it more.

You may want to not force push and start stacking some commits in case Andrew or someone else doesn't like a change and then you can drop that commit and rebase later.

trevarj

I think the parser is turning out pretty good, I like it.
Probably my last round of review since everything else seems ok to me.
Thanks!

trevarj

ACK 511e81b

Don't forget to squash commits later. Probably last 5 into a single commit?

trevarj

ACK 9aeffc7

febyeji force-pushed the semantic-policy-display branch from a0d28b3 to aee1d82 Compare April 1, 2026 05:50

febyeji force-pushed the semantic-policy-display branch from b796520 to 73ee818 Compare April 2, 2026 18:09

tcharding approved these changes Apr 21, 2026

View reviewed changes

apoelstra reviewed Apr 21, 2026

View reviewed changes

Comment thread fuzz/fuzz_targets/regression_descriptor_parse.rs Outdated

tcharding mentioned this pull request Apr 21, 2026

Fix Display impl for Semantic #926

Open

apoelstra reviewed Apr 22, 2026

View reviewed changes

Comment thread src/policy/semantic.rs Outdated

febyeji force-pushed the semantic-policy-display branch 2 times, most recently from 5a0282a to eb6011e Compare April 22, 2026 14:06

febyeji force-pushed the semantic-policy-display branch from eb6011e to 9852620 Compare April 23, 2026 11:12

trevarj reviewed Apr 23, 2026

View reviewed changes

Comment thread src/policy/semantic.rs Outdated

febyeji force-pushed the semantic-policy-display branch 2 times, most recently from 9201b03 to 357554e Compare April 24, 2026 05:04

febyeji marked this pull request as draft April 27, 2026 03:23

feat(policy): change semantic::Policy Display to mathematical notation

1384586

febyeji force-pushed the semantic-policy-display branch from 357554e to 1384586 Compare April 27, 2026 05:29

trevarj reviewed Apr 27, 2026

View reviewed changes

Comment thread src/policy/semantic.rs Outdated

Comment thread src/policy/semantic.rs Outdated

Comment thread src/policy/semantic.rs Outdated

Comment thread src/policy/semantic.rs Outdated

Comment thread src/policy/semantic.rs Outdated

trevarj suggested changes Apr 28, 2026

View reviewed changes

trevarj reviewed Apr 28, 2026

View reviewed changes

Comment thread src/policy/semantic.rs Outdated

trevarj approved these changes Apr 28, 2026

View reviewed changes

refactor(policy): rewrite semantic Policy parser for math notation

9aeffc7

febyeji force-pushed the semantic-policy-display branch from 511e81b to 9aeffc7 Compare April 29, 2026 02:55

febyeji marked this pull request as ready for review April 29, 2026 08:34

trevarj approved these changes Apr 30, 2026

View reviewed changes

Conversation

febyeji commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

apoelstra commented Apr 2, 2026

Uh oh!

febyeji commented Apr 2, 2026

Uh oh!

apoelstra commented Apr 2, 2026

Uh oh!

tcharding commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

trevarj commented Apr 21, 2026

Uh oh!

apoelstra commented Apr 21, 2026

Uh oh!

tcharding commented Apr 21, 2026

Uh oh!

febyeji commented Apr 22, 2026

Uh oh!

Uh oh!

apoelstra commented Apr 22, 2026

Uh oh!

Uh oh!

febyeji commented Apr 24, 2026

Uh oh!

apoelstra commented Apr 24, 2026

Uh oh!

febyeji commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

trevarj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

trevarj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

trevarj left a comment

Choose a reason for hiding this comment

Uh oh!

trevarj left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

febyeji commented Apr 1, 2026 •

edited

Loading

tcharding commented Apr 21, 2026 •

edited

Loading

febyeji commented Apr 27, 2026 •

edited

Loading