Invert miniscript dependency #162
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
uncomputable
force-pushed
the
invert-miniscript-dep
branch
from
6fe644f to
237eab6
Compare
Collaborator
|
I think it's ok that we can't use |
Remove Simplicity descriptors to be reimplemented in elements-miniscript. We introduce an new layer for Simplicity keys and satisfiers. Miniscript will import this layer to implement Simplicity descriptors. This approach enables us to keep the messy Simplicity details in rust-simplicity while making use of the descriptor infrastructure in elements-miniscript.
Simplicity key replaces MiniscriptKey. ToXOnlyPubkey replaces ToPublicKey. (Simplicity)Translator replaces (Miniscript)Translator. These traits are exported at the crate root, like in Miniscript. We still need the elements feature guard because of the satisfier. Policy will also switch to height-based timelocks etc. at some point which requires Elements types. The FromStr (embed) and Satisfier (satisfy) implementations are commented out to prevent this commit from blowing up. This also means we have to temporarily disable the related fuzz test.
I would like to keep an implementation of FromStr, but I don't want to copy all of the expression parsing from Miniscript. We don't need FromStr right now, and Miniscript can implement a parser using <Tree as FromStr> and FromTree<Policy>. We might want to implement a parser on the Simplicity side, which might use ideas from the dag module or simplang. Removing FromStr also means removing the associated fuzz test.
The Satisfier from elements-miniscript, but just the methods that we actually need, using the Simplicity key traits. Implemented absolute and relative locktime checks because they are confusing. Made use of the new relative locktime struct from rust-bitcoin, which we might want to repeat upstream (there is a todo in the code). The rest of the satisfier is bare-bones, as miniscript will implement (Simplicity)Satisfier for (Miniscript)Satisfier, which will bring most of the convenience. We can extend this later if we want. For now there are as few changes to rust-simplicity as possible.
Until now we asserted that the satisfaction of thresh looks a certain way, but not that the satisfied program executes successfully.
This is what we strived towards: Removing elements-miniscript from rust-simplicity so that the former can depend on the latter. We invert dependencies so Simplicity descriptors can be handled in elements-miniscript while all the messy Simplicity details are handled in rust-simplicity.
uncomputable
force-pushed
the
invert-miniscript-dep
branch
from
237eab6 to
d7e66a0
Compare
apoelstra
approved these changes
Jul 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We remove elements-miniscript from rust-simplicity so that the former can depend on the latter. It will be much easier to implement Simplicity descriptors in elements-miniscript where there exists most of the required infrastructure. Doing this in rust-simplicity would lead to a lot of copy and pasting.
We decided to create a new abstraction layer for Simplicity policy (keys, satisfiers, ...) to keep all the messy details about constructing Simplicity programs in rust-simplicity. elements-miniscript can simply implement these traits for the existing analogues (MiniscriptKey, Satisfier, ...) to make use of them.
One drawback of this PR is that Policy no longer implements FromStr. We would have to copy a lot of code from elements-miniscript to adapt the approach from master, and we don't use FromStr outside of tests. elements-miniscript can use its Tree trait to parse Policy (see commit message).