Skip to content

Conversation

@uncomputable
Copy link
Collaborator

@uncomputable uncomputable commented Apr 19, 2022

Remove dependency of (implicit) feature elements upon bitcoin by using elements::bitcoin. Fix GitHub CI to test for successful compilation using each of these features separately.

Proposed in #21.

@uncomputable uncomputable force-pushed the fix-features branch 3 times, most recently from d59977b to 8bc817f Compare April 19, 2022 19:16
@uncomputable uncomputable mentioned this pull request Apr 20, 2022
1 task
@sanket1729
Copy link
Member

I think there are two different ways of looking at features here:

there is no way how dependency / implicit feature elements can enable dependency / implicit dependency bitcoin.

  1. Features as dependency tree: I think when you say there is no way to have elements without bitcoin, you mean making sure that the rust-bitcoin crate is not being used as a dependency.

  2. Features are simplicity extensions: In this way of separation, when we are using simplicity with elements extension, we can use both rust-elements and rust-bitcoin. But the jets are available only for elements intropsection.

I think we should classify our features by the latter method. If possible in rust, we should also mark features as incompatible with each other. That is, you can run in both elements and the bitcoin feature.

@apoelstra
Copy link
Collaborator

@sanket1729 unfortunately we can't make features incompatibile with each other, so we have made sure that it is possible to have both features enabled.

The problem that this PR addresses is that right now you can't have elements without bitcoin

@apoelstra
Copy link
Collaborator

@chlewe I would like it to be possible to use elements without bitcoin. This is what we want for use with elements-miniscript, for example.

Making the dependency explicit fixes the compilation issue but I'd like to remove the dependency. Heads up that you can access the rust-bitcoin crate through rust-elements, as elements::bitcoin.

@uncomputable
Copy link
Collaborator Author

As far as I can see, the namespaces of features and dependencies get properly separated in Rust 1.60. In earlier versions, implicit features cannot enable other features / dependencies because implicit features are defined in [dependencies] and not in [features]. Incompatible features should be avoided.

@apoelstra
Copy link
Collaborator

@chlewe the two features are not incompatible, are they?

This fix looks great, exactly what I was looking for. If there are actual incompatibilties we should find and fix those but they can wait..

Copy link
Collaborator

@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 0385213

@uncomputable
Copy link
Collaborator Author

Right now, all features are compatible and no feature depends on another.

@apoelstra apoelstra merged commit a36b4ff into BlockstreamResearch:master Apr 22, 2022
@uncomputable uncomputable deleted the fix-features branch April 24, 2022 17:19
@apoelstra apoelstra mentioned this pull request May 19, 2022
4 tasks
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