Bump MSRV to rustc 1.63.0 #2681

tnull · 2023-10-24T13:29:20Z

We bump our MSRV to 1.63.0, which is a reasonable common ground, and also supported by Debian stable (bookworm).

Generally, supporting "Debian Stable MSRV" seems to be a reasonable middleground between the Rust ecosystem's "bump constantly, MSRV is stable" and keep supporting really old rustc versions that increase maintenance overhead.

Notably, this change reduces complexity of our CI scripts and will allow Mac users to run the CI scripts on MSRV locally on Apple Silicone chips.

(cc @notmandatory)

notmandatory · 2023-10-24T22:35:18Z

If LDK is bumping your MSRV to 1.63.0 with no objections from key users then I'll do the same for BDK.

codecov-commenter · 2023-10-25T07:37:51Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2d26679) 88.52% compared to head (21fa12f) 88.47%.

❗ Current head 21fa12f differs from pull request most recent head 1dcb02d. Consider uploading reports for the commit 1dcb02d to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2681      +/-   ##
==========================================
- Coverage   88.52%   88.47%   -0.06%     
==========================================
  Files         115      115              
  Lines       90996    91017      +21     
  Branches    90996    91017      +21     
==========================================
- Hits        80557    80530      -27     
- Misses       8012     8029      +17     
- Partials     2427     2458      +31

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tcharding · 2023-10-25T19:51:11Z

Hey @tnull can you mention someone from rust-bitcoin for things like this please - this one is massive, I was literally looking this week at LDK's usage of and old version of rust-bitcoin and wondering if it was an MSRV problem. Please holla at us with anything we can do to better support you guys, we are here to serve.

tnull · 2023-10-26T08:22:40Z

Hey @tnull can you mention someone from rust-bitcoin for things like this please - this one is massive, I was literally looking this week at LDK's usage of and old version of rust-bitcoin and wondering if it was an MSRV problem. Please holla at us with anything we can do to better support you guys, we are here to serve.

Yes, thank you, excuse the omission. Currently we're still double-checking we can actually make the MSRV bump work for all users, but looking to get this in next version if nothing comes up.

TheBlueMatt · 2023-10-27T17:16:39Z

Is there a reason to go all the way to 1.63? Sure, that's Debian's current stable, but what are our requirements aside from "macOS users should be able to build with our MSRV for local testing"?

tnull · 2023-10-27T17:55:33Z

Is there a reason to go all the way to 1.63? Sure, that's Debian's current stable, but what are our requirements aside from "macOS users should be able to build with our MSRV for local testing"?

Yes there are several reasons why I believe 1.63 to be a good candidate:

It covers the MSRV of all of our "release" downstream dependencies, and really the vast majority of all our dependencies. This includes several of the more important dependencies such as tokio,reqwest, etc which are currently at 1.63. This does not only allow us to generally use more up-to-date features but first and foremost allows us to remove almost all special casing in CI, e.g., we don't need to treat Windows builds separately and can now also build and test lightning-transaction-sync without special treatment.
Furthermore, it covers and includes a small buffer on top of the MSRVs of downstream TLS dependencies used in said crate, which would allow us to easily upgrade without delay/further versioning considerations if ever some security fixes would need to be shipped ASAP. It also means we can now introduce the tx-sync crate to bindings.
Generally having a small buffer on top of the 'bare minimum' MSRV bump is good as it at least delays us having to touch it again, even if some crates would slowly increase their MSRVs further.
It's shipped with Debian stable, which seems like a good MSRV/LTS target to track.
Maybe most importantly, it's a good candidate for a common MSRV that covers most projects in the rust-bitcoin space, namely it would allow LDK/BDK and dependent projects such as LDK Node to use a single MSRV which reduces friction when integrating different crates and reduces maintenance overhead.

notmandatory · 2023-10-27T22:37:00Z

In addition to what @tnull wrote above I'd like to add that, at least for the BDK team, maintaining the 1.57 MSRV has become an almost weekly battle to trouble shoot and then pin dependencies in our CI. We're now pinning 17 dependencies, from only five three months ago. We should also have an easier time making the case for dependency maintainers to support 1.63 due to it shipping with Debian stable.

TheBlueMatt · 2023-10-29T00:38:36Z

Assigning myself. The next java release will use 1.63 and Debian stable's glibc, so as long as that doesn't cause problems we're good.

tnull · 2023-11-27T17:45:20Z

Rebased to resolve minor conflicts after #2685 landed.

tnull · 2023-11-27T18:51:55Z

Seems I'll have some more pinning to do. Looking into it...

lightning-block-sync/Cargo.toml

lightning-transaction-sync/Cargo.toml

lightning/src/ln/payment_tests.rs

ci/ci-tests.sh

.. which is a reasonable common ground, also supported by Debian stable.

Since we now have a consistent MSRV and edition, we can move `lightning-custom-message` to the main workspace.

tnull · 2023-12-08T13:08:41Z

Rebased on current main and addressed outstanding feedback.

Previously, we used the auto-download feature of the `electrsd`/`bitcoind` crates. While convenient, they unnecessarily introduced a lot of dependecies (`zip`, `zstd`, `time`, etc.) to our test environment which needed pinning for the MSRV of 1.63. Here, we introduce a new `no_download` config flag to the `lightning-transaction-sync` crate allowing us to disable this auto-download feature in CI, where we now opt to download the corresponding binaries manually. We keep the default-auto-download as a convenience feature for running tests locally though.

dunxen

Other than what’s mentioned above I think there’s a few other warnings that we can finally clean up.

tnull · 2023-12-11T18:27:24Z

Other than what’s mentioned above I think there’s a few other warnings that we can finally clean up.

Hm, anything beside the rustdoc and const_err warnings that relate to MSRV?

TheBlueMatt · 2023-12-11T18:31:26Z

Either way, warnings can always be cleaned up later!

TheBlueMatt · 2023-12-11T18:35:40Z

We still have some broken_intra_doc_links tags, should those be cleaned up?

tnull · 2023-12-11T18:44:54Z

We still have some broken_intra_doc_links tags, should those be cleaned up?

~~Not sure I get what you're referring to? The warnings were about them being written as without the rustdoc:: prefix, which we now added as it was introduced in 1.63.~~

~~There should only be #![deny(rustdoc::broken_intra_doc_links)] left, no?~~

Lol, no, there are others left. Must have slipped in during rebase. Duh. Will open a warning-fix-pr.

169385b ci: change MSRV to 1.63.0 (Steve Myers) Pull request description: ### Description fixes #331 ### Notes to the reviewers We won't merge this PR until LDK merges lightningdevkit/rust-lightning#2681. There are alot of clippy checks to fix at 1.63.0 so I left the clippy MSRV at 1.57.0 for now. ### Changelog notice Changed - MSRV is now 1.63.0 for bdk, chain, and bitcoind_rpc crates ### Checklists #### All Submissions: * [x] I've signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) * [x] I ran `cargo fmt` and `cargo clippy` before committing ACKs for top commit: evanlinjin: ACK 169385b Tree-SHA512: ad69038173b4f050b57f637fc06a4153cf76929992cfea77e3f25d1e66be102bd2f83a46401a7e3245e9cc54602210c95b75a578f18c5c8b55d1e7229e92197f

tnull added this to the 0.0.119 milestone Oct 24, 2023

tnull requested a review from TheBlueMatt October 24, 2023 13:29

tnull force-pushed the 2023-10-bump-msrv-to-1.63.0 branch 3 times, most recently from 22caf96 to 50e765d Compare October 24, 2023 16:41

tnull force-pushed the 2023-10-bump-msrv-to-1.63.0 branch from 50e765d to ddee205 Compare October 25, 2023 07:37

tnull force-pushed the 2023-10-bump-msrv-to-1.63.0 branch 8 times, most recently from ffa9ff8 to ca62df4 Compare October 25, 2023 10:35

notmandatory mentioned this pull request Oct 25, 2023

Bump MSRV to 1.63.0 bitcoindevkit/bdk#1183

Merged

3 tasks

tnull force-pushed the 2023-10-bump-msrv-to-1.63.0 branch 3 times, most recently from a8f9de2 to eb8ab69 Compare October 26, 2023 12:48

TheBlueMatt self-assigned this Oct 29, 2023

tnull force-pushed the 2023-10-bump-msrv-to-1.63.0 branch 3 times, most recently from 27f827d to 44d1d24 Compare November 1, 2023 10:06

TheBlueMatt reviewed Nov 27, 2023

View reviewed changes

lightning-block-sync/Cargo.toml Show resolved Hide resolved

lightning-transaction-sync/Cargo.toml Show resolved Hide resolved

lightning/src/ln/payment_tests.rs Show resolved Hide resolved

ci/ci-tests.sh Outdated Show resolved Hide resolved

nondiremanuel mentioned this pull request Dec 2, 2023

W49 Lib Team Call bitcoindevkit/.github#87

Closed

55 tasks

tnull force-pushed the 2023-10-bump-msrv-to-1.63.0 branch 7 times, most recently from f96881a to 3e82357 Compare December 8, 2023 11:57

tnull added 2 commits December 8, 2023 14:03

Bump MSRV to rustc 1.63.0 and edition to 2021

ddf2509

.. which is a reasonable common ground, also supported by Debian stable.

Move lightning-custom-message to main workspace

7301ff0

Since we now have a consistent MSRV and edition, we can move `lightning-custom-message` to the main workspace.

tnull force-pushed the 2023-10-bump-msrv-to-1.63.0 branch from 3e82357 to 215cad6 Compare December 8, 2023 13:03

tnull force-pushed the 2023-10-bump-msrv-to-1.63.0 branch 4 times, most recently from 1dcb02d to 9f2218f Compare December 8, 2023 13:39

tnull force-pushed the 2023-10-bump-msrv-to-1.63.0 branch from 9f2218f to f368fac Compare December 8, 2023 13:41

TheBlueMatt approved these changes Dec 8, 2023

View reviewed changes

dunxen approved these changes Dec 11, 2023

View reviewed changes

TheBlueMatt merged commit e839d49 into lightningdevkit:main Dec 11, 2023
15 checks passed

tnull mentioned this pull request Feb 6, 2024

Introduce CI workflow running cargo audit #2861

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump MSRV to rustc 1.63.0 #2681

Bump MSRV to rustc 1.63.0 #2681

tnull commented Oct 24, 2023 •

edited

Loading

notmandatory commented Oct 24, 2023

codecov-commenter commented Oct 25, 2023 •

edited

Loading

tcharding commented Oct 25, 2023

tnull commented Oct 26, 2023

TheBlueMatt commented Oct 27, 2023

tnull commented Oct 27, 2023

notmandatory commented Oct 27, 2023

TheBlueMatt commented Oct 29, 2023

tnull commented Nov 27, 2023

tnull commented Nov 27, 2023

tnull commented Dec 8, 2023

dunxen left a comment

tnull commented Dec 11, 2023 •

edited

Loading

TheBlueMatt commented Dec 11, 2023

TheBlueMatt commented Dec 11, 2023

tnull commented Dec 11, 2023 •

edited

Loading

Bump MSRV to rustc 1.63.0 #2681

Bump MSRV to rustc 1.63.0 #2681

Conversation

tnull commented Oct 24, 2023 • edited Loading

notmandatory commented Oct 24, 2023

codecov-commenter commented Oct 25, 2023 • edited Loading

Codecov Report

tcharding commented Oct 25, 2023

tnull commented Oct 26, 2023

TheBlueMatt commented Oct 27, 2023

tnull commented Oct 27, 2023

notmandatory commented Oct 27, 2023

TheBlueMatt commented Oct 29, 2023

tnull commented Nov 27, 2023

tnull commented Nov 27, 2023

tnull commented Dec 8, 2023

dunxen left a comment

Choose a reason for hiding this comment

tnull commented Dec 11, 2023 • edited Loading

TheBlueMatt commented Dec 11, 2023

TheBlueMatt commented Dec 11, 2023

tnull commented Dec 11, 2023 • edited Loading

tnull commented Oct 24, 2023 •

edited

Loading

codecov-commenter commented Oct 25, 2023 •

edited

Loading

tnull commented Dec 11, 2023 •

edited

Loading

tnull commented Dec 11, 2023 •

edited

Loading