RFC: refactoring the inverse websocket for ergonomic use

This PR is based on #6 and #7

Hi! I'm opening this PR as a draft so I can get some early feedback; I've started taking the inverse websocket in a different direction than the spot/linear implementations and wanted to sync with you on this before it went too far off in the weeds. I'm not sure if some of these changes uphold the goals or direction you want to take this project, so I thought I'd ask!

I started consuming the inverse websocket library from an application I'm writing and these changes were helpful in keeping the consuming code readable and fun:

exporting the type of an enum variant (I'm not sure if these words make sense, hopefully I'm close) This allows me to write a function expecting data from a particular response message
```
pub fn initialize_from_snapshot(&mut self, snapshot_message: &OrderBookL2SnapshotMessage) {
    for rung in snapshot_message.data.iter() {
        self.insert_bybit_rung(rung);
    }
}
```
Well... now that I say this, I should probably just pass &Vec<OrderBookItem> and &OrderBookDelta (both already exported). Maybe this case is no longer as important to support, but it did bring the next point to my attention
create a distinction between the response or message and the contained data For example, we had
```
pub struct BaseResponse<'a, D> {
  pub topic: &'a str,
  pub data: D,
}

pub enum PublicResponse<'a> {
  #[serde(borrow)]
  Trade(BaseResponse<Vec<Trade>>),
  // ...
}
```
This naturally had me thinking of the entire BaseResponse<Vec<Trade>> as a Trade (the enum variant) which caused a bit of a jerk when I accessed the data which was a ... Trade. It seemed more natural to refer to the unedited response as a TradeMessage which contains a Vec<Trade>. This rename causes the callback to be a little more verbose but seems to increase clarity everywhere else.
deserializing some raw websocket values into a different type This is the most questionable change, because if the project goal is to be as fast as possible, this may perform more work than is necessary in all cases. Also, there can be some precision issues when changing a string-of-a-number into a f64, but I'm not sure of any other, superior way of dealing with floats in Rust so I figure this conversion is sort of inevitable.

Rather than handling these conversions on the client's side, I moved this logic into the deserializer using a crate called serde-aux. I've started to deserialize Side into an enum as well, which makes the client's logic cleaner and lets me lean on the type system more.
adding tests, currently only of deserialization logic I'm always a bit confused reading the serde documentation, so I wrote tests to prove the deserialization would work as I expected it to. I'm not sure yet how to test every part of this implementation but I'm excited to keep pushing for higher test coverage

That's about it, so I guess my goals for these diffs are

leverage the type system to increase confidence about correctness and improve developer experience through "implementation by cases"
provide a great developer experience by presenting information from Bybit's API in a way that minimizes wtfs-per-minute (Bybit leaves a lot of room for improvement here)

Thanks in advance!

Hi! Thank you for the proposal.

I have read it carefully and here are some comments and responses.

exporting the type of an enum variant (I'm not sure if these words make sense, hopefully I'm close) This allows me to write a function expecting data from a particular response message
  pub fn initialize_from_snapshot(&mut self, snapshot_message: &OrderBookL2SnapshotMessage) {
      for rung in snapshot_message.data.iter() {
          self.insert_bybit_rung(rung);
      }
  }   

We can already do this under current design, though it may have a little inconvinience and confusion as you said. Take bybit::inverse::PublicResponse as an example, it would be destructed to a data wrapper structures like BaseResponseWithTimestamp,Response in the match arms. They are generic structures who's data property is what we are really interested in. If you have a function expecting a particular data, you can pass the whole destructed response to it or pass the response's data which may just be a vector such as Vec<OrderBookItem>. If the vector is not convinient to use, we can wrap it to a structure, says, a OrderBook. So I think it won't be a big problem.

create a distinction between the response or message and the contained data.

Indeed the enum name and the real WebSocket message name which the enum's variants wrap in are a little confusing. The WebSocket message names are took from Bybit docs, where they call a message is a response. So I adopted this naming convention, then I found it's difficult to come up with a good name for the enum, so I finally just name it to PublicResponse , PrivateResponse et cetera. We can refactor this as long as keep the consistency.

deserializing some raw websocket values into a different type

This is essential for a sophisticated library. Currently, the data flow is designed like bellow:

receive WebSocket message data and save to local buffer (in memory) --> deserialize to Rust enum in a zero-copy way (for performance consideration) --> move enum to callback (a closure) the use provided.

No data copied in above steps. So I believe it would be quite fast. It leaves the freedom of how to consume the WebSocket message data to users. For example, for users who concern performance, they just copy come values from specific fields (such as the price and quantity) and leave other data untouched, which is a cheap operation. For users who more interested in the whole message data, they can make the data to be owned then the owned data can be brrowed or moved freely between functions or methods. We can provide some convinient methods for above scenarios. Here is a basic example for demonstrating above idea:

#[derive(Deserialize, Debug)]
pub struct Trade<'a> {
  // ...
}

impl Trade {
  // return a OwnedTrade which all values are owned, so the lifetime bound in original Trade is gone.
  pub to_owned() -> OwnedTrade {}

  // return a f64 price value
  pub get_owned_price() -> f64 {}
}

adding tests, currently only of deserialization logic

This is also very essential for a sophisticated library. I'll keep work on it.

This is my first Rust project and I'm also new to Rust like you. Since you have devoted a lot of energy to this project. Do you accept to be a collaborator. If do, I'll send you an invitation. Glad to wait you reply.

We can already do this under current design, though it may have a little inconvinience and confusion as you said.

Thanks for the pointers! I will keep playing with the options here until I have a better understanding of the different approaches.

We can refactor this as long as keep the consistency.

:+1: I like the name Response, I'll keep that in mind

Currently, the data flow is designed like bellow:

receive WebSocket message data and save to local buffer (in memory) --> deserialize to Rust enum in a zero-copy way (for performance consideration) --> move enum to callback (a closure) the use provided.

No data copied in above steps. So I believe it would be quite fast.

I noticed this, I like how efficient the current design is. This is what made me wonder if any computation to change the response values from one type to another would be welcome in this library!

For example, for users who concern performance, they just copy come values from specific fields (such as the price and quantity) and leave other data untouched, which is a cheap operation. For users who more interested in the whole message data, they can make the data to be owned then the owned data can be brrowed or moved freely between functions or methods. We can provide some convinient methods for above scenarios.

I caution against trying to satisfy many use-cases with a single code base, in the worse case will create a poor library for all use cases and at best we will have a very complicated project. Given the lack of existing Bybit libraries in Rust, I think it would be sufficient for this project to focus on just one of the two following goals:

speed
ease of use

As you mentioned, if we improve the ease of use we will naturally make trade-offs that negatively affect performance. If we put all of our effort into just one of these goals, we will have a compass to help us navigate trade-offs in our implementation.

For my intended use-cases, Rust provides sufficient performance so I don't have an immediate need to optimize my code for extra speed. I find I'm able to accomplish more if I make each library as easy to build on as possible, so I tend to favor ease of use. I've seen that the small population of developers who require better performance would not use such a library, but when performance is this important you end up building almost everything from scratch anyway.

If we were to focus on speed, we'd enjoy getting to a code-complete state more quickly, and could possibly build another library as another layer on top of this blazing-fast one with more developer amenities. I suppose Rust's features could also be used for this but I'm not sure off the top of my head how exactly that would look.

I'm interested to hear more about where you intend on taking this library :smiley:

This is my first Rust project and I'm also new to Rust like you. Since you have devoted a lot of energy to this project. Do you accept to be a collaborator. If do, I'll send you an invitation. Glad to wait you reply.

I would be honored to join as a collaborator! Thank you for offering :grinning:

yufuquant / rust-bybit

RFC: refactoring the inverse websocket for ergonomic use #8