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

Multiple compiler versions leads to overwrite the artifacts/contracts files #1384

Closed
montyly opened this issue Apr 8, 2021 · 3 comments
Closed

Comments

@montyly
Copy link

montyly commented Apr 8, 2021

Hi,

If a project has multiple compiler versions, the compilation artifacts generated in artifacts/contracts can be incomplete. This is because there is one file per contract/path, but multiple compiler versions will lead to a collision in the artifacts. This can prevent the correct analysis from third parties tools (such as Slither / Echidna)

For example, for a project with these three files:

  • contracts/0.6.sol
import "./test.sol";

contract B is A{

}
  • contracts/0.8.sol
import "./test.sol";

contract C is A{

}
  • contracts/test.sol:
contract A{
    
    function add(uint a, uint b) public returns(uint){
        return a + b;
    }
}

And this configuration:

module.exports = {
  solidity: {
    compilers: [
      {
        version: "0.8.0"
      },
      {
        version: "0.6.7",
      }
    ],
    overrides: {
      "contracts/0.6.sol": {
        version: "0.6.7",
      },
      "contracts/0.8.sol": {
        version: "0.8.0",
      }
    }
  }
};

Both artifacts/contracts/test.sol/A.json and artifacts/contracts/test.sol/A.dbg.json will be generated only once, while the contract A will generate a different bytecode with solidity 0.6.7 and 0.8.0. In my tests both files are the 0.8.0 version, but I am assuming this might be random.

Note that the files in artifacts/build-info don't have this issue.

@alcuadrado
Copy link
Member

Hey @montyly,

Artifacts aren't being overwritten, but we emit one version for each of them. This is like this by design, as otherwise users would have to be thinking about which version they want to use for testing, deployment, etc.

A solidity file can be included multiple compilation jobs, and hence multiple build info files. To figure out which was the one that was used for emitting its artifacts, you have to look for the .dbg.json files.

I'm not sure which kind of analysis crytic runs, but it may require some adaptions to work with this multiple versions and compilation jobs per project model. If you tell me more about your needs I can help you understand how it can be done.

@alcuadrado alcuadrado added package:hardhat-core status:needs-more-info There's not enough information to start working on this issue labels Apr 10, 2021
@montyly
Copy link
Author

montyly commented Apr 15, 2021

Thanks, @alcuadrado for the details. I am not sure to understand why there are not being overwritten. If by reading artifacts/contracts/test.sol/A.dbg.json, I will have access only to one bytecode and not the other (in this example, either 0.6.7 or 0.8.0). If an external tool reads this file, it will have no information about the existence of the second version, right?

But we ended up not using the .dbg.json files, and directly reading the files generated by build-info.

Regarding our usage, I think there are two cases here: either the codebase requires multiple compilation units (ex: two solidity versions), or it does not require it, and from my understanding hardhat does it for optimization (so you don't need to recompile everything every time).

For the first case, we are working on its support in our tools.

For the second case, it is more complex. We need to analyze the AST of each contract, and the AST contains unique IDs such that you should analyze the contracts within the same compilation unit context (even if the compiler version is the same). For example, one of these IDs is for the linearizedBaseContracts info, which looks like:

      "linearizedBaseContracts":
      [
        4,
        20
      ],

This returns the inherited contracts, in the expected order. Each number matches an id field in one of the other contract's AST. These numbers can change between compilation units. So I can take the contract A, and easily know all the derived contracts by looking at one compilation unit. Now if it is split into two compilation units, I have two contracts A, with different ID. We can determine that both are the same based on the filename, but it does make the analysis of the AST more complex. These ids appear in a lot of locations, as a lot of objects produce them, (ex: structure, ...).

Would it be possible to have a command-line flag to disable the split into multiple compilation units optimization? This would ease a lot the integrations of hardhat in our tools (and probably other ones too). A command-line flag would be less intrusive than an option in the config file, so that we can run hardhat within the context of our tools, without messing with the target's configuration.

@fvictorio
Copy link
Member

Hey @montyly, I'm re-visiting old issues and I'm not sure what to do about this one.

Some clarifications about our compilation pipeline:

First, this is not an optimization, it's more of a design decision to have a simpler model. The bytecode that ends up in an artifact for a given contract is generated with a single version, and we have some rules to decide which one is used. In fact, there's no way to emit bytecode with different compilers for the same contract.

Second, this is (or should be!) deterministic. So for a given configuration of set of files, each file will always be compiled with the same version.

It's true that in your example the contract A is compiled with 0.6.7 because it's necessary to compile the B contract in 0.6.sol. But the artifact of A will be the one that is generated by 0.8.

If an external tool reads this file, it will have no information about the existence of the second version, right?

This is correct, unless you inspect the sources entries of each build-info. Maybe we could add a new entry to the .dbg.json, something like secondaryBuildInfos, which is an array with the other build infos that include that contract? If that seems useful to you, I will open a new issue for that.


I think I'm going to close this issue, on the grounds that this is working as intended. But I would very much like to continue discussing this, so if you have any other comments or ideas on what we could do to make your life easier, please let us know.

@fvictorio fvictorio closed this as not planned Won't fix, can't repro, duplicate, stale Dec 29, 2022
@github-project-automation github-project-automation bot moved this to Done in Hardhat Dec 29, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

3 participants