Bump MSRV to 1.75.0

[tnull]

We generally align our MSRV with Debian's stable channel. Debian 13 'Trixie' was just released, shipping rustc 1.85. However, as 1.85.0 is only about ~7months old at this point, we opt to bump to the more conservative 1.75.0, which approaches two years of age.

[ldk-reviews-bot]

👋 Thanks for assigning @TheBlueMatt as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

[tnull]

I also addressed most MSRV-related TODOs in the code, only thing left is

rust-lightning/lightning/src/ln/channelmanager.rs

Line 236 in 192618b

short_channel_id:u64,// This should be NonZero<u64> eventually when we bump MSRV

@TheBlueMatt any opinion on whether to do it here, in a separate PR, or as part of #3973 ?

[codecov]

Codecov Report

❌ Patch coverage is 85.10638% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.78%. Comparing base (7f20d47) to head (165a6b3).
⚠️ Report is 27 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning-persister/src/utils.rs	0.00%	5 Missing ⚠️
lightning-liquidity/src/manager.rs	33.33%	2 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4002 +/- ##
==========================================
- Coverage 88.80% 88.78% -0.02% 
==========================================
Files 180 180 Lines 137004 136986 -18 Branches 137004 136986 -18 ==========================================
- Hits 121660 121627 -33 - Misses 12522 12545 +23 + Partials 2822 2814 -8 

Flag	Coverage Δ
fuzzing	21.55% <6.81%> (+<0.01%)	⬆️
tests	88.63% <85.10%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We generally align our MSRV with Debian's stable channel

I don't think this is entirely true :). We generally align our MSRV with rust-bitcoin and the rest of the ecosystem. IIRC rust-bitcoin usually does something like min(2-year-old-rustc, debian stable, rustc that introduces materially useful features). The last time we bumped MSRV (#2681) the rustc we bumped to was a year and a few months old, and the release containing it wasn't until Dec, so rustc 1.63 was almost a year and a half old.

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

[tnull]

We generally align our MSRV with rust-bitcoin and the rest of the ecosystem.

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.). Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec<Utxo>, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

IMO it makes a whole lot of sense to upgrade now that we can, if just because it makes our lives easier in many little places, but also because it allows us to expose a more coherent Rust-native API.

[TheBlueMatt]

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

We don't wait for rust-bitcoin, but we definitely coordinate around specific version of rustc.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.).

This doesn't sound 1.85-specific?

Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

Sadly even with rustc nightly we wouldn't want to change that. Our async KVStore requires ordering, which is not possible with native rust async methods as they do not run any code at all until polled. I guess in theory we could require "ordering after the first poll" and poll once whenever we persist, but that seems even more brittle than the current version which at least exposes the concept to the implementer.

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

We should also want 1.68 for the pin macro, afair (removes allocations in each loop of the BP).

[TheBlueMatt]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

[tnull]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

While I'd personally still be in favor of 1.85, I now switched this PR to bump to 1.75.0.

[TheBlueMatt]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

[ldk-reviews-bot]

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

[tnull]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Ah, missed re-adding the deleted pin for this.

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

I guess? Also don't see a strong argument to wait? Or do you mean so that the 0.2 release would still have the old MSRV? Then again, 1.75.0 is ancient by now.

[TheBlueMatt]

Or do you mean so that the 0.2 release would still have the old MSRV?

This.

[tnull]

Putting in draft until then.

[tnull]

Rebased on current main and addressed remaining linter warnings.

[tnull]

Now actually whack-a-mole'd all remaining lints.

[ldk-reviews-bot]

🔔 1st Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[TheBlueMatt]

Thanks. We should also move lightning-transaction-sync back into the workspace now that they have the same MSRV. There's probably some Box::pin we can drop now too.

[tnull]

We generally align our MSRV with Debian's stable channel. Debian 13 'Trixie' was just released, shipping rustc 1.85. However, as 1.85.0 is only about ~7months old at this point, we opt to bump to the more conservative 1.75.0, which approaches two years of age.

[ldk-reviews-bot]

👋 Thanks for assigning @TheBlueMatt as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

[tnull]

I also addressed most MSRV-related TODOs in the code, only thing left is

rust-lightning/lightning/src/ln/channelmanager.rs

Line 236 in 192618b

short_channel_id:u64,// This should be NonZero<u64> eventually when we bump MSRV

@TheBlueMatt any opinion on whether to do it here, in a separate PR, or as part of #3973 ?

[codecov]

Codecov Report

❌ Patch coverage is 85.10638% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.78%. Comparing base (7f20d47) to head (165a6b3).
⚠️ Report is 27 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning-persister/src/utils.rs	0.00%	5 Missing ⚠️
lightning-liquidity/src/manager.rs	33.33%	2 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4002 +/- ##
==========================================
- Coverage 88.80% 88.78% -0.02% 
==========================================
Files 180 180 Lines 137004 136986 -18 Branches 137004 136986 -18 ==========================================
- Hits 121660 121627 -33 - Misses 12522 12545 +23 + Partials 2822 2814 -8 

Flag	Coverage Δ
fuzzing	21.55% <6.81%> (+<0.01%)	⬆️
tests	88.63% <85.10%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We generally align our MSRV with Debian's stable channel

I don't think this is entirely true :). We generally align our MSRV with rust-bitcoin and the rest of the ecosystem. IIRC rust-bitcoin usually does something like min(2-year-old-rustc, debian stable, rustc that introduces materially useful features). The last time we bumped MSRV (#2681) the rustc we bumped to was a year and a few months old, and the release containing it wasn't until Dec, so rustc 1.63 was almost a year and a half old.

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

[tnull]

We generally align our MSRV with rust-bitcoin and the rest of the ecosystem.

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.). Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec<Utxo>, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

IMO it makes a whole lot of sense to upgrade now that we can, if just because it makes our lives easier in many little places, but also because it allows us to expose a more coherent Rust-native API.

[TheBlueMatt]

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

We don't wait for rust-bitcoin, but we definitely coordinate around specific version of rustc.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.).

This doesn't sound 1.85-specific?

Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

Sadly even with rustc nightly we wouldn't want to change that. Our async KVStore requires ordering, which is not possible with native rust async methods as they do not run any code at all until polled. I guess in theory we could require "ordering after the first poll" and poll once whenever we persist, but that seems even more brittle than the current version which at least exposes the concept to the implementer.

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

We should also want 1.68 for the pin macro, afair (removes allocations in each loop of the BP).

[TheBlueMatt]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

[tnull]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

While I'd personally still be in favor of 1.85, I now switched this PR to bump to 1.75.0.

[TheBlueMatt]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

[ldk-reviews-bot]

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

[tnull]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Ah, missed re-adding the deleted pin for this.

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

I guess? Also don't see a strong argument to wait? Or do you mean so that the 0.2 release would still have the old MSRV? Then again, 1.75.0 is ancient by now.

[TheBlueMatt]

Or do you mean so that the 0.2 release would still have the old MSRV?

This.

[tnull]

Putting in draft until then.

[tnull]

Rebased on current main and addressed remaining linter warnings.

[tnull]

Now actually whack-a-mole'd all remaining lints.

[ldk-reviews-bot]

🔔 1st Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[TheBlueMatt]

Thanks. We should also move lightning-transaction-sync back into the workspace now that they have the same MSRV. There's probably some Box::pin we can drop now too.

[tnull]

We generally align our MSRV with Debian's stable channel. Debian 13 'Trixie' was just released, shipping rustc 1.85. However, as 1.85.0 is only about ~7months old at this point, we opt to bump to the more conservative 1.75.0, which approaches two years of age.

[ldk-reviews-bot]

👋 Thanks for assigning @TheBlueMatt as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

[tnull]

I also addressed most MSRV-related TODOs in the code, only thing left is

rust-lightning/lightning/src/ln/channelmanager.rs

Line 236 in 192618b

short_channel_id:u64,// This should be NonZero<u64> eventually when we bump MSRV

@TheBlueMatt any opinion on whether to do it here, in a separate PR, or as part of #3973 ?

[codecov]

Codecov Report

❌ Patch coverage is 85.10638% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.78%. Comparing base (7f20d47) to head (165a6b3).
⚠️ Report is 27 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning-persister/src/utils.rs	0.00%	5 Missing ⚠️
lightning-liquidity/src/manager.rs	33.33%	2 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4002 +/- ##
==========================================
- Coverage 88.80% 88.78% -0.02% 
==========================================
Files 180 180 Lines 137004 136986 -18 Branches 137004 136986 -18 ==========================================
- Hits 121660 121627 -33 - Misses 12522 12545 +23 + Partials 2822 2814 -8 

Flag	Coverage Δ
fuzzing	21.55% <6.81%> (+<0.01%)	⬆️
tests	88.63% <85.10%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We generally align our MSRV with Debian's stable channel

I don't think this is entirely true :). We generally align our MSRV with rust-bitcoin and the rest of the ecosystem. IIRC rust-bitcoin usually does something like min(2-year-old-rustc, debian stable, rustc that introduces materially useful features). The last time we bumped MSRV (#2681) the rustc we bumped to was a year and a few months old, and the release containing it wasn't until Dec, so rustc 1.63 was almost a year and a half old.

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

[tnull]

We generally align our MSRV with rust-bitcoin and the rest of the ecosystem.

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.). Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec<Utxo>, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

IMO it makes a whole lot of sense to upgrade now that we can, if just because it makes our lives easier in many little places, but also because it allows us to expose a more coherent Rust-native API.

[TheBlueMatt]

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

We don't wait for rust-bitcoin, but we definitely coordinate around specific version of rustc.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.).

This doesn't sound 1.85-specific?

Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

Sadly even with rustc nightly we wouldn't want to change that. Our async KVStore requires ordering, which is not possible with native rust async methods as they do not run any code at all until polled. I guess in theory we could require "ordering after the first poll" and poll once whenever we persist, but that seems even more brittle than the current version which at least exposes the concept to the implementer.

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

We should also want 1.68 for the pin macro, afair (removes allocations in each loop of the BP).

[TheBlueMatt]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

[tnull]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

While I'd personally still be in favor of 1.85, I now switched this PR to bump to 1.75.0.

[TheBlueMatt]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

[ldk-reviews-bot]

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

[tnull]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Ah, missed re-adding the deleted pin for this.

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

I guess? Also don't see a strong argument to wait? Or do you mean so that the 0.2 release would still have the old MSRV? Then again, 1.75.0 is ancient by now.

[TheBlueMatt]

Or do you mean so that the 0.2 release would still have the old MSRV?

This.

[tnull]

Putting in draft until then.

[tnull]

Rebased on current main and addressed remaining linter warnings.

[tnull]

Now actually whack-a-mole'd all remaining lints.

[ldk-reviews-bot]

🔔 1st Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[TheBlueMatt]

Thanks. We should also move lightning-transaction-sync back into the workspace now that they have the same MSRV. There's probably some Box::pin we can drop now too.

[tnull]

We generally align our MSRV with Debian's stable channel. Debian 13 'Trixie' was just released, shipping rustc 1.85. However, as 1.85.0 is only about ~7months old at this point, we opt to bump to the more conservative 1.75.0, which approaches two years of age.

[ldk-reviews-bot]

👋 Thanks for assigning @TheBlueMatt as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

[tnull]

I also addressed most MSRV-related TODOs in the code, only thing left is

rust-lightning/lightning/src/ln/channelmanager.rs

Line 236 in 192618b

short_channel_id:u64,// This should be NonZero<u64> eventually when we bump MSRV

@TheBlueMatt any opinion on whether to do it here, in a separate PR, or as part of #3973 ?

[codecov]

Codecov Report

❌ Patch coverage is 85.10638% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.78%. Comparing base (7f20d47) to head (165a6b3).
⚠️ Report is 27 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning-persister/src/utils.rs	0.00%	5 Missing ⚠️
lightning-liquidity/src/manager.rs	33.33%	2 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4002 +/- ##
==========================================
- Coverage 88.80% 88.78% -0.02% 
==========================================
Files 180 180 Lines 137004 136986 -18 Branches 137004 136986 -18 ==========================================
- Hits 121660 121627 -33 - Misses 12522 12545 +23 + Partials 2822 2814 -8 

Flag	Coverage Δ
fuzzing	21.55% <6.81%> (+<0.01%)	⬆️
tests	88.63% <85.10%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We generally align our MSRV with Debian's stable channel

I don't think this is entirely true :). We generally align our MSRV with rust-bitcoin and the rest of the ecosystem. IIRC rust-bitcoin usually does something like min(2-year-old-rustc, debian stable, rustc that introduces materially useful features). The last time we bumped MSRV (#2681) the rustc we bumped to was a year and a few months old, and the release containing it wasn't until Dec, so rustc 1.63 was almost a year and a half old.

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

[tnull]

We generally align our MSRV with rust-bitcoin and the rest of the ecosystem.

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.). Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec<Utxo>, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

IMO it makes a whole lot of sense to upgrade now that we can, if just because it makes our lives easier in many little places, but also because it allows us to expose a more coherent Rust-native API.

[TheBlueMatt]

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

We don't wait for rust-bitcoin, but we definitely coordinate around specific version of rustc.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.).

This doesn't sound 1.85-specific?

Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

Sadly even with rustc nightly we wouldn't want to change that. Our async KVStore requires ordering, which is not possible with native rust async methods as they do not run any code at all until polled. I guess in theory we could require "ordering after the first poll" and poll once whenever we persist, but that seems even more brittle than the current version which at least exposes the concept to the implementer.

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

We should also want 1.68 for the pin macro, afair (removes allocations in each loop of the BP).

[TheBlueMatt]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

[tnull]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

While I'd personally still be in favor of 1.85, I now switched this PR to bump to 1.75.0.

[TheBlueMatt]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

[ldk-reviews-bot]

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

[tnull]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Ah, missed re-adding the deleted pin for this.

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

I guess? Also don't see a strong argument to wait? Or do you mean so that the 0.2 release would still have the old MSRV? Then again, 1.75.0 is ancient by now.

[TheBlueMatt]

Or do you mean so that the 0.2 release would still have the old MSRV?

This.

[tnull]

Putting in draft until then.

[tnull]

Rebased on current main and addressed remaining linter warnings.

[tnull]

Now actually whack-a-mole'd all remaining lints.

[ldk-reviews-bot]

🔔 1st Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[TheBlueMatt]

Thanks. We should also move lightning-transaction-sync back into the workspace now that they have the same MSRV. There's probably some Box::pin we can drop now too.

[tnull]

We generally align our MSRV with Debian's stable channel. Debian 13 'Trixie' was just released, shipping rustc 1.85. However, as 1.85.0 is only about ~7months old at this point, we opt to bump to the more conservative 1.75.0, which approaches two years of age.

[ldk-reviews-bot]

👋 Thanks for assigning @TheBlueMatt as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

[tnull]

I also addressed most MSRV-related TODOs in the code, only thing left is

rust-lightning/lightning/src/ln/channelmanager.rs

Line 236 in 192618b

short_channel_id:u64,// This should be NonZero<u64> eventually when we bump MSRV

@TheBlueMatt any opinion on whether to do it here, in a separate PR, or as part of #3973 ?

[codecov]

Codecov Report

❌ Patch coverage is 85.10638% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.78%. Comparing base (7f20d47) to head (165a6b3).
⚠️ Report is 27 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning-persister/src/utils.rs	0.00%	5 Missing ⚠️
lightning-liquidity/src/manager.rs	33.33%	2 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4002 +/- ##
==========================================
- Coverage 88.80% 88.78% -0.02% 
==========================================
Files 180 180 Lines 137004 136986 -18 Branches 137004 136986 -18 ==========================================
- Hits 121660 121627 -33 - Misses 12522 12545 +23 + Partials 2822 2814 -8 

Flag	Coverage Δ
fuzzing	21.55% <6.81%> (+<0.01%)	⬆️
tests	88.63% <85.10%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We generally align our MSRV with Debian's stable channel

I don't think this is entirely true :). We generally align our MSRV with rust-bitcoin and the rest of the ecosystem. IIRC rust-bitcoin usually does something like min(2-year-old-rustc, debian stable, rustc that introduces materially useful features). The last time we bumped MSRV (#2681) the rustc we bumped to was a year and a few months old, and the release containing it wasn't until Dec, so rustc 1.63 was almost a year and a half old.

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

[tnull]

We generally align our MSRV with rust-bitcoin and the rest of the ecosystem.

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.). Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec<Utxo>, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

IMO it makes a whole lot of sense to upgrade now that we can, if just because it makes our lives easier in many little places, but also because it allows us to expose a more coherent Rust-native API.

[TheBlueMatt]

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

We don't wait for rust-bitcoin, but we definitely coordinate around specific version of rustc.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.).

This doesn't sound 1.85-specific?

Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

Sadly even with rustc nightly we wouldn't want to change that. Our async KVStore requires ordering, which is not possible with native rust async methods as they do not run any code at all until polled. I guess in theory we could require "ordering after the first poll" and poll once whenever we persist, but that seems even more brittle than the current version which at least exposes the concept to the implementer.

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

We should also want 1.68 for the pin macro, afair (removes allocations in each loop of the BP).

[TheBlueMatt]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

[tnull]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

While I'd personally still be in favor of 1.85, I now switched this PR to bump to 1.75.0.

[TheBlueMatt]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

[ldk-reviews-bot]

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

[tnull]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Ah, missed re-adding the deleted pin for this.

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

I guess? Also don't see a strong argument to wait? Or do you mean so that the 0.2 release would still have the old MSRV? Then again, 1.75.0 is ancient by now.

[TheBlueMatt]

Or do you mean so that the 0.2 release would still have the old MSRV?

This.

[tnull]

Putting in draft until then.

[tnull]

Rebased on current main and addressed remaining linter warnings.

[tnull]

Now actually whack-a-mole'd all remaining lints.

[ldk-reviews-bot]

🔔 1st Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[TheBlueMatt]

Thanks. We should also move lightning-transaction-sync back into the workspace now that they have the same MSRV. There's probably some Box::pin we can drop now too.

[tnull]

We generally align our MSRV with Debian's stable channel. Debian 13 'Trixie' was just released, shipping rustc 1.85. However, as 1.85.0 is only about ~7months old at this point, we opt to bump to the more conservative 1.75.0, which approaches two years of age.

[ldk-reviews-bot]

👋 Thanks for assigning @TheBlueMatt as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

[tnull]

I also addressed most MSRV-related TODOs in the code, only thing left is

rust-lightning/lightning/src/ln/channelmanager.rs

Line 236 in 192618b

short_channel_id:u64,// This should be NonZero<u64> eventually when we bump MSRV

@TheBlueMatt any opinion on whether to do it here, in a separate PR, or as part of #3973 ?

[codecov]

Codecov Report

❌ Patch coverage is 85.10638% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.78%. Comparing base (7f20d47) to head (165a6b3).
⚠️ Report is 27 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning-persister/src/utils.rs	0.00%	5 Missing ⚠️
lightning-liquidity/src/manager.rs	33.33%	2 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4002 +/- ##
==========================================
- Coverage 88.80% 88.78% -0.02% 
==========================================
Files 180 180 Lines 137004 136986 -18 Branches 137004 136986 -18 ==========================================
- Hits 121660 121627 -33 - Misses 12522 12545 +23 + Partials 2822 2814 -8 

Flag	Coverage Δ
fuzzing	21.55% <6.81%> (+<0.01%)	⬆️
tests	88.63% <85.10%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We generally align our MSRV with Debian's stable channel

I don't think this is entirely true :). We generally align our MSRV with rust-bitcoin and the rest of the ecosystem. IIRC rust-bitcoin usually does something like min(2-year-old-rustc, debian stable, rustc that introduces materially useful features). The last time we bumped MSRV (#2681) the rustc we bumped to was a year and a few months old, and the release containing it wasn't until Dec, so rustc 1.63 was almost a year and a half old.

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

[tnull]

We generally align our MSRV with rust-bitcoin and the rest of the ecosystem.

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.). Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec<Utxo>, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

IMO it makes a whole lot of sense to upgrade now that we can, if just because it makes our lives easier in many little places, but also because it allows us to expose a more coherent Rust-native API.

[TheBlueMatt]

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

We don't wait for rust-bitcoin, but we definitely coordinate around specific version of rustc.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.).

This doesn't sound 1.85-specific?

Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

Sadly even with rustc nightly we wouldn't want to change that. Our async KVStore requires ordering, which is not possible with native rust async methods as they do not run any code at all until polled. I guess in theory we could require "ordering after the first poll" and poll once whenever we persist, but that seems even more brittle than the current version which at least exposes the concept to the implementer.

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

We should also want 1.68 for the pin macro, afair (removes allocations in each loop of the BP).

[TheBlueMatt]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

[tnull]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

While I'd personally still be in favor of 1.85, I now switched this PR to bump to 1.75.0.

[TheBlueMatt]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

[ldk-reviews-bot]

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

[tnull]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Ah, missed re-adding the deleted pin for this.

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

I guess? Also don't see a strong argument to wait? Or do you mean so that the 0.2 release would still have the old MSRV? Then again, 1.75.0 is ancient by now.

[TheBlueMatt]

Or do you mean so that the 0.2 release would still have the old MSRV?

This.

[tnull]

Putting in draft until then.

[tnull]

Rebased on current main and addressed remaining linter warnings.

[tnull]

Now actually whack-a-mole'd all remaining lints.

[ldk-reviews-bot]

🔔 1st Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[TheBlueMatt]

Thanks. We should also move lightning-transaction-sync back into the workspace now that they have the same MSRV. There's probably some Box::pin we can drop now too.

[tnull]

We generally align our MSRV with Debian's stable channel. Debian 13 'Trixie' was just released, shipping rustc 1.85. However, as 1.85.0 is only about ~7months old at this point, we opt to bump to the more conservative 1.75.0, which approaches two years of age.

[ldk-reviews-bot]

👋 Thanks for assigning @TheBlueMatt as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

[tnull]

I also addressed most MSRV-related TODOs in the code, only thing left is

rust-lightning/lightning/src/ln/channelmanager.rs

Line 236 in 192618b

short_channel_id:u64,// This should be NonZero<u64> eventually when we bump MSRV

@TheBlueMatt any opinion on whether to do it here, in a separate PR, or as part of #3973 ?

[codecov]

Codecov Report

❌ Patch coverage is 85.10638% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.78%. Comparing base (7f20d47) to head (165a6b3).
⚠️ Report is 27 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning-persister/src/utils.rs	0.00%	5 Missing ⚠️
lightning-liquidity/src/manager.rs	33.33%	2 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4002 +/- ##
==========================================
- Coverage 88.80% 88.78% -0.02% 
==========================================
Files 180 180 Lines 137004 136986 -18 Branches 137004 136986 -18 ==========================================
- Hits 121660 121627 -33 - Misses 12522 12545 +23 + Partials 2822 2814 -8 

Flag	Coverage Δ
fuzzing	21.55% <6.81%> (+<0.01%)	⬆️
tests	88.63% <85.10%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We generally align our MSRV with Debian's stable channel

I don't think this is entirely true :). We generally align our MSRV with rust-bitcoin and the rest of the ecosystem. IIRC rust-bitcoin usually does something like min(2-year-old-rustc, debian stable, rustc that introduces materially useful features). The last time we bumped MSRV (#2681) the rustc we bumped to was a year and a few months old, and the release containing it wasn't until Dec, so rustc 1.63 was almost a year and a half old.

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

[tnull]

We generally align our MSRV with rust-bitcoin and the rest of the ecosystem.

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.). Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec<Utxo>, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

IMO it makes a whole lot of sense to upgrade now that we can, if just because it makes our lives easier in many little places, but also because it allows us to expose a more coherent Rust-native API.

[TheBlueMatt]

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

We don't wait for rust-bitcoin, but we definitely coordinate around specific version of rustc.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.).

This doesn't sound 1.85-specific?

Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

Sadly even with rustc nightly we wouldn't want to change that. Our async KVStore requires ordering, which is not possible with native rust async methods as they do not run any code at all until polled. I guess in theory we could require "ordering after the first poll" and poll once whenever we persist, but that seems even more brittle than the current version which at least exposes the concept to the implementer.

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

We should also want 1.68 for the pin macro, afair (removes allocations in each loop of the BP).

[TheBlueMatt]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

[tnull]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

While I'd personally still be in favor of 1.85, I now switched this PR to bump to 1.75.0.

[TheBlueMatt]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

[ldk-reviews-bot]

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

[tnull]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Ah, missed re-adding the deleted pin for this.

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

I guess? Also don't see a strong argument to wait? Or do you mean so that the 0.2 release would still have the old MSRV? Then again, 1.75.0 is ancient by now.

[TheBlueMatt]

Or do you mean so that the 0.2 release would still have the old MSRV?

This.

[tnull]

Putting in draft until then.

[tnull]

Rebased on current main and addressed remaining linter warnings.

[tnull]

Now actually whack-a-mole'd all remaining lints.

[ldk-reviews-bot]

🔔 1st Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[TheBlueMatt]

Thanks. We should also move lightning-transaction-sync back into the workspace now that they have the same MSRV. There's probably some Box::pin we can drop now too.

[tnull]

We generally align our MSRV with Debian's stable channel. Debian 13 'Trixie' was just released, shipping rustc 1.85. However, as 1.85.0 is only about ~7months old at this point, we opt to bump to the more conservative 1.75.0, which approaches two years of age.

[ldk-reviews-bot]

👋 Thanks for assigning @TheBlueMatt as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

[tnull]

I also addressed most MSRV-related TODOs in the code, only thing left is

rust-lightning/lightning/src/ln/channelmanager.rs

Line 236 in 192618b

short_channel_id:u64,// This should be NonZero<u64> eventually when we bump MSRV

@TheBlueMatt any opinion on whether to do it here, in a separate PR, or as part of #3973 ?

[codecov]

Codecov Report

❌ Patch coverage is 85.10638% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.78%. Comparing base (7f20d47) to head (165a6b3).
⚠️ Report is 27 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning-persister/src/utils.rs	0.00%	5 Missing ⚠️
lightning-liquidity/src/manager.rs	33.33%	2 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4002 +/- ##
==========================================
- Coverage 88.80% 88.78% -0.02% 
==========================================
Files 180 180 Lines 137004 136986 -18 Branches 137004 136986 -18 ==========================================
- Hits 121660 121627 -33 - Misses 12522 12545 +23 + Partials 2822 2814 -8 

Flag	Coverage Δ
fuzzing	21.55% <6.81%> (+<0.01%)	⬆️
tests	88.63% <85.10%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We generally align our MSRV with Debian's stable channel

I don't think this is entirely true :). We generally align our MSRV with rust-bitcoin and the rest of the ecosystem. IIRC rust-bitcoin usually does something like min(2-year-old-rustc, debian stable, rustc that introduces materially useful features). The last time we bumped MSRV (#2681) the rustc we bumped to was a year and a few months old, and the release containing it wasn't until Dec, so rustc 1.63 was almost a year and a half old.

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

[tnull]

We generally align our MSRV with rust-bitcoin and the rest of the ecosystem.

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

1.85 is currently only around 7 months old, and while it contains the rust 2024 edition, its not clear to me what we get that's worth bumping to what, presumably, rust-bitcoin won't do. There's also async closures but async blocks seem to have done us just fine nearly everywhere.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.). Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec<Utxo>, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

IMO it makes a whole lot of sense to upgrade now that we can, if just because it makes our lives easier in many little places, but also because it allows us to expose a more coherent Rust-native API.

[TheBlueMatt]

Do we? #2681 happened 8-9 months before rust-bitcoin bumped in rust-bitcoin/rust-bitcoin#3100. I also recall reading a comment where they indicated they had bumped earlier if they'd known LDK already went ahead / was fine with it. BDK is also looking to bump soon (bitcoindevkit/bdk#2009) and LDK Node will bump shortly: lightningdevkit/ldk-node#606

We don't wait for rust-bitcoin, but we definitely coordinate around specific version of rustc.

What we get is a lot of little things, most importantly general reduction of friction (a period where we don't constantly have to fight with pinned-back dependencies, where all crates have the same MSRV, where cargo fmt and cargo +1.85 fmt have the exact same output, etc. etc.).

This doesn't sound 1.85-specific?

Note we can then also finally switch to 'proper' async traits, which is IMO crucial since we're about to ship the async KVStore interface, which is currently pretty clunky to use (return values of Pin<Box<dyn Future<Output = Result<Vec, ()>> + Send + 'a>> are not exactly ergonomic to type, to say the least).

Sadly even with rustc nightly we wouldn't want to change that. Our async KVStore requires ordering, which is not possible with native rust async methods as they do not run any code at all until polled. I guess in theory we could require "ordering after the first poll" and poll once whenever we persist, but that seems even more brittle than the current version which at least exposes the concept to the implementer.

We also get some language features (GATs, let-else bindings, both stabilized with 1.65, for example), which some devs were looking forward to be able to use finally.

We should also want 1.68 for the pin macro, afair (removes allocations in each loop of the BP).

[TheBlueMatt]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

[tnull]

Per https://pkgs.org/download/rustc (and packages.ubuntu.com) the latest Ubuntu LTS is on rustc 1.75, which given its also the lightning-transaction-sync MSRV seems like an obvious candidate.

While I'd personally still be in favor of 1.85, I now switched this PR to bump to 1.75.0.

[TheBlueMatt]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

[ldk-reviews-bot]

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

[tnull]

error: package `backtrace v0.3.75` cannot be built because it requires rustc 1.82.0 or newer, while the currently active rustc version is 1.75.0
Either upgrade to rustc 1.82.0 or newer, or use
cargo update backtrace@0.3.75 --precise ver
where `ver` is the latest version of `backtrace` supporting rustc 1.75.0

Ah, missed re-adding the deleted pin for this.

Also should we wait for 0.3 for this? I don't see a strong reason to do it now vs in three weeks.

I guess? Also don't see a strong argument to wait? Or do you mean so that the 0.2 release would still have the old MSRV? Then again, 1.75.0 is ancient by now.

[TheBlueMatt]

Or do you mean so that the 0.2 release would still have the old MSRV?

This.

[tnull]

Putting in draft until then.

[tnull]

Rebased on current main and addressed remaining linter warnings.

[tnull]

Now actually whack-a-mole'd all remaining lints.

[ldk-reviews-bot]

🔔 1st Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[TheBlueMatt]

Thanks. We should also move lightning-transaction-sync back into the workspace now that they have the same MSRV. There's probably some Box::pin we can drop now too.