Merge pull request #154 from greatest-ape/work-2023-10-30

improve ws protocol struct naming and documentation; update TODO

TODO.md

@@ -2,22 +2,22 @@
 
 ## High priority
 
-* udp uring
+* aquatic_ws
-  * miri
+  * Store OfferId that a peer has sent out and only allow answers matching
-  * thiserror?
+    them to be sent? HashMap<(OfferId, PeerId), ValidUntil> could work, but
-  * CI
+    not if peers reuse offer ids
-  * uring load test?
+  * Validate SDP data
-* More non-CI integration tests?
-* Non-trivial dependency updates
-  * toml v0.7
-  * syn v2.0
 
 ## Medium priority
 
 * stagger cleaning tasks?
 * Run cargo-fuzz on protocol crates
 
-* udp: support link to arbitrary homepage as well as embedded tracker URL in statistics page
+* udp 
+  * support link to arbitrary homepage as well as embedded tracker URL in statistics page
+  * Non-trivial dependency updates
+    * toml v0.7
+    * syn v2.0
 
 * quit whole program if any thread panics
   * But it would be nice not to panic in workers, but to return errors instead.
@@ -47,6 +47,11 @@
 ## Low priority
 
 * aquatic_udp
+  * udp uring
+    * miri
+    * thiserror?
+    * CI
+    * uring load test?
   * what poll event capacity is actually needed?
   * load test
       * move additional request sending to for each received response, maybe
@@ -54,6 +59,8 @@
 
 * aquatic_ws
   * large amount of temporary allocations in serialize_20_bytes, pretty many in deserialize_20_bytes
+    * 20 byte parsing: consider using something like ArrayString<80> to avoid
+      heap allocations
 
 # Not important
 
@@ -65,10 +72,6 @@
     * 'left' optional in magnet requests? Probably not. Transmission sends huge
       positive number.
 
-* aquatic_ws
-  * write new version of extract_response_peers which checks for equality with
-    peer sending request???
-
 # Don't do
 
 * general: PGO didn't seem to help way back

crates/ws/src/workers/socket.rs

@@ -775,7 +775,7 @@ impl<S: futures::AsyncRead + futures::AsyncWrite + Unpin> ConnectionReader<S> {
 
                 for (consumer_index, info_hashes) in info_hashes_by_worker {
                     let in_message = InMessage::ScrapeRequest(ScrapeRequest {
-                        action: ScrapeAction,
+                        action: ScrapeAction::Scrape,
                         info_hashes: Some(ScrapeRequestInfoHashes::Multiple(info_hashes)),
                     });
 
@@ -877,7 +877,7 @@ impl<S: futures::AsyncRead + futures::AsyncWrite + Unpin> ConnectionWriter<S> {
                             slab.shrink_to_fit();
 
                             OutMessage::ScrapeResponse(ScrapeResponse {
-                                action: ScrapeAction,
+                                action: ScrapeAction::Scrape,
                                 files: pending.stats,
                             })
                         };
@@ -906,8 +906,8 @@ impl<S: futures::AsyncRead + futures::AsyncWrite + Unpin> ConnectionWriter<S> {
                 #[cfg(feature = "metrics")]
                 {
                     let out_message_type = match &out_message {
-                        OutMessage::Offer(_) => "offer",
+                        OutMessage::OfferOutMessage(_) => "offer",
-                        OutMessage::Answer(_) => "offer_answer",
+                        OutMessage::AnswerOutMessage(_) => "offer_answer",
                         OutMessage::AnnounceResponse(_) => "announce",
                         OutMessage::ScrapeResponse(_) => "scrape",
                         OutMessage::ErrorResponse(_) => "error",

crates/ws/src/workers/swarm.rs

@@ -443,8 +443,8 @@ fn handle_announce_request(
         );
 
         for (offer, offer_receiver) in offers.into_iter().zip(offer_receivers) {
-            let middleman_offer = MiddlemanOfferToPeer {
+            let offer_out_message = OfferOutMessage {
-                action: AnnounceAction,
+                action: AnnounceAction::Announce,
                 info_hash: request.info_hash,
                 peer_id: request.peer_id,
                 offer: offer.offer,
@@ -457,18 +457,20 @@ fn handle_announce_request(
                 pending_scrape_id: None,
             };
 
-            out_messages.push((meta, OutMessage::Offer(middleman_offer)));
+            out_messages.push((meta, OutMessage::OfferOutMessage(offer_out_message)));
             ::log::trace!("sending middleman offer to {:?}", meta);
         }
     }
 
     // If peer sent answer, send it on to relevant peer
-    if let (Some(answer), Some(answer_receiver_id), Some(offer_id)) =
+    if let (Some(answer), Some(answer_receiver_id), Some(offer_id)) = (
-        (request.answer, request.to_peer_id, request.offer_id)
+        request.answer,
-    {
+        request.answer_to_peer_id,
+        request.answer_offer_id,
+    ) {
         if let Some(answer_receiver) = torrent_data.peers.get(&answer_receiver_id) {
-            let middleman_answer = MiddlemanAnswerToPeer {
+            let answer_out_message = AnswerOutMessage {
-                action: AnnounceAction,
+                action: AnnounceAction::Announce,
                 peer_id: request.peer_id,
                 info_hash: request.info_hash,
                 answer,
@@ -481,13 +483,13 @@ fn handle_announce_request(
                 pending_scrape_id: None,
             };
 
-            out_messages.push((meta, OutMessage::Answer(middleman_answer)));
+            out_messages.push((meta, OutMessage::AnswerOutMessage(answer_out_message)));
             ::log::trace!("sending middleman answer to {:?}", meta);
         }
     }
 
     let out_message = OutMessage::AnnounceResponse(AnnounceResponse {
-        action: AnnounceAction,
+        action: AnnounceAction::Announce,
         info_hash: request.info_hash,
         complete: torrent_data.num_seeders,
         incomplete: torrent_data.num_leechers(),
@@ -513,7 +515,7 @@ fn handle_scrape_request(
     let num_to_take = info_hashes.len().min(config.protocol.max_scrape_torrents);
 
     let mut out_message = ScrapeResponse {
-        action: ScrapeAction,
+        action: ScrapeAction::Scrape,
         files: HashMap::with_capacity(num_to_take),
     };
 

crates/ws_load_test/src/network.rs

@@ -6,7 +6,7 @@ use std::{
     time::Duration,
 };
 
-use aquatic_ws_protocol::{InMessage, JsonValue, OfferId, OutMessage, PeerId};
+use aquatic_ws_protocol::{InMessage, OfferId, OutMessage, PeerId, RtcAnswer, RtcAnswerType};
 use async_tungstenite::{client_async, WebSocketStream};
 use futures::{SinkExt, StreamExt};
 use futures_rustls::{client::TlsStream, TlsConnector};
@@ -131,11 +131,12 @@ impl Connection {
                 // the request an offer answer
                 let request = if let InMessage::AnnounceRequest(mut r) = request {
                     if let Some((peer_id, offer_id)) = self.send_answer {
-                        r.to_peer_id = Some(peer_id);
+                        r.answer_to_peer_id = Some(peer_id);
-                        r.offer_id = Some(offer_id);
+                        r.answer_offer_id = Some(offer_id);
-                        r.answer = Some(JsonValue(::serde_json::json!(
+                        r.answer = Some(RtcAnswer {
-                            {"sdp": "abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-"}
+                            t: RtcAnswerType::Answer,
-                        )));
+                            sdp: "abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-".into()
+                        });
                         r.event = None;
                         r.offers = None;
                     }
@@ -182,7 +183,7 @@ impl Connection {
         };
 
         match OutMessage::from_ws_message(message) {
-            Ok(OutMessage::Offer(offer)) => {
+            Ok(OutMessage::OfferOutMessage(offer)) => {
                 self.load_test_state
                     .statistics
                     .responses_offer
@@ -192,7 +193,7 @@ impl Connection {
 
                 self.can_send = true;
             }
-            Ok(OutMessage::Answer(_)) => {
+            Ok(OutMessage::AnswerOutMessage(_)) => {
                 self.load_test_state
                     .statistics
                     .responses_answer

crates/ws_load_test/src/utils.rs

@@ -50,14 +50,15 @@ fn create_announce_request(
     for _ in 0..config.torrents.offers_per_request {
         offers.push(AnnounceRequestOffer {
             offer_id: OfferId(rng.gen()),
-            offer: JsonValue(::serde_json::json!(
+            offer: RtcOffer {
-                {"sdp": "abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-"}
+                t: RtcOfferType::Offer,
-            )),
+                sdp: "abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-abcdefg-".into()
+            },
         })
     }
 
     InMessage::AnnounceRequest(AnnounceRequest {
-        action: AnnounceAction,
+        action: AnnounceAction::Announce,
         info_hash: state.info_hashes[info_hash_index],
         peer_id,
         bytes_left: Some(bytes_left),
@@ -65,8 +66,8 @@ fn create_announce_request(
         numwant: Some(offers.len()),
         offers: Some(offers),
         answer: None,
-        to_peer_id: None,
+        answer_to_peer_id: None,
-        offer_id: None,
+        answer_offer_id: None,
     })
 }
 
@@ -81,7 +82,7 @@ fn create_scrape_request(config: &Config, state: &LoadTestState, rng: &mut impl
     }
 
     InMessage::ScrapeRequest(ScrapeRequest {
-        action: ScrapeAction,
+        action: ScrapeAction::Scrape,
         info_hashes: Some(ScrapeRequestInfoHashes::Multiple(scrape_hashes)),
     })
 }

crates/ws_protocol/benches/bench_deserialize_announce_request.rs

@@ -15,7 +15,10 @@ pub fn bench(c: &mut Criterion) {
             offer_id.0[i] = i as u8;
 
             AnnounceRequestOffer {
-                offer: JsonValue(::serde_json::json!({ "sdp": "abcdef" })),
+                offer: RtcOffer {
+                    t: RtcOfferType::Offer,
+                    sdp: "abcdef".into(),
+                },
                 offer_id,
             }
         })
@@ -23,16 +26,19 @@ pub fn bench(c: &mut Criterion) {
     let offers_len = offers.len();
 
     let request = InMessage::AnnounceRequest(AnnounceRequest {
-        action: AnnounceAction,
+        action: AnnounceAction::Announce,
         info_hash,
         peer_id,
         bytes_left: Some(2),
         event: Some(AnnounceEvent::Started),
         offers: Some(offers),
         numwant: Some(offers_len),
-        answer: Some(JsonValue(::serde_json::json!({ "sdp": "abcdef" }))),
+        answer: Some(RtcAnswer {
-        to_peer_id: Some(peer_id),
+            t: RtcAnswerType::Answer,
-        offer_id: Some(OfferId(info_hash.0)),
+            sdp: "abcdef".into(),
+        }),
+        answer_to_peer_id: Some(peer_id),
+        answer_offer_id: Some(OfferId(info_hash.0)),
     });
 
     let ws_message = request.to_ws_message();

crates/ws_protocol/src/common.rs

@@ -30,93 +30,54 @@ pub struct OfferId(
     pub [u8; 20],
 );
 
-/// Some kind of nested structure from https://www.npmjs.com/package/simple-peer
+/// Serializes to and deserializes from "announce"
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-#[serde(transparent)]
+#[serde(rename_all = "lowercase")]
-pub struct JsonValue(pub ::serde_json::Value);
+pub enum AnnounceAction {
-
+    Announce,
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub struct AnnounceAction;
-
-impl Serialize for AnnounceAction {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: Serializer,
-    {
-        serializer.serialize_str("announce")
-    }
 }
 
-impl<'de> Deserialize<'de> for AnnounceAction {
+/// Serializes to and deserializes from "scrape"
-    fn deserialize<D>(deserializer: D) -> Result<AnnounceAction, D::Error>
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-    where
+#[serde(rename_all = "lowercase")]
-        D: Deserializer<'de>,
+pub enum ScrapeAction {
-    {
+    Scrape,
-        deserializer.deserialize_str(AnnounceActionVisitor)
-    }
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+/// Serializes to and deserializes from "offer"
-pub struct ScrapeAction;
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-
+#[serde(rename_all = "lowercase")]
-impl Serialize for ScrapeAction {
+pub enum RtcOfferType {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    Offer,
-    where
-        S: Serializer,
-    {
-        serializer.serialize_str("scrape")
-    }
 }
 
-impl<'de> Deserialize<'de> for ScrapeAction {
+/// Serializes to and deserializes from "answer"
-    fn deserialize<D>(deserializer: D) -> Result<ScrapeAction, D::Error>
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-    where
+#[serde(rename_all = "lowercase")]
-        D: Deserializer<'de>,
+pub enum RtcAnswerType {
-    {
+    Answer,
-        deserializer.deserialize_str(ScrapeActionVisitor)
-    }
 }
 
-pub struct AnnounceActionVisitor;
+/// Nested structure with SDP offer from https://www.npmjs.com/package/simple-peer
-
+///
-impl<'de> Visitor<'de> for AnnounceActionVisitor {
+/// Created using https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/createOffer
-    type Value = AnnounceAction;
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-
+pub struct RtcOffer {
-    fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
+    /// Always "offer"
-        formatter.write_str("string with value 'announce'")
+    #[serde(rename = "type")]
+    pub t: RtcOfferType,
+    pub sdp: String,
 }
 
-    fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+/// Nested structure with SDP answer from https://www.npmjs.com/package/simple-peer
-    where
+///
-        E: ::serde::de::Error,
+/// Created using https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/createAnswer
-    {
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-        if v == "announce" {
+pub struct RtcAnswer {
-            Ok(AnnounceAction)
+    /// Always "answer"
-        } else {
+    #[serde(rename = "type")]
-            Err(E::custom("value is not 'announce'"))
+    pub t: RtcAnswerType,
-        }
+    pub sdp: String,
-    }
-}
-
-pub struct ScrapeActionVisitor;
-
-impl<'de> Visitor<'de> for ScrapeActionVisitor {
-    type Value = ScrapeAction;
-
-    fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
-        formatter.write_str("string with value 'scrape'")
-    }
-
-    fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
-    where
-        E: ::serde::de::Error,
-    {
-        if v == "scrape" {
-            Ok(ScrapeAction)
-        } else {
-            Err(E::custom("value is not 'scrape'"))
-        }
-    }
 }
 
 fn serialize_20_bytes<S>(data: &[u8; 20], serializer: S) -> Result<S::Ok, S::Error>

crates/ws_protocol/src/incoming/announce.rs

@@ -0,0 +1,80 @@
+use serde::{Deserialize, Serialize};
+
+use crate::common::*;
+
+/// Announce request
+///
+/// Can optionally contain:
+/// - A number of WebRTC offers to be sent on to other peers. In this case,
+///   fields 'offers' and 'numwant' are set
+/// - An answer to a WebRTC offer from another peer. In this case, fields
+///   'answer', 'to_peer_id' and 'offer_id' are set.
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct AnnounceRequest {
+    /// Always "announce"
+    pub action: AnnounceAction,
+    pub info_hash: InfoHash,
+    pub peer_id: PeerId,
+    /// Bytes left
+    ///
+    /// Just called "left" in protocol. Is set to None in some cases, such as
+    /// when opening a magnet link
+    #[serde(rename = "left")]
+    pub bytes_left: Option<usize>,
+    /// Can be empty. Then, default is "update"
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub event: Option<AnnounceEvent>,
+
+    /// WebRTC offers (with offer id's) that peer wants sent on to random other peers
+    ///
+    /// Notes from reference implementation:
+    /// - Only when this is an array offers are sent to other peers
+    /// - Length of this is number of peers wanted?
+    /// - Max length of this is 10 in reference client code
+    /// - Not sent when announce event is stopped or completed
+    pub offers: Option<Vec<AnnounceRequestOffer>>,
+    /// Number of peers wanted
+    ///
+    /// Notes from reference implementation:
+    /// - Seems to only get sent by client when sending offers, and is also
+    ///   same as length of offers vector (or at least never smaller)
+    /// - Max length of this is 10 in reference client code
+    /// - Could probably be ignored, `offers.len()` should provide needed info
+    pub numwant: Option<usize>,
+
+    /// WebRTC answer to previous offer from other peer, to be passed on to it
+    ///
+    /// Notes from reference implementation:
+    /// - If empty, send response before sending offers (or possibly "skip
+    ///   sending update back"?)
+    /// - Else, send AnswerOutMessage to peer with "to_peer_id" as peer_id
+    pub answer: Option<RtcAnswer>,
+    /// Which peer to send answer to
+    #[serde(rename = "to_peer_id")]
+    pub answer_to_peer_id: Option<PeerId>,
+    /// OfferID of offer this is an answer to
+    #[serde(rename = "offer_id")]
+    pub answer_offer_id: Option<OfferId>,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(rename_all = "lowercase")]
+pub enum AnnounceEvent {
+    Started,
+    Stopped,
+    Completed,
+    Update,
+}
+
+impl Default for AnnounceEvent {
+    fn default() -> Self {
+        Self::Update
+    }
+}
+
+/// Element of AnnounceRequest.offers
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct AnnounceRequestOffer {
+    pub offer: RtcOffer,
+    pub offer_id: OfferId,
+}

crates/ws_protocol/src/incoming/scrape.rs

@@ -2,6 +2,19 @@ use serde::{Deserialize, Serialize};
 
 use crate::common::*;
 
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct ScrapeRequest {
+    /// Always "scrape"
+    pub action: ScrapeAction,
+    /// Info hash or info hashes
+    ///
+    /// Notes from reference implementation:
+    /// - If omitted, scrape for all torrents, apparently
+    /// - Accepts a single info hash or an array of info hashes
+    #[serde(rename = "info_hash")]
+    pub info_hashes: Option<ScrapeRequestInfoHashes>,
+}
+
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(untagged)]
 pub enum ScrapeRequestInfoHashes {
@@ -17,13 +30,3 @@ impl ScrapeRequestInfoHashes {
         }
     }
 }
-
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-pub struct ScrapeRequest {
-    pub action: ScrapeAction,
-    // If omitted, scrape for all torrents, apparently
-    // There is some kind of parsing here too which accepts a single info hash
-    // and puts it into a vector
-    #[serde(rename = "info_hash")]
-    pub info_hashes: Option<ScrapeRequestInfoHashes>,
-}

crates/ws_protocol/src/lib.rs

@@ -11,12 +11,12 @@
 //! - Peer sends scrape request and receives scrape response
 
 pub mod common;
-pub mod request;
+pub mod incoming;
-pub mod response;
+pub mod outgoing;
 
 pub use common::*;
-pub use request::*;
+pub use incoming::*;
-pub use response::*;
+pub use outgoing::*;
 
 #[cfg(test)]
 mod tests {
@@ -35,8 +35,17 @@ mod tests {
         bytes
     }
 
-    fn sdp_json_value() -> JsonValue {
+    fn rtc_offer() -> RtcOffer {
-        JsonValue(::serde_json::json!({ "sdp": "test" }))
+        RtcOffer {
+            t: RtcOfferType::Offer,
+            sdp: "test".into(),
+        }
+    }
+    fn rtc_answer() -> RtcAnswer {
+        RtcAnswer {
+            t: RtcAnswerType::Answer,
+            sdp: "test".into(),
+        }
     }
 
     impl Arbitrary for InfoHash {
@@ -68,26 +77,26 @@ mod tests {
         }
     }
 
-    impl Arbitrary for MiddlemanOfferToPeer {
+    impl Arbitrary for OfferOutMessage {
         fn arbitrary(g: &mut quickcheck::Gen) -> Self {
             Self {
-                action: AnnounceAction,
+                action: AnnounceAction::Announce,
                 peer_id: Arbitrary::arbitrary(g),
                 info_hash: Arbitrary::arbitrary(g),
                 offer_id: Arbitrary::arbitrary(g),
-                offer: sdp_json_value(),
+                offer: rtc_offer(),
             }
         }
     }
 
-    impl Arbitrary for MiddlemanAnswerToPeer {
+    impl Arbitrary for AnswerOutMessage {
         fn arbitrary(g: &mut quickcheck::Gen) -> Self {
             Self {
-                action: AnnounceAction,
+                action: AnnounceAction::Announce,
                 peer_id: Arbitrary::arbitrary(g),
                 info_hash: Arbitrary::arbitrary(g),
                 offer_id: Arbitrary::arbitrary(g),
-                answer: sdp_json_value(),
+                answer: rtc_answer(),
             }
         }
     }
@@ -96,7 +105,7 @@ mod tests {
         fn arbitrary(g: &mut quickcheck::Gen) -> Self {
             Self {
                 offer_id: Arbitrary::arbitrary(g),
-                offer: sdp_json_value(),
+                offer: rtc_offer(),
             }
         }
     }
@@ -106,7 +115,7 @@ mod tests {
             let has_offers_or_answer_or_neither: Option<bool> = Arbitrary::arbitrary(g);
 
             let mut offers: Option<Vec<AnnounceRequestOffer>> = None;
-            let mut answer: Option<JsonValue> = None;
+            let mut answer: Option<RtcAnswer> = None;
             let mut to_peer_id: Option<PeerId> = None;
             let mut offer_id: Option<OfferId> = None;
 
@@ -115,7 +124,7 @@ mod tests {
                     offers = Some(Arbitrary::arbitrary(g));
                 }
                 Some(false) => {
-                    answer = Some(sdp_json_value());
+                    answer = Some(rtc_answer());
                     to_peer_id = Some(Arbitrary::arbitrary(g));
                     offer_id = Some(Arbitrary::arbitrary(g));
                 }
@@ -125,7 +134,7 @@ mod tests {
             let numwant = offers.as_ref().map(|offers| offers.len());
 
             Self {
-                action: AnnounceAction,
+                action: AnnounceAction::Announce,
                 info_hash: Arbitrary::arbitrary(g),
                 peer_id: Arbitrary::arbitrary(g),
                 bytes_left: Arbitrary::arbitrary(g),
@@ -133,8 +142,8 @@ mod tests {
                 offers,
                 numwant,
                 answer,
-                to_peer_id,
+                answer_to_peer_id: to_peer_id,
-                offer_id,
+                answer_offer_id: offer_id,
             }
         }
     }
@@ -142,7 +151,7 @@ mod tests {
     impl Arbitrary for AnnounceResponse {
         fn arbitrary(g: &mut quickcheck::Gen) -> Self {
             Self {
-                action: AnnounceAction,
+                action: AnnounceAction::Announce,
                 info_hash: Arbitrary::arbitrary(g),
                 complete: Arbitrary::arbitrary(g),
                 incomplete: Arbitrary::arbitrary(g),
@@ -154,7 +163,7 @@ mod tests {
     impl Arbitrary for ScrapeRequest {
         fn arbitrary(g: &mut quickcheck::Gen) -> Self {
             Self {
-                action: ScrapeAction,
+                action: ScrapeAction::Scrape,
                 info_hashes: Arbitrary::arbitrary(g),
             }
         }
@@ -185,7 +194,7 @@ mod tests {
             let files: Vec<(InfoHash, ScrapeStatistics)> = Arbitrary::arbitrary(g);
 
             Self {
-                action: ScrapeAction,
+                action: ScrapeAction::Scrape,
                 files: files.into_iter().collect(),
             }
         }
@@ -206,8 +215,8 @@ mod tests {
             match (Arbitrary::arbitrary(g), Arbitrary::arbitrary(g)) {
                 (false, false) => Self::AnnounceResponse(Arbitrary::arbitrary(g)),
                 (true, false) => Self::ScrapeResponse(Arbitrary::arbitrary(g)),
-                (false, true) => Self::Offer(Arbitrary::arbitrary(g)),
+                (false, true) => Self::OfferOutMessage(Arbitrary::arbitrary(g)),
-                (true, true) => Self::Answer(Arbitrary::arbitrary(g)),
+                (true, true) => Self::AnswerOutMessage(Arbitrary::arbitrary(g)),
             }
         }
     }
@@ -268,7 +277,7 @@ mod tests {
         ]);
 
         let expected = ScrapeRequest {
-            action: ScrapeAction,
+            action: ScrapeAction::Scrape,
             info_hashes: Some(info_hashes),
         };
 
@@ -291,7 +300,7 @@ mod tests {
             ScrapeRequestInfoHashes::Single(info_hash_from_bytes(b"aaaabbbbccccddddeeee"));
 
         let expected = ScrapeRequest {
-            action: ScrapeAction,
+            action: ScrapeAction::Scrape,
             info_hashes: Some(info_hashes),
         };
 
@@ -321,7 +330,7 @@ mod tests {
         };
 
         let expected = ScrapeRequest {
-            action: ScrapeAction,
+            action: ScrapeAction::Scrape,
             info_hashes: None,
         };
 
@@ -340,7 +349,7 @@ mod tests {
         };
 
         let expected = ScrapeRequest {
-            action: ScrapeAction,
+            action: ScrapeAction::Scrape,
             info_hashes: None,
         };
 

crates/ws_protocol/src/outgoing/announce.rs

@@ -2,6 +2,7 @@ use serde::{Deserialize, Serialize};
 
 use crate::common::*;
 
+/// Plain response to an AnnounceRequest
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub struct AnnounceResponse {
     pub action: AnnounceAction,

crates/ws_protocol/src/outgoing/answer.rs

@@ -2,15 +2,16 @@ use serde::{Deserialize, Serialize};
 
 use crate::common::*;
 
-/// If announce request has answer = true, send this to peer with
+/// Message sent to peer when other peer has replied to its WebRTC offer
-/// peer id == "to_peer_id" field
+///
-/// Action field should be 'announce'
+/// Sent if fields answer, to_peer_id and offer_id are set in announce request
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-pub struct MiddlemanAnswerToPeer {
+pub struct AnswerOutMessage {
+    /// Always "announce"
     pub action: AnnounceAction,
     /// Note: if equal to client peer_id, client ignores answer
     pub peer_id: PeerId,
     pub info_hash: InfoHash,
-    pub answer: JsonValue,
+    pub answer: RtcAnswer,
     pub offer_id: OfferId,
 }

crates/ws_protocol/src/outgoing/error.rs

@@ -4,13 +4,6 @@ use serde::{Deserialize, Serialize};
 
 use crate::common::*;
 
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-#[serde(rename_all = "lowercase")]
-pub enum ErrorResponseAction {
-    Announce,
-    Scrape,
-}
-
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub struct ErrorResponse {
     #[serde(rename = "failure reason")]
@@ -22,3 +15,10 @@ pub struct ErrorResponse {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub info_hash: Option<InfoHash>,
 }
+
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(rename_all = "lowercase")]
+pub enum ErrorResponseAction {
+    Announce,
+    Scrape,
+}

crates/ws_protocol/src/outgoing/mod.rs

@@ -16,8 +16,8 @@ pub use scrape::*;
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(untagged)]
 pub enum OutMessage {
-    Offer(MiddlemanOfferToPeer),
+    OfferOutMessage(OfferOutMessage),
-    Answer(MiddlemanAnswerToPeer),
+    AnswerOutMessage(AnswerOutMessage),
     AnnounceResponse(AnnounceResponse),
     ScrapeResponse(ScrapeResponse),
     ErrorResponse(ErrorResponse),

crates/ws_protocol/src/outgoing/offer.rs

@@ -2,18 +2,21 @@ use serde::{Deserialize, Serialize};
 
 use crate::common::*;
 
-/// Apparently, these are sent to a number of peers when they are set
+/// Message sent to peer when other peer wants to initiate a WebRTC connection
-/// in an AnnounceRequest
+///
-/// action = "announce"
+/// One is sent for each offer in an announce request
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-pub struct MiddlemanOfferToPeer {
+pub struct OfferOutMessage {
+    /// Always "announce"
     pub action: AnnounceAction,
     /// Peer id of peer sending offer
-    /// Note: if equal to client peer_id, client ignores offer
+    ///
+    /// Note: if equal to client peer_id, reference client ignores offer
     pub peer_id: PeerId,
+    /// Torrent info hash
     pub info_hash: InfoHash,
     /// Gets copied from AnnounceRequestOffer
-    pub offer: JsonValue,
+    pub offer: RtcOffer,
     /// Gets copied from AnnounceRequestOffer
     pub offer_id: OfferId,
 }

crates/ws_protocol/src/outgoing/scrape.rs

@@ -7,7 +7,7 @@ use crate::common::*;
 pub struct ScrapeResponse {
     pub action: ScrapeAction,
     pub files: HashMap<InfoHash, ScrapeStatistics>,
-    // Looks like `flags` field is ignored in reference client
+    // It looks like `flags` field is ignored in reference client
     // pub flags: HashMap<String, usize>,
 }
 

crates/ws_protocol/src/request/announce.rs

@@ -1,60 +0,0 @@
-use serde::{Deserialize, Serialize};
-
-use crate::common::*;
-
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-#[serde(rename_all = "lowercase")]
-pub enum AnnounceEvent {
-    Started,
-    Stopped,
-    Completed,
-    Update,
-}
-
-impl Default for AnnounceEvent {
-    fn default() -> Self {
-        Self::Update
-    }
-}
-
-/// Element of AnnounceRequest.offers
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-pub struct AnnounceRequestOffer {
-    pub offer: JsonValue,
-    pub offer_id: OfferId,
-}
-
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-pub struct AnnounceRequest {
-    pub action: AnnounceAction,
-    pub info_hash: InfoHash,
-    pub peer_id: PeerId,
-    /// Just called "left" in protocol. Is set to None in some cases, such as
-    /// when opening a magnet link
-    #[serde(rename = "left")]
-    pub bytes_left: Option<usize>,
-    /// Can be empty. Then, default is "update"
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub event: Option<AnnounceEvent>,
-
-    /// Only when this is an array offers are sent to random peers
-    /// Length of this is number of peers wanted?
-    /// Max length of this is 10 in reference client code
-    /// Not sent when announce event is stopped or completed
-    pub offers: Option<Vec<AnnounceRequestOffer>>,
-    /// Seems to only get sent by client when sending offers, and is also same
-    /// as length of offers vector (or at least never less)
-    /// Max length of this is 10 in reference client code
-    /// Could probably be ignored, `offers.len()` should provide needed info
-    pub numwant: Option<usize>,
-
-    /// If empty, send response before sending offers (or possibly "skip sending update back"?)
-    /// Else, send MiddlemanAnswerToPeer to peer with "to_peer_id" as peer_id.
-    /// I think using Option is good, it seems like this isn't always set
-    /// (same as `offers`)
-    pub answer: Option<JsonValue>,
-    /// Likely undefined if !(answer == true)
-    pub to_peer_id: Option<PeerId>,
-    /// Sent if answer is set
-    pub offer_id: Option<OfferId>,
-}