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(torii-core): json value for ty for efficient json ser #2730

Merged
merged 10 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/dojo/types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ starknet-crypto.workspace = true
strum.workspace = true
strum_macros.workspace = true
thiserror.workspace = true
indexmap.workspace = true
194 changes: 194 additions & 0 deletions crates/dojo/types/src/schema.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
use std::any::type_name;
use std::str::FromStr;

use cainome::cairo_serde::{ByteArray, CairoSerde};
use crypto_bigint::{Encoding, U256};
use indexmap::IndexMap;
use itertools::Itertools;
use num_traits::ToPrimitive;
use serde::{Deserialize, Serialize};
use serde_json::{json, Value as JsonValue};
use starknet::core::types::Felt;
use strum_macros::AsRefStr;

Expand Down Expand Up @@ -308,6 +312,196 @@ impl Ty {
}
}
}

/// Convert a Ty to a JSON Value
pub fn to_json_value(&self) -> Result<JsonValue, PrimitiveError> {
match self {
Ty::Primitive(primitive) => match primitive {
Primitive::Bool(Some(v)) => Ok(json!(*v)),
Primitive::I8(Some(v)) => Ok(json!(*v)),
Primitive::I16(Some(v)) => Ok(json!(*v)),
Primitive::I32(Some(v)) => Ok(json!(*v)),
Primitive::I64(Some(v)) => Ok(json!(v.to_string())),
Primitive::I128(Some(v)) => Ok(json!(v.to_string())),
Primitive::U8(Some(v)) => Ok(json!(*v)),
Primitive::U16(Some(v)) => Ok(json!(*v)),
Primitive::U32(Some(v)) => Ok(json!(*v)),
Primitive::U64(Some(v)) => Ok(json!(v.to_string())),
Primitive::U128(Some(v)) => Ok(json!(v.to_string())),
Primitive::USize(Some(v)) => Ok(json!(*v)),
Primitive::U256(Some(v)) => {
let bytes = v.to_be_bytes();
let high = u128::from_be_bytes(bytes[..16].try_into().unwrap());
let low = u128::from_be_bytes(bytes[16..].try_into().unwrap());
Ok(json!({
"high": high.to_string(),
"low": low.to_string()
}))
}
Primitive::Felt252(Some(v)) => Ok(json!(v.to_string())),
Primitive::ClassHash(Some(v)) => Ok(json!(v.to_string())),
Primitive::ContractAddress(Some(v)) => Ok(json!(v.to_string())),
_ => Err(PrimitiveError::MissingFieldElement),
},
Ty::Struct(s) => {
let mut obj = IndexMap::new();
for member in &s.children {
obj.insert(member.name.clone(), member.ty.to_json_value()?);
}
Ok(json!(obj))
}
Ty::Enum(e) => {
let option = e.option().map_err(|_| PrimitiveError::MissingFieldElement)?;
Ok(json!({
option.name.clone(): option.ty.to_json_value()?
}))
}
Ty::Array(items) => {
let values: Result<Vec<_>, _> = items.iter().map(|ty| ty.to_json_value()).collect();
Ok(json!(values?))
}
Ty::Tuple(items) => {
let values: Result<Vec<_>, _> = items.iter().map(|ty| ty.to_json_value()).collect();
Ok(json!(values?))
}
Ty::ByteArray(bytes) => Ok(json!(bytes.clone())),
}
}

Larkooo marked this conversation as resolved.
Show resolved Hide resolved
/// Parse a JSON Value into a Ty
pub fn from_json_value(&mut self, value: JsonValue) -> Result<(), PrimitiveError> {
match (self, value) {
(Ty::Primitive(primitive), value) => match primitive {
Primitive::Bool(v) => {
if let JsonValue::Bool(b) = value {
*v = Some(b);
}
}
Primitive::I8(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_i64().map(|n| n as i8);
}
}
Primitive::I16(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_i64().map(|n| n as i16);
}
}
Primitive::I32(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_i64().map(|n| n as i32);
}
}
Primitive::I64(v) => {
if let JsonValue::String(s) = value {
*v = s.parse().ok();
}
}
Primitive::I128(v) => {
if let JsonValue::String(s) = value {
*v = s.parse().ok();
}
}
Primitive::U8(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_u64().map(|n| n as u8);
}
}
Primitive::U16(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_u64().map(|n| n as u16);
}
}
Primitive::U32(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_u64().map(|n| n as u32);
}
}
Primitive::U64(v) => {
if let JsonValue::String(s) = value {
*v = s.parse().ok();
}
}
Primitive::U128(v) => {
if let JsonValue::String(s) = value {
*v = s.parse().ok();
}
}
Primitive::USize(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_u64().map(|n| n as u32);
}
}
Primitive::U256(v) => {
if let JsonValue::Object(obj) = value {
if let (Some(JsonValue::String(high)), Some(JsonValue::String(low))) =
(obj.get("high"), obj.get("low"))
{
if let (Ok(high), Ok(low)) = (high.parse::<u128>(), low.parse::<u128>())
{
let mut bytes = [0u8; 32];
bytes[..16].copy_from_slice(&high.to_be_bytes());
bytes[16..].copy_from_slice(&low.to_be_bytes());
*v = Some(U256::from_be_slice(&bytes));
}
}
}
}
Primitive::Felt252(v) => {
if let JsonValue::String(s) = value {
*v = Felt::from_str(&s).ok();
}
}
Primitive::ClassHash(v) => {
if let JsonValue::String(s) = value {
*v = Felt::from_str(&s).ok();
}
}
Primitive::ContractAddress(v) => {
if let JsonValue::String(s) = value {
*v = Felt::from_str(&s).ok();
}
}
},
(Ty::Struct(s), JsonValue::Object(obj)) => {
for member in &mut s.children {
if let Some(value) = obj.get(&member.name) {
member.ty.from_json_value(value.clone())?;
}
}
}
(Ty::Enum(e), JsonValue::Object(obj)) => {
if let Some((name, value)) = obj.into_iter().next() {
e.set_option(&name).map_err(|_| PrimitiveError::TypeMismatch)?;
if let Some(option) = e.option {
e.options[option as usize].ty.from_json_value(value)?;
}
}
}
(Ty::Array(items), JsonValue::Array(values)) => {
let template = items[0].clone();
items.clear();
for value in values {
let mut item = template.clone();
item.from_json_value(value)?;
items.push(item);
}
}
Comment on lines +481 to +489
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Handle empty arrays safely

The current implementation assumes arrays are non-empty and could panic when deserializing an empty array.

Apply this fix:

     (Ty::Array(items), JsonValue::Array(values)) => {
+        if items.is_empty() {
+            return Err(PrimitiveError::TypeMismatch);
+        }
         let template = items[0].clone();
         items.clear();
         for value in values {
             let mut item = template.clone();
             item.from_json_value(value)?;
             items.push(item);
         }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
(Ty::Array(items), JsonValue::Array(values)) => {
let template = items[0].clone();
items.clear();
for value in values {
let mut item = template.clone();
item.from_json_value(value)?;
items.push(item);
}
}
(Ty::Array(items), JsonValue::Array(values)) => {
if items.is_empty() {
return Err(PrimitiveError::TypeMismatch);
}
let template = items[0].clone();
items.clear();
for value in values {
let mut item = template.clone();
item.from_json_value(value)?;
items.push(item);
}
}

(Ty::Tuple(items), JsonValue::Array(values)) => {
if items.len() != values.len() {
return Err(PrimitiveError::TypeMismatch);
}
for (item, value) in items.iter_mut().zip(values) {
item.from_json_value(value)?;
}
}
(Ty::ByteArray(bytes), JsonValue::String(s)) => {
*bytes = s;
}
_ => return Err(PrimitiveError::TypeMismatch),
}
Ok(())
}
Comment on lines +371 to +504
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improving Error Handling in from_json_value

The from_json_value method effectively deserializes JSON values into Ty instances. However, there are several areas where error handling can be enhanced to prevent silent failures.

Handle parsing errors for numeric primitives

Ohayo sensei, when parsing numeric values from JSON, assigning None on failure may lead to unexpected behavior. It's better to return an error if parsing fails.

Example modification for Primitive::I64 (lines 396-398):

 if let JsonValue::String(s) = value {
-    *v = s.parse().ok();
+    *v = Some(s.parse().map_err(|_| PrimitiveError::InvalidValue)?);
 }

Apply similar changes for other numeric primitives (I128, U64, U128), ensuring that parsing errors are properly propagated.

Ensure proper error handling for Primitive::U256 parsing

In lines 436-449, when parsing high and low values, failure to parse should result in an error rather than being silently ignored.

 if let JsonValue::Object(obj) = value {
     if let (Some(JsonValue::String(high)), Some(JsonValue::String(low))) =
         (obj.get("high"), obj.get("low"))
     {
-        if let (Ok(high), Ok(low)) = (high.parse::<u128>(), low.parse::<u128>()) {
+        let high = high.parse::<u128>().map_err(|_| PrimitiveError::InvalidValue)?;
+        let low = low.parse::<u128>().map_err(|_| PrimitiveError::InvalidValue)?;
         let mut bytes = [0u8; 32];
         bytes[..16].copy_from_slice(&high.to_be_bytes());
         bytes[16..].copy_from_slice(&low.to_be_bytes());
         *v = Some(U256::from_be_slice(&bytes));
     }
 }

Consistent error handling across all primitive types will enhance the robustness of the deserialization process.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// Parse a JSON Value into a Ty
pub fn from_json_value(&mut self, value: JsonValue) -> Result<(), PrimitiveError> {
match (self, value) {
(Ty::Primitive(primitive), value) => match primitive {
Primitive::Bool(v) => {
if let JsonValue::Bool(b) = value {
*v = Some(b);
}
}
Primitive::I8(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_i64().map(|n| n as i8);
}
}
Primitive::I16(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_i64().map(|n| n as i16);
}
}
Primitive::I32(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_i64().map(|n| n as i32);
}
}
Primitive::I64(v) => {
if let JsonValue::String(s) = value {
*v = s.parse().ok();
}
}
Primitive::I128(v) => {
if let JsonValue::String(s) = value {
*v = s.parse().ok();
}
}
Primitive::U8(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_u64().map(|n| n as u8);
}
}
Primitive::U16(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_u64().map(|n| n as u16);
}
}
Primitive::U32(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_u64().map(|n| n as u32);
}
}
Primitive::U64(v) => {
if let JsonValue::String(s) = value {
*v = s.parse().ok();
}
}
Primitive::U128(v) => {
if let JsonValue::String(s) = value {
*v = s.parse().ok();
}
}
Primitive::USize(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_u64().map(|n| n as u32);
}
}
Primitive::U256(v) => {
if let JsonValue::Object(obj) = value {
if let (Some(JsonValue::String(high)), Some(JsonValue::String(low))) =
(obj.get("high"), obj.get("low"))
{
if let (Ok(high), Ok(low)) = (high.parse::<u128>(), low.parse::<u128>())
{
let mut bytes = [0u8; 32];
bytes[..16].copy_from_slice(&high.to_be_bytes());
bytes[16..].copy_from_slice(&low.to_be_bytes());
*v = Some(U256::from_be_slice(&bytes));
}
}
}
}
Primitive::Felt252(v) => {
if let JsonValue::String(s) = value {
*v = Felt::from_str(&s).ok();
}
}
Primitive::ClassHash(v) => {
if let JsonValue::String(s) = value {
*v = Felt::from_str(&s).ok();
}
}
Primitive::ContractAddress(v) => {
if let JsonValue::String(s) = value {
*v = Felt::from_str(&s).ok();
}
}
},
(Ty::Struct(s), JsonValue::Object(obj)) => {
for member in &mut s.children {
if let Some(value) = obj.get(&member.name) {
member.ty.from_json_value(value.clone())?;
}
}
}
(Ty::Enum(e), JsonValue::Object(obj)) => {
if let Some((name, value)) = obj.into_iter().next() {
e.set_option(&name).map_err(|_| PrimitiveError::TypeMismatch)?;
if let Some(option) = e.option {
e.options[option as usize].ty.from_json_value(value)?;
}
}
}
(Ty::Array(items), JsonValue::Array(values)) => {
let template = items[0].clone();
items.clear();
for value in values {
let mut item = template.clone();
item.from_json_value(value)?;
items.push(item);
}
}
(Ty::Tuple(items), JsonValue::Array(values)) => {
if items.len() != values.len() {
return Err(PrimitiveError::TypeMismatch);
}
for (item, value) in items.iter_mut().zip(values) {
item.from_json_value(value)?;
}
}
(Ty::ByteArray(bytes), JsonValue::String(s)) => {
*bytes = s;
}
_ => return Err(PrimitiveError::TypeMismatch),
}
Ok(())
}
/// Parse a JSON Value into a Ty
pub fn from_json_value(&mut self, value: JsonValue) -> Result<(), PrimitiveError> {
match (self, value) {
(Ty::Primitive(primitive), value) => match primitive {
Primitive::Bool(v) => {
if let JsonValue::Bool(b) = value {
*v = Some(b);
}
}
Primitive::I8(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_i64().map(|n| n as i8);
}
}
Primitive::I16(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_i64().map(|n| n as i16);
}
}
Primitive::I32(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_i64().map(|n| n as i32);
}
}
Primitive::I64(v) => {
if let JsonValue::String(s) = value {
*v = Some(s.parse().map_err(|_| PrimitiveError::InvalidValue)?);
}
}
Primitive::I128(v) => {
if let JsonValue::String(s) = value {
*v = Some(s.parse().map_err(|_| PrimitiveError::InvalidValue)?);
}
}
Primitive::U8(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_u64().map(|n| n as u8);
}
}
Primitive::U16(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_u64().map(|n| n as u16);
}
}
Primitive::U32(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_u64().map(|n| n as u32);
}
}
Primitive::U64(v) => {
if let JsonValue::String(s) = value {
*v = Some(s.parse().map_err(|_| PrimitiveError::InvalidValue)?);
}
}
Primitive::U128(v) => {
if let JsonValue::String(s) = value {
*v = Some(s.parse().map_err(|_| PrimitiveError::InvalidValue)?);
}
}
Primitive::USize(v) => {
if let JsonValue::Number(n) = value {
*v = n.as_u64().map(|n| n as u32);
}
}
Primitive::U256(v) => {
if let JsonValue::Object(obj) = value {
if let (Some(JsonValue::String(high)), Some(JsonValue::String(low))) =
(obj.get("high"), obj.get("low"))
{
let high = high.parse::<u128>().map_err(|_| PrimitiveError::InvalidValue)?;
let low = low.parse::<u128>().map_err(|_| PrimitiveError::InvalidValue)?;
let mut bytes = [0u8; 32];
bytes[..16].copy_from_slice(&high.to_be_bytes());
bytes[16..].copy_from_slice(&low.to_be_bytes());
*v = Some(U256::from_be_slice(&bytes));
}
}
}
Primitive::Felt252(v) => {
if let JsonValue::String(s) = value {
*v = Felt::from_str(&s).ok();
}
}
Primitive::ClassHash(v) => {
if let JsonValue::String(s) = value {
*v = Felt::from_str(&s).ok();
}
}
Primitive::ContractAddress(v) => {
if let JsonValue::String(s) = value {
*v = Felt::from_str(&s).ok();
}
}
},
(Ty::Struct(s), JsonValue::Object(obj)) => {
for member in &mut s.children {
if let Some(value) = obj.get(&member.name) {
member.ty.from_json_value(value.clone())?;
}
}
}
(Ty::Enum(e), JsonValue::Object(obj)) => {
if let Some((name, value)) = obj.into_iter().next() {
e.set_option(&name).map_err(|_| PrimitiveError::TypeMismatch)?;
if let Some(option) = e.option {
e.options[option as usize].ty.from_json_value(value)?;
}
}
}
(Ty::Array(items), JsonValue::Array(values)) => {
let template = items[0].clone();
items.clear();
for value in values {
let mut item = template.clone();
item.from_json_value(value)?;
items.push(item);
}
}
(Ty::Tuple(items), JsonValue::Array(values)) => {
if items.len() != values.len() {
return Err(PrimitiveError::TypeMismatch);
}
for (item, value) in items.iter_mut().zip(values) {
item.from_json_value(value)?;
}
}
(Ty::ByteArray(bytes), JsonValue::String(s)) => {
*bytes = s;
}
_ => return Err(PrimitiveError::TypeMismatch),
}
Ok(())
}

}

#[derive(Debug)]
Expand Down
9 changes: 1 addition & 8 deletions crates/torii/core/src/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,14 +548,7 @@ impl<'c, P: Provider + Sync + Send + 'static> Executor<'c, P> {
if em_query.is_historical {
event_counter += 1;

let data = em_query
.ty
.serialize()?
.iter()
.map(|felt| format!("{:#x}", felt))
.collect::<Vec<String>>()
.join("/");

let data = serde_json::to_string(&em_query.ty.to_json_value()?)?;
sqlx::query(
"INSERT INTO event_messages_historical (id, keys, event_id, data, \
model_id, executed_at) VALUES (?, ?, ?, ?, ?, ?) RETURNING *",
Expand Down
Loading