shazow / whatsabi

Extract the ABI (and resolve proxies, and get other metadata) from Ethereum bytecode, even without source code.
https://shazow.github.io/whatsabi/
MIT License
1.04k stars 71 forks source link

Proposal: Include `ContractResult` in the `AutoloadResult` #130

Open yohamta opened 1 day ago

yohamta commented 1 day ago

Hi @shazow, I raised a similar issue last week (#115), but I'd like to revisit this proposal with further clarification.

In the current implementation, the focus of autoload is on retrieving the ABI. However, when ContractResult is also needed, the user is required to call both autoload and ABILoader.getContract separately, which introduces unnecessary complexity to the users's code.

If ContractResult could be retrieved within autoload, the user would be able to obtain both the ABI and detailed contract information (contract name, compiler version, userdoc, devdoc, etc) seamlessly, without needing to make additional calls. Furthermore, including the bytecode would offer even greater convenience.


Proposal 1: Add ContractResult to AutoloadResult

Proposal 2: Add bytecode to AutoloadResult

If this is acceptable, I would be more than happy to submit a PR. What do you think?

shazow commented 16 hours ago

Thank you for the reminder! This is a little complex because there are substantial tradeoffs between different kinds of users, so let me play with it.

My first inclination is I'd like to add something like Proposal 1.

My one hesitation is that autoload is intended to satisfy 80% of users with minimal effort, not 100%. But, it should still be fairly convenient for the remaining 20% to do what they need to do. So another option is to have a separate helper(s) that is parallel to autoload which handles your more specific cases. :)

Could you explain how you're going to be using AutoloadResult.bytecode?

yohamta commented 16 hours ago

Thank you for your reply; it's very helpful. A separate helper would be acceptable. However, in a naïve implementation, requests to Etherscan and Sourcify might be duplicated, leading to unnecessary API calls. I'm concerned that avoiding these redundant requests could make the user's implementation a bit more complex.

Another idea is to modify the autoload function to accept an argument of AutoloadConfig | bytecode. If bytecode is provided, it could perform bytecode analysis and proxy detection only. What do you think?

One of the reasons we need the bytecode is to deploy the contract to other networks, such as testnets or other chains.

shazow commented 16 hours ago

Ah, I see. So I just added a helper which allows you to wrap a provider that has a cached bytecode mapping, for a similar reason!

I originally needed something similar: If I already had bytecode, I wanted to skip loading it from getCode. I experimented with adding extra fields to supply bytecode, but it just ended up looking more messy than fixing the bytecode in the provider instead.

What do you think of this?

const address = "0x...";
const bytecode = "0x..."; // Already loaded from somewhere

const cachedCodeProvider = whatsabi.providers.WithCachedCode(provider, {
  [address]: bytecode,
});

const result = whatsabi.autoload(cachedCodeProvider, address, {
  abiLoader: false, // Skip ABI loaders
  signatureLookup: false, // Skip looking up selector signatures
  followProxies: true,
}));

PR is here: https://github.com/shazow/whatsabi/pull/120

yohamta commented 15 hours ago

Thank you very much! Avoiding redundancy in fetching bytecode is indeed a great improvement. Do you think a similar approach could be applied to requests to Etherscan and Sourcify to prevent duplication there as well?

From my perspective, it might be helpful to make the autoload functionality more modular. If bytecode analysis and proxy detection could be executed separately, users could implement custom solutions without relying entirely on autoload, depending on their specific needs. What are your thoughts on this?

shazow commented 15 hours ago

Do you think a similar approach could be applied to requests to Etherscan and Sourcify to prevent duplication there as well?

What do you mean? Could do something like a CachedABILoader which wraps a regular loader. Would be fairly easy to do outside of whatsabi too, but I'd be open to including a helper?

From my perspective, it might be helpful to make the autoload functionality more modular. If bytecode analysis and proxy detection could be executed separately, users could implement custom solutions without relying entirely on autoload, depending on their specific needs. What are your thoughts on this?

That was my initial intuition as well, and I started working on whatsabi.resolveProxy(provider, address|bytecode, resolveProxyConfig) but by the time I got something working, it ended up looking a lot like whatsabi.autoload :P Worth keeping in mind that a provider will be required either way, because resolving proxies requires a provider to do calls and getStorageAt.

It's basically all the exact same code, except with { abiLoader: false, signatureLookup: false } branching.

But we could still pull out the same code into internal helpers, and call the same codepaths from both.

I could be convinced because it might be more obvious/self-documenting?

I guess breaking down the scenarios we're considering here:

  1. Overload everything into whatsabi.autoload via complex AutoloadConfig flags. Seems to be working so far, but might be painful in the long term.
  2. Factor out common code, and introduce a parallel whatsabi.resolveProxy that does the same thing as autoload but without abi/signature loading.
  3. Add support for only proxy detection without resolving? This could be done with just bytecode I think: const proxies = whatsabi.proxies.detect(bytecode) (would we also want a helper for resolving? it's just a loop, or 99% of the time take the first result and call resolve on it iff it's not a DiamondProxy).

Maybe we need to think more about how this is being used. Could you expand more on your end?

For example, Sourcify was going to use WhatsABI just for proxy resolving on their API, so they can return verified results for proxies in addition to implementations. I think they don't mind using autoload for that, but detect + loop might be a worthwhile substitute for them too?

Basically this code:

https://github.com/shazow/whatsabi/blob/1055953d55185ed4912dd14afeb8b455f776b1a0/src/auto.ts#L172-L192