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

Make all features additive, none mutually exclusive #75

Closed
notmandatory opened this issue Mar 5, 2022 · 5 comments
Closed

Make all features additive, none mutually exclusive #75

notmandatory opened this issue Mar 5, 2022 · 5 comments

Comments

@notmandatory
Copy link
Member

notmandatory commented Mar 5, 2022

All features should be additive and none should be mutually exclusive. See Cargo book section on feature unification.

Current blockchains features (electrum, esplora, compact_filters, rpc) are mutually exclusive. The need to be made to be additive with a runtime flag to select which one the wallet should use.

@rajarshimaitra
Copy link
Contributor

One problem of this I feel gonna come is, it will further increase the args list?

@notmandatory
Copy link
Member Author

notmandatory commented Mar 5, 2022

I think we should be able to work out a reasonable default behavior if multiple blockchain features are enabled such as if you enable everything, but don't specify which one you want to use it uses electrum. Or if you only enable esplora-ureq it uses that, and if you enable rpc and compact_filters, it defautls to rpc. It will be nice for testing to be able to enable all features without recompiling. The CI testing could be easier too if we can have one job that runs all the tests for all the features.

I don't think we need to allow users to enable and use multiple blockchain features and have all their cli options available. One way to go is add new wallet option: --blockchain <electrum | esplora-ureq | esplora-reqwest | rpc | compact_filters> And the corresponding opts are only required if the right blockchain is selected.

@notmandatory
Copy link
Member Author

If it does get too complicated we can also leave things as is, but I think it's worth a try.

@rajarshimaitra
Copy link
Contributor

@notmandatory Do we still want this one?? Having multiple databases together feels like can complicate things a lot..

On further thought on this, having non mutually exclusive features might make sense in general, but given the kind of things we are unleashing here with feature flags (like blockchains and dbs) trying to simulate some kind of over ridding feature gates (like use esplora, if both electrum and eslora is there) can make things complex. And we already have many feature gating going on all through out the code..

@notmandatory
Copy link
Member Author

I'll close this one for now, no need to add extra complications right now. But if someone feels strongly about giving it a try we can always reopen it. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants