trufflesuite / truffle

:warning: The Truffle Suite is being sunset. For information on ongoing support, migration options and FAQs, visit the Consensys blog. Thank you for all the support over the years.
https://consensys.io/blog/consensys-announces-the-sunset-of-truffle-and-ganache-and-new-hardhat?utm_source=github&utm_medium=referral&utm_campaign=2023_Sep_truffle-sunset-2023_announcement_
MIT License
14.02k stars 2.32k forks source link

sourcemap "source index" unspecified in build/contracts #1645

Closed rocky closed 5 years ago

rocky commented 5 years ago

Issue

The "source index" number (3rd field in s:l:f of https://solidity.readthedocs.io/en/v0.5.2/miscellaneous.html#source-mappings ) is unspecified in build/contracts/*.json

Steps to Reproduce

Consider this simple contract as the only contract in a truffle project:

pragma solidity ^0.5.0;

contract PublicStorageArray {
    bytes32[] public states = [bytes32(0)];
}

When compiled, here is an excerpt of the build/contracts/PublicStorageArray.json:

{
    "contractName": "PublicStorageArray",
    ...
    "sourceMap": "25:75:1:-;...",
    "deployedSourceMap": "25:75:1:-;...",
    ...
    "ast": {
      "nodes": [
        {
        "nodeType": "PragmaDirective",
        "src": "0:23:1"
        } ...
      ]
    }
}

Note that the 1 isn't really reconstructable from the rest of the file.

Expected Behavior

Best in my opinion would be to add a "sources" which refers to all of the contracts that were in the collection compiled, where the 1 position is in fact the file that contans the contract. In other words:

{
    "contractName": "PublicStorageArray",
    "sources": ["/tmp/foo/contracts/Migrations.sol", "/tmp/foo/contracts/PublicStorageArray.sol"],
   ...
}

Another less good possibility is change all of the 1's to 0's and assume 0 refers to the "sourcePath" field. A potential problem here is that this assumes that it assmes that code from a contract can come from only one file. (I am not sure that is true and will always be true; e.g. do "imports" change this?)

Finally, I should note, nothing might be done and would-be consumers of this information would have to adjust ast "src" and source and "sourceMap" reading to ingore the source index field.

Environment

Possible Fix concept.

In truffle-workflow-compile writeContracts() add before the call to saveAll():

    const contractNames = Object.keys(contracts).sort();
    const sources = contractNames.map(c => contracts[c].sourcePath);
    for (let c of contractNames) {
    contracts[c].sources = sources;
    }

which creates a sources field. And in truffle-contract-schema/index.js add before "userdoc":

  "sources": {},
gnidan commented 5 years ago

Hey @rocky,

Thanks for opening this. Wanted to say first that this is a problem we're extremely aware of and currently looking at solutions for.

Besides that, however, simply including sources does not work because Truffle does partial compilations: not all sources are compiled every time, and so there is the risk of mismatch between file index and the current revision of a particular sourcePath.

Feel free to reach out if you'd like me to go into more detail. This particular issue has been an enormous pain-point internally and so I can tell you that you are not alone in this frustration.

rocky commented 5 years ago

Hmm. Given the complexity of what can be compiled when, the dynamic nature of files getting renamed, added, and removed, the only sane (simple) solution would be to make sure that each artifacts JSON file written is self contained and self consistent.

When a compilation is passed off to solc-js, it has a consistent, fixed view of what files it is dealing with. So it seems to me that this list of files needs to be recorded somewhere in the output JSON. In other words, means adding something like sources in each written JSON file, right?

If I am missing something, have faulty logic, or things are more complex, please do educate me.

gnidan commented 5 years ago

Yep @rocky, those JSON files really do need to be self-contained. But they're already too large (#1269), so we can't just go adding all the information we need.

I've been spending some time recently looking into this, and I believe I am on the trail of a working solution.

rocky commented 5 years ago

Ok @gnidan will wait patiently and I am eager to see what you all come up with. I hope allowing what's in there to be user/tool configurable is possible.

Otherwise developers like the MythX folks who use truffle packages as a library will need a separate artifacts folder or files.

rocky commented 5 years ago

@gnidan I wrote that comment before I looked at the issue you cited - my bad. I see you may be considering this, at least from the query side. Great!

gnidan commented 5 years ago

@rocky more than only the query side! goal is a viable upgrade path for an improved system of record-keeping. See some wip docs here if you're curious about the current requirements/design.

gnidan commented 5 years ago

Closing this as duplicate of #1236. Thanks for raising this @rocky!