Skip to content

Commit

Permalink
fix(ci): exit with non-zero status code if any function call fails (#…
Browse files Browse the repository at this point in the history
…1853)

* fix(ci): exit with non-zero status code if any function call fails

* fix: clippy lints

* fix: more clippy lints
  • Loading branch information
lambda-0x authored Apr 21, 2024
1 parent d92decd commit 010444f
Show file tree
Hide file tree
Showing 13 changed files with 53 additions and 61 deletions.
2 changes: 1 addition & 1 deletion bin/sozo/src/commands/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl BuildArgs {
if self.stats {
let target_dir = &compile_info.target_dir;
let contracts_statistics = get_contract_statistics_for_dir(target_dir)
.context(format!("Error getting contracts stats"))?;
.context("Error getting contracts stats")?;
let table = create_stats_table(contracts_statistics);
table.printstd()
}
Expand Down
6 changes: 3 additions & 3 deletions bin/sozo/src/commands/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,13 @@ mod tests {
let manifest_toml = profile_manifests_dir.join("manifest").with_extension("toml");
let manifest_json = profile_manifests_dir.join("manifest").with_extension("json");

assert!(fs::read_dir(&target_dev_dir).is_err(), "Expected 'target/dev' to be empty");
assert!(fs::read_dir(target_dev_dir).is_err(), "Expected 'target/dev' to be empty");
assert!(
fs::read_dir(&manifests_dev_base_dir).is_err(),
fs::read_dir(manifests_dev_base_dir).is_err(),
"Expected 'manifests/dev/base' to be empty"
);
assert!(
fs::read_dir(&manifests_dev_abis_base_dir).is_err(),
fs::read_dir(manifests_dev_abis_base_dir).is_err(),
"Expected 'manifests/dev/abis/base' to be empty"
);
assert!(
Expand Down
10 changes: 3 additions & 7 deletions bin/sozo/src/commands/options/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,10 @@ impl SignerOptions {
.or_else(|| env_metadata.and_then(|env| env.keystore_password()))
{
password.to_owned()
} else if no_wait {
return Err(anyhow!("Could not find password. Please specify the password."));
} else {
if no_wait {
return Err(anyhow!(
"Could not find password. Please specify the password."
));
} else {
rpassword::prompt_password("Enter password: ")?
}
rpassword::prompt_password("Enter password: ")?
}
};
let private_key = SigningKey::from_keystore(path, &password)?;
Expand Down
2 changes: 1 addition & 1 deletion crates/dojo-bindgen/src/plugins/unity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ public class {} : ModelInstance {{
fn map_type(token: &Token) -> String {
match token {
Token::CoreBasic(t) => UnityPlugin::map_type(&t.type_name()),
Token::Composite(t) => format!("{}", t.type_name()),
Token::Composite(t) => t.type_name().to_string(),
Token::Array(t) => format!("{}[]", map_type(&t.inner)),
_ => panic!("Unsupported token type: {:?}", token),
}
Expand Down
16 changes: 8 additions & 8 deletions crates/dojo-test-utils/src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub fn copy_build_project_temp(
assert_fs::TempDir::new().unwrap().to_path_buf().to_string_lossy().to_string(),
);

let temp_project_path = temp_project_dir.join(&"Scarb").with_extension("toml").to_string();
let temp_project_path = temp_project_dir.join("Scarb").with_extension("toml").to_string();

copy_project_temp(&source_project_dir, &temp_project_dir).unwrap();

Expand Down Expand Up @@ -66,13 +66,13 @@ pub fn copy_project_temp(
source_dir: &Utf8PathBuf,
destination_dir: &Utf8PathBuf,
) -> io::Result<()> {
let ignore_dirs = vec!["manifests", "target"];
let ignore_dirs = ["manifests", "target"];

if !destination_dir.exists() {
fs::create_dir_all(&destination_dir)?;
fs::create_dir_all(destination_dir)?;
}

for entry in fs::read_dir(&source_dir)? {
for entry in fs::read_dir(source_dir)? {
let entry = entry?;
let path = entry.path();
if path.is_dir() {
Expand All @@ -99,7 +99,7 @@ pub fn copy_project_temp(
let mut contents = String::new();
File::open(&path)
.and_then(|mut file| file.read_to_string(&mut contents))
.expect(&format!("Failed to read {file_name}"));
.unwrap_or_else(|_| panic!("Failed to read {file_name}"));

let mut table = contents.parse::<Table>().expect("Failed to parse Scab.toml");

Expand Down Expand Up @@ -211,21 +211,21 @@ mod tests {

// Create a file in the project directory
let file_path = project_dir.join("file.txt");
let mut file = File::create(&file_path).unwrap();
let mut file = File::create(file_path).unwrap();
writeln!(file, "Hello, world!").unwrap();

// Create a subdirectory with a file in the project directory
let sub_dir = project_dir.join("subdir");
fs::create_dir(&sub_dir).unwrap();
let sub_file_path = sub_dir.join("subfile.txt");
let mut sub_file = File::create(&sub_file_path).unwrap();
let mut sub_file = File::create(sub_file_path).unwrap();
writeln!(sub_file, "Hello, from subdir!").unwrap();

// Create a subdir that should be ignored
let ignored_sub_dir = project_dir.join("manifests");
fs::create_dir(&ignored_sub_dir).unwrap();
let ignored_sub_file_path = ignored_sub_dir.join("ignored_file.txt");
let mut ignored_sub_file = File::create(&ignored_sub_file_path).unwrap();
let mut ignored_sub_file = File::create(ignored_sub_file_path).unwrap();
writeln!(ignored_sub_file, "This should be ignored!").unwrap();

// Perform the copy
Expand Down
8 changes: 4 additions & 4 deletions crates/dojo-world/src/migration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ pub trait Declarable {

let DeclareTransactionResult { transaction_hash, class_hash } = account
.declare(Arc::new(flattened_class), casm_class_hash)
.send_with_cfg(&txn_config)
.send_with_cfg(txn_config)
.await
.map_err(MigrationError::Migrator)?;

Expand Down Expand Up @@ -197,7 +197,7 @@ pub trait Deployable: Declarable + Sync {

let InvokeTransactionResult { transaction_hash } = account
.execute(vec![call])
.send_with_cfg(&txn_config)
.send_with_cfg(txn_config)
.await
.map_err(MigrationError::Migrator)?;

Expand Down Expand Up @@ -268,7 +268,7 @@ pub trait Deployable: Declarable + Sync {
}]);

let InvokeTransactionResult { transaction_hash } =
txn.send_with_cfg(&txn_config).await.map_err(MigrationError::Migrator)?;
txn.send_with_cfg(txn_config).await.map_err(MigrationError::Migrator)?;

let receipt = TransactionWaiter::new(transaction_hash, account.provider()).await?;
let block_number = get_block_number_from_receipt(receipt);
Expand Down Expand Up @@ -329,7 +329,7 @@ pub trait Upgradable: Deployable + Declarable + Sync {

let InvokeTransactionResult { transaction_hash } = account
.execute(vec![Call { calldata, selector: selector!("upgrade"), to: contract_address }])
.send_with_cfg(&txn_config)
.send_with_cfg(txn_config)
.await
.map_err(MigrationError::Migrator)?;

Expand Down
24 changes: 10 additions & 14 deletions crates/saya/core/src/prover/program_input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,18 @@ fn get_messages_recursively(info: &CallInfo) -> Vec<MessageToStarknet> {
}

pub fn extract_messages(
exec_infos: &Vec<TxExecInfo>,
exec_infos: &[TxExecInfo],
mut transactions: Vec<&L1HandlerTx>,
) -> (Vec<MessageToStarknet>, Vec<MessageToAppchain>) {
let message_to_starknet_segment = exec_infos
.iter()
.map(|t| t.execute_call_info.iter().chain(t.validate_call_info.iter()).chain(t.fee_transfer_call_info.iter())) // Take into account both validate and execute calls.
.flatten()
.map(get_messages_recursively)
.flatten()
.flat_map(|t| t.execute_call_info.iter().chain(t.validate_call_info.iter()).chain(t.fee_transfer_call_info.iter())) // Take into account both validate and execute calls.
.flat_map(get_messages_recursively)
.collect();

let message_to_appchain_segment = exec_infos
.iter()
.map(|t| t.execute_call_info.iter())
.flatten()
.flat_map(|t| t.execute_call_info.iter())
.filter(|c| c.entry_point_type == EntryPointType::L1Handler)
.map(|c| {
let message_hash =
Expand All @@ -71,10 +68,9 @@ pub fn extract_messages(
&& c.contract_address == t.contract_address
&& c.calldata == t.calldata
})
.expect(&format!(
"No matching transaction found for message hash: {}",
message_hash
))
.unwrap_or_else(|| {
panic!("No matching transaction found for message hash: {}", message_hash)
})
.0;

// Removing, to have different nonces, even for the same message content.
Expand Down Expand Up @@ -129,7 +125,7 @@ impl ProgramInput {

result.push_str(&state_updates_to_json_like(&self.state_updates));

result.push_str(&format!("{}", "}"));
result.push('}');

Ok(result)
}
Expand All @@ -145,7 +141,7 @@ pub struct MessageToStarknet {
impl MessageToStarknet {
pub fn serialize(&self) -> anyhow::Result<Vec<FieldElement>> {
let mut result = vec![*self.from_address, *self.to_address];
result.push(FieldElement::try_from(self.payload.len())?);
result.push(FieldElement::from(self.payload.len()));
result.extend(self.payload.iter().cloned());
Ok(result)
}
Expand All @@ -163,7 +159,7 @@ pub struct MessageToAppchain {
impl MessageToAppchain {
pub fn serialize(&self) -> anyhow::Result<Vec<FieldElement>> {
let mut result = vec![*self.from_address, *self.to_address, self.nonce, self.selector];
result.push(FieldElement::try_from(self.payload.len())?);
result.push(FieldElement::from(self.payload.len()));
result.extend(self.payload.iter().cloned());
Ok(result)
}
Expand Down
5 changes: 3 additions & 2 deletions crates/sozo/ops/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ pub async fn new(signer: LocalWallet, force: bool, file: PathBuf) -> Result<()>
Ok(())
}

#[allow(clippy::too_many_arguments)]
pub async fn deploy(
provider: JsonRpcClient<HttpTransport>,
signer: LocalWallet,
Expand Down Expand Up @@ -284,7 +285,7 @@ pub async fn deploy(

if simulate {
simulate_account_deploy(&account_deployment).await?;
return Ok(());
Ok(())
} else {
do_account_deploy(
max_fee,
Expand Down Expand Up @@ -398,7 +399,7 @@ async fn simulate_account_deploy(
colored_json::to_colored_json(&simulation_json, ColorMode::Auto(Output::StdOut))?;

println!("{simulation_json}");
return Ok(());
Ok(())
}

pub async fn fetch(
Expand Down
3 changes: 2 additions & 1 deletion crates/sozo/ops/src/migration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ where
Ok(())
}

#[allow(clippy::too_many_arguments)]
async fn update_manifests_and_abis(
ws: &Workspace<'_>,
local_manifest: BaseManifest,
Expand Down Expand Up @@ -723,7 +724,7 @@ where
.collect::<Vec<_>>();

let InvokeTransactionResult { transaction_hash } =
world.account.execute(calls).send_with_cfg(&txn_config).await.map_err(|e| {
world.account.execute(calls).send_with_cfg(txn_config).await.map_err(|e| {
ui.verbose(format!("{e:?}"));
anyhow!("Failed to register models to World: {e}")
})?;
Expand Down
6 changes: 3 additions & 3 deletions crates/sozo/ops/src/statistics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ fn get_contract_statistics_for_file(
sierra_json_file: File,
contract_artifact: FlattenedSierraClass,
) -> Result<ContractStatistics> {
let file_size = get_file_size(&sierra_json_file).context(format!("Error getting file size"))?;
let file_size = get_file_size(&sierra_json_file).context("Error getting file size")?;
let number_felts = get_sierra_byte_code_size(contract_artifact);
Ok(ContractStatistics { file_size, contract_name, number_felts })
}
Expand Down Expand Up @@ -136,7 +136,7 @@ mod tests {
let target_dir = Utf8PathBuf::from(TEST_SIERRA_FOLDER_CONTRACTS);

let contract_statistics = get_contract_statistics_for_dir(&target_dir)
.expect(format!("Error getting contracts in dir {target_dir}").as_str());
.unwrap_or_else(|_| panic!("Error getting contracts in dir {target_dir}"));

assert_eq!(contract_statistics.len(), 1, "Mismatch number of contract statistics");
}
Expand All @@ -148,7 +148,7 @@ mod tests {
const EXPECTED_SIZE: u64 = 114925;

let file_size = get_file_size(&sierra_json_file)
.expect(format!("Error getting file size for test file").as_str());
.unwrap_or_else(|_| panic!("Error getting file size for test file"));

assert_eq!(file_size, EXPECTED_SIZE, "File size mismatch");
}
Expand Down
23 changes: 7 additions & 16 deletions crates/torii/grpc/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ impl DojoWorld {
async fn events_all(&self, limit: u32, offset: u32) -> Result<Vec<proto::types::Event>, Error> {
let query = r#"
SELECT keys, data, transaction_hash
FROM events
FROM events
ORDER BY id DESC
LIMIT ? OFFSET ?
"#
Expand Down Expand Up @@ -262,12 +262,9 @@ impl DojoWorld {
SELECT count(*)
FROM {table}
JOIN {model_relation_table} ON {table}.id = {model_relation_table}.entity_id
WHERE {model_relation_table}.model_id = '{}' and {table}.keys LIKE ?
WHERE {model_relation_table}.model_id = '{:#x}' and {table}.keys LIKE ?
"#,
format!(
"{:#x}",
get_selector_from_name(&keys_clause.model).map_err(ParseError::NonAsciiName)?
),
get_selector_from_name(&keys_clause.model).map_err(ParseError::NonAsciiName)?
);

// total count of rows that matches keys_pattern without limit and offset
Expand All @@ -281,13 +278,10 @@ impl DojoWorld {
JOIN {model_relation_table} ON {table}.id = {model_relation_table}.entity_id
WHERE {table}.keys LIKE ?
GROUP BY {table}.id
HAVING INSTR(model_ids, '{}') > 0
HAVING INSTR(model_ids, '{:#x}') > 0
LIMIT 1
"#,
format!(
"{:#x}",
get_selector_from_name(&keys_clause.model).map_err(ParseError::NonAsciiName)?
),
get_selector_from_name(&keys_clause.model).map_err(ParseError::NonAsciiName)?
);
let (models_str,): (String,) =
sqlx::query_as(&models_query).bind(&keys_pattern).fetch_one(&self.pool).await?;
Expand Down Expand Up @@ -394,13 +388,10 @@ impl DojoWorld {
FROM {table}
JOIN {model_relation_table} ON {table}.id = {model_relation_table}.entity_id
GROUP BY {table}.id
HAVING INSTR(model_ids, '{}') > 0
HAVING INSTR(model_ids, '{:#x}') > 0
LIMIT 1
"#,
format!(
"{:#x}",
get_selector_from_name(&member_clause.model).map_err(ParseError::NonAsciiName)?
),
get_selector_from_name(&member_clause.model).map_err(ParseError::NonAsciiName)?
);
let (models_str,): (String,) = sqlx::query_as(&models_query).fetch_one(&self.pool).await?;

Expand Down
2 changes: 1 addition & 1 deletion crates/torii/grpc/src/server/tests/entities_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ async fn test_entities_queries() {

assert_eq!(entities.len(), 1);

let entity: Entity = entities.get(0).unwrap().clone().try_into().unwrap();
let entity: Entity = entities.first().unwrap().clone().try_into().unwrap();
assert_eq!(entity.models.first().unwrap().name, "Position");
assert_eq!(entity.models.get(1).unwrap().name, "Moves");
assert_eq!(entity.hashed_keys, poseidon_hash_many(&[account.address()]));
Expand Down
7 changes: 7 additions & 0 deletions scripts/clippy.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
#!/bin/bash

# Tells the shell to exit immediately if a command exits with a non-zero status
set -e
# Enables tracing of the commands as they are executed, showing the commands and their arguments
set -x
# Causes a pipeline to return a failure status if any command in the pipeline fails
set -o pipefail

run_clippy() {
cargo clippy --all-targets "$@" -- -D warnings -D future-incompatible -D nonstandard-style -D rust-2018-idioms -D unused
}
Expand Down

0 comments on commit 010444f

Please sign in to comment.