Repository: gakonst/ethers-rs (rust)

Issue:
Use solc --base-path to project root by default
currently, source files in the compilerinput are canonic, this can cause some issues with metadata/hashes.

in #1307 we already save the artifacts with relative files, this should be obsolete if we set base-path == root and strip the root from all sources

Pull request:
feat(solc): use relative paths and --base-path option
<!--
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
Closes #1315

working with paths is hard.

Ideally, we want to work with absolute paths because we resolve them in the Graph anyway and `Project::root` is not guaranteed to be cwd.

however, absolute/relative paths result in different solc metadata, so when invoking solc, we want to be consistent, or rather OS and local disk independent.

with 0.6.9, solc supports a `--base-path` option so we use that instead and also set the cwd of the solc process to `Project::root`.

This also introduces the `CompilerInput::with_base_path` function that will strip `Project::root` from:
* source paths
* output selection
* remappings 

<!--
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

<!--
Summarize the solution and provide any necessary context needed to understand
the code change.
-->

## PR Checklist

- [x] Added Tests
- [x] Added Documentation
- [x] Updated the changelog


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 543fd391..8b517dd0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -98,6 +98,8 @@
 
 ### Unreleased
 
+- Use relative source paths and `solc --base-path`
+  [#1317](https://github.com/gakonst/ethers-rs/pull/1317)
 - Save cache entry objects with relative paths
   [#1307](https://github.com/gakonst/ethers-rs/pull/1307)
 - Bundle svm, svm-builds and sha2 dependencies in new `svm-solc` feature
diff --git a/ethers-solc/src/artifacts/mod.rs b/ethers-solc/src/artifacts/mod.rs
index 3a8c6ffc..732f5956 100644
--- a/ethers-solc/src/artifacts/mod.rs
+++ b/ethers-solc/src/artifacts/mod.rs
@@ -1,21 +1,18 @@
 //! Solc artifact types
-use ethers_core::abi::Abi;
-
+use crate::{
+    compile::*, error::SolcIoError, remappings::Remapping, utils, ProjectPathsConfig, SolcError,
+};
 use colored::Colorize;
+use ethers_core::abi::Abi;
 use md5::Digest;
 use semver::{Version, VersionReq};
+use serde::{de::Visitor, Deserialize, Deserializer, Serialize, Serializer};
 use std::{
     collections::{BTreeMap, HashSet},
     fmt, fs,
     path::{Path, PathBuf},
     str::FromStr,
 };
-
-use crate::{
-    compile::*, error::SolcIoError, remappings::Remapping, utils, ProjectPathsConfig, SolcError,
-};
-
-use serde::{de::Visitor, Deserialize, Deserializer, Serialize, Serializer};
 use tracing::warn;
 
 pub mod ast;
@@ -189,10 +186,20 @@ impl CompilerInput {
         self.sources = self
             .sources
             .into_iter()
-            .map(|(path, s)| (path.strip_prefix(base).map(|p| p.to_path_buf()).unwrap_or(path), s))
+            .map(|(path, s)| (path.strip_prefix(base).map(Into::into).unwrap_or(path), s))
             .collect();
         self
     }
+
+    /// Similar to `Self::strip_prefix()`. Remove a base path from all
+    /// sources _and_ all paths in solc settings such as remappings
+    ///
+    /// See also `solc --base-path`
+    pub fn with_base_path(mut self, base: impl AsRef<Path>) -> Self {
+        let base = base.as_ref();
+        self.settings = self.settings.with_base_path(base);
+        self.strip_prefix(base)
+    }
 }
 
 /// A `CompilerInput` representation used for verify
@@ -385,6 +392,38 @@ impl Settings {
         output.insert("".to_string(), vec!["ast".to_string()]);
         self
     }
+
+    /// Strips `base` from all paths
+    pub fn with_base_path(mut self, base: impl AsRef<Path>) -> Self {
+        let base = base.as_ref();
+        self.remappings.iter_mut().for_each(|r| {
+            r.strip_prefix(base);
+        });
+
+        self.libraries.libs = self
+            .libraries
+            .libs
+            .into_iter()
+            .map(|(file, libs)| (file.strip_prefix(base).map(Into::into).unwrap_or(file), libs))
+            .collect();
+
+        self.output_selection = OutputSelection(
+            self.output_selection
+                .0
+                .into_iter()
+                .map(|(file, selection)| {
+                    (
+                        Path::new(&file)
+                            .strip_prefix(base)
+                            .map(|p| format!("{}", p.display()))
+                            .unwrap_or(file),
+                        selection,
+                    )
+                })
+                .collect(),
+        );
+        self
+    }
 }
 
 impl Default for Settings {
diff --git a/ethers-solc/src/compile/mod.rs b/ethers-solc/src/compile/mod.rs
index 7c3a77fd..1476ecd0 100644
--- a/ethers-solc/src/compile/mod.rs
+++ b/ethers-solc/src/compile/mod.rs
@@ -127,6 +127,8 @@ impl fmt::Display for SolcVersion {
 pub struct Solc {
     /// Path to the `solc` executable
     pub solc: PathBuf,
+    /// The base path to set when invoking solc, see also <https://docs.soliditylang.org/en/v0.8.11/path-resolution.html#base-path-and-include-paths>
+    pub base_path: Option<PathBuf>,
     /// Additional arguments passed to the `solc` exectuable
     pub args: Vec<String>,
 }
@@ -163,7 +165,15 @@ impl fmt::Display for Solc {
 impl Solc {
     /// A new instance which points to `solc`
     pub fn new(path: impl Into<PathBuf>) -> Self {
-        Solc { solc: path.into(), args: Vec::new() }
+        Solc { solc: path.into(), base_path: None, args: Vec::new() }
+    }
+
+    /// Sets solc's base path
+    ///
+    /// Ref: <https://docs.soliditylang.org/en/v0.8.11/path-resolution.html#base-path-and-include-paths>
+    pub fn with_base_path(mut self, base_path: impl Into<PathBuf>) -> Self {
+        self.base_path = Some(base_path.into());
+        self
     }
 
     /// Adds an argument to pass to the `solc` command.
@@ -513,6 +523,9 @@ impl Solc {
 
     pub fn compile_output<T: Serialize>(&self, input: &T) -> Result<Vec<u8>> {
         let mut cmd = Command::new(&self.solc);
+        if let Some(ref base_path) = self.base_path {
+            cmd.current_dir(base_path);
+        }
         let mut child = cmd
             .args(&self.args)
             .arg("--standard-json")
@@ -575,7 +588,11 @@ impl Solc {
     pub async fn async_compile_output<T: Serialize>(&self, input: &T) -> Result<Vec<u8>> {
         use tokio::io::AsyncWriteExt;
         let content = serde_json::to_vec(input)?;
-        let mut child = tokio::process::Command::new(&self.solc)
+        let mut cmd = tokio::process::Command::new(&self.solc);
+        if let Some(ref base_path) = self.base_path {
+            cmd.current_dir(base_path);
+        }
+        let mut child = cmd
             .args(&self.args)
             .arg("--standard-json")
             .stdin(Stdio::piped())
diff --git a/ethers-solc/src/compile/output/contracts.rs b/ethers-solc/src/compile/output/contracts.rs
index 2945cd19..ed1f0069 100644
--- a/ethers-solc/src/compile/output/contracts.rs
+++ b/ethers-solc/src/compile/output/contracts.rs
@@ -136,7 +136,7 @@ impl VersionedContracts {
         self.0 = std::mem::take(&mut self.0)
             .into_iter()
             .map(|(contract_path, contracts)| {
-                (root.join(contract_path).to_string_lossy().to_string(), contracts)
+                (format!("{}", root.join(contract_path).display()), contracts)
             })
             .collect();
         self
diff --git a/ethers-solc/src/compile/output/mod.rs b/ethers-solc/src/compile/output/mod.rs
index e6e0a61c..508ffb0e 100644
--- a/ethers-solc/src/compile/output/mod.rs
+++ b/ethers-solc/src/compile/output/mod.rs
@@ -366,6 +366,14 @@ impl AggregatedCompilerOutput {
         (self.sources, self.contracts)
     }
 
+    /// Joins all file path with `root`
+    pub fn join_all(&mut self, root: impl AsRef<Path>) -> &mut Self {
+        let root = root.as_ref();
+        self.contracts.join_all(root);
+        self.sources.join_all(root);
+        self
+    }
+
     /// Strips the given prefix from all file paths to make them relative to the given
     /// `base` argument.
     ///
diff --git a/ethers-solc/src/compile/project.rs b/ethers-solc/src/compile/project.rs
index 59b6774e..f25f7e7e 100644
--- a/ethers-solc/src/compile/project.rs
+++ b/ethers-solc/src/compile/project.rs
@@ -106,6 +106,7 @@ use crate::{
     artifacts::{Settings, VersionedFilteredSources, VersionedSources},
     cache::ArtifactsCache,
     error::Result,
+    filter::SparseOutputFilter,
     output::AggregatedCompilerOutput,
     report,
     resolver::GraphEdges,
@@ -113,8 +114,6 @@ use crate::{
     Sources,
 };
 use rayon::prelude::*;
-
-use crate::filter::SparseOutputFilter;
 use std::{collections::btree_map::BTreeMap, path::PathBuf, time::Instant};
 
 #[derive(Debug)]
@@ -155,7 +154,9 @@ impl<'a, T: ArtifactOutput> ProjectCompiler<'a, T> {
     pub fn with_sources(project: &'a Project<T>, sources: Sources) -> Result<Self> {
         let graph = Graph::resolve_sources(&project.paths, sources)?;
         let (versions, edges) = graph.into_sources_by_version(project.offline)?;
-        let sources_by_version = versions.get(&project.allowed_lib_paths)?;
+
+        let base_path = project.root();
+        let sources_by_version = versions.get(&project.allowed_lib_paths, base_path)?;
 
         let sources = if project.solc_jobs > 1 && sources_by_version.len() > 1 {
             // if there are multiple different versions, and we can use multiple jobs we can compile
@@ -239,13 +240,20 @@ impl<'a, T: ArtifactOutput> PreprocessedState<'a, T> {
     /// advance to the next state by compiling all sources
     fn compile(self) -> Result<CompiledState<'a, T>> {
         let PreprocessedState { sources, cache, sparse_output } = self;
-        let output = sources.compile(
+        let mut output = sources.compile(
             &cache.project().solc_config.settings,
             &cache.project().paths,
             sparse_output,
             cache.graph(),
         )?;
 
+        // source paths get stripped before handing them over to solc, so solc never uses absolute
+        // paths, instead `--base-path <root dir>` is set. this way any metadata that's derived from
+        // data (paths) is relative to the project dir and should be independent of the current OS
+        // disk. However internally we still want to keep absolute paths, so we join the
+        // contracts again
+        output.join_all(cache.project().root());
+
         Ok(CompiledState { output, cache })
     }
 }
@@ -457,9 +465,10 @@ fn compile_sequential(
                 continue
             }
             let input = input
-                .settings(opt_settings.clone())
+                .settings(opt_settings.clone().with_base_path(&paths.root))
                 .normalize_evm_version(&version)
                 .with_remappings(paths.remappings.clone())
+                .with_base_path(&paths.root)
                 .sanitized(&version);
 
             tracing::trace!(
@@ -539,6 +548,7 @@ fn compile_parallel(
                 .settings(settings.clone())
                 .normalize_evm_version(&version)
                 .with_remappings(paths.remappings.clone())
+                .with_base_path(&paths.root)
                 .sanitized(&version);
 
             jobs.push((solc.clone(), version.clone(), job, actually_dirty))
@@ -740,7 +750,6 @@ mod tests {
             .unwrap();
         let project = Project::builder().paths(paths).build().unwrap();
         let compiler = ProjectCompiler::new(&project).unwrap();
-        let out = compiler.compile().unwrap();
-        println!("{}", out);
+        let _out = compiler.compile().unwrap();
     }
 }
diff --git a/ethers-solc/src/lib.rs b/ethers-solc/src/lib.rs
index bf6c41fc..58e4dd8a 100644
--- a/ethers-solc/src/lib.rs
+++ b/ethers-solc/src/lib.rs
@@ -141,10 +141,12 @@ impl<T: ArtifactOutput> Project<T> {
     ///
     /// This will set the `--allow-paths` to the paths configured for the `Project`, if any.
     fn configure_solc(&self, mut solc: Solc) -> Solc {
-        if solc.args.is_empty() && !self.allowed_lib_paths.0.is_empty() {
+        if !self.allowed_lib_paths.0.is_empty() &&
+            !solc.args.iter().any(|arg| arg == "--allow-paths")
+        {
             solc = solc.arg("--allow-paths").arg(self.allowed_lib_paths.to_string());
         }
-        solc
+        solc.with_base_path(self.root())
     }
 
     /// Sets the maximum number of parallel `solc` processes to run simultaneously.
diff --git a/ethers-solc/src/remappings.rs b/ethers-solc/src/remappings.rs
index e8902673..7c2382bd 100644
--- a/ethers-solc/src/remappings.rs
+++ b/ethers-solc/src/remappings.rs
@@ -55,6 +55,14 @@ impl Remapping {
     pub fn into_relative(self, root: impl AsRef<Path>) -> RelativeRemapping {
         RelativeRemapping::new(self, root)
     }
+
+    /// Removes the `base` path from the remapping
+    pub fn strip_prefix(&mut self, base: impl AsRef<Path>) -> &mut Self {
+        if let Ok(stripped) = Path::new(&self.path).strip_prefix(base.as_ref()) {
+            self.path = format!("{}", stripped.display());
+        }
+        self
+    }
 }
 
 #[derive(thiserror::Error, Debug, PartialEq, Eq, PartialOrd)]
diff --git a/ethers-solc/src/resolver/mod.rs b/ethers-solc/src/resolver/mod.rs
index 9183fd0b..15fb77fb 100644
--- a/ethers-solc/src/resolver/mod.rs
+++ b/ethers-solc/src/resolver/mod.rs
@@ -701,12 +701,21 @@ pub struct VersionedSources {
 #[cfg(all(feature = "svm-solc"))]
 impl VersionedSources {
     /// Resolves or installs the corresponding `Solc` installation.
+    ///
+    /// This will also configure following solc arguments:
+    ///    - `allowed_paths`
+    ///    - `base_path`
     pub fn get(
         self,
         allowed_lib_paths: &crate::AllowedLibPaths,
+        base_path: impl AsRef<Path>,
     ) -> Result<std::collections::BTreeMap<crate::Solc, (semver::Version, Sources)>> {
         use crate::Solc;
 
+        // `--base-path` was introduced in 0.6.9 <https://github.com/ethereum/solidity/releases/tag/v0.6.9>
+        static SUPPORTS_BASE_PATH: once_cell::sync::Lazy<VersionReq> =
+            once_cell::sync::Lazy::new(|| VersionReq::parse(">=0.6.9").unwrap());
+
         // we take the installer lock here to ensure installation checking is done in sync
         #[cfg(any(test, feature = "tests"))]
         let _lock = crate::compile::take_solc_installer_lock();
@@ -743,8 +752,16 @@ impl VersionedSources {
                     tracing::trace!("reinstalled solc: \"{}\"", version);
                 }
             }
-            let solc = solc.arg("--allow-paths").arg(allowed_lib_paths.to_string());
+            let mut solc = solc
+                .arg("--allow-paths")
+                .arg(allowed_lib_paths.to_string())
+                .with_base_path(base_path.as_ref());
             let version = solc.version()?;
+
+            if SUPPORTS_BASE_PATH.matches(&version) {
+                solc = solc.arg("--base-path").arg(format!("{}", base_path.as_ref().display()));
+            }
+
             sources_by_version.insert(solc, (version, sources));
         }
         Ok(sources_by_version)


Test patch:
diff --git a/ethers-solc/tests/project.rs b/ethers-solc/tests/project.rs
index 8da70148..5aaad5aa 100644
--- a/ethers-solc/tests/project.rs
+++ b/ethers-solc/tests/project.rs
@@ -1434,7 +1434,6 @@ fn can_detect_contract_def_source_files() {
         .unwrap();
 
     let compiled = tmp.compile().unwrap();
-    println!("{}", compiled);
     assert!(!compiled.has_compiler_errors());
 
     let mut sources = compiled.output().sources;
@@ -1750,7 +1749,6 @@ contract D { }
         .unwrap();
 
     let compiled = project.compile().unwrap();
-    println!("{}", compiled);
     assert!(!compiled.has_compiler_errors());
 
     let cache = SolFilesCache::read(project.cache_path()).unwrap();