Isolate delivery library#178
Conversation
d468365 to
0d3e561
Compare
0d3e561 to
fdad104
Compare
| enum WakuPayload { | ||
| Base64(String), | ||
| Bytes(Vec<u8>), | ||
| pub fn content_topic_for(delivery_address: &str) -> String { |
There was a problem hiding this comment.
[nitpick] define CHAT_TOPIC_PREFIX first and reuse it here.
There was a problem hiding this comment.
sure.
[!] All topic definitions need to be defined in core to maintain interop, so this won't last long but lets keep it clean.
| @@ -1,325 +1,91 @@ | |||
| //! The embedded logos-delivery transport service. | |||
There was a problem hiding this comment.
[suggestion] The thin wrapper in this crate can be moved to logo-chat, make it a separate crate looks like a overkill.
There was a problem hiding this comment.
I'm not sure I am following here. As I see the history here.
- The trait implementations(DeliveryService, Transport) were moved from a high level app to components so they could be re-used. Componentize logos delivery #148
- In feat: separate embedded logos client #166 you wanted to separate it to its own crate ref: feat: separate embedded logos client #166 (comment)
Now I'm understanding the suggestion is to move the trait implementations to logos-chat which would undo this, by making a transport impl un-accessible again.
I'm worried this is going in circles.
There was a problem hiding this comment.
Now I'm understanding the suggestion is to move the trait implementations to logos-chat
My thoughts on these changes are that we have separated logos-delivery, and clear feature gates in definition and its usage. The trait can be moved to the places that meets this target, make it an extra crate looks not the best in current shape, since:
- the logos-delivery-rust is splited
- logos-chat has been splited
This is a general suggestion, I'm happy to take any path you decide on it.
There was a problem hiding this comment.
Very much agree, that embedded-logos-delivery should not be a crate.
Problem
embedded-logos-deliveryis not usable as a crate, as it assumed it was going to be used as a Libchat::DeliveryService which is not always true. LogosDelivery has no plans to support a rust binding, which makes using delivery for other tools.Solution
Split the embedded logos-delivery transport into two layers:
extensions/logos-delivery-rust/— owns everything tied to the native node: the raw FFI (sys), the safe synchronous FFI wrapper (wrapper::LogosNodeCtx), and a generic, transport-agnostic ThreadedDeliveryWrapper that runs the node on a dedicated thread. It re-exports P2pConfig, DeliveryError, WakuEvent, and ReceivedMessage.Key changes