Bump MSRV to 1.85.0 and rust edition to 2024#4602
Bump MSRV to 1.85.0 and rust edition to 2024#4602tnull wants to merge 16 commits intolightningdevkit:mainfrom
Conversation
The workspace now requires `rustc` 1.85.0 across manifests, CI, docs, and the pending changelog entry. This keeps contributor and CI expectations aligned with the toolchain needed by the rest of this PR. Code and lint configuration are adjusted where 1.85 reports new diagnostics so the raised MSRV builds cleanly. Co-Authored-By: HAL 9000
The old pins only existed to keep `rustc` 1.75 builds working. With the workspace MSRV raised, the normal dependency graph can move forward without carrying those compatibility constraints. Co-Authored-By: HAL 9000
`rustc` 1.85 no longer needs the crate-level test allowances that guarded older diagnostics. Removing the allowances lets the test code follow the normal lint policy under the new MSRV. Co-Authored-By: HAL 9000
`Iterator::is_sorted_by_key` is available with the raised MSRV. The validation behavior stays the same while block source initialization avoids a local manual ordering check. Co-Authored-By: HAL 9000
`Option::take_if` is available with the raised MSRV. Monitor cleanup still removes only matching alternative funding entries, while the condition and removal stay together. Co-Authored-By: HAL 9000
`Result::inspect_err` is available with the raised MSRV. Channel logic still records the same shutdown action before returning the error, with the side effect tied to the failing result. Co-Authored-By: HAL 9000
`slice::chunk_by` is available with the raised MSRV. Forward processing keeps grouping contiguous entries by node id without maintaining an explicit chunk cursor. Co-Authored-By: HAL 9000
The onion packet size is fixed by the message format. Keeping packet bytes in arrays preserves that invariant directly and avoids heap backed vectors where the length cannot vary. Co-Authored-By: HAL 9000
`Waker::noop` is available with the raised MSRV. Shared polling sites no longer need a local dummy waker helper just to drive futures that do not depend on wakeups. Co-Authored-By: HAL 9000
`Waker::noop` is available with the raised MSRV. Background processing can poll persistence futures without carrying its local dummy waker helper. Co-Authored-By: HAL 9000
The background processor no longer needs boxed futures for persistence polling under the new MSRV. Keeping them pinned on the stack avoids unnecessary allocation while preserving polling order. Co-Authored-By: HAL 9000
Async closures are available with the raised MSRV. Background event paths can express their deferred work directly without the wrapper future pattern that was only needed for older compilers. Co-Authored-By: HAL 9000
`Option::take` for `NonZero` types is available under the new MSRV. The comment no longer describes an active limitation, so keeping it would mislead future cleanup work. Co-Authored-By: HAL 9000
The MSRV bump makes the 2024 edition available to all workspace crates. Updating the manifests now keeps the codebase on the current edition and applies the required compatibility fixes in production, test, and fuzz targets. This commit leaves the broad formatting churn to the following `rustfmt` commit so the compatibility changes remain reviewable. Co-Authored-By: HAL 9000
The edition migration changes `rustfmt` output across the workspace. Keeping the formatting-only delta separate leaves the preceding commit focused on manifest and compatibility changes. Co-Authored-By: HAL 9000
The persister comment claimed the boxed future could be dropped after Rust 2024. That is not accurate on `rustc` 1.85 because precise captures are still unavailable for `impl Trait` in trait methods. Removing the note avoids pointing future work at an unsupported change. Co-Authored-By: HAL 9000
|
I've assigned @tankyleo as a reviewer! |
Review SummaryThis is a clean, comprehensive MSRV bump (1.75 -> 1.85) and Rust edition migration (2021 -> 2024). All 23 workspace crates are consistently updated. The substantive code changes are limited to:
No bugs, security issues, or logic errors found. Inline comments posted:
|
joostjager
left a comment
joostjager
left a comment
There was a problem hiding this comment.
Great to see the MSRV bump. If LDK Node and LDK Server are already on 1.85, and the vast majority of users consume rust-lightning through those projects, I do not see much value in holding rust-lightning back.
I am less sure about bundling all of the cleanup and edition-driven code changes into the same PR. That adds review and regression risk, similar to the rustfmt-style reformatting projects we have had in the past, and also makes this harder to get merged. I think it would be better to keep this as an MSRV-only bump first, then do the code cleanups opportunistically when touching those areas in the future.
|
We also get rid of the annoying cargo lock file conflicts |
|
IIRC we previously discussed holding off until 0.3 as well, otherwise we're bumping from 1.63 to 1.85 in one release which seems excessive. Would have been kinda nice to do the same in LDK Node. |
Well, I made sure to split-out the |
Okay, but that is around the corner, no? Happy to rebase so we can get this out of the way first thing after we branch off 0.3-beta.
Well, we won't have two bumps in one release there, as v0.7.0 already bumped to v1.85.0. |
|
Oh yea, sheesh that diff is huge. What causes rustfmt to want to change everything? Can we leave the old edition and ignore it (is there anything in the edition that we want?). Its also not clear to me that this is worth much - the only change that's actually a nice improvement seems to be the dropping of a few Boxes in the async BP code? |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4602 +/- ##
==========================================
- Coverage 86.16% 85.84% -0.32%
==========================================
Files 156 157 +1
Lines 108669 109418 +749
Branches 108669 109418 +749
==========================================
+ Hits 93638 93934 +296
- Misses 12420 12864 +444
- Partials 2611 2620 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Obviously you don't see the places where things were cleaned up. Or where code was cleaned without comments. Not sure if looking at what remains is a strong signal. But even if we don't clean up, I don't think it is a big deal. At least someone could if they wanted too. And new code can make use of the larger feature set. For me, the gain would mostly be to get rid of that annoying lock file conflict. On macos, libfuzzer needs nightly. |
5 participants
We previously decided on bumping the MSRV to the latest version Debian 13 'trixie' provides (1.85.0). As previously discussed (offline, but also on #4002) we however decided to defer that until after the release of Ubuntu 26.04 LTS as the prior Ubuntu LTS version didn't provide packages for
rustc1.85+. Now Ubuntu 26.04 is almost out for a month and we're free to finally bump the MSRV to 1.85.0.This not only allows us to clean up the codebase and quite a few workarounds that were previously blocked on the Rust version, but also aligns the MSRV across LDK, LDK Node, and LDK Server (the latter have been on 1.85.0 for a while).