8000 feat: Add support for the new CLNRest as a funding source by Amperstrand · Pull Request #2778 · lnbits/lnbits · GitHub
[go: up one dir, main page]
Skip to content

feat: Add support for the new CLNRest as a funding source #2778

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 34 commits into from
Jul 8, 2025
Merged

feat: Add support for the new CLNRest as a funding source #2778

merged 34 commits into from
Jul 8, 2025

Conversation

Amperstrand
Copy link
Contributor
@Amperstrand Amperstrand commented Nov 19, 2024 •
edited
Loading

#2232 never quite worked and was more of a POC.

Still a work in progress, but should just work.

To test it:

CLNREST_URL=https://127.0.0.1:3010
CLNREST_CA=/home/lightningd/.lightning/bitcoin/ca.pem
CLNREST_CERT=/home/lightningd/.lightning/bitcoin/server.pem

You can also use the contents of these files instead of the path to the file:
CLNREST_CA = cat ca.pem | awk '{printf "%s\\n", $0} END {printf "\n"}'
CLNREST_CERT =  cat server.pem | awk '{printf "%s\\n", $0} END {printf "\n"}'

CLNREST_READONLY_RUNE=lightning-cli createrune restrictions='[["method=listfunds", "method=listpays", "method=listinvoices", "method=getinfo", "method=summary", "method=waitanyinvoice"]]' | jq -r .rune
CLNREST_INVOICE_RUNE=lightning-cli createrune restrictions='[["method=invoice"], ["pnameamount_msat<1000001"], ["pnamelabel^LNbits"], ["rate=60"]]' | jq -r .rune

These will give LNbits permission to pay invoices:
#CLNREST_PAY_RUNE=lightning-cli createrune restrictions='[["method=pay"], ["pinvbolt11_amount<1001"],   ["pnamelabel^LNbits"], ["rate=1"]]' | jq -r .rune
#CLNREST_RENEPAY_RUNE=lightning-cli createrune restrictions='[["method=renepay"], ["pinvinvstring_amount<1001"], ["pnamelabel^LNbits"], ["rate=1"]]' | jq -r .rune

This is not really needed, but reduces start up time because it will otherwise ask for all invoices since the beginning of time from your core lightning node:
#CLNREST_LAST_PAY_INDEX='lightning-cli listinvoices | jq -r '.invoices | map(.created_index) | max'

# For core lightning v23.08, you had to provide the nodeid in order to call CLNRest, but this is no longer needed for more recent releases
#CLNREST_NODEID=lightning-cli getinfo | jq -r .id # only required for v23.08

@Amperstrand Amperstrand mentioned this pull request Nov 19, 2024
Closed
@Schnema1
Copy link

Great work @Amperstrand and thank you for the support.

I did first tests with this pull request. Setup:
Testnode with separate users for each application. Bitcoin Core v28.rc2 and Core Lightning 24.08 (testnet3).

The setup of runes worked fine, although with slightly different commands.

Set up the .env file.
The runes have to be added like this: CLNREST_READONLY_RUNE=Bp4TZmzsTNnQzA.....
Attention: You cannot point to ca.pem and server.pem files within lightning user .lighting/testnet folder. Lnbits user has no right to read the certificates, and you get an error during start of lnbits.
Solution: You need to copy the two certificates into lnbits home and change the path in .env to the correct path. Similar as we had to do with access.macaroon file.

Improvement suggestion:
.env file: Instead of adding the content of a rune in the .env file, we could point to a rune file.

Open:
Testing the restriction of runes.
Code review, as I have no programming skills.

@dni dni changed the title Add support for the new CLNRest as a funding source feat: Add support for the new CLNRest as a funding source Dec 19, 2024
@bitcoinlizard bitcoinlizard mentioned this pull request Jan 16, 2025
Closed
@dni dni self-requested a review July 2, 2025 07:56
@dni dni force-pushed the wallets/newCLNRest branch from 1b72153 to 3428546 Compare July 2, 2025 13:22
@codecov Codecov
Copy link
codecov bot commented Jul 2, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 15.50388% with 218 lines in your changes missing coverage. Please review.

Project coverage is 59.79%. Comparing base (264d1fa) to head (8c4f6b0).

Files with missing lines Patch % Lines
lnbits/wallets/clnrest.py 11.74% 218 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##              dev    #2778      +/-   ##
==========================================
- Coverage   60.56%   59.79%   -0.77%     
==========================================
  Files         111      112       +1     
  Lines       14587    14845     +258     
==========================================
+ Hits         8834     8876      +42     
- Misses       5753     5969     +216

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dni dni mentioned this pull request Jul 2, 2025
Merged
@dni dni force-pushed the wallets/newCLNRest branch 2 times, most recently from ed8ac8f to e09ccbc Compare July 8, 2025 05:01
@dni dni marked this pull request as ready for review July 8, 2025 05:32
@motorina0 motorina0 force-pushed the wallets/newCLNRest branch from 345f67b to c83fb98 Compare July 8, 2025 08:25
@motorina0
Copy link
Collaborator

code review: Amperstrand#1

Amperstrand and others added 24 commits July 8, 2025 16:30
@motorina0
better error handling
29f617c
@motorina0
try to figure out why the old code called listinvoices in addition to…
b2699cf 
… listanyinvoice
@motorina0
improve consistency
6eaa857
@motorina0
cleanup status function
a953792
@motorina0
should be known good state
2d3ac35
@motorina0
typo
910ab15
@motorina0
This should work and a new merge request should probably be opened of…
ea5bea5 
…f this branch. lnbits#2232
@motorina0
forgot this
706d070
@motorina0
npm bundle install
4dbc5f6
@motorina0
increase verbosity of log
bc020d4
@dni @motorina0
fix linting
1e61e5c
@dni @motorina0
fixup!
4e4f534
@dni @motorina0
rename labels
b723418
@dni @motorina0
simplify
ec68d24
@dni @motorina0
refactor, cleanup fixes
56ed746
@motorina0
fix: remove verbose hasattr
d651ff5
@motorina0
fix: mark private function with _ prefix and move them to the bottom
79f412a
@motorina0
chore: lower log level; add Exception catch block
cb20f2f
@motorina0
chore: lower log level
54d791f
@motorina0
fix: move PaymentResponse creation inside the try block so errors c…
94809af 
…an be handled
@motorina0
refactor: move http calls inside the try block
a449ddb
@motorina0
fix: replace raise with return
9097cc6
@motorina0
refactor: simplify conditions
cb75aa8
@motorina0
refactor: lower log level
7873231
@motorina0 motorina0 force-pushed the wallets/newCLNRest branch from 8c4f6b0 to 7873231 Compare July 8, 2025 13:30
@dni dni merged commit cba8c02 into lnbits:dev Jul 8, 2025
dni added a commit that referenced this pull request Jul 15, 2025
@Amperstrand @dni @motorina0
feat: Add support for the new CLNRest as a funding source (#2778)
cdc1a6d 
Co-authored-by: Amperstrand <amperstrand@localhost>
Co-authored-by: dni ⚡ <office@dnilabs.com>
Co-authored-by: Vlad Stan <stan.v.vlad@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dni dni dni approved these changes

@motorina0 motorina0 motorina0 approved these changes

@talvasconcelos talvasconcelos Awaiting requested review from talvasconcelos

@arcbtc arcbtc Awaiting requested review from arcbtc

@blackcoffeexbt blackcoffeexbt Awaiting requested review from blackcoffeexbt

Assignees

@dni dni

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Amperstrand @Schnema1 @motorina0 @dni
0