-
-
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
proxies: Multiple instances of the same resolver #142
Comments
@manuelwedler Thank you for reporting these! It has been hard to find good edge cases to test against. Definitely need to fix this. :) |
Do we have a distinguishing definition for what constitutes as a library from an EVM bytecode perspective? I think traditionally we consider contracts called with
This one is easy to fix, on it!
Hmm this one is tricky too, not sure we can reliably distinguish this. The other perspective I'm considering: Is there any additional information I can include with the resolver results that would allow you to productively decide how to treat the detected resolvers? |
Yeah, libraries and factories are really difficult to detect. At least it's good to be aware of these issues now :) I think you could potentially differentiate a proxy from a library by detecting For our purposes, I will discuss with the team how we go about it. We should actually be able to tell if a detected proxy is a library, since we have the source code. We would only need to check against our DB. |
That's still tricky since it could be in the stack, and unfortunately WhatsABI doesn't have full stack tracing yet. (This is on the table but would substantially change how WhatsABI works, so I'm kinda punting it to post-EOF at the moment.) I think it would be a matter of how conservative we want the results to be. In the case of sourcify, I imagine it's better to present a subset of proxies rather than a superset? Off the top of my head, some available information that you can influence how you use the proxies result:
|
These are some good hints. Thanks. A subset is definitely better than a superset. I think regarding FixedProxies the easiest for us is to only support EIP-1167 proxies. We can just check if the bytecode matches the opcodes of the EIP-1167 standard. Other detected FixedProxies would be non-standard proxies in my understanding and it's totally fine to not support such at the moment. I still wonder what we can do about factories. If I understand it correctly, any type of detected proxy could be a factory. I guess a factory would usually resolve to |
Trying to imagine a more general case: You could treat any contract that has CREATE/CREATE2 opcodes as factories? I don't know how reliably the resolved address would be |
I think yes, if there is any CREATE opcode, we shouldn't consider a contract as a proxy. However, it is still possible that you embed a contract's bytecode but only pass this bytecode to another contract which would call a CREATE opcode with the passed bytecode. So the contract that embeds the bytecode would not have a CREATE opcode itself.
For EIP-1967, it should not be impossible to write to the storage slot by accident in Solidity. You would have to so on purpose in assembly code, and that could be a problem. |
Or we could contrive a contract that is both a valid proxy and also acts a factory in some conditions. Plenty of proxies that also have non-proxied selectors that it checks first. I might need to figure out a sensible way to reveal CREATE opcodes with AutoloadResult, or have a different call flow altogether... Hmm. |
One more thing I'm thinking of adding is a
I'll also see if it's feasible to detect more specific types of FixedProxy types that can be distinguished from weird usage of DELEGATECALL. |
Revealing
My data (here) actually showed that only contracts with various combinations of So not sure if this priorization is perfect. Maybe it is worth to look more deeply into why there are
That would also be great :) |
@manuelwedler Are any of these are blockers for sourcify at this time? Trying to figure out my priorities for the next sprint. :) |
None of these are blockers, but I still need to figure out how we should handle embedded bytecodes. The only thing that might be useful for this are the |
Adding |
@manuelwedler Just released v0.16.0 which has some of these improvements. :) https://github.com/shazow/whatsabi/releases |
I noticed a few more issues while running the proxy detection on the Sourcify database. Let's have a look at two example contracts on Ethereum mainnet:
0x56C5Aef1296d004707475c8440f540DdA409b53D
https://eth.blockscout.com/address/0x56C5Aef1296d004707475c8440f540DdA409b53D?tab=contract
For this contract I got 14 instances of the
FixedProxyResolver
. They resolved only to 4 distinct addresses:0xdd0694ed7ecb6494a9055a78cf848a6b146c88b5
0x652c78d85cdf25e85fe7df8d4c61b2a0f3c25d4f
0x2184298d600b5f2ff2febca0993626581a994095
0xcc985298cc007901c9994920fecedadbeba8380f
All of the 4 above addresses are libraries.
0x7dB8637A5fd20BbDab1176BdF49C943A96F2E9c6
https://eth.blockscout.com/address/0x7dB8637A5fd20BbDab1176BdF49C943A96F2E9c6?tab=contract
For this contract I got 1
FixedProxyResolver
and 5EIP1967ProxyResolver
instances. TheFixedProxyResolver
resolved to0xf208a8fe1ef2b31b0e408c7399a5d696cadb0be9
which is a library again. AllEIP1967ProxyResolver
instances resolved to0x0000000000000000000000000000000000000000
, but the above address is actually no proxy. It is only able to deploy new EIP1967 proxies.Problems
There are multiple problems here. I would expect the following behavior:
If you want to see more examples, you can have a look here: https://github.com/sourcifyeth/data-analysis-scripts/blob/main/multi-proxy-analysis-results/multi-implementation-addresses.json
There, I collected all contracts that resolved to multiple implementation addresses (except DiamondProxies). The format is
{chainId: [{proxyAddress: [[detectedProxies], ...distinctImplementationAddresses]}]}
.The text was updated successfully, but these errors were encountered: