Absolute paths in contract metadata

[cameel]

Issue

When compiling a Solidity project, Truffle gives the compiler absolute paths of all source files. Relative paths are used only for source located in libraries found by the resolver (e.g. when importing from node_modules/) but the fs module of the resolver sometimes creates absolute paths too.

The use of absolute paths to refer to Solidity source files makes it harder to reproduce the same bytecode on different machines because they are included in contract metadata,the hash of which is always appended at the end of the bytecode. It also potentially leaks user's local information (the name of the home directory if the contract is stored there).

Truffle uses Standard JSON, which gives it complete control over paths in compiler's VFS. The only real constraint is that they should match imports. Using relative paths for files from main project directory should be completely doable according to my knowledge.

There are two potential problems:

1. Files located somewhere outside of the main project directory and not being a part of a library.
2. Absolute paths used in imports.

I see several ways to deal with them, depending on how backwards-compatible this change needs to be:

1. Keep using absolute paths in those cases but warn the user that his file structure may leak into metadata.
2. Add extra features to keep fully support such use cases. For example let the user have multiple project directories (declared in the config file) so that paths can be made relative to them.
3. Disallow them completely (that's for example what Hardhat does).

Context

We're also currently working on solving this problem in the Solidity compiler (ethereum/solidity#11408, ethereum/solidity#11409, ethereum/solidity#11410). Unfortunately this will only help people who use the CLI. Truffle's use of Standard JSON mode isolates it from such changes as it's the path resolver that determines paths here.

This problem is also a direct cause of #1621 (and trufflesuite/truffle-compile#77).

Steps to Reproduce

mkdir /tmp/path-test/
cd /tmp/path-test/
truffle init
echo 'module.exports.compilers.solc.version = "0.8.5";' >> truffle-config.js

npm install @openzeppelin/contracts
truffle create contract C
echo 'import "../node_modules/@openzeppelin/contracts/utils/Arrays.sol";' >> contracts/C.sol
truffle compile
grep '\.sol' build/contracts/C.json

  "metadata": "{\"compiler\":{\"version\":\"0.8.5+commit.a4f2e591\"},\"language\":\"Solidity\",\"output\":{\"abi\":[{\"inputs\":[],\"stateMutability\":\"nonpayable\",\"type\":\"constructor\"}],\"devdoc\":{\"kind\":\"dev\",\"methods\":{},\"version\":1},\"userdoc\":{\"kind\":\"user\",\"methods\":{},\"version\":1}},\"settings\":{\"compilationTarget\":{\"/tmp/path-test/contracts/C.sol\":\"C\"},\"evmVersion\":\"berlin\",\"libraries\":{},\"metadata\":{\"bytecodeHash\":\"ipfs\"},\"optimizer\":{\"enabled\":false,\"runs\":200},\"remappings\":[]},\"sources\":{\"/tmp/path-test/contracts/C.sol\":{\"keccak256\":\"0x832eeeb2ab4434a1868d494358fd6f19436af78d9508d535d1b114fede11aee9\",\"license\":\"MIT\",\"urls\":[\"bzz-raw://08773a2a54b143ff179fd0103d917109c72a81d8afdec908993f14b3edeeb048\",\"dweb:/ipfs/QmRu2UgdBpZ8reaYJmdYQhnGc6qACs62SUVYEXt3cM5YM2\"]},\"/tmp/path-test/node_modules/@openzeppelin/contracts/utils/Arrays.sol\":{\"keccak256\":\"0x415faff3ea57601e0d5ebb1f02b0b808a302f8b661ff08d6b4a00c0ee00fa57e\",\"license\":\"MIT\",\"urls\":[\"bzz-raw://0520bd87d3e82f9761054220cc1c58b92598852d99e5ec090b427f3ae9d67a2c\",\"dweb:/ipfs/QmYTbyVy45dzkWnjvb9jhgVCfycEiefbATvusxk1Wepz32\"]},\"/tmp/path-test/node_modules/@openzeppelin/contracts/utils/math/Math.sol\":{\"keccak256\":\"0xa1477864def7febd9826918e50482a1ee7068b337b03804a7e0e98c674ac57c2\",\"license\":\"MIT\",\"urls\":[\"bzz-raw://5bdd8ea2ace1bf716007318f8aca21d32384f0b8b295adac55147270767441fa\",\"dweb:/ipfs/QmSv235N45Ub3wFXdTLNiS3k4b7FAudtEL8s6g1PwcsCcS\"]}},\"version\":1}",
  "source": "// SPDX-License-Identifier: MIT\npragma solidity >=0.4.22 <0.9.0;\n\ncontract C {\n  constructor() public {\n  }\n}\nimport \"../node_modules/@openzeppelin/contracts/utils/Arrays.sol\";\n",
  "sourcePath": "/tmp/path-test/contracts/C.sol",
    "absolutePath": "/tmp/path-test/contracts/C.sol",
        "absolutePath": "/tmp/path-test/node_modules/@openzeppelin/contracts/utils/Arrays.sol",
        "file": "../node_modules/@openzeppelin/contracts/utils/Arrays.sol",
    "absolutePath": "/tmp/path-test/contracts/C.sol",
        "absolutePath": "/tmp/path-test/node_modules/@openzeppelin/contracts/utils/Arrays.sol",
        "file": "../node_modules/@openzeppelin/contracts/utils/Arrays.sol",

Expected Behavior

• /tmp/path-test/contracts/C.sol should not be present anywhere in metadata. contracts/C.sol should be there instead.
• I think that importing files directly from npm's package directories should be disallowed or at least generate a warning. Users should be directed to import @openzeppelin/contracts/utils/Arrays.sol instead. Otherwise it's possible for the same file to end up under two different names in the JSON input.
  • If such imports are to be allowed, the path in the JSON input should be node_modules/@openzeppelin/contracts/utils/Arrays.sol because that's how the compiler would resolve ../node_modules/@openzeppelin/contracts/utils/Arrays.sol (which is a special, relative import).

Actual Results

Absolute paths present in metadata.

Environment

• Operating System: Arch Linux
• Truffle version: 5.2.4
• node version: v16.2.0
• npm version: 7.15.1

[cameel]

@gnidan, @haltman-at, @eggplantzzz would you be up for a call with Solidity team on this topic? We're very interested in solving this problem and would like to hear if there are any obstacles and how we could help.

[gnidan]

@cameel getting on a call sounds great. How should I reach out to schedule this?

[cameel]

We could discuss details on the #solc-tooling channel. Everyone is there from our side and any others from your team are welcome too :)

[cameel]

Here's my summary from the call. Sorry if I missed something. My notes were a bit chaotic unfortunately so please correct me if something's not right. The main takeaways are in bold.

• Why is Truffle using absolute paths?
  • Truffle needs to uniquely identify files internally.
  • Currently absolute paths are used as canonical IDs.
  • Truffle could convert them before passing them to the compiler but the requirement is that it has to be able to translate back.
• Changing paths in artifacts would be a breaking change and would have to wait for Truffle 6. Changing how it interfaces with the compiler is fine though.
• Problems with getting unique paths:
  • Generated files, e.g. for test assertions. These do not exist at all on disk.
  • Packages installed in home directory by EthPM.
• The files in the project directory could have some prefix in the JSON input to be distinguishable.
  • The lack of prefix in solc after Import path normalization (path spec v3) ethereum/solidity#11411 will make conflicts possible where the same path is present in project dir and in another location. In the compiler this will be solved by issuing an error or warning if a conflicting file actually exists.
  • Why not use a prefix only for generated files and leave everything without one?
    • This would require rewriting imports and Truffle does not do this.
  • First idea: prefix files from project directory with ./ and ../
    • Problem: such paths will likely be disallowed by the compiler in JSON input in the future.
      • (This was not on the call but please take a look at the new docs for relative imports for some context)
      • The way such imports interact with ../ used in imports is very unintuitive. If a file is called ../../a.sol and you put import "../b.sol" in it, the compiler will look for ../a.sol, not ../../../a.sol as one might expect.
  • Other ideas for a prefix: !/, @/.
    • What if someone has a directory name like that in his project directory?
    • Also, could a npm package have such a name?
      • Probably not.
  • Prefix that looks like a protocol: source:// or project://.
    • We agreed on project://. Truffle will start converting paths inside project directory to relative ones and prepending this prefix.
  • How will Sourcify deal with the prefix?
• Will prefixed files line up will what Remix or Hardhat do?
  • They will not but it's a separate concern.
  • There's a similar concern with files compiled on the command line with solc.
    • The way it is handled in solc is being reworked in Search path for libraries (path spec v3) ethereum/solidity#11409 but there's no provision for such a prefix. Tentative decision from @chriseth on the call was that the compiler will not be adding such a prefix (though there might be a way to specify a name for base path in the future).
• Files outside of the project directory imported directly:
  • Could be something like !/../../../file.sol.
    • This would not work as expected with import paths containing ../.
  • Hardhat disallows importing them.
  • It would be reasonable to disallow these in Truffle too.

[haltman-at]

So, as to how this could actually be done -- we actually already do something very similar with Vyper! When compiling Vyper, we remove the contracts directory, and treat it as the root directory. (This is for technical reasons involving how Vyper handles imports that I'm going to skip going into right now.)

Specifically, this happens in the collectSources function here; this is used for both Solidity and Vyper, but that third argument, contractsDirectory, is currently used only by Vyper. (Here is where Solidity invokes it, and here is where Vyper invokes it.)

So, this should be a fairly simple change. Say we rename the variable from contractsDirectory to something like baseDirectory (because for Vyper we'd still want to remove the contracts directory, whereas for Solidity it'd be the project directory we want to remove), and then maybe add a fourth argument that gives something to replace that directory with; and then of course just modify the invocation in compile-solidity appropriately.

So yay, we already mostly have machinery for this!

[gnidan]

Appreciate the excellent notes @cameel 🙇

[haltman-at]

OK, closing this now that the "project:/" fix is out. (We changed it from double-slash to single-slash in #4157 as double-slash was causing problems.) Let us know if there's more to do here.

[cameel]

Thanks! I did some testing to see if it handles all paths correctly and found a few issues. I think all of these are relatively minor problems:

1. Direct imports from project directory do not have the project:/ prefix #4161
2. Relative imports with leading ../ segments going beyond compiler's VFS root are not resolved correctly #4163
3. Difference in behavior between absolute import paths starting with multiple slashes and ones starting with a single one #4165
4. File names containing backslashes can shadow files in subdirectories on Linux #4166
5. Broken symlinks in contracts/ directory are silently ignored #4167

Another one I did not report: the way .. is resolved in relative imports slightly differs from how compiler does it (see docs for relative imports). When the path to the file containing an import contains .. or ., a relative import will "cancel" these instead of behaving like in an actual filesystem. E.g. import "../a.sol" inside /a/b/../c.sol will resolve into /a/b/a.sol, not /a/a.sol as one might expect. I could not find any situation where this discrepancy would do anything more than make a weird import not compile so I guess this is not a big issue. I did not report this because, the compiler's behavior here is not very intuitive I think it should be changed. The current behavior will still remain in released versions though so I'm just letting you know about this quirk in case you want to take it into account after all.

[aaronmboyd]

Fantastic, this makes verification so much easier, assuming the buidler is using the latest truffle version including the project:// fix.

[haltman-at]

@aaronmboyd Yes, note we ultimately changed it to project:/ rather than project://, though.