Core Lightning: CLNRest backend

[niteshbalusu11]

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:

• [X] New feature
• [ ] Bug fix
• [ ] Code refactor
• [ ] Configuration change
• [ ] Locales update
• [ ] Quality assurance
• [ ] Other

Checklist

• [ ] I’ve run yarn run tsc and made sure my code compiles correctly
• [X] I’ve run yarn run lint and made sure my code didn’t contain any problematic patterns
• [X] I’ve run yarn run prettier and made sure my code is formatted correctly
• [X] 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):

• [X] Android
• [ ] iOS

[niteshbalusu11]

Fixed and rebased

[sha-265]

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]

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]

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.

[sha-265]

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]

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]

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]

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

I think they are relevant because channel opens and closes are chain txs. I pushed a commit to include those. Please test if you can. I just pushed a commit to make the txs more readable so you know what the tx is for with a new notes field.

[sha-265]

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

I think they are relevant because channel opens and closes are chain txs. I pushed a commit to include those. Please test if you can. I just pushed a commit to make the txs more readable so you know what the tx is for with a new notes field.

i don't know if there is no duplicate events in wallet account for channel opening (at least).

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 (payments and routing), that could be a lot of events, so it should be considered also.

[niteshbalusu11]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

look here: https://github.com/ZeusLN/zeus/blob/e89187c68c6de95487ce94267849e1c1b416e9c5/stores/ActivityStore.ts#L165-L168

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

[niteshbalusu11]

look here: https://github.com/ZeusLN/zeus/blob/e89187c68c6de95487ce94267849e1c1b416e9c5/stores/ActivityStore.ts#L165-L168

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]

https://github.com/ElementsProject/lightning/issues/7397

[Sjors]

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

[Sjors]

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]

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

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

[Sjors]

@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]

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]

This might fix some issues.

https://github.com/ZeusLN/zeus/pull/2290

[sha-265]

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]

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.