Reduce `Box`ing using `impl Trait` in trait methods post-MSRV-bump #4175

TheBlueMatt · 2025-10-25T18:28:13Z

A handful of cleanups now that we bumped the MSRV and can use impl Trait in trait methods.

ldk-reviews-bot · 2025-10-25T18:28:15Z

👋 Thanks for assigning @tnull 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.

TheBlueMatt · 2025-10-25T18:29:16Z

There's still a few Box::pins in lightning-background-processor that we really should be able to get rid of, but its not entirely trivial so I didn't do it here.

codecov · 2025-10-25T21:05:54Z

Codecov Report

❌ Patch coverage is 72.97297% with 70 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.27%. Comparing base (c1bca16) to head (2069705).
⚠️ Report is 19 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning-background-processor/src/lib.rs	0.00%	20 Missing ⚠️
lightning-block-sync/src/rest.rs	27.77%	13 Missing ⚠️
lightning-block-sync/src/rpc.rs	27.77%	13 Missing ⚠️
lightning-block-sync/src/gossip.rs	0.00%	9 Missing ⚠️
lightning/src/util/test_utils.rs	25.00%	6 Missing ⚠️
lightning-persister/src/fs_store.rs	90.90%	4 Missing ⚠️
lightning-net-tokio/src/lib.rs	60.00%	2 Missing ⚠️
lightning/src/util/persist.rs	86.66%	1 Missing and 1 partial ⚠️
lightning-liquidity/src/lsps2/service.rs	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4175      +/-   ##
==========================================
+ Coverage   88.85%   89.27%   +0.41%     
==========================================
  Files         180      180              
  Lines      137901   137934      +33     
  Branches   137901   137934      +33     
==========================================
+ Hits       122537   123136     +599     
+ Misses      12552    12176     -376     
+ Partials     2812     2622     -190

Flag	Coverage Δ
fuzzing	`33.48% <11.92%> (+12.04%)`	⬆️
tests	`88.67% <72.97%> (-0.03%)`	⬇️

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.

tnull

Given how much this cleans up the new async traits, I do wonder if we want to ship this for 0.2 afterall?

tnull · 2025-10-27T06:57:59Z

Cargo.toml

    "lightning-macros",
    "lightning-dns-resolver",
    "lightning-liquidity",
+    "lightning-transaction-sync",


As BDK is about to bump their MSRV to 1.85, let's not bother with this if we need to move it out again shortly.

I thought they agreed to keep the utility crates to 1.75?

Actually, I thought they were gonna walk it back to 1.75...

ldk-reviews-bot · 2025-10-27T07:06:20Z

👋 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.

TheBlueMatt · 2025-10-27T12:26:33Z

Given how much this cleans up the new async traits, I do wonder if we want to ship this for 0.2 afterall?

Meh, I don't think they clean them up that much, they just remove a Box::pin, and actually they become a bit more annoying to work with (see the few places we have to keep the Box::pin cause of rustc mess). It becomes a bit easier to work with in edition 2024, which luckily a downstream crate could choose to use if they want.

TheBlueMatt · 2025-10-30T14:42:10Z

Rebased to pick up #4180, this should now pass CI (modulo fuzzing, which still confuses me).

TheBlueMatt · 2025-10-30T15:27:29Z

The CI failure is bitcoindevkit/rust-electrum-client#181 which is unrelated to this PR and its just that we're now testing it.

Now that it has the same MSRV as everything else in the workspace, it doesn't need to live on its own.

Now that our MSRV is above 1.68 we can use the `pin!` macro to avoid having to `Box` various futures, avoiding some allocations, especially in `lightning-net-tokio`, which happens in a tight loop.

TheBlueMatt · 2025-10-31T14:39:12Z

Rebased for the restoration of the lazy flag.

TheBlueMatt · 2025-11-03T13:54:45Z

Fixed a bunk rebase.

Now that our MSRV is 1.75, we can return `impl Trait` from trait methods. Here we use this to clean up `KVStore` methods, dropping the `Pin<Box<dyn ...>>` we had to use to have trait methods return a concrete type. Sadly, there's two places where we can't drop a `Box::pin` until we switch to edition 2024.

Now that our MSRV is 1.75, we can return `impl Trait` from trait methods. Here we use this to clean up `lightning-block-sync` trait methods, dropping the `Pin<Box<dyn ...>>` we had to use to have trait methods return a concrete type.

Now that our MSRV is 1.75, we can return `impl Trait` from trait methods. Here we use this to clean up `lightning` crate trait methods, dropping the `Pin<Box<dyn ...>>`/`AsyncResult` we had to use to have trait methods return a concrete type.

Now that we have an MSRV that supports returning `impl Trait` in trait methods, we can use it to avoid the `Box<dyn ...>` we had spewed all over our BOLT 11 invoice serialization.

TheBlueMatt · 2025-11-04T12:41:34Z

Oops, fixed a no-std error in the new BP stuff:

$ git diff-tree -U1 05717d0860 20697053c
diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs
index 874c82f19d..bc0d42ac19 100644
--- a/lightning-background-processor/src/lib.rs
+++ b/lightning-background-processor/src/lib.rs
@@ -43,2 +43,4 @@ use lightning::util::ser::Writeable;

+#[cfg(not(c_bindings))]
+use lightning::io::Error;
 use lightning::ln::channelmanager::AChannelManager;
@@ -53,2 +55,4 @@ use lightning::sign::{
 };
+#[cfg(not(c_bindings))]
+use lightning::util::async_poll::MaybeSend;
 use lightning::util::logger::Logger;
@@ -430,4 +434,3 @@ impl KVStore for DummyKVStore {
 		&self, _: &str, _: &str, _: &str,
-	) -> impl core::future::Future<Output = Result<Vec<u8>, lightning::io::Error>> + Send + 'static
-	{
+	) -> impl core::future::Future<Output = Result<Vec<u8>, Error>> + MaybeSend + 'static {
 		async { unimplemented!() }
@@ -437,3 +440,3 @@ impl KVStore for DummyKVStore {
 		&self, _: &str, _: &str, _: &str, _: Vec<u8>,
-	) -> impl core::future::Future<Output = Result<(), lightning::io::Error>> + Send + 'static {
+	) -> impl core::future::Future<Output = Result<(), Error>> + MaybeSend + 'static {
 		async { unimplemented!() }
@@ -443,3 +446,3 @@ impl KVStore for DummyKVStore {
 		&self, _: &str, _: &str, _: &str, _: bool,
-	) -> impl core::future::Future<Output = Result<(), lightning::io::Error>> + Send + 'static {
+	) -> impl core::future::Future<Output = Result<(), Error>> + MaybeSend + 'static {
 		async { unimplemented!() }
@@ -449,4 +452,3 @@ impl KVStore for DummyKVStore {
 		&self, _: &str, _: &str,
-	) -> impl core::future::Future<Output = Result<Vec<String>, lightning::io::Error>> + Send + 'static
-	{
+	) -> impl core::future::Future<Output = Result<Vec<String>, Error>> + MaybeSend + 'static {
 		async { unimplemented!() }

ldk-reviews-bot requested a review from wpaulino October 25, 2025 18:38

TheBlueMatt force-pushed the 2025-10-msrv-less-box branch 14 times, most recently from e8fa417 to d54050d Compare October 25, 2025 20:55

TheBlueMatt force-pushed the 2025-10-msrv-less-box branch 2 times, most recently from 9d5a3e2 to 724caa8 Compare October 26, 2025 11:50

tnull requested review from tnull and removed request for wpaulino October 27, 2025 06:56

tnull reviewed Oct 27, 2025

View reviewed changes

TheBlueMatt mentioned this pull request Oct 28, 2025

Bump electrsd to 0.36.1, fix CI #4180

Merged

TheBlueMatt force-pushed the 2025-10-msrv-less-box branch from 724caa8 to a467664 Compare October 30, 2025 14:36

TheBlueMatt mentioned this pull request Oct 31, 2025

Fix lightning-transaction-sync docs #4199

Open

TheBlueMatt added 2 commits October 31, 2025 14:31

Move lightning-transaction-sync back into the main workspace

9b81e0d

Now that it has the same MSRV as everything else in the workspace, it doesn't need to live on its own.

Trivially replace Box::pin with pin! in a few places

6440f14

Now that our MSRV is above 1.68 we can use the `pin!` macro to avoid having to `Box` various futures, avoiding some allocations, especially in `lightning-net-tokio`, which happens in a tight loop.

TheBlueMatt force-pushed the 2025-10-msrv-less-box branch from a467664 to 60a21c6 Compare October 31, 2025 14:38

TheBlueMatt force-pushed the 2025-10-msrv-less-box branch from 60a21c6 to 05717d0 Compare November 3, 2025 13:54

TheBlueMatt force-pushed the 2025-10-msrv-less-box branch from 05717d0 to 5161991 Compare November 4, 2025 12:40

TheBlueMatt added 5 commits November 4, 2025 12:41

Drop required Boxing of lightning-block-sync Futures

7fef1d2

Now that our MSRV is 1.75, we can return `impl Trait` from trait methods. Here we use this to clean up `lightning-block-sync` trait methods, dropping the `Pin<Box<dyn ...>>` we had to use to have trait methods return a concrete type.

Drop Boxing of iterators during BOLT 11 invoice serialization

5791b28

Now that we have an MSRV that supports returning `impl Trait` in trait methods, we can use it to avoid the `Box<dyn ...>` we had spewed all over our BOLT 11 invoice serialization.

Fix new warning-errors in 1.75 CI builds

2069705

TheBlueMatt force-pushed the 2025-10-msrv-less-box branch from 5161991 to 2069705 Compare November 4, 2025 12:41

Reduce Boxing using impl Trait in trait methods post-MSRV-bump #4175

Are you sure you want to change the base?

Reduce Boxing using impl Trait in trait methods post-MSRV-bump #4175

Conversation

TheBlueMatt commented Oct 25, 2025

Uh oh!

ldk-reviews-bot commented Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheBlueMatt commented Oct 25, 2025

Uh oh!

codecov bot commented Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

tnull Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Oct 27, 2025

Uh oh!

TheBlueMatt commented Oct 27, 2025

Uh oh!

TheBlueMatt commented Oct 30, 2025

Uh oh!

TheBlueMatt commented Oct 30, 2025

Uh oh!

TheBlueMatt commented Oct 31, 2025

Uh oh!

TheBlueMatt commented Nov 3, 2025

Uh oh!

TheBlueMatt commented Nov 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Reduce `Box`ing using `impl Trait` in trait methods post-MSRV-bump #4175

Reduce `Box`ing using `impl Trait` in trait methods post-MSRV-bump #4175

ldk-reviews-bot commented Oct 25, 2025 •

edited

Loading

codecov bot commented Oct 25, 2025 •

edited

Loading

tnull Oct 27, 2025 •

edited

Loading