-
Notifications
You must be signed in to change notification settings - Fork 174
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(torii): configutation file for all torii cli options #2646
Changes from 5 commits
d6c77c0
3108039
aa985f7
7c19061
cf20035
f671543
8b41cf2
a2c064e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,7 +39,7 @@ use crate::processors::store_update_member::StoreUpdateMemberProcessor; | |
use crate::processors::store_update_record::StoreUpdateRecordProcessor; | ||
use crate::processors::{BlockProcessor, EventProcessor, TransactionProcessor}; | ||
use crate::sql::{Cursors, Sql}; | ||
use crate::types::ContractType; | ||
use crate::types::{Contract, ContractType}; | ||
|
||
type EventProcessorMap<P> = HashMap<Felt, Vec<Box<dyn EventProcessor<P>>>>; | ||
|
||
|
@@ -217,8 +217,10 @@ impl<P: Provider + Send + Sync + std::fmt::Debug + 'static> Engine<P> { | |
config: EngineConfig, | ||
shutdown_tx: Sender<()>, | ||
block_tx: Option<BoundedSender<u64>>, | ||
contracts: Arc<HashMap<Felt, ContractType>>, | ||
contracts: &Vec<Contract>, | ||
) -> Self { | ||
let contracts = Arc::new(contracts.iter().map(|contract| (contract.address, contract.r#type)).collect()); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding contract validation. Ohayo, sensei! The contract initialization lacks validation for duplicate addresses. Add validation to ensure no duplicate contract addresses exist in the input vector. This could lead to undefined behavior if multiple contracts share the same address but have different types. +let mut seen_addresses = HashSet::new();
+for contract in contracts {
+ if !seen_addresses.insert(contract.address) {
+ return Err(anyhow::anyhow!("Duplicate contract address: {:#x}", contract.address));
+ }
+}
let contracts = Arc::new(contracts
.iter()
.map(|contract| (contract.address, contract.r#type))
.collect::<HashMap<_, _>>()); Also applies to: 32-38 |
||
Self { | ||
world: Arc::new(world), | ||
db, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
🛠️ Refactor suggestion
Consider using clap directly instead of clap_config, sensei!
The verification reveals:
clap_config
is only used in one place (bin/torii/src/main.rs
) for a simpleskip
attributeclap
directly, as seen in multiple filesThe functionality can be replaced with clap's built-in attributes, making the dependency unnecessary:
#[clap_config(skip)]
with#[arg(skip)]
🔗 Analysis chain
Consider workspace version management and package stability, sensei!
The
clap_config
dependency has a few concerns:.workspace = true
) like other dependenciesLet's check the package's health:
Consider:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 257
Script:
Length of output: 15413