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

Add API-level proxy detection and implementation resolution #1711

Open
wants to merge 8 commits into
base: staging
Choose a base branch
from

Conversation

manuelwedler
Copy link
Collaborator

@manuelwedler manuelwedler commented Oct 22, 2024

Closes #1655

This implements an API-level proxy detection as in #1655 by making use of the whatsabi library. A few explanations about this PR:

  • I had to add two methods to the SourcifyChain class for making it compatible to the whatsabi library as a provider, and for being able to resolve the implementation addresses of DiamondProxy contracts.
  • I added a proxyResolutionError to the API response for the case that the proxy resolution fails. This could be due to an unresponsive RPC for example. In such a case, I still want to respond successfully to the user but let them know why the proxy fields are not there.
  • Related to proxies: Multiple instances of the same resolver shazow/whatsabi#142:
    • A FixedProxyResolver was returned in a lot of cases where actually a library was called by a DELEGATECALL. I decided to only support EIP1167 proxies for this resolver. Any other FixedProxy would be a non-standardised proxy (or even a library). So in case a FixedProxyResolver is detected, we check explicitly for the opcodes standardised in EIP1167.
    • I saw a few cases where multiple resolvers were returned, and also some cases where a resolver resolves to 0x0. The latter was for example the case for a factory contract which deploys EIP1967 contracts. Therefore, I iterate over the returned resolvers and return the first implementation address which is not 0x0.
  • This feature is only added to the /check-all-by-addresses endpoint and not the check-by-addresses endpoint, because in the latter the chainIds array does not contain objects.

Remaining issue (non-blocking)

There is still a problem left from shazow/whatsabi#142:
A contract that embeds a proxy (as for example a factory contract) could falsely be detected as a proxy.

A first proposal to solve this is to not consider contracts with CREATE/CREATE2 opcodes as proxies. However, this will not catch all contracts that only embed proxy bytecodes, and also exclude factory contracts that could be at the same time a proxy.

Edit: We decided that this problem is not a blocker for this PR. IMO, it is good to get this out and test it. The proxy data won't be persisted in our database with this change, so before we persist it we might be able to improve the proxy detection.

@manuelwedler manuelwedler changed the title API-level proxy detection and implementation resolution Add API-level proxy detection and implementation resolution Oct 22, 2024
@manuelwedler manuelwedler marked this pull request as ready for review October 24, 2024 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Triage
Development

Successfully merging this pull request may close these issues.

API-level proxy detection and implementation resolution
1 participant