-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: add Sourcify support to TraceIdentifiers #11817
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you, makes sense! left couple of nits, can you add a test too?
Fixed and test now. |
thank you, have to get back on this, seems that the changes hangs CI (probably waiting on sourcify response) maybe you can run external tests locally and see if you figure it out |
The CI hang was because tests were making real HTTP calls to Sourcify. Should be good now. |
CI should make real HTTP calls to sourcify when running external tests (they clone known projects and run forge tests on them) |
Thanks for the clarification! Simplified the approach - removed the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes that use APIv2 LGTM
Just a comment if it should only work for Ethereum Mainnet.
Taken over in #11917, taken a different approach and will take a bit more to get through since of various requirements, and this being turned on by default. Thanks for the PR anwyay |
close #11809
a new SourceifyIdentifier that fetches contract ABIs from Sourcify's public API without requiring an API key. integrates into the existing TraceIdentifiers system, checking Sourcify after local contracts but before Etherscan.
use it by calling
.with_sourcify()
when building TraceIdentifiers.