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

Add feature simplicity #68

Merged

Conversation

LeoComandini
Copy link
Contributor

rust-simplicity is still evolving and it might need fixes or interface changes. Make it optional.

@LeoComandini LeoComandini force-pushed the 2024-01-15-simplicity-feat branch from 9b00e2c to d9b3caf Compare January 15, 2024 15:35
@@ -37,6 +37,7 @@ pub enum TapTree<Pk: MiniscriptKey, Ext: Extension = NoExt> {
// are of Leafversion::default
Leaf(Arc<Miniscript<Pk, Tap, Ext>>),
/// A taproot leaf denoting a spending condition in terms of Simplicity
#[cfg(feature = "simplicity")]
SimplicityLeaf(Arc<simplicity::Policy<Pk>>),
Copy link
Member

Choose a reason for hiding this comment

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

In d5c0d65320816bfdf36411feed4bdff0708b5b12:

We can't make an enum variant dependent on a feature-gate like this because it will make adding the feature a breaking change for anyone trying to match on the enum.

Our choices are either to mark the enum #[non_exhaustive] (probably the easiest thing for now) or to create a dummy simplicity module when the simplicity feature is disabled, that will include a dummy simplicity::Policy so that this can still compile with the variant present but no Simplicity library.

Copy link
Member

Choose a reason for hiding this comment

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

In d9b3caf, sorry. Not sure what that other hash is.

Copy link
Contributor Author

@LeoComandini LeoComandini Jan 15, 2024

Choose a reason for hiding this comment

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

marked the enums #[non_exhaustive]

rust-simplicity is still evolving and it might need fixes or
interface changes. Make it optional.
@LeoComandini LeoComandini force-pushed the 2024-01-15-simplicity-feat branch from d9b3caf to b80cb46 Compare January 15, 2024 16:40
@apoelstra
Copy link
Member

Neat, TIL that you can match exhaustively on a #[non_exhaustive] enum, at least if you're in the same module that it's defined in.

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 b80cb46

@apoelstra apoelstra merged commit 2f58314 into ElementsProject:master Jan 16, 2024
18 checks passed
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