Repository: gakonst/ethers-rs
Pull request #1273 (2022-05-16 22:11:09)

fix(solc): purge obsolete cached artifacts
<!--
Thank you for your Pull Request. Please provide a description above and review
the requirements below.

Bug fixes and new features should include tests.

Contributors guide: https://github.com/gakonst/ethers-rs/blob/master/CONTRIBUTING.md

The contributors guide includes instructions for running rustfmt and building the
documentation.
-->

## Motivation
Fixes a bug where changing the pinned solc between two runs resulted in the cached artifacts of the old version also being included in the output of the project compiler.

Ref https://github.com/foundry-rs/foundry/issues/1306

<!--
Explain the context and why you're making that change. What is the problem
you're trying to solve? In some cases there is not a problem and this can be
thought of as being the motivation for your change.
-->

## Solution
when postprocessing the cached artifacts add another check that the versioned artifact must also be included in the filtered set, which contains the versions that don't need to be compiled. This way we will always purge (Contract, 0.8.10) if we change the pinned version to `0.8.13` and recompile.
<!--
Summarize the solution and provide any necessary context needed to understand
the code change.
-->

## PR Checklist

- [x] Added Tests
- [x] Added Documentation
- [ ] Updated the changelog



Patch:
diff --git a/ethers-solc/src/cache.rs b/ethers-solc/src/cache.rs
index 59dcb858..f9af00e3 100644
--- a/ethers-solc/src/cache.rs
+++ b/ethers-solc/src/cache.rs
@@ -294,7 +294,7 @@ impl SolFilesCache {
         I: IntoIterator<Item = (&'a Path, V)>,
         V: IntoIterator<Item = &'a Version>,
     {
-        let mut files: HashMap<_, _> = files.into_iter().map(|(p, v)| (p, v)).collect();
+        let mut files: HashMap<_, _> = files.into_iter().collect();
 
         self.files.retain(|file, entry| {
             if entry.artifacts.is_empty() {
@@ -474,7 +474,9 @@ impl CacheEntry {
         }
     }
 
-    /// Retains only those artifacts that match the provided version.
+    /// Retains only those artifacts that match the provided versions.
+    ///
+    /// Removes an artifact entry if none of its versions is included in the `versions` set.
     pub fn retain_versions<'a, I>(&mut self, versions: I)
     where
         I: IntoIterator<Item = &'a Version>,
@@ -874,35 +876,54 @@ impl<'a, T: ArtifactOutput> ArtifactsCache<'a, T> {
                 // keep only those files that were previously filtered (not dirty, reused)
                 cache.retain(filtered.iter().map(|(p, (_, v))| (p.as_path(), v)));
 
-                // add the artifacts to the cache entries, this way we can keep a mapping from
-                // solidity file to its artifacts
+                // add the written artifacts to the cache entries, this way we can keep a mapping
+                // from solidity file to its artifacts
                 // this step is necessary because the concrete artifacts are only known after solc
                 // was invoked and received as output, before that we merely know the file and
                 // the versions, so we add the artifacts on a file by file basis
-                for (file, artifacts) in written_artifacts.as_ref() {
+                for (file, written_artifacts) in written_artifacts.as_ref() {
                     let file_path = Path::new(&file);
-                    if let Some((entry, versions)) = dirty_source_files.get_mut(file_path) {
-                        entry.insert_artifacts(artifacts.iter().map(|(name, artifacts)| {
-                            let artifacts = artifacts
-                                .iter()
-                                .filter(|artifact| versions.contains(&artifact.version))
-                                .collect::<Vec<_>>();
-                            (name, artifacts)
-                        }));
+                    if let Some((cache_entry, versions)) = dirty_source_files.get_mut(file_path) {
+                        cache_entry.insert_artifacts(written_artifacts.iter().map(
+                            |(name, artifacts)| {
+                                let artifacts = artifacts
+                                    .iter()
+                                    .filter(|artifact| versions.contains(&artifact.version))
+                                    .collect::<Vec<_>>();
+                                (name, artifacts)
+                            },
+                        ));
                     }
 
                     // cached artifacts that were overwritten also need to be removed from the
                     // `cached_artifacts` set
                     if let Some((f, mut cached)) = cached_artifacts.0.remove_entry(file) {
-                        cached.retain(|name, files| {
-                            if let Some(written_files) = artifacts.get(name) {
-                                files.retain(|f| {
-                                    written_files.iter().all(|other| other.version != f.version)
+                        tracing::trace!("checking {} for obsolete cached artifact entries", file);
+                        cached.retain(|name, cached_artifacts| {
+                            if let Some(written_files) = written_artifacts.get(name) {
+                                // written artifact clashes with a cached artifact, so we need to decide whether to keep or to remove the cached
+                                cached_artifacts.retain(|f| {
+                                    // we only keep those artifacts that don't conflict with written artifacts and which version was a compiler target
+                                    let retain = written_files
+                                        .iter()
+                                        .all(|other| other.version != f.version) && filtered.get(
+                                        &PathBuf::from(file)).map(|(_, versions)| {
+                                            versions.contains(&f.version)
+                                        }).unwrap_or_default();
+                                    if !retain {
+                                        tracing::trace!(
+                                            "purging obsolete cached artifact for contract {} and version {}",
+                                            name,
+                                            f.version
+                                        );
+                                    }
+                                    retain
                                 });
-                                return !files.is_empty()
+                                return !cached_artifacts.is_empty()
                             }
                             false
                         });
+
                         if !cached.is_empty() {
                             cached_artifacts.0.insert(f, cached);
                         }
diff --git a/ethers-solc/src/compile/mod.rs b/ethers-solc/src/compile/mod.rs
index 92e3b9a9..74a98915 100644
--- a/ethers-solc/src/compile/mod.rs
+++ b/ethers-solc/src/compile/mod.rs
@@ -269,6 +269,29 @@ impl Solc {
         Ok(Some(Solc::new(solc)))
     }
 
+    /// Returns the path for a [svm](https://github.com/roynalnaruto/svm-rs) installed version.
+    ///
+    /// If the version is not installed yet, it will install it.
+    ///
+    /// # Example
+    /// ```no_run
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> {
+    ///  use ethers_solc::Solc;
+    /// let solc = Solc::find_or_install_svm_version("0.8.9").unwrap();
+    /// assert_eq!(solc, Solc::new("~/.svm/0.8.9/solc-0.8.9"));
+    /// # Ok(())
+    /// # }
+    /// ```
+    #[cfg(all(not(target_arch = "wasm32"), all(feature = "svm-solc")))]
+    pub fn find_or_install_svm_version(version: impl AsRef<str>) -> Result<Self> {
+        let version = version.as_ref();
+        if let Some(solc) = Solc::find_svm_installed_version(version)? {
+            Ok(solc)
+        } else {
+            Ok(Solc::blocking_install(&version.parse::<Version>()?)?)
+        }
+    }
+
     /// Assuming the `versions` array is sorted, it returns the first element which satisfies
     /// the provided [`VersionReq`]
     pub fn find_matching_installation(
diff --git a/ethers-solc/src/project_util/mod.rs b/ethers-solc/src/project_util/mod.rs
index dcb5e76b..e1bf7956 100644
--- a/ethers-solc/src/project_util/mod.rs
+++ b/ethers-solc/src/project_util/mod.rs
@@ -9,7 +9,7 @@ use crate::{
     utils,
     utils::tempdir,
     Artifact, ArtifactOutput, Artifacts, ConfigurableArtifacts, ConfigurableContractArtifact,
-    FileFilter, PathStyle, Project, ProjectCompileOutput, ProjectPathsConfig, SolFilesCache,
+    FileFilter, PathStyle, Project, ProjectCompileOutput, ProjectPathsConfig, SolFilesCache, Solc,
     SolcIoError,
 };
 use fs_extra::{dir, file};
@@ -66,6 +66,13 @@ impl<T: ArtifactOutput> TempProject<T> {
         self
     }
 
+    /// Explicitly sets the solc version for the project
+    pub fn set_solc(&mut self, solc: impl AsRef<str>) -> &mut Self {
+        self.inner.solc = Solc::find_or_install_svm_version(solc).unwrap();
+        self.inner.auto_detect = false;
+        self
+    }
+
     pub fn project(&self) -> &Project<T> {
         &self.inner
     }


Test patch:
diff --git a/ethers-solc/tests/project.rs b/ethers-solc/tests/project.rs
index 21698fd7..f2cee711 100644
--- a/ethers-solc/tests/project.rs
+++ b/ethers-solc/tests/project.rs
@@ -1333,7 +1333,7 @@ fn can_compile_model_checker_sample() {
 
 fn remove_solc_if_exists(version: &Version) {
     match Solc::find_svm_installed_version(version.to_string()).unwrap() {
-        Some(_) => svm::remove_version(&version).expect("failed to remove version"),
+        Some(_) => svm::remove_version(version).expect("failed to remove version"),
         None => {}
     };
 }
@@ -1351,7 +1351,7 @@ async fn can_install_solc_and_compile_version() {
 pragma solidity {};
 contract Contract {{ }}
 "#,
-                version.to_string()
+                version
             ),
         )
         .unwrap();
@@ -1380,3 +1380,34 @@ async fn can_install_solc_and_compile_std_json_input_async() {
     assert!(!out.has_error());
     assert!(out.sources.contains_key("lib/ds-test/src/test.sol"));
 }
+
+#[test]
+fn can_purge_obsolete_artifacts() {
+    let mut project = TempProject::<ConfigurableArtifacts>::dapptools().unwrap();
+    project.set_solc("0.8.10");
+    project
+        .add_source(
+            "Contract",
+            r#"
+    pragma solidity >=0.8.10;
+
+   contract Contract {
+        function xyz() public {
+        }
+   }
+   "#,
+        )
+        .unwrap();
+
+    let compiled = project.compile().unwrap();
+    assert!(!compiled.has_compiler_errors());
+    assert!(!compiled.is_unchanged());
+    assert_eq!(compiled.into_artifacts().count(), 1);
+
+    project.set_solc("0.8.13");
+
+    let compiled = project.compile().unwrap();
+    assert!(!compiled.has_compiler_errors());
+    assert!(!compiled.is_unchanged());
+    assert_eq!(compiled.into_artifacts().count(), 1);
+}