ElementsProject
Use the peerswaprpc proto file for shared serialization
Currently we don't have a shared serialization for peerswapd and the cln-plugin. We should take advantage of the peerswaprpc proto file and use the generated stubs for the responses of the cln-plugin.
Hey @NonsoAmadi10, welcome aboard!
This issue is not worked out really well and I am certain that I was a bit quick with the good first issue label.
The broader idea is to have a common source for the responses of peerswap rpc/grpc methods.
- Core-Lightning uses json-rpcv2 that is currently handled by glightning.
- Lnd uses grpc.
We currently maintain two different data models. The grpc stubs are generated from the peerswaprpc.proto file while the json responses for core-lightning are defined in the clightning dir. It would be nice to have a common model for lnd and core-lightning responses as they already have diverted in the past.
One idea was to use the messages that are generated from peerswap.proto for both implementations. Some core-lightning response messages do already use the generated stubs. Look out for peerswaprpc in the clightning/clightning_commands.go file.
Any thoughts?
Using the generated messages from the peerswap.proto file for both implementations could be a good solution, as it would provide a common source of truth for response messages. But won't it require significant refactoring in both Core-Lightning and Lnd to adopt the new message definitions? One possible approach to minimize the impact of such a change is to gradually introduce the new messages into both components in a backwards-compatible way, while maintaining the existing response models for backwards compatibility. This could involve mapping the new messages to the old models.
I am not quite conversant with proto buffers but I won't mind working on it if I get to understand clearly where all this pieces fit in. My answer is based on the context you defined above. I hope I made sense
Lnd already uses the generated stubs from the peerswaprpc.proto, so there we are already good. For cln, a good first step might be to compare and find differences in the response messages, if there are any (ideally there should be none).
@ShahanaFarooqui (Ride The Lightning) might be the biggest consumer of our api right now. Maybe she can help point to the differences?
If she is okay with the changes that are imposed by using the peerswaprpc.proto as a SSOT for our response messages we should not care too much about compatibility layers right now as the software is in quite an early stage.
Did you join our discord server? Most of the peerswap operators are members of our discord server and we could ask who would be affected by any changes before we make them.
Cheers!
@nepet @NonsoAmadi10 I already have to adjust some of the response signatures for CLN as they are outdated. I wouldn't mind to update some more responses. So I am not worried about backward compatibility right now. Rather it will be helpful when we will implement peerswap UI on LND.
Hello @nepet
I am considering a policy of using messages generated from peerswaprpc for both implementations.
I think there will probably be differences in the struct's json tags.
For example, GetAddressResponse.Address will be changed from lbtc_address to address,omitempty.
Is this acceptable?
Also, it looks like the short channel id and state for PeerSwapPeerChannel needs to be converted to channel id and active.
The existing behavior appears to be that the short channel id is set, though it is named channel id.
I have created a draft https://github.com/ElementsProject/peerswap/pull/231 to show you my thoughts.
