solcpiler icon indicating copy to clipboard operation
solcpiler copied to clipboard

Artifacts are overwritten when two files have contracts with the same name.

Open 0xjac opened this issue 7 years ago • 2 comments

If I have two solidity files: such as

contracts/Alice.sol

contract Bob {...}

contract Alice {
  address bob = Bob(0x...);
  ...
}

contracts/Bob.sol

contract Bob {...}

Since Alice < Bob, when compiling with ./contracts/**/*.sol, Alice.sol is first correctly compiled and artifacts Alice.json and Bob.json are created. Then Bob.sol is compiled and Bob.json is overwritten.

This works because the js code requiring the json will expect Bob.json to be for Bob.sol.

However, if a change is made in Alice.sol when compiling again, Bob.sol is skipped and Bob.json is overwritten and contain the bytecode and abi of Alice.sol's implementation of Bob.

I'm not sure what the best solution is for this. Here are a suggestions on how to approach it:

  1. Renamed the artifacts to <file_path>_contract_name.json similar to what solcjs does (Here you'd get contracts_Alice_Alice.json, contracts_Alice_Bob.json, contracts_Bob_Bob.json).

  2. Modify contracts.js such that it is a function to dynamically load the requested contract, where:

    1. The first parameter is the contract name
    2. The second parameter is path to the file containing the contract (with or without extension)
    3. The second parameter should default to contracts/<first_paramter>

    This would allow to import contracts with:

    const Alice = require('../artifacts/contracts')('Alice');
    const Bob = require('../artifacts/contracts')('Bob'); // from Bob.sol
    const Bob2 = require('../artifacts/contracts')('Bob', 'contracts/Alice'); // from Alice.sol
    const Bob3 = require('../artifacts/contracts')('Bob', 'contracts/Alice.sol'); // also from Alice.sol
    

    In the above example:

    • ('Alice, undefined) resolves to contracts_Alice_Alice.json`
    • ('Bob, undefined) resolves to contracts_Bob_Bob.json`
    • ('Bob', 'contracts/Alice') resolves to contracts_Alice_Bob.json
    • ('Bob', 'contracts/Alice.sol') resolves to contracts_Alice_Bob.json

This also has the added advantages that not all jsons are loaded when requiring contracts.js.

What do you think?

0xjac avatar Aug 04 '18 13:08 0xjac

I have a few concerns with this approach...

What happens when you need to generate an artifact for a library contract that is installed via npm. The name could get long and the node_modules in the name is likely to break the artifact name parsing.

Another thing to think about is will the names be absolute? What if the path is relative and traverses back a dir first (../node_modules/aragon/contracts/Kernel.sol)?

Also, contract.js is a convenience function and can't be used in a browser env. If I need to require(...) for each artifact I want to load, I might as we require the artifact directly and not use contracts.js.

Do you happen to know how truffle handles duplicate contract names?

I'll think about this some more. lmk if you have any more ideas.

ewingrj avatar Aug 08 '18 00:08 ewingrj

What happens when you need to generate an artifact for a library contract that is installed via npm.

The artifacts are only generated for the files you explcitly compile. Usually those are not in node_modules.

That being said, it may be possible to call solcpiler with a node module such as solcpiler -i ./node_modules/aragon/contracts/Kernel.sol. In that case you have:

  • file name: node_modules_aragon_contracts_Kernel_Kernel.json
  • require:
    const Kernel = require('../artifacts/contracts')('Kernel', 'node_modules/aragon/contracts/Kernel.sol');
    

Notice I left out the parent dir (..) out of the example because I am not exactly sure how to do it.

If I need to require(...) for each artifact I want to load, I might as we require the artifact directly and not use contracts.js.

You should be able to use something like webpack in the browser to require the contracts.js. With stable mapping for file names, you can also use webpack to require the json files directly. And if the json file name is a bit ugly, webpack should be able to map them.

Do you happen to know how truffle handles duplicate contract names? All I know is that in tests they use a custom function requireArtifact which is injected in the mocha test runner. In my example, you can consider require(contracts.js)(...) to be equivalent to requireArtifact(...). The difference in syntax is because you may not be able to inject global functions. I don't know how they do it under the hood tho.

I'm a bit busy at the moment, but I'll keep this issue in mind and I'm happy to work on it at a later date.

0xjac avatar Aug 13 '18 02:08 0xjac