solcpiler icon indicating copy to clipboard operation
solcpiler copied to clipboard

Do not inclue `urls` for sources in the standard input if `useLiteralContent` is true.

Open 0xjac opened this issue 6 years ago • 3 comments

solcpiler version: 1.0.0-beta.8

Currently, the file:// url of all the sources are included in the current input in every case.
If the standard input sets settings.metadata.useLiteralContent = true, then the content should be used instead of the urls.

I am not a 100% sure that this is not a bug with solc or solcjs as well. Part of me would expect that urls should be dropped if the content is present. On the other hand both may be present and useLiteralContent could be considered merely an indication as to which to use.

In any case, since the urls are included, the metadata of the standard output will change if the path changes. (This happens when running on different machines or simply different folders). Since the hash of the metadata is included in the bytecode, this means that the bytecode changes every time the contract is compiled in a different folder or machine.

It would be nice to remove the local file path as url from the standard input and output if useLiteralContent is used.

Ultimately it may be nice as well to let users provide other urls for their files, such as for swarm.

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

Hmm, well we really are using the files.

solcpiler does the file reading b/c solcjs gives the error File import callback not supported when using a file url and native solc (which should be able to read files) gives me the error Unknown exception in read callback.

I do think that changing bytecode is an issue, so I merged your PR.

Ultimately it may be nice as well to let users provide other urls for their files, such as for swarm.

A nice option would be to provide a sources.json file that contains standardInput sources object to use in place of the --input option.

ewingrj avatar Aug 08 '18 00:08 ewingrj

@perissology Thanks for merging and yes changing bytecode is a real issue.

I do believe you should be able to set a URL to a file. However I don't think it should be a local file:// url. If you look in the standard input example in the solidity doc They say the following:

"myFile.sol": {
  // Required: keccak256 hash of the source file
  "keccak256": "0x123...",
  // Required (unless "content" is used, see below): Sorted URL(s)
  // to the source file, protocol is more or less arbitrary, but a
  // Swarm URL is recommended
  "urls": [ "bzzr://56ab..." ]
},

A swarm URL makes sense as it allows people to look at the file, IPFS could make sense as well but not a local file path. I think replacing the URL with the actual content makes sense too in some cases like ERC820 where the code may be deployed on multiple chains and clients may not have access to Swarm or IPFS.

I like the option of being able to pass a sources.json or even a partial solcStandardInput.json. I would recommend that if there is no files or if useLiteralContent=true, then the urls are explicitly removed. If a path is passed explicitly via sources.json then it will be used instead.

What do you think?

Also would you consider releasing a new version of solcpiler with the current changes?

0xjac avatar Aug 08 '18 11:08 0xjac

I like the option of being able to pass a sources.json or even a partial solcStandardInput.json. I would recommend that if there is no files or if useLiteralContent=true, then the urls are explicitly removed. If a path is passed explicitly via sources.json then it will be used instead.

That sounds good.

I just published a new version

ewingrj avatar Aug 08 '18 15:08 ewingrj