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.

Sign up for GitHub

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

fix(sozo): model get not using prefixes #2867

Merged
merged 27 commits into from
Feb 5, 2025
Merged

fix(sozo): model get not using prefixes #2867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
69d83d0
fix(sozo): model get not using prefixes
emarc99 Jan 6, 2025
f34a85e
rewrote model get key parser
emarc99 Jan 16, 2025
2df1d0e
Merge branch 'dojoengine:main' into fix-sozo-model-prefix
emarc99 Jan 16, 2025
fee9cfb
Merge branch 'fix-sozo-model-prefix' of https://github.com/emarc99/do…
emarc99 Jan 16, 2025
2b0dac4
handle the potential empty felts
emarc99 Jan 16, 2025
183ae33
rust fmt
emarc99 Jan 17, 2025
efe1905
Merge branch 'main' into fix-sozo-model-prefix
emarc99 Jan 17, 2025
7d3d719
Merge branch 'fix-sozo-model-prefix' of https://github.com/emarc99/do…
emarc99 Jan 17, 2025
018847f
rust fmt
emarc99 Jan 17, 2025
47df4b2
rust fmt
emarc99 Jan 17, 2025
cfa44f4
Merge branch 'main' into fix-sozo-model-prefix
emarc99 Jan 17, 2025
70a8da2
Merge branch 'main' into fix-sozo-model-prefix
emarc99 Jan 23, 2025
0af4d74
Merge branch 'main' into fix-sozo-model-prefix
emarc99 Jan 25, 2025
860c6c0
Merge branch 'main' into fix-sozo-model-prefix
emarc99 Jan 29, 2025
4fff908
Merge branch 'main' into fix-sozo-model-prefix
emarc99 Jan 29, 2025
ef3144b
Merge branch 'main' into fix-sozo-model-prefix
emarc99 Jan 30, 2025
597ba88
add support for all prfixes
emarc99 Jan 30, 2025
42a8b53
Merge branch 'fix-sozo-model-prefix' of https://github.com/emarc99/do…
emarc99 Jan 30, 2025
0cfa683
updated help text of model get keys to show supported prefixes
emarc99 Jan 30, 2025
d6d84fe
Merge branch 'main' into fix-sozo-model-prefix
emarc99 Jan 30, 2025
aa29464
remove trimming of quotes
emarc99 Jan 30, 2025
c0141d0
Merge branch 'fix-sozo-model-prefix' of https://github.com/emarc99/do…
emarc99 Jan 30, 2025
265cd5c
Merge remote-tracking branch 'dojo/main' into fix-sozo-model-prefix
glihm Feb 5, 2025
039ad49
refactor to use new calldata
glihm Feb 5, 2025
10d9419
add tests for quotes management in calldata decoder
glihm Feb 5, 2025
02ee540
Merge remote-tracking branch 'dojo/main' into fix-sozo-model-prefix
glihm Feb 5, 2025
acaea86
update test db and policies
glihm Feb 5, 2025
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
164 changes: 159 additions & 5 deletions bin/sozo/src/commands/model.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use anyhow::Result;
use anyhow::{Context, Result};
use clap::{Args, Subcommand};
use dojo_world::config::calldata_decoder;
use scarb::core::Config;
use sozo_ops::model;
use sozo_ops::resource_descriptor::ResourceDescriptor;
Expand All @@ -10,6 +11,7 @@ use tracing::trace;
use super::options::starknet::StarknetOptions;
use super::options::world::WorldOptions;
use crate::utils;
use crate::utils::CALLDATA_DOC;

#[derive(Debug, Args)]
pub struct ModelArgs {
Expand Down Expand Up @@ -108,9 +110,12 @@ hashes, called 'hash' in the following documentation.
tag_or_name: ResourceDescriptor,

#[arg(value_name = "KEYS")]
#[arg(value_delimiter = ',')]
#[arg(help = "Comma seperated values e.g., 0x12345,0x69420,...")]
keys: Vec<Felt>,
#[arg(num_args = 1..)]
#[arg(required = true)]
#[arg(
help = format!("List of values representing the serialized keys of the model.\n{CALLDATA_DOC}")
)]
keys: Vec<String>,

#[command(flatten)]
world: WorldOptions,
Expand Down Expand Up @@ -207,7 +212,7 @@ impl ModelArgs {

let (record, _, _) = model::model_get(
tag.to_string(),
keys,
parse_keys(&keys)?,
world_diff.world_info.address,
&provider,
block_id,
Expand All @@ -222,3 +227,152 @@ impl ModelArgs {
})
}
}

/// Parses the keys from the command line into a vector of Felt representing the serialized keys of
/// the model.
fn parse_keys(keys: &[String]) -> Result<Vec<Felt>> {
let mut keys_serde = vec![];

for key in keys {
let key_felt = calldata_decoder::decode_single_calldata(key)
.with_context(|| format!("Failed to decode key: {}", key))?;
keys_serde.extend(key_felt);
}

Ok(keys_serde)
}

Comment on lines +231 to +243
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 validation to the parse_keys function.

The function should validate empty results and supported prefixes to prevent runtime errors.

Apply this diff to add validation:

 fn parse_keys(keys: &[String]) -> Result<Vec<Felt>> {
     let mut keys_serde = vec![];
 
     for key in keys {
         let key_felt = calldata_decoder::decode_single_calldata(key)
             .with_context(|| format!("Failed to decode key: {}", key))?;
+        if key_felt.is_empty() {
+            anyhow::bail!("Failed to parse key '{}': no values returned", key);
+        }
+        
+        // Validate prefix if present
+        if key.contains(':') {
+            let prefix = key.split(':').next().unwrap();
+            if !["u256", "sstr", "str", "int"].contains(&prefix) {
+                anyhow::bail!("Unsupported prefix '{}' in key '{}'", prefix, key);
+            }
+        }
         keys_serde.extend(key_felt);
     }
 
     Ok(keys_serde)
 }
📝 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
/// Parses the keys from the command line into a vector of Felt representing the serialized keys of
/// the model.
fn parse_keys(keys: &[String]) -> Result<Vec<Felt>> {
let mut keys_serde = vec![];
for key in keys {
let key_felt = calldata_decoder::decode_single_calldata(key)
.with_context(|| format!("Failed to decode key: {}", key))?;
keys_serde.extend(key_felt);
}
Ok(keys_serde)
}
fn parse_keys(keys: &[String]) -> Result<Vec<Felt>> {
let mut keys_serde = vec![];
for key in keys {
let key_felt = calldata_decoder::decode_single_calldata(key)
.with_context(|| format!("Failed to decode key: {}", key))?;
if key_felt.is_empty() {
anyhow::bail!("Failed to parse key '{}': no values returned", key);
}
// Validate prefix if present
if key.contains(':') {
let prefix = key.split(':').next().unwrap();
if !["u256", "sstr", "str", "int"].contains(&prefix) {
anyhow::bail!("Unsupported prefix '{}' in key '{}'", prefix, key);
}
}
keys_serde.extend(key_felt);
}
Ok(keys_serde)
}

#[cfg(test)]
mod tests {
// To do: Add more tests for the flattening of keys
glihm marked this conversation as resolved.
Show resolved Hide resolved
// let flattened_keys: Vec<Felt> = keys.into_iter().flatten().collect();

use clap::Parser;
use starknet::core::utils::cairo_short_string_to_felt;
use starknet::macros::felt;

use super::*;

#[derive(Parser, Debug)]
struct TestCommand {
#[command(subcommand)]
command: ModelCommand,
}

#[test]
fn test_model_get_argument_parsing() {
// Test parsing with hex
let args = TestCommand::parse_from([
"model",
"get",
"Account",
"0x054cb935d86d80b5a0a6e756edf448ab33876d01dd2b07a2a4e63a41e06d0ef5",
"0x6d69737479",
]);

if let ModelCommand::Get { keys, .. } = args.command {
let expected = vec![
felt!("0x054cb935d86d80b5a0a6e756edf448ab33876d01dd2b07a2a4e63a41e06d0ef5"),
felt!("0x6d69737479"),
];

assert_eq!(parse_keys(&keys).unwrap(), expected);
} else {
panic!("Expected Get command");
}

// Test parsing with short string prefix
let args = TestCommand::parse_from([
"model",
"get",
"Account",
"0x054cb935d86d80b5a0a6e756edf448ab33876d01dd2b07a2a4e63a41e06d0ef5",
"sstr:\"misty\"",
]);

if let ModelCommand::Get { keys, .. } = args.command {
let expected = vec![
felt!("0x054cb935d86d80b5a0a6e756edf448ab33876d01dd2b07a2a4e63a41e06d0ef5"),
cairo_short_string_to_felt("misty").unwrap(),
];

assert_eq!(parse_keys(&keys).unwrap(), expected);
} else {
panic!("Expected Get command");
}

// Test parsing with u256 prefix
let args = TestCommand::parse_from([
"model",
"get",
"Account",
"0x054cb935d86d80b5a0a6e756edf448ab33876d01dd2b07a2a4e63a41e06d0ef5",
"u256:0x1",
]);

if let ModelCommand::Get { keys, .. } = args.command {
let expected = vec![
felt!("0x054cb935d86d80b5a0a6e756edf448ab33876d01dd2b07a2a4e63a41e06d0ef5"),
Felt::ONE,
Felt::ZERO,
];

assert_eq!(parse_keys(&keys).unwrap(), expected);
} else {
panic!("Expected Get command");
}

// Test parsing with int prefix
let args = TestCommand::parse_from(["model", "get", "Account", "int:-123456789"]);

if let ModelCommand::Get { keys, .. } = args.command {
let expected = vec![(-123456789_i64).into()];

assert_eq!(parse_keys(&keys).unwrap(), expected);
} else {
panic!("Expected Get command");
}

// Test parsing with str prefix
let args = TestCommand::parse_from(["model", "get", "Account", "str:hello"]);

if let ModelCommand::Get { keys, .. } = args.command {
let expected = vec![
Felt::ZERO,
cairo_short_string_to_felt("hello").unwrap(),
Felt::from_dec_str("5").unwrap(),
];

assert_eq!(parse_keys(&keys).unwrap(), expected);
} else {
panic!("Expected Get command");
}

// Test parsing with all prefixes
let args = TestCommand::parse_from([
"model",
"get",
"Account",
"0x054cb935d86d80b5a0a6e756edf448ab33876d01dd2b07a2a4e63a41e06d0ef5",
"u256:0x1",
"int:-123456789",
"str:hello",
]);

if let ModelCommand::Get { keys, .. } = args.command {
let expected = vec![
felt!("0x054cb935d86d80b5a0a6e756edf448ab33876d01dd2b07a2a4e63a41e06d0ef5"),
Felt::ONE,
Felt::ZERO,
(-123456789_i64).into(),
Felt::ZERO,
cairo_short_string_to_felt("hello").unwrap(),
Felt::from_dec_str("5").unwrap(),
];

assert_eq!(parse_keys(&keys).unwrap(), expected);
} else {
panic!("Expected Get command");
}
}
}
130 changes: 65 additions & 65 deletions bin/sozo/tests/test_data/policies.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,134 +1,134 @@
[
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "uuid"
"target": "0x780e3207b4f11b56f32cc0f19975af5b3a4df3cad6a4b0ab59a1702ba0304d8",
"method": "upgrade"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "set_metadata"
"target": "0x7447baef53fdcc376b73963aa2bd3b0894be7d5bd40f596cc44d1d54d80ea52",
"method": "spawn"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "register_namespace"
"target": "0x7447baef53fdcc376b73963aa2bd3b0894be7d5bd40f596cc44d1d54d80ea52",
"method": "move"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "register_event"
"target": "0x7447baef53fdcc376b73963aa2bd3b0894be7d5bd40f596cc44d1d54d80ea52",
"method": "set_player_config"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "register_model"
"target": "0x7447baef53fdcc376b73963aa2bd3b0894be7d5bd40f596cc44d1d54d80ea52",
"method": "update_player_config_name"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "register_contract"
"target": "0x7447baef53fdcc376b73963aa2bd3b0894be7d5bd40f596cc44d1d54d80ea52",
"method": "reset_player_config"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "init_contract"
"target": "0x7447baef53fdcc376b73963aa2bd3b0894be7d5bd40f596cc44d1d54d80ea52",
"method": "set_player_server_profile"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "upgrade_event"
"target": "0x7447baef53fdcc376b73963aa2bd3b0894be7d5bd40f596cc44d1d54d80ea52",
"method": "set_models"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "upgrade_model"
"target": "0x7447baef53fdcc376b73963aa2bd3b0894be7d5bd40f596cc44d1d54d80ea52",
"method": "enter_dungeon"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "upgrade_contract"
"target": "0x7447baef53fdcc376b73963aa2bd3b0894be7d5bd40f596cc44d1d54d80ea52",
"method": "upgrade"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "emit_event"
"target": "0xca72f1cd782b614fa12c8b54afa895a169a4de1792738d4e3f09d0929f7834",
"method": "upgrade"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "emit_events"
"target": "0x608ffd2e6b74a7bede256770ebe3d07bc65c79622e6a9396ea764011152102",
"method": "upgrade"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "set_entity"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "uuid"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "set_entities"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "set_metadata"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "delete_entity"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "register_namespace"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "delete_entities"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "register_event"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "grant_owner"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "register_model"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "revoke_owner"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "register_contract"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "grant_writer"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "init_contract"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "revoke_writer"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "upgrade_event"
},
{
"target": "0x736c456dec44741fad1940876e62342a626ca73fd4ac574a94eb3ded2e25d8d",
"method": "upgrade"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "upgrade_model"
},
{
"target": "0xe1fb33e10629d61ac3eb7c9bdca6a0ce947bcf7ecf598fbc0c42f9c76b3808",
"method": "upgrade"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "upgrade_contract"
},
{
"target": "0x77a851ab985e88cbfdaee77a7c10e7afde9c9c3e49aa9ec2537e41de44e857c",
"method": "upgrade"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "emit_event"
},
{
"target": "0x4ba8772b4785c0afce5b73ed98d30cf8832e3bfcceff5a688b085ef6d0f164e",
"method": "spawn"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "emit_events"
},
{
"target": "0x4ba8772b4785c0afce5b73ed98d30cf8832e3bfcceff5a688b085ef6d0f164e",
"method": "move"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "set_entity"
},
{
"target": "0x4ba8772b4785c0afce5b73ed98d30cf8832e3bfcceff5a688b085ef6d0f164e",
"method": "set_player_config"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "set_entities"
},
{
"target": "0x4ba8772b4785c0afce5b73ed98d30cf8832e3bfcceff5a688b085ef6d0f164e",
"method": "update_player_config_name"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "delete_entity"
},
{
"target": "0x4ba8772b4785c0afce5b73ed98d30cf8832e3bfcceff5a688b085ef6d0f164e",
"method": "reset_player_config"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "delete_entities"
},
{
"target": "0x4ba8772b4785c0afce5b73ed98d30cf8832e3bfcceff5a688b085ef6d0f164e",
"method": "set_player_server_profile"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "grant_owner"
},
{
"target": "0x4ba8772b4785c0afce5b73ed98d30cf8832e3bfcceff5a688b085ef6d0f164e",
"method": "set_models"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "revoke_owner"
},
{
"target": "0x4ba8772b4785c0afce5b73ed98d30cf8832e3bfcceff5a688b085ef6d0f164e",
"method": "enter_dungeon"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "grant_writer"
},
{
"target": "0x4ba8772b4785c0afce5b73ed98d30cf8832e3bfcceff5a688b085ef6d0f164e",
"method": "upgrade"
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "revoke_writer"
},
{
"target": "0x3032d716e69f67e05983edad3d3b5b8efa9b08d09c778e2eecf224e095a1160",
"target": "0x5baea2d83fc19bae80dc5d4626a27b2b2d5012822cd862c56ed7007eb92eaa2",
"method": "upgrade"
},
{
Expand Down
Loading
Loading