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

autodetection of constructor arguments #2821

Merged
merged 15 commits into from
Nov 22, 2019

Conversation

ayrat555
Copy link
Contributor

@ayrat555 ayrat555 commented Oct 30, 2019

resolves #2338

Changlelog:

  • add autodetection of constructor arguments

@coveralls
Copy link

coveralls commented Oct 30, 2019

Pull Request Test Coverage Report for Build 8f73d800-ee1d-43bf-bb9c-1b90dc01e448

  • 27 of 33 (81.82%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.05%) to 75.096%

Changes Missing Coverage Covered Lines Changed/Added Lines %
apps/explorer/lib/explorer/smart_contract/verifier/constructor_arguments.ex 20 26 76.92%
Totals Coverage Status
Change from base Build adfacdd1-46cb-40e3-8b82-22a1ac534ec5: 0.05%
Covered Lines: 5259
Relevant Lines: 7003

💛 - Coveralls

@ayrat555 ayrat555 force-pushed the ab-autoappending-constructor-arguments branch from 9bca115 to ae8e162 Compare October 30, 2019 13:58
@ayrat555 ayrat555 changed the title [WIP] autoappending of constructor arguments autodetection of constructor arguments Oct 31, 2019
@ayrat555 ayrat555 requested review from vbaranov and pasqu4le October 31, 2019 12:09
Copy link
Member

@vbaranov vbaranov left a comment

Choose a reason for hiding this comment

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

@ayrat555

I thought that Blockscout will show constructor arguments if it is chosen to retrieve them automatically. Anyway, UX suffers a little bit, imho.

Request for changes:

  1. If a user chooses to fetch contructor arguments automatically, the field ABI-encoded Constructor Arguments ... should be disabled
  2. otherwise, it should be enabled
    3.a. the best option would be to fetch and show constructor arguments immediately on enabling of automatic fetching
    3.b. if you do not choose to retrieve them immediately for some reasons, I'd suggest to completely hide/show the field ABI-encoded Constructor Arguments ... when a user enables/disables auto fetching

@ayrat555
Copy link
Contributor Author

ayrat555 commented Nov 7, 2019

@vbaranov I added hiding of constructor arguments field

@vbaranov vbaranov self-requested a review November 14, 2019 15:56
Copy link
Member

@vbaranov vbaranov left a comment

Choose a reason for hiding this comment

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

@ayrat555 I tried to use auto-appending of arguments. It works incorrectly.

I deployed smart-contract to Sokol with on uint constructor argument /address/0x3ee96c3a3a570de90602384d0d18e5cac06bf791/contracts verified it locally with auto-appending flag. The input value for a single constructor argument was 2, the contract was verified, but this is what I see in the Constructor arguments field of verified contract page. Obviously, the value is wrong:

Screenshot 2019-11-14 at 18 58 52

@ayrat555 ayrat555 force-pushed the ab-autoappending-constructor-arguments branch from dc5252b to 3bdcf4d Compare November 19, 2019 07:58
@vbaranov vbaranov self-requested a review November 22, 2019 14:39
@vbaranov vbaranov merged commit 6caefe3 into master Nov 22, 2019
@vbaranov vbaranov deleted the ab-autoappending-constructor-arguments branch November 22, 2019 15:05
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.

Auto-appending of constructor arguments on verification page
3 participants