-
Notifications
You must be signed in to change notification settings - Fork 77
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
Fix open zeppelin proxy detection #155
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Nice catch! Thanks for tackling it so fast.
Now it detects the proxy nicely for USDC in mainnet, but I got a question there.
In etherscan read proxy you can read the balance of an account nicely (0x4bBB30C26AEB7ABDBFb937D5C206CC29f9706323) but not in abi ninja
It seems like if we are querying directly the implementation address, like etherscan does here
Also if you open USDC mainnet in abi ninja and select a method, the URL reloads to the implementation address
@Pabl0cks thanks for the feedback! Merging in https://github.com/BuidlGuidl/abi.ninja/tree/feat/getsourcecode fixes that problem as well as improving the overall app. Here is a deployment link for that branch if you'd like to test: My suggestion here would be, since the problem exists on prod deployment as well, is either: How do those options sound? Happy to hear any ideas! |
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.
I think the changes makes sense, btw @portdeveloper do you remember why we commented that OZ slot? I don't remember any discussion on that :/.
Nevertheless I think we can merge this just added a small comment and create a new PR for getSourceCode
branch
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.
Ok! I also agree is a good idea to merge this and then test nicely the getsourcecode
PR and merge it when looks good.
I will document the issue just to make sure we verify it in the new PR 🙏
I don't remember the exact reason why we commented it out (might be related to when we separated the logic) |
Description
This PR adds support for detecting OpenZeppelin proxies by uncommenting and implementing the OpenZeppelin implementation slot detection logic in the
detectProxyTarget
function.Changes made:
OPEN_ZEPPELIN_IMPLEMENTATION_SLOT
constant.detectUsingOpenZeppelinSlot
function to check for OpenZeppelin proxies.Additional Information
Related Issues
Fixes #154
Your ENS/address: portdev.eth