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): model upgrades #2637

Merged
merged 38 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
b1f3e35
feat(torii): model upgrades
Larkooo Nov 5, 2024
fa71dbd
upgrade processor for event and model
Larkooo Nov 6, 2024
60a870d
Merge branch 'main' into model-upgrade
Larkooo Nov 6, 2024
62750f6
fix processors
Larkooo Nov 6, 2024
6313f44
fix: model upgrade
Larkooo Nov 6, 2024
f7fd6ae
wrap up model upgrade
Larkooo Nov 6, 2024
e85bdd0
ftm
Larkooo Nov 6, 2024
d641ba0
clippy
Larkooo Nov 6, 2024
3336c4a
fmt
Larkooo Nov 6, 2024
3742676
refactor: diff
Larkooo Nov 7, 2024
b3ee109
fmtg
Larkooo Nov 7, 2024
0d26d43
fix: set model cache
Larkooo Nov 7, 2024
cbc39c7
fix add model members
Larkooo Nov 7, 2024
14ffc8e
fix
Larkooo Nov 7, 2024
ddd9921
feat: shared cache between grpc & engine and fix partial deser
Larkooo Nov 8, 2024
cf9c45c
fix: test and fmt
Larkooo Nov 8, 2024
3925cce
Merge remote-tracking branch 'upstream/main' into model-upgrade
Larkooo Nov 10, 2024
ae8bf42
fix
Larkooo Nov 10, 2024
210ac31
primitives not option
Larkooo Nov 12, 2024
d86fcc4
fix: enums
Larkooo Nov 13, 2024
ab855dd
Revert "fix: enums"
Larkooo Nov 13, 2024
5d57e6d
Revert "primitives not option"
Larkooo Nov 13, 2024
b04e848
fix enum sql value
Larkooo Nov 13, 2024
e3d8a7b
Merge remote-tracking branch 'upstream/main' into model-upgrade
Larkooo Nov 13, 2024
b7f3264
main
Larkooo Nov 13, 2024
87857c2
remove prints & format
Larkooo Nov 13, 2024
9b5d624
fix quer ytest
Larkooo Nov 13, 2024
9ac0d21
fix: bool
Larkooo Nov 14, 2024
a8d6737
fix: ararys
Larkooo Nov 14, 2024
d2c7696
fmt
Larkooo Nov 14, 2024
255d4fb
fix: map row to ty
Larkooo Nov 14, 2024
6235080
fix: enum
Larkooo Nov 14, 2024
612fc4b
fix: enum
Larkooo Nov 14, 2024
0dbec7e
fmt
Larkooo Nov 14, 2024
551c6bd
fix: primitive len
Larkooo Nov 14, 2024
cea8102
Revert "fix: primitive len"
Larkooo Nov 14, 2024
155033d
refactotr: dont use modelr eader block
Larkooo Nov 14, 2024
4e921c3
Revert "Revert "fix: primitive len""
Larkooo Nov 14, 2024
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
17 changes: 13 additions & 4 deletions bin/torii/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
use torii_core::processors::store_transaction::StoreTransactionProcessor;
use torii_core::processors::EventProcessorConfig;
use torii_core::simple_broker::SimpleBroker;
use torii_core::sql::cache::ModelCache;
use torii_core::sql::Sql;
use torii_core::types::{Contract, ContractType, Model};
use torii_server::proxy::Proxy;
Expand Down Expand Up @@ -243,7 +244,9 @@
executor.run().await.unwrap();
});

let db = Sql::new(pool.clone(), sender.clone(), &args.indexing.contracts).await?;
let model_cache = Arc::new(ModelCache::new(pool.clone()));
let db = Sql::new(pool.clone(), sender.clone(), &args.indexing.contracts, model_cache.clone())

Check warning on line 248 in bin/torii/src/main.rs

View check run for this annotation

Codecov / codecov/patch

bin/torii/src/main.rs#L247-L248

Added lines #L247 - L248 were not covered by tests
.await?;

let processors = Processors {
transaction: vec![Box::new(StoreTransactionProcessor)],
Expand Down Expand Up @@ -283,9 +286,15 @@
);

let shutdown_rx = shutdown_tx.subscribe();
let (grpc_addr, grpc_server) =
torii_grpc::server::new(shutdown_rx, &pool, block_rx, world_address, Arc::clone(&provider))
.await?;
let (grpc_addr, grpc_server) = torii_grpc::server::new(
shutdown_rx,
&pool,
block_rx,
world_address,
Arc::clone(&provider),
model_cache,
)
.await?;

let mut libp2p_relay_server = torii_relay::server::Relay::new(
db,
Expand Down
55 changes: 19 additions & 36 deletions crates/dojo/types/src/primitive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,46 +190,29 @@
}
}

pub fn to_sql_value(&self) -> Result<String, PrimitiveError> {
let value = self.serialize()?;

if value.is_empty() {
return Err(PrimitiveError::MissingFieldElement);
}

pub fn to_sql_value(&self) -> String {
match self {
// Integers
Primitive::I8(_) => Ok(format!("{}", try_from_felt::<i8>(value[0])?)),
Primitive::I16(_) => Ok(format!("{}", try_from_felt::<i16>(value[0])?)),
Primitive::I32(_) => Ok(format!("{}", try_from_felt::<i32>(value[0])?)),
Primitive::I64(_) => Ok(format!("{}", try_from_felt::<i64>(value[0])?)),
Primitive::I8(i8) => format!("{}", i8.unwrap_or_default()),
Primitive::I16(i16) => format!("{}", i16.unwrap_or_default()),
Primitive::I32(i32) => format!("{}", i32.unwrap_or_default()),
Primitive::I64(i64) => format!("{}", i64.unwrap_or_default()),

Primitive::U8(_)
| Primitive::U16(_)
| Primitive::U32(_)
| Primitive::USize(_)
| Primitive::Bool(_) => Ok(format!("{}", value[0])),
Primitive::U8(u8) => format!("{}", u8.unwrap_or_default()),
Primitive::U16(u16) => format!("{}", u16.unwrap_or_default()),
Primitive::U32(u32) => format!("{}", u32.unwrap_or_default()),
Primitive::USize(u32) => format!("{}", u32.unwrap_or_default()),

Check warning on line 204 in crates/dojo/types/src/primitive.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/types/src/primitive.rs#L204

Added line #L204 was not covered by tests
Primitive::Bool(bool) => format!("{}", bool.unwrap_or_default() as i32),

// Hex string
Primitive::I128(_) => Ok(format!("{:#064x}", try_from_felt::<i128>(value[0])?)),
Primitive::ContractAddress(_)
| Primitive::ClassHash(_)
| Primitive::Felt252(_)
| Primitive::U128(_)
| Primitive::U64(_) => Ok(format!("{:#064x}", value[0])),

Primitive::U256(_) => {
if value.len() < 2 {
Err(PrimitiveError::NotEnoughFieldElements)
} else {
let mut buffer = [0u8; 32];
let value0_bytes = value[0].to_bytes_be();
let value1_bytes = value[1].to_bytes_be();
buffer[16..].copy_from_slice(&value0_bytes[16..]);
buffer[..16].copy_from_slice(&value1_bytes[16..]);
Ok(format!("0x{}", hex::encode(buffer)))
}
}
Primitive::I128(i128) => format!("0x{:064x}", i128.unwrap_or_default()),
Primitive::ContractAddress(felt) => format!("0x{:064x}", felt.unwrap_or_default()),
Primitive::ClassHash(felt) => format!("0x{:064x}", felt.unwrap_or_default()),
Primitive::Felt252(felt) => format!("0x{:064x}", felt.unwrap_or_default()),
Primitive::U128(u128) => format!("0x{:064x}", u128.unwrap_or_default()),
Primitive::U64(u64) => format!("0x{:064x}", u64.unwrap_or_default()),

Primitive::U256(u256) => format!("0x{:064x}", u256.unwrap_or_default()),
Comment on lines +193 to +215
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ohayo, sensei! Consider handling None values explicitly in to_sql_value

The current implementation uses unwrap_or_default() which might lead to unintended default values being used, especially in a database context where data accuracy is crucial. Consider explicitly handling None values to prevent data corruption.

}
}

Expand Down Expand Up @@ -436,7 +419,7 @@
let primitive = Primitive::U256(Some(U256::from_be_hex(
"aaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbccccccccccccccccdddddddddddddddd",
)));
let sql_value = primitive.to_sql_value().unwrap();
let sql_value = primitive.to_sql_value();
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Ohayo, sensei! Add error case tests

The test only covers the happy path with Some values. We should also test error cases with None values.

Add these test cases:

#[test]
fn test_u256_none() {
    let primitive = Primitive::U256(None);
    assert!(matches!(
        primitive.serialize(),
        Err(PrimitiveError::MissingFieldElement)
    ));
}

let serialized = primitive.serialize().unwrap();

let mut deserialized = primitive;
Expand Down
161 changes: 159 additions & 2 deletions crates/dojo/types/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,11 @@
}

pub fn deserialize(&mut self, felts: &mut Vec<Felt>) -> Result<(), PrimitiveError> {
if felts.is_empty() {
// return early if there are no felts to deserialize
return Ok(());

Check warning on line 178 in crates/dojo/types/src/schema.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/types/src/schema.rs#L178

Added line #L178 was not covered by tests
}

match self {
Ty::Primitive(c) => {
c.deserialize(felts)?;
Expand Down Expand Up @@ -224,6 +229,85 @@
}
Ok(())
}

/// Returns a new Ty containing only the differences between self and other
pub fn diff(&self, other: &Ty) -> Option<Ty> {
match (self, other) {
(Ty::Struct(s1), Ty::Struct(s2)) => {
// Find members that exist in s1 but not in s2, or are different
let diff_children: Vec<Member> = s1
.children
.iter()
.filter(|m1| {
s2.children
.iter()
.find(|m2| m2.name == m1.name)
.map_or(true, |m2| *m1 != m2)
})
.cloned()
.collect();

if diff_children.is_empty() {
None
} else {
Some(Ty::Struct(Struct { name: s1.name.clone(), children: diff_children }))
}
}
(Ty::Enum(e1), Ty::Enum(e2)) => {
// Find options that exist in e1 but not in e2, or are different
let diff_options: Vec<EnumOption> = e1
.options
.iter()
.filter(|o1| {
e2.options.iter().find(|o2| o2.name == o1.name).map_or(true, |o2| *o1 != o2)
})
.cloned()
.collect();

if diff_options.is_empty() {
None

Check warning on line 268 in crates/dojo/types/src/schema.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/types/src/schema.rs#L268

Added line #L268 was not covered by tests
} else {
Some(Ty::Enum(Enum {
name: e1.name.clone(),
option: e1.option,
options: diff_options,
}))
}
}
(Ty::Array(a1), Ty::Array(a2)) => {
if a1 == a2 {
None

Check warning on line 279 in crates/dojo/types/src/schema.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/types/src/schema.rs#L277-L279

Added lines #L277 - L279 were not covered by tests
} else {
Some(Ty::Array(a1.clone()))

Check warning on line 281 in crates/dojo/types/src/schema.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/types/src/schema.rs#L281

Added line #L281 was not covered by tests
}
}
(Ty::Tuple(t1), Ty::Tuple(t2)) => {
if t1 == t2 {
None

Check warning on line 286 in crates/dojo/types/src/schema.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/types/src/schema.rs#L284-L286

Added lines #L284 - L286 were not covered by tests
} else {
Some(Ty::Tuple(t1.clone()))

Check warning on line 288 in crates/dojo/types/src/schema.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/types/src/schema.rs#L288

Added line #L288 was not covered by tests
}
}
(Ty::ByteArray(b1), Ty::ByteArray(b2)) => {
if b1 == b2 {
None

Check warning on line 293 in crates/dojo/types/src/schema.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/types/src/schema.rs#L291-L293

Added lines #L291 - L293 were not covered by tests
} else {
Some(Ty::ByteArray(b1.clone()))

Check warning on line 295 in crates/dojo/types/src/schema.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/types/src/schema.rs#L295

Added line #L295 was not covered by tests
}
}
(Ty::Primitive(p1), Ty::Primitive(p2)) => {
if p1 == p2 {
None

Check warning on line 300 in crates/dojo/types/src/schema.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/types/src/schema.rs#L298-L300

Added lines #L298 - L300 were not covered by tests
} else {
Some(Ty::Primitive(*p1))

Check warning on line 302 in crates/dojo/types/src/schema.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/types/src/schema.rs#L302

Added line #L302 was not covered by tests
}
}
// Different types entirely - we cannot diff them
_ => {
panic!("Type mismatch between self {:?} and other {:?}", self.name(), other.name())

Check warning on line 307 in crates/dojo/types/src/schema.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/types/src/schema.rs#L307

Added line #L307 was not covered by tests
}
}
}
}

#[derive(Debug)]
Expand Down Expand Up @@ -351,8 +435,8 @@
}
}

pub fn to_sql_value(&self) -> Result<String, EnumError> {
self.option().map(|option| option.name.clone())
pub fn to_sql_value(&self) -> String {
self.option().unwrap_or(&self.options[0]).name.clone()
Comment on lines +438 to +439
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 potential panics in to_sql_value.

Ohayo! The unwrap_or usage could panic if the enum option is invalid.

Consider this safer implementation:

- pub fn to_sql_value(&self) -> String {
-     self.option().unwrap_or(&self.options[0]).name.clone()
+ pub fn to_sql_value(&self) -> Result<String, EnumError> {
+     Ok(self.option().unwrap_or_else(|_| &self.options[0]).name.clone())

Committable suggestion skipped: line range outside the PR's diff.

}
}

Expand Down Expand Up @@ -597,4 +681,77 @@
assert_eq!(format_member(&member), expected);
}
}

#[test]
fn test_ty_diff() {
// Test struct diff
let struct1 = Ty::Struct(Struct {
name: "TestStruct".to_string(),
children: vec![
Member {
name: "field1".to_string(),
ty: Ty::Primitive(Primitive::U32(None)),
key: false,
},
Member {
name: "field2".to_string(),
ty: Ty::Primitive(Primitive::U32(None)),
key: false,
},
Member {
name: "field3".to_string(),
ty: Ty::Primitive(Primitive::U32(None)),
key: false,
},
],
});

let struct2 = Ty::Struct(Struct {
name: "TestStruct".to_string(),
children: vec![Member {
name: "field1".to_string(),
ty: Ty::Primitive(Primitive::U32(None)),
key: false,
}],
});

// Should show only field2 and field3 as differences
let diff = struct1.diff(&struct2).unwrap();
if let Ty::Struct(s) = diff {
assert_eq!(s.children.len(), 2);
assert_eq!(s.children[0].name, "field2");
assert_eq!(s.children[1].name, "field3");
} else {
panic!("Expected Struct diff");

Check warning on line 725 in crates/dojo/types/src/schema.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/types/src/schema.rs#L725

Added line #L725 was not covered by tests
}

// Test enum diff
let enum1 = Ty::Enum(Enum {
name: "TestEnum".to_string(),
option: None,
options: vec![
EnumOption { name: "Option1".to_string(), ty: Ty::Tuple(vec![]) },
EnumOption { name: "Option2".to_string(), ty: Ty::Tuple(vec![]) },
],
});

let enum2 = Ty::Enum(Enum {
name: "TestEnum".to_string(),
option: None,
options: vec![EnumOption { name: "Option1".to_string(), ty: Ty::Tuple(vec![]) }],
});

// Should show only Option2 as difference
let diff = enum1.diff(&enum2).unwrap();
if let Ty::Enum(e) = diff {
assert_eq!(e.options.len(), 1);
assert_eq!(e.options[0].name, "Option2");
} else {
panic!("Expected Enum diff");

Check warning on line 750 in crates/dojo/types/src/schema.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/types/src/schema.rs#L750

Added line #L750 was not covered by tests
}

// Test no differences
let same_struct = struct2.diff(&struct2);
assert!(same_struct.is_none());
}
Larkooo marked this conversation as resolved.
Show resolved Hide resolved
}
18 changes: 14 additions & 4 deletions crates/dojo/world/src/contracts/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use dojo_types::packing::{PackingError, ParseError};
use dojo_types::primitive::{Primitive, PrimitiveError};
use dojo_types::schema::{Enum, EnumOption, Member, Struct, Ty};
use starknet::core::types::Felt;
use starknet::core::types::{BlockId, Felt};
use starknet::core::utils::{
cairo_short_string_to_felt, parse_cairo_short_string, CairoShortStringToFeltError,
NonAsciiNameError, ParseCairoShortStringError,
Expand Down Expand Up @@ -86,13 +86,22 @@
namespace: &str,
name: &str,
world: &'a WorldContractReader<P>,
) -> Result<ModelRPCReader<'a, P>, ModelError> {
Self::new_with_block(namespace, name, world, world.block_id).await
}

Check warning on line 91 in crates/dojo/world/src/contracts/model.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/world/src/contracts/model.rs#L89-L91

Added lines #L89 - L91 were not covered by tests

pub async fn new_with_block(
namespace: &str,
name: &str,
world: &'a WorldContractReader<P>,
block_id: BlockId,
) -> Result<ModelRPCReader<'a, P>, ModelError> {
let model_selector = naming::compute_selector_from_names(namespace, name);

// Events are also considered like models from a off-chain perspective. They both have
// introspection and convey type information.
let (contract_address, class_hash) =
match world.resource(&model_selector).block_id(world.block_id).call().await? {
match world.resource(&model_selector).block_id(block_id).call().await? {
abigen::world::Resource::Model((address, hash)) => (address, hash),
abigen::world::Resource::Event((address, hash)) => (address, hash),
_ => return Err(ModelError::ModelNotFound),
Expand All @@ -104,7 +113,8 @@
return Err(ModelError::ModelNotFound);
}

let model_reader = ModelContractReader::new(contract_address.into(), world.provider());
let mut model_reader = ModelContractReader::new(contract_address.into(), world.provider());
model_reader.set_block(block_id);

Ok(Self {
namespace: namespace.into(),
Expand Down Expand Up @@ -176,7 +186,7 @@
parse_schema(&abigen::model::Ty::Struct(res)).map_err(ModelError::Parse)
}

// For non fixed layouts, packed and unpacked sizes are None.
// For non fixed layouts, packed and unpacked sizes are None.
// Therefore we return 0 in this case.
async fn packed_size(&self) -> Result<u32, ModelError> {
Ok(self.model_reader.packed_size().call().await?.unwrap_or(0))
Expand Down
10 changes: 10 additions & 0 deletions crates/dojo/world/src/contracts/world.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::result::Result;

use starknet::core::types::BlockId;
use starknet::providers::Provider;

pub use super::abigen::world::{
Expand Down Expand Up @@ -33,4 +34,13 @@ where
) -> Result<ModelRPCReader<'_, P>, ModelError> {
ModelRPCReader::new(namespace, name, self).await
}

pub async fn model_reader_with_block(
&self,
namespace: &str,
name: &str,
block_id: BlockId,
) -> Result<ModelRPCReader<'_, P>, ModelError> {
ModelRPCReader::new_with_block(namespace, name, self, block_id).await
}
}
2 changes: 1 addition & 1 deletion crates/sozo/ops/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@
let mut _p = *p;
let _ = _p.deserialize(values);

format!("{}{}", _start_indent(level, start_indent), _p.to_sql_value().unwrap())
format!("{}{}", _start_indent(level, start_indent), _p.to_sql_value())

Check warning on line 415 in crates/sozo/ops/src/model.rs

View check run for this annotation

Codecov / codecov/patch

crates/sozo/ops/src/model.rs#L415

Added line #L415 was not covered by tests
}

fn format_byte_array(values: &mut Vec<Felt>, level: usize, start_indent: bool) -> String {
Expand Down
4 changes: 4 additions & 0 deletions crates/torii/core/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ use crate::processors::store_del_record::StoreDelRecordProcessor;
use crate::processors::store_set_record::StoreSetRecordProcessor;
use crate::processors::store_update_member::StoreUpdateMemberProcessor;
use crate::processors::store_update_record::StoreUpdateRecordProcessor;
use crate::processors::upgrade_event::UpgradeEventProcessor;
use crate::processors::upgrade_model::UpgradeModelProcessor;
use crate::processors::{
BlockProcessor, EventProcessor, EventProcessorConfig, TransactionProcessor,
};
Expand Down Expand Up @@ -76,6 +78,8 @@ impl<P: Provider + Send + Sync + std::fmt::Debug + 'static> Processors<P> {
vec![
Box::new(RegisterModelProcessor) as Box<dyn EventProcessor<P>>,
Box::new(RegisterEventProcessor) as Box<dyn EventProcessor<P>>,
Box::new(UpgradeModelProcessor) as Box<dyn EventProcessor<P>>,
Box::new(UpgradeEventProcessor) as Box<dyn EventProcessor<P>>,
Box::new(StoreSetRecordProcessor),
Box::new(StoreDelRecordProcessor),
Box::new(StoreUpdateRecordProcessor),
Expand Down
Loading
Loading