Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: create verifiable Standard JSON for projects with external files #36

Merged
merged 5 commits into from
Dec 28, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 80 additions & 10 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,8 +490,6 @@ impl<T: ArtifactOutput> Project<T> {
&self,
target: impl AsRef<Path>,
) -> Result<StandardJsonCompilerInput> {
use path_slash::PathExt;

let target = target.as_ref();
tracing::trace!("Building standard-json-input for {:?}", target);
let graph = Graph::resolve(&self.paths)?;
Expand All @@ -514,14 +512,7 @@ impl<T: ArtifactOutput> Project<T> {
let root = self.root();
let sources = sources
.into_iter()
.map(|(path, source)| {
let path: PathBuf = if let Ok(stripped) = path.strip_prefix(root) {
stripped.to_slash_lossy().into_owned().into()
} else {
path.to_slash_lossy().into_owned().into()
};
(path, source.clone())
})
.map(|(path, source)| (rebase_path(root, path), source.clone()))
.collect();

let mut settings = self.solc_config.settings.clone();
Expand Down Expand Up @@ -954,6 +945,46 @@ impl<T: ArtifactOutput> ArtifactOutput for Project<T> {
}
}

// Rebases the given path to the base directory lexically.
//
// The returned path from this function usually starts either with a normal component (e.g., `src`)
// or a parent directory component (i.e., `..`), which is based on the base directory. Additionally,
// this function converts the path into a UTF-8 string and replaces all separators with forward
// slashes (`/`).
//
// The rebasing process is as follows:
//
// 1. Remove the leading components from the path that match the base components.
// 2. Prepend `..` components to the path, equal in number to the remaining base components.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add an example here that illustrates this, otherwise this is a bit hard to understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the comment, adding examples to make it easier to understand.

fn rebase_path(base: impl AsRef<Path>, path: impl AsRef<Path>) -> PathBuf {
use path_slash::PathExt;

let mut base_components = base.as_ref().components();
let mut path_components = path.as_ref().components();

let mut new_path = PathBuf::new();

while let Some(path_component) = path_components.next() {
let base_component = base_components.next();

if Some(path_component) != base_component {
if base_component.is_some() {
new_path.extend(
std::iter::repeat(std::path::Component::ParentDir)
.take(base_components.count() + 1),
);
}

new_path.push(path_component);
new_path.extend(path_components);

break;
}
}

new_path.to_slash_lossy().into_owned().into()
}

#[cfg(test)]
#[cfg(all(feature = "svm-solc", not(target_arch = "wasm32")))]
mod tests {
Expand Down Expand Up @@ -1014,4 +1045,43 @@ mod tests {
let contracts = project.compile().unwrap().succeeded().output().contracts;
assert_eq!(contracts.contracts().count(), 2);
}

#[test]
fn can_rebase_path() {
assert_eq!(rebase_path("a/b", "a/b/c"), PathBuf::from("c"));
assert_eq!(rebase_path("a/b", "a/c"), PathBuf::from("../c"));
assert_eq!(rebase_path("a/b", "c"), PathBuf::from("../../c"));

assert_eq!(
rebase_path("/home/user/project", "/home/user/project/A.sol"),
PathBuf::from("A.sol")
);
assert_eq!(
rebase_path("/home/user/project", "/home/user/project/src/A.sol"),
PathBuf::from("src/A.sol")
);
assert_eq!(
rebase_path("/home/user/project", "/home/user/project/lib/forge-std/src/Test.sol"),
PathBuf::from("lib/forge-std/src/Test.sol")
);
assert_eq!(
rebase_path("/home/user/project", "/home/user/A.sol"),
PathBuf::from("../A.sol")
);
assert_eq!(rebase_path("/home/user/project", "/home/A.sol"), PathBuf::from("../../A.sol"));
assert_eq!(rebase_path("/home/user/project", "/A.sol"), PathBuf::from("../../../A.sol"));
assert_eq!(
rebase_path("/home/user/project", "/tmp/A.sol"),
PathBuf::from("../../../tmp/A.sol")
);

assert_eq!(
rebase_path("/Users/ah/temp/verif", "/Users/ah/temp/remapped/Child.sol"),
PathBuf::from("../remapped/Child.sol")
);
assert_eq!(
rebase_path("/Users/ah/temp/verif", "/Users/ah/temp/verif/../remapped/Parent.sol"),
PathBuf::from("../remapped/Parent.sol")
);
}
}
67 changes: 66 additions & 1 deletion tests/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use foundry_compilers::{
info::ContractInfo,
project_util::*,
remappings::Remapping,
Artifact, CompilerInput, ConfigurableArtifacts, ExtraOutputValues, Graph, Project,
utils, Artifact, CompilerInput, ConfigurableArtifacts, ExtraOutputValues, Graph, Project,
ProjectCompileOutput, ProjectPathsConfig, Solc, TestFileFilter,
};
use pretty_assertions::assert_eq;
Expand Down Expand Up @@ -1600,6 +1600,71 @@ fn can_sanitize_bytecode_hash() {
assert!(compiled.find_first("A").is_some());
}

// https://github.com/foundry-rs/foundry/issues/5307
#[test]
fn can_create_standard_json_input_with_external_file() {
// File structure:
// .
// ├── verif
// │   └── src
// │   └── Counter.sol
// └── remapped
// ├── Child.sol
// └── Parent.sol

let dir = tempfile::tempdir().unwrap();
let verif_dir = utils::canonicalize(dir.path()).unwrap().join("verif");
let remapped_dir = utils::canonicalize(dir.path()).unwrap().join("remapped");
Comment on lines +1616 to +1617
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After my previous PR is merged, this canonicalization can be removed. It is necessary on this branch because the symlink-resolving canonicalization is still in effect, and the temporary directory on macOS is a symlink.

fs::create_dir_all(verif_dir.join("src")).unwrap();
fs::create_dir(&remapped_dir).unwrap();

let mut verif_project = Project::builder()
.paths(ProjectPathsConfig::dapptools(&verif_dir).unwrap())
.build()
.unwrap();

verif_project.paths.remappings.push(Remapping {
context: None,
name: "@remapped/".into(),
path: "../remapped/".into(),
});
verif_project.allowed_paths.insert(remapped_dir.clone());

fs::write(remapped_dir.join("Parent.sol"), "pragma solidity >=0.8.0; import './Child.sol';")
.unwrap();
fs::write(remapped_dir.join("Child.sol"), "pragma solidity >=0.8.0;").unwrap();
fs::write(
verif_dir.join("src/Counter.sol"),
"pragma solidity >=0.8.0; import '@remapped/Parent.sol'; contract Counter {}",
)
.unwrap();

// solc compiles using the host file system; therefore, this setup is considered valid
let compiled = verif_project.compile().unwrap();
compiled.assert_success();

// can create project root based paths
let std_json = verif_project.standard_json_input(verif_dir.join("src/Counter.sol")).unwrap();
assert_eq!(
std_json.sources.iter().map(|(path, _)| path.clone()).collect::<Vec<_>>(),
vec![
PathBuf::from("src/Counter.sol"),
PathBuf::from("../remapped/Parent.sol"),
PathBuf::from("../remapped/Child.sol")
]
);

// can compile using the created json
let compiler_errors = Solc::default()
.compile(&std_json)
.unwrap()
.errors
.into_iter()
.filter_map(|e| if e.severity.is_error() { Some(e.message) } else { None })
.collect::<Vec<_>>();
assert!(compiler_errors.is_empty(), "{:?}", compiler_errors);
}

#[test]
fn can_compile_std_json_input() {
let tmp = TempProject::dapptools_init().unwrap();
Expand Down