python library introduced new dependencies in 0.2.2

[SomberNight]

The ledger-bitcoin python library introduced new dependencies in version 0.2.2, in #166.
Compare:

$ pipdeptree
ledger-bitcoin==0.2.1
├── ledgercomm [required: >=1.1.0, installed: 1.2.0]
├── packaging [required: >=21.3, installed: 23.1]
└── typing-extensions [required: >=3.7, installed: 4.7.1]

$ pipdeptree
ledger-bitcoin==0.2.2
├── bip32 [required: ~=3.0, installed: 3.4]
│   ├── base58 [required: ~=2.0, installed: 2.1.1]
│   └── coincurve [required: >=15.0,<19, installed: 18.0.0]
│       ├── asn1crypto [required: Any, installed: 1.5.1]
│       └── cffi [required: >=1.3.0, installed: 1.15.1]
│           └── pycparser [required: Any, installed: 2.21]
├── coincurve [required: ~=18.0, installed: 18.0.0]
│   ├── asn1crypto [required: Any, installed: 1.5.1]
│   └── cffi [required: >=1.3.0, installed: 1.15.1]
│       └── pycparser [required: Any, installed: 2.21]
├── ledgercomm [required: >=1.1.0, installed: 1.2.0]
├── packaging [required: >=21.3, installed: 23.1]
└── typing-extensions [required: >=3.7, installed: 4.7.1]

coincurve in particular is a large dependency and is not pure python. It is non-trivial to build it.

Looking at the PR and the linked vulnerability, I guess I can't persuade you to remove these dependencies...

Could you perhaps make them optional?
It seems like you could easily import bip380 only when needed, only for non-trivial miniscripts, e.g. here:

app-bitcoin-new/bitcoin_client/ledger_bitcoin/client.py

Line 279 in b4905a4

desc = Descriptor.from_str(wallet.get_descriptor(change))

That would at least mean that if a library user does not support generic miniscripts, they don't need the new dependencies.

---

We are using ledger-bitcoin in Electrum, and would rather avoid the new dependencies, if at all possible. Note that our usage atm is limited to trivial miniscripts, for which bip380 is not even used (but it is imported nevertheless in 0.2.2). Further note that even when we add more complex miniscript support in Electrum in the future, almost surely we will have logic there outside ledger-bitcoin doing equivalent checks. Point being, it would be good to let the library user disable the new checks and not require the new dependencies.

[bigspider]

Good point about the dependency size, definitely something to fix. For now, I guess feel free to freeze the version to 0.2.1, as there shouldn't be any substantial changes affecting you.

The root of the dependency import is python-bip32,so that would be the best place to solve it. Perhaps one could consider replacing coincurve there with some more lightweight library, or a standalone implementation, as performance is probably not a big consideration here?

cc @darosior, maybe you know if there is a good replacement?

[darosior]

Getting rid of coincurve has been a long standing goal of python-bip32. However it was only to replace it with another libsecp256k1 wrapper.

I would welcome a PR making our secp256k1 interface configurable. For instance, it could take the form of an abstract interface to the libsecp functions we need. The default implementation would use the libsecp256k1 binding (either through coincurve or python-secp256k1), and callers could specify an interface that use something else. For instance Electrum's Python implementation. A pure Python implementation of this interface could also be upstreamed to python-bip32.

How does that sound?

[SomberNight]

I would welcome a PR making our secp256k1 interface configurable. For instance, it could take the form of an abstract interface to the libsecp functions we need.

Sounds good.

For now, I guess feel free to freeze the version to 0.2.1,

Yes, will do that for now.

Still, I think the import of the vendored bip380 lib (which I am fine with besides that pulling in the new deps) could be done on demand, and the new deps could be moved to a requirements extra. Note that if we had the mentioned libsecp interface abstraction, you would probably still need extras for the default implementation (as the whole point is to avoid unconditionally pulling in coincurve/etc).

[bigspider]

@SomberNight the 0.3.0 version of the python client is available on pypi and removed the heavy dependencies.

[SomberNight]

Great! Thanks a lot.