search

bitcoindevkit / bdk-cli

A CLI wallet library and REPL tool to demo and test the BDK library
Other
108 stars 64 forks source link

Make all features additive, none mutually exclusive #75

Closed notmandatory closed 2 years ago

notmandatory commented 2 years ago

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 commented 2 years ago

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

notmandatory commented 2 years ago

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 commented 2 years ago

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

rajarshimaitra commented 2 years ago

@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 commented 2 years ago

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. :-)