Repository: gakonst/ethers-rs
Pull request #948 (2022-02-22 13:56:41)

fix(solc): consistent serde for linked and unlinked bytecode
<!--
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
Fix https://github.com/gakonst/foundry/issues/787
<!--
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
Always prefix bytecode object with "0x"
<!--
Summarize the solution and provide any necessary context needed to understand
the code change.
-->

## PR Checklist

- [x] Added Tests
- [ ] Added Documentation
- [ ] Updated the changelog



Patch:
diff --git a/ethers-solc/src/artifacts/mod.rs b/ethers-solc/src/artifacts/mod.rs
index 9ad98b5e..d8f55081 100644
--- a/ethers-solc/src/artifacts/mod.rs
+++ b/ethers-solc/src/artifacts/mod.rs
@@ -1592,6 +1592,7 @@ pub enum BytecodeObject {
     #[serde(deserialize_with = "serde_helpers::deserialize_bytes")]
     Bytecode(Bytes),
     /// Bytecode as hex string that's not fully linked yet and contains library placeholders
+    #[serde(with = "serde_helpers::string_bytes")]
     Unlinked(String),
 }
 
@@ -1612,6 +1613,13 @@ impl BytecodeObject {
             BytecodeObject::Unlinked(_) => None,
         }
     }
+    /// Returns a reference to the underlying `String` if the object is unlinked
+    pub fn as_str(&self) -> Option<&str> {
+        match self {
+            BytecodeObject::Bytecode(_) => None,
+            BytecodeObject::Unlinked(s) => Some(s.as_str()),
+        }
+    }
 
     /// Returns the unlinked `String` if the object is unlinked or empty
     pub fn into_unlinked(self) -> Option<String> {
@@ -1715,10 +1723,10 @@ impl BytecodeObject {
     }
 }
 
-// Returns a not deployable bytecode by default as "0x"
+// Returns a not deployable bytecode by default as empty
 impl Default for BytecodeObject {
     fn default() -> Self {
-        BytecodeObject::Unlinked("0x".to_string())
+        BytecodeObject::Unlinked("".to_string())
     }
 }
 
diff --git a/ethers-solc/src/artifacts/serde_helpers.rs b/ethers-solc/src/artifacts/serde_helpers.rs
index 6aba852e..d2a6c151 100644
--- a/ethers-solc/src/artifacts/serde_helpers.rs
+++ b/ethers-solc/src/artifacts/serde_helpers.rs
@@ -77,6 +77,34 @@ pub mod json_string_opt {
     }
 }
 
+/// serde support for string
+pub mod string_bytes {
+    use serde::{Deserialize, Deserializer, Serializer};
+
+    pub fn serialize<S>(value: &String, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        if value.starts_with("0x") {
+            serializer.serialize_str(value.as_str())
+        } else {
+            serializer.serialize_str(&format!("0x{}", value))
+        }
+    }
+
+    pub fn deserialize<'de, D>(deserializer: D) -> Result<String, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let value = String::deserialize(deserializer)?;
+        if let Some(rem) = value.strip_prefix("0x") {
+            Ok(rem.to_string())
+        } else {
+            Ok(value)
+        }
+    }
+}
+
 pub mod display_from_str_opt {
     use serde::{de, Deserialize, Deserializer, Serializer};
     use std::{fmt, str::FromStr};
diff --git a/ethers-solc/src/config.rs b/ethers-solc/src/config.rs
index 9a7077b5..bff8bdea 100644
--- a/ethers-solc/src/config.rs
+++ b/ethers-solc/src/config.rs
@@ -278,8 +278,11 @@ impl ProjectPathsConfig {
 
         for import in imports.iter() {
             let import_path = self.resolve_import(target_dir, import.data())?;
-            let import_content = self.flatten_node(&import_path, graph, imported, true, true)?;
-            let import_content = import_content.trim().as_bytes().to_owned();
+            let import_content = self
+                .flatten_node(&import_path, graph, imported, true, true)?
+                .trim()
+                .as_bytes()
+                .to_owned();
             let import_content_len = import_content.len() as isize;
             let (start, end) = import.loc_by_offset(offset);
             content.splice(start..end, import_content);
diff --git a/ethers-solc/src/resolver/mod.rs b/ethers-solc/src/resolver/mod.rs
index c8e1ca60..104865fd 100644
--- a/ethers-solc/src/resolver/mod.rs
+++ b/ethers-solc/src/resolver/mod.rs
@@ -807,9 +807,13 @@ fn parse_data(content: &str, file: &Path) -> SolData {
                 .map(|(cap, name)| {
                     SolDataUnit::new(name.as_str().to_owned(), cap.to_owned().into())
                 });
-            imports = utils::find_import_paths(content)
-                .map(|m| SolDataUnit::new(PathBuf::from(m.as_str()), m.to_owned().into()))
-                .collect();
+            imports =
+                capture_outer_and_inner(content, &utils::RE_SOL_IMPORT, &["p1", "p2", "p3", "p4"])
+                    .iter()
+                    .map(|(cap, m)| {
+                        SolDataUnit::new(PathBuf::from(m.as_str()), cap.to_owned().into())
+                    })
+                    .collect();
         }
     };
     let license = content.lines().next().and_then(|line| {


Test patch:
diff --git a/ethers-solc/test-data/flatten-sample/contracts/Bar.sol b/ethers-solc/test-data/test-flatten-duplicates/contracts/Bar.sol
similarity index 52%
rename from ethers-solc/test-data/flatten-sample/contracts/Bar.sol
rename to ethers-solc/test-data/test-flatten-duplicates/contracts/Bar.sol
index 5cb19aba..5e9034a5 100644
--- a/ethers-solc/test-data/flatten-sample/contracts/Bar.sol
+++ b/ethers-solc/test-data/test-flatten-duplicates/contracts/Bar.sol
@@ -1,4 +1,4 @@
-//SPDX-License-Identifier: Unlicense
+//SPDX-License-Identifier: UNLICENSED
 pragma solidity >=0.6.0;
 
 contract Bar {}
diff --git a/ethers-solc/test-data/flatten-sample/contracts/Foo.sol b/ethers-solc/test-data/test-flatten-duplicates/contracts/Foo.sol
similarity index 66%
rename from ethers-solc/test-data/flatten-sample/contracts/Foo.sol
rename to ethers-solc/test-data/test-flatten-duplicates/contracts/Foo.sol
index f9cf562d..1f7086de 100644
--- a/ethers-solc/test-data/flatten-sample/contracts/Foo.sol
+++ b/ethers-solc/test-data/test-flatten-duplicates/contracts/Foo.sol
@@ -1,4 +1,4 @@
-//SPDX-License-Identifier: Unlicense
+//SPDX-License-Identifier: UNLICENSED
 pragma solidity >=0.6.0;
 
 import { Bar } from './Bar.sol';
diff --git a/ethers-solc/test-data/flatten-sample/contracts/FooBar.sol b/ethers-solc/test-data/test-flatten-duplicates/contracts/FooBar.sol
similarity index 74%
rename from ethers-solc/test-data/flatten-sample/contracts/FooBar.sol
rename to ethers-solc/test-data/test-flatten-duplicates/contracts/FooBar.sol
index eda3b10b..58ec146b 100644
--- a/ethers-solc/test-data/flatten-sample/contracts/FooBar.sol
+++ b/ethers-solc/test-data/test-flatten-duplicates/contracts/FooBar.sol
@@ -1,4 +1,4 @@
-//SPDX-License-Identifier: Unlicense
+//SPDX-License-Identifier: UNLICENSED
 pragma solidity >=0.6.0;
 
 import { Bar } from './Bar.sol';
diff --git a/ethers-solc/test-data/test-flatten-solang-failure/contracts/Contract.sol b/ethers-solc/test-data/test-flatten-solang-failure/contracts/Contract.sol
new file mode 100644
index 00000000..c0be0077
--- /dev/null
+++ b/ethers-solc/test-data/test-flatten-solang-failure/contracts/Contract.sol
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.10;
+
+import { Lib } from "./Lib.sol";
+
+// Intentionally erroneous code
+contract Contract {
+    failure();
+}
diff --git a/ethers-solc/test-data/test-flatten-solang-failure/contracts/Lib.sol b/ethers-solc/test-data/test-flatten-solang-failure/contracts/Lib.sol
new file mode 100644
index 00000000..407fb855
--- /dev/null
+++ b/ethers-solc/test-data/test-flatten-solang-failure/contracts/Lib.sol
@@ -0,0 +1,4 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.10;
+
+library Lib {}
diff --git a/ethers-solc/tests/project.rs b/ethers-solc/tests/project.rs
index 2a32d16f..4777c041 100644
--- a/ethers-solc/tests/project.rs
+++ b/ethers-solc/tests/project.rs
@@ -375,6 +375,7 @@ fn can_flatten_file() {
     assert!(result.is_ok());
 
     let result = result.unwrap();
+    assert!(!result.contains("import"));
     assert!(result.contains("contract Foo"));
     assert!(result.contains("contract Bar"));
 }
@@ -393,6 +394,7 @@ fn can_flatten_file_with_external_lib() {
     assert!(result.is_ok());
 
     let result = result.unwrap();
+    assert!(!result.contains("import"));
     assert!(result.contains("library console"));
     assert!(result.contains("contract Greeter"));
 }
@@ -409,6 +411,7 @@ fn can_flatten_file_in_dapp_sample() {
     assert!(result.is_ok());
 
     let result = result.unwrap();
+    assert!(!result.contains("import"));
     assert!(result.contains("contract DSTest"));
     assert!(result.contains("contract Dapp"));
     assert!(result.contains("contract DappTest"));
@@ -416,7 +419,7 @@ fn can_flatten_file_in_dapp_sample() {
 
 #[test]
 fn can_flatten_file_with_duplicates() {
-    let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("test-data/flatten-sample");
+    let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("test-data/test-flatten-duplicates");
     let paths = ProjectPathsConfig::builder().sources(root.join("contracts"));
     let project = TempProject::<ConfigurableArtifacts>::new(paths).unwrap();
 
@@ -426,10 +429,44 @@ fn can_flatten_file_with_duplicates() {
     assert!(result.is_ok());
 
     let result = result.unwrap();
-    assert_eq!(result.matches("contract Foo {").count(), 1);
-    assert_eq!(result.matches("contract Bar {").count(), 1);
-    assert_eq!(result.matches("contract FooBar {").count(), 1);
-    assert_eq!(result.matches(';').count(), 1);
+    assert_eq!(
+        result,
+        r#"//SPDX-License-Identifier: UNLICENSED
+pragma solidity >=0.6.0;
+
+contract Bar {}
+contract Foo {}
+
+contract FooBar {}
+"#
+    );
+}
+
+#[test]
+fn can_flatten_on_solang_failure() {
+    let root =
+        PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("test-data/test-flatten-solang-failure");
+    let paths = ProjectPathsConfig::builder().sources(&root.join("contracts"));
+    let project = TempProject::<ConfigurableArtifacts>::new(paths).unwrap();
+
+    let target = root.join("contracts/Contract.sol");
+
+    let result = project.flatten(&target);
+    assert!(result.is_ok());
+
+    let result = result.unwrap();
+    assert_eq!(
+        result,
+        r#"// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.10;
+
+library Lib {}
+// Intentionally erroneous code
+contract Contract {
+    failure();
+}
+"#
+    );
 }
 
 #[test]
@@ -459,7 +496,7 @@ fn can_detect_type_error() {
 fn can_compile_single_files() {
     let tmp = TempProject::dapptools().unwrap();
 
-    let foo = tmp
+    let f = tmp
         .add_contract(
             "examples/Foo",
             r#"
@@ -470,7 +507,7 @@ fn can_compile_single_files() {
         )
         .unwrap();
 
-    let compiled = tmp.project().compile_file(foo.clone()).unwrap();
+    let compiled = tmp.project().compile_file(f.clone()).unwrap();
     assert!(!compiled.has_compiler_errors());
     assert!(compiled.find("Foo").is_some());
 
@@ -485,8 +522,43 @@ fn can_compile_single_files() {
         )
         .unwrap();
 
-    let compiled = tmp.project().compile_files(vec![foo, bar]).unwrap();
+    let compiled = tmp.project().compile_files(vec![f, bar]).unwrap();
     assert!(!compiled.has_compiler_errors());
     assert!(compiled.find("Foo").is_some());
     assert!(compiled.find("Bar").is_some());
 }
+
+#[test]
+fn consistent_bytecode() {
+    let tmp = TempProject::dapptools().unwrap();
+
+    tmp.add_source(
+        "LinkTest",
+        r#"
+// SPDX-License-Identifier: MIT
+library LibTest {
+    function foobar(uint256 a) public view returns (uint256) {
+    	return a * 100;
+    }
+}
+contract LinkTest {
+    function foo() public returns (uint256) {
+        return LibTest.foobar(1);
+    }
+}
+"#,
+    )
+    .unwrap();
+
+    let compiled = tmp.compile().unwrap();
+    assert!(!compiled.has_compiler_errors());
+
+    let contract = compiled.find("LinkTest").unwrap();
+    let bytecode = &contract.bytecode.as_ref().unwrap().object;
+    assert!(bytecode.is_unlinked());
+    let s = bytecode.as_str().unwrap();
+    assert!(!s.starts_with("0x"));
+
+    let s = serde_json::to_string(&bytecode).unwrap();
+    assert_eq!(bytecode.clone(), serde_json::from_str(&s).unwrap());
+}