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

Etherscan metadata variations #976

Merged
merged 24 commits into from
Apr 20, 2023
Merged

Conversation

kuzdogan
Copy link
Member

@kuzdogan kuzdogan commented Apr 5, 2023

View in Huly HI-713

experiment to create files varations before running verification searching for perfect matches
…bytecode WIP

*  add tryToFindOriginalMetadata to CheckedContract: tries to recontruct the original metadata by iterating on all files variations
* call tryToFindOriginalMetadata after `matchWithDeployedBytecode` if match is not perfect
@codecov-commenter
Copy link

codecov-commenter commented Apr 5, 2023

Codecov Report

Patch coverage: 72.72% and project coverage change: -4.24 ⚠️

Comparison is base (1ef0e73) 77.56% compared to head (ad406a8) 73.33%.

Additional details and impacted files
@@             Coverage Diff             @@
##           staging     #976      +/-   ##
===========================================
- Coverage    77.56%   73.33%   -4.24%     
===========================================
  Files           29       38       +9     
  Lines         1373     2171     +798     
  Branches       254      416     +162     
===========================================
+ Hits          1065     1592     +527     
- Misses         180      356     +176     
- Partials       128      223      +95     
Flag Coverage Δ
lib-sourcify 66.11% <ø> (?)
server 77.46% <72.72%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../server/controllers/VerificationController-util.ts 83.82% <57.14%> (-0.96%) ⬇️
src/server/controllers/VerificationController.ts 75.09% <100.00%> (+0.19%) ⬆️

... and 9 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@marcocastignoli
Copy link
Member

@kuzdogan PR is not ready but if you want to check what I did till now and give me some feedback, it's appreciated.

@marcocastignoli marcocastignoli marked this pull request as ready for review April 5, 2023 14:25
packages/lib-sourcify/package.json Outdated Show resolved Hide resolved
packages/lib-sourcify/src/lib/CheckedContract.ts Outdated Show resolved Hide resolved
packages/lib-sourcify/src/lib/CheckedContract.ts Outdated Show resolved Hide resolved
…hash by updating the source section of the metadata and generating the metadata CID
* I translated the ipfsHash.cpp file from the solidity repo
* now that everything is synchronous I simplified the code that replace the ipfs and bzz hash in the urls array in metadata.sources
* I added some comments to clearify the storebyhash grouped by variation
@marcocastignoli
Copy link
Member

marcocastignoli commented Apr 11, 2023

TODO

  • Add tests
  • handle bzz encoded metadata

@marcocastignoli
Copy link
Member

marcocastignoli commented Apr 11, 2023

Tests are failing because https://ipfs.io is down

EDIT: it's not down, but it doesn't support anymore url with double slashes

https://ipfs.io//ipfs/QmT82ab2aGqBc5owXEqmiE1NYfYEdWCKpc4V11ncummXit

instead of

https://ipfs.io/ipfs/QmT82ab2aGqBc5owXEqmiE1NYfYEdWCKpc4V11ncummXit

@marcocastignoli
Copy link
Member

@kuzdogan now I think we are ready for merge, wanna review again?

@kuzdogan
Copy link
Member Author

kuzdogan commented Apr 12, 2023

Added some comments. Specifically how we return the match at verification.ts needs to change.

Don't we have verification test cases? Like the one that is our starting point from Etherscan?

@marcocastignoli
Copy link
Member

marcocastignoli commented Apr 12, 2023

Don't we have verification test cases? Like the one that is our starting point from Etherscan?

what do you mean?

@kuzdogan
Copy link
Member Author

Don't we have verification test cases? Like the one that is our starting point from Etherscan?

what do you mean?

I see the function is just unit tested but the reason we develop this at the first place is to be able to verify contracts perfectly when they have common and known changes in the metadata that we can try to fix. I was referring to the contracts in this issue #936 We should test the verification on those, as well as find and add other cases that we know.

@marcocastignoli
Copy link
Member

The contract in the test is actually one of those, the one from this comment: #936 (comment)

@kuzdogan
Copy link
Member Author

The contract in the test is actually one of those, the one from this comment: #936 (comment)

Good, let's use it in the verification tests then. With that we will have tested the whole workflow and not just that part

* refactor `verifyDeployed` so that it uses the new function ´tryToFindOriginalMetadataAndMatch´
@kuzdogan kuzdogan merged commit 486732c into staging Apr 20, 2023
@kuzdogan kuzdogan deleted the etherscan-metadata-variations branch May 10, 2023 14:31
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.

3 participants