search

bitcoindevkit / rust-esplora-client

Bitcoin Esplora API client library. Supports plaintext, TLS and Onion servers. Blocking or async.
MIT License
29 stars 44 forks source link

Support MSRV 1.48.0 with "blocking" feature #37

Closed notmandatory closed 7 months ago

notmandatory commented 1 year ago

The rust-bitcoin and rust-lightning projects are targeting their MSRV at 1.48.0. Switching the rust-esplora-client from ureq to minreq 2.x it should make it possible to support the same MSRV for at least the blocking feature (without TLS).

CI tests also need to be added to ensure 1.48.0 MSRV for blocking, and lowest possible MSRV for async, and async-https features.

See: https://github.com/neonmoe/minreq

LLFourn commented 1 year ago

Is there any real benefit of supporting 1.48.0 without TLS though? Most of the time you want to verify the TLS cert when using esplora because you're probably using a public server.

notmandatory commented 1 year ago

For most apps you're probably right that people will need TLS. My initial thinking was that it wouldn't be too hard to do and would give bdk 1.0 users who are stuck on a low MSRV an alternative blockchain client to rust-bitcoincore-rpc.

I can also see this used by enterprise users (the ones more likely on an old MSRV) who want to connect multiple internal app wallets on their private network to a few trusted, shared esplora servers + full nodes. Esplora should scale better than setting up a full node+rpc for each app. There could also be very privacy minded apps that only use TOR proxies + hidden services or similar and won't need TLS for privacy or certs to verify the identity of the server.

That said, if the change is going to make this crate a lot harder to maintain, I haven't heard of anyone who specifically requested it. Could leave it open to collect feedback and close later if no one else sees a need for it.

vladimirfomene commented 1 year ago

Personally, I'm more for us leaving this one open and waiting to see if we get request for it.

tnull commented 1 year ago

I'd like to request it, as minreq would at least give the chance of building the blocking crate on a lower MSRV. ureq unfortunately depends on once_cell which as no versioning guarantees at all I believe. I might look into this eventually. IIUC the only blocker might be that minreq currently only has HTTP, not SOCKS proxy support, but since the feature has been broken up until recently (#34), we may be fine with starting the migration out without SOCKS?

tnull commented 1 year ago

As discussed in the call, we should probably start out with adding minreq as a third option hidden behind a feature flag, an d remove ureq only after we've seen there are no issues and there is a solution to the missing SOCKS support.

oleonardolima commented 7 months ago

@notmandatory @tnull I think we could close this one, right ? As we already have minreq support now https://github.com/bitcoindevkit/rust-esplora-client/pull/75, and both rust-bitcoin and rust-lightning are way past the 1.48.0 msrv

tnull commented 7 months ago

Jup, probably safe to close this by now.