Skip to content
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

cln-grpc: Patches rpc-file path #6842

Merged

Conversation

sr-gi
Copy link
Contributor

@sr-gi sr-gi commented Nov 3, 2023

cln-gprc is assuming the rpc-file path is set as default, which may not always be the case. Set the path the what the plugin manager reports

Context

Lately, Polar for macOS has been unable to deploy CLN nodes due to an issue with how the rpc-file is generated on the docker volume (see jamaljsr/polar#779 for details). Recently, a workaround has been found, consisting of moving the rpc-file outside of its default location. However, doing so makes cln-grpc unable to connect to the backend.

This can be worked around by creating a simlink in the default location to point to where lightning-rpc is actually located. However, the underlying issue lies in cln-grpc not getting the info from cln-plugin but instead, assuming the location of the file.

@sr-gi sr-gi requested a review from cdecker as a code owner November 3, 2023 20:11
@sr-gi
Copy link
Contributor Author

sr-gi commented Nov 3, 2023

Note the issue with Polar is on the Polar/docker side, the issue with the workaround is on CLN 😅

Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ACK 5608509

This make sense I do not know how this was working before? maybe there is something that was implicit assumed by let path = Path::new("lightning-rpc");

@cdecker
Copy link
Member

cdecker commented Nov 15, 2023

ACK 5608509

@cdecker cdecker force-pushed the 20231103-fix-clngrpc-rpc-file-path branch from 5608509 to 41cb263 Compare January 16, 2024 17:46
@cdecker
Copy link
Member

cdecker commented Jan 16, 2024

Rebased on top of master, and merging as soon as CI passes.

@cdecker cdecker force-pushed the 20231103-fix-clngrpc-rpc-file-path branch 2 times, most recently from caedac8 to 9cbcd91 Compare February 2, 2024 08:14
`cln-gprc` is assuming the `rpc-file` path is set as default, which may not always
be the case. Set the path the what the plugin manager reports
@cdecker cdecker force-pushed the 20231103-fix-clngrpc-rpc-file-path branch from 9cbcd91 to af8b189 Compare February 5, 2024 09:34
@cdecker
Copy link
Member

cdecker commented Feb 5, 2024

Rebased on top of master. This will get merged as soon as it passes CI.

@cdecker cdecker merged commit 23bd03c into ElementsProject:master Feb 6, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants