Skip to content

Document And/Or vectors contain 2 elements #602

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 1 commit into from
Sep 27, 2023

Conversation

tcharding
Copy link
Member

It is not immediately obvious that the Concrete::And and Or vectors contain exactly 2 elements, add code comment to document it.

It is not immediately obvious that the `Concrete::And` and `Or` vectors
contain exactly 2 elements, add code comment to document it.
@tcharding
Copy link
Member Author

After #594 I might investigate whether it helps to remove the vectors all together. In miniscript::decode::Terminal we don't use vecs for ands and ors.

@apoelstra
Copy link
Member

@tcharding that's because miniscirpt::decode::Terminal is part of Miniscript itself, which is never going to have n-ary ands or ors. The policy types are a completely different beast, and they're far less rigid in how we can change them.

@tcharding
Copy link
Member Author

Do you mean policies are a thing that is still being improved but miniscript itself is more 'complete'?

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 47c8776

@apoelstra
Copy link
Member

Miniscript is actual on-chain data. Policies are part of the "policy compiler" which is an entirely offchain tool used to produce efficient Miniscripts, and which has no guarantees about determinism or anything. As far as I know it isn't specced out or anything. So in that sense, yes, Miniscript is more "complete".

In another sense, that way more people care about Miniscirpt than care about this compiler thing, I expect more future work to go into Miniscript than the compiler. But that's just me :).

@apoelstra
Copy link
Member

We also care much less about memory efficiency for the compiler, for this reason, which is why we have Vecs in there even though there are no immediate plans to generalize the ands and ors beyond binary operators.

@tcharding
Copy link
Member Author

Awesome, thanks man.

@sanket1729 sanket1729 merged commit f8717e4 into rust-bitcoin:master Sep 27, 2023
@tcharding tcharding deleted the 09-27-vec-docs branch September 28, 2023 19:20
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