Skip to content
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

Improve error messages on mismatched argument types #964

Open
carver opened this issue Jul 19, 2018 · 11 comments · May be fixed by #3491
Open

Improve error messages on mismatched argument types #964

carver opened this issue Jul 19, 2018 · 11 comments · May be fixed by #3491

Comments

@carver
Copy link
Collaborator

carver commented Jul 19, 2018

What was wrong?

When sending an argument with the wrong type to a contract function, people find it difficult to debug and end up frustrated, like in #625

How can it be fixed?

We largely solve for the general case: that multiple functions can exist with the same name. Obviously we have to support that, but it's incredibly common to have names that are unique even without the argument types. In these cases, we can give clearer error messages about exactly which types are expected for which arguments.

For example, we could raise the function:

ValidationError: a function with name testFunction is defined by the contract, but some argument types don't match:

  • address _location ✔️
  • uint256 _reading ✔️
  • uint256 _minParameter ✔️
  • uint256 _maxParameter ✔️
  • bytes32 _codeHash ⚠️ This argument was supplied as a string that was too long. If you want to supply a hex string, this function only accepts values that are exactly 64 characters (plus 2 for 0x). Alternatively, supply this argument as python native bytes, with a length of 32 bytes. Actually supplied argument: "0xFFFFF..." (length: 67)
@MatthiasLohr
Copy link
Contributor

Is there any official documentation, about how to provide the values for several types?

It could help to add some documentation in the Web3 libraries (JS, Python, ...) to provide examples on how to encode specific Ethereum types in these languages/libraries.

@kclowes
Copy link
Collaborator

kclowes commented Aug 26, 2019

There is some documentation here about different types, but it's not very comprehensive, and could certainly stand to be improved. What types would you like to see documentation on @MatthiasLohr?

@MatthiasLohr
Copy link
Contributor

Oh, that would have been good to have had this link yesterday. ;) So first: It seems to be somehow hard to find. I have seen this point "Conventions", but didn't take a deep look into it because it somehow felt far away from my problem.

Yesterday I tried to call a contract with web3py with error (something like) "found (str, bytes[]), need (address, bytes32). For me, at first place, it looked like a a working mapping, because the str object was a (checksum) address hex string and bytes[] was 32 bytes long (actually, it was not, that was the actual problem).

  1. I really would have liked to have had a hint which of these arguments cannot be mapped.
  2. A "mapping" table (EVM type - Python type) like the following would be great (in my opinion):
  • address: str with checksum address, create with Web3.toChecksumAddress
  • bytes32: bytes[] array with exactly 32 bytes.
  • intX/uintX: int
  • ...

If there are multiple possibilities for the mapping, all variants should be listed. I'm not that into Solidity/Web3Py so that I don't know all mappings. But maybe my idea is clear and helps to improve the documentation.

@kclowes
Copy link
Collaborator

kclowes commented Aug 27, 2019

Oh, that would have been good to have had this link yesterday

Ah, dang! That's the worst.

It seems to be somehow hard to find. I have seen this point "Conventions", but didn't take a deep look into it because it somehow felt far away from my problem.

Yep, agreed. Maybe a rename to "Type Conventions" or even "Special Type Conventions" would be clearer? Open to other options though if you think of something better.

A "mapping" table (EVM type - Python type) like the following would be great (in my opinion):

I like this idea! I'm doing something similar in #1419 here for bytes types, but probably addresses and ints also deserve to have their own sections.

And the error message can definitely stand to be improved as well. Thanks for the comments @MatthiasLohr!

@MatthiasLohr
Copy link
Contributor

Why not just "Types"? Conventions can be so much more (Coding Conventions, if you want to start to contribute, Naming Conventions for internal variables, ...). Just "Types" reflects, what the page currently contains.

@carver
Copy link
Collaborator Author

carver commented Aug 28, 2019

Perhaps "ABI Types" (or "ABI Type Conversion")? I think it's specific and comprehensive enough to say we are talking about how to convert python native types to the ABI-encoded types, and back.

@MatthiasLohr
Copy link
Contributor

Sounds good.

Would also be good to have a link in Smart Contract calling examples (how to convert your Python datatypes to Solidity types, see (e.g.) "ABI Type Conversion").

@MatthiasLohr
Copy link
Contributor

Anything new on this? Any chance on getting some more verbose on information what actually went wrong on type conversion when executing code?

@pipermerriam
Copy link
Member

We'd be happy to review and provide feedback on a pull request if you'd like to take a stab at it.

@MatthiasLohr
Copy link
Contributor

The problem is: If I would know how to debug/find out what exactly the problem is, I wouldn't have to write here and ask for more details ;)

@pipermerriam
Copy link
Member

Here are the two entry points where I expect this functionality would be easiest to add:

def find_matching_event_abi(
abi: ABI, event_name: str=None, argument_names: Sequence[str]=None
) -> ABIEvent:
filters = [
functools.partial(filter_by_type, 'event'),
]
if event_name is not None:
filters.append(functools.partial(filter_by_name, event_name))
if argument_names is not None:
filters.append(
functools.partial(filter_by_argument_name, argument_names)
)
event_abi_candidates = pipe(abi, *filters)
if len(event_abi_candidates) == 1:
return event_abi_candidates[0]
elif not event_abi_candidates:
raise ValueError("No matching events found")
else:
raise ValueError("Multiple events found")
def find_matching_fn_abi(
abi: ABI,
abi_codec: ABICodec,
fn_identifier: Union[str, Type[FallbackFn], Type[ReceiveFn]]=None,
args: Sequence[Any]=None,
kwargs: Any=None,
) -> ABIFunction:
args = args or tuple()
kwargs = kwargs or dict()
num_arguments = len(args) + len(kwargs)
if fn_identifier is FallbackFn:
return get_fallback_func_abi(abi)
if fn_identifier is ReceiveFn:
return get_receive_func_abi(abi)
if not is_text(fn_identifier):
raise TypeError("Unsupported function identifier")
name_filter = functools.partial(filter_by_name, fn_identifier)
arg_count_filter = functools.partial(filter_by_argument_count, num_arguments)
encoding_filter = functools.partial(filter_by_encodability, abi_codec, args, kwargs)
function_candidates = pipe(abi, name_filter, arg_count_filter, encoding_filter)
if len(function_candidates) == 1:
return function_candidates[0]
else:
matching_identifiers = name_filter(abi)
matching_function_signatures = [abi_to_signature(func) for func in matching_identifiers]
arg_count_matches = len(arg_count_filter(matching_identifiers))
encoding_matches = len(encoding_filter(matching_identifiers))
if arg_count_matches == 0:
diagnosis = "\nFunction invocation failed due to improper number of arguments."
elif encoding_matches == 0:
diagnosis = "\nFunction invocation failed due to no matching argument types."
elif encoding_matches > 1:
diagnosis = (
"\nAmbiguous argument encoding. "
"Provided arguments can be encoded to multiple functions matching this call."
)
message = (
"\nCould not identify the intended function with name `{name}`, "
"positional argument(s) of type `{arg_types}` and "
"keyword argument(s) of type `{kwarg_types}`."
"\nFound {num_candidates} function(s) with the name `{name}`: {candidates}"
"{diagnosis}"
).format(
name=fn_identifier,
arg_types=tuple(map(type, args)),
kwarg_types=valmap(type, kwargs),
num_candidates=len(matching_identifiers),
candidates=matching_function_signatures,
diagnosis=diagnosis,
)
raise ValidationError(message)

In the case of find_matching_fn_abi we already have a good start at an improved error message, so the same pattern could be ported over to find_matching_event_abi. This is probably the smallest and simplest change that would improve things for events in the same way that function lookups have historically been improved.

To further improve the messages, we'll need to do a bit deeper adjustment of the code upon failure to find a match to gather the following data: For each ABI definition, determine whether it matches each individual criteria (name,number-of-args,argument-types) and then format that data into a single error message detailing each of the function's, their function signatures, and whether they matched the required critera.

(Just dumping this here so that when this does get worked on, it's a bit easier to figure out where to start and what needs to be done)

@reedsa reedsa self-assigned this Jul 25, 2024
reedsa added a commit to reedsa/web3.py that referenced this issue Aug 19, 2024
reedsa added a commit to reedsa/web3.py that referenced this issue Aug 19, 2024
reedsa added a commit to reedsa/web3.py that referenced this issue Aug 19, 2024
@reedsa reedsa linked a pull request Sep 19, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment