rust-teos
rust-teos copied to clipboard
feat(lightning): Add Lightning as transport
This PR introduces bolt13 message structs and their lightning (de)serialization, to be used by the tower (for the lightning server) and client apps.
LDK's (de)serialization traits are used to make the message structs sendable over wire (Readable and Writeable).
A small customized macro-based serialization framework similar to LDK's was added to help organize the code and reduce its size.
Will push the lightning server to this PR later on.
Addresses #31
Sorry I haven't taken the time to review this yet. Will do as soon as v0.1.2
gets released.
@sr-gi
ser_macros.rs
is based onlightning::util::ser_macros.rs
but it significantly deviates from it. Is there any good reasoning behind it? Haslightning::util::ser_macros.rs
been updated since you wrote this?
I mainly wanted impl_writeable_msg!
to be exposed (and of course any macros called inside of it) because it handles all the TLV stuff, but that wasn't what LDK team wanted. They wanted to expose more of the utility macros instead (e.g. encode_tlv_stream!
) which then the users could use to craft their things.
Not a big deal too. Exposing the utility macros would do fine. But after going on with BOLT13, some TLV datatypes there weren't covered by LDK's macro system (like vectors and strings) as they don't use it internally nor that it is used in other BOLTs, which led me to make some tweaks to my copy till it deviated that much.
You can also find other parts trimmed out (like check_tlv_order!
, check_missing_tlv!
, and default
macro arm option), this is mainly because these parts weren't used in BOLT13 (and I think not even in other BOLTs, I think they mainly use it for internal messaging stuff).
As a general question, I saw you had some discussion regarding some of the serialization functionality being exposed in lightning. Is that still on track? What parts of this would be exported and how would that affect this part of the codebase?
Exposing them wouldn't help much now, because LDK's macros don't support strings and vector in TLVs, which we would like to have being supported.
@sr-gi
ser_macros.rs
is based onlightning::util::ser_macros.rs
but it significantly deviates from it. Is there any good reasoning behind it? Haslightning::util::ser_macros.rs
been updated since you wrote this?I mainly wanted
impl_writeable_msg!
to be exposed (and of course any macros called inside of it) because it handles all the TLV stuff, but that wasn't what LDK team wanted. They wanted to expose more of the utility macros instead (e.g.encode_tlv_stream!
) which then the users could use to craft their things.Not a big deal too. Exposing the utility macros would do fine. But after going on with BOLT13, some TLV datatypes there weren't covered by LDK's macro system (like vectors and strings) as they don't use it internally nor that it is used in other BOLTs, which led me to make some tweaks to my copy till it deviated that much.
You can also find other parts trimmed out (like
check_tlv_order!
,check_missing_tlv!
, anddefault
macro arm option), this is mainly because these parts weren't used in BOLT13 (and I think not even in other BOLTs, I think they mainly use it for internal messaging stuff).As a general question, I saw you had some discussion regarding some of the serialization functionality being exposed in lightning. Is that still on track? What parts of this would be exported and how would that affect this part of the codebase?
Exposing them wouldn't help much now, because LDK's macros don't support strings and vector in TLVs, which we would like to have being supported.
Right. I'd like to do one last pass on this before accepting it.
Ok, I think this looks good. Not super used to reviewing macros but if something is not correct we will notice it when using the macros to implement the LN API.
LGTM modulo making the pending changes.
Not super used to reviewing macros but if something is not correct we will notice it when using the macros to implement the LN API.
Yeah macros are tough but so cool and powerful. It's a pain that it ends up that you need to test macros with other macros :laughing:.
LGTM modulo making the pending changes.
Will patch make them soon :+1:.