Core Lightning: CLNRest backend #2228

niteshbalusu11 · 2024-06-08T22:29:09Z

Description

Relates to issue: #1637

Depracates c-lightning-rest and adds a new backend using core-lightning team maintained CLN Rest.
Important Note: Minimum core-lightning version to use this is v24.02 due to breaking changes from cln team.
This PR should cover all features which the old backend was covering.
I have not tested this on iOS coz I wasn't able to get xcode to build zeus.

This pull request is categorized as a:

Checklist

I’ve run yarn run tsc and made sure my code compiles correctly
I’ve run yarn run lint and made sure my code didn’t contain any problematic patterns
I’ve run yarn run prettier and made sure my code is formatted correctly
I’ve run yarn run test and made sure all of the tests pass

Testing

If you modified or added a utility file, did you add new unit tests?

No, I’m a fool
Yes
N/A

I have tested this PR on the following platforms (please specify OS version and phone model/VM):

Android
iOS

backends/CLightningREST.ts

backends/CoreLightningRestApi.ts

stores/SettingsStore.ts

niteshbalusu11 · 2024-06-10T03:28:29Z

Fixed and rebased

vincenzopalazzo

Ok looks like that you arrive before me, I left some comments because in this way I can build on top of you

backends/CLNRest.ts

vincenzopalazzo · 2024-06-10T14:24:45Z

views/Settings/NodeConfiguration.tsx

@@ -1347,6 +1357,75 @@ export default class NodeConfiguration extends React.Component<
                                )}
                            </>
                        )}
+                        {implementation === 'cln-rest' && (


why not use the same of c-lightning-REST? at the end of the day the UI is the same

Because cln-rest uses runes and the old one uses a macaroon. We can just delete the old when Zeus wants to depracate it completely. Right now both are supported.

Because cln-rest uses runes and the old one uses a macaroon.

are the same things a the concept level, you can just replace the string with implementation === 'cln-rest' ? "rune" : "macaroon" no?

I just find them cleaner to be separate because old c-lightning-rest is sharing with LND anyway. Whatever Evan wants.

sha-265 · 2024-06-10T20:17:40Z

you are too fast 😵‍💫

anyway, this is probably a good opportunity to fix getTransactions() that uses wrong api method (listfunds which return the wallet utxos, and not transactions history), as i did in my version:
https://github.com/sha-265/zeus/blob/c40d9c1ba20c504f5e3355cf33d9e8b4409fca09/backends/CLNRest.ts#L33

niteshbalusu11 · 2024-06-10T22:07:59Z

anyway, this is probably a good opportunity to fix getTransactions() that uses wrong api method (listfunds which return the wallet utxos, and not transactions history), as i did in my version: https://github.com/sha-265/zeus/blob/c40d9c1ba20c504f5e3355cf33d9e8b4409fca09/backends/CLNRest.ts#L33

Thank you for catching that. I think your code is missing tag == channel_open and tag == channel_close which idk if cln also records has deposit and withdrawal. also it records chain_fee separately.
Have to look into that.

newtonick · 2024-06-11T03:08:38Z

I get this error when trying send onchain in signet via CLNRest (via this PR)

"unknown parameter: address, this may be caused by
a failure to autodetect key=value-style parameters.
Please try using the -k flag and explicit key=value
pairs of parameters."

I checked to make sure I had not issue with withdraw on my node via CLI.

backends/CLNRest.ts

sha-265 · 2024-06-11T14:28:30Z

anyway, this is probably a good opportunity to fix getTransactions() that uses wrong api method (listfunds which return the wallet utxos, and not transactions history), as i did in my version: https://github.com/sha-265/zeus/blob/c40d9c1ba20c504f5e3355cf33d9e8b4409fca09/backends/CLNRest.ts#L33

Thank you for catching that. I think your code is missing tag == channel_open and tag == channel_close which idk if cln also records has deposit and withdrawal. also it records chain_fee separately. Have to look into that.

i don't see this tags (at least under wallet account), but there is a journal_entry tag that should be filtered out maybe.

niteshbalusu11 · 2024-06-11T14:31:22Z

i don't see this tags (at least under wallet account), but there is a journal_entry tag that should be filtered out maybe.

Yeah, that's the problem, it doesn't put it under wallet. Have to figure out where it does. In the examples in the docs, it puts it under some long string account. Have to see what that is.

sha-265 · 2024-06-11T14:44:12Z

i don't see this tags (at least under wallet account), but there is a journal_entry tag that should be filtered out maybe.

Yeah, that's the problem, it doesn't put it under wallet. Have to figure out where it does. In the examples in the docs, it puts it under some long string account. Have to see what that is.

it's channel id according to the cln documentation:

account (string): The account name. If the account is a channel, the channel_id.

i'm not sure these events are relevant if you want only the on-chain transactions.

niteshbalusu11 · 2024-06-11T15:35:49Z

but i think in terms of performance query without account parameter, would be much more heavy as it returns all the non on-chain events

I agree. CLNs APIs don't seem to allow pagination as well which is annoying. Maybe someone can test with their mainnet node which is old and see how it performs.
They should ideally have a simple RPC for chain txs but I'm not gonna rant about their RPC build choices here.

sha-265 · 2024-06-11T15:45:14Z

but i think in terms of performance query without account parameter, would be much more heavy as it returns all the non on-chain events

I agree. CLNs APIs don't seem to allow pagination as well which is annoying. Maybe someone can test with their mainnet node which is old and see how it performs. They should ideally have a simple RPC for chain txs but I'm not gonna rant about their RPC build choices here.

yep, not the best api i've come across 🤷‍♂️

another mitigation could be somehow to make only one query for getTransactions() and getInvoices() instead of two separated calls for the same data. creating getEvents() method and use it for all of them, or something

EDIT: and getPayments(), so 3 different calls

i would do that myself, but it won't get me the bounty probably haha 🥲

newtonick · 2024-06-13T21:01:16Z

From what testing I have been able to do, I believe activity view needs some love. It might be helpful to start with what transactions should be included in this view. I'm not sure if this is documented somewhere (like a previous zeus GitHub issue/PR/etc). What I expect to be listed in the activity view is:

lightning Invoices (paid, unpaid, expired)
lightning payment (successful payments only) w/ fees
onchain withdraw/payment (in mempool and onchain) w/ fees
onchain receives (in mempool and onchain)

I don't expect to see channel open / close transactions but I also don't mind if they are in this view if it's communicated that they are part of an open/close channel transaction.

Right now I see multiple rows in the activity view for a single on chain payments. I have a hard time dissecting what information bkpr-listaccountevents is returning. Sorry this comment is not more helpful. I just wanted to provide feedback on what I have tested so far with this PR. Thank you for this work.

niteshbalusu11 · 2024-06-13T21:09:36Z

What I expect to be listed in the activity view is:

lightning Invoices (paid, unpaid, expired)
lightning payment (successful payments only) >w/ fees
onchain withdraw/payment (in mempool and >onchain) w/ fees
onchain receives (in mempool and onchain)

This sounds good to me except expired and unpaid invoices, that could be a bit too much specially expired.
I will leave it to @kaloudis once he tests and gives feedback on what he thinks.

newtonick · 2024-06-13T21:12:28Z

Here is an example of 40,000 sat onchain spend (https://mutinynet.com/tx/99a918361919956cc76801b6014e233d834466dbb25b4f031825e1eb60b972e8) that shows 3 rows in the activity view for a single spend transaction. The 40,000 sat row appears first as soon as I complete the spend (while the tx is in the mempool). This shows as a credit but is actually a debit. Then once the transaction is confirmed into 1+ blocks, 2 more rows appear. A debit of the utxo I used to make the spend and then a credit of the change back to my wallet.

niteshbalusu11 · 2024-06-13T21:16:33Z

Here is an example of 40,000 sat onchain spend (https://mutinynet.com/tx/99a918361919956cc76801b6014e233d834466dbb25b4f031825e1eb60b972e8) that shows 3 rows in the activity view for a single spend transaction. The 40,000 sat row appears first as soon as I complete the spend (while the tx is in the mempool). This shows as a credit but is actually a debit. Then once the transaction is confirmed into 1+ blocks, 2 more rows appear. A debit of the utxo I used to make the spend and then a credit of the change back to my wallet.

Ok, yeah this needs to be fixed. Only the spend should show up, not the utxos spent and change received.

sha-265 · 2024-06-13T21:24:06Z

I don't expect to see channel open / close transactions

i agree, and it's also solve the potential performance issue, as you can request only wallet account events, and not all the events for all the accounts in this node, as i did in my code.

niteshbalusu11 · 2024-06-14T00:07:45Z

ok @newtonick can you test now? I think it should be fixed. Note that if a channel closes, you will see an onchain payment because you're getting your money back.

@sha-265 unfortunately i think we can't get around the performance issue without compromising accuracy of the info because CLN displays change outputs as deposits and its really difficult to figure out if its a real deposit or a change output from like channel opens without checking channel opens from events. I asked in the CLN group if there is an easy way to check if an address is ours in CLN so that we can easily filter out change, let's see.

sha-265 · 2024-06-14T00:53:32Z

i don't understand how tons of irrelevant events solving this, but at least you should get the same data only once and not three different times.

niteshbalusu11 · 2024-06-14T00:55:53Z

i don't understand how tons of irrelevant events solving this, but at least you should get the same data only once and not three different times.

I don't understand what you mean? I am getting only once.

sha-265 · 2024-06-14T01:03:28Z

look here:

zeus/stores/ActivityStore.ts

Lines 165 to 168 in e89187c

    
           await this.paymentsStore.getPayments(); 
        
           if (BackendUtils.supportsOnchainSends()) 
        
               await this.transactionsStore.getTransactions(); 
        
           await this.invoicesStore.getInvoices();

i think if you are using bkpr-listaccountevents this three calls are overlapping.

niteshbalusu11 · 2024-06-14T01:05:13Z

look here:

zeus/stores/ActivityStore.ts

Lines 165 to 168 in e89187c

await this.paymentsStore.getPayments();

if (BackendUtils.supportsOnchainSends())

await this.transactionsStore.getTransactions();

await this.invoicesStore.getInvoices();

i think if you are using bkpr-listaccountevents this three calls are overlapping.

Ah, that's old code. I never touched that. Will look into it.

niteshbalusu11 · 2024-06-16T00:49:07Z

ElementsProject/lightning#7397

Sjors · 2024-06-26T06:36:17Z

I finally got the clnrest plugin to work (Python environments are a pain...). Happy to test all this when there's a testflight build.

backends/CLightningREST.ts

kaloudis

tACK

All things looking good. Congrats, @niteshbalusu11 you will be receiving the bounty.

We are expecting you to help address any issues that may arise during alpha and beta testing - not expecting anything major as we've already done some extensive testing

Sjors · 2024-07-15T16:30:25Z

Just tested the https://github.com/ZeusLN/zeus/releases/tag/v0.9.0-beta1 release on iOs. It manages to connect!

I can seen my utxos on the coins screen. I can see my channels.

I can create an invoice to receive. The screen doesn't update when the invoice is paid, but afaik it didn't do that with c-lightning-rest either.

The Activity tab still crashes as it did with c-lightning-rest.

The combination of the above two things means I can't see if a payment to me succeeded.

I can pay a regular bolt11 invoice.

niteshbalusu11 · 2024-07-15T16:42:18Z

The Activity tab still crashes as it did with c-lightning-rest.

@Sjors is your node really old with a lot of data?

Sjors · 2024-07-15T16:48:27Z

@niteshbalusu11 the node is very old, but it does run the latest v24.05. It has 10K+ transactions, mostly keysends from podcasting 2.0 listeners.

I guess it's tripping over missing field in older transactions?

niteshbalusu11 · 2024-07-15T17:27:32Z

I guess it's tripping over missing field in older transactions?

It could also be that the response from the 10k+ txns could be very large because the RPC call right now has no way limit the records returned. Working on figuring that part out.

niteshbalusu11 · 2024-07-16T00:09:13Z

This might fix some issues.

#2290

sha-265 · 2024-09-01T20:32:40Z

Important Note: Minimum core-lightning version to use this is v24.02 due to breaking changes from cln team.

I don't know how, but it's working for me with older version

Sjors · 2024-09-02T06:34:04Z

The screen doesn't update when the invoice is paid, but afaik it didn't do that with c-lightning-rest either.

More recently this usually does work.

The Activity tab still crashes as it did with c-lightning-rest.

This is fixed since.

kaloudis changed the title ~~New-cln-rest-api~~ Core Lightning: CLNRest backend Jun 9, 2024

kaloudis added Core Lightning Issues specific to Core Lightning c-lightning-REST CLNRest labels Jun 9, 2024

kaloudis added this to the v0.9.0 milestone Jun 9, 2024

kaloudis reviewed Jun 10, 2024

View reviewed changes

backends/CLightningREST.ts Show resolved Hide resolved

kaloudis reviewed Jun 10, 2024

View reviewed changes

backends/CoreLightningRestApi.ts Outdated Show resolved Hide resolved

kaloudis reviewed Jun 10, 2024

View reviewed changes

stores/SettingsStore.ts Outdated Show resolved Hide resolved

nitesh and others added 5 commits June 9, 2024 23:25

initial working of the new plugin

4c9e4c9

add payments, invoices, receiving onchain, offchain, txs and balances

c6ea92c

add open channel, add peer, decode, keysend, pay, closechannel

cd4af6f

add fees and bolt12

82595e6

fix nits

4088cc6

niteshbalusu11 force-pushed the new-cln-rest-api branch from 460d7ca to 4088cc6 Compare June 10, 2024 03:26

vincenzopalazzo suggested changes Jun 10, 2024

View reviewed changes

nitesh added 4 commits June 10, 2024 11:22

fixed some nits

10a38b8

mark coin controls as default

3e67c1b

decouple cln and lnd backends

f5e2c27

minor compatibility fixes

194f994

newtonick reviewed Jun 11, 2024

View reviewed changes

backends/CLNRest.ts Outdated Show resolved Hide resolved

fix chain withdrawal

f47ce54

newtonick reviewed Jun 11, 2024

View reviewed changes

backends/CLNRest.ts Outdated Show resolved Hide resolved

fix chain withdrawal

49a4c8b

nitesh added 2 commits June 13, 2024 19:44

fix transactions list

e192c9d

actually fix transactions list

68f5c0f

kaloudis mentioned this pull request Jun 18, 2024

[feature-request]: Enhance listtransactions rpc ElementsProject/lightning#7397

Open

kaloudis requested changes Jul 5, 2024

View reviewed changes

backends/CLightningREST.ts Outdated Show resolved Hide resolved

revert old changes

34ce487

niteshbalusu11 requested a review from kaloudis July 8, 2024 16:59

kaloudis approved these changes Jul 9, 2024

View reviewed changes

kaloudis merged commit c1e948a into ZeusLN:master Jul 9, 2024
3 checks passed

niteshbalusu11 deleted the new-cln-rest-api branch September 2, 2024 17:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Core Lightning: CLNRest backend #2228

Core Lightning: CLNRest backend #2228

niteshbalusu11 commented Jun 8, 2024

niteshbalusu11 commented Jun 10, 2024

vincenzopalazzo left a comment

vincenzopalazzo Jun 10, 2024

niteshbalusu11 Jun 10, 2024

vincenzopalazzo Jun 10, 2024

niteshbalusu11 Jun 10, 2024 •

edited

Loading

sha-265 commented Jun 10, 2024 •

edited

Loading

niteshbalusu11 commented Jun 10, 2024

newtonick commented Jun 11, 2024 •

edited

Loading

sha-265 commented Jun 11, 2024 •

edited

Loading

niteshbalusu11 commented Jun 11, 2024

sha-265 commented Jun 11, 2024 •

edited

Loading

niteshbalusu11 commented Jun 11, 2024

sha-265 commented Jun 11, 2024 •

edited

Loading

newtonick commented Jun 13, 2024

niteshbalusu11 commented Jun 13, 2024 •

edited

Loading

newtonick commented Jun 13, 2024

niteshbalusu11 commented Jun 13, 2024

sha-265 commented Jun 13, 2024 •

edited

Loading

niteshbalusu11 commented Jun 14, 2024 •

edited

Loading

sha-265 commented Jun 14, 2024

niteshbalusu11 commented Jun 14, 2024

sha-265 commented Jun 14, 2024

niteshbalusu11 commented Jun 14, 2024

niteshbalusu11 commented Jun 16, 2024

Sjors commented Jun 26, 2024

kaloudis left a comment

Sjors commented Jul 15, 2024

niteshbalusu11 commented Jul 15, 2024

Sjors commented Jul 15, 2024 •

edited

Loading

niteshbalusu11 commented Jul 15, 2024

niteshbalusu11 commented Jul 16, 2024

sha-265 commented Sep 1, 2024

Sjors commented Sep 2, 2024

Core Lightning: CLNRest backend #2228

Core Lightning: CLNRest backend #2228

Conversation

niteshbalusu11 commented Jun 8, 2024

Description

Checklist

Testing

niteshbalusu11 commented Jun 10, 2024

vincenzopalazzo left a comment

Choose a reason for hiding this comment

vincenzopalazzo Jun 10, 2024

Choose a reason for hiding this comment

niteshbalusu11 Jun 10, 2024

Choose a reason for hiding this comment

vincenzopalazzo Jun 10, 2024

Choose a reason for hiding this comment

niteshbalusu11 Jun 10, 2024 • edited Loading

Choose a reason for hiding this comment

sha-265 commented Jun 10, 2024 • edited Loading

niteshbalusu11 commented Jun 10, 2024

newtonick commented Jun 11, 2024 • edited Loading

sha-265 commented Jun 11, 2024 • edited Loading

niteshbalusu11 commented Jun 11, 2024

sha-265 commented Jun 11, 2024 • edited Loading

niteshbalusu11 commented Jun 11, 2024

sha-265 commented Jun 11, 2024 • edited Loading

newtonick commented Jun 13, 2024

niteshbalusu11 commented Jun 13, 2024 • edited Loading

newtonick commented Jun 13, 2024

niteshbalusu11 commented Jun 13, 2024

sha-265 commented Jun 13, 2024 • edited Loading

niteshbalusu11 commented Jun 14, 2024 • edited Loading

sha-265 commented Jun 14, 2024

niteshbalusu11 commented Jun 14, 2024

sha-265 commented Jun 14, 2024

niteshbalusu11 commented Jun 14, 2024

niteshbalusu11 commented Jun 16, 2024

Sjors commented Jun 26, 2024

kaloudis left a comment

Choose a reason for hiding this comment

Sjors commented Jul 15, 2024

niteshbalusu11 commented Jul 15, 2024

Sjors commented Jul 15, 2024 • edited Loading

niteshbalusu11 commented Jul 15, 2024

niteshbalusu11 commented Jul 16, 2024

sha-265 commented Sep 1, 2024

Sjors commented Sep 2, 2024

niteshbalusu11 Jun 10, 2024 •

edited

Loading

sha-265 commented Jun 10, 2024 •

edited

Loading

newtonick commented Jun 11, 2024 •

edited

Loading

sha-265 commented Jun 11, 2024 •

edited

Loading

sha-265 commented Jun 11, 2024 •

edited

Loading

sha-265 commented Jun 11, 2024 •

edited

Loading

niteshbalusu11 commented Jun 13, 2024 •

edited

Loading

sha-265 commented Jun 13, 2024 •

edited

Loading

niteshbalusu11 commented Jun 14, 2024 •

edited

Loading

Sjors commented Jul 15, 2024 •

edited

Loading