Skip to content

Simplicity descriptors #50

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

Merged
merged 13 commits into from
Aug 20, 2023
Merged

Conversation

uncomputable
Copy link
Contributor

@uncomputable uncomputable commented Aug 3, 2023

Takes ideas from BlockstreamResearch/rust-simplicity#154 and implements Simplicity descriptors as a special kind of script-spend inside a Taproot descriptor. Simplicity and Miniscript can be mixed in the same tree. TapLeafScript is an interface that unifies both worlds. All methods use this interface to work as normal.

@uncomputable uncomputable force-pushed the simplicity branch 2 times, most recently from b84258e to 197b195 Compare August 4, 2023 12:15
@apoelstra
Copy link
Member

Some high-level comments:

  • Why do the iterators panic? TapTreeIter should just be able to yield both kinds of leaves.
  • Why are we restricted to only one Simplicity leaf exactly? This isn't really explained in the PR.

@uncomputable uncomputable marked this pull request as ready for review August 15, 2023 20:34
@uncomputable
Copy link
Contributor Author

uncomputable commented Aug 15, 2023

I generalized Simplicity descriptors to multiple leaves which might be mixed with Miniscript.

What is left to do is test the code with the wallet and to implement the max weight calculations. I will also open a PR on rust-simplicity for the "policy fixes", which we have to merge first.

@uncomputable uncomputable changed the title Simplicity descriptors (MVP) Simplicity descriptors Aug 16, 2023
@uncomputable uncomputable force-pushed the simplicity branch 2 times, most recently from 75b8dda to a3b735f Compare August 16, 2023 12:34
Replace with master of BlockstreamResearch/rust-miniscript when ready.
@uncomputable
Copy link
Contributor Author

Rebased, bumped the MSRV to fix CI, and fixed the sub crates that silently broke.

BlockstreamResearch/rust-simplicity#170 adds the required changes on the Simplicity side. We need to merge that first, but the Miniscript side dictates what we need on the Simplicity side. So we need to look at both PRs.

Right now the control block is broken somehow. I cannot spend UTXOs with my wallet. I will investigate.

@apoelstra
Copy link
Member

I don't think we need to merge rust-simplicity 170 first -- we should merge that, update the vendored copy of simplicity, and cut a new release, but for now here we can just keep pointing to your branch in our Cargo.toml. We can fix that in a later PR.

@@ -136,12 +138,30 @@ impl<Pk: MiniscriptKey, Ext: Extension> TapTree<Pk, Ext> {
Q: MiniscriptKey,
Ext: Extension,
{
struct SimTranslator<'a, T>(&'a mut T);

impl<'a, Pk, T, Q, Error> simplicity::Translator<Pk, Q, Error> for SimTranslator<'a, T>
Copy link
Member

@apoelstra apoelstra Aug 17, 2023

Choose a reason for hiding this comment

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

In 899269a:

I wonder if, in rust-simplicity, we could just implement simplicity::Translator for all Translators. Or if that'd break stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

elements_miniscript::Translator and miniscript::Translator are distinct for some reason. If this doesn't clash with future plans, we could use miniscript::Translator across elements_miniscript, and implement simplicity::Translator for every miniscript::Translator.

I tried it out and the code compiled on my end.

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 194edcf

Extends the tap iterator to handle both kinds of leaves.
The iterator returns an enum that has to be split into either variant
as required. For now all methods assume Miniscript and will panic if
Simplicity is encountered. We will add Simplicity support to these
methods in the next commits.
We implement the FromTree trait for Policy to do the heavy lifting.
This was policy/embed.rs in rust-simplicity:
BlockstreamResearch/rust-simplicity@35d3fca

I would like to implement FromStr for Policy, but the orphan rule won't
let me. As a workaround I implement FromStr for a crate-local wrapper of
Policy. Users will never encounter this type.
Workaround because we cannot directly implement ForEachKey for
simplicity::Policy<Pk>.
The sanity of Simplicity leaves is not yet defined.
We allow Simplicity inside sim{...} in Taproot script-spends.
We use curly brackets because this is what is used by the existing
methods. Legal serializations are:

eltr(...,sim{pk(A)})          Simplicity without brackets
eltr(...,{sim{pk(A)}})        Simplicity with brackets
eltr(...,{sim{pk(A)},pk(B)})  Simplicity + Miniscript

Multiple Simplicity leaves (mixed with Miniscript) are possible.
@uncomputable
Copy link
Contributor Author

uncomputable commented Aug 18, 2023

Added the FIXME and squashed. I can also confirm that the descriptors work inside the Simplicity wallet if one uses the Simplicity version that is compatible with Elements. Let's merge this and update Cargo.toml later.

@apoelstra
Copy link
Member

72fde20 does not compile.

00862f0 is good otherwise.

Simplicity is converted into Bitcoin Script just like Miniscript.
Each policy fragment is converted into its CMR.
Spending policy fragments will be handled by a later commit.
TapLeafScript is extended to handle everything that is required for
spending in a generic way. The witness for Simplicity looks familiar:
"Witness elements", "witness script" and the control block.
What these are is slighly different:

There is one "Witness element" which is one large byte blob.
This deserializes to the Simplicity program plus its witness data.
Unlike Miniscript, there is always exactly one witness element.

"Witness script" is a Bitcoin Script that contains the CMR.

The control block is as usual. Even though there are these differences,
the code for computing the "witness size" should be general enough to
work for both Miniscript and Simplicity.
The length of the witness stack is always the same: 3 elements.
There is the serialized program+witness, the CMR and the control block.
The maximum witness size is trickier, because of the custom encoding
that Simplicity uses and because of the existence of disconnect (will be
present in asm fragments). There exists no function to compute a bound,
and the real witness size would be much smaller than what this function
returned. max_satisfaction_size will return an error for Simplicity
leaves. We can still choose the best leaf to spend because the spending
methods generate a witness from a satisfier, which we _can_ do.
Uses the public-key iterator and version number of a tap leaf script
to update a PSBT in a generic way.
These tests silently broke when we changed the tap tree iterator.
These also silently broke. Export TapLeafScript to make it available
outside the library.
We have to use rust-simplicity's MSRV, which is 1.58.0:
BlockstreamResearch/rust-simplicity@c3b86f5
@uncomputable
Copy link
Contributor Author

Now everything should compile. Sorry for that. I should have checked.

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 2ff1a17

@apoelstra apoelstra merged commit a832fda into ElementsProject:master Aug 20, 2023
@uncomputable uncomputable deleted the simplicity branch August 20, 2023 19:29
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.

2 participants