bdk
bdk copied to clipboard
Support for MSRV shipped with Debian stable for default features
For improved build tool security we should support as MSRV the rustc version shipped with Debian stable, currently 1.41.1. This may not be ppossible for all features, but should be possible for our wallet with default features, key-value-db and electrum.
Discord discussion here: https://discord.com/channels/753336465005608961/753336465005608964/832682901950431323
If other features can also be made to work with a lower MSRV we should create separate issues for those. Another approach for libraries with higher MSRV may be to create a guide for verifying dependencies and/or creating reproducible builds.
The current testing debian release is 11 (bullseye) and it will ship with rustc 1.48.0. I think it's worth considering that bullseye may be the stable debian by the time any mainnet ready projects adopt BDK.
https://tracker.debian.org/pkg/rustc
Something else to note is there appear to be CVEs that affect rustc versions prior to 1.52.0. It looks like the debian rustc maintainer is trying to get the fixes back-ported to 1.48. :crossed_fingers:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=986803****
CVE-2021-28875 CVE-2021-28876 CVE-2021-28877 CVE-2021-28878 CVE-2021-28879 CVE-2020-36317 CVE-2020-36318
As I mentioned on the call I think setting a conservative MSRV is a good long term goal but I wouldn't fix it now. It takes time and effort to maintain things pinned to old versions of rustc. I think so far the version bumps in bdk were made to save developer time and keep things simple. I think this is the right thing to do since the library is more likely to survive and thrive based on functionality and getting the APIs right rather than compiler toolchain reproducibility concerns or default Debian package versions. Once the functionality and the users are there then the balance tips the other way of course but I don't think this library is at that stage yet.
I am biased btw since I use bdk to explore ideas mostly rather than to "put things in production".
Yeah I think lowering the MSRV for the entire project is kind of unrealistic right now. I also don't think it would really make any difference in practice because it's very likely that larger projects using BDK would have other dependencies that require a higher MSRV as well.
I think it makes sense to try doing that for the core wallet stuff though, because that part by itself only requires very few deps and it's also the most critical in terms of security. So if someone is building a wallet and really cares about security he would still be able to use the core of BDK and then maybe implement custom Blockchain or Database however he likes to replace our implementation which requires a lot of dependencies and higher MSRV.
Based on discussion above, how does this sound as a policy:
-
The
bdklib with default features ("key-value-db", "electrum") will have MSRV equal to version of rustc shipped with Debian stable (currently 1.48.0). Stable Debian releases come out approximately every 2 years. -
Optional features will be have a MSRV that is a minimum of 4 minor rustc release versions old (currently 1.52.0). Minor releases are every 6 weeks, so users of optional features will have at least 6 months before they may have to upgrade.
To make this policy work we'll have to do a few things:
- [ ] Update README.md + website docs to inform users of policy
- [ ] Update CI to test default features and optional features with different MSRVs
Anything else?
Note that rust development team recommend at least 1.52.1 due to incremental compilation bugs in previous versions.
[edit] whoops I see this was mentioned earlier: https://github.com/bitcoindevkit/bdk/issues/331#issuecomment-821768965
Good point, how does this sound for a third rule:
- The MSRV for default or non-default features will be incremented the minimum over the above policies if needed to fix a high priority Rust CVE, as determined by rough consensus of the
bdkteam.
My 2c on your plan:
- I think it's preferable to say that YMMV with MSRV on non-default features and bump dependencies/testing versions whenever needed. Having a rigid policy will just lead to more of our time being wasted with fixing different versions of things again and again.
- If you want to have a MSRV policy per feature it makes more sense to extract those features into their own crates e.g.
bdk-esploraand manage them separately. I don't think we're ready for that though because I expect APIs to be changing a bit still.
If we wanted to have a strict MSRV policy that was low hassle we could fix the MSRV to 1.56 and refuse to change it. This will allow us to recognise the rust-version attribute in dependencies. We can then make sure all our dependencies set this in their cargo.toml and then complain loudly if they violate this promise or change their rust-version in a patch release. Our main problem atm is that new minor versions and patch releases are changing the effective MSRV of their crate without even the authors of the crate being aware. This annoying problem may go away after 1.56 as rust-version gives an explicit way of signalling changes to MSRV and hopefully raises awareness of this problem.
My preference now is to not make any MSRV promises about non-default features for now and if we have a particular feature that is stable and we want to make sure it has a fixed MSRV different than the main one then we extract it into its own crate.
I agree it doesn't make sense to extract features right now. And makes sense to not pin down the MSRV on non-default features yet. I'd like to try keeping the default features supported back to rust 1.48.0 but if it gets too complicated to try to support default and non-default features at different versions then we can try extracting features or moving MSRV up to 1.56.
At this point I think the best option is set MSRV to 1.56.X, it's not the current stable for Debian but is being tested and should be shipped with next stable Debian "bookworm" release. https://tracker.debian.org/pkg/rustc
I think as a policy we can state, the bdk MSRV:
- Will only be changed with a new MINOR release, never a PATCH release.
- Will target Debian "stable" or "testing" release rust versions as long as possible given required dependencies.
A good discussion in favor of supporting older MSRVs here: https://github.com/tokio-rs/tokio/issues/3412
Since we started discussing this we've spent a lot of time having our CI break. It feels like it happens every other week. From my perspective the main security concern for BDK isn't having to use a recent version of the compiler it's losing coins because nobody has had time to fix things because the CI is often broken and we are backlogged with PRs. I acknowledge that not all the CI problems are MSRV related but most of them are.
I do think that a library like BDK should eventually have a conservative MSRV. If everyone thinks that the time is now then I think the right way to move forward is to extract components with dependencies into their own crate. That way we can keep the main BDK one lower and stable. We are doing a lot of work towards this but one of the main things slowing us down is the CI breaking. It's a getting a little bit demoralizing TBH. So I suggest the following:
- Fix MSRV to 1.56 for now which is the rust 2021 edition release and available on debian unstable.
- Remove all testing against any other version (stop testing against nightly which can be buggy).
- Have CI working for a while
- Do the work to extract things that have dependencies like Blockchains and DB backends to their own crate. This includes testing tools which are one of the main culprits. i.e. move the electrum/bitcoin RPC
TestClientto its own crate. - Do a v1.0.0 release where we fix the MSRV to debian stable and try to keep the API stable.
Of course I am only putting this forward as a suggestion. It's not me that suffers from this the most. @notmandatory is the one who has to fix this every time this happens. When I see that BDK is going through a rough period of CI stuff I just work on other things until I see that it's over.
I agree we need to get to a point where the BDK CI isn't breaking and when it does it's easier to fix. I like the plan above, two small corrections, if we go to MSRV 1.56.0 that will put us on Debian "testing" so it's not as bad as "unstable". And also we're not currently doing any testing with nightly but we do use it to build docs in order to use some unreleased feature but hopefully those will be released at some point.
Anyone else interested in this topic please comment here, and we'll talk about it at the team call on Tuesday.
I've always been very careful bumping the MSRV, but I agree that now it's starting to get very annoying and maybe it's time to relax our policy a little bit.
I also think we should eventually have many different crates (which would also help with the no-std work), but if you recall testutils used to be a separate crate and it was a pain in the ass to maintain, because we kept changing the API which meant having to constantly publish new releases of all the separate crates. I would wait at least for the big changes to Blockchain (splitting it up) and Database (removing &mut) before starting the "splitting up" work.
Btw I don't know why we keep talking so much about Debian, it's not the only distro out there and I believe Ubuntu is much more popular on the market (not sure which rustc version is shipped with Ubuntu though). Maybe a better target is based on how old a release is? i.e. "we will never use a compiler less than 6 months old" or something like that. We could go to 1.50.0 for a one year old release, or something more recent if there are some specific benefits from doing so (e.g. from using edition 2021).
Once we split up BDK in multiple crates we will consider going back to older versions (2y+) on the core lib.
I agree we should look at multiple distros, I was only focused on Debian because it seems to be one of the more conservative. But really what matters is which platforms our users want to distribute their apps for, and I don't think we have any data there yet.
Ubuntu 18.04 LTS and 20.04 LTS currently include rustc 1.57.0.
This comment in a tokio issue about supporting only back to the rust version the oldest supported version of Firefox needs, because most distributions will want to support Firefox, also makes sense.
I've always been very careful bumping the MSRV, but I agree that now it's starting to get very annoying and maybe it's time to relax our policy a little bit.
I also think we should eventually have many different crates (which would also help with the no-std work), but if you recall testutils used to be a separate crate and it was a pain in the ass to maintain, because we kept changing the API which meant having to constantly publish new releases of all the separate crates. I would wait at least for the big changes to
Blockchain(splitting it up) andDatabase(removing&mut) before starting the "splitting up" work.
Agree with this. The reason I moved testutils into the main crate was because of the test blockchains macros. I don't 100% recall the order of things but at some point @RCasatta developed this improved "TestClient" thing that helped us do testing without docker. I was suggesting moving TestClient out not the blockchain tests macro which still should be kept in the main repo until the blockchains have been extracted into their own crates. TestClient doesn't change too much during development and is useful for crates outside of BDK (I use it in gun for example).
Btw I don't know why we keep talking so much about Debian, it's not the only distro out there and I believe Ubuntu is much more popular on the market (not sure which rustc version is shipped with Ubuntu though). Maybe a better target is based on how old a release is? i.e. "we will never use a compiler less than 6 months old" or something like that. We could go to
1.50.0for a one year old release, or something more recent if there are some specific benefits from doing so (e.g. from using edition 2021).
# on ubuntu
$ apt show rustc
Package: rustc
Version: 1.57.0+dfsg1+llvm-0ubuntu1~21.10.1
FWIW I am also not too concerned about what debian is doing. I use FreeBSD and Ubunutu which both keep up relatively well. I do use rustup for development of course.
I think I lean towards doing things based on rust editions because big changes tend to cluster around those. So set BDK MSRV to 1.56 now and work to keep it there until some time after next edition and only change it if we need it. If some dependency needs a newer one we should try to spin it off into its own crate. I'm hoping that setting MSRV to 1.56 will buy us enough time to make this work.
I had to bump the MSRV to 1.57 again in #842. Even if we put things like the blockchain modules in different crates (as in bdk_core wip) to make an online wallet the users will still have to use a higher MSRV. Below are my notes on which dependencies we would need to find lower MSRV replacements for, or get PRs in to establish lower MSRVs:
- rand (1.56)
- electrum-client (1.57 due to rustls)
- use-esplora-async (1.56 due to reqwest)
- use-esplora-blocking (1.57 due to rustls)
- tokio (1.49 but only needed for async)
If we still need sqlite in bdk_core also:
- rusqlite (no MSRV)
- ahash (no MSRV)
BDK Features
minimal
| crate | version | MSRV (with no_std) |
|---|---|---|
| bitcoin | 0.29.2 | 1.41.1 (1.47.0) |
| log | 0.4.17 | 1.31.0 |
| miniscript | 9.0.0 | 1.41.1 (1.47.0) |
| rand | 0.8.5 | 1.56.0 |
| serde | 1.0.152 | 1.19.0 |
| serde_json | 1.0.92 | 1.36.0 |
| tokio | 1.25.0 | 1.49.0 |
with bitcoinconsensus
| crate | version | MSRV |
|---|---|---|
| bitcoinconsensus | 0.19.0 | 1.41.1 |
with electrum
| crate | version | MSRV |
|---|---|---|
| electrum-client | 0.12.0 | 1.57.0 |
with use-esplora-async
| crate | version | MSRV |
|---|---|---|
| esplora-client | 0.3.0 | 1.56.1 |
| futures | 0.3.26 | 1.36.0 |
with use-esplora-blocking
| crate | version | MSRV |
|---|---|---|
| esplora-client | 0.3.0 | 1.57.0 |
with key-value-db
| crate | version | MSRV |
|---|---|---|
| sled | 0.34.7 | 1.39.0 |
with sqlite
| crate | version | MSRV |
|---|---|---|
| ahash | 0.7.6 | none |
| rusqlite | 0.27.0 | none |
So this could be done now for bdk and bdk_chain with a few tiny fixups here and there. The problem is that it's extremely hard to get CI to build only bdk and bdk_chain now that they're in a workspace. There must be a way to hack around this. At the very least you could copy those two crates out of the workspace and just build them independently on 1.48.0. There's no need to run tests on that rust version just check that they build. Maybe there can be a "custom" workspace Cargo.toml that only includes things that are 1.48.0 compatible.
I think file_store will also probably build on 1.48.0 now.
@tnull mentioned on Discord that it will be nice for bdk_chain to have a MSRV of 1.48:
LDK is in the middle of raising it's MSRV to 1.48 for all its crates, which is a general target in the rust-bitcoin space I believe. For consistency's sake it would be awesome if BDK 1.0 could target that, too. (my two cents)
Related tickets:
- https://github.com/lightningdevkit/rust-lightning/pull/2107
- https://github.com/LLFourn/bdk_core_staging/issues/187
Tasks from our call last night:
- [ ] Update CI
- [ ] make sure complete subsets of bdk crates needed for a wallet compile with rust v1.48
- [ ] Create different .toml files for different MSRVs, does this work with cargo workspaces?
See this PR which makes progress on how to build for 1.48.0 separately: https://github.com/bitcoindevkit/bdk/pull/924 (it's not pretty).
We're still targeting rust 1.48, but in case that ever changes here's link from the Rust team on glibc and Linux kernel requirements: https://blog.rust-lang.org/2022/08/01/Increasing-glibc-kernel-requirements.html
Also for reference, glibc version for RHEL 6 is 2.11 and RHEL 7 is at 2.17, Debian 11 (bullseye) is at 2.31 and Debian 12 (bookworm) is 2.36.
It looks like LDK team is working on updating their MSRV to 1.63.0 which is the version that ships with the current debian stable (bookworm). If there are no objections from key users then I want to do the same for BDK 1.0.0-alpha.3 release.
lightningdevkit/rust-lightning/pull/268