Verify contracts command #339

jbcarpanelli · 2018-07-26T15:31:54Z

Fixes #274

This is the first approach to a possible implementation of a contract verification on etherchain. I still have to code some integration tests for the etherchain communication workflow part.
Some validations have been implemented in order to forbid a user to try to verify a contract without first initializing, adding and pushing a contract to a network so we can retrieve the contract address from the zos.network.json file. Consider that etherchain only allows contract verification and publish on the mainnet, so if other network is specified an error (Unable to retrieve the contract creation bytecode) will be thrown.

Defined required and non-required command parameters.

Basically, network ControllerFor function was used to verify if zOS project was initialized, together with NetworkBaseController checkLocalContractDeployed to verify contract definition, deploy status and source code modification status

Implemented a local controller method to retrieve the contract source code path and also tiny function to flatten the contract source code

Implemented a new model Verifier to manage verifications with etherchain

spalladino

Awesome work @jcarpanelli! There are a few changes needed, but the code makes perfect use of the existing codebase, and seems quite robust. As soon as you implement these changes, and push the integration tests, we're ready to merge this into master.

spalladino · 2018-07-26T16:31:14Z

package-lock.json

@@ -9,8 +9,8 @@
      "resolved": "https://registry.npmjs.org/@mrmlnc/readdir-enhanced/-/readdir-enhanced-2.2.1.tgz",
      "integrity": "sha512-bPHp6Ji8b41szTOcaP63VlnbbO5Ny6dwAATtY6JTjh5N2OLrb5Qk/Th5cRkRQhkWCt+EJsYrNB0MiL+Gpn6e3g==",
      "requires": {
-        "call-me-maybe": "^1.0.1",
-        "glob-to-regexp": "^0.3.0"
+        "call-me-maybe": "1.0.1",


These diffs are caused by different versions of npm. Please make sure to use the latest one (6.1.0 at the time of this writing), and regen the package-lock using that one.

spalladino · 2018-07-26T16:33:39Z

src/commands/verify.js

+  if (optimizer && !optimizerRuns) {
+    throw new Error('Cannot verify contract without defining optimizer runs')
+  }
+  runWithTruffle(() => verify(contractAlias, options), options)


Note that the usage of runWithTruffle has changed recently, since it may alter options to inject defaults. For instance, in create.js:

await runWithTruffle(async (opts) => await createProxy({ contractAlias, initMethod, initArgs, force, ... opts }), options)

spalladino · 2018-07-26T16:34:30Z

src/models/Verifier.js

+  verifyAndPublish(remote, params) {
+    if (remote === 'etherchain') {
+      publishToEtherchain(params)
+    }


Let's either hardcode the remote option, or throw an error if we don't match any

spalladino · 2018-07-26T17:51:27Z

src/models/local/LocalBaseController.js

+    const contractName = this.packageFile.contract(contractAlias)
+    if (contractName) {
+      const contractDataPath = Contracts.getLocalPath(contractName)
+      const { compiler, sourcePath } = fs.parseJson(contractDataPath)


Note that the resulting sourcePath does not resolve any import aliases. This means that if the contract is part of a package (such as OpenZeppelin), the source path will be openzeppelin-solidity/contracts/MyContract.sol, instead of the correct node_modules/openzeppelin-solidity/contracts/MyContract.sol.

You should be able to distinguish between the two, as local contracts have an absolute path (such as /home/spalladino/Projects/zeppelinos/cli/contracts/mocks/ImplV1.sol) and the ones in a node module have a relative path.

I'm ok with leaving this as-is now and handling this case in a future PR.

As truffle-flattener is using the truffle-resolve package to resolve dependencies imports, it will automatically prepend the filesystem directory path (i.e., /home/spalladino/Projects/zeppelinos/cli/node_modules/) to the relative path that is read from the Contract.json file. This is also the expected behaviour when you flatten contracts that are not inside the project contracts folder, for example an OpenZeppelin contract.
You can check that implementation here. Please, let me know if I'm misunderstanding you or if I'm wrong.

spalladino · 2018-07-26T17:54:21Z

src/scripts/verify.js

@@ -0,0 +1,7 @@
+import ControllerFor from '../models/network/ControllerFor'
+
+export default function verify(contractAlias, { network = 'mainnet', txParams = {}, networkFile = undefined, optimizer = false, optimizerRuns = 200, remote = 'etherchain' }) {


Down the road, we may want to add a flag to verify all contracts (such as a --all option, like in the add command). Let's make sure that we have an issue that capture this, along with any future improvements we may want to work on.

spalladino

Awesome! Left a minor comment in the tests, after that, I think we're ready to merge.

spalladino · 2018-07-31T20:04:26Z

test/scripts/verify.test.js

+      networkFile = packageFile.networkFile(network)
+      await push({ network, networkFile, txParams })
+      const contracts = networkFile.contracts
+      networkFile.contracts = contracts


Why these two lines?

spalladino · 2018-07-31T20:05:49Z

test/scripts/verify.test.js

+  let packageFile
+  let networkFile
+  let logs
+  let axiosStub


Let's use this.VARNAME in the beforeEach blocks instead of let VARNAME here, for consistency. This makes sure the tests work if we later abstract them to a behaviour separate from the setup.

spalladino · 2018-07-31T20:07:07Z

Also: could you add an entry to the changelog.md with this new command?

- Some function naming enhancement - Removed unused imports in tests

* Start implementing verify command Defined required and non-required command parameters. * Change contract name parameter to contract alias * Implement necessary validations before publication Basically, network ControllerFor function was used to verify if zOS project was initialized, together with NetworkBaseController checkLocalContractDeployed to verify contract definition, deploy status and source code modification status * Tests for verify script validations * Add default remote * Flatten contract sourcecode Implemented a local controller method to retrieve the contract source code path and also tiny function to flatten the contract source code * Retrieve contract sourcecode and compiler version * Request etherchain with axios to verify contract Implemented a new model Verifier to manage verifications with etherchain * Modify optimization parameter handling * Modify variables names * Use more descriptive command options * Fix in contract address parameter * Regenerate package-lock * Fix strings typos * Add integration tests for remote http calls * Inject session options to verify script * Change error message in verify tests * Change some test variables management * Update changelog * Add some final fixes - Some function naming enhancement - Removed unused imports in tests

xinbenlv · 2018-08-10T02:53:41Z

Kudos for building such a wonderful feature. When is it planned to be released?

xinbenlv · 2018-08-10T03:40:35Z

Or how can I use the dev/nightly version? @jcarpanelli @spalladino

spalladino · 2018-08-10T14:14:38Z

Hey @xinbenlv! We are planning on a new 1.4.0 RC early next week. If not, you can checkout the CLI and build it on your end, but I wouldn't advise this for production usage, only for testing.

xinbenlv · 2018-08-10T15:57:29Z

Thank you, let me check how to build a dev version. Thank you for answering my question. Also, the actual question we have is that:

when built with ZOS proxy pattern, what's the proxy contract code that we can post and verify it on etherscan.io?
does ZOS plan to support verify on etherscan.io as oppose to etherchain.org which seems not supporting ropsten and other testnet?

spalladino · 2018-08-13T13:54:39Z

@xinbenlv

You can find it here.
Our original plans were working on etherscan, but they have a captcha set up that prevents us from automating the verification process.

xinbenlv · 2018-08-13T18:29:40Z

I see, thanks Santiago. Have etherscan told people if they have plan to support command-line verification process in the future, e.g. with a ssh public key or what other ways that both allow the verification to be fully automated and mitigate the risk of abuse and security attacks?

spalladino · 2018-08-13T19:05:00Z

We have contacted them privately, but got no response back so far

xinbenlv · 2018-08-13T20:06:49Z

I see, thank you, I posted in public on https://www.reddit.com/r/etherscan/comments/970voh/verification_through_api_commendline/

jbcarpanelli added 11 commits July 18, 2018 23:48

Start implementing verify command

8c4b9d7

Defined required and non-required command parameters.

Change contract name parameter to contract alias

12a6319

Tests for verify script validations

66887d5

Add default remote

acb40a8

Flatten contract sourcecode

f555512

Implemented a local controller method to retrieve the contract source code path and also tiny function to flatten the contract source code

Retrieve contract sourcecode and compiler version

f3bf1eb

Request etherchain with axios to verify contract

bb1c81a

Implemented a new model Verifier to manage verifications with etherchain

Modify optimization parameter handling

13525d5

Modify variables names

9aaa510

Use more descriptive command options

86c09e6

spalladino added the status:in progress Pull Request is a work in progress label Jul 26, 2018

spalladino self-assigned this Jul 26, 2018

spalladino suggested changes Jul 26, 2018

View reviewed changes

jbcarpanelli added 7 commits July 26, 2018 17:46

Fix in contract address parameter

fa8c2db

Regenerate package-lock

7867515

Fix strings typos

d3c6dee

Add integration tests for remote http calls

ebf654a

Inject session options to verify script

689fc65

Merge branch 'master' into verify_contracts_command

4568d54

Change error message in verify tests

cbb5b3b

spalladino suggested changes Jul 31, 2018

View reviewed changes

jbcarpanelli added 2 commits July 31, 2018 18:13

Change some test variables management

6590ec2

Update changelog

453960d

jbcarpanelli force-pushed the verify_contracts_command branch from 4f2111c to 453960d Compare July 31, 2018 21:14

Add some final fixes

04ceb17

- Some function naming enhancement - Removed unused imports in tests

spalladino approved these changes Aug 1, 2018

View reviewed changes

spalladino removed the status:in progress Pull Request is a work in progress label Aug 1, 2018

spalladino merged commit cb886ef into zeppelinos:master Aug 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Verify contracts command #339

Verify contracts command #339

jbcarpanelli commented Jul 26, 2018 •

edited

Loading

spalladino left a comment

spalladino Jul 26, 2018

spalladino Jul 26, 2018

spalladino Jul 26, 2018

spalladino Jul 26, 2018

jbcarpanelli Jul 31, 2018

spalladino Jul 26, 2018

spalladino left a comment

spalladino Jul 31, 2018

spalladino Jul 31, 2018

spalladino commented Jul 31, 2018

xinbenlv commented Aug 10, 2018

xinbenlv commented Aug 10, 2018 •

edited

Loading

spalladino commented Aug 10, 2018

xinbenlv commented Aug 10, 2018

spalladino commented Aug 13, 2018

xinbenlv commented Aug 13, 2018

spalladino commented Aug 13, 2018

xinbenlv commented Aug 13, 2018

		@@ -0,0 +1,7 @@
		import ControllerFor from '../models/network/ControllerFor'

		export default function verify(contractAlias, { network = 'mainnet', txParams = {}, networkFile = undefined, optimizer = false, optimizerRuns = 200, remote = 'etherchain' }) {

Verify contracts command #339

Verify contracts command #339

Conversation

jbcarpanelli commented Jul 26, 2018 • edited Loading

spalladino left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spalladino left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spalladino commented Jul 31, 2018

xinbenlv commented Aug 10, 2018

xinbenlv commented Aug 10, 2018 • edited Loading

spalladino commented Aug 10, 2018

xinbenlv commented Aug 10, 2018

spalladino commented Aug 13, 2018

xinbenlv commented Aug 13, 2018

spalladino commented Aug 13, 2018

xinbenlv commented Aug 13, 2018

jbcarpanelli commented Jul 26, 2018 •

edited

Loading

xinbenlv commented Aug 10, 2018 •

edited

Loading