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

Link from unverified contracts to Sourcify to encourage verifying #669

Merged
merged 1 commit into from Jul 11, 2023

Conversation

lukaw3d
Copy link
Member

@lukaw3d lukaw3d commented Jul 5, 2023

image

@lukaw3d lukaw3d requested a review from csillag July 5, 2023 14:48
@github-actions
Copy link

github-actions bot commented Jul 5, 2023

Deployed to Cloudflare Pages

Latest commit: aa3ed82ed21eb777d588aea252b760a300ca1d09
Status:✅ Deploy successful!
Preview URL: https://b61a4c6f.oasis-explorer.pages.dev

@csillag
Copy link
Contributor

csillag commented Jul 5, 2023

I like the general idea, but how can a random visitor start the verification process, when it needs to have the source code and the metadata manually inputted? At sourcify, there is an option called "import from contract", but it doesn't seem to work in the test case. (Also the address of the contract must be entered manually again, which is poor UI.) Would it be possible to provide a closer integration? At least avoid having to re-enter the address, but ideally also automatically provide the alleged sources? Or is this information not available anywhere?

Also, what happens is the verification fails? What does it mean? Does it say something about the contract itself, or does it just mean that the person who started the verification process provided the wrong data? Do we even get notified about failed verification attempts?

@lukaw3d
Copy link
Member Author

lukaw3d commented Jul 5, 2023

Sometimes a random visitor can do the verification, since our chains are not monitored; I verified a few contracts just using "Import from Contract" + input address and chain. Other than that, it's intended for contract owners.

I don't think we can avoid re-entering the address 🤷

but ideally also automatically provide the alleged sources? Or is this information not available anywhere?

not sure what you mean here

Verification fails

Pretty sure it just means the compilation result has a different hash, so the source code or the compilation parameters differed. And oasis is not notified about such mismatches.

@csillag
Copy link
Contributor

csillag commented Jul 5, 2023

I verified a few contracts just using "Import from Contract" + input address and chain.

Is there a way for us to determine in advance whether or not this will be possible, just by looking at our own data? Btw, how does Sourcify pull the data? What kind of service do they use?

I think we should show a different message (or at least tooltip) depending on whether or not this is possible for the given contract. Since if it's not, I think we should probably say something like "Verify this contract (assuming you have the sources)" or something like that...

I don't think we can avoid re-entering the address shrug

Can we talk to the Sourcify team and ask them to provide a route which accepts the address as part of the path? (And hopefully also the "import from contract" setting...)

@lukaw3d
Copy link
Member Author

lukaw3d commented Jul 5, 2023

I don't think we can pre-determine it https://docs.sourcify.dev/docs/monitoring/

I guess the longer label "Verify this contract (assuming you have the sources)" is also a substitute for explaining what verified contracts even mean.

Address in path

TODO: check https://github.com/ethereum/sourcify

@lukaw3d lukaw3d force-pushed the lw/verify-link branch 2 times, most recently from 91ebc93 to da4a9da Compare July 10, 2023 19:25
@lukaw3d
Copy link
Member Author

lukaw3d commented Jul 10, 2023

Don said

I don’t think it’s particularly inviting to have that text link, but I guess we could reword it to be less actionable; Verify through Sourcify - let’s go with that, please.

Copy link
Contributor

@csillag csillag left a comment

Choose a reason for hiding this comment

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

LGTM.

@lukaw3d lukaw3d enabled auto-merge July 11, 2023 10:47
@lukaw3d lukaw3d merged commit fd62e0b into master Jul 11, 2023
7 checks passed
@lukaw3d lukaw3d deleted the lw/verify-link branch July 11, 2023 10:50
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.

None yet

2 participants