From 9f2202630b0294965f6426ff719fc8f8ce8c3b0a Mon Sep 17 00:00:00 2001 From: Felipe Selmo Date: Fri, 1 Jul 2022 16:15:34 -0600 Subject: [PATCH 1/2] Better messaging for wrong tuple arguments to contract functions - collapse tuple types into their nested types for the recognized function signature(s) - collapse user argument types to better compare against the recognized function signature(s) --- .../contracts/test_contract_call_interface.py | 51 +++++++++++++++++-- web3/_utils/abi.py | 3 +- web3/_utils/contracts.py | 46 +++++++++++++++-- 3 files changed, 91 insertions(+), 9 deletions(-) diff --git a/tests/core/contracts/test_contract_call_interface.py b/tests/core/contracts/test_contract_call_interface.py index 257a47de30..4f14dd7cba 100644 --- a/tests/core/contracts/test_contract_call_interface.py +++ b/tests/core/contracts/test_contract_call_interface.py @@ -620,9 +620,8 @@ def test_returns_data_from_specified_block(w3, math_contract): message_regex = ( - r"\nCould not identify the intended function with name `.*`, " - r"positional argument\(s\) of type `.*` and " - r"keyword argument\(s\) of type `.*`." + r"\nCould not identify the intended function with name `.*`, positional arguments " + r"with type\(s\) `.*` and keyword arguments with type\(s\) `.*`." r"\nFound .* function\(s\) with the name `.*`: .*" ) diagnosis_arg_regex = ( @@ -672,6 +671,52 @@ def test_function_multiple_error_diagnoses(w3, arg1, arg2, diagnosis): Contract.functions.a(arg1).call() +@pytest.mark.parametrize( + "address", ( + "0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE", # checksummed + b'\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee', # noqa: E501 + ) +) +def test_function_wrong_args_for_tuple_collapses_args_in_message( + address, tuple_contract, +): + with pytest.raises(ValidationError) as e: + tuple_contract.functions.method( + (1, [2, 3], [(4, [True, [False]], [address])]) + ).call() + + # assert the user arguments are formatted as expected: + # (int,(int,int),((int,(bool,(bool)),(address)))) + e.match("\\(int,\\(int,int\\),\\(\\(int,\\(bool,\\(bool\\)\\),\\(address\\)\\)\\)\\)") # noqa: E501 + + # assert the found method signature is formatted as expected: + # ['method((uint256,uint256[],(int256,bool[2],address[])[]))'] + e.match("\\['method\\(\\(uint256,uint256\\[\\],\\(int256,bool\\[2\\],address\\[\\]\\)\\[\\]\\)\\)'\\]") # noqa: E501 + + +@pytest.mark.parametrize( + "address", ( + "0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE", # checksummed + b'\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee', # noqa: E501 + ) +) +def test_function_wrong_args_for_tuple_collapses_kwargs_in_message( + address, tuple_contract +): + with pytest.raises(ValidationError) as e: + tuple_contract.functions.method( + a=(1, [2, 3], [(4, [True, [False]], [address])]) # noqa: E501 + ).call() + + # assert the user keyword arguments are formatted as expected: + # {'a': '(int,(int,int),((int,(bool,(bool)),(address))))'} + e.match("{'a': '\\(int,\\(int,int\\),\\(\\(int,\\(bool,\\(bool\\)\\),\\(address\\)\\)\\)\\)'}") # noqa: E501 + + # assert the found method signature is formatted as expected: + # ['method((uint256,uint256[],(int256,bool[2],address[])[]))'] + e.match("\\['method\\(\\(uint256,uint256\\[\\],\\(int256,bool\\[2\\],address\\[\\]\\)\\[\\]\\)\\)'\\]") # noqa: E501 + + def test_function_no_abi(w3): contract = w3.eth.contract() with pytest.raises(NoABIFound): diff --git a/web3/_utils/abi.py b/web3/_utils/abi.py index ca6493e3b0..fbb9d3f384 100644 --- a/web3/_utils/abi.py +++ b/web3/_utils/abi.py @@ -724,7 +724,8 @@ def abi_to_signature(abi: Union[ABIFunction, ABIEvent]) -> str: function_signature = "{fn_name}({fn_input_types})".format( fn_name=abi["name"], fn_input_types=",".join( - [arg["type"] for arg in normalize_event_input_types(abi.get("inputs", []))] + collapse_if_tuple(dict(arg)) + for arg in normalize_event_input_types(abi.get("inputs", [])) ), ) return function_signature diff --git a/web3/_utils/contracts.py b/web3/_utils/contracts.py index b7efe6d747..460f6973e3 100644 --- a/web3/_utils/contracts.py +++ b/web3/_utils/contracts.py @@ -21,11 +21,13 @@ add_0x_prefix, encode_hex, function_abi_to_4byte_selector, + is_binary_address, + is_checksum_address, + is_list_like, is_text, ) from eth_utils.toolz import ( pipe, - valmap, ) from hexbytes import ( HexBytes, @@ -73,6 +75,28 @@ from web3 import Web3 # noqa: F401 +def extract_argument_types(*args: Sequence[Any]) -> str: + """ + Takes a list of arguments and returns a string representation of the argument types, + appropriately collapsing `tuple` types into the respective nested types. + """ + collapsed_args = [] + + for arg in args: + if is_list_like(arg): + collapsed_nested = [] + for nested in arg: + if is_list_like(nested): + collapsed_nested.append(f"({extract_argument_types(nested)})") + else: + collapsed_nested.append(_get_argument_readable_type(nested)) + collapsed_args.append(",".join(collapsed_nested)) + else: + collapsed_args.append(_get_argument_readable_type(arg)) + + return ",".join(collapsed_args) + + def find_matching_event_abi( abi: ABI, event_name: Optional[str] = None, @@ -150,12 +174,17 @@ def find_matching_fn_abi( "Provided arguments can be encoded to multiple functions " "matching this call." ) + + collapsed_args = extract_argument_types(args) + collapsed_kwargs = dict( + {(k, extract_argument_types([v])) for k, v in kwargs.items()} + ) message = ( f"\nCould not identify the intended function with name `{fn_identifier}`, " - f"positional argument(s) of type `{tuple(map(type, args))}` and keyword " - f"argument(s) of type `{valmap(type, kwargs)}`.\nFound " - f"{len(matching_identifiers)} function(s) with the name " - f"`{fn_identifier}`: {matching_function_signatures}{diagnosis}" + f"positional arguments with type(s) `{collapsed_args}` and " + f"keyword arguments with type(s) `{collapsed_kwargs}`." + f"\nFound {len(matching_identifiers)} function(s) with " + f"the name `{fn_identifier}`: {matching_function_signatures}{diagnosis}" ) raise ValidationError(message) @@ -333,3 +362,10 @@ def validate_payable(transaction: TxParams, abi: ABIFunction) -> None: "with payable=False. Please ensure that " "transaction's value is 0." ) + + +def _get_argument_readable_type(arg: Any) -> str: + if is_checksum_address(arg) or is_binary_address(arg): + return "address" + + return arg.__class__.__name__ From 0df507e527e9294a43c2d37fb0c027e1136d1d6f Mon Sep 17 00:00:00 2001 From: Felipe Selmo Date: Fri, 1 Jul 2022 16:18:08 -0600 Subject: [PATCH 2/2] Update newsfragment README.md and add newsfragment for 2556 --- newsfragments/2556.breaking-change.rst | 1 + newsfragments/README.md | 2 ++ .../contracts/test_contract_call_interface.py | 35 ++++++++++++------- 3 files changed, 26 insertions(+), 12 deletions(-) create mode 100644 newsfragments/2556.breaking-change.rst diff --git a/newsfragments/2556.breaking-change.rst b/newsfragments/2556.breaking-change.rst new file mode 100644 index 0000000000..9dee27415a --- /dev/null +++ b/newsfragments/2556.breaking-change.rst @@ -0,0 +1 @@ +Provide better messaging to wrong arguments for contract functions, especially for ``tuple`` argument types. diff --git a/newsfragments/README.md b/newsfragments/README.md index a606af454f..7b233a150f 100644 --- a/newsfragments/README.md +++ b/newsfragments/README.md @@ -12,6 +12,8 @@ relevant to people working on the code itself.) * `bugfix` * `doc` * `misc` +* `breaking-change` +* `deprecation` So for example: `123.feature.rst`, `456.bugfix.rst` diff --git a/tests/core/contracts/test_contract_call_interface.py b/tests/core/contracts/test_contract_call_interface.py index 4f14dd7cba..62692198d3 100644 --- a/tests/core/contracts/test_contract_call_interface.py +++ b/tests/core/contracts/test_contract_call_interface.py @@ -672,13 +672,15 @@ def test_function_multiple_error_diagnoses(w3, arg1, arg2, diagnosis): @pytest.mark.parametrize( - "address", ( + "address", + ( "0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE", # checksummed - b'\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee', # noqa: E501 - ) + b"\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee", # noqa: E501 + ), ) def test_function_wrong_args_for_tuple_collapses_args_in_message( - address, tuple_contract, + address, + tuple_contract, ): with pytest.raises(ValidationError) as e: tuple_contract.functions.method( @@ -687,18 +689,23 @@ def test_function_wrong_args_for_tuple_collapses_args_in_message( # assert the user arguments are formatted as expected: # (int,(int,int),((int,(bool,(bool)),(address)))) - e.match("\\(int,\\(int,int\\),\\(\\(int,\\(bool,\\(bool\\)\\),\\(address\\)\\)\\)\\)") # noqa: E501 + e.match( + "\\(int,\\(int,int\\),\\(\\(int,\\(bool,\\(bool\\)\\),\\(address\\)\\)\\)\\)" + ) # assert the found method signature is formatted as expected: # ['method((uint256,uint256[],(int256,bool[2],address[])[]))'] - e.match("\\['method\\(\\(uint256,uint256\\[\\],\\(int256,bool\\[2\\],address\\[\\]\\)\\[\\]\\)\\)'\\]") # noqa: E501 + e.match( + "\\['method\\(\\(uint256,uint256\\[\\],\\(int256,bool\\[2\\],address\\[\\]\\)\\[\\]\\)\\)'\\]" # noqa: E501 + ) @pytest.mark.parametrize( - "address", ( + "address", + ( "0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE", # checksummed - b'\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee', # noqa: E501 - ) + b"\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee", # noqa: E501 + ), ) def test_function_wrong_args_for_tuple_collapses_kwargs_in_message( address, tuple_contract @@ -710,11 +717,15 @@ def test_function_wrong_args_for_tuple_collapses_kwargs_in_message( # assert the user keyword arguments are formatted as expected: # {'a': '(int,(int,int),((int,(bool,(bool)),(address))))'} - e.match("{'a': '\\(int,\\(int,int\\),\\(\\(int,\\(bool,\\(bool\\)\\),\\(address\\)\\)\\)\\)'}") # noqa: E501 + e.match( + "{'a': '\\(int,\\(int,int\\),\\(\\(int,\\(bool,\\(bool\\)\\),\\(address\\)\\)\\)\\)'}" # noqa: E501 + ) # assert the found method signature is formatted as expected: # ['method((uint256,uint256[],(int256,bool[2],address[])[]))'] - e.match("\\['method\\(\\(uint256,uint256\\[\\],\\(int256,bool\\[2\\],address\\[\\]\\)\\[\\]\\)\\)'\\]") # noqa: E501 + e.match( + "\\['method\\(\\(uint256,uint256\\[\\],\\(int256,bool\\[2\\],address\\[\\]\\)\\[\\]\\)\\)'\\]" # noqa: E501 + ) def test_function_no_abi(w3): @@ -1471,7 +1482,7 @@ async def test_async_accepts_block_hash_as_identifier(async_w3, async_math_contr ) new = await async_math_contract.functions.counter().call( block_identifier=more_blocks["result"][2] - ) # noqa: E501 + ) assert old == 0 assert new == 1