foundry-rs / foundry

Foundry is a blazing fast, portable and modular toolkit for Ethereum application development written in Rust.
https://getfoundry.sh
Apache License 2.0
8.04k stars 1.64k forks source link

solc compiler input contains same file multiple times, making build artifacts unusable #7591

Open 0xalpharush opened 4 months ago

0xalpharush commented 4 months ago

Component

Forge

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

What version of Foundry are you on?

forge 0.2.0 (0c961f7 2024-04-07T00:19:04.896666000Z)

What command(s) is the bug in?

forge build

Operating System

macOS (Apple Silicon)

Describe the bug

Foundry's artifacts contain multiple AST's for the same source file and this causes issues in tools like slither which maintain a mapping of source file -> AST. It makes it such that there are multiple reference ID's for what is an "identical" declaration presumably because the solc import resolution fails and adds the entry itself. I believe this is similar to but distinct from https://github.com/gakonst/ethers-rs/issues/2609

Steps to reproduce:

git clone https://github.com/code-423n4/2023-01-timeswap
yarn
cd v2-token
forge build --build-info --skip '*/test/** */script/**' --force
ls out/build-info 
cat $(find out/build-info/*.json)  | jq '.output.sources | keys' |  grep 'node_modules/@openzeppelin/contracts/token/ERC1155/IERC1155.sol'

The output:

  "../../node_modules/@openzeppelin/contracts/token/ERC1155/IERC1155.sol",
  "/Users/alpharush/2023-01-timeswap/node_modules/@openzeppelin/contracts/token/ERC1155/IERC1155.sol",

I expect there only to be one AST for each source unit (file) in each compilation unit (all sources compiled together) as this is should be unique and be an absolute path on disk. This will also causes issue for LSP-like functionality as there would appear to be twice as many declarations in the same file since multiple ASTs are produced with different reference ID's.

As explained in this comment, this happens when the import lookup fails.

I checked the solc compilerinput which looks ok, there's something weird with the output however because it contains duplicate entries for Divider.sol: src/Divider.sol and /src/Divider.sol, this is usually a sign that solc fails to look up an import of a file in the json compilerinput (which works like a virtual file system) and instead tries to find it on disk and also adds this to the compileroutput.

The repo's commit I ran this against:

$ git log
ef4c84fb8535aad8abd6b67cc45d994337ec4514
klkvr commented 4 months ago

This seems to be the case only for projects using libs outside of root. In the case above we get two different paths for the same file while resolving imports:

  1. import "@openzeppelin/contracts/token/ERC1155/IERC1155.sol" at "src/interfaces/IERC1155Enumerable.sol" is resolved as "$ROOT/../../node_modules/@openzeppelin/contracts/token/ERC1155/IERC1155.sol"
  2. import "./IERC1155.sol" at "$ROOT/../../node_modules/@openzeppelin/contracts/token/ERC1155/ERC1155.sol" is getting normalized (because it is relative) and resolved as "/Users/alpharush/2023-01-timeswap/node_modules/@openzeppelin/contracts/token/ERC1155/IERC1155.sol",

cc @mattsse we can probably address this by ensuring that we never strip project root while normalizing them, or just replace some parts of global path prefixes with $ROOT/../../../.. before passing to solc

not sure if we support that kind of layout at all, this might probably be not the only issue

0xalpharush commented 2 months ago

Another user is reporting issues relating to this https://github.com/crytic/slither/issues/2483