fix: add better message for u256 being used as a key (#1769)

dojoengine · Apr 4, 2024 · ba1b250 · ba1b250
1 parent c2a45c9
commit ba1b250
Show file tree

Hide file tree

Showing 4 changed files with 179 additions and 1 deletion.
diff --git a/crates/dojo-lang/src/contract.rs b/crates/dojo-lang/src/contract.rs
@@ -239,7 +239,7 @@ impl DojoContract {
                 if param_modifiers.contains(&"ref".to_string())
                     && param_type.eq(&"ContractState".to_string())
                 {
-                    has_ref_self = false;
+                    has_ref_self = true;
                     add_self = false;
                 }
 

diff --git a/crates/dojo-lang/src/model.rs b/crates/dojo-lang/src/model.rs
@@ -53,6 +53,18 @@ pub fn handle_model_struct(
         });
     }
 
+    for k in &keys {
+        if k.ty == "u256" {
+            diagnostics.push(PluginDiagnostic {
+                message: "Key is only supported for core types that are 1 felt long once \
+                          serialized. `u256` is a struct of 2 u128, hence not supported."
+                    .into(),
+                stable_ptr: struct_ast.name(db).stable_ptr().untyped(),
+                severity: Severity::Error,
+            });
+        }
+    }
+
     let serialize_member = |m: &Member, include_key: bool| {
         if m.key && !include_key {
             return None;

diff --git a/crates/dojo-lang/src/plugin_test_data/model b/crates/dojo-lang/src/plugin_test_data/model
@@ -49,6 +49,12 @@ struct OnlyKeyModel {
     id: felt252
 }
 
+#[derive(Model, Serde)]
+struct U256KeyModel {
+    #[key]
+    id: u256
+}
+
 use starknet::ContractAddress;
 
 #[derive(Model, Copy, Drop, Serde)]
@@ -536,6 +542,16 @@ error: Model must define at least one member that is not a key
 struct OnlyKeyModel {
        ^**********^
 
+error: Model must define at least one member that is not a key
+ --> test_src/lib.cairo:47:8
+struct U256KeyModel {
+       ^**********^
+
+error: Key is only supported for core types that are 1 felt long once serialized. `u256` is a struct of 2 u128, hence not supported.
+ --> test_src/lib.cairo:47:8
+struct U256KeyModel {
+       ^**********^
+
 error: Unsupported attribute.
  --> test_src/lib.cairo[Position]:73:13
             #[starknet::contract]
@@ -551,6 +567,11 @@ error: Unsupported attribute.
             #[starknet::contract]
             ^*******************^
 
+error: Unsupported attribute.
+ --> test_src/lib.cairo[U256KeyModel]:68:13
+            #[starknet::contract]
+            ^*******************^
+
 error: Unsupported attribute.
  --> test_src/lib.cairo[Player]:77:13
             #[starknet::contract]
@@ -601,6 +622,21 @@ error: Unsupported attribute.
                 #[abi(embed_v0)]
                 ^**************^
 
+error: Unsupported attribute.
+ --> test_src/lib.cairo[U256KeyModel]:73:17
+                #[storage]
+                ^********^
+
+error: Unsupported attribute.
+ --> test_src/lib.cairo[U256KeyModel]:76:17
+                #[abi(embed_v0)]
+                ^**************^
+
+error: Unsupported attribute.
+ --> test_src/lib.cairo[U256KeyModel]:104:17
+                #[abi(embed_v0)]
+                ^**************^
+
 error: Unsupported attribute.
  --> test_src/lib.cairo[Player]:82:17
                 #[storage]
@@ -662,6 +698,12 @@ struct OnlyKeyModel {
     id: felt252
 }
 
+#[derive(Model, Serde)]
+struct U256KeyModel {
+    #[key]
+    id: u256
+}
+
 use starknet::ContractAddress;
 
 #[derive(Model, Copy, Drop, Serde)]
@@ -1092,6 +1134,125 @@ impl OnlyKeyModelIntrospect<> of dojo::database::introspect::Introspect<OnlyKeyM
                     }
                 }
             }
+impl U256KeyModelSerde of core::serde::Serde::<U256KeyModel> {
+    fn serialize(self: @U256KeyModel, ref output: core::array::Array<felt252>) {
+        core::serde::Serde::serialize(self.id, ref output)
+    }
+    fn deserialize(ref serialized: core::array::Span<felt252>) -> core::option::Option<U256KeyModel> {
+        core::option::Option::Some(U256KeyModel {
+            id: core::serde::Serde::deserialize(ref serialized)?,
+        })
+    }
+}
+
+            impl U256KeyModelModel of dojo::model::Model<U256KeyModel> {
+                #[inline(always)]
+                fn name(self: @U256KeyModel) -> felt252 {
+                    'U256KeyModel'
+                }
+
+                #[inline(always)]
+                fn keys(self: @U256KeyModel) -> Span<felt252> {
+                    let mut serialized = core::array::ArrayTrait::new();
+                    core::serde::Serde::serialize(self.id, ref serialized);
+                    core::array::ArrayTrait::span(@serialized)
+                }
+
+                #[inline(always)]
+                fn values(self: @U256KeyModel) -> Span<felt252> {
+                    let mut serialized = core::array::ArrayTrait::new();
+
+                    core::array::ArrayTrait::span(@serialized)
+                }
+
+                #[inline(always)]
+                fn layout(self: @U256KeyModel) -> Span<u8> {
+                    let mut layout = core::array::ArrayTrait::new();
+                    dojo::database::introspect::Introspect::<U256KeyModel>::layout(ref layout);
+                    core::array::ArrayTrait::span(@layout)
+                }
+
+                #[inline(always)]
+                fn packed_size(self: @U256KeyModel) -> usize {
+                    let mut layout = self.layout();
+                    dojo::packing::calculate_packed_size(ref layout)
+                }
+            }
+
+
+impl U256KeyModelIntrospect<> of dojo::database::introspect::Introspect<U256KeyModel<>> {
+    #[inline(always)]
+    fn size() -> usize {
+        0
+    }
+
+    #[inline(always)]
+    fn layout(ref layout: Array<u8>) {
+
+    }
+
+    #[inline(always)]
+    fn ty() -> dojo::database::introspect::Ty {
+        dojo::database::introspect::Ty::Struct(dojo::database::introspect::Struct {
+            name: 'U256KeyModel',
+            attrs: array![].span(),
+            children: array![dojo::database::introspect::serialize_member(@dojo::database::introspect::Member {
+                name: 'id',
+                ty: dojo::database::introspect::Ty::Primitive('u256'),
+                attrs: array!['key'].span()
+            })].span()
+        })
+    }
+}
+
+
+            #[starknet::interface]
+            trait Iu_256_key_model<T> {
+                fn ensure_abi(self: @T, model: U256KeyModel);
+            }
+
+            #[starknet::contract]
+            mod u_256_key_model {
+                use super::U256KeyModel;
+                use super::Iu_256_key_model;
+
+                #[storage]
+                struct Storage {}
+
+                #[abi(embed_v0)]
+                impl DojoModelImpl of dojo::model::IDojoModel<ContractState>{
+                    fn name(self: @ContractState) -> felt252 {
+                        'U256KeyModel'
+                    }
+
+                    fn unpacked_size(self: @ContractState) -> usize {
+                        dojo::database::introspect::Introspect::<U256KeyModel>::size()
+                    }
+
+                    fn packed_size(self: @ContractState) -> usize {
+                        let mut layout = core::array::ArrayTrait::new();
+                        dojo::database::introspect::Introspect::<U256KeyModel>::layout(ref layout);
+                        let mut layout_span = layout.span();
+                        dojo::packing::calculate_packed_size(ref layout_span)
+                    }
+
+                    fn layout(self: @ContractState) -> Span<u8> {
+                        let mut layout = core::array::ArrayTrait::new();
+                        dojo::database::introspect::Introspect::<U256KeyModel>::layout(ref layout);
+                        core::array::ArrayTrait::span(@layout)
+                    }
+
+                    fn schema(self: @ContractState) -> dojo::database::introspect::Ty {
+                        dojo::database::introspect::Introspect::<U256KeyModel>::ty()
+                    }
+                }
+
+                #[abi(embed_v0)]
+                impl u_256_key_modelImpl of Iu_256_key_model<ContractState>{
+                    fn ensure_abi(self: @ContractState, model: U256KeyModel) {
+                    }
+                }
+            }
 impl PlayerCopy of core::traits::Copy::<Player>;
 impl PlayerDrop of core::traits::Drop::<Player>;
 impl PlayerSerde of core::serde::Serde::<Player> {

diff --git a/crates/dojo-lang/src/plugin_test_data/system b/crates/dojo-lang/src/plugin_test_data/system
@@ -319,6 +319,11 @@ error: Functions of dojo::interface cannot have `ref self` parameter.
     fn do_ref_self(ref self: TContractState);
     ^***************************************^
 
+error: Functions of dojo::contract cannot have 'ref self' parameter.
+ --> test_src/lib.cairo:119:9
+        fn do_with_ref_self(ref self: ContractState) -> felt252 {
+        ^*******************************************************^
+
 error: Only one parameter of type IWorldDispatcher is allowed.
  --> test_src/lib.cairo:123:9
         fn do_with_several_world_dispatchers(