Merge pull request #2392 from eqlabs/t00ts/rpc-remove-serde-derive

feat(rpc): Move away from `serde` in RPC DTO's (in favor of our own `[De]SerializeForVersion`)

crates/pathfinder/src/monitoring.rs

@@ -158,7 +158,7 @@ mod tests {
         let readiness = Arc::new(AtomicBool::new(false));
         let handle = PrometheusBuilder::new().build_recorder().handle();
         let sync_state = Arc::new(SyncState {
-            status: RwLock::new(Syncing::False(false)),
+            status: RwLock::new(Syncing::False),
         });
         let (addr, _) = super::spawn_server(
             ([127, 0, 0, 1], 0),

crates/pathfinder/src/state/sync.rs

@@ -543,7 +543,7 @@ async fn consumer(
 
                 // Update sync status
                 match &mut *state.status.write().await {
-                    Syncing::False(_) => {}
+                    Syncing::False => {}
                     Syncing::Status(status) => {
                         status.current = NumberedBlock::from((block_hash, block_number));
 
@@ -758,7 +758,7 @@ async fn update_sync_status_latest(
         latest_hash = hash;
         let latest = NumberedBlock::from((hash, number));
         match &mut *state.status.write().await {
-            sync_status @ Syncing::False(_) => {
+            sync_status @ Syncing::False => {
                 *sync_status = Syncing::Status(syncing::Status {
                     starting,
                     current: starting,

crates/rpc/src/dto.rs

@@ -30,7 +30,7 @@ pub trait DeserializeForVersion: Sized {
     fn deserialize(value: Value) -> Result<Self, serde_json::Error>;
 }
 
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 pub struct Value {
     data: serde_json::Value,
     pub version: RpcVersion,
@@ -56,6 +56,10 @@ impl Value {
         self.data.is_null()
     }
 
+    pub fn json_value(&self) -> serde_json::Value {
+        self.data.clone()
+    }
+
     pub fn deserialize<T: DeserializeForVersion>(self) -> Result<T, serde_json::Error> {
         T::deserialize(self)
     }

crates/rpc/src/dto/block.rs

@@ -13,7 +13,7 @@ pub struct PendingBlockHeader<'a>(pub &'a starknet_gateway_types::reply::Pending
 impl crate::dto::DeserializeForVersion for pathfinder_common::BlockId {
     fn deserialize(value: super::Value) -> Result<Self, serde_json::Error> {
         if value.is_string() {
-            let value: String = value.deserialize_serde()?;
+            let value: String = value.deserialize()?;
             match value.as_str() {
                 "latest" => Ok(Self::Latest),
                 "pending" => Ok(Self::Pending),
@@ -23,10 +23,8 @@ impl crate::dto::DeserializeForVersion for pathfinder_common::BlockId {
             value.deserialize_map(|value| {
                 if value.contains_key("block_number") {
                     Ok(Self::Number(
-                        pathfinder_common::BlockNumber::new(
-                            value.deserialize_serde("block_number")?,
-                        )
-                        .ok_or_else(|| serde_json::Error::custom("Invalid block number"))?,
+                        pathfinder_common::BlockNumber::new(value.deserialize("block_number")?)
+                            .ok_or_else(|| serde_json::Error::custom("Invalid block number"))?,
                     ))
                 } else if value.contains_key("block_hash") {
                     Ok(Self::Hash(pathfinder_common::BlockHash(

crates/rpc/src/dto/primitives.rs

@@ -1,5 +1,5 @@
 use pathfinder_common::{ContractAddress, L1TransactionHash};
-use primitive_types::H256;
+use primitive_types::{H160, H256};
 use serde::de::Error;
 
 use super::serialize::SerializeForVersion;
@@ -152,14 +152,82 @@ pub mod hex_str {
         Ok(buf)
     }
 }
+impl DeserializeForVersion for u64 {
+    fn deserialize(value: Value) -> Result<Self, serde_json::Error> {
+        match &value.data {
+            serde_json::Value::Number(n) => n
+                .as_u64()
+                .ok_or_else(|| serde_json::Error::custom("invalid u64 value")),
+            _ => Err(serde_json::Error::custom("expected number")),
+        }
+    }
+}
+
+impl DeserializeForVersion for u32 {
+    fn deserialize(value: Value) -> Result<Self, serde_json::Error> {
+        match &value.data {
+            serde_json::Value::Number(n) => n
+                .as_u64()
+                .and_then(|n| u32::try_from(n).ok())
+                .ok_or_else(|| serde_json::Error::custom("value is too large for u32")),
+            _ => Err(serde_json::Error::custom("expected number")),
+        }
+    }
+}
+
+impl DeserializeForVersion for i32 {
+    fn deserialize(value: Value) -> Result<Self, serde_json::Error> {
+        match &value.data {
+            serde_json::Value::Number(n) => n
+                .as_i64()
+                .and_then(|n| i32::try_from(n).ok())
+                .ok_or_else(|| serde_json::Error::custom("value is outside i32 range")),
+            _ => Err(serde_json::Error::custom("expected number")),
+        }
+    }
+}
+
+impl DeserializeForVersion for usize {
+    fn deserialize(value: Value) -> Result<Self, serde_json::Error> {
+        match &value.data {
+            serde_json::Value::Number(n) => n
+                .as_u64()
+                .and_then(|n| usize::try_from(n).ok())
+                .ok_or_else(|| serde_json::Error::custom("value is outside usize range")),
+            _ => Err(serde_json::Error::custom("expected number")),
+        }
+    }
+}
+
+impl DeserializeForVersion for bool {
+    fn deserialize(value: Value) -> Result<Self, serde_json::Error> {
+        match &value.data {
+            serde_json::Value::Bool(b) => Ok(*b),
+            _ => Err(serde_json::Error::custom("expected boolean")),
+        }
+    }
+}
+
+impl DeserializeForVersion for String {
+    fn deserialize(value: Value) -> Result<Self, serde_json::Error> {
+        match &value.data {
+            serde_json::Value::String(s) => Ok(s.clone()),
+            _ => Err(serde_json::Error::custom("expected string")),
+        }
+    }
+}
 
 impl DeserializeForVersion for U64Hex {
     fn deserialize(value: Value) -> Result<Self, serde_json::Error> {
-        let hex_str: String = value.deserialize_serde()?;
-        let bytes = hex_str::bytes_from_hex_str_stripped::<8>(&hex_str).map_err(|e| {
-            serde_json::Error::custom(format!("failed to parse hex string as u64: {}", e))
-        })?;
-        Ok(Self(u64::from_be_bytes(bytes)))
+        match &value.data {
+            serde_json::Value::String(s) => {
+                let bytes = hex_str::bytes_from_hex_str_stripped::<8>(s).map_err(|e| {
+                    serde_json::Error::custom(format!("failed to parse hex string as u64: {}", e))
+                })?;
+                Ok(Self(u64::from_be_bytes(bytes)))
+            }
+            _ => Err(serde_json::Error::custom("expected hex string")),
+        }
     }
 }
 
@@ -185,10 +253,15 @@ impl SerializeForVersion for Felt<'_> {
 
 impl DeserializeForVersion for pathfinder_crypto::Felt {
     fn deserialize(value: Value) -> Result<Self, serde_json::Error> {
-        let hex_str: String = value.deserialize_serde()?;
-        let bytes = hex_str::bytes_from_hex_str_stripped::<32>(&hex_str)
-            .map_err(|e| serde_json::Error::custom(format!("failed to parse hex string: {}", e)))?;
-        Self::from_be_bytes(bytes).map_err(|e| serde_json::Error::custom("felt overflow"))
+        match &value.data {
+            serde_json::Value::String(hex_str) => {
+                let bytes = hex_str::bytes_from_hex_str_stripped::<32>(hex_str).map_err(|e| {
+                    serde_json::Error::custom(format!("failed to parse hex string: {}", e))
+                })?;
+                Self::from_be_bytes(bytes).map_err(|_| serde_json::Error::custom("felt overflow"))
+            }
+            _ => Err(serde_json::Error::custom("expected hex string")),
+        }
     }
 }
 
@@ -225,11 +298,15 @@ impl SerializeForVersion for U128Hex {
 
 impl DeserializeForVersion for U128Hex {
     fn deserialize(value: Value) -> Result<Self, serde_json::Error> {
-        let hex_str: String = value.deserialize_serde()?;
-        let bytes = hex_str::bytes_from_hex_str_stripped::<16>(&hex_str).map_err(|e| {
-            serde_json::Error::custom(format!("failed to parse hex string as u128: {}", e))
-        })?;
-        Ok(Self(u128::from_be_bytes(bytes)))
+        match &value.data {
+            serde_json::Value::String(s) => {
+                let bytes = hex_str::bytes_from_hex_str_stripped::<16>(s).map_err(|e| {
+                    serde_json::Error::custom(format!("failed to parse hex string as u128: {}", e))
+                })?;
+                Ok(Self(u128::from_be_bytes(bytes)))
+            }
+            _ => Err(serde_json::Error::custom("expected hex string")),
+        }
     }
 }
 
@@ -249,11 +326,32 @@ impl SerializeForVersion for U256Hex {
 
 impl DeserializeForVersion for H256 {
     fn deserialize(value: Value) -> Result<Self, serde_json::Error> {
-        let hex_str: String = value.deserialize_serde()?;
-        let bytes = hex_str::bytes_from_hex_str_stripped::<32>(&hex_str).map_err(|e| {
-            serde_json::Error::custom(format!("failed to parse hex string as u256: {}", e))
-        })?;
-        Ok(H256(bytes))
+        match &value.data {
+            serde_json::Value::String(hex_str) => {
+                let bytes = hex_str::bytes_from_hex_str_stripped::<32>(hex_str).map_err(|e| {
+                    serde_json::Error::custom(format!("failed to parse hex string as u256: {}", e))
+                })?;
+                Ok(H256(bytes))
+            }
+            _ => Err(serde_json::Error::custom("expected hex string")),
+        }
+    }
+}
+
+impl DeserializeForVersion for pathfinder_common::EthereumAddress {
+    fn deserialize(value: Value) -> Result<Self, serde_json::Error> {
+        match &value.data {
+            serde_json::Value::String(hex_str) => {
+                let bytes = hex_str::bytes_from_hex_str_stripped::<20>(hex_str).map_err(|e| {
+                    serde_json::Error::custom(format!(
+                        "failed to parse hex string as ethereum address: {}",
+                        e
+                    ))
+                })?;
+                Ok(Self(H160::from(bytes)))
+            }
+            _ => Err(serde_json::Error::custom("expected hex string")),
+        }
     }
 }
 

crates/rpc/src/dto/simulation.rs

@@ -562,7 +562,7 @@ pub enum SimulationFlag {
 
 impl crate::dto::DeserializeForVersion for SimulationFlag {
     fn deserialize(value: crate::dto::Value) -> Result<Self, serde_json::Error> {
-        let value: String = value.deserialize_serde()?;
+        let value: String = value.deserialize()?;
         match value.as_str() {
             "SKIP_FEE_CHARGE" => Ok(Self::SkipFeeCharge),
             "SKIP_VALIDATE" => Ok(Self::SkipValidate),

crates/rpc/src/dto/transaction.rs

@@ -319,7 +319,7 @@ impl SerializeForVersion for DataAvailabilityMode<'_> {
 
 impl DeserializeForVersion for pathfinder_common::TransactionIndex {
     fn deserialize(value: dto::Value) -> Result<Self, serde_json::Error> {
-        let idx = value.deserialize_serde()?;
+        let idx = value.deserialize()?;
         Self::new(idx).ok_or_else(|| serde_json::Error::custom("Invalid transaction index"))
     }
 }

crates/rpc/src/felt.rs

@@ -81,7 +81,7 @@ impl From<RpcFelt> for Felt {
 /// This can be easily accomplished by marking a field with `#[serde_as(as =
 /// "RpcFelt251")]`.
 #[derive(serde::Serialize)]
-pub struct RpcFelt251(RpcFelt);
+pub struct RpcFelt251(pub RpcFelt);
 
 mod serialization {
     //! Blanket [serde::Serialize] and [serde_with::SerializeAs] implementations

crates/rpc/src/jsonrpc/router.rs

@@ -453,8 +453,8 @@ mod tests {
                 fn deserialize(value: crate::dto::Value) -> Result<Self, serde_json::Error> {
                     value.deserialize_map(|value| {
                         Ok(Self {
-                            minuend: value.deserialize_serde("minuend")?,
-                            subtrahend: value.deserialize_serde("subtrahend")?,
+                            minuend: value.deserialize("minuend")?,
+                            subtrahend: value.deserialize("subtrahend")?,
                         })
                     })
                 }
@@ -469,9 +469,7 @@ mod tests {
 
             impl DeserializeForVersion for SumInput {
                 fn deserialize(value: crate::dto::Value) -> Result<Self, serde_json::Error> {
-                    Ok(Self(
-                        value.deserialize_array(|value| value.deserialize_serde())?,
-                    ))
+                    Ok(Self(value.deserialize_array(|value| value.deserialize())?))
                 }
             }
 

crates/rpc/src/jsonrpc/router/subscription.rs

@@ -659,7 +659,9 @@ async fn handle_request(
                 panic!("subscription id overflow");
             }
             Ok(Some(RpcResponse {
-                output: Ok(serde_json::to_value(subscription_id).unwrap()),
+                output: Ok(subscription_id
+                    .serialize(serialize::Serializer::new(state.version))
+                    .unwrap()),
                 id: req_id,
                 version: state.version,
             }))
@@ -1013,7 +1015,7 @@ mod tests {
             execution_storage: StorageBuilder::in_memory().unwrap(),
             pending_data: PendingWatcher::new(pending_data),
             sync_status: SyncState {
-                status: Syncing::False(false).into(),
+                status: Syncing::False.into(),
             }
             .into(),
             chain_id: ChainId::MAINNET,

crates/rpc/src/lib.rs

@@ -225,12 +225,12 @@ pub struct SyncState {
 impl Default for SyncState {
     fn default() -> Self {
         Self {
-            status: RwLock::new(Syncing::False(false)),
+            status: RwLock::new(Syncing::False),
         }
     }
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Deserialize)]
 pub(crate) struct SubscriptionId(pub u32);
 
 impl SubscriptionId {
@@ -240,6 +240,22 @@ impl SubscriptionId {
     }
 }
 
+impl crate::dto::serialize::SerializeForVersion for SubscriptionId {
+    fn serialize(
+        &self,
+        serializer: crate::dto::serialize::Serializer,
+    ) -> Result<crate::dto::serialize::Ok, crate::dto::serialize::Error> {
+        serializer.serialize_u64(self.0 as u64)
+    }
+}
+
+impl crate::dto::DeserializeForVersion for SubscriptionId {
+    fn deserialize(value: crate::dto::Value) -> Result<Self, serde_json::Error> {
+        let id: u32 = value.deserialize()?;
+        Ok(Self(id))
+    }
+}
+
 #[cfg(test)]
 pub mod test_utils {
     use std::collections::HashMap;
@@ -816,18 +832,19 @@ pub mod test_utils {
 
 #[cfg(test)]
 mod tests {
+    use dto::serialize::SerializeForVersion;
+    use dto::DeserializeForVersion;
     use serde_json::json;
 
     use super::*;
+    use crate::dto::serialize::Serializer;
 
     #[test]
     fn roundtrip_syncing() {
         use crate::types::syncing::{NumberedBlock, Status, Syncing};
 
         let examples = [
-            (line!(), "false", Syncing::False(false)),
-            // this shouldn't exist but it exists now
-            (line!(), "true", Syncing::False(true)),
+            (line!(), "false", Syncing::False),
             (
                 line!(),
                 r#"{"starting_block_hash":"0xa","starting_block_num":"0x1","current_block_hash":"0xb","current_block_num":"0x2","highest_block_hash":"0xc","highest_block_num":"0x3"}"#,
@@ -840,11 +857,20 @@ mod tests {
         ];
 
         for (line, input, expected) in examples {
-            let parsed = serde_json::from_str::<Syncing>(input).unwrap();
-            let output = serde_json::to_string(&parsed).unwrap();
+            let parsed = Syncing::deserialize(crate::dto::Value::new(
+                serde_json::from_str(input).unwrap(),
+                RpcVersion::V07,
+            ))
+            .unwrap();
+            let output = parsed.serialize(Serializer::new(RpcVersion::V07)).unwrap();
 
             assert_eq!(parsed, expected, "example from line {line}");
-            assert_eq!(&output, input, "example from line {line}");
+
+            // Compare parsed JSON values instead of strings
+            let output_value: serde_json::Value =
+                serde_json::from_str(&output.to_string()).unwrap();
+            let input_value: serde_json::Value = serde_json::from_str(input).unwrap();
+            assert_eq!(output_value, input_value, "example from line {line}");
         }
     }