Fix contract function name collision (#3147)

* Added test to demonstrate error when w3 is used as a function name in an abi.

* Use function argument w3 variable rather than self member in ContractCaller incase it has been replaced.

* Corrected import order.

* Add integration test with contract containing w3 variable

* Add async test

* Fix async contract init

* Add test for sync version

* Add test for async contract call

* Use existing fixture for abi

---------

Co-authored-by: Rolo <[email protected]>

Author: Stuart Reed

newsfragments/3147.bugfix.rst

@@ -0,0 +1 @@
+Fix collision of ``w3`` variable when initializing contract with function of the same name

tests/core/contracts/conftest.py

@@ -28,6 +28,10 @@
 from web3._utils.contract_sources.contract_data.fallback_function_contract import (
     FALLBACK_FUNCTION_CONTRACT_DATA,
 )
+from web3._utils.contract_sources.contract_data.function_name_tester_contract import (
+    FUNCTION_NAME_TESTER_CONTRACT_ABI,
+    FUNCTION_NAME_TESTER_CONTRACT_DATA,
+)
 from web3._utils.contract_sources.contract_data.math_contract import (
     MATH_CONTRACT_ABI,
     MATH_CONTRACT_BYTECODE,
@@ -55,6 +59,24 @@
     TUPLE_CONTRACT_DATA,
 )
 
+# --- function name tester contract --- #
+
+
+@pytest.fixture(scope="session")
+def function_name_tester_contract_abi():
+    return FUNCTION_NAME_TESTER_CONTRACT_ABI
+
+
+@pytest.fixture
+def function_name_tester_contract(w3, address_conversion_func):
+    function_name_tester_contract_factory = w3.eth.contract(
+        **FUNCTION_NAME_TESTER_CONTRACT_DATA
+    )
+    return deploy(w3, function_name_tester_contract_factory, address_conversion_func)
+
+
+# --- math contract --- #
+
 
 @pytest.fixture(scope="session")
 def math_contract_bytecode():
@@ -81,6 +103,9 @@ def math_contract(w3, math_contract_factory, address_conversion_func):
     return deploy(w3, math_contract_factory, address_conversion_func)
 
 
+# --- constructor contracts --- #
+
+
 @pytest.fixture
 def simple_constructor_contract_factory(w3):
     return w3.eth.contract(**SIMPLE_CONSTRUCTOR_CONTRACT_DATA)
@@ -113,6 +138,9 @@ def contract_with_constructor_address(
     )
 
 
+# --- address reflector contract --- #
+
+
 @pytest.fixture
 def address_reflector_contract(w3, address_conversion_func):
     address_reflector_contract_factory = w3.eth.contract(
@@ -121,6 +149,9 @@ def address_reflector_contract(w3, address_conversion_func):
     return deploy(w3, address_reflector_contract_factory, address_conversion_func)
 
 
+# --- string contract --- #
+
+
 @pytest.fixture(scope="session")
 def string_contract_data():
     return STRING_CONTRACT_DATA
@@ -153,6 +184,9 @@ def non_strict_string_contract(
     )
 
 
+# --- emitter contract --- #
+
+
 @pytest.fixture
 def non_strict_emitter(
     w3_non_strict_abi,
@@ -180,6 +214,9 @@ def non_strict_emitter(
     return emitter_contract
 
 
+# --- event contract --- #
+
+
 @pytest.fixture
 def event_contract(
     w3,
@@ -223,6 +260,9 @@ def indexed_event_contract(
     return indexed_event_contract
 
 
+# --- arrays contract --- #
+
+
 # bytes_32 = [keccak('0'), keccak('1')]
 BYTES32_ARRAY = [
     b"\x04HR\xb2\xa6p\xad\xe5@~x\xfb(c\xc5\x1d\xe9\xfc\xb9eB\xa0q\x86\xfe:\xed\xa6\xbb\x8a\x11m",  # noqa: E501
@@ -255,12 +295,18 @@ def non_strict_arrays_contract(w3_non_strict_abi, address_conversion_func):
     )
 
 
+# --- payable tester contract --- #
+
+
 @pytest.fixture
 def payable_tester_contract(w3, address_conversion_func):
     payable_tester_contract_factory = w3.eth.contract(**PAYABLE_TESTER_CONTRACT_DATA)
     return deploy(w3, payable_tester_contract_factory, address_conversion_func)
 
 
+# --- fixed reflector contract --- #
+
+
 # no matter the function selector, this will return back the 32 bytes of data supplied
 FIXED_REFLECTOR_CONTRACT_BYTECODE = "0x610011566020600460003760206000f3005b61000461001103610004600039610004610011036000f3"  # noqa: E501
 # reference source used to generate it:
@@ -306,6 +352,9 @@ def fixed_reflector_contract(w3, address_conversion_func):
     return deploy(w3, fixed_reflector_contract_factory, address_conversion_func)
 
 
+# --- test data and functions contracts --- #
+
+
 @pytest.fixture
 def fallback_function_contract(w3, address_conversion_func):
     fallback_function_contract_factory = w3.eth.contract(
@@ -387,6 +436,9 @@ def some_address(address_conversion_func):
     return address_conversion_func("0x5B2063246F2191f18F2675ceDB8b28102e957458")
 
 
+# --- invoke contract --- #
+
+
 def invoke_contract(
     api_call_desig="call",
     contract=None,
@@ -444,6 +496,16 @@ async def async_math_contract(
     )
 
 
+@pytest_asyncio.fixture
+async def async_function_name_tester_contract(async_w3, address_conversion_func):
+    function_name_tester_contract_factory = async_w3.eth.contract(
+        **FUNCTION_NAME_TESTER_CONTRACT_DATA
+    )
+    return await async_deploy(
+        async_w3, function_name_tester_contract_factory, address_conversion_func
+    )
+
+
 @pytest.fixture
 def async_simple_constructor_contract_factory(async_w3):
     return async_w3.eth.contract(**SIMPLE_CONSTRUCTOR_CONTRACT_DATA)

tests/core/contracts/test_contract_call_interface.py

@@ -178,6 +178,11 @@ def test_saved_method_call_with_multiple_arguments(
     assert result == 16
 
 
+def test_call_get_w3_value(function_name_tester_contract, call):
+    result = call(contract=function_name_tester_contract, contract_function="w3")
+    assert result is True
+
+
 def test_call_get_string_value(string_contract, call):
     result = call(contract=string_contract, contract_function="getValue")
     # eth_abi.decode() does not assume implicit utf-8
@@ -1295,6 +1300,14 @@ async def test_async_saved_method_call_with_multiple_arguments(
     assert result == 16
 
 
+@pytest.mark.asyncio
+async def test_async_call_get_w3_value(async_function_name_tester_contract, async_call):
+    result = await async_call(
+        contract=async_function_name_tester_contract, contract_function="w3"
+    )
+    assert result is True
+
+
 @pytest.mark.asyncio
 async def test_async_call_get_string_value(async_string_contract, async_call):
     result = await async_call(

tests/core/contracts/test_contract_class_construction.py

@@ -42,6 +42,39 @@ def test_abi_as_json_string(w3, math_contract_abi, some_address):
     assert math.abi == math_contract_abi
 
 
+def test_contract_init_with_w3_function_name(
+    w3,
+    function_name_tester_contract_abi,
+    function_name_tester_contract,
+):
+    # test `w3` function name does not throw when creating the contract factory
+    contract_factory = w3.eth.contract(abi=function_name_tester_contract_abi)
+
+    # re-instantiate the contract
+    contract = contract_factory(function_name_tester_contract.address)
+
+    # assert the `w3` function returns true when called
+    result = contract.functions.w3().call()
+    assert result is True
+
+
+@pytest.mark.asyncio
+async def test_async_contract_init_with_w3_function_name(
+    async_w3,
+    function_name_tester_contract_abi,
+    async_function_name_tester_contract,
+):
+    # test `w3` function name does not throw when creating the contract factory
+    contract_factory = async_w3.eth.contract(abi=function_name_tester_contract_abi)
+
+    # re-instantiate the contract
+    contract = contract_factory(async_function_name_tester_contract.address)
+
+    # assert the `w3` function returns true when called
+    result = await contract.functions.w3().call()
+    assert result is True
+
+
 def test_error_to_call_non_existent_fallback(
     w3, math_contract_abi, math_contract_bytecode, math_contract_runtime
 ):

web3/_utils/contract_sources/FunctionNameTesterContract.sol

@@ -0,0 +1,12 @@
+pragma solidity ^0.8.23;
+
+contract FunctionNameTesterContract {
+    function w3() public returns (bool) {
+        return true;
+    }
+
+    // unused, this just needs to come after `w3` in the abi... so name it "z"
+    function z() public returns (bool) {
+        return false;
+    }
+}

web3/_utils/contract_sources/contract_data/function_name_tester_contract.py

@@ -0,0 +1,29 @@
+"""
+Generated by `compile_contracts.py` script.
+Compiled with Solidity v0.8.23.
+"""
+
+# source: web3/_utils/contract_sources/FunctionNameTesterContract.sol:FunctionNameTesterContract  # noqa: E501
+FUNCTION_NAME_TESTER_CONTRACT_BYTECODE = "0x608060405234801561000f575f80fd5b5060d98061001c5f395ff3fe6080604052348015600e575f80fd5b50600436106030575f3560e01c8063a044c987146034578063c5d7802e14604e575b5f80fd5b603a6068565b60405160459190608c565b60405180910390f35b60546070565b604051605f9190608c565b60405180910390f35b5f6001905090565b5f90565b5f8115159050919050565b6086816074565b82525050565b5f602082019050609d5f830184607f565b9291505056fea264697066735822122056b76f22006829335981c36eca76f8aa0c6cf66d23990263a18b17fa27ab3db064736f6c63430008170033"  # noqa: E501
+FUNCTION_NAME_TESTER_CONTRACT_RUNTIME = "0x6080604052348015600e575f80fd5b50600436106030575f3560e01c8063a044c987146034578063c5d7802e14604e575b5f80fd5b603a6068565b60405160459190608c565b60405180910390f35b60546070565b604051605f9190608c565b60405180910390f35b5f6001905090565b5f90565b5f8115159050919050565b6086816074565b82525050565b5f602082019050609d5f830184607f565b9291505056fea264697066735822122056b76f22006829335981c36eca76f8aa0c6cf66d23990263a18b17fa27ab3db064736f6c63430008170033"  # noqa: E501
+FUNCTION_NAME_TESTER_CONTRACT_ABI = [
+    {
+        "inputs": [],
+        "name": "w3",
+        "outputs": [{"internalType": "bool", "name": "", "type": "bool"}],
+        "stateMutability": "nonpayable",
+        "type": "function",
+    },
+    {
+        "inputs": [],
+        "name": "z",
+        "outputs": [{"internalType": "bool", "name": "", "type": "bool"}],
+        "stateMutability": "nonpayable",
+        "type": "function",
+    },
+]
+FUNCTION_NAME_TESTER_CONTRACT_DATA = {
+    "bytecode": FUNCTION_NAME_TESTER_CONTRACT_BYTECODE,
+    "bytecode_runtime": FUNCTION_NAME_TESTER_CONTRACT_RUNTIME,
+    "abi": FUNCTION_NAME_TESTER_CONTRACT_ABI,
+}

web3/contract/async_contract.py

@@ -575,7 +575,7 @@ def __init__(
             for func in self._functions:
                 fn = AsyncContractFunction.factory(
                     func["name"],
-                    w3=self.w3,
+                    w3=w3,
                     contract_abi=self.abi,
                     address=self.address,
                     function_identifier=func["name"],
@@ -585,9 +585,7 @@ def __init__(
                 # TODO: The no_extra_call method gets around the fact that we can't call
                 #  the full async method from within a class's __init__ method. We need
                 #  to see if there's a way to account for all desired elif cases.
-                block_id = parse_block_identifier_no_extra_call(
-                    self.w3, block_identifier
-                )
+                block_id = parse_block_identifier_no_extra_call(w3, block_identifier)
                 caller_method = partial(
                     self.call_function,
                     fn,

web3/contract/contract.py

@@ -577,14 +577,14 @@ def __init__(
             for func in self._functions:
                 fn = ContractFunction.factory(
                     func["name"],
-                    w3=self.w3,
+                    w3=w3,
                     contract_abi=self.abi,
                     address=self.address,
                     function_identifier=func["name"],
                     decode_tuples=decode_tuples,
                 )
 
-                block_id = parse_block_identifier(self.w3, block_identifier)
+                block_id = parse_block_identifier(w3, block_identifier)
                 caller_method = partial(
                     self.call_function,
                     fn,