Extract build from declare (#1565)

<!-- Reference any GitHub issues resolved by this PR -->

-> * Extract build from declare
-- * Add support for --package flag
(#1569)
-- * Add workspace tests
(#1572)

todo:
-- * cleanup scarb_utils, use things from scarb_api/scarb_ui

## Introduced changes

Closes #1474

<!-- A brief description of the changes -->

- Extracts build logic from declare to avoid multiple artifact builds

## Checklist

<!-- Make sure all of these are complete -->

- [X] Linked relevant issue
- [X] Updated relevant documentation
- [X] Added relevant tests
- [X] Performed self-review of the code
- [X] Added changes to `CHANGELOG.md`

Author: THenry14

crates/sncast/src/helpers/constants.rs

@@ -27,3 +27,5 @@ pub const DEFAULT_ACCOUNTS_FILE: &str = "~/.starknet_accounts/starknet_open_zepp
 
 pub const KEYSTORE_PASSWORD_ENV_VAR: &str = "KEYSTORE_PASSWORD";
 pub const CREATE_KEYSTORE_PASSWORD_ENV_VAR: &str = "CREATE_KEYSTORE_PASSWORD";
+
+pub const SCRIPT_LIB_ARTIFACT_NAME: &str = "__sncast_script_lib";

crates/sncast/src/helpers/scarb_utils.rs

@@ -1,10 +1,11 @@
 use super::constants::{WAIT_RETRY_INTERVAL, WAIT_TIMEOUT};
 use anyhow::{anyhow, bail, Context, Result};
 use camino::{Utf8Path, Utf8PathBuf};
-use scarb_api::ScarbCommand;
+use scarb_api::{get_contracts_map, ScarbCommand, StarknetContractArtifacts};
 use scarb_metadata;
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
+use std::collections::HashMap;
 use std::default::Default;
 use std::env;
 use std::fs::canonicalize;
@@ -276,6 +277,28 @@ pub fn get_package_tool_sncast(metadata: &scarb_metadata::Metadata) -> Result<&V
     Ok(tool_sncast)
 }
 
+pub struct BuildConfig {
+    pub scarb_toml_path: Utf8PathBuf,
+    pub json: bool,
+}
+
+pub fn build(config: &BuildConfig) -> Result<HashMap<String, StarknetContractArtifacts>> {
+    let mut cmd = ScarbCommand::new_with_stdio();
+    cmd.arg("build").manifest_path(&config.scarb_toml_path);
+    if config.json {
+        cmd.json();
+    }
+    cmd.run()
+        .map_err(|e| anyhow!(format!("Failed to build using scarb; {e}")))?;
+
+    let metadata = get_scarb_metadata_command(&config.scarb_toml_path)?
+        .exec()
+        .expect("Failed to obtain metadata");
+    let package = get_package_metadata(&metadata, &config.scarb_toml_path)
+        .with_context(|| anyhow!("Failed to find package"))?;
+    get_contracts_map(&metadata, &package.id)
+}
+
 #[cfg(test)]
 mod tests {
     use crate::helpers::scarb_utils::get_scarb_metadata;

crates/sncast/src/main.rs

@@ -7,11 +7,13 @@ use crate::starknet_commands::{
 use anyhow::{Context, Result};
 use sncast::response::print::{print_command_result, OutputFormat};
 
-use crate::starknet_commands::declare::BuildConfig;
 use camino::Utf8PathBuf;
 use clap::{Parser, Subcommand};
 use sncast::helpers::constants::{DEFAULT_ACCOUNTS_FILE, DEFAULT_MULTICALL_CONTENTS};
-use sncast::helpers::scarb_utils::{parse_scarb_config, CastConfig};
+use sncast::helpers::scarb_utils::{
+    build, get_package_metadata, get_scarb_manifest, get_scarb_metadata_with_deps,
+    parse_scarb_config, BuildConfig, CastConfig,
+};
 use sncast::{
     chain_id_to_network_name, get_account, get_block_id, get_chain_id, get_nonce, get_provider,
     NumbersFormat, WaitForTx,
@@ -122,9 +124,22 @@ fn main() -> Result<()> {
     let runtime = Runtime::new().expect("Failed to instantiate Runtime");
 
     if let Commands::Script(script) = cli.command {
+        let manifest_path = match cli.path_to_scarb_toml {
+            Some(path) => path,
+            None => get_scarb_manifest().context("Failed to obtain manifest path from Scarb")?,
+        };
+        let metadata = get_scarb_metadata_with_deps(&manifest_path)?;
+        let package_metadata = get_package_metadata(&metadata, &manifest_path)?;
+        let mut artifacts = build(&BuildConfig {
+            scarb_toml_path: manifest_path.clone(),
+            json: cli.json,
+        })
+        .expect("Failed to build script");
         let mut result = starknet_commands::script::run(
             &script.script_module_name,
-            &cli.path_to_scarb_toml,
+            &metadata,
+            package_metadata,
+            &mut artifacts,
             &provider,
             runtime,
             &config,
@@ -156,10 +171,6 @@ async fn run_async_command(
         timeout: config.wait_timeout,
         retry_interval: config.wait_retry_interval,
     };
-    let build_config = BuildConfig {
-        scarb_toml_path: cli.path_to_scarb_toml.clone(),
-        json: cli.json,
-    };
     match cli.command {
         Commands::Declare(declare) => {
             let account = get_account(
@@ -169,12 +180,22 @@ async fn run_async_command(
                 config.keystore,
             )
             .await?;
+            let manifest_path = match cli.path_to_scarb_toml.clone() {
+                Some(path) => path,
+                None => {
+                    get_scarb_manifest().context("Failed to obtain manifest path from Scarb")?
+                }
+            };
+            let artifacts = build(&BuildConfig {
+                scarb_toml_path: manifest_path,
+                json: cli.json,
+            })?;
             let mut result = starknet_commands::declare::declare(
                 &declare.contract,
                 declare.max_fee,
                 &account,
                 declare.nonce,
-                build_config,
+                &artifacts,
                 wait_config,
             )
             .await;

crates/sncast/src/starknet_commands/declare.rs

@@ -1,10 +1,6 @@
 use anyhow::{anyhow, Context, Result};
-use camino::Utf8PathBuf;
 use clap::Args;
-use scarb_api::{get_contracts_map, ScarbCommand};
-
-use sncast::helpers::scarb_utils::get_package_metadata;
-use sncast::helpers::scarb_utils::get_scarb_manifest;
+use scarb_api::StarknetContractArtifacts;
 use sncast::response::structs::DeclareResponse;
 use sncast::response::structs::Hex;
 use sncast::{apply_optional, handle_rpc_error, handle_wait_for_tx, WaitForTx};
@@ -18,6 +14,7 @@ use starknet::{
     providers::jsonrpc::{HttpTransport, JsonRpcClient},
     signers::LocalWallet,
 };
+use std::collections::HashMap;
 use std::sync::Arc;
 
 #[derive(Args)]
@@ -36,44 +33,17 @@ pub struct Declare {
     pub nonce: Option<FieldElement>,
 }
 
-pub struct BuildConfig {
-    pub scarb_toml_path: Option<Utf8PathBuf>,
-    pub json: bool,
-}
-
 #[allow(clippy::too_many_lines)]
 pub async fn declare(
     contract_name: &str,
     max_fee: Option<FieldElement>,
     account: &SingleOwnerAccount<&JsonRpcClient<HttpTransport>, LocalWallet>,
     nonce: Option<FieldElement>,
-    build_config: BuildConfig,
+    artifacts: &HashMap<String, StarknetContractArtifacts>,
     wait_config: WaitForTx,
 ) -> Result<DeclareResponse> {
     let contract_name: String = contract_name.to_string();
-    let manifest_path = match build_config.scarb_toml_path.clone() {
-        Some(path) => path,
-        None => get_scarb_manifest().context("Failed to obtain manifest path from Scarb")?,
-    };
-
-    let mut cmd = ScarbCommand::new_with_stdio();
-    cmd.arg("build").manifest_path(&manifest_path);
-    if build_config.json {
-        cmd.json();
-    }
-    cmd.run().context("Failed to build contracts with Scarb")?;
-
-    let metadata = scarb_metadata::MetadataCommand::new()
-        .manifest_path(&manifest_path)
-        .inherit_stderr()
-        .exec()
-        .context("Failed to get scarb metadata")?;
-
-    let package = get_package_metadata(&metadata, &manifest_path)
-        .with_context(|| anyhow!("Failed to find package for contract = {contract_name}"))?;
-    let contracts = get_contracts_map(&metadata, &package.id)?;
-
-    let contract_artifacts = contracts
+    let contract_artifacts = artifacts
         .get(&contract_name)
         .ok_or(anyhow!("Failed to find artifacts in starknet_artifacts.json file. Please ensure you have enabled sierra and casm code generation in Scarb.toml"))?;
 

crates/sncast/src/starknet_commands/script.rs

@@ -1,10 +1,9 @@
 use std::collections::HashMap;
 use std::fs;
 
-use crate::starknet_commands::declare::BuildConfig;
 use crate::starknet_commands::{call, declare, deploy, invoke};
 use crate::{get_account, get_nonce, WaitForTx};
-use anyhow::{anyhow, ensure, Context, Result};
+use anyhow::{anyhow, Context, Result};
 use blockifier::execution::common_hints::ExecutionMode;
 use blockifier::execution::deprecated_syscalls::DeprecatedSyscallSelector;
 use blockifier::execution::entry_point::{
@@ -23,7 +22,6 @@ use cairo_lang_utils::ordered_hash_map::OrderedHashMap;
 use cairo_vm::types::relocatable::Relocatable;
 use cairo_vm::vm::errors::hint_errors::HintError;
 use cairo_vm::vm::vm_core::VirtualMachine;
-use camino::Utf8PathBuf;
 use clap::command;
 use clap::Args;
 use conversions::{FromConv, IntoConv};
@@ -35,12 +33,11 @@ use runtime::{
     CheatcodeHandlingResult, EnhancedHintError, ExtendedRuntime, ExtensionLogic, StarknetRuntime,
     SyscallHandlingResult,
 };
-use scarb_api::{package_matches_version_requirement, ScarbCommand};
-use scarb_metadata::Metadata;
+use scarb_api::{package_matches_version_requirement, StarknetContractArtifacts};
+use scarb_metadata::{Metadata, PackageMetadata};
 use semver::{Comparator, Op, Version, VersionReq};
-use sncast::helpers::scarb_utils::{
-    get_package_metadata, get_scarb_manifest, get_scarb_metadata_with_deps, CastConfig,
-};
+use sncast::helpers::constants::SCRIPT_LIB_ARTIFACT_NAME;
+use sncast::helpers::scarb_utils::CastConfig;
 use sncast::response::print::print_as_warning;
 use sncast::response::structs::ScriptResponse;
 use starknet::accounts::Account;
@@ -49,6 +46,8 @@ use starknet::providers::jsonrpc::HttpTransport;
 use starknet::providers::JsonRpcClient;
 use tokio::runtime::Runtime;
 
+type ScriptStarknetContractArtifacts = StarknetContractArtifacts;
+
 #[derive(Args)]
 #[command(about = "Execute a deployment script")]
 pub struct Script {
@@ -61,6 +60,7 @@ pub struct CastScriptExtension<'a> {
     pub provider: &'a JsonRpcClient<HttpTransport>,
     pub tokio_runtime: Runtime,
     pub config: &'a CastConfig,
+    pub artifacts: &'a HashMap<String, StarknetContractArtifacts>,
 }
 
 impl<'a> ExtensionLogic for CastScriptExtension<'a> {
@@ -118,10 +118,7 @@ impl<'a> ExtensionLogic for CastScriptExtension<'a> {
                     max_fee,
                     &account,
                     nonce,
-                    BuildConfig {
-                        scarb_toml_path: None,
-                        json: false,
-                    },
+                    self.artifacts,
                     WaitForTx {
                         wait: true,
                         timeout: self.config.wait_timeout,
@@ -253,21 +250,25 @@ impl<'a> ExtensionLogic for CastScriptExtension<'a> {
 
 pub fn run(
     module_name: &str,
-    path_to_scarb_toml: &Option<Utf8PathBuf>,
+    metadata: &Metadata,
+    package_metadata: &PackageMetadata,
+    artifacts: &mut HashMap<String, StarknetContractArtifacts>,
     provider: &JsonRpcClient<HttpTransport>,
     tokio_runtime: Runtime,
     config: &CastConfig,
 ) -> Result<ScriptResponse> {
-    let path = compile_script(path_to_scarb_toml.clone())?;
+    warn_if_sncast_std_not_compatible(metadata)?;
+    let artifacts = inject_lib_artifact(metadata, package_metadata, artifacts)?;
 
-    let sierra_program = serde_json::from_str::<VersionedProgram>(
-        &fs::read_to_string(path.clone())
-            .with_context(|| format!("Failed to read Sierra file at path = {path}"))?,
-    )
-    .with_context(|| format!("Failed to deserialize Sierra program at path = {path}"))?
-    .into_v1()
-    .with_context(|| format!("Failed to load Sierra program at path = {path}"))?
-    .program;
+    let artifact = artifacts
+        .get(SCRIPT_LIB_ARTIFACT_NAME)
+        .ok_or(anyhow!("Failed to find script artifact"))?;
+
+    let sierra_program = serde_json::from_str::<VersionedProgram>(&artifact.sierra)
+        .with_context(|| "Failed to deserialize Sierra program")?
+        .into_v1()
+        .with_context(|| "Failed to load Sierra program")?
+        .program;
 
     let runner = SierraCasmRunner::new(
         sierra_program,
@@ -327,6 +328,7 @@ pub fn run(
         provider,
         tokio_runtime,
         config,
+        artifacts: &artifacts,
     };
 
     let mut cast_runtime = ExtendedRuntime {
@@ -385,42 +387,28 @@ fn warn_if_sncast_std_not_compatible(scarb_metadata: &Metadata) -> Result<()> {
     Ok(())
 }
 
-fn compile_script(path_to_scarb_toml: Option<Utf8PathBuf>) -> Result<Utf8PathBuf> {
-    let scripts_manifest_path = path_to_scarb_toml.unwrap_or_else(|| {
-        get_scarb_manifest()
-            .context("Failed to retrieve manifest path from scarb")
-            .unwrap()
-    });
-    ensure!(
-        scripts_manifest_path.exists(),
-        "The path = {scripts_manifest_path} does not exist"
-    );
-
-    ScarbCommand::new_with_stdio()
-        .arg("build")
-        .manifest_path(&scripts_manifest_path)
-        .run()
-        .context("failed to compile script with scarb")?;
+fn inject_lib_artifact(
+    metadata: &Metadata,
+    package_metadata: &PackageMetadata,
+    artifacts: &mut HashMap<String, StarknetContractArtifacts>,
+) -> Result<HashMap<String, StarknetContractArtifacts>> {
+    let sierra_filename = format!("{}.sierra.json", package_metadata.name);
 
-    let metadata = get_scarb_metadata_with_deps(&scripts_manifest_path)?;
-
-    warn_if_sncast_std_not_compatible(&metadata)?;
-
-    let package_metadata = get_package_metadata(&metadata, &scripts_manifest_path)?;
-
-    let filename = format!("{}.sierra.json", package_metadata.name);
-    let path = metadata
+    let target_dir = &metadata
         .target_dir
-        .unwrap_or(metadata.workspace.root.join("target"))
-        .join(metadata.current_profile)
-        .join(filename.clone());
-
-    ensure!(
-        path.exists(),
-        "The package has not been compiled, the file at path = {path} does not exist"
-    );
+        .clone()
+        .unwrap_or_else(|| metadata.workspace.root.join("target"));
+    let sierra_path = &target_dir
+        .join(&metadata.current_profile)
+        .join(sierra_filename);
+
+    let lib_artifacts = ScriptStarknetContractArtifacts {
+        sierra: fs::read_to_string(sierra_path)?,
+        casm: String::new(),
+    };
 
-    Ok(path)
+    artifacts.insert(SCRIPT_LIB_ARTIFACT_NAME.to_string(), lib_artifacts);
+    Ok(artifacts.clone())
 }
 
 // taken from starknet-foundry/crates/forge/src/test_case_summary.rs

crates/sncast/tests/e2e/declare.rs

@@ -107,8 +107,8 @@ fn scarb_build_fails(contract_path: &str, accounts_file_path: &str) {
         .args(args);
 
     snapbox.assert().stderr_matches(indoc! {r"
-        command: declare
-        error: Failed to build contracts with Scarb: `scarb` exited with error
+        ...
+        Error: Failed to build using scarb; `scarb` exited with error
     "});
 }
 
@@ -161,9 +161,9 @@ fn scarb_no_sierra_artifact() {
         .current_dir(CONTRACTS_DIR.to_string() + "/no_sierra")
         .args(args);
 
-    snapbox.assert().success().stderr_matches(indoc! {r"
-        command: declare
-        [..]Make sure you have enabled sierra code generation in Scarb.toml[..]
+    snapbox.assert().failure().stderr_matches(indoc! {r"
+        [..]Make sure you have enabled sierra code generation in Scarb.toml
+        ...
     "});
 }
 

crates/sncast/tests/e2e/script/declare.rs

@@ -19,7 +19,7 @@ async fn test_missing_field() {
     let snapbox = Command::new(cargo_bin!("sncast"))
         .current_dir(SCRIPTS_DIR.to_owned() + "/declare/missing_field")
         .args(args);
-    snapbox.assert().success().stdout_matches(indoc! {r"
+    snapbox.assert().failure().stdout_matches(indoc! {r"
         ...
         error: Wrong number of arguments. Expected 3, found: 2
         ...