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

tash-2s · 2023-12-26T16:43:54Z

Resolves etherscan verification failure (when remapping points to external file) foundry#5307

Currently, Foundry projects containing Solidity files outside the project root directory face contract verification failures on block explorers. This issue occurs from the Standard JSON including unusable source paths for external files, represented as full absolute paths in their host file systems.

This PR addresses the issue by improving the path conversion process. For files not located under the project root directory, relative parent directory paths (..) are used, enabling the compiler to find the files within the json.

Steps to reproduce the issue are detailed in the linked issue above. Additionally, a test case representing that scenario has been added.

With this change, the json created in the reproduction scenario will appear as follows:

{
  "language": "Solidity",
  "sources": {
    "src/Counter.sol": {
      "content": "// SPDX-License-Identifier: UNLICENSED\npragma solidity ^0.8.13;\n\nimport \"@remapped/Parent.sol\";\n\ncontract Counter {\n    uint256 public number;\n\n    function setNumber(uint256 newNumber) public {\n        number = newNumber;\n    }\n\n    function increment() public {\n        number++;\n    }\n}\n"
    },
    "../remapped/Parent.sol": {
      "content": "// SPDX-License-Identifier: UNLICENSED\npragma solidity ^0.8.13;\nimport \"./Child.sol\";\n"
    },
    "../remapped/Child.sol": {
      "content": "// SPDX-License-Identifier: UNLICENSED\npragma solidity ^0.8.13;\n"
    }
  },
  "settings": {
    "remappings": [
      "@remapped/=../remapped/",
      "ds-test/=lib/forge-std/lib/ds-test/src/",
      "forge-std/=lib/forge-std/src/"
    ],
    "optimizer": {
      "enabled": true,
      "runs": 200
    },
    "metadata": {
      "useLiteralContent": false,
      "bytecodeHash": "ipfs",
      "appendCBOR": true
    },
    "outputSelection": {
      "*": {
        "": [
          "ast"
        ],
        "*": [
          "abi",
          "evm.bytecode",
          "evm.deployedBytecode",
          "evm.methodIdentifiers",
          "metadata"
        ]
      }
    },
    "evmVersion": "paris",
    "libraries": {}
  }
}

The source path is now aligned with the project root.

I have successfully deployed and verified the contract on Etherscan using this change.

forge create --rpc-url "wss://ethereum-holesky.publicnode.com" --verify --verifier-url "https://api-holesky.etherscan.io/api" --etherscan-api-key "..." --private-key "..." src/Counter.sol:Counter

https://holesky.etherscan.io/address/0xe08c332706185521fc8bc2b224f67adf814b1880#code

``` $ cargo test --test project --all-features can_create_standard_json_input_with_external_file Finished test [unoptimized + debuginfo] target(s) in 0.10s Running tests/project.rs (target/debug/deps/project-bc070fde513cf057) running 1 test test can_create_standard_json_input_with_external_file ... FAILED failures: ---- can_create_standard_json_input_with_external_file stdout ---- thread 'can_create_standard_json_input_with_external_file' panicked at tests/project.rs:1649:5: assertion failed: `(left == right)` Diff < left / right > : [ "src/Counter.sol", "../remapped/Parent.sol", < "/private/var/folders/1m/9vppwqks3pz4btz9gnczfmgh0000gn/T/.tmpSWn2KX/remapped/Child.sol", > "../remapped/Child.sol", ] note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace failures: can_create_standard_json_input_with_external_file test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 52 filtered out; finished in 1.38s error: test failed, to rerun pass `--test project` ```

tash-2s · 2023-12-26T16:49:47Z

tests/project.rs

+    let verif_dir = utils::canonicalize(dir.path()).unwrap().join("verif");
+    let remapped_dir = utils::canonicalize(dir.path()).unwrap().join("remapped");


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.

mattsse

amazing, tysm!

as mentioned in the other PR, I'd like to see a foundry companion PR with patched crate dep as a sanity check before we merge this

tash-2s · 2023-12-27T19:26:31Z

Thank you for your review!

I've created a companion PR in foundry with this patch. Also, I have confirmed that the tests in foundry passed successfully.

tash-2s · 2023-12-27T19:39:12Z

The workflow test for this PR failed due to a failure in nextest installation. As this issue is unrelated to the changes made, re-running the test should resolve it. https://github.com/foundry-rs/compilers/actions/runs/7331166894/job/19979490471

mattsse · 2023-12-27T20:02:54Z

src/lib.rs

+    for (i, (base_component, path_component)) in
+        base.components().zip(path.components()).enumerate()
+    {
+        if base_component == path_component {
+            new_path.pop_front();
+        } else {
+            let mut parent_dirs =
+                vec![std::path::Component::ParentDir; base.components().count() - i];
+            parent_dirs.extend(new_path);
+            new_path = parent_dirs.into();
+
+            break;
+        }
+    }


I think we can solve this a bit differently
don't zip the two iterators but create the two iters separately and then iterate over the path
and continue as long as base and path components match, once they diverge collect the remaining into a PathBuf

this way we don't need the vecdeque

Hmm, I'm not sure I understand your suggestion.

If we compare the components of base and path at the same positions, it seems similar to using zip.

Could you please elaborate on the type of implementation you have in mind?

something like

let mut components = Vec::new(); // perhaps we can even directly push components to a PathBuf? let mut base_iter = base.components(); let mut path_iter = path.components(); while let Some(path) = path_iter.next() { if Some(path) != base_iter.next() { components.push(path); components.extend(path_iter); } }

Thank you for your explanation. I now understand your point. I've made the change and pushed it.

mattsse

last pedantic doc nit

mattsse · 2023-12-28T17:51:27Z

src/lib.rs

+// 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.


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

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

mattsse

awesome job!

I'll merge both and upstream to foundry after a new release

PaulRBerg · 2023-12-29T08:39:17Z

Amazing work.

Doesn't this PR also address issue foundry-rs/foundry#3507?

tash-2s · 2023-12-29T15:40:41Z

@PaulRBerg I haven't looked into it deeply, but it seems like a different matter from this PR and #35, as they don't involve --via-ir.

tash-2s added 3 commits December 25, 2023 16:09

fix: create valid Standard JSON to verify for projects w/ external files

e0e1bfb

refactor: extract path rebasing process from standard json fuction

5c781d0

tash-2s requested review from DaniPopes and Evalir as code owners December 26, 2023 16:43

tash-2s commented Dec 26, 2023

View reviewed changes

mattsse reviewed Dec 27, 2023

View reviewed changes

tash-2s mentioned this pull request Dec 27, 2023

test(foundry-compilers): companion for fixing Standard JSON Input with external file foundry-rs/foundry#6676

Closed

tash-2s requested a review from mattsse December 27, 2023 19:39

mattsse requested changes Dec 27, 2023

View reviewed changes

refactor: more efficient way without VecDeque

38eff70

tash-2s requested a review from mattsse December 28, 2023 17:39

mattsse requested changes Dec 28, 2023

View reviewed changes

docs: improve rebase_path comment

bf564fb

tash-2s requested a review from mattsse December 28, 2023 19:46

mattsse approved these changes Dec 28, 2023

View reviewed changes

mattsse merged commit 247161c into foundry-rs:main Dec 28, 2023
20 checks passed

tash-2s deleted the fix-standard-json-input-external branch December 28, 2023 21:17

tash-2s mentioned this pull request Jan 5, 2024

feat: allow single file remappings foundry-rs/foundry#6706

Open

2 tasks

mattsse mentioned this pull request Jan 9, 2024

Contract verification fails with dependency "File not found" in pnpm monorepo foundry-rs/foundry#6653

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

tash-2s commented Dec 26, 2023

tash-2s Dec 26, 2023

mattsse left a comment

tash-2s commented Dec 27, 2023

tash-2s commented Dec 27, 2023

mattsse Dec 27, 2023 •

edited

Loading

tash-2s Dec 28, 2023

mattsse Dec 28, 2023 •

edited

Loading

tash-2s Dec 28, 2023

mattsse left a comment

mattsse Dec 28, 2023

tash-2s Dec 28, 2023

mattsse left a comment

PaulRBerg commented Dec 29, 2023

tash-2s commented Dec 29, 2023

		let verif_dir = utils::canonicalize(dir.path()).unwrap().join("verif");
		let remapped_dir = utils::canonicalize(dir.path()).unwrap().join("remapped");

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

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

Conversation

tash-2s commented Dec 26, 2023

tash-2s Dec 26, 2023

Choose a reason for hiding this comment

mattsse left a comment

Choose a reason for hiding this comment

tash-2s commented Dec 27, 2023

tash-2s commented Dec 27, 2023

mattsse Dec 27, 2023 • edited Loading

Choose a reason for hiding this comment

tash-2s Dec 28, 2023

Choose a reason for hiding this comment

mattsse Dec 28, 2023 • edited Loading

Choose a reason for hiding this comment

tash-2s Dec 28, 2023

Choose a reason for hiding this comment

mattsse left a comment

Choose a reason for hiding this comment

mattsse Dec 28, 2023

Choose a reason for hiding this comment

tash-2s Dec 28, 2023

Choose a reason for hiding this comment

mattsse left a comment

Choose a reason for hiding this comment

PaulRBerg commented Dec 29, 2023

tash-2s commented Dec 29, 2023

mattsse Dec 27, 2023 •

edited

Loading

mattsse Dec 28, 2023 •

edited

Loading