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

feat(torii-sql): granular model indices #3073

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+355 −218
Open

feat(torii-sql): granular model indices #3073

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
098ce9a
feat(torii-sql): granular model indices
Larkooo Feb 28, 2025
fb0488f
sql config
Larkooo Feb 28, 2025
f60aeef
new iwth config
Larkooo Feb 28, 2025
795330b
fmt
Larkooo Feb 28, 2025
4051f80
allow too many arguments
Larkooo Feb 28, 2025
80e4ea6
add config file support
Larkooo Feb 28, 2025
6c636f8
add tests
Larkooo Feb 28, 2025
2a01d08
fmt
Larkooo Feb 28, 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
37 changes: 36 additions & 1 deletion crates/torii/cli/src/args.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ pub struct ToriiArgs {
#[command(flatten)]
pub erc: ErcOptions,

#[command(flatten)]
pub sql: SqlOptions,

Comment on lines +54 to +56
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

LGTM! Adding SQL options to the CLI interface.

Ohayo! This change looks good, adding a new sql field to ToriiArgs with the #[command(flatten)] attribute to expose SQL options in the CLI.

However, I noticed that while you've added this field to ToriiArgs, there's no corresponding update to ToriiArgsConfig or the methods that handle configuration file loading (with_config_file and the TryFrom<ToriiArgs> implementation).

Run the following script to examine the current pattern of including options in the configuration structure:

🏁 Script executed:

#!/bin/bash
# Check if other flattened options have config counterparts
rg -A 2 "\[command\(flatten\)\]" crates/torii/cli/src/args.rs
rg "Option<\w+Options>" crates/torii/cli/src/args.rs

Length of output: 744

Ohayo, Sensei!
Great job adding the new SQL options to the CLI interface using the flattened attribute. However, our verification shows that unlike the other options (e.g., indexing, events, erc, metrics, server, relay), the sql option lacks a corresponding counterpart in the configuration structure and the associated config file loading methods.

  • Please update ToriiArgsConfig to include an equivalent Option<SqlOptions> field.
  • Also, revise the configuration file loading methods (like with_config_file and the TryFrom<ToriiArgs> implementation) to properly process the new SQL options.

#[cfg(feature = "server")]
#[command(flatten)]
pub metrics: MetricsOptions,
Expand Down Expand Up @@ -107,6 +110,10 @@ impl ToriiArgs {
self.erc = config.erc.unwrap_or_default();
}

if self.sql == SqlOptions::default() {
self.sql = config.sql.unwrap_or_default();
}

#[cfg(feature = "server")]
{
if self.server == ServerOptions::default() {
Expand Down Expand Up @@ -136,6 +143,7 @@ pub struct ToriiArgsConfig {
pub indexing: Option<IndexingOptions>,
pub events: Option<EventsOptions>,
pub erc: Option<ErcOptions>,
pub sql: Option<SqlOptions>,
#[cfg(feature = "server")]
pub metrics: Option<MetricsOptions>,
#[cfg(feature = "server")]
Expand Down Expand Up @@ -167,6 +175,7 @@ impl TryFrom<ToriiArgs> for ToriiArgsConfig {
config.events =
if args.events == EventsOptions::default() { None } else { Some(args.events) };
config.erc = if args.erc == ErcOptions::default() { None } else { Some(args.erc) };
config.sql = if args.sql == SqlOptions::default() { None } else { Some(args.sql) };

#[cfg(feature = "server")]
{
Expand All @@ -187,7 +196,7 @@ mod test {
use std::net::{IpAddr, Ipv4Addr};
use std::str::FromStr;

use torii_sqlite::types::{Contract, ContractType};
use torii_sqlite::types::{Contract, ContractType, ModelIndices};

use super::*;

Expand All @@ -208,6 +217,10 @@ mod test {
"ns-E",
"ns-EH"
]

[[sql.model_indices]]
model_tag = "ns-Position"
fields = ["vec.x", "vec.y"]
"#;
let path = std::env::temp_dir().join("torii-config2.json");
std::fs::write(&path, content).unwrap();
Expand All @@ -225,6 +238,8 @@ mod test {
"--events.historical",
"a-A",
"--indexing.transactions",
"--sql.model_indices",
"ns-Position:vec.x,vec.y;ns-Moves:player",
"--config",
path_str.as_str(),
];
Expand All @@ -238,6 +253,15 @@ mod test {
assert_eq!(torii_args.events.historical, vec!["a-A".to_string()]);
assert_eq!(torii_args.server, ServerOptions::default());
assert!(torii_args.indexing.transactions);
Comment on lines 253 to 255
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix formatting issue flagged by the CI pipeline.

The CI pipeline reports a formatting issue on line 253. Please run cargo fmt to automatically format the code according to Rust's style guidelines.

🧰 Tools
🪛 GitHub Actions: ci

[error] 253-253: Rust formatting check failed. The code does not adhere to the expected formatting style.

assert_eq!(torii_args.sql.model_indices, Some(vec![ModelIndices {
model_tag: "ns-Position".to_string(),
fields: vec!["vec.x".to_string(), "vec.y".to_string()],
},
ModelIndices {
model_tag: "ns-Moves".to_string(),
fields: vec!["player".to_string()],
},
]));
}

#[test]
Expand Down Expand Up @@ -269,6 +293,10 @@ mod test {
"erc721:0x5678"
]
namespaces = []

[[sql.model_indices]]
model_tag = "ns-Position"
fields = ["vec.x", "vec.y"]
"#;
let path = std::env::temp_dir().join("torii-config.json");
std::fs::write(&path, content).unwrap();
Expand Down Expand Up @@ -303,6 +331,13 @@ mod test {
}
]
);
assert_eq!(
torii_args.sql.model_indices,
Some(vec![ModelIndices {
model_tag: "ns-Position".to_string(),
fields: vec!["vec.x".to_string(), "vec.y".to_string()],
}])
);
assert_eq!(torii_args.server.http_addr, IpAddr::V4(Ipv4Addr::LOCALHOST));
assert_eq!(torii_args.server.http_port, 7777);
assert_eq!(torii_args.server.http_cors_origins, Some(vec!["*".to_string()]));
Expand Down
47 changes: 46 additions & 1 deletion crates/torii/cli/src/options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use camino::Utf8PathBuf;
use serde::ser::SerializeSeq;
use serde::{Deserialize, Serialize};
use starknet::core::types::Felt;
use torii_sqlite::types::{Contract, ContractType};
use torii_sqlite::types::{Contract, ContractType, ModelIndices};

pub const DEFAULT_HTTP_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::LOCALHOST);
pub const DEFAULT_HTTP_PORT: u16 = 8080;
Expand Down Expand Up @@ -372,6 +372,51 @@ impl Default for ErcOptions {
}
}

#[derive(Debug, clap::Args, Clone, Serialize, Deserialize, PartialEq)]
#[command(next_help_heading = "SQL options")]
pub struct SqlOptions {
/// Whether model tables should default to having indices on all columns
#[arg(
long = "sql.model_indices_keys",
default_value_t = false,
help = "If true, creates indices on only key fields columns of model tables by default. \
If false, all model field columns will have indices."
)]
#[serde(default)]
pub model_indices_keys: bool,

/// Specify which fields should have indices for specific models
/// Format: "model_name:field1,field2;another_model:field3,field4"
#[arg(
long = "sql.model_indices",
value_delimiter = ';',
value_parser = parse_model_indices,
help = "Specify which fields should have indices for specific models. Format: \"model_name:field1,field2;another_model:field3,field4\""
)]
#[serde(default)]
pub model_indices: Option<Vec<ModelIndices>>,
}

impl Default for SqlOptions {
fn default() -> Self {
Self { model_indices_keys: false, model_indices: None }
}
}

// Parses clap cli argument which is expected to be in the format:
// - model-tag:field1,field2;othermodel-tag:field3,field4
fn parse_model_indices(part: &str) -> anyhow::Result<ModelIndices> {
let parts = part.split(':').collect::<Vec<&str>>();
if parts.len() != 2 {
return Err(anyhow::anyhow!("Invalid model indices format"));
}

let model_tag = parts[0].to_string();
let fields = parts[1].split(',').map(|s| s.to_string()).collect::<Vec<_>>();

Ok(ModelIndices { model_tag, fields })
}

// Parses clap cli argument which is expected to be in the format:
// - erc_type:address:start_block
// - address:start_block (erc_type defaults to ERC20)
Expand Down
5 changes: 3 additions & 2 deletions crates/torii/runner/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use torii_sqlite::cache::ModelCache;
use torii_sqlite::executor::Executor;
use torii_sqlite::simple_broker::SimpleBroker;
use torii_sqlite::types::{Contract, ContractType, Model};
use torii_sqlite::Sql;
use torii_sqlite::{Sql, SqlConfig};
use tracing::{error, info};
use tracing_subscriber::{fmt, EnvFilter};
use url::form_urlencoded;
Expand Down Expand Up @@ -148,11 +148,12 @@ impl Runner {
let executor_handle = tokio::spawn(async move { executor.run().await });

let model_cache = Arc::new(ModelCache::new(readonly_pool.clone()));
let db = Sql::new(
let db = Sql::new_with_config(
pool.clone(),
sender.clone(),
&self.args.indexing.contracts,
model_cache.clone(),
SqlConfig { model_indices: self.args.sql.model_indices.unwrap_or_default() },
)
.await?;

Expand Down
Loading
Loading