CLN support, first draft #125
Conversation
There was a problem hiding this comment.
thanks for the PR!
I'm not very familiar with CLN's API but I spun up a mint and did some testing. Happy path in basic operations (like mint and melt) seem to be working fine. Found some issues in certain cases tho.
Tried a payment that will definitively fail (no route) and rather than returning a failed payment status, it returned pending so it left the melt quote in PENDING. Seems like an issue in OutgoingPaymentStatus. I see the response from CLN is empty so then it defaults to pending since it didn't find the payment.
Tried doing an MPP (which would use PayPartialAmount method) and didn't work. Both mints in the payment are able to pay the destination so it should not have failed.
Also, would be good to add some tests. This will likely require some refactoring on my side as those are doing the tests with LND specifically but you can take a look here: https://github.com/elnosh/gonuts/blob/562edf19cc12b969cdddaab3f613d111f1751cdf/mint/mint_integration_test.go. It spins up a mint with LND as the backend and tests all mint operations. I will probably need to make it more generic to pass any lightning backend and test the mint with whatever backend.
| const ( | ||
| InvoiceExpiryTimeCLN = 3600 // 1 hour | ||
| FeePercentCLN = 0.01 | ||
| ) |
There was a problem hiding this comment.
we are using the same expiry time for LND and fakebackend. Maybe it would be good to just set this constant one time for all lightning backends rather than for each one individually.
There was a problem hiding this comment.
They should be configurable by the user imo. And could be defaulted to what they are now.
mint/lightning/cln.go
Outdated
| req.Header.Set("Rune", cln.config.Rune) | ||
| req.Header.Set("accept", "application/json") | ||
| req.Header.Set("Content-Type", "application/json") | ||
|
|
||
| client := &http.Client{} |
There was a problem hiding this comment.
I see all methods are creating a new http client and adding the same headers. I think one http.Client can be reused across all methods. Also probably good to have method that will create request with all these headers.
There was a problem hiding this comment.
Ah yes, very good. Will add that for sure.
mint/lightning/cln.go
Outdated
| // Accept both 200 OK and 201 Created | ||
| if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { | ||
| return Invoice{}, fmt.Errorf("failed to create invoice: %s", resp.Status) | ||
| } |
There was a problem hiding this comment.
will responses have errors? Can it return errors rather than just the status code.
There was a problem hiding this comment.
The HTTP responses absolutely will for other codes. I'll have to handle those for better debugging. I'll have to take a closer look at the API response body for CLN commands.
mint/lightning/cln.go
Outdated
| func (sub *CLNInvoiceSub) Recv() (Invoice, error) { | ||
| for { | ||
| invoice, err := sub.client.InvoiceStatus(sub.paymentHash) | ||
| if err != nil { | ||
| return Invoice{}, err | ||
| } | ||
|
|
||
| // If the invoice is settled, return immediately | ||
| if invoice.Settled { | ||
| return invoice, nil | ||
| } | ||
|
|
||
| // Wait before checking again | ||
| time.Sleep(sub.pollInterval) | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm not familiar with CLN's API but is there no way to get updates on an invoice instead of doing this polling?
There was a problem hiding this comment.
There probably is , I'll look through all the API again, but if there isn't you could make something to do that with a CLN plugin. But this was a way I knew would work.
|
Got it. I think from your testing I have a little better idea on how to fix all the issues. Sorry I'm familiar with CLN but not quite as familiar with all the duties of a mint and what it's looking for. But I'll make a few changes based on your experience and commit them and let you try again. |
…dd timeout to subscription
… Added a different scheme to partial payments, it's closer to the correct restapi calls
|
@elnosh I addressed a few things last night but only had time really to retest that I didn't break anything but could you confirm a MPP ? I was not able to get one to work. |
yeah setup is a bit tricky. You will need 2 mints (with MPP enabled) and have a wallet with funds in both. You can try it from the cli wallet using |
|
@0ceanSlim I refactored the tests here: #126. After this you would just need to do some setup to start the mint with a |
|
perfect! I'll get around to testing soon! Sorry I've been very busy developing my nostr relay lately. |
|
tried running this and failed test but it was for 2025/02/26 15:58:06 invalid lightning client
FAIL github.com/elnosh/gonuts/mint 14.238s
FAIL |
|
you will still need to add some setup to start the mint with CLN like here: gonuts/mint/mint_integration_test.go Lines 97 to 100 in 7ff7d76 |
I think the partial Payment function may still be broken. But I'm able to spin up my mint, pay and mint all working well. Also I know I need to change the .env to path to rune rather than having to put the rune in the config.
There could also be a short guide to generate a proper rune for all the commands you need rather than just using a master rune. Of course the readme will need changes too when the CLN support is actually added.
Take a look and let me know your feedback