search

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.06k stars 74 forks source link

Feature Request: Include `ContractResult` in the `AutoloadResult` and Add `loader_name` #115

Closed yohamta closed 1 month ago

yohamta commented 1 month ago

Hi @shazow,

I would like to propose the following enhancement to streamline the retrieval process:

Modify autoload Function to Return ContractResult and Include loader_name in AutoloadResult

Currently, to obtain all necessary contract information for our use case, we need to perform two separate steps:

  1. Retrieve the ABI using the autoload function.
  2. Obtain contract information through MultiABILoader (e.g., SourcifyABILoader, EtherscanABILoader).

Proposal

We are willing to submit a pull request to implement this feature if that would be acceptable.

shazow commented 1 month ago

Hi there, thanks for flagging this, I've been thinking about doing something similar too!

  1. My main concern is that the ContractResult response can be quite huge, so I didn't want to always use it by default. I was thinking of maybe adding a flag in AutoloadConfig whether to use that codepath or not, how does that sound?

Another hesitation I had is that it also includes the ABI, so if you're able to retrieve the ContractResult via MultiABILoader then you can skip autoload altogether, right? I'm not certain whether this is not ergonomic enough.

  1. Regarding loader_name, I actually just added isVerified to AutoloadResult (not released yet), what if instead I changed it to something like verifiedBy: undefined|ABILoader? [Edit: Actually maybe you're right, it would be more convenient to put it inside ContractResult]

  2. Could you tell me a bit about how you're using WhatsABi so that I have more context?

yohamta commented 1 month ago

Hi @shazow,

Thank you for the quick response and your thoughtful insights.

  1. Regarding your concern about the size of ContractResult, I completely agree. Adding a flag, like includeContract in AutoloadConfig, to control this behavior sounds like a good solution. This would give users the flexibility to opt-in based on their specific needs.

Another hesitation I had is that it also includes the ABI, so if you're able to retrieve the ContractResult via MultiABILoader then you can skip autoload altogether, right? I'm not certain whether this is not ergonomic enough.

In our case, we might not be able to skip autoload due to the need to account for proxies. If users have control over how to handle the proxy resolution with their own autoload like implementation, I believe they can make an informed decision based on their use case.

  1. As for the verifiedBy field, I think that would be an excellent addition. Including it in AutoloadResult would be particularly useful for our needs.

  2. To provide some context, we are developing a feature that retrieves ABIs from addresses to provide a contract interface. We have developed an internal service similar to Whatsabi, but only noticed this project afterward. Since Whatsabi is already open-source and nearly feature-complete, we are considering replacing our internal service with it and contributing to the project to fill any gaps.

shazow commented 1 month ago

Sounds good, let me play around with it and I'll ping you on a PR to review when I have something. :)

shazow commented 1 month ago

Opened a PR here, let me know what you think :)

https://github.com/shazow/whatsabi/pull/122

shazow commented 1 month ago

Merged #122, let me know if there's anything else missing before I do a release sometime next week.

yohamta commented 1 month ago

@shazow Thank you for the quick implementation. I've looked at the changes. I'll continue testing, and if anything stands out, I'll provide feedback :)