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

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
36 changes: 24 additions & 12 deletions bin/sozo/src/commands/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,11 @@ async fn clone_permissions(
.external_writers
.iter()
.filter_map(|(resource_selector, writers)| {
if writers.contains(&from_address) { Some(*resource_selector) } else { None }
if writers.contains(&from_address) {
Some(*resource_selector)
} else {
None
}
})
.collect();

Expand All @@ -251,7 +255,11 @@ async fn clone_permissions(
.external_owners
.iter()
.filter_map(|(resource_selector, owners)| {
if owners.contains(&from_address) { Some(*resource_selector) } else { None }
if owners.contains(&from_address) {
Some(*resource_selector)
} else {
None
}
})
.collect();

Expand Down Expand Up @@ -303,16 +311,20 @@ async fn clone_permissions(
writers_resource_selectors.extend(external_writer_of.iter().copied());
owners_resource_selectors.extend(external_owner_of.iter().copied());

writer_of.extend(
external_writer_of
.iter()
.map(|r| if r != &WORLD { format!("{:#066x}", r) } else { "World".to_string() }),
);
owner_of.extend(
external_owner_of
.iter()
.map(|r| if r != &WORLD { format!("{:#066x}", r) } else { "World".to_string() }),
);
writer_of.extend(external_writer_of.iter().map(|r| {
if r != &WORLD {
format!("{:#066x}", r)
} else {
"World".to_string()
}
}));
owner_of.extend(external_owner_of.iter().map(|r| {
if r != &WORLD {
format!("{:#066x}", r)
} else {
"World".to_string()
}
}));

// Sort the tags to have a deterministic output.
let mut writer_of = writer_of.into_iter().collect::<Vec<_>>();
Expand Down
6 changes: 4 additions & 2 deletions bin/sozo/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ pub enum Commands {
Build(Box<BuildArgs>),
#[command(about = "Build and migrate the world every time a file changes")]
Dev(Box<DevArgs>),
#[command(about = "Run a migration, declaring and deploying contracts as necessary to update \
the world")]
#[command(
about = "Run a migration, declaring and deploying contracts as necessary to update \
the world"
)]
Migrate(Box<MigrateArgs>),
#[command(about = "Execute a system with the given calldata.")]
Execute(Box<ExecuteArgs>),
Expand Down
46 changes: 43 additions & 3 deletions bin/sozo/src/commands/model.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use anyhow::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 Down Expand Up @@ -109,8 +110,11 @@ hashes, called 'hash' in the following documentation.

#[arg(value_name = "KEYS")]
#[arg(value_delimiter = ',')]
#[arg(help = "Comma seperated values e.g., 0x12345,0x69420,...")]
keys: Vec<Felt>,
#[arg(help = "Comma separated values e.g., \
0x12345,0x69420,sstr:\"hello\",sstr:\"misty\". Supported prefixes:\n \
- sstr: A cairo short string\n \
- no prefix: A cairo felt")]
Comment on lines +113 to +116
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the case of the keys, only one felt long serialized type are supported (everything that can fit in one felt).

That's great you've only put the sstr which is the only one supported actually.

Suggested change
#[arg(help = "Comma separated values e.g., \
0x12345,0x69420,sstr:\"hello\",sstr:\"misty\". Supported prefixes:\n \
- sstr: A cairo short string\n \
- no prefix: A cairo felt")]
#[arg(help = "Comma separated values e.g., \
0x12345,0x69420,sstr:\"hello\". Supported prefixes:\n \
- sstr: A cairo short string\n \
- no prefix: A cairo felt")]

Copy link
Collaborator

Choose a reason for hiding this comment

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

We may add a check that only this prefix is actually used, wdyt?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, the check will be quite useful. I'll add it.

keys: String,

#[command(flatten)]
world: WorldOptions,
Expand All @@ -119,7 +123,9 @@ hashes, called 'hash' in the following documentation.
starknet: StarknetOptions,

#[arg(short, long)]
#[arg(help = "Block number at which to retrieve the model data (pending block by default)")]
#[arg(
help = "Block number at which to retrieve the model data (pending block by default)"
)]
block: Option<u64>,
},
}
Expand Down Expand Up @@ -205,6 +211,8 @@ impl ModelArgs {
let (world_diff, provider, _) =
utils::get_world_diff_and_provider(starknet, world, &ws).await?;

let keys = calldata_decoder::decode_calldata(&keys)?;

let (record, _, _) = model::model_get(
tag.to_string(),
keys,
Expand All @@ -222,3 +230,35 @@ impl ModelArgs {
})
}
}

#[cfg(test)]
mod tests {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for thinking about tests.

In the current context, this test should already be covered into the calldata_decoder file. We can remove this one. 👍

You could write a test on the argument though.

Copy link
Author

Choose a reason for hiding this comment

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

Awesome, that'll be the better option. These tests were meant to go away eventually.

use super::*;
use starknet::core::utils::cairo_short_string_to_felt;

#[test]
fn test_short_string_equals_felt() {
// Test that sstr:"misty" equals 0x6d69737479
let with_prefix = "sstr:\"misty\"";
let with_hex = "0x6d69737479";

let felt_from_string = calldata_decoder::decode_calldata(with_prefix).unwrap();
let felt_from_hex = calldata_decoder::decode_calldata(with_hex).unwrap();

assert_eq!(felt_from_string, felt_from_hex);
assert_eq!(felt_from_string[0], Felt::from_hex_str("0x6d69737479").unwrap());
}

#[test]
fn test_hex_equals_decimal() {
// Test that 0x6d69737479 equals 469920609401
let with_hex = "0x6d69737479";
let with_decimal = "469920609401";

let felt_from_hex = calldata_decoder::decode_calldata(with_hex).unwrap();
let felt_from_decimal = calldata_decoder::decode_calldata(with_decimal).unwrap();

assert_eq!(felt_from_hex, felt_from_decimal);
assert_eq!(felt_from_hex[0], Felt::from(469920609401u128));
}
}
Loading