Move code out of `add_entry!` in `get_route`

[Velnbur]

Move all of the code out of add_entry! macro inside get_route to facilitate development experience inside it (as by this time macro backtrace is still only available in nightly versions of compiler) and reduce enourmous get_route function size.

[coderabbitai[bot]]

Walkthrough

The recent update involves a significant refactoring effort within the routing logic of the application. A complex macro previously embedded in the code has been elegantly extracted and encapsulated into a new function named add_entry_internal. This strategic move not only enhances the readability and maintainability of the codebase but also streamlines the overall code organization, ensuring that the logic initially encapsulated by the macro remains intact and functionally coherent.

Changes

File(s)	Change Summary
lightning/src/routing/router.rs	Refactored to extract a complex macro into a separate function add_entry_internal, improving readability and maintainability.

🐇✨
In the realm of code, where logic intertwines,
A rabbit hopped through, refining the lines.
From macro to function, it leaped with grace,
Enhancing the flow, in the digital space.
Readability blooms, maintainability sings,
In the heart of the code, where the magic springs.
🌟🐰

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 83.13609% with 57 lines in your changes missing coverage. Please review.

Project coverage is 89.69%. Comparing base (78c0eaa) to head (b4c62dc). Report is 1071 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/routing/router.rs	83.13%	48 Missing and 9 partials :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2920      +/-   ##
==========================================
- Coverage   89.80%   89.69%   -0.12%     
==========================================
  Files         121      121              
  Lines      100045   100039       -6     
  Branches   100045   100039       -6     
==========================================
- Hits        89845    89728     -117     
- Misses       7533     7623      +90     
- Partials     2667     2688      +21     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[TheBlueMatt]

Hmmm, I'm definitely a fan of moving code out of get_route, but passing in 28 arguments doesn't really improve readability :/. For panics specifically, luckily the panic messages share the arguments passed to the panic, so its still not that hard to just search for the specific panic we hit (and we can add relevant messages to any panics that don't have such clarity). For generally cleaning up routing, if you want to continue down this path we'll need to try to build state structs that we can pass around as singular objects rather than individual fields.

[Velnbur]

but passing in 28 arguments doesn't really improve readability :/.

Yeah, that's a great point, and capturing them through macro makes it hard to read as well. Just wanted to make it "compatible" with other code that's written in parallel with this PR.

For generally cleaning up routing, if you want to continue down this path we'll need to try to build state structs that we can pass around as singular objects rather than individual fields.

I wanted to hear that, so if there is some interest in it, I may continue

P.S.: Also it seems that the codecov found untested lines with this PR :)

[TheBlueMatt]

The other issue we're gonna hit is the routefinding rewrites in #2803 are going to conflict heavily here :/

[TheBlueMatt]

Okay...the bulk of #2803 landed. Any desire to pick this back up?

[Velnbur]

Rebased.

Okay...the bulk of #2803 landed. Any desire to pick this back up?

Yes, if there is still any interest in it.

to try to build state structs that we can pass around as singular objects rather than individual fields.

I'll start from this then. Beginning with obvious ones like num_ignored_* and config variables

[Velnbur]

Still need to add something for the "state" of the path-finding algorithm, but even at this point I'll be glad for any comments about the direction of the changes

[TheBlueMatt]

Nice, yea, in principle this looks great to me, assuming benchmarks don't turn up anything weird (might well be faster cause improved code density).

[TheBlueMatt]

Closing as abandoned.