BitBanana icon indicating copy to clipboard operation
BitBanana copied to clipboard

Show keysend messages on Core Lightning

Open sha-265 opened this issue 1 year ago • 57 comments

I'm using BitBanana with CLN.

Memos are shown only for incoming transactions but not for outgoing. Memos are missing in the history view, but also in the transaction's details modal.

Please add the memos for outgoing transactions also, in history and in the modal.

Thank you

sha-265 avatar Dec 03 '24 12:12 sha-265

Thanks for the issue. I found and fixed the bug. It will be working in the next version!

michaelWuensch avatar Dec 05 '24 09:12 michaelWuensch

Thank you

sha-265 avatar Dec 05 '24 10:12 sha-265

This is fixed with 0.8.8 which got just released

michaelWuensch avatar Dec 23 '24 10:12 michaelWuensch

Thank you. When this version will be available on the Play store?

sha-265 avatar Dec 24 '24 17:12 sha-265

@sha-265 It is already live, but I did a partial rollout to only 30% of users. I'll observe crash reports and if nothing bad occurs, I'll roll out to 100 % in 2 days.

michaelWuensch avatar Dec 25 '24 01:12 michaelWuensch

@michaelWuensch, is it just me or the memo should appear for both of these entries, that belong to the same rebalancing transaction? I'm using the new version - 0.8.8

image

sha-265 avatar Dec 26 '24 16:12 sha-265

@sha-265 yeah, I guess it should show there as well. I am reopening the issue for furter investigation.

Did anything change for you in 0.8.8 in regard to memos for outgoing transactions? Are some shown now?

michaelWuensch avatar Dec 26 '24 18:12 michaelWuensch

Did anything change for you in 0.8.8 in regard to memos for outgoing transactions? Are some shown now?

Yes, memos are shown for some of the outgoing transactions, for other outgoing transactions memos are missing, but I'm not sure if it is because there is no memo for these transactions or the memo exists but not shown. The rebalance transaction is a transaction that I was pretty sure that memo exists but not shown.

sha-265 avatar Dec 26 '24 18:12 sha-265

Just to confirm. I found other standard transactions (not rebalance) that the memo is missing. I compared the same transactions in Zeus wallet, and the memo appears there.

image image

sha-265 avatar Jan 05 '25 20:01 sha-265

Is there any pattern/difference you recognize? Like is it a keysend transaction or maybe a transaction to a specific node type or version etc?

michaelWuensch avatar Jan 06 '25 00:01 michaelWuensch

It's not keysend transactions, but I can't find a pattern. One is the rebalance transaction, second is regular one, and the third is MPP transaction (that appears as three different transactions, instead of one, but this is another bug I guess).

sha-265 avatar Jan 06 '25 10:01 sha-265

Thanks for the info, I am wrapping up the next commit for this issue as we speak. This should fix MPP transactions being shown as different payments. It also adds the description (and Payer note) for outgoing payments that paid a bolt 12 offer. These were not shown so far. Furthermore I replaced some code that might have failed with a more robust one. I hope all of these measures will finally fix the issue. I'll let you know when the next version is out so you can test.

michaelWuensch avatar Jan 06 '25 11:01 michaelWuensch

Thank you.

sha-265 avatar Jan 06 '25 11:01 sha-265

I just released v0.8.9, it should be available on Google Play soon. Please test it and let me know if it worked ! :smiley:

michaelWuensch avatar Jan 06 '25 12:01 michaelWuensch

Thank you again, @michaelWuensch, but I tried the new version, and I have major performance issue (the app is very choppy), and the transactions history list not loaded. And if it been loaded (once), it loaded only incoming transactions for some reason 🫢

sha-265 avatar Jan 06 '25 14:01 sha-265

Damn, what version of core lightning are you running? I tested with 24.11. And do you have a huge transaction history? My own node is LND, so I don't have a core lightning node with lots of transactions, just a small regtest node and there everything worked fine.

michaelWuensch avatar Jan 06 '25 14:01 michaelWuensch

Ok, I'll see if I can test it tomorrow with 23.11 and find out whats going on.

michaelWuensch avatar Jan 06 '25 16:01 michaelWuensch

Hmm, I am a bit lost right now. I tried a core lightning 23.11.2 regtest node with 0.8.9 and made some transactions. Everything works. I have an updaid invoice, 3 incoming transactions from different nodes, an outgoing to another core lightning node, an outgoing to an LND node, a keysend transaction and an mpp transaction to a LND node. Transaction history loads without any problems and the mpp transaction is displayed as a single transaction now. On the one hand this is good news, but on the other hand it is bad, because it might also affect nodes running new versions of Core Lightning.

I see a few possibilities on how to go on from here:

  1. You download the 0.8.8 version from github and install that, then you should be able to use it again.
  2. I have to find a way to simulate hundreds and thousands of transactions without doing it manually. So I can see if there is a performance problem.
  3. If you are willing to install non official releases, I can provide you some APKs where I revert parts of the code. This way we can test which code changes break the transaction history for your node.

michaelWuensch avatar Jan 07 '25 01:01 michaelWuensch

  1. I did, and it's working fine again.
  2. I don't know if it has this functionality but maybe try Polar.
  3. I can test it if you unable to reproduce this issue.

sha-265 avatar Jan 07 '25 10:01 sha-265

Thanks, yes, I already use Polar a lot. Ok, I did some more tests today. The description is not exposed in the API that is why I have to parse the bolt11 string for the description which needs some time. I had a custom self written function to do this that I replaced with proper bolt11 parsing from a library in the last update. I did this as maybe my own function fails in some cases and therefore the app might not show the description on some payments. It turns out the new parsing is about 8 times slower than the custom one I made. On my phone parsing for 1000 payments took about 1 second with the new one which isn't that bad, but maybe you use an old slow smartphone? Anyway, I now changed the code so that it does not automatically parse all payments on load time, but only those that are actually displayed, which means it will do the parsing on demand while you scroll in the transaction history. The result should actually be better performance than version 0.8.8 while still providing the proper parsing. Use this link to download a preview of 0.8.10 and and let me know if it works for you: https://drive.proton.me/urls/SF4MDBBNCG#wZKwjAimMMQv

michaelWuensch avatar Jan 07 '25 11:01 michaelWuensch

Ok, thanks for testing. If that didn't fix it can only be the change that combined the mpp transactions. I'll provide another version for you to test in the coming days.

michaelWuensch avatar Jan 07 '25 13:01 michaelWuensch

Ok, I was able to reproduce it and found the issue. The new endpoint I used (that combines mpp payments) does not support pagination for versions older than 24.11. This means the issue affects all core lightning users with a version older than 24.11 and more than 500 outgoing lightning payments. I fixed it and still kept the performance improvement I did earlier. Please test the new version, which you can download below and report back the following:

  • Is transaction history loading?
  • Are mpp payments combined?
  • Are all descriptions for outgoing lightning payments displayed now?

Here is the apk: https://drive.proton.me/urls/CA9RCY10VG#gNeBXYdcelyJ

michaelWuensch avatar Jan 08 '25 09:01 michaelWuensch

Ok, according to my test:

  • Yes, the transaction history is loading.
  • Yes, MPP payments are combined.
  • No, not all descriptions outgoing lightning payments displayed. The same payments' memos as before are missing.

Thank you.

sha-265 avatar Jan 08 '25 10:01 sha-265

Thanks for testing! Well, I am out of ideas why some memos are still missing. What really boggles my mind is how Zeus is able to show it. They do nothing fancy in their code. At least I improved the app while trying to fix this (combined mpp in transaction history, description and payer note added for bolt12 payments & performance improvements) I am giving up on this until I'm either able to reproduce it myself, or until you have found a pattern or you know how to reproduce it. Alternatively, as you have access to the payments without memos, you could try to debug and fix it yourself, BitBanana is open source :smile:

michaelWuensch avatar Jan 08 '25 11:01 michaelWuensch

Thank you for your efforts.

Alternatively, as you have access to the payments without memos, you could try to debug and fix it yourself, BitBanana is open source 😄

I only need to learn Java, and I'm ready to go 🤣

sha-265 avatar Jan 08 '25 11:01 sha-265

I have an update. After upgrading my CLN node to version 24.11.1, I see incoming transactions, but no outgoing transaction in the history view.

I tried your last custom build in https://github.com/michaelWuensch/BitBanana/issues/99#issuecomment-2577194697, and the stable release of BitBanana (0.8.9). Both have this problem.

@michaelWuensch, any idea why?

sha-265 avatar Feb 25 '25 13:02 sha-265

@sha-265 was the outgoing on-chain transaction to an address the wallet controls? (Basically just moving or consolidating coins?) In this case I am aware that it is not shown right now. If it was to addresses of other wallets it is a bug and I have to investigate.

michaelWuensch avatar Feb 25 '25 15:02 michaelWuensch

No. Not on-chain txs at all, I'm talking about regular LN txs.

sha-265 avatar Feb 25 '25 15:02 sha-265

Oh ok. Actually I wanted to release a new version today. In this version I also reworked the whole pagination in a hopefuly proper way, while it was a quick fix in the last test version I sent you... So can you please try if the new release would fix it? https://drive.proton.me/urls/XWFPD322XW#zgGzxzGgzr29 If not, I'll investigate this further before doing the release.

michaelWuensch avatar Feb 25 '25 16:02 michaelWuensch

So can you please try if the new release would fix it?

I tried. Unfortunately the problem still exists.

sha-265 avatar Feb 25 '25 16:02 sha-265