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

Allow direct compilation of a Policy into a valid Descriptor #352

Merged

Conversation

SarcasticNastik
Copy link
Contributor

@SarcasticNastik SarcasticNastik commented Apr 19, 2022

This PR works on top of #291 and #342. It introduces a new function compile_to_descriptor which requires a DescriptorCtx (suggestions for a better enum name are welcome!) allowing directly compilation of a policy into the specified descriptor type.

@SarcasticNastik SarcasticNastik changed the title Allow direct compilation of a policy into a valid Descriptor Allow direct compilation of a Policy into a valid Descriptor Apr 19, 2022
@SarcasticNastik SarcasticNastik marked this pull request as ready for review April 19, 2022 23:31
@SarcasticNastik SarcasticNastik force-pushed the refactor/compile-to-descriptor branch from 1a67189 to 63ee549 Compare May 4, 2022 14:07
Wsh,
/// ShWsh
ShWsh,
/// Tr
Copy link
Member

Choose a reason for hiding this comment

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

This should mention that Option is internal key

@SarcasticNastik SarcasticNastik mentioned this pull request May 9, 2022
@SarcasticNastik SarcasticNastik force-pushed the refactor/compile-to-descriptor branch from 63ee549 to 3fa3436 Compare May 9, 2022 22:29
@sanket1729
Copy link
Member

I will review this after we get the first PR in.

@SarcasticNastik SarcasticNastik force-pushed the refactor/compile-to-descriptor branch 3 times, most recently from d8a7caf to faee661 Compare May 12, 2022 18:10
sanket1729 added a commit that referenced this pull request May 19, 2022
2b13694 Add non-trivial multi-node example (Aman Rojjha)
1c2a80e Add validity and malleability checks. (Aman Rojjha)
0866807 Add P2Tr compiler (Aman Rojjha)
285207e Internal-key extraction done (Aman Rojjha)
26fc574 Policy to single-leaf TapTree compilation done (Aman Rojjha)

Pull request description:

  This PR builds on top of #278.

  This is one in a series of PRs aimed to implement a feature-complete *Pay-to-Taproot* **P2Tr** compiler.
  Specifically, this introduces a basic compilation for a given policy by collecting the leaf nodes of the *tree* generated by considering root-level disjunctive `OR`and using this to generate a `TapTree`.

  > Assuming that _duplicate keys_ are **NOT** possible even in different branches of the TapTree.

  # Follow Up PRs

  - #342  - Uses heuristic for tree-generation/ _merging_ algorithm while buillding `TapTree` to optimize over the *expected average total cost*.
  - #352
  - A future PR implementing enumerative strategies for optimizing `thresh` `k`-of-`n` and similar structures.

ACKs for top commit:
  apoelstra:
    ACK 2b13694
  sanket1729:
    ACK 2b13694 . Let's push forward on this. In the interest of ACKs and multiple rebases already done here, I am merging this. We recently shifted to rust 2018. And this code has a bunch of warnings because of it.

Tree-SHA512: 4ceca51a383f5d52b572a16937bbcc3a9c53f0247e4b6df57a6547fd0b1c7cc33ff04dd9a476914bcf6d0a09e255918b8f7ebfe176c839d6ae31c84613dce67f
@sanket1729
Copy link
Member

This can be rebased again after v2 one. Let's try to merge this before thresh enumeration

@SarcasticNastik SarcasticNastik force-pushed the refactor/compile-to-descriptor branch 2 times, most recently from f8bdd1e to 02d08f2 Compare June 17, 2022 21:08
@sanket1729
Copy link
Member

Merged v2 one, this can be rebased on top of master now

apoelstra
apoelstra previously approved these changes Jun 18, 2022
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 f464e33

Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

Left two small documentation comments

src/policy/concrete.rs Outdated Show resolved Hide resolved
src/policy/concrete.rs Show resolved Hide resolved
@SarcasticNastik
Copy link
Contributor Author

@sanket1729 Done!

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 169d849

Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

ACK 169d849

@sanket1729 sanket1729 merged commit c6835f7 into rust-bitcoin:master Jun 20, 2022
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.

3 participants