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(dojo-core): add read schema support #2932

Merged
merged 14 commits into from
Jan 27, 2025
Merged
72 changes: 72 additions & 0 deletions crates/dojo/core-cairo-test/src/tests/model/model.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,39 @@ struct Foo3 {
v2: u32,
}

#[derive(Copy, Drop, Serde, Debug, Introspect)]
struct AStruct {
a: u8,
b: u8,
c: u8,
d: u8,
}

#[dojo::model]
#[derive(Copy, Drop, Serde, Debug)]
struct Foo4 {
#[key]
id: felt252,
v0: u256,
v1: felt252,
v2: u128,
v3: AStruct,
}

#[derive(Copy, Drop, Serde, Debug, Introspect)]
struct FooSchema {
v0: u256,
v3: AStruct,
}

fn namespace_def() -> NamespaceDef {
NamespaceDef {
namespace: "dojo_cairo_test",
resources: [
TestResource::Model(m_Foo::TEST_CLASS_HASH.try_into().unwrap()),
TestResource::Model(m_Foo2::TEST_CLASS_HASH.try_into().unwrap()),
TestResource::Model(m_Foo3::TEST_CLASS_HASH.try_into().unwrap()),
TestResource::Model(m_Foo4::TEST_CLASS_HASH.try_into().unwrap()),
]
.span(),
}
Expand Down Expand Up @@ -268,3 +295,48 @@ fn test_ptrs_from() {
let ptrs_d = Model::<Foo>::ptrs_from_ids([foo.entity_id(), foo2.entity_id()].span());
assert!(ptrs_a == ptrs_b && ptrs_a == ptrs_c && ptrs_a == ptrs_d);
}

#[test]
fn test_read_schema() {
let mut world = spawn_foo_world();
let foo = Foo4 { id: 1, v0: 2, v1: 3, v2: 4, v3: AStruct { a: 5, b: 6, c: 7, d: 8 } };
world.write_model(@foo);

let schema: FooSchema = world.read_schema(foo.ptr());
assert!(
schema.v0 == foo.v0
&& schema.v3.a == foo.v3.a
&& schema.v3.b == foo.v3.b
&& schema.v3.c == foo.v3.c
&& schema.v3.d == foo.v3.d,
);
}

#[test]
fn test_read_schemas() {
let mut world = spawn_foo_world();
let foo = Foo4 { id: 1, v0: 2, v1: 3, v2: 4, v3: AStruct { a: 5, b: 6, c: 7, d: 8 } };
let mut foo_2 = foo;
foo_2.id = 2;
foo_2.v0 = 12;

world.write_models([@foo, @foo_2].span());

let mut values: Array<FooSchema> = world.read_schemas([foo.ptr(), foo_2.ptr()].span());
let schema_1 = values.pop_front().unwrap();
let schema_2 = values.pop_front().unwrap();
assert!(
schema_1.v0 == foo.v0
&& schema_1.v3.a == foo.v3.a
&& schema_1.v3.b == foo.v3.b
&& schema_1.v3.c == foo.v3.c
&& schema_1.v3.d == foo.v3.d,
);
assert!(
schema_2.v0 == foo_2.v0
&& schema_2.v3.a == foo_2.v3.a
&& schema_2.v3.b == foo_2.v3.b
&& schema_2.v3.c == foo_2.v3.c
&& schema_2.v3.d == foo_2.v3.d,
);
}
Comment on lines +315 to +342
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add array bounds checking for safer array operations.

While the test is comprehensive, it could be more robust by checking the array length before accessing elements:

     let mut values: Array<FooSchema> = world.read_schemas([foo.ptr(), foo_2.ptr()].span());
+    assert!(values.len() == 2, "Expected 2 schemas");
     let schema_1 = values.pop_front().unwrap();
     let schema_2 = values.pop_front().unwrap();
📝 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
#[test]
fn test_read_schemas() {
let mut world = spawn_foo_world();
let foo = Foo4 { id: 1, v0: 2, v1: 3, v2: 4, v3: AStruct { a: 5, b: 6, c: 7, d: 8 } };
let mut foo_2 = foo;
foo_2.id = 2;
foo_2.v0 = 12;
world.write_models([@foo, @foo_2].span());
let mut values: Array<FooSchema> = world.read_schemas([foo.ptr(), foo_2.ptr()].span());
let schema_1 = values.pop_front().unwrap();
let schema_2 = values.pop_front().unwrap();
assert!(
schema_1.v0 == foo.v0
&& schema_1.v3.a == foo.v3.a
&& schema_1.v3.b == foo.v3.b
&& schema_1.v3.c == foo.v3.c
&& schema_1.v3.d == foo.v3.d,
);
assert!(
schema_2.v0 == foo_2.v0
&& schema_2.v3.a == foo_2.v3.a
&& schema_2.v3.b == foo_2.v3.b
&& schema_2.v3.c == foo_2.v3.c
&& schema_2.v3.d == foo_2.v3.d,
);
}
#[test]
fn test_read_schemas() {
let mut world = spawn_foo_world();
let foo = Foo4 { id: 1, v0: 2, v1: 3, v2: 4, v3: AStruct { a: 5, b: 6, c: 7, d: 8 } };
let mut foo_2 = foo;
foo_2.id = 2;
foo_2.v0 = 12;
world.write_models([@foo, @foo_2].span());
let mut values: Array<FooSchema> = world.read_schemas([foo.ptr(), foo_2.ptr()].span());
assert!(values.len() == 2, "Expected 2 schemas");
let schema_1 = values.pop_front().unwrap();
let schema_2 = values.pop_front().unwrap();
assert!(
schema_1.v0 == foo.v0
&& schema_1.v3.a == foo.v3.a
&& schema_1.v3.b == foo.v3.b
&& schema_1.v3.c == foo.v3.c
&& schema_1.v3.d == foo.v3.d,
);
assert!(
schema_2.v0 == foo_2.v0
&& schema_2.v3.a == foo_2.v3.a
&& schema_2.v3.b == foo_2.v3.b
&& schema_2.v3.c == foo_2.v3.c
&& schema_2.v3.d == foo_2.v3.d,
);
}

8 changes: 8 additions & 0 deletions crates/dojo/core/src/model/model.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ pub struct ModelPtr<M> {
pub id: felt252,
}

/// Trait that defines a method for converting a span of model pointers to a span of model indexes.
///
/// # Type Parameters
/// - `M`: The type of the model.
///
/// # Methods
/// - `to_indexes(self: Span<ModelPtr<M>>) -> Span<ModelIndex>`:
/// Converts the span of model pointers to a span of model indexes.
pub trait ModelPtrsTrait<M> {
fn to_indexes(self: Span<ModelPtr<M>>) -> Span<ModelIndex>;
fn to_member_indexes(self: Span<ModelPtr<M>>, field_selector: felt252) -> Span<ModelIndex>;
Expand Down
10 changes: 9 additions & 1 deletion crates/dojo/core/src/model/storage.cairo
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use dojo::{model::{ModelPtr, model_value::ModelValueKey}};
use dojo::{model::{ModelPtr, model_value::ModelValueKey}, meta::Introspect};

// TODO: define the right interface for member accesses.

Expand Down Expand Up @@ -53,6 +53,14 @@ pub trait ModelStorage<S, M> {
ref self: S, ptrs: Span<ModelPtr<M>>, field_selector: felt252, values: Span<T>,
);

/// Retrieves a subset of members in a model, matching a defined schema <T>.
fn read_schema<T, +Serde<T>, +Introspect<T>>(self: @S, ptr: ModelPtr<M>) -> T;

/// Retrieves part of multiple models, matching a schema.
fn read_schemas<T, +Drop<T>, +Serde<T>, +Introspect<T>>(
self: @S, ptrs: Span<ModelPtr<M>>,
) -> Array<T>;

/// Returns the current namespace hash.
fn namespace_hash(self: @S) -> felt252;
}
Expand Down
40 changes: 39 additions & 1 deletion crates/dojo/core/src/world/storage.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use dojo::model::{
Model, ModelIndex, ModelValueKey, ModelValue, ModelStorage, ModelPtr, ModelPtrsTrait,
};
use dojo::event::{Event, EventStorage};
use dojo::meta::Layout;
use dojo::meta::{Layout, FieldLayout, Introspect};
use dojo::utils::{
entity_id_from_keys, entity_id_from_serialized_keys, serialize_inline, find_model_field_layout,
deserialize_unwrap,
Expand All @@ -26,6 +26,17 @@ fn field_layout_unwrap<M, +Model<M>>(field_selector: felt252) -> Layout {
}
}

fn make_partial_struct_layout<M, +Model<M>>(field_selectors: Span<felt252>) -> Layout {
let mut layouts: Array<FieldLayout> = array![];
for selector in field_selectors {
layouts
.append(
FieldLayout { selector: *selector, layout: field_layout_unwrap::<M>(*selector) },
);
};
Layout::Struct(layouts.span())
}

#[generate_trait]
pub impl WorldStorageInternalImpl of WorldStorageTrait {
fn new(world: IWorldDispatcher, namespace: @ByteArray) -> WorldStorage {
Expand Down Expand Up @@ -267,6 +278,33 @@ pub impl ModelStorageWorldStorageImpl<M, +Model<M>, +Drop<M>> of ModelStorage<Wo
);
}

fn read_schema<T, +Serde<T>, +Introspect<T>>(self: @WorldStorage, ptr: ModelPtr<M>) -> T {
deserialize_unwrap(
IWorldDispatcherTrait::entity(
*self.dispatcher,
Model::<M>::selector(*self.namespace_hash),
ModelIndex::Id(ptr.id),
Introspect::<T>::layout(),
),
)
}

fn read_schemas<T, +Drop<T>, +Serde<T>, +Introspect<T>>(
self: @WorldStorage, ptrs: Span<ModelPtr<M>>,
) -> Array<T> {
let mut values = ArrayTrait::<T>::new();

for entity in IWorldDispatcherTrait::entities(
*self.dispatcher,
Model::<M>::selector(*self.namespace_hash),
ptrs.to_indexes(),
Introspect::<T>::layout(),
) {
values.append(deserialize_unwrap(*entity));
};
values
}

fn namespace_hash(self: @WorldStorage) -> felt252 {
*self.namespace_hash
}
Expand Down
28 changes: 28 additions & 0 deletions examples/benchmark/Scarb.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Code generated by scarb DO NOT EDIT.
version = 1

[[package]]
name = "dojo"
version = "1.0.12"
dependencies = [
"dojo_plugin",
]

[[package]]
name = "dojo_benchmark"
version = "0.1.0"
dependencies = [
"dojo",
"dojo_cairo_test",
]

[[package]]
name = "dojo_cairo_test"
version = "1.0.12"
dependencies = [
"dojo",
]

[[package]]
name = "dojo_plugin"
version = "2.9.2"
23 changes: 23 additions & 0 deletions examples/benchmark/Scarb.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
[package]
cairo-version = "=2.9.2"
name = "dojo_benchmark"
version = "0.1.0"
edition = "2024_07"

[[target.starknet-contract]]
sierra = true
casm = true
build-external-contracts = ["dojo::world::world_contract::world"]

[dependencies]
dojo = { path = "../../crates/dojo/core" }
starknet = "2.9.2"

[dev-dependencies]
cairo_test = "2.9.2"
dojo_cairo_test = { path = "../../crates/dojo/core-cairo-test" }

[features]
default = []

[profile.sepolia]
4 changes: 4 additions & 0 deletions examples/benchmark/dojo_dev.toml
Copy link
Collaborator

Choose a reason for hiding this comment

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

Keep this file to the minimum, I think only the world is actually needed and the default namespace.

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[world]
description = "Simple world."
name = "simple"
seed = "simple"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
description = "Simple world."
name = "simple"
seed = "simple"
description = "Benchmark world."
name = "bench"
seed = "bench"

Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Ohayo sensei! Inconsistent world configuration in benchmark directory.

The benchmark directory's configuration should use "bench" instead of "simple" to maintain consistency with other benchmark-related files:

  • crates/benches/contracts/dojo_dev.toml uses name="bench", seed="bench"
  • Current file uses name="simple", seed="simple" but is located in benchmark directory
🔗 Analysis chain

Ohayo sensei! Verify world configuration consistency.

The world configuration appears minimal and clean. However, the values differ from those suggested in previous reviews ("simple" vs "bench"). Please ensure these values are consistent with other configuration files and documentation.

Let's check for consistency across the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for "simple" and "bench" in configuration files and documentation
echo "Searching for world name/seed usage..."
rg -g '*.{toml,json,md}' -i 'name.*=.*"?(simple|bench)"?' 
rg -g '*.{toml,json,md}' -i 'seed.*=.*"?(simple|bench)"?'

Length of output: 803

Loading
Loading