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 could also have a plain source mapping #2491

Merged

Conversation

wtdcode
Copy link
Contributor

@wtdcode wtdcode commented Jul 1, 2023

Motivation

Always finding funny things when playing with Etherscan API, try:

curl "https://api.etherscan.io/api?module=contract&action=getsourcecode&address=0x68b26dcf21180d2a8de5a303f8cc5b14c8d99c4c"

This gives a plain source code mapping in the SourceCode field instead of plain source code.

Solution

As shown.

@DaniPopes
Copy link
Collaborator

language and settings in SourceCodeMetadata::Metadata are tagged #[serde(default)], so they would be set to None if they are not present. Does that contract not parse already?

@wtdcode
Copy link
Contributor Author

wtdcode commented Jul 1, 2023

language and settings in SourceCodeMetadata::Metadata are tagged #[serde(default)], so they would be set to None if they are not present. Does that contract not parse already?

If you try with the link above, you will find that it's missing the key fields, i.e. just plain source code mapping like:

{ "a.sol" : "..." }

EDIT: Oh wait, you are correct, I made a mistake here. Will update the code.

Reference: 0x68b26dcf21180d2a8de5a303f8cc5b14c8d99c4c
@DaniPopes
Copy link
Collaborator

DaniPopes commented Jul 1, 2023

Ah alright, that makes sense.
Shouldn't it be just Sources(HashMap<String, SourceCodeEntry>) then? Updated right as I posted.
Can you also add a test for this case with that contract?

@wtdcode wtdcode force-pushed the etherscan-may-have-sources-mapping branch from 8fc7368 to 053aa61 Compare July 1, 2023 15:19
@wtdcode
Copy link
Contributor Author

wtdcode commented Jul 1, 2023

Ah alright, that makes sense. Shouldn't it be just Sources(HashMap<String, SourceCodeEntry>,) then? Can you also add a test for this case with that contract?

Fixed already. Regarding the test case, I didn't see any existing test cases actually requesting the network (even no existing test cases for SourceCodeMetadata). Is it proper to make unit tests rely on network service?

@DaniPopes
Copy link
Collaborator

You can add one at the bottom of ethers-etherscan/tests/it/contract.rs

@wtdcode
Copy link
Contributor Author

wtdcode commented Jul 1, 2023

You can add one at the bottom of ethers-etherscan/tests/it/contract.rs

The problem is not about the place for tests, but how to test it? Request etherscan?

@DaniPopes
Copy link
Collaborator

Yes, see can_fetch_contract_source_tree_for_multi_entry_contract in that file, you can copy it and replace the assertions

@wtdcode
Copy link
Contributor Author

wtdcode commented Jul 1, 2023

Done

Copy link
Collaborator

@DaniPopes DaniPopes 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 spotting this! I've fixed the tests in #2492.

@DaniPopes DaniPopes merged commit 3d6af3f into gakonst:master Jul 1, 2023
17 of 19 checks passed
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.

2 participants