search

Ride-The-Lightning / c-lightning-REST

REST APIs for Core Lightning written with node.js
MIT License
119 stars 43 forks source link

Updated fields for msat values #168 #173

Closed ShahanaFarooqui closed 1 year ago

willcl-ark commented 1 year ago

Have you tested this with Zeus?

I just gave it a try and I see my balance there in the form:

xxx_xxx_xxx . xxx_xxx_xxx_x sats (9 decimal places?)

I think for sure on the c-lightning-REST side the final decimal place (which looks like some sort of float rounding error) should be truncated, but I am unsure why Zeus is getting tripped up on what (from the CLR logs) appears to be the correct balance...

edit: actually I think even if we use msat precision here on C-L-R, we would only want balances in the form, not the 9 decimal places I see currently xxx_xxx_xxx . xxx?

edit2: I also still don't see any channels in Zeus, but i also don't see any calls to listpeers in the CLR logs, so perhaps Zeus is not making the request properly for some reason, IDK

sha-265 commented 1 year ago

edit2: I also still don't see any channels in Zeus, but i also don't see any calls to listpeers in the CLR logs, so perhaps Zeus is not making the request properly for some reason, IDK

Maybe it's unrelated but in the RTL app channels appear as offline, even though they are online.

ShahanaFarooqui commented 1 year ago

Maybe it's unrelated but in the RTL app channels appear as offline, even though they are online.

Great catch, thanks. It was due to API endpoint changed field name from connected to peer_connected. Fixed with PR #1241.

ShahanaFarooqui commented 1 year ago

Have you tested this with Zeus?

I just gave it a try and I see my balance there in the form:

xxx_xxx_xxx . xxx_xxx_xxx_x sats (9 decimal places?)

I think for sure on the c-lightning-REST side the final decimal place (which looks like some sort of float rounding error) should be truncated, but I am unsure why Zeus is getting tripped up on what (from the CLR logs) appears to be the correct balance...

edit: actually I think even if we use msat precision here on C-L-R, we would only want balances in the form, not the 9 decimal places I see currently xxx_xxx_xxx . xxx?

Thank you for the quick testing. Core lightning was previously sending these values in Sats also but with v23.05, they are only available in mSats. So we are now passing them to the UI after converting them into Sats (divided by 1000). However removing the precision at API level is less preferred and we would recommend the UI to round the value if needed.

edit2: I also still don't see any channels in Zeus, but i also don't see any calls to listpeers in the CLR logs, so perhaps Zeus is not making the request properly for some reason, IDK

Not sure but it might be due to endpoint updates. Older versions are using listpeers for channels list. v23.05 onwards, channels list will be fetched from listpeerchannels and the field named connected has to be renamed as peer_connected.

Edit: After discussing the backward compatibility issue with @saubyk, I created a separate API endpoint for listpeerchannels. So if you will fetch the latest commit from release 0.10.3 branch, listpeers should work as before.

sha-265 commented 1 year ago

Great catch, thanks. It was due to API endpoint changed field name from connected to peer_connected. Fixed with PR #1241.

Thanks, @ShahanaFarooqui, now the channels appears online as should, but the balance values are still 0, FYI. image

ShahanaFarooqui commented 1 year ago

Yup, I am working on them right now. I will hopefully push all msat changes by the end of tomorrow. Thank you for testing it so quickly.

sha-265 commented 1 year ago

Thank you for fixing it so quickly :)