bitcoindevkit / bdk-cli

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

Version conflict in dependencies #91

Closed rajarshimaitra closed 2 years ago

rajarshimaitra commented 2 years ago

Currently we have two different source of bdk in the library. Our own dependency itself and one that is used by bdk-reserves which is in the bdk-cli dependency tree.

So as we are updating bdk-cli to maintain with current version of bdk, bdk-reserve is stable at v0.17.0 so we are basically duplicating the entire bdk dependencies in our tree twice. And there has been a lot of changes in upstreams of bdk which are included in this open PR. https://github.com/bitcoindevkit/bdk/pull/593/files#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542

So this issue is to open discussion on how to handle handle this now and in future properly..

Is it possible to expose bdk through bdk-cli and then have bdk-reserve use it, so we ensure both are using the same version?? But then we might have circular dependencies as bdk-cli also depends on bdk-reserve..

cc @notmandatory @ulrichard

ulrichard commented 2 years ago

If I understand this correctly, it would mean that all other clients who use bdk-reserves would then also pull in bdk-cli?

Usually I try to upgrade bdk-reserves in a timely manner. It was just merged to 0.18: https://github.com/bitcoindevkit/bdk-cli/pull/89

Initially, I wanted to add the proof of reserves functionality to bdk itself, but I was told to do it in a separate lib, so that bdk itself can concentrate on the core. The plan was to treat it as a template for future functionality that could also live outside of the core bdk. So in this sense, the discussion is not about bdk-reserves specifically, but about such auxiliary libraries in general.

The plan was to integrate bdk-reserves into the bdk umbrella from the beginning. I am not sure what needs to be done in this regard. At the moment, https://github.com/weareseba/bdk-reserves is the leading repo, but that could just as well be https://github.com/bitcoindevkit/bdk-reserves. So you wouldn't necessarily have to wait for me to adapt to a new bdk and release it. Those upgrades are in general very easy. I invited notmandatory and afilini to be admins of https://crates.io/crates/bdk-reserves, and I can add more of the bdk people.

The first time I saw the error messages of the two bdk versions not matching, I was very confused, but in the meantime, I know where to look. Maybe we could also do something to improve these error messages?

Ah, but the main problem you face is probably not about the released version, but when you want to use a bdk version from a git branch, right? That's indeed a bit difficult to solve. My first idea was to import bdk in bdk-reserves with an alias. But since it produces traits on the main bdk objects, I don't think that would work.

rajarshimaitra commented 2 years ago

Hmm.. Yeah I understand.. Its a tricky situation..

I checked out the bdk-reserve repo, and I feel it can be included in bdk-cli project itself as a separate module.. The code isn't large and bdk-cli is the only cli app as far as I know to use that code to expose the reserve capabilities. So might make sense to add it in bdk-cli than in bdk??

That will also solve the purpose of duplicate bdk as they can then source it from single cargo.toml..

@notmandatory do you have a thought on this?? By seeing the code of reserve I feel it makes sense to just put them in a local crate here instead of download it from crates-io.. And its a fairly stable and simple protocol so I don't see major maintenance burden on it..

We can also have a new module in bdk-cli called extensions that can include such kind of extension made on top of bdk and use the bdk-cli interface to make them available for devs to play in command line.. In that module bip127-reserve can be an example implementation of how to do some onchain protocols using bdk..

ulrichard commented 2 years ago

bdk-reserves is also used in our proprietary cold storage solution, and I hope other projects will make use of it in the future. If it was included in a service component, I'm not sure it would be best to have it as part of a commandline project. Thus I think it is best to keep it as a separate library. One place where I think it could be useful would be for private key control for the FATF travel rule on multisig wallets.

I thought of creating an intermediary crate, such as bdk_with_extensions that would combine bdk with such additions. But I don't think that would solve the problem, but rather just move it to another place.

notmandatory commented 2 years ago

I think it's best to leave bdk-reserves as it's own crate for now with it's own dependency on bdk since it has other uses outside of bdk-cli. It is a downside that we have to upgrade the bdk versions in bdk-cli and bdk-reserves at the same time, but as long as @ulrichard is willing to help keep it in sync it shouldn't be a problem. Probably the best long-term solution is to make bdk-reserves an optional feature of the bdk repo but it's still a bit early for that.

rajarshimaitra commented 2 years ago

Sounds good to me.. That's probably the easiest approach.. Closing this on that note..