CLNRest: performance improvements using sql query #2290
Conversation
| @@ -210,7 +210,8 @@ export default class CLNRest { | |||
| return this.postRequest('/v1/withdraw', request); | |||
| }; | |||
| getMyNodeInfo = () => this.postRequest('/v1/getinfo'); | |||
| getInvoices = () => this.postRequest('/v1/listinvoices'); | |||
| getInvoices = () => | |||
| this.postRequest('/v1/listinvoices', { limit: 150, index: 'created' }); | |||
There was a problem hiding this comment.
This seems like it might help with my 10k+ transactions :-)
(could make this paginate in a followup)
There was a problem hiding this comment.
Yeah, that's the idea.
|
I did some testing on this PR. I checked out the PR locally and built from source. I replaced this It's a noticeable performance increase. I'm running my node on the same local network my phone is connecting to via WIFI. I'm comparing to the current v0.9.0-beta1 release from yesterday to this PR built from source. The activity view loads around 6-7 seconds faster. Here is a screen recording of the performance. The first (light green) activity view loaded is v0.9.0-beta1 (not this PR). The second (yellow) activity view loaded is this PR. I have lurker privacy mode turned on for the recording. screenrecording2.mp4 |
|
Nice. Thank you for testing this. This again isn't the perfect solution because of the 150 records limitation but at least it makes the activity view usable. |
kaloudis
changed the title
|
Let's try this in |
|
@Sjors can you test RC2 please? |
|
Just tested, I can see my transactions now! 🎉 When I click on a (sent) lightning transaction it immediately crashes. Clicking on on chain does behave. I get the impression that I'm not seeing received lightning transactions, though would be mostly zaps and keysend from podcast streaming. |
|
By the way, I'm not a huge fan of the "€1,00 + €0,01 fee" notation. Especially for lightning fees are usually irrelevant and it just seems like visual clutter. It would be better to display fees only if you click on a transaction to see details. What I would like to see in the overview is a description. Maybe that can go where it says "lightning". And then "You sent" could say "You sent (⚡)" This is probably totally unrelated to CLNRest :-) |
Will look into this. |
| @@ -210,7 +210,8 @@ export default class CLNRest { | |||
| return this.postRequest('/v1/withdraw', request); | |||
| }; | |||
| getMyNodeInfo = () => this.postRequest('/v1/getinfo'); | |||
| getInvoices = () => this.postRequest('/v1/listinvoices'); | |||
| getInvoices = () => | |||
| this.postRequest('/v1/listinvoices', { limit: 150, index: 'created' }); | |||
There was a problem hiding this comment.
{ limit: 150, index: 'created' } limits the list of invoices to the first 150 invoices by created index/timestamp. It would make more sense if it limited it to the last 150 invoices, but this is not what it is doing.
There was a problem hiding this comment.
I replaced the /v1/listinvoices call with the following and it worked well for me.
this.postRequest('/v1/sql', {
query: "SELECT label, bolt11, bolt12, payment_hash, amount_msat, status, amount_received_msat, paid_at, payment_preimage, description, expires_at FROM invoices WHERE status = 'paid' ORDER BY created_index DESC LIMIT 150;"
}).then((data: any) => {
const invoices: any[] = [];
data.rows.forEach((invoice: any) => {
invoices.push({
label: invoice[0],
bolt11: invoice[1],
bolt12: invoice[2],
payment_hash: invoice[3],
amount_msat: invoice[4],
status: invoice[5],
amount_received_msat: invoice[6],
paid_at: invoice[7],
payment_preimage: invoice[8],
description: invoice[9],
expires_at: invoice[10]
});
});
return {
invoices: invoices
};
});
Description
Relates to issue: #1637
We spoke about this a bit on the telegram chat, to begin with we will limit the list of transactions to 150. Right now I was able to limit the expensive
bkpr-listaccounteventsandlistinvoices. I couldn't restrictlistpaysbecause it doesn't take a limit argument. There is alistsendpaysthat can take a limit but it doesn't return bolt12 payments (classic CLN).This pull request is categorized as a:
I have tested this PR on the following platforms (please specify OS version and phone model/VM):
I have tested this PR with the following types of nodes (please specify node version and API version where appropriate):