Skip to content

Refactor litclient and perms to reduce LNC dependencies #1057

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ffranr
Copy link
Contributor

@ffranr ffranr commented May 2, 2025

Context: lightninglabs/lightning-node-connect#113


  • Extracted perms into a standalone module to limit lnc dependencies.
  • Moved jsoncallbacks.go into litrpc to reduce lnc import scope.
  • Bumped lnd version in litrpc to match parent module.

ffranr added 3 commits May 2, 2025 15:00
Moved jsoncallbacks.go into the litrpc package, allowing Lightning Node
Connect to stop importing the litclient package. This change avoids
pulling in the rest of lit and taproot-assets, as litclient was not a
separate module. The now-empty litclient directory has also been
removed.
Updated the LND version in litrpc to align with the version used in
the parent module, ensuring consistency across dependencies.
Converted the perms package into its own module to prevent Lightning
Node Connect from importing the entire lit repo, which includes
taproot-assets. This change avoids exhausting symbol limits in the LNC
WASM client due to unnecessary transitive dependencies.
@ffranr ffranr added the LNC lightning-node-connect sessions label May 2, 2025
@ffranr ffranr requested review from bhandras and guggero May 2, 2025 15:50
@ffranr ffranr self-assigned this May 2, 2025
@ffranr ffranr added the no-changelog This PR is does not require a release notes entry label May 2, 2025
Copy link
Member

@bhandras bhandras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, very nice! 🎉 (needs release notes to pass CI)

@ffranr ffranr removed the no-changelog This PR is does not require a release notes entry label May 4, 2025
Copy link
Member

@guggero guggero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, LGTM 🎉

@@ -0,0 +1,190 @@
module github.com/lightninglabs/lightning-terminal/perms
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option would be to just move this into the litrpc package, to avoid needing yet another submodule. But not sure if it fits there 100% content/topic wise. So just a thought.

@@ -21,6 +21,7 @@ require (
github.com/lightninglabs/lightning-node-connect v0.3.3-alpha.0.20250306111457-cad4234830cc
github.com/lightninglabs/lightning-terminal/autopilotserverrpc v0.0.2
github.com/lightninglabs/lightning-terminal/litrpc v1.0.1
github.com/lightninglabs/lightning-terminal/perms v1.0.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be v1.0.0, as it's the first version. I only needed to bump litrpc to v1.0.1 because the go sum proxy somehow cached that the v1.0.0 didn't exist yet (probably just needed to wait a bit but was impatient).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LNC lightning-node-connect sessions
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

3 participants