Convert file-level rustfmt skip to fn level skips by joostjager · Pull Request #3809 · lightningdevkit/rust-lightning

joostjager · 2025-05-29T13:13:30Z

Extend the approach proposed in #3767 to the full repository. The change set is automatically generated using a script. The script omits skip directives when they would not impact formatting (code already rustfmt-compliant).

I've fixed various problems with the script, and it may be that there are a few edge cases left that should have had a skip directive. There could also be a few redundant ones. I think we can live with that.

Omitted are files that are formatted in in-flight rustfmt PRs: #3783, #3725~~, #3338 and #3337~~

ldk-reviews-bot · 2025-05-29T13:13:33Z

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

joostjager · 2025-05-29T17:35:40Z

As discussed in sync meet, I will try to extend the script to ignore rustfmt changes that only affect the function signature.

joostjager · 2025-05-30T08:43:55Z

Pushed fix up commit to ignore sig changes. Modest reduction in skips, with a few edge cases.

codecov · 2025-06-02T13:22:16Z

Codecov Report

Attention: Patch coverage is 99.01153% with 6 lines in your changes missing coverage. Please review.

Project coverage is 89.91%. Comparing base (e797180) to head (a38db49).

Files with missing lines	Patch %	Lines
lightning/src/chain/channelmonitor.rs	91.17%	6 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #3809 +/- ##
==========================================
+ Coverage 89.87% 89.91% +0.03% 
==========================================
Files 162 162 Lines 130214 130684 +470 Branches 130214 130684 +470 ==========================================
+ Hits 117034 117508 +474 + Misses 10486 10484 -2 + Partials 2694 2692 -2

☔ 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

Why is this in draft? This needs a rebase by now, too.

tnull · 2025-06-03T11:40:57Z

@@ -1,48 +1,47 @@
-#![cfg_attr(rustfmt, rustfmt_skip)]


This was put here deliberately as we opted against running rustfmt on autogenerated files. We need to fix it either way, likely just revert it to include the skip.

Ah, didn't know it was auto-generated. Removed the commit that formats this file.

joostjager · 2025-06-03T16:10:31Z

It's in draft because I want to see #3767 merged first. Don't want to spend time on rebase if we don't go this way.

joostjager · 2025-06-11T10:41:27Z

#3767 seems a positive change. New code being added in PRs (for example async payments) is now automatically formatted and not creating more formatting debt. No bad rebase conflicts that I've heard of either.

Enough to go ahead and re-run the script on the latest main.

tnull

LGTM from what I've seen so far.

Mind including the same changes for the remaining files in chain,ln, and routing, too?

joostjager · 2025-06-11T11:15:11Z

LGTM from what I've seen so far.
Mind including the same changes for the remaining files in chain,ln, and routing, too?

Intentionally left these out because there are fmt prs in flight for those files. Links in pr desc

tnull · 2025-06-11T11:19:40Z

Intentionally left these out because there are fmt prs in flight for those files. Links in pr desc

Ah, right. The remaining ones in routing never went far, so now closed them for now. Please include routing here.

joostjager · 2025-06-11T11:45:42Z

Re-added routing files.

TheBlueMatt · 2025-06-11T23:11:00Z

I admit I'm kinda meh oh doing this for everything. There's a handful of files left that are pretty small and can be easily formatted (at least once trampoline PRs land for chain and the router module PRs get picked back up), and for tests I understood the plan to be "move forward, get there soon" (I know, I know, I plan on finishing #3783 this week), so applying skips to everything in tests (which generally aren't regularly touched) just to format them soon seems kinda weird?

For large files that we want to format iteratively (channelmanager, channel, maybe some of chain though many of those files aren't that large, just very actively being edited), this approach makes a ton of sense, for every file I'm not sure that we want to always format iteratively?

joostjager · 2025-06-12T07:58:47Z

I admit I'm kinda meh oh doing this for everything. There's a handful of files left that are pretty small and can be easily formatted (at least once trampoline PRs land for chain and the router module PRs get picked back up), and for tests I understood the plan to be "move forward, get there soon" (I know, I know, I plan on finishing #3783 this week), so applying skips to everything in tests (which generally aren't regularly touched) just to format them soon seems kinda weird?

I see what you mean. Applying skips now and removing them "soon" feels unnecessary. The way I see it though is that "soon" may be quite a bit further out than we'd like. Trampoline for example may take a while. Adding the skips doesn't hurt, they can easily be removed again if someone feels inclined to format a full file, and they add flexibility for devs to selectively format for whatever reason.

For large files that we want to format iteratively (channelmanager, channel, maybe some of chain though many of those files aren't that large, just very actively being edited), this approach makes a ton of sense, for every file I'm not sure that we want to always format iteratively?

I needed to rebase this PR anyway, and while I did I left out the test files. If you feel it is still too much, maybe you can list the exact files that are definitely good for the incremental approach?

joostjager · 2025-06-12T12:44:34Z

Also not sure at all that doing the tests all in one with mass edits is ideal: #3783 (review)

TheBlueMatt

I think the file list here makes sense. I thought there were substantially more changes to onion_payment.rs for trampoline to come but @valentinewallace tells me they're more in outbound_payment.rs which has more active development and thus higher cost to delaying pushing the format skips down in it.

TheBlueMatt · 2025-06-12T18:08:58Z

 secrets.push([0; 32]);
- secrets.last_mut().unwrap()[0..32].clone_from_slice(&<Vec<u8>>::from_hex("c7518c8ae4660ed02894df8976fa1a3659c1a8b4b5bec0c4b872abeba4cb8964").unwrap());
+ secrets.last_mut().unwrap()[0..32].clone_from_slice(
+ &<Vec<u8>>::from_hex(


Do you want to go ahead and break the hex constants out to give them names?

Done. All gave them the same hex name though...

Formatting improvement.

joostjager · 2025-06-12T19:31:48Z

Removed redundant skips. Didn't do the optional improvements (format short fns), because it is probably easiest to keep this big PR "pure" and do the rest in follow ups.

joostjager · 2025-06-12T19:36:13Z

Didn't squash the redundant skip removal commit, in case I need to re-run the script and re-apply the edge case fixes.

TheBlueMatt

11 files changed, 1984 insertions(+), 668 deletions(-)

Holy diff-size batman!

joostjager · 2025-05-29T13:13:30Z

Extend the approach proposed in #3767 to the full repository. The change set is automatically generated using a script. The script omits skip directives when they would not impact formatting (code already rustfmt-compliant).

I've fixed various problems with the script, and it may be that there are a few edge cases left that should have had a skip directive. There could also be a few redundant ones. I think we can live with that.

Omitted are files that are formatted in in-flight rustfmt PRs: #3783, #3725~~, #3338 and #3337~~

ldk-reviews-bot · 2025-05-29T13:13:33Z

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

joostjager · 2025-05-29T17:35:40Z

As discussed in sync meet, I will try to extend the script to ignore rustfmt changes that only affect the function signature.

joostjager · 2025-05-30T08:43:55Z

Pushed fix up commit to ignore sig changes. Modest reduction in skips, with a few edge cases.

codecov · 2025-06-02T13:22:16Z

Codecov Report

Attention: Patch coverage is 99.01153% with 6 lines in your changes missing coverage. Please review.

Project coverage is 89.91%. Comparing base (e797180) to head (a38db49).

Files with missing lines	Patch %	Lines
lightning/src/chain/channelmonitor.rs	91.17%	6 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #3809 +/- ##
==========================================
+ Coverage 89.87% 89.91% +0.03% 
==========================================
Files 162 162 Lines 130214 130684 +470 Branches 130214 130684 +470 ==========================================
+ Hits 117034 117508 +474 + Misses 10486 10484 -2 + Partials 2694 2692 -2

☔ 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

Why is this in draft? This needs a rebase by now, too.

tnull · 2025-06-03T11:40:57Z

@@ -1,48 +1,47 @@
-#![cfg_attr(rustfmt, rustfmt_skip)]


This was put here deliberately as we opted against running rustfmt on autogenerated files. We need to fix it either way, likely just revert it to include the skip.

Ah, didn't know it was auto-generated. Removed the commit that formats this file.

joostjager · 2025-06-03T16:10:31Z

It's in draft because I want to see #3767 merged first. Don't want to spend time on rebase if we don't go this way.

joostjager · 2025-06-11T10:41:27Z

#3767 seems a positive change. New code being added in PRs (for example async payments) is now automatically formatted and not creating more formatting debt. No bad rebase conflicts that I've heard of either.

Enough to go ahead and re-run the script on the latest main.

tnull

LGTM from what I've seen so far.

Mind including the same changes for the remaining files in chain,ln, and routing, too?

joostjager · 2025-06-11T11:15:11Z

LGTM from what I've seen so far.
Mind including the same changes for the remaining files in chain,ln, and routing, too?

Intentionally left these out because there are fmt prs in flight for those files. Links in pr desc

tnull · 2025-06-11T11:19:40Z

Intentionally left these out because there are fmt prs in flight for those files. Links in pr desc

Ah, right. The remaining ones in routing never went far, so now closed them for now. Please include routing here.

joostjager · 2025-06-11T11:45:42Z

Re-added routing files.

TheBlueMatt · 2025-06-11T23:11:00Z

I admit I'm kinda meh oh doing this for everything. There's a handful of files left that are pretty small and can be easily formatted (at least once trampoline PRs land for chain and the router module PRs get picked back up), and for tests I understood the plan to be "move forward, get there soon" (I know, I know, I plan on finishing #3783 this week), so applying skips to everything in tests (which generally aren't regularly touched) just to format them soon seems kinda weird?

For large files that we want to format iteratively (channelmanager, channel, maybe some of chain though many of those files aren't that large, just very actively being edited), this approach makes a ton of sense, for every file I'm not sure that we want to always format iteratively?

joostjager · 2025-06-12T07:58:47Z

I admit I'm kinda meh oh doing this for everything. There's a handful of files left that are pretty small and can be easily formatted (at least once trampoline PRs land for chain and the router module PRs get picked back up), and for tests I understood the plan to be "move forward, get there soon" (I know, I know, I plan on finishing #3783 this week), so applying skips to everything in tests (which generally aren't regularly touched) just to format them soon seems kinda weird?

I see what you mean. Applying skips now and removing them "soon" feels unnecessary. The way I see it though is that "soon" may be quite a bit further out than we'd like. Trampoline for example may take a while. Adding the skips doesn't hurt, they can easily be removed again if someone feels inclined to format a full file, and they add flexibility for devs to selectively format for whatever reason.

For large files that we want to format iteratively (channelmanager, channel, maybe some of chain though many of those files aren't that large, just very actively being edited), this approach makes a ton of sense, for every file I'm not sure that we want to always format iteratively?

I needed to rebase this PR anyway, and while I did I left out the test files. If you feel it is still too much, maybe you can list the exact files that are definitely good for the incremental approach?

joostjager · 2025-06-12T12:44:34Z

Also not sure at all that doing the tests all in one with mass edits is ideal: #3783 (review)

TheBlueMatt

I think the file list here makes sense. I thought there were substantially more changes to onion_payment.rs for trampoline to come but @valentinewallace tells me they're more in outbound_payment.rs which has more active development and thus higher cost to delaying pushing the format skips down in it.

TheBlueMatt · 2025-06-12T18:08:58Z

 secrets.push([0; 32]);
- secrets.last_mut().unwrap()[0..32].clone_from_slice(&<Vec<u8>>::from_hex("c7518c8ae4660ed02894df8976fa1a3659c1a8b4b5bec0c4b872abeba4cb8964").unwrap());
+ secrets.last_mut().unwrap()[0..32].clone_from_slice(
+ &<Vec<u8>>::from_hex(


Do you want to go ahead and break the hex constants out to give them names?

Done. All gave them the same hex name though...

Formatting improvement.

joostjager · 2025-06-12T19:31:48Z

Removed redundant skips. Didn't do the optional improvements (format short fns), because it is probably easiest to keep this big PR "pure" and do the rest in follow ups.

joostjager · 2025-06-12T19:36:13Z

Didn't squash the redundant skip removal commit, in case I need to re-run the script and re-apply the edge case fixes.

TheBlueMatt

11 files changed, 1984 insertions(+), 668 deletions(-)

Holy diff-size batman!

joostjager · 2025-05-29T13:13:30Z

Extend the approach proposed in #3767 to the full repository. The change set is automatically generated using a script. The script omits skip directives when they would not impact formatting (code already rustfmt-compliant).

I've fixed various problems with the script, and it may be that there are a few edge cases left that should have had a skip directive. There could also be a few redundant ones. I think we can live with that.

Omitted are files that are formatted in in-flight rustfmt PRs: #3783, #3725~~, #3338 and #3337~~

ldk-reviews-bot · 2025-05-29T13:13:33Z

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

joostjager · 2025-05-29T17:35:40Z

As discussed in sync meet, I will try to extend the script to ignore rustfmt changes that only affect the function signature.

joostjager · 2025-05-30T08:43:55Z

Pushed fix up commit to ignore sig changes. Modest reduction in skips, with a few edge cases.

codecov · 2025-06-02T13:22:16Z

Codecov Report

Attention: Patch coverage is 99.01153% with 6 lines in your changes missing coverage. Please review.

Project coverage is 89.91%. Comparing base (e797180) to head (a38db49).

Files with missing lines	Patch %	Lines
lightning/src/chain/channelmonitor.rs	91.17%	6 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #3809 +/- ##
==========================================
+ Coverage 89.87% 89.91% +0.03% 
==========================================
Files 162 162 Lines 130214 130684 +470 Branches 130214 130684 +470 ==========================================
+ Hits 117034 117508 +474 + Misses 10486 10484 -2 + Partials 2694 2692 -2

☔ 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

Why is this in draft? This needs a rebase by now, too.

tnull · 2025-06-03T11:40:57Z

@@ -1,48 +1,47 @@
-#![cfg_attr(rustfmt, rustfmt_skip)]


This was put here deliberately as we opted against running rustfmt on autogenerated files. We need to fix it either way, likely just revert it to include the skip.

Ah, didn't know it was auto-generated. Removed the commit that formats this file.

joostjager · 2025-06-03T16:10:31Z

It's in draft because I want to see #3767 merged first. Don't want to spend time on rebase if we don't go this way.

joostjager · 2025-06-11T10:41:27Z

#3767 seems a positive change. New code being added in PRs (for example async payments) is now automatically formatted and not creating more formatting debt. No bad rebase conflicts that I've heard of either.

Enough to go ahead and re-run the script on the latest main.

tnull

LGTM from what I've seen so far.

Mind including the same changes for the remaining files in chain,ln, and routing, too?

joostjager · 2025-06-11T11:15:11Z

LGTM from what I've seen so far.
Mind including the same changes for the remaining files in chain,ln, and routing, too?

Intentionally left these out because there are fmt prs in flight for those files. Links in pr desc

tnull · 2025-06-11T11:19:40Z

Intentionally left these out because there are fmt prs in flight for those files. Links in pr desc

Ah, right. The remaining ones in routing never went far, so now closed them for now. Please include routing here.

joostjager · 2025-06-11T11:45:42Z

Re-added routing files.

TheBlueMatt · 2025-06-11T23:11:00Z

I admit I'm kinda meh oh doing this for everything. There's a handful of files left that are pretty small and can be easily formatted (at least once trampoline PRs land for chain and the router module PRs get picked back up), and for tests I understood the plan to be "move forward, get there soon" (I know, I know, I plan on finishing #3783 this week), so applying skips to everything in tests (which generally aren't regularly touched) just to format them soon seems kinda weird?

For large files that we want to format iteratively (channelmanager, channel, maybe some of chain though many of those files aren't that large, just very actively being edited), this approach makes a ton of sense, for every file I'm not sure that we want to always format iteratively?

joostjager · 2025-06-12T07:58:47Z

I admit I'm kinda meh oh doing this for everything. There's a handful of files left that are pretty small and can be easily formatted (at least once trampoline PRs land for chain and the router module PRs get picked back up), and for tests I understood the plan to be "move forward, get there soon" (I know, I know, I plan on finishing #3783 this week), so applying skips to everything in tests (which generally aren't regularly touched) just to format them soon seems kinda weird?

I see what you mean. Applying skips now and removing them "soon" feels unnecessary. The way I see it though is that "soon" may be quite a bit further out than we'd like. Trampoline for example may take a while. Adding the skips doesn't hurt, they can easily be removed again if someone feels inclined to format a full file, and they add flexibility for devs to selectively format for whatever reason.

For large files that we want to format iteratively (channelmanager, channel, maybe some of chain though many of those files aren't that large, just very actively being edited), this approach makes a ton of sense, for every file I'm not sure that we want to always format iteratively?

I needed to rebase this PR anyway, and while I did I left out the test files. If you feel it is still too much, maybe you can list the exact files that are definitely good for the incremental approach?

joostjager · 2025-06-12T12:44:34Z

Also not sure at all that doing the tests all in one with mass edits is ideal: #3783 (review)

TheBlueMatt

I think the file list here makes sense. I thought there were substantially more changes to onion_payment.rs for trampoline to come but @valentinewallace tells me they're more in outbound_payment.rs which has more active development and thus higher cost to delaying pushing the format skips down in it.

TheBlueMatt · 2025-06-12T18:08:58Z

 secrets.push([0; 32]);
- secrets.last_mut().unwrap()[0..32].clone_from_slice(&<Vec<u8>>::from_hex("c7518c8ae4660ed02894df8976fa1a3659c1a8b4b5bec0c4b872abeba4cb8964").unwrap());
+ secrets.last_mut().unwrap()[0..32].clone_from_slice(
+ &<Vec<u8>>::from_hex(


Do you want to go ahead and break the hex constants out to give them names?

Done. All gave them the same hex name though...

Formatting improvement.

joostjager · 2025-06-12T19:31:48Z

Removed redundant skips. Didn't do the optional improvements (format short fns), because it is probably easiest to keep this big PR "pure" and do the rest in follow ups.

joostjager · 2025-06-12T19:36:13Z

Didn't squash the redundant skip removal commit, in case I need to re-run the script and re-apply the edge case fixes.

TheBlueMatt

11 files changed, 1984 insertions(+), 668 deletions(-)

Holy diff-size batman!

joostjager · 2025-05-29T13:13:30Z

Extend the approach proposed in #3767 to the full repository. The change set is automatically generated using a script. The script omits skip directives when they would not impact formatting (code already rustfmt-compliant).

I've fixed various problems with the script, and it may be that there are a few edge cases left that should have had a skip directive. There could also be a few redundant ones. I think we can live with that.

Omitted are files that are formatted in in-flight rustfmt PRs: #3783, #3725~~, #3338 and #3337~~

ldk-reviews-bot · 2025-05-29T13:13:33Z

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

joostjager · 2025-05-29T17:35:40Z

As discussed in sync meet, I will try to extend the script to ignore rustfmt changes that only affect the function signature.

joostjager · 2025-05-30T08:43:55Z

Pushed fix up commit to ignore sig changes. Modest reduction in skips, with a few edge cases.

codecov · 2025-06-02T13:22:16Z

Codecov Report

Attention: Patch coverage is 99.01153% with 6 lines in your changes missing coverage. Please review.

Project coverage is 89.91%. Comparing base (e797180) to head (a38db49).

Files with missing lines	Patch %	Lines
lightning/src/chain/channelmonitor.rs	91.17%	6 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #3809 +/- ##
==========================================
+ Coverage 89.87% 89.91% +0.03% 
==========================================
Files 162 162 Lines 130214 130684 +470 Branches 130214 130684 +470 ==========================================
+ Hits 117034 117508 +474 + Misses 10486 10484 -2 + Partials 2694 2692 -2

☔ 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

Why is this in draft? This needs a rebase by now, too.

tnull · 2025-06-03T11:40:57Z

@@ -1,48 +1,47 @@
-#![cfg_attr(rustfmt, rustfmt_skip)]


This was put here deliberately as we opted against running rustfmt on autogenerated files. We need to fix it either way, likely just revert it to include the skip.

Ah, didn't know it was auto-generated. Removed the commit that formats this file.

joostjager · 2025-06-03T16:10:31Z

It's in draft because I want to see #3767 merged first. Don't want to spend time on rebase if we don't go this way.

joostjager · 2025-06-11T10:41:27Z

#3767 seems a positive change. New code being added in PRs (for example async payments) is now automatically formatted and not creating more formatting debt. No bad rebase conflicts that I've heard of either.

Enough to go ahead and re-run the script on the latest main.

tnull

LGTM from what I've seen so far.

Mind including the same changes for the remaining files in chain,ln, and routing, too?

joostjager · 2025-06-11T11:15:11Z

LGTM from what I've seen so far.
Mind including the same changes for the remaining files in chain,ln, and routing, too?

Intentionally left these out because there are fmt prs in flight for those files. Links in pr desc

tnull · 2025-06-11T11:19:40Z

Intentionally left these out because there are fmt prs in flight for those files. Links in pr desc

Ah, right. The remaining ones in routing never went far, so now closed them for now. Please include routing here.

joostjager · 2025-06-11T11:45:42Z

Re-added routing files.

TheBlueMatt · 2025-06-11T23:11:00Z

I admit I'm kinda meh oh doing this for everything. There's a handful of files left that are pretty small and can be easily formatted (at least once trampoline PRs land for chain and the router module PRs get picked back up), and for tests I understood the plan to be "move forward, get there soon" (I know, I know, I plan on finishing #3783 this week), so applying skips to everything in tests (which generally aren't regularly touched) just to format them soon seems kinda weird?

For large files that we want to format iteratively (channelmanager, channel, maybe some of chain though many of those files aren't that large, just very actively being edited), this approach makes a ton of sense, for every file I'm not sure that we want to always format iteratively?

joostjager · 2025-06-12T07:58:47Z

I admit I'm kinda meh oh doing this for everything. There's a handful of files left that are pretty small and can be easily formatted (at least once trampoline PRs land for chain and the router module PRs get picked back up), and for tests I understood the plan to be "move forward, get there soon" (I know, I know, I plan on finishing #3783 this week), so applying skips to everything in tests (which generally aren't regularly touched) just to format them soon seems kinda weird?

I see what you mean. Applying skips now and removing them "soon" feels unnecessary. The way I see it though is that "soon" may be quite a bit further out than we'd like. Trampoline for example may take a while. Adding the skips doesn't hurt, they can easily be removed again if someone feels inclined to format a full file, and they add flexibility for devs to selectively format for whatever reason.

For large files that we want to format iteratively (channelmanager, channel, maybe some of chain though many of those files aren't that large, just very actively being edited), this approach makes a ton of sense, for every file I'm not sure that we want to always format iteratively?

I needed to rebase this PR anyway, and while I did I left out the test files. If you feel it is still too much, maybe you can list the exact files that are definitely good for the incremental approach?

joostjager · 2025-06-12T12:44:34Z

Also not sure at all that doing the tests all in one with mass edits is ideal: #3783 (review)

TheBlueMatt

I think the file list here makes sense. I thought there were substantially more changes to onion_payment.rs for trampoline to come but @valentinewallace tells me they're more in outbound_payment.rs which has more active development and thus higher cost to delaying pushing the format skips down in it.

TheBlueMatt · 2025-06-12T18:08:58Z

 secrets.push([0; 32]);
- secrets.last_mut().unwrap()[0..32].clone_from_slice(&<Vec<u8>>::from_hex("c7518c8ae4660ed02894df8976fa1a3659c1a8b4b5bec0c4b872abeba4cb8964").unwrap());
+ secrets.last_mut().unwrap()[0..32].clone_from_slice(
+ &<Vec<u8>>::from_hex(


Do you want to go ahead and break the hex constants out to give them names?

Done. All gave them the same hex name though...

Formatting improvement.

joostjager · 2025-06-12T19:31:48Z

Removed redundant skips. Didn't do the optional improvements (format short fns), because it is probably easiest to keep this big PR "pure" and do the rest in follow ups.

joostjager · 2025-06-12T19:36:13Z

Didn't squash the redundant skip removal commit, in case I need to re-run the script and re-apply the edge case fixes.

TheBlueMatt

11 files changed, 1984 insertions(+), 668 deletions(-)

Holy diff-size batman!

joostjager · 2025-05-29T13:13:30Z

Extend the approach proposed in #3767 to the full repository. The change set is automatically generated using a script. The script omits skip directives when they would not impact formatting (code already rustfmt-compliant).

I've fixed various problems with the script, and it may be that there are a few edge cases left that should have had a skip directive. There could also be a few redundant ones. I think we can live with that.

Omitted are files that are formatted in in-flight rustfmt PRs: #3783, #3725~~, #3338 and #3337~~

ldk-reviews-bot · 2025-05-29T13:13:33Z

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

joostjager · 2025-05-29T17:35:40Z

As discussed in sync meet, I will try to extend the script to ignore rustfmt changes that only affect the function signature.

joostjager · 2025-05-30T08:43:55Z

Pushed fix up commit to ignore sig changes. Modest reduction in skips, with a few edge cases.

codecov · 2025-06-02T13:22:16Z

Codecov Report

Attention: Patch coverage is 99.01153% with 6 lines in your changes missing coverage. Please review.

Project coverage is 89.91%. Comparing base (e797180) to head (a38db49).

Files with missing lines	Patch %	Lines
lightning/src/chain/channelmonitor.rs	91.17%	6 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #3809 +/- ##
==========================================
+ Coverage 89.87% 89.91% +0.03% 
==========================================
Files 162 162 Lines 130214 130684 +470 Branches 130214 130684 +470 ==========================================
+ Hits 117034 117508 +474 + Misses 10486 10484 -2 + Partials 2694 2692 -2

☔ 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

Why is this in draft? This needs a rebase by now, too.

tnull · 2025-06-03T11:40:57Z

@@ -1,48 +1,47 @@
-#![cfg_attr(rustfmt, rustfmt_skip)]


This was put here deliberately as we opted against running rustfmt on autogenerated files. We need to fix it either way, likely just revert it to include the skip.

Ah, didn't know it was auto-generated. Removed the commit that formats this file.

joostjager · 2025-06-03T16:10:31Z

It's in draft because I want to see #3767 merged first. Don't want to spend time on rebase if we don't go this way.

joostjager · 2025-06-11T10:41:27Z

#3767 seems a positive change. New code being added in PRs (for example async payments) is now automatically formatted and not creating more formatting debt. No bad rebase conflicts that I've heard of either.

Enough to go ahead and re-run the script on the latest main.

tnull

LGTM from what I've seen so far.

Mind including the same changes for the remaining files in chain,ln, and routing, too?

joostjager · 2025-06-11T11:15:11Z

LGTM from what I've seen so far.
Mind including the same changes for the remaining files in chain,ln, and routing, too?

Intentionally left these out because there are fmt prs in flight for those files. Links in pr desc

tnull · 2025-06-11T11:19:40Z

Intentionally left these out because there are fmt prs in flight for those files. Links in pr desc

Ah, right. The remaining ones in routing never went far, so now closed them for now. Please include routing here.

joostjager · 2025-06-11T11:45:42Z

Re-added routing files.

TheBlueMatt · 2025-06-11T23:11:00Z

I admit I'm kinda meh oh doing this for everything. There's a handful of files left that are pretty small and can be easily formatted (at least once trampoline PRs land for chain and the router module PRs get picked back up), and for tests I understood the plan to be "move forward, get there soon" (I know, I know, I plan on finishing #3783 this week), so applying skips to everything in tests (which generally aren't regularly touched) just to format them soon seems kinda weird?

For large files that we want to format iteratively (channelmanager, channel, maybe some of chain though many of those files aren't that large, just very actively being edited), this approach makes a ton of sense, for every file I'm not sure that we want to always format iteratively?

joostjager · 2025-06-12T07:58:47Z

I admit I'm kinda meh oh doing this for everything. There's a handful of files left that are pretty small and can be easily formatted (at least once trampoline PRs land for chain and the router module PRs get picked back up), and for tests I understood the plan to be "move forward, get there soon" (I know, I know, I plan on finishing #3783 this week), so applying skips to everything in tests (which generally aren't regularly touched) just to format them soon seems kinda weird?

I see what you mean. Applying skips now and removing them "soon" feels unnecessary. The way I see it though is that "soon" may be quite a bit further out than we'd like. Trampoline for example may take a while. Adding the skips doesn't hurt, they can easily be removed again if someone feels inclined to format a full file, and they add flexibility for devs to selectively format for whatever reason.

For large files that we want to format iteratively (channelmanager, channel, maybe some of chain though many of those files aren't that large, just very actively being edited), this approach makes a ton of sense, for every file I'm not sure that we want to always format iteratively?

I needed to rebase this PR anyway, and while I did I left out the test files. If you feel it is still too much, maybe you can list the exact files that are definitely good for the incremental approach?

joostjager · 2025-06-12T12:44:34Z

Also not sure at all that doing the tests all in one with mass edits is ideal: #3783 (review)

TheBlueMatt

I think the file list here makes sense. I thought there were substantially more changes to onion_payment.rs for trampoline to come but @valentinewallace tells me they're more in outbound_payment.rs which has more active development and thus higher cost to delaying pushing the format skips down in it.

TheBlueMatt · 2025-06-12T18:08:58Z

 secrets.push([0; 32]);
- secrets.last_mut().unwrap()[0..32].clone_from_slice(&<Vec<u8>>::from_hex("c7518c8ae4660ed02894df8976fa1a3659c1a8b4b5bec0c4b872abeba4cb8964").unwrap());
+ secrets.last_mut().unwrap()[0..32].clone_from_slice(
+ &<Vec<u8>>::from_hex(


Do you want to go ahead and break the hex constants out to give them names?

Done. All gave them the same hex name though...

Formatting improvement.

joostjager · 2025-06-12T19:31:48Z

Removed redundant skips. Didn't do the optional improvements (format short fns), because it is probably easiest to keep this big PR "pure" and do the rest in follow ups.

joostjager · 2025-06-12T19:36:13Z

Didn't squash the redundant skip removal commit, in case I need to re-run the script and re-apply the edge case fixes.

TheBlueMatt

11 files changed, 1984 insertions(+), 668 deletions(-)

Holy diff-size batman!

joostjager · 2025-05-29T13:13:30Z

Extend the approach proposed in #3767 to the full repository. The change set is automatically generated using a script. The script omits skip directives when they would not impact formatting (code already rustfmt-compliant).

I've fixed various problems with the script, and it may be that there are a few edge cases left that should have had a skip directive. There could also be a few redundant ones. I think we can live with that.

Omitted are files that are formatted in in-flight rustfmt PRs: #3783, #3725~~, #3338 and #3337~~

ldk-reviews-bot · 2025-05-29T13:13:33Z

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

joostjager · 2025-05-29T17:35:40Z

As discussed in sync meet, I will try to extend the script to ignore rustfmt changes that only affect the function signature.

joostjager · 2025-05-30T08:43:55Z

Pushed fix up commit to ignore sig changes. Modest reduction in skips, with a few edge cases.

codecov · 2025-06-02T13:22:16Z

Codecov Report

Attention: Patch coverage is 99.01153% with 6 lines in your changes missing coverage. Please review.

Project coverage is 89.91%. Comparing base (e797180) to head (a38db49).

Files with missing lines	Patch %	Lines
lightning/src/chain/channelmonitor.rs	91.17%	6 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #3809 +/- ##
==========================================
+ Coverage 89.87% 89.91% +0.03% 
==========================================
Files 162 162 Lines 130214 130684 +470 Branches 130214 130684 +470 ==========================================
+ Hits 117034 117508 +474 + Misses 10486 10484 -2 + Partials 2694 2692 -2

☔ 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

Why is this in draft? This needs a rebase by now, too.

tnull · 2025-06-03T11:40:57Z

@@ -1,48 +1,47 @@
-#![cfg_attr(rustfmt, rustfmt_skip)]


This was put here deliberately as we opted against running rustfmt on autogenerated files. We need to fix it either way, likely just revert it to include the skip.

Ah, didn't know it was auto-generated. Removed the commit that formats this file.

joostjager · 2025-06-03T16:10:31Z

It's in draft because I want to see #3767 merged first. Don't want to spend time on rebase if we don't go this way.

joostjager · 2025-06-11T10:41:27Z

#3767 seems a positive change. New code being added in PRs (for example async payments) is now automatically formatted and not creating more formatting debt. No bad rebase conflicts that I've heard of either.

Enough to go ahead and re-run the script on the latest main.

tnull

LGTM from what I've seen so far.

Mind including the same changes for the remaining files in chain,ln, and routing, too?

joostjager · 2025-06-11T11:15:11Z

LGTM from what I've seen so far.
Mind including the same changes for the remaining files in chain,ln, and routing, too?

Intentionally left these out because there are fmt prs in flight for those files. Links in pr desc

tnull · 2025-06-11T11:19:40Z

Intentionally left these out because there are fmt prs in flight for those files. Links in pr desc

Ah, right. The remaining ones in routing never went far, so now closed them for now. Please include routing here.

joostjager · 2025-06-11T11:45:42Z

Re-added routing files.

TheBlueMatt · 2025-06-11T23:11:00Z

I admit I'm kinda meh oh doing this for everything. There's a handful of files left that are pretty small and can be easily formatted (at least once trampoline PRs land for chain and the router module PRs get picked back up), and for tests I understood the plan to be "move forward, get there soon" (I know, I know, I plan on finishing #3783 this week), so applying skips to everything in tests (which generally aren't regularly touched) just to format them soon seems kinda weird?

For large files that we want to format iteratively (channelmanager, channel, maybe some of chain though many of those files aren't that large, just very actively being edited), this approach makes a ton of sense, for every file I'm not sure that we want to always format iteratively?

joostjager · 2025-06-12T07:58:47Z

I admit I'm kinda meh oh doing this for everything. There's a handful of files left that are pretty small and can be easily formatted (at least once trampoline PRs land for chain and the router module PRs get picked back up), and for tests I understood the plan to be "move forward, get there soon" (I know, I know, I plan on finishing #3783 this week), so applying skips to everything in tests (which generally aren't regularly touched) just to format them soon seems kinda weird?

I see what you mean. Applying skips now and removing them "soon" feels unnecessary. The way I see it though is that "soon" may be quite a bit further out than we'd like. Trampoline for example may take a while. Adding the skips doesn't hurt, they can easily be removed again if someone feels inclined to format a full file, and they add flexibility for devs to selectively format for whatever reason.

For large files that we want to format iteratively (channelmanager, channel, maybe some of chain though many of those files aren't that large, just very actively being edited), this approach makes a ton of sense, for every file I'm not sure that we want to always format iteratively?

I needed to rebase this PR anyway, and while I did I left out the test files. If you feel it is still too much, maybe you can list the exact files that are definitely good for the incremental approach?

joostjager · 2025-06-12T12:44:34Z

Also not sure at all that doing the tests all in one with mass edits is ideal: #3783 (review)

TheBlueMatt

I think the file list here makes sense. I thought there were substantially more changes to onion_payment.rs for trampoline to come but @valentinewallace tells me they're more in outbound_payment.rs which has more active development and thus higher cost to delaying pushing the format skips down in it.

TheBlueMatt · 2025-06-12T18:08:58Z

 secrets.push([0; 32]);
- secrets.last_mut().unwrap()[0..32].clone_from_slice(&<Vec<u8>>::from_hex("c7518c8ae4660ed02894df8976fa1a3659c1a8b4b5bec0c4b872abeba4cb8964").unwrap());
+ secrets.last_mut().unwrap()[0..32].clone_from_slice(
+ &<Vec<u8>>::from_hex(


Do you want to go ahead and break the hex constants out to give them names?

Done. All gave them the same hex name though...

Formatting improvement.

joostjager · 2025-06-12T19:31:48Z

Removed redundant skips. Didn't do the optional improvements (format short fns), because it is probably easiest to keep this big PR "pure" and do the rest in follow ups.

joostjager · 2025-06-12T19:36:13Z

Didn't squash the redundant skip removal commit, in case I need to re-run the script and re-apply the edge case fixes.

TheBlueMatt

11 files changed, 1984 insertions(+), 668 deletions(-)

Holy diff-size batman!

joostjager · 2025-05-29T13:13:30Z

Extend the approach proposed in #3767 to the full repository. The change set is automatically generated using a script. The script omits skip directives when they would not impact formatting (code already rustfmt-compliant).

I've fixed various problems with the script, and it may be that there are a few edge cases left that should have had a skip directive. There could also be a few redundant ones. I think we can live with that.

Omitted are files that are formatted in in-flight rustfmt PRs: #3783, #3725~~, #3338 and #3337~~

ldk-reviews-bot · 2025-05-29T13:13:33Z

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

joostjager · 2025-05-29T17:35:40Z

As discussed in sync meet, I will try to extend the script to ignore rustfmt changes that only affect the function signature.

joostjager · 2025-05-30T08:43:55Z

Pushed fix up commit to ignore sig changes. Modest reduction in skips, with a few edge cases.

codecov · 2025-06-02T13:22:16Z

Codecov Report

Attention: Patch coverage is 99.01153% with 6 lines in your changes missing coverage. Please review.

Project coverage is 89.91%. Comparing base (e797180) to head (a38db49).

Files with missing lines	Patch %	Lines
lightning/src/chain/channelmonitor.rs	91.17%	6 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #3809 +/- ##
==========================================
+ Coverage 89.87% 89.91% +0.03% 
==========================================
Files 162 162 Lines 130214 130684 +470 Branches 130214 130684 +470 ==========================================
+ Hits 117034 117508 +474 + Misses 10486 10484 -2 + Partials 2694 2692 -2

☔ 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

Why is this in draft? This needs a rebase by now, too.

tnull · 2025-06-03T11:40:57Z

@@ -1,48 +1,47 @@
-#![cfg_attr(rustfmt, rustfmt_skip)]


This was put here deliberately as we opted against running rustfmt on autogenerated files. We need to fix it either way, likely just revert it to include the skip.

Ah, didn't know it was auto-generated. Removed the commit that formats this file.

joostjager · 2025-06-03T16:10:31Z

It's in draft because I want to see #3767 merged first. Don't want to spend time on rebase if we don't go this way.

joostjager · 2025-06-11T10:41:27Z

#3767 seems a positive change. New code being added in PRs (for example async payments) is now automatically formatted and not creating more formatting debt. No bad rebase conflicts that I've heard of either.

Enough to go ahead and re-run the script on the latest main.

tnull

LGTM from what I've seen so far.

Mind including the same changes for the remaining files in chain,ln, and routing, too?

joostjager · 2025-06-11T11:15:11Z

LGTM from what I've seen so far.
Mind including the same changes for the remaining files in chain,ln, and routing, too?

Intentionally left these out because there are fmt prs in flight for those files. Links in pr desc

tnull · 2025-06-11T11:19:40Z

Intentionally left these out because there are fmt prs in flight for those files. Links in pr desc

Ah, right. The remaining ones in routing never went far, so now closed them for now. Please include routing here.

joostjager · 2025-06-11T11:45:42Z

Re-added routing files.

TheBlueMatt · 2025-06-11T23:11:00Z

I admit I'm kinda meh oh doing this for everything. There's a handful of files left that are pretty small and can be easily formatted (at least once trampoline PRs land for chain and the router module PRs get picked back up), and for tests I understood the plan to be "move forward, get there soon" (I know, I know, I plan on finishing #3783 this week), so applying skips to everything in tests (which generally aren't regularly touched) just to format them soon seems kinda weird?

For large files that we want to format iteratively (channelmanager, channel, maybe some of chain though many of those files aren't that large, just very actively being edited), this approach makes a ton of sense, for every file I'm not sure that we want to always format iteratively?

joostjager · 2025-06-12T07:58:47Z

I admit I'm kinda meh oh doing this for everything. There's a handful of files left that are pretty small and can be easily formatted (at least once trampoline PRs land for chain and the router module PRs get picked back up), and for tests I understood the plan to be "move forward, get there soon" (I know, I know, I plan on finishing #3783 this week), so applying skips to everything in tests (which generally aren't regularly touched) just to format them soon seems kinda weird?

I see what you mean. Applying skips now and removing them "soon" feels unnecessary. The way I see it though is that "soon" may be quite a bit further out than we'd like. Trampoline for example may take a while. Adding the skips doesn't hurt, they can easily be removed again if someone feels inclined to format a full file, and they add flexibility for devs to selectively format for whatever reason.

For large files that we want to format iteratively (channelmanager, channel, maybe some of chain though many of those files aren't that large, just very actively being edited), this approach makes a ton of sense, for every file I'm not sure that we want to always format iteratively?

I needed to rebase this PR anyway, and while I did I left out the test files. If you feel it is still too much, maybe you can list the exact files that are definitely good for the incremental approach?

joostjager · 2025-06-12T12:44:34Z

Also not sure at all that doing the tests all in one with mass edits is ideal: #3783 (review)

TheBlueMatt

I think the file list here makes sense. I thought there were substantially more changes to onion_payment.rs for trampoline to come but @valentinewallace tells me they're more in outbound_payment.rs which has more active development and thus higher cost to delaying pushing the format skips down in it.

TheBlueMatt · 2025-06-12T18:08:58Z

 secrets.push([0; 32]);
- secrets.last_mut().unwrap()[0..32].clone_from_slice(&<Vec<u8>>::from_hex("c7518c8ae4660ed02894df8976fa1a3659c1a8b4b5bec0c4b872abeba4cb8964").unwrap());
+ secrets.last_mut().unwrap()[0..32].clone_from_slice(
+ &<Vec<u8>>::from_hex(


Do you want to go ahead and break the hex constants out to give them names?

Done. All gave them the same hex name though...

Formatting improvement.

joostjager · 2025-06-12T19:31:48Z

Removed redundant skips. Didn't do the optional improvements (format short fns), because it is probably easiest to keep this big PR "pure" and do the rest in follow ups.

joostjager · 2025-06-12T19:36:13Z

Didn't squash the redundant skip removal commit, in case I need to re-run the script and re-apply the edge case fixes.

TheBlueMatt

11 files changed, 1984 insertions(+), 668 deletions(-)

Holy diff-size batman!

joostjager force-pushed the rustfmt-skip-all branch from 9874e55 to 69a9581Compare May 29, 2025 13:40

joostjager mentioned this pull request May 29, 2025
Run rustfmt on channelmanager#3767
Merged

joostjager force-pushed the rustfmt-skip-all branch 2 times, most recently from a88062e to 16025f2Compare May 29, 2025 14:26

joostjager force-pushed the rustfmt-skip-all branch from 16025f2 to 9ff8996Compare May 30, 2025 08:42

joostjager force-pushed the rustfmt-skip-all branch from 9ff8996 to cf4e480Compare June 2, 2025 13:00

tnull reviewed Jun 3, 2025
View reviewed changes

joostjager added the rustfmt label Jun 11, 2025

joostjager force-pushed the rustfmt-skip-all branch from cf4e480 to 06eba3bCompare June 11, 2025 10:39

joostjager force-pushed the rustfmt-skip-all branch from 06eba3b to a38db49Compare June 11, 2025 10:48

tnull reviewed Jun 11, 2025
View reviewed changes

joostjager force-pushed the rustfmt-skip-all branch from a38db49 to f097207Compare June 11, 2025 11:45

joostjager force-pushed the rustfmt-skip-all branch from f097207 to 1abfab8Compare June 11, 2025 12:30

joostjager marked this pull request as ready for review June 11, 2025 12:53

ldk-reviews-bot requested a review from valentinewallace June 11, 2025 12:54

joostjager requested a review from tnull June 11, 2025 15:04

joostjager force-pushed the rustfmt-skip-all branch from 1abfab8 to 8cecc34Compare June 12, 2025 07:57

joostjager force-pushed the rustfmt-skip-all branch from 8cecc34 to 2dda75dCompare June 12, 2025 08:00

joostjager mentioned this pull request Jun 12, 2025
Rustfmt chainmonitor.rs #3847
Merged

TheBlueMatt reviewed Jun 12, 2025
View reviewed changes

joostjager force-pushed the rustfmt-skip-all branch 3 times, most recently from 7d69403 to 516fdb3Compare June 12, 2025 18:42

joostjager added 2 commits June 12, 2025 20:45

Convert file-level rustfmt skip to fn level skips
30ceb31

Extract hex constants in test_per_commitment_storage
6932d3e
Formatting improvement.

joostjager force-pushed the rustfmt-skip-all branch from 516fdb3 to 6932d3eCompare June 12, 2025 18:46

valentinewallace reviewed Jun 12, 2025
View reviewed changes

Remove redundant skips
ee3d225

joostjager requested a review from valentinewallace June 12, 2025 19:32

valentinewallace approved these changes Jun 12, 2025
View reviewed changes

joostjager requested a review from TheBlueMatt June 12, 2025 19:36

valentinewallace mentioned this pull request Jun 12, 2025
Remove low-hanging fruit rustfmt::skips in Channel #3854
Merged

TheBlueMatt approved these changes Jun 13, 2025
View reviewed changes

joostjager merged commit 3e9e6a3 into lightningdevkit:mainJun 13, 2025

joostjager mentioned this pull request Jun 18, 2025
Rustfmt Maximalism #3749
Closed