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

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

Closed
2 tasks done
adhusson opened this issue Jul 5, 2023 · 1 comment · Fixed by foundry-rs/compilers#36
Closed
2 tasks done
Labels
T-bug Type: bug
Milestone

Comments

@adhusson
Copy link
Contributor

adhusson commented Jul 5, 2023

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (bf56869 2023-07-05T00:05:02.811991000Z)

What command(s) is the bug in?

forge script

Operating System

macOS (Apple Silicon)

Describe the bug

If you use files that are outside the project through a remapping (like@remapped/=../remapped/), and a file there does a relative import, then verification fails.

Reproduction

  1. forge init bad_verif,

  2. Add this to the initial bad_verif/foundry.ml:

remappings = ["@remapped/=../remapped/"]
allow_paths = ["../remapped/"]
  1. Create remapped/ so that you have
.
├── remapped
├── bad_verif
  1. In remapped/, create Parent.sol:
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
import "./Child.sol";

and Child.sol:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
  1. In Counter.sol, add:
import "@remapped/Parent.sol";
  1. Deploy with --broadcast --verify

Expected

Verification works.

Actual

If you deploy and verify, you will get the following error (I tried on polygon):

Fail - Unable to verify. Solidity Compilation Error: Source "../remapped/Child.sol" not found: File not found. Searched the following locations: "".

This is because the input json contains the following:

{
   "sources":{
      "src/Counter.sol": <content of Counter.sol>
      "../remapped/Parent.sol":{
         "content":"// SPDX-License-Identifier: UNLICENSED\npragma solidity ^0.8.13;\nimport \"./Child.sol\";\n"
      },
      "/Users/ah/temp/remapped/Child.sol": <content of Child.sol>
   },
   "settings":{
      "remappings":[
         "@remapped/=../remapped/"
      ],
   }
}

Suggestion

The key for Child.sol should probably use the relative path ../remapped/Child.sol instead of the absolute path /Users/ah/temp/remapped/Child.sol.

@adhusson adhusson added the T-bug Type: bug label Jul 5, 2023
@gakonst gakonst added this to Foundry Jul 5, 2023
@github-project-automation github-project-automation bot moved this to Todo in Foundry Jul 5, 2023
@adhusson adhusson changed the title Remappings to files outside the project subdirectory + 1 import fails verification etherscan verification failure (when remapping points to external file) Jul 6, 2023
@Evalir Evalir added this to the v1.0.0 milestone Jul 13, 2023
@Evalir Evalir removed this from Foundry Sep 15, 2023
@PaulRBerg
Copy link
Contributor

PaulRBerg commented Dec 17, 2023

Update

This comment is outdated, please see #3507.

Original Comment

Any updates on this? It's a blocker for projects that use Node.js packages instead of git submodules.

We've bumped into this issue while deploying a new release of Sablier V2. Steps for reproduction:

  1. Clone https://github.com/sablier-labs/v2-core
  2. Checkout 6104b54
  3. Run pnpm install
  4. Run this command:
FFOUNDRY_PROFILE=optimized \
forge verify-contract 0x26a8515af672e4d66d71a941a3d6a3e9e5e61a8c \
./src/SablierV2LockupLinear.sol:SablierV2LockupLinear \
--chain sepolia \
--etherscan-api-key $API_KEY_ETHERSCAN \
--watch \
--constructor-args \
$(cast abi-encode "constructor(address,address,address)" 0xb1bEF51ebCA01EB12001a639bDBbFF6eEcA12B9F 0x2006d43E65e66C5FF20254836E63947FA8bAaD68 0xE8fFEbA8963CD9302ffD39c704dc2c027128D36F)

This is definitely a bug in Forge because the IERC165.sol file exists:

cat node_modules/@openzeppelin/contracts/interfaces/IERC165.sol
// SPDX-License-Identifier: MIT
// OpenZeppelin Contracts v4.4.1 (interfaces/IERC165.sol)

pragma solidity ^0.8.0;

import "../utils/introspection/IERC165.sol";

cc @Evalir @mattsse

Interestingly, the verify command worked for a similar contract called LockupDynamic:

https://app.warp.dev/block/Pi8KR6PWF6cSqlpowkginR

It's strange because both contracts import stuff from OpenZeppelin (actually, they both import the exact same things).

mattsse pushed a commit to foundry-rs/compilers that referenced this issue Dec 28, 2023
…#36)

- Resolves foundry-rs/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:

```json
{
  "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
mattsse pushed a commit to foundry-rs/compilers that referenced this issue Dec 28, 2023
…35)

Currently, when a project includes symbolic links to Solidity files,
`Project::standard_json_input` returns a Standard JSON Input struct that
is unusable for contract verification on block explorers. `solc` cannot
compile these contracts based on the json, leading to verification
failures. This pull request addresses this issue.

I encountered this problem when using the `forge verify-contract`
command, which internally uses this function. While `forge build`
completes successfully, the verification command fails, indicating that
the block explorer cannot find a source file.

This problem occurs in projects that contain symlinked Solidity files
and when these files are imported from other files. My project,
structured as a monorepo, uses external libraries installed via pnpm.
These libraries, accessible from the `./node_modules/` directory, are
set up with remappings in my Foundry project. However, directories under
`./node_modules/` are symlinks pointing to other locations, leading to
this issue.

## Reproduction

I added a test named `can_create_standard_json_input_with_symlink` to
demonstrate the issue within this repository. Also, the error can be
reproduced using the `forge verify-contract` command and steps below:

Environment: macOS (Apple silicon), forge 0.2.0 (`c312c0d`
2023-12-22T00:20:29.297186000Z)

```console
$ mkdir repro && cd repro

$ mkdir -p project/src project/node_modules dependency

$ cat << EOF > project/src/A.sol
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.23;
import "@dependency/B.sol";
contract A is B {}
EOF

$ cat << EOF > dependency/B.sol
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.23;
import "./C.sol";
contract B is C {}
EOF

$ cat << EOF > dependency/C.sol
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.23;
contract C {}
EOF

$ cat << EOF > project/foundry.toml
[profile.default]
remappings = ["@dependency/=node_modules/dependency/"]
allow_paths = ["../dependency/"]
EOF

# Create a symbolic link
$ cd project/node_modules
$ ln -s ../../dependency dependency
$ cd ../..

# Display the file structure
$ tree
.
├── dependency
│   ├── B.sol
│   └── C.sol
└── project
    ├── foundry.toml
    ├── node_modules
    │   └── dependency -> ../../dependency
    └── src
        └── A.sol

# `build` succeeds
$ cd project
$ forge build

# `verify-contract` generates an unintended json (C.sol has a host absolute path name)
$ forge verify-contract --show-standard-json-input 0x0000000000000000000000000000000000000000 A | jq . > A.json
$ cat A.json
{
  "language": "Solidity",
  "sources": {
    "src/A.sol": {
      "content": "// SPDX-License-Identifier: UNLICENSED\npragma solidity ^0.8.23;\nimport \"@dependency/B.sol\";\ncontract A is B {}\n"
    },
    "node_modules/dependency/B.sol": {
      "content": "// SPDX-License-Identifier: UNLICENSED\npragma solidity ^0.8.23;\nimport \"./C.sol\";\ncontract B is C {}\n"
    },
    "/Users/t/bench/repro/dependency/C.sol": {
      "content": "// SPDX-License-Identifier: UNLICENSED\npragma solidity ^0.8.23;\ncontract C {}\n"
    }
  },
  "settings": {
    "remappings": [
      "@dependency/=node_modules/dependency/",
      "dependency/=node_modules/dependency/"
    ],
    "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": {}
  }
}

# `solc` cannot compile using the json
$ solc --standard-json --no-import-callback A.json | jq .errors
[
  {
    "component": "general",
    "errorCode": "6275",
    "formattedMessage": "ParserError: Source \"node_modules/dependency/C.sol\" not found: No import callback.\n --> node_modules/dependency/B.sol:3:1:\n  |\n3 | import \"./C.sol\";\n  | ^^^^^^^^^^^^^^^^^\n\n",
    "message": "Source \"node_modules/dependency/C.sol\" not found: No import callback.",
    "severity": "error",
    "sourceLocation": {
      "end": 81,
      "file": "node_modules/dependency/B.sol",
      "start": 64
    },
    "type": "ParserError"
  }
]
```

Manually editing the host filesystem's absolute path as shown below
allows the `solc` command to succeed.

```diff
   "node_modules/dependency/B.sol": {
     "content": "// SPDX-License-Identifier: UNLICENSED\npragma solidity ^0.8.23;\nimport \"./C.sol\";\ncontract B is C {}\n"
   },
-  "/Users/t/bench/repro/dependency/C.sol": {
+  "node_modules/dependency/C.sol": {
     "content": "// SPDX-License-Identifier: UNLICENSED\npragma solidity ^0.8.23;\ncontract C {}\n"
   }
 },
```

## Cause

The issue arises because import paths in Solidity files are processed by
the
[`std::fs::canonicalize`](https://doc.rust-lang.org/std/fs/fn.canonicalize.html)
function. This function resolves symlinks and normalizes paths. When
symlinks are resolved, it results in `solc` being unable to locate the
corresponding source paths in the json, as it relies on Solidity import
statements. Therefore, symlinks should not be resolved here. The paths
should be maintained as specified in the Solidity files, except for
basic normalization.

## Solution

To address this, I implemented an import path normalization function and
replaced the canonicalization function where necessary. Based on [the
Solidity documentation
page](https://docs.soliditylang.org/en/v0.8.23/path-resolution.html),
this function resolves `.` and `..` segments/components without
resolving symlinks.

The Standard JSON's source paths, for verification purposes, should be
based on the project root. This allows the compiler to find sources
within the json. The conversion of normalized paths to project
root-based paths occurs after all sources are processed. That conversion
is already implemented, so this PR doesn't need to address it. ([It
seems like the path conversion needs
improvement](foundry-rs/foundry#5307), but it
is a separate issue and should be handled in another PR.)


https://github.com/foundry-rs/compilers/blob/b1561d807c246c066c7c4b0c72c8eb64c696a43d/src/lib.rs#L514-L525

With the changes proposed in this PR, I confirmed the fix of the issue
by building `forge` with this patch and testing both the reproduction
case and my project. The resulting json matches the manually edited json
diff mentioned earlier.

A potential downside of this approach is that the same file could be
represented differently in the json if accessed via both symlinked and
real paths. However, I see no issues with this, and it aligns with the
behavior of the compiler's virtual filesystem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants