Repository: huff-language/huff-rs
Pull request #154 (2022-07-03 21:56:26)

fix: Bad Artifact Caching
## Overview

Artifact caching wasn't taking into account different constructor arguments, resulting in erroneous bytecode output.

✅ [Tested] Additionally fixes constructor arg non-interactive array inputs. ie `100, 200` would error as a constructor arg input since there were no surrounding brackets.



Patch:
diff --git a/huff_cli/src/huffc.rs b/huff_cli/src/huffc.rs
index f818ae9..541411c 100644
--- a/huff_cli/src/huffc.rs
+++ b/huff_cli/src/huffc.rs
@@ -98,6 +98,7 @@ fn main() {
         }
     };
 
+    let mut use_cache = true;
     if cli.interactive {
         // Don't accept configured inputs
         cli.inputs = None;
@@ -105,6 +106,8 @@ fn main() {
         // Have to first generate artifacts, prompt user for args,
         // and finally save artifacts with the new constructor args.
         cli.artifacts = false;
+        // Don't use cache if interactive since there's no way constructor arguments can match
+        use_cache = false;
     }
 
     let output = match (&cli.output, cli.artifacts) {
@@ -119,6 +122,7 @@ fn main() {
         construct_args: cli.inputs,
         optimize: cli.optimize,
         bytecode: cli.bytecode,
+        cached: use_cache,
     };
 
     // Create compiling spinner
@@ -130,6 +134,7 @@ fn main() {
     }
 
     let compile_res = compiler.execute();
+
     // Stop spinner animation if it exists
     if let Some(mut sp) = sp {
         sp.stop();
diff --git a/huff_core/README.md b/huff_core/README.md
index 2883ab2..880cbb3 100644
--- a/huff_core/README.md
+++ b/huff_core/README.md
@@ -21,7 +21,7 @@ use huff_utils::artifact::Artifact;
 use std::sync::Arc;
 
 // Instantiate the Compiler Instance
-let mut compiler = Compiler::new(Arc::new(vec!["../huff-examples/erc20/contracts/ERC20.huff".to_string()]), None, None, false);
+let mut compiler = Compiler::new(Arc::new(vec!["../huff-examples/erc20/contracts/ERC20.huff".to_string()]), None, None, false, false);
 
 // Execute the compiler
 let res: Result<Vec<Arc<Artifact>>, Arc<CompilerError<'_>>> = compiler.execute();
diff --git a/huff_core/src/cache.rs b/huff_core/src/cache.rs
index 29da390..3a5fef0 100644
--- a/huff_core/src/cache.rs
+++ b/huff_core/src/cache.rs
@@ -7,10 +7,12 @@ use walkdir::WalkDir;
 pub fn get_cached_artifacts(
     files: &[Arc<FileSource>],
     out: &OutputLocation,
+    constructor_args: String,
 ) -> Option<Vec<Arc<Artifact>>> {
     // Check if the file artifacts are already generated the the default "./artifacts/" directory or
     // the specified output dir
-    let artifacts: Vec<(Arc<FileSource>, Artifact)> = resolve_existing_artifacts(files, out)?;
+    let artifacts: Vec<(Arc<FileSource>, Artifact)> =
+        resolve_existing_artifacts(files, out, constructor_args)?;
 
     // Return the artifacts if cached
     Some(artifacts.into_iter().map(|(_, artifact)| Arc::new(artifact)).collect())
@@ -20,6 +22,7 @@ pub fn get_cached_artifacts(
 pub fn resolve_existing_artifacts(
     files: &[Arc<FileSource>],
     output: &OutputLocation,
+    constructor_args: String,
 ) -> Option<Vec<(Arc<FileSource>, Artifact)>> {
     let mut artifacts: Vec<(Arc<FileSource>, Artifact)> = Vec::new();
 
@@ -27,9 +30,12 @@ pub fn resolve_existing_artifacts(
     let mut file_sources: std::collections::HashMap<String, Arc<FileSource>> =
         files.iter().map(|f| (f.path.clone().to_lowercase(), Arc::clone(f))).collect();
 
+    // If outputdir is not specified, use the default "./artifacts/" directory
+    let output_dir = (!output.0.is_empty()).then(|| &*output.0).unwrap_or("./artifacts");
+
     // For each file, check if the artifact file exists at the location
-    tracing::debug!(target: "core", "Traversing output directory {}", output.0);
-    for entry in WalkDir::new(&output.0)
+    tracing::debug!(target: "core", "Traversing output directory {}", output_dir);
+    for entry in WalkDir::new(&output_dir)
         .into_iter()
         .filter_map(Result::ok)
         .filter(|e| !e.file_type().is_dir())
@@ -40,7 +46,7 @@ pub fn resolve_existing_artifacts(
             .display()
             .to_string()
             .replace(".json", "")
-            .replace(&output.0, ".")
+            .replace(&output_dir, ".")
             .to_lowercase();
         let expected = file_sources.remove(&formatted_path);
 
@@ -50,6 +56,10 @@ pub fn resolve_existing_artifacts(
                 // If we expected compilation, the sources must match
                 match expected {
                     Some(expected_fs) => {
+                        if !artifact.bytecode.ends_with(&constructor_args) {
+                            tracing::warn!(target: "core", "Mismatched Constructor Args for Cached Artifact \"{}\"", artifact.file.path);
+                            return None
+                        }
                         if artifact.file.source != expected_fs.source {
                             tracing::warn!(target: "core", "Cache Resolution Failed: \"{}\" Artifact Outdated", artifact.file.path);
                             return None
diff --git a/huff_core/src/lib.rs b/huff_core/src/lib.rs
index 8b596e7..90a80f7 100644
--- a/huff_core/src/lib.rs
+++ b/huff_core/src/lib.rs
@@ -4,6 +4,7 @@
 #![forbid(unsafe_code)]
 #![forbid(where_clauses_object_safety)]
 
+use ethers_core::utils::hex;
 use huff_codegen::*;
 use huff_lexer::*;
 use huff_parser::*;
@@ -41,6 +42,7 @@ pub(crate) mod cache;
 ///     Arc::new(vec!["../huff-examples/erc20/contracts/ERC20.huff".to_string()]),
 ///     Some("./artifacts".to_string()),
 ///     None,
+///     false,
 ///     false
 /// );
 /// ```
@@ -56,6 +58,8 @@ pub struct Compiler {
     pub optimize: bool,
     /// Generate and log bytecode
     pub bytecode: bool,
+    /// Whether to check cached artifacts
+    pub cached: bool,
 }
 
 impl<'a> Compiler {
@@ -65,11 +69,12 @@ impl<'a> Compiler {
         output: Option<String>,
         construct_args: Option<Vec<String>>,
         verbose: bool,
+        cached: bool,
     ) -> Self {
         if cfg!(feature = "verbose") || verbose {
             Compiler::init_tracing_subscriber(Some(vec![tracing::Level::INFO.into()]));
         }
-        Self { sources, output, construct_args, optimize: false, bytecode: false }
+        Self { sources, output, construct_args, optimize: false, bytecode: false, cached }
     }
 
     /// Tracing
@@ -134,9 +139,16 @@ impl<'a> Compiler {
 
         let mut artifacts: Vec<Arc<Artifact>> = vec![];
 
+        // Get our constructor arguments as a hex encoded string to compare to the cache
+        let inputs = self.get_constructor_args();
+        let encoded_inputs = Codegen::encode_constructor_args(inputs);
+        let encoded: Vec<Vec<u8>> =
+            encoded_inputs.iter().map(|tok| ethers_core::abi::encode(&[tok.clone()])).collect();
+        let constructor_args = encoded.iter().map(|tok| hex::encode(tok.as_slice())).collect();
+
         // Get Cached or Generate Artifacts
         tracing::debug!(target: "core", "Output directory: {}", output.0);
-        match cache::get_cached_artifacts(&files, &output) {
+        match cache::get_cached_artifacts(&files, &output, constructor_args) {
             Some(arts) => artifacts = arts,
             None => {
                 // Parallel Dependency Resolution
diff --git a/huff_utils/src/types.rs b/huff_utils/src/types.rs
index 9d4b7dd..a5aa1d6 100644
--- a/huff_utils/src/types.rs
+++ b/huff_utils/src/types.rs
@@ -151,6 +151,15 @@ impl TryFrom<String> for EToken {
         }
         if input.chars().all(|x| x.is_alphanumeric()) {
             Ok(EToken(Token::String(input)))
+        } else if input.contains(',') {
+            // Try to unwrap something like "100,0x123,20" without brackets
+            let e_tokens: Result<Vec<EToken>, _> = input
+                .split(',')
+                .map(|x| x.replace(' ', "").replace('"', "").replace('\'', ""))
+                .map(EToken::try_from)
+                .collect();
+            let tokens: Vec<Token> = e_tokens?.into_iter().map(|x| x.0).collect();
+            Ok(EToken(Token::Array(tokens)))
         } else {
             Err(format!("Invalid input: {}", input))
         }


Test patch:
diff --git a/huff_codegen/tests/constructor_args.rs b/huff_codegen/tests/constructor_args.rs
index edbfcef..065c175 100644
--- a/huff_codegen/tests/constructor_args.rs
+++ b/huff_codegen/tests/constructor_args.rs
@@ -86,3 +86,54 @@ fn encode_array_constructor_args() {
     assert_eq!(results[4], expected_array);
     assert_eq!(results[5], expected_array);
 }
+
+#[test]
+fn encode_missing_brackets_array_constructor_args() {
+    let expected_address: [u8; 20] = [
+        100, 109, 184, 255, 194, 30, 125, 220, 43, 99, 39, 68, 141, 217, 250, 86, 13, 244, 16, 135,
+    ];
+    let _expected_bytes32: Vec<u8> =
+        str_to_vec("87674fa174add091f082eab424cc60625118fa4c553592a4e54a76fb9e8512f6").unwrap();
+    // Bogus constructors args
+    let args: Vec<String> = vec![
+        "  100,  200,  300   ",
+        " 0x646dB8ffC21e7ddc2B6327448dd9Fa560Df41087,    0x646dB8ffC21e7ddc2B6327448dd9Fa560Df41087",
+        "true,  false,   false",
+        "Hello, World, Yes",
+        "   \"Hello\", \"World\", \"Yes\"",
+        "'Hello',  'World', 'Yes'   ",
+    ]
+    .iter()
+    .map(|s| s.to_string())
+    .collect();
+
+    let results = Codegen::encode_constructor_args(args);
+
+    assert_eq!(
+        results[0],
+        Token::Array(vec![
+            Token::Uint(U256::from_dec_str("100").unwrap()),
+            Token::Uint(U256::from_dec_str("200").unwrap()),
+            Token::Uint(U256::from_dec_str("300").unwrap()),
+        ])
+    );
+    assert_eq!(
+        results[1],
+        Token::Array(vec![
+            Token::Address(H160::from(expected_address)),
+            Token::Address(H160::from(expected_address)),
+        ])
+    );
+    assert_eq!(
+        results[2],
+        Token::Array(vec![Token::Bool(true), Token::Bool(false), Token::Bool(false),])
+    );
+    let expected_array = Token::Array(vec![
+        Token::String("Hello".to_string()),
+        Token::String("World".to_string()),
+        Token::String("Yes".to_string()),
+    ]);
+    assert_eq!(results[3], expected_array);
+    assert_eq!(results[4], expected_array);
+    assert_eq!(results[5], expected_array);
+}
diff --git a/huff_core/tests/file_resolution.rs b/huff_core/tests/file_resolution.rs
index a04d882..507ed36 100644
--- a/huff_core/tests/file_resolution.rs
+++ b/huff_core/tests/file_resolution.rs
@@ -5,7 +5,7 @@ use huff_utils::prelude::{CompilerError, OutputLocation, UnpackError};
 
 #[test]
 fn test_get_outputs_no_output() {
-    let compiler: Compiler = Compiler::new(Arc::new(vec![]), None, None, false);
+    let compiler: Compiler = Compiler::new(Arc::new(vec![]), None, None, false, false);
     let ol: OutputLocation = compiler.get_outputs();
     assert_eq!(ol, OutputLocation::default());
 }
@@ -13,7 +13,7 @@ fn test_get_outputs_no_output() {
 #[test]
 fn test_get_outputs_with_output() {
     let compiler: Compiler =
-        Compiler::new(Arc::new(vec![]), Some("./test_out/".to_string()), None, false);
+        Compiler::new(Arc::new(vec![]), Some("./test_out/".to_string()), None, false, false);
     let ol: OutputLocation = compiler.get_outputs();
     assert_eq!(ol, OutputLocation("./test_out/".to_string()));
 }
@@ -21,7 +21,7 @@ fn test_get_outputs_with_output() {
 #[test]
 fn test_transform_paths() {
     let _compiler: Compiler =
-        Compiler::new(Arc::new(vec![]), Some("./test_out/".to_string()), None, false);
+        Compiler::new(Arc::new(vec![]), Some("./test_out/".to_string()), None, false, false);
     let path_bufs: Result<Vec<PathBuf>, CompilerError<'_>> = Compiler::transform_paths(&vec![
         "../huff-examples/erc20/contracts/ERC20.huff".to_string(),
         "../huff-examples/erc20/contracts/utils/".to_string(),
@@ -55,7 +55,7 @@ fn test_transform_paths() {
 #[test]
 fn test_transform_paths_non_huff() {
     let _compiler: Compiler =
-        Compiler::new(Arc::new(vec![]), Some("./test_out/".to_string()), None, false);
+        Compiler::new(Arc::new(vec![]), Some("./test_out/".to_string()), None, false, false);
     let path_bufs: Result<Vec<PathBuf>, CompilerError<'_>> =
         Compiler::transform_paths(&vec!["./ERC20.txt".to_string()]);
     assert!(path_bufs.is_err());
@@ -72,7 +72,7 @@ fn test_transform_paths_non_huff() {
 #[test]
 fn test_transform_paths_no_dir() {
     let _compiler: Compiler =
-        Compiler::new(Arc::new(vec![]), Some("./test_out/".to_string()), None, false);
+        Compiler::new(Arc::new(vec![]), Some("./test_out/".to_string()), None, false, false);
     let path_bufs: Result<Vec<PathBuf>, CompilerError<'_>> =
         Compiler::transform_paths(&vec!["./examples/random_dir/".to_string()]);
     assert!(path_bufs.is_err());
diff --git a/huff_core/tests/gen_artifact.rs b/huff_core/tests/gen_artifact.rs
index 13aef34..a1d4e82 100644
--- a/huff_core/tests/gen_artifact.rs
+++ b/huff_core/tests/gen_artifact.rs
@@ -31,7 +31,7 @@ fn test_missing_constructor() {
     };
 
     // Instantiate a new compiler
-    let compiler = Compiler::new(Arc::new(vec![]), None, None, false);
+    let compiler = Compiler::new(Arc::new(vec![]), None, None, false, false);
 
     // Generate the compile artifact
     let arc_source = Arc::new(full_source);
@@ -76,7 +76,7 @@ fn test_missing_constructor_with_inputs() {
     };
 
     // Instantiate a new compiler
-    let compiler = Compiler::new(Arc::new(vec![]), None, Some(vec!["0".to_string()]), false);
+    let compiler = Compiler::new(Arc::new(vec![]), None, Some(vec!["0".to_string()]), false, false);
 
     // Generate the compile artifact
     let arc_source = Arc::new(full_source);