Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Dec 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/pathfinder/src/monitoring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
4 changes: 2 additions & 2 deletions crates/pathfinder/src/state/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion crates/rpc/src/dto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
}
Expand Down
8 changes: 3 additions & 5 deletions crates/rpc/src/dto/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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(
Expand Down
138 changes: 118 additions & 20 deletions crates/rpc/src/dto/primitives.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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")),
}
}
}

Expand All @@ -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")),
}
}
}

Expand Down Expand Up @@ -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")),
}
}
}

Expand All @@ -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")),
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/src/dto/simulation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/src/dto/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
}
2 changes: 1 addition & 1 deletion crates/rpc/src/felt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 3 additions & 5 deletions crates/rpc/src/jsonrpc/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,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")?,
})
})
}
Expand All @@ -471,9 +471,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())?))
}
}

Expand Down
6 changes: 4 additions & 2 deletions crates/rpc/src/jsonrpc/router/subscription.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}))
Expand Down Expand Up @@ -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,
Expand Down
42 changes: 34 additions & 8 deletions crates/rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
Expand Down Expand Up @@ -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"}"#,
Expand All @@ -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}");
}
}

Expand Down
Loading
Loading