Idea: Bubble up invalid ABI type errors on readContract and others alike.
#201
Unanswered
Raiden1411
asked this question in
Idea / Feature Request
Replies: 1 comment 2 replies
-
|
@jxom and @tmm, any thoughts regarding this? I can work on a PR if you think that makes sense. |
Beta Was this translation helpful? Give feedback.
2 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Problem
Recently on the ABIType repository, there was an issue where an invalid ABI was being used and the
readContracthere was not inferring thefunctionNamecorrectly, but we found out the problem was a missing required property on the provided ABI. But the problem here is that the ABI was extracted from a third-party website like etherscan, and currently there is no way to let developers know that they are using invalid ABIs.So, with this in mind, I currently see two solutions to this problem.
Solution 1
The easiest solution here is to just "throw" an invalid ABI error if the
functionNamecannot be inferred from the ABI.But this poses a problem... What about ABIs that folks use from JSON files or simply don't
constassert?With solution 1, those ABIs would also fail, and that is not good. Since they could indeed be using valid ABIs and would essentially be forcing folks to
constassert their ABIs.So what can we do to mitigate this?
Solution 2
Here we check for all ABIs that are either
constasserted or simply aren't and check if any of them have errors in the case that they don't evaluate to valid ABIs. This would work on declared ABIs, zodParsed, and ABIs that are imported via JSON files.I will go into more details below, but to follow along, assume that these ABIs can all be the following:
constAsserted,jsonAbi,zodAbi, andnotConstAsserted.Invalid ABI
[ { "name": "foo", "type": "error", "inputs": [] }, { "name": "foo", "stateMutability": "view", "inputs": [], "outputs": [{ "type": "string", "name": "" }] }, { "name": "bar", "type": "function", "stateMutability": "view", "inputs": [{ "type": "address", "name": "" }], "outputs": [{ "type": "address", "name": "" }] }, { "name": "foo", "type": "function", "inputs": [], "outputs": [{ "type": "string", "name": "" }], } ]Valid ABI
[ { "name": "foo", "type": "error", "inputs": [] }, { "name": "foo", "type": "function", "stateMutability": "view", "inputs": [], "outputs": [{ "type": "string", "name": "" }] }, { "name": "bar", "type": "function", "stateMutability": "view", "inputs": [{ "type": "address", "name": "" }], "outputs": [{ "type": "address", "name": "" }] } ]For solution two to work, we need a way to validate that the ABIs that aren't
constasserted are also valid, and to do so, we will need some helper types, and luckily, ABIType will get us there pretty quickly.Using the already-defined types for
constasserted ABIs, we can pick the properties that we want and create the rest. With this setup, we can quickly get to the required ABI.Now that we have all of these types helpers, we need a way to validate to check if the abi that was passed on to the function is valid, and if it's not valid, we need to check what failed.
Using the
GetFunctionNametype above, we can extend it to check forNotConstAssertedAbiwith something like this:So what can we do to check which member of the ABI is invalid? Using something like the
isSignatureandSignaturestypes from ABIType, we can achieve that. This will allow us to pinpoint exactly where the invalid ABI members are on theconstasserted ABIs, and we can display that to the developer.Using the
ValidAbitype helper, we will get the ABI with the error messages, and we can extract them and show them.Using the invalid ABI above as
const, here is an example of how it would show up.It correctly grabs the spec with missing properties. In this example, position 1 is missing the
typeproperty, and position 3 is missing thestateMutabilityproperty.What about the
NotConstAsserted?Well, in cases of error, we won't have the same level of details, but we can also give a general idea of what the problem is with this type helper.
And then we can check if there are any error messages from the previous
ValidAbitype, and in case there aren't, we use this type to grab the invalid ABI spec. Where is what the finalGetFunctionNametype would look like:And here is an example of the error in
NotConstAssertedAbisAs you can see, you won't have the same level of details, and it will only catch one invalid member at a time. Here the problem is that the
typeproperty is marked asundefined. This error checking will also work for imported JSON ABIs.Final thoughts
With Solution 2, we would increase the DX by having developers know exactly what to expect when using ABIs in these functions.
constasserted ABIs, the typescript LSP will correctly display all the possible valid names.constasserted but are valid. The typescript LSP won't suggest any names, but it won't show any error messages either.If the typescript LSP shows you any "error" messages, then you will know that the ABI is invalid and will have a general idea of what the problem is, and in the case of
constasserted ABIs, you will know exactly where the problem lies.And finally, if the LSP suggests
neveras a type, you know something bad has happened.All the code shown here, in this example, can be found here. I recommend copying and opening it in your editor of choice, in case you want to try it out, since there are some problems with this codesandbox as its not getting all the correct types from ABIType.
TL:DR
Check for invalid ABIs at the type level. Good idea? Bad idea?
Looking forward to hearing your thoughts! Keep in mind that I'm not proposing to use the types as they are, as they were created for the purpose of this discussion.
Beta Was this translation helpful? Give feedback.
All reactions