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

Add Multi-File Etherscan Verification Option #630

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

naszam
Copy link

@naszam naszam commented Mar 29, 2021

Description:

Add --multiple opt to dapp-verify-contract

Copy link
Contributor

@d-xo d-xo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the submission! Very much appreciated ✨ 🙏

I'm wondering if this should be the default verification method used in dapp create --verify, @MrChico do you have opinions here?

src/dapp/libexec/dapp/dapp-verify-contract Outdated Show resolved Hide resolved
src/dapp/libexec/dapp/dapp-verify-contract Outdated Show resolved Hide resolved
@MrChico
Copy link
Member

MrChico commented Apr 20, 2021

I don't think this should be the default verification method right now

@naszam
Copy link
Author

naszam commented Apr 20, 2021

If the proposed changes are ok, just need to explore the inputJSON a bit more, if working on the missing libraries field or trying to fix and use mk-standard-json instead to generate the inputJSON (that currently doesn't work out-of-the-box), then it will be ready for testing.

@MrChico
Copy link
Member

MrChico commented Apr 28, 2021

the changes so far look good! With mk-standard-json accomodated as discussed this should make for a nice improvement!

@d-xo
Copy link
Contributor

d-xo commented May 10, 2021

I'm getting a syntax error in dapp-mk-standard-json when I try to run dapp init with the latest state of this branch:

> /home/me/code/dapphub/dapptools/verify_multiple/result/bin/dapp  init
+ git init
Initialized empty Git repository in /tmp/tmp.cJ7ZCxPb4l/.git/
+ mkdir -p lib src
+ git add .gitignore
+ git add .gitattributes
+ git add Makefile
+ git add src/TmpCj7zcxpb4l.sol
+ git add src/TmpCj7zcxpb4l.t.sol
+ git commit -m 'dapp init TmpCj7zcxpb4l'
[master (root-commit) e94620b] dapp init TmpCj7zcxpb4l
 5 files changed, 31 insertions(+)
 create mode 100644 .gitattributes
 create mode 100644 .gitignore
 create mode 100644 Makefile
 create mode 100644 src/TmpCj7zcxpb4l.sol
 create mode 100644 src/TmpCj7zcxpb4l.t.sol
+ dapp install ds-test
+ git submodule add --force https://github.com/dapphub/ds-test lib/ds-test
Cloning into '/tmp/tmp.cJ7ZCxPb4l/lib/ds-test'...
remote: Enumerating objects: 166, done.
remote: Counting objects: 100% (70/70), done.
remote: Compressing objects: 100% (29/29), done.
remote: Total 166 (delta 29), reused 60 (delta 26), pack-reused 96
Receiving objects: 100% (166/166), 41.74 KiB | 689.00 KiB/s, done.
Resolving deltas: 100% (67/67), done.
+ git submodule update --init --recursive lib/ds-test
+ git commit -m 'dapp install ds-test'
[master 3ee0723] dapp install ds-test
 2 files changed, 4 insertions(+)
 create mode 100644 .gitmodules
 create mode 160000 lib/ds-test
++ dapp-remappings
+ DAPP_REMAPPINGS='ds-test/=lib/ds-test/src/
ds-test=lib/ds-test/src/index.sol'
+ make test
dapp test
  File "/nix/store/vn3bkvdds4gx1gz3xym2dvjfgd9455z1-dapp-0.32.2/libexec/dapp/dapp-mk-standard-json", line 34
    else
       ^
SyntaxError: invalid syntax
* Line 2, Column 1
  Syntax error: value, object or array expected.
* Line 1, Column 1
  A valid JSON document must be either an array or an object value.

make: *** [Makefile:3: test] Error 1

Copy link
Contributor

@d-xo d-xo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

---verify seems to be broken now. I consistently get Fail - Unable to verify when running dapp create --verify on kovan with this branch.

and you're still missing an entry for --verify-multiple in the OPTS string here so right now I get an error: unknown option 'verify-multiple' when I try dapp create --verify-multiple

@d-xo
Copy link
Contributor

d-xo commented May 11, 2021

oh, also please update the readme to mention these new options

@d-xo d-xo closed this May 11, 2021
@d-xo d-xo reopened this May 11, 2021
@d-xo
Copy link
Contributor

d-xo commented May 11, 2021

the close was an accident 🤦‍♀️

@transmissions11
Copy link
Contributor

any blockers here? i much prefer multi-file verification

I don't think this should be the default verification method right now

why so? 🤔

@naszam
Copy link
Author

naszam commented Sep 3, 2021

Hi @transmissions11,

Thanks for your interest!

The only blocker so far is the dapp-mk-standard-json that doesn't work out-of-the-box for multi-file verification as it's used mainly in dapp-build to compile source code.
We tried to adjust it but we had a blocker with the remappings to get all the imported contract dependencies (cc: @WilfredTA).
The idea is to write a new script from scratch (probably in python) to generate a standard inputJSON for multi-file verification inspired by truffle-plugin-verify or hardhat-deploy and then use it inside dapp-verify-contracts instead of dapp-mk-standard-json.
I think @e18r will give it a try, but feel free to jump in if you like!
Thanks! 🙂

@transmissions11
Copy link
Contributor

Ah interesting. The compilation stuff is a bit out of my league sadly.

Thanks for all the work on this so far 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants