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
import path canonicalization results in file paths that are not on disk #2609
Comments
if this is about the absolutePath json field, I'm pretty sure that we never touch that since this is solc output But I think you found the buggy code already. Looking at: "sources": {
"lib/solady/ext/woke/ERC20Mock.sol": {
"keccak256": "0x508149a04ca17da422e50df1d25fcc5dd88ef666398eeca6800686a6987f95ff",
"urls": [
"bzz-raw://30e858817901565516da9416fb800c3f64cc3dc95b38736ef008248e4b71891c",
"dweb:/ipfs/QmcKh4wdg7Gy5QWawJavG694W3RjLmmaifALFFZyu6adGp"
],
"license": "MIT"
},
"src/tokens/ERC20.sol": {
"keccak256": "0x264e4675697d05dfb9bbe9cc91c6bda7962d934f1e940336fd75d509b7f396c4",
"urls": [
"bzz-raw://5856338689f03f36c057203c5085243e104b8487274432062ebf076b512edeea",
"dweb:/ipfs/QmXrqgaWQikKkHfoBkYPxeMTJWUY5uf7kSmipNbpU35XwK"
],
"license": "MIT"
},
"src/utils/SafeTransferLib.sol": {
"keccak256": "0x64af21dfdc2042a9dc0c6e970072523f11c934e14aba05696b17e8f736d2a70e",
"urls": [
"bzz-raw://4f9ffeb99b644857294b1535ad3d952ee705237d4208e48e18f54a20818aa09c",
"dweb:/ipfs/QmQKmo7qqxUsyivySvbvSe3cgyvJRAqztejcdgGEWRCddx"
],
"license": "MIT"
} for some reason the relative path from project dir to solady need to debug what's going on here. thanks for this. |
Yeah, I think the root cause is related to the path prefix being stripped in the sources which I believe ultimately affects solc's absolutePath |
fyi @Evalir |
Ive recently been messing with ethers_solc, remappings, paths and also am familiar enough with slither. I wouldn't mind giving this fix a try. A question for @mattsse, @DaniPopes and @Evalir. since this includes foundry and ethers, what's the easiest way to iterate on a fix? |
@Evalir was looking into this but no fix yet, sharing some context:
you can check out both repos, then uncomment this in foundry, so it uses your local ethers and compile forge locally, I recommend compiling in debug mode I think the bug is somewhere here: ethers-rs/ethers-solc/src/config.rs Lines 272 to 277 in edf3353
ethers-rs/ethers-solc/src/config.rs Line 328 in edf3353
ethers-rs/ethers-solc/src/config.rs Line 361 in edf3353
|
Thanks for the tips @mattsse. I built the debug bin as you said and then symlinked the bin to Started digging into this. I have a feeling that ethers is handling this correctly. I looked through the Here is CompilerInput for alpharush's repro (i minified the contract code to be readable): [ethers-rs/ethers-solc/src/compile/project.rs:555] &input = CompilerInput {
language: "Solidity",
sources: {
"lib/solady/ext/woke/ERC20Mock.sol": Source {
content: "pragma solidity ^0.8.4;\nimport \"src/tokens/ERC20.sol\";\ncontract ERC20Mock is ERC20 {}",
},
"lib/solady/src/tokens/ERC20.sol": Source {
content: "pragma solidity ^0.8.4;\nabstract contract ERC20 {}",
},
"src/Counter.sol": Source {
content: "pragma solidity 0.8.13;\nimport \"lib/solady/src/tokens/ERC20.sol\";\ncontract Counter {}",
},
"test/Counter.t.sol": Source {
content: "pragma solidity 0.8.13;\nimport {Counter} from \"../src/Counter.sol\";\nimport {ERC20Mock} from \"lib/solady/ext/woke/ERC20Mock.sol\";\ncontract CounterTest {}",
},
},
settings: Settings {
remappings: [
Remapping {
context: None,
name: "ds-test/",
path: "lib/forge-std/lib/ds-test/src",
},
Remapping {
context: None,
name: "forge-std/",
path: "lib/forge-std/src",
},
Remapping {
context: None,
name: "solady/",
path: "lib/solady",
},
],
...,
libraries: Libraries {
libs: {},
},
},
} Notice that all these paths are absolute from the root of the repo. Then i followed this object to see what the actual [ethers-rs/ethers-solc/src/compile/mod.rs:579] &cmd = Command {
program: "/Users/plotchy/.svm/0.8.13/solc-0.8.13",
args: [
"/Users/plotchy/.svm/0.8.13/solc-0.8.13",
"--base-path",
"/Users/plotchy/code/learning/ethers-issue-2609",
],
cwd: Some(
"/Users/plotchy/code/learning/ethers-issue-2609",
),
}
[ethers-rs/ethers-solc/src/compile/mod.rs:579] &self.args = [
"--allow-paths",
"/Users/plotchy/code/learning/ethers-issue-2609,/Users/plotchy/code/learning/ethers-issue-2609/lib",
"--include-path",
"/Users/plotchy/code/learning/ethers-issue-2609/lib/solady",
] This is what i'd expect again. From here, we add on the full command is something like: Leaving these breadcrumbs here if anyone sees something out of place. Do we know what solc command hardhat uses? |
thanks for this! hmm, it maybe there's something wrong with postprocessing of the compileroutput then you can run with env var |
Ok, here are the jsons for in and out. I pruned the out.json in this post to contain relevant portions. full jsons are here: in.json has all absolute paths. out.json's main points of contention are that the ast for Knowing that in.json has all absolute paths for the sources, I'm not sure how ethers could provide the input differently. As far as I can tell, postprocessing for build info doesnt touch the output ast absolutePaths. There could be an opportunity to touch them up for sure. Does hardhat do that? Or do they provide the input differently? in.json {
"language": "Solidity",
"sources": {
"lib/solady/ext/woke/ERC20Mock.sol": {
"content": "pragma solidity ^0.8.4;\nimport \"src/tokens/ERC20.sol\";\ncontract ERC20Mock is ERC20 {}"
},
"lib/solady/src/tokens/ERC20.sol": {
"content": "pragma solidity ^0.8.4;\nabstract contract ERC20 {}"
},
"src/Counter.sol": {
"content": "pragma solidity 0.8.13;\nimport \"lib/solady/src/tokens/ERC20.sol\";\ncontract Counter {}"
},
"test/Counter.t.sol": {
"content": "pragma solidity 0.8.13;\nimport {Counter} from \"../src/Counter.sol\";\nimport {ERC20Mock} from \"lib/solady/ext/woke/ERC20Mock.sol\";\ncontract CounterTest {}"
}
},
"settings": {
"remappings": [
"ds-test/=lib/forge-std/lib/ds-test/src/",
"forge-std/=lib/forge-std/src/",
"solady/=lib/solady/"
],
"optimizer": {
"enabled": true,
"runs": 200
},
"metadata": {
"useLiteralContent": false,
"bytecodeHash": "ipfs"
},
"outputSelection": {
"*": {
"": [
"ast"
],
"*": [
"abi",
"evm.bytecode",
"evm.deployedBytecode",
"evm.methodIdentifiers",
"metadata"
]
}
},
"evmVersion": "london",
"libraries": {}
}
} out.json {
"sources": {
"lib/solady/ext/woke/ERC20Mock.sol": {
"id": 0,
"ast": {
"absolutePath": "lib/solady/ext/woke/ERC20Mock.sol",
"id": 6,
"exportedSymbols": {
"ERC20": [
22
],
"ERC20Mock": [
5
]
},
"nodeType": "SourceUnit",
"src": "0:85:0",
"nodes": [
{
"id": 2,
"nodeType": "ImportDirective",
"src": "24:30:0",
"nodes": [],
"absolutePath": "src/tokens/ERC20.sol",
"file": "src/tokens/ERC20.sol",
"nameLocation": "-1:-1:-1",
"scope": 6,
"sourceUnit": 23,
"symbolAliases": [],
"unitAlias": ""
}
]
}
},
"lib/solady/src/tokens/ERC20.sol": {
"id": 1,
"ast": {
"absolutePath": "lib/solady/src/tokens/ERC20.sol",
}
},
"src/Counter.sol": {
"id": 2,
"ast": {
"absolutePath": "src/Counter.sol",
"id": 13,
"exportedSymbols": {
"Counter": [
12
],
"ERC20": [
8
]
},
"nodeType": "SourceUnit",
"src": "0:85:2",
"nodes": [
{
"id": 11,
"nodeType": "ImportDirective",
"src": "24:41:2",
"nodes": [],
"absolutePath": "lib/solady/src/tokens/ERC20.sol",
"file": "lib/solady/src/tokens/ERC20.sol",
"nameLocation": "-1:-1:-1",
"scope": 13,
"sourceUnit": 9,
"symbolAliases": [],
"unitAlias": ""
}
]
}
},
"src/tokens/ERC20.sol": {
"id": 3,
"ast": {
"absolutePath": "src/tokens/ERC20.sol",
"id": 23,
}
},
"test/Counter.t.sol": {}
},
"contracts": {
"lib/solady/ext/woke/ERC20Mock.sol": {},
"lib/solady/src/tokens/ERC20.sol": {},
"src/Counter.sol": {},
"src/tokens/ERC20.sol": {},
"test/Counter.t.sol": {}
}
} |
Since solc appends "src/tokens/ERC20.sol" to sources&contracts and uses that relative path as the key, now I was curious, and copied/renamed the solady lib as solmate. Then I imported another ERC20Mock into the Counter test file so that there would be two imports of the same relative path (colliding key). Counter.t.sol pragma solidity 0.8.13;
import {Counter} from "../src/Counter.sol";
// these both import "src/tokens/ERC20.sol"
import {ERC20Mock} from "lib/solady/ext/woke/ERC20Mock.sol";
import {ERC20Mock2} from "lib/solmate/ext/woke/ERC20Mock.sol";
contract CounterTest {} Now the compiler run fails.
I dont quite know how to digest all this info. If there is a way for ethers to help make the sources more unambiguous then we should we do that. From what I see in the input commands, we are already doing this as best we can. Postprocessing adjustments don't seem to fix this compiler issue with multiple libs colliding on names. |
@0xalpharush do you have a hardhat repro that complies with your needs that I could take a look at? Their docs do mention some special preprocess and postprocess steps.
|
Thanks for taking this on and thorough investigation @plotchy. I realize now hardhat works because it doesn't seem to include the It looks like we can use the remapping context feature to translate imports in dependencies to be relative to the root of the project. For example, in.json
out.json
|
Version
Via Foundry
forge 0.2.0 (8c1a569 2023-09-19T00:32:46.118501000Z)
Platform
Darwin macbook arm64
Description
The build artifacts output by forge using ethers-solc are not consistently converting import paths into absolute paths with the root of the project resolved. These file paths cannot be opened bc they are not on disk and it's not ideal to have to search the project directory for candidates as there's potentially duplicates that are not interchangeable.
I think when foundry projects are nested, the root resolution is relative to the context of the dependency and not the project (however the example below demonstrates two different behaviors). This causes issues for tools like Slither because when a project takes a dependency on solmate and solady, there's ambiguity about which
src/tokens/ERC20.sol
is referenced and the given path isn't truly absolute.How to create the project:
If I run
cat out/ERC20Mock.sol/ERC20Mock.json | grep 'absolutePath": "src/tokens/ERC20.sol'
, there is a result that I do not expect.Instead, I would expect the path for ERC20Mock to be absolute relative to the project root i.e. only the following command would return a result:
This expectation reflects the behavior of the import path resolution for Counter artifact which references the path,
lib/solady/src/tokens/ERC20.sol
:Anywhere
src/tokens/ERC20.sol
appears in the build artifacts in reference to Solady, I would expect to seelib/solady/src/tokens/ERC20.sol
(this is the behavior of hardhat and what I would consider correct as it's unambiguous)I think the relevant code is here but I haven't figured out a solution yet. Also, I imagine this is particularly delicate and probably requires more context and consideration that I can provide...
ethers-rs/ethers-solc/src/config.rs
Lines 240 to 261 in 08bcb67
The text was updated successfully, but these errors were encountered: