Skip to content
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.

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

Human-readable encoding 2: cleanups and syntax changes #158

Merged
merged 10 commits into from
Jul 18, 2023

Conversation

apoelstra
Copy link
Collaborator

After/during review of #152 we decided to change the syntax in a couple ways:

  • We introduced the ? syntax in types where A? is shorthand for 1 + A
  • We changed the CMR syntax from #expr to #{expr}
  • We added a new #<64 hex chars> format for directly specifying CMRs without giving an expression

This PR also includes some cleanups and new types and utility methods that will be used by the parser. Let me know if you want me to move any to the next PR, or if you'd like me to pull the parser into this one.

Fixes #157

Avoids a warning with --no-default-features.
So e.g. (1 * 1) -> ((1 + 1) * 1) becomes 1 * 1 -> (1 + 1) * 1
For now NamedConstructNode has no disconnect right child. A later PR
will introduce typed holes, and then we can add disconnect children,
but for now we'll just have single-child disconnects in the human
readable encoding.
This illustrates how `ErrorSet` will be used to accumulate errors that
can all be shown to the user at once.
@apoelstra
Copy link
Collaborator Author

Sigh, CI failure is a rustc bug rust-lang/rust#110475

I will need to patch it in elements-miniscript.

Newly-released rustc 1.71 no longer compiles our rust-miniscript
dependency. Patch rust-miniscript, then elements-miniscript, then this,
to hack around it.
@uncomputable uncomputable mentioned this pull request Jul 15, 2023
@@ -151,7 +159,7 @@ Expressions may be
* one of the core combinators `unit`, `iden`, `comp`, `injl`, `injr`, `case`, `take`, `drop`, `pair`, followed by subexpression(s) as needed;
* the `disconnect` combinator followed by an expression and a hole;
* the `witness` combinator which currently allows no subexpressions;
* the assertions, `assertl` or `assertr`, which take two subexpressions, one of which will be hidden in the decoded program. The hidden subexpression should be prefixed by `#` which indicates to the parser to take the CMR of that expression, not the expression itself.
* the assertions, `assertl` or `assertr`, which take a subexpressions and a CMR. The CMR is encoded as a full expression prefixed by `#{` and suffixed by `}`; but in the bit-encoding the expression does not appear, only its CMR;
Copy link
Collaborator

Choose a reason for hiding this comment

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

64c6ea4: Typo, and the definition of CMR was extended. I would leave out the latter part because it is already explained in an earlier section.

Suggested change
* the assertions, `assertl` or `assertr`, which take a subexpressions and a CMR. The CMR is encoded as a full expression prefixed by `#{` and suffixed by `}`; but in the bit-encoding the expression does not appear, only its CMR;
the assertions, `assertl` or `assertr`, which take a subexpression and a CMR;

Copy link
Collaborator

@uncomputable uncomputable left a comment

Choose a reason for hiding this comment

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

ACK b608ec1 Feel free to address the nit.

The named construct node and the error sets look interesting. They are not being used at the moment, but I'm sure the next PR is already waiting.

@apoelstra apoelstra merged commit 0fb0b8f into master Jul 18, 2023
@apoelstra apoelstra deleted the 2023-07--human-2 branch July 18, 2023 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support option types
2 participants