Skip to content

Ranged descriptors #154

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

Closed

Conversation

uncomputable
Copy link
Collaborator

@uncomputable uncomputable commented Jul 11, 2023

Adds rudimentary support for ranged descriptors. We could add more convenience methods, but this requires us to copy / implement a lot of traits such as ForEachKey.

@uncomputable uncomputable force-pushed the descriptors branch 2 times, most recently from 4792869 to afc4068 Compare July 12, 2023 15:04
@uncomputable
Copy link
Collaborator Author

uncomputable commented Jul 12, 2023

Loosened the trait bound in Descriptor<Pk> as FromStr. Now Pk can be pretty much any MiniscriptKey. We cannot deserialize Policies with xpubs. I need to fix this.

@uncomputable uncomputable marked this pull request as draft July 12, 2023 15:13
@uncomputable uncomputable marked this pull request as ready for review July 12, 2023 15:34
@uncomputable
Copy link
Collaborator Author

Fixed the issue. Now we can decode arbitrary Policys and Descriptors.

@uncomputable
Copy link
Collaborator Author

str::split_once() is available from 1.52.0 so I need to find a workaround.

This copies ideas from the Tr descriptor in rust-miniscript:
The spend info is inside an Arc inside a Mutex.
Ranged descriptors can be created but have to be converted (todo)
before they can be spent.
Removed the cmr() method because it is no longer used.
This might be useful when creating xpub descriptors
with unspendable internal key.
This enables us to derive children descriptors
from parent descriptors with ranged xpubs.

Copied from rust_miniscript. Added a unit test.
This enables us to iterate over all keys of a policy or descriptor.
Iterative implementation for Policy.
Descriptor uses this to implement the convenience method has_wildcard().
The ToPublic bound was simply unnecessary.
(Is there a tool to check this?)
Now we can decode policies with xpubs, which I also added a test for.
This also enables the decoding of descriptors with general keys.

The next commit will drop UnspendableKey to make FromStr even more
flexible.
@uncomputable uncomputable force-pushed the descriptors branch 2 times, most recently from ea3e89a to 6eed8a0 Compare July 13, 2023 13:25
@uncomputable
Copy link
Collaborator Author

Fixed and rebased on master.

@@ -276,29 +277,50 @@ impl Descriptor<DescriptorPublicKey> {

impl<Pk: MiniscriptKey> fmt::Display for Descriptor<Pk> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(&self.policy, f)
write!(f, "sim({},{})", self.internal_key, self.policy)
Copy link
Collaborator

Choose a reason for hiding this comment

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

In afae99a:

This commit seems to introduce a new sim descriptor. What is the value of having such a descriptor? What is the difference between a sim and a normal tr descriptor?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sim takes a Simplicity policy as second argument while tr takes Miniscript. We can overload tr if you want, but that might be confusing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm confused. The difference between Simplicity and Script happens at the tapleaf level. It wouldn't be "overloading" tr to use it for different kinds of taproot descriptors.

.and_then(|s| {
let mut split = s.split(',');
let x = split.next()?;
let y = split.next()?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

In afae99a:

This code will do the wrong thing if there is more than one ,. You should use splitn rather than split.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

We want to focus on the Simplicity leaves, but Simplicity descriptors
are first and foremost Taproot descriptors with an internal key.
We have the trait UnspendableKey to construct outputs with unspendable
internal key, but for serialization we don't want to be this limited.
In particular, it will be annoying to parse a descriptor with xprivs
if the unspendable internal key requires special treatment because its
private component is unknown.
There is no reason why PolicySatisfier is required for our satisfaction
methods. We could even consider removing it entirely / moving it to the
test cases.
This is a messy commit but the changes are quite limited:

1) Remove PolicySatisfier because we use generic Satisfier
2) Adapt unit tests and define HashSatisfier and SigSatisfier
   to that end
3) Check whether thresh satisfaction runs on the Bit Machine.
   This was uncovered because `env` was suddently unused.
This enables us to handle xpriv descriptors.
They don't exist in data, but we can parse to a keymap and an xpub
descriptor. This gives us a guarantee that the keymap corresponds to the
xpub, so it's safe to use during spending.

Copied from rust-bitcoin.

I don't like copy-pasting so many LOC but it seems inevitable.
There is a lot of boilerplate and repetions that could probably be
handled by LISP-style metaprogramming that is not available to us.
This enables us to work with pubkey descriptors instead of inaccessible
xpubs without wildcards.

Again copied from rust-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
Copy link
Collaborator Author

Same CI failure as in #158 . Cherry-picked the fix.

@uncomputable uncomputable marked this pull request as draft July 15, 2023 15:24
@uncomputable
Copy link
Collaborator Author

Superseded by #162. We will use ideas from this PR when we implement Simplicity descriptors over in elements-miniscript. But there is no reason to keep this PR open.

apoelstra added a commit to ElementsProject/elements-miniscript that referenced this pull request Aug 20, 2023
2ff1a17 Bump MSRV to 1.58.0 (Christian Lewe)
8bb85e3 examples: Adapt to Simplicity (Christian Lewe)
f85036a bitcoind-tests: Adapt to Simplicity (Christian Lewe)
029848c psbt: Add support for Simplicity (Christian Lewe)
0c62c05 tr: Weight calculations for Simplicity (Christian Lewe)
2a077f5 tr: Spend Simplicity descriptors (Christian Lewe)
3a8c495 tr: Implement Simplicity spending info (Christian Lewe)
4ffcc00 tr: FromStr for Simplicity policy inside Tr (Christian Lewe)
20073b1 tr: Todo sanity check for Simplicity descriptors (Christian Lewe)
edd15f3 tr: implement ForEachKey for Simplicity descriptors (Christian Lewe)
24b5296 simplicity: Parse policy (Christian Lewe)
5e06478 tr: Add Simplicity leaf to TapTree (Christian Lewe)
3e7440d fixme: Add rust-simplicity dependency (Christian Lewe)

Pull request description:

  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.

ACKs for top commit:
  apoelstra:
    ACK 2ff1a17

Tree-SHA512: cb31e8221ab976e0ab505354a1c3267d62a0c9c98c239c30ceed486f04a0fd81ce962861ef3c945438df68be5ef4d5b1e9205878de63dbeca3a5d11fbf2a1575
@uncomputable uncomputable deleted the descriptors branch October 2, 2023 14:31
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