Verify contracts using Solc standard JSON input

[canepat]

This PR introduces the contract source verification on Etherscan using the Solc standard JSON input as proposed in issue #414.

Fixes #414

Working Assumptions
I've made the following working assumptions:

• a new test project contracts-nameclash-project is added to prove the name clash using flattening
• the verify-contract task now uses the Solc standard JSON input (instead of the Solidity single-file provided by flattening) as code format with Etherscan API
• the buidler-etherscan test project is extended with contracts producing some name clash
• the Solc standard JSON input is currently saved in config.paths.artifacts folder
• the Solc version for the buidler-etherscan test project is upgraded to 0.5.15 because using version 0.5.1 I was unable to verify on Etherscan the existent TestContract1, just upgrading Solc makes it work

Limitations
Currently there are some limitations:

• the Solc standard JSON input is saved always in config.paths.artifacts folder, no customization is possible
• when using the Solc standard JSON input the Etherscan API spec requires that the contractName parameter is provided in the <path_to_sol>:<contract_name> format. Currently it is hard-coded as contracts/${taskArgs.contractName}.sol:${taskArgs.contractName} in buidler-etherscan: far from ideal but making it work for any contract would require to change the verify-contract task arguments, so I prefer to leave this for a next PR, if this one is accepted

I'm looking forward to hear if this PR sounds good and in case to receive any advice or suggestion for improving this solution

[alcuadrado]

Hey @canepat,

Thanks for making this PR. I'm sorry I couldn't review it earlier.

I'm a little confused about how this works, as my understanding is that etherscan's API only accepts flattened sources and not the standard json input.

[alcuadrado]

Ohhhh, they just reuse some param names, but support the standard json format now! This is great news! 🥳

I'll look deeper into this later, but looking really good so far.

Much appreciated!

[canepat]

I've just added the usage of await bre.run(TASK_COMPILE_GET_COMPILER_INPUT) removing the unnecessary custom functions for loading/saving the standard Solc JSON input.

The last commit passes in Travis CI but fails in Netlify for some reason I don't understand, if you have any clue it would be great.

[alcuadrado]

Thanks @canepat!

I cleared netlifies cache, re-run it, and not it works.

I'll review your changes tomorrow.

[alcuadrado]

This line is the only problematic thing I can see, as it imposes the same names for files and contracts.

This could be fixed by doing something like

const solcInput : SolcInput = JSON.stringify(await run(TASK_COMPILE_GET_COMPILER_INPUT)); 

const filesWithContract = Object.entries(solcInput.sources).find((fileGlobalName, {content}) => hasContractDeclaration(content, taskArgs.contractName)).map((globalName) => globalName);

if (filesWithContract.length === 0) {
   // contract not found
   throw BuidlerPluginError(...);
}

if (filesWithContract.length > 1) {
   // throw error? what should be done here??
}

const contractName = `${filesWithContract[0}:${taskArgs.contractName}`;

[canepat]

Yes, you're absolutely right, it's a limitation of this PR.

An idea for handling any use case is extending the verify-contract task arguments to accept an optional contractFilePath argument (e.g. contracts/A.sol) to build the complete contract name as ${taskArgs.contractFilePath}:${taskArgs.contractName}; when the contractFilePath is not given, the default simplified name contracts/${taskArgs.contractName}.sol:${taskArgs.contractName} will be used.

Another option could be changing the contractName argument of the verify-contract task to become the full contract name, specified exactly as expected by Etherscan API ${contractFilePath}:${contractName} (e.g. contracts/A.sol:B.sol).

IMHO the latter option is slightly preferable even if it changes a bit the semantic of the contractName argument of the verify-contract task.

What do you think?

[pcowgill]

@alcuadrado Does this issue gate merging? Thanks!

[canepat]

@pcowgill this issue has been resolved in last commit 3695e1c

[pcowgill]

@canepat Great, thanks. I thought so but I wanted to confirm that

[pcowgill]

@alcuadrado Whenever you have time, there’s another commit here for your review. Thanks!

[alcuadrado]

Hey @canepat and @pcowgill, we were a little busy preparing this new release. Now that it's out, I'll review this soon.

Can you update it and change the base branch to development? Thanks!

[canepat]

I've just merged from and changed the base branch to development.

[pcowgill]

@tmilar @fvictorio Might either of you have a chance to review this PR this week or next? Thanks!

[alcuadrado]

Thanks for creating this PR @canepat, and thanks for your patience!