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

SourcifyABILoader won't even attempt a partial match query #87

Closed 0xpolarzero closed 8 months ago

0xpolarzero commented 8 months ago

I'm not sure if this is intended behavior; basically, the SourcifyABILoader will first attempt a full_match query, then a partial_match query if the first one doesn't throw an error.

However, taking this contract on OP Mainnet for instance: 0x4200000000000000000000000000000000000042

With autoload, the full_match query will return a 404 error, which will have isSourcifyNotFound(error) return true.

This is a shame; since this will throw the error, it won't be able to attempt the partial match query, which would have actually succeeded, thus skipping straight to the selector lookup.

Is it the intended behavior? Because we end up with unnamed parameters, provided even that it could find the function selector in the database.

Maybe it could first attempt both the full_match and partial_match queries, and then throw an error only if both returned a 404 status?

shazow commented 8 months ago

@0xpolarzero Hmm this is not intended behaviour, thanks for flagging it. Definitely open to a PR if you'd like to try to fix it. :)

I'm trying to understand what's happening, so I'm going to rephrase what you explained to make sure we're on the same page:

  1. https://repo.sourcify.dev/contracts/full_match/10/0x4200000000000000000000000000000000000042/metadata.json gets hit first, which returns 404
  2. This try/catch gets hit: https://github.com/shazow/whatsabi/blob/main/src/loaders.ts#L140-L142 if (!isSourcifyNotFound(error)) throw error;
  3. Since it's a 404, it should not throw (note the negating ! in front), and fall through to the partial block, no?

Is isSourcifyNotFound incorrect? Or am I confusing some other call flow condition?

0xpolarzero commented 8 months ago

@shazow Oh, I completely overlooked the ! in front, sorry about that... It should definitely get to the second block then. Thanks for making it clearer.

Please let me play with it a bit to try to exhibit the issue. I'll gladly submit a PR if I can find it!

shazow commented 8 months ago

@0xpolarzero Awesome, thank you!

0xpolarzero commented 8 months ago

@shazow Ok, that was due to a silly mistake/misunderstanding from my end, truly sorry for wasting your time...

The core issue was making the request from my Next.js app on the client side; the 404 status of the issue was due to the usual CORS header ‘Access-Control-Allow-Origin’ missing. I was too quick to attribute it to WhatsABI.

So basically, the request needs to be done on the server side—e.g. via an API route. Which unfortunately makes the use of the callbacks (onProgress, onError) not possible. But that's purely a React/Next/client components problem.

I'm not sure about that, but from my understanding, these are the few solutions (for documentation/in case anyone else faces this):

shazow commented 8 months ago

@0xpolarzero No worries at all, thank you for diving in and participating/contributing. :)

I'm open to improving the ergonomics if you have ideas in the future too.

Please share what you end up building with WhatsABI, would love to see it and showcase it. :)

0xpolarzero commented 8 months ago

@shazow I'll let you know if there is anything I believe might help/improve.

And yes absolutely, I was actually planning on sharing it to you very soon. It's a Tevm example, my PR is currently being reviewed. And I'm integrating it inside a personal project as well.

I'll send you the link with a TLDR + a Vercel preview once it's ready!

shazow commented 8 months ago

Very cool! @roninjin10 will be so hyped