Additional test coverage for invalid addresses and return string values for single item lists

Author: Stuart Reed

tests/core/utilities/test_construct_event_filter_params.py

@@ -3,23 +3,34 @@
 from web3._utils.filters import (
     construct_event_filter_params,
 )
+from web3.exceptions import (
+    InvalidAddress,
+    Web3ValueError,
+)
+
+
+def hex_and_pad(i):
+    unpadded_hex_value = hex(i).rstrip("L")
+    return "0x" + unpadded_hex_value[2:].zfill(64)
 
-EVENT_1_ABI = {
-    "anonymous": False,
-    "inputs": [
-        {"indexed": False, "name": "arg0", "type": "uint256"},
-        {"indexed": True, "name": "arg1", "type": "uint256"},
-    ],
-    "name": "Event_1",
-    "type": "event",
-}
+
+@pytest.fixture
+def event_abi():
+    return {
+        "anonymous": False,
+        "inputs": [
+            {"indexed": False, "name": "arg0", "type": "uint256"},
+            {"indexed": True, "name": "arg1", "type": "uint256"},
+        ],
+        "name": "Event_1",
+        "type": "event",
+    }
 
 
 @pytest.mark.parametrize(
-    "event_abi,fn_kwargs,expected",
+    "fn_kwargs,expected",
     (
         pytest.param(
-            EVENT_1_ABI,
             {},
             {
                 "topics": [
@@ -29,13 +40,20 @@
             id="no-args",
         ),
         pytest.param(
-            EVENT_1_ABI,
             {"topics": ["should-overwrite-topics"]},
             {"topics": ["should-overwrite-topics"]},
             id="overwrite-topics",
         ),
         pytest.param(
-            EVENT_1_ABI,
+            {"address": None},
+            {
+                "topics": [
+                    "0xb470a829ed7792f06947f0ca3730a570cb378329ddcf09f2b4efabd6326f51f6"
+                ],
+            },
+            id="no-address",
+        ),
+        pytest.param(
             {"contract_address": "0xd3CdA913deB6f67967B99D67aCDFa1712C293601"},
             {
                 "topics": [
@@ -46,7 +64,6 @@
             id="contract_address-string",
         ),
         pytest.param(
-            EVENT_1_ABI,
             {
                 "contract_address": "0xd3CdA913deB6f67967B99D67aCDFa1712C293601",
                 "address": "0xBB9bc244D798123fDe783fCc1C72d3Bb8C189413",
@@ -63,7 +80,22 @@
             id="address-with-contract_address",
         ),
         pytest.param(
-            EVENT_1_ABI,
+            {
+                "contract_address": ["0xd3CdA913deB6f67967B99D67aCDFa1712C293601"],
+                "address": ["0xBB9bc244D798123fDe783fCc1C72d3Bb8C189413"],
+            },
+            {
+                "topics": [
+                    "0xb470a829ed7792f06947f0ca3730a570cb378329ddcf09f2b4efabd6326f51f6"
+                ],
+                "address": [
+                    "0xBB9bc244D798123fDe783fCc1C72d3Bb8C189413",
+                    "0xd3CdA913deB6f67967B99D67aCDFa1712C293601",
+                ],
+            },
+            id="single-item-address-list-with-contract_address-list",
+        ),
+        pytest.param(
             {"address": "0xd3CdA913deB6f67967B99D67aCDFa1712C293601"},
             {
                 "topics": [
@@ -74,18 +106,16 @@
             id="address-string",
         ),
         pytest.param(
-            EVENT_1_ABI,
             {"address": ["0xd3CdA913deB6f67967B99D67aCDFa1712C293601"]},
             {
                 "topics": [
                     "0xb470a829ed7792f06947f0ca3730a570cb378329ddcf09f2b4efabd6326f51f6"
                 ],
-                "address": ["0xd3CdA913deB6f67967B99D67aCDFa1712C293601"],
+                "address": "0xd3CdA913deB6f67967B99D67aCDFa1712C293601",
             },
             id="address-list",
         ),
         pytest.param(
-            EVENT_1_ABI,
             {
                 "address": [
                     "0xd3CdA913deB6f67967B99D67aCDFa1712C293601",
@@ -104,7 +134,6 @@
             id="multiple-address",
         ),
         pytest.param(
-            EVENT_1_ABI,
             {
                 "address": "0xd3CdA913deB6f67967B99D67aCDFa1712C293601",
                 "contract_address": ["0xBB9bc244D798123fDe783fCc1C72d3Bb8C189413"],
@@ -121,7 +150,6 @@
             id="one-address-with-multiple-contract_address",
         ),
         pytest.param(
-            EVENT_1_ABI,
             {
                 "address": [
                     "0xd3CdA913deB6f67967B99D67aCDFa1712C293601",
@@ -141,26 +169,125 @@
             },
             id="multiple-address-with-one-contract_address",
         ),
+        pytest.param(
+            {
+                "address": [
+                    "0xd3CdA913deB6f67967B99D67aCDFa1712C293601",
+                    "0x1234567890123456789012345678901234567890",
+                ],
+                "contract_address": [
+                    "0xBB9bc244D798123fDe783fCc1C72d3Bb8C189413",
+                    "0x1234567890123456789012345678901234567890",
+                ],
+            },
+            {
+                "topics": [
+                    "0xb470a829ed7792f06947f0ca3730a570cb378329ddcf09f2b4efabd6326f51f6"
+                ],
+                "address": [
+                    "0xd3CdA913deB6f67967B99D67aCDFa1712C293601",
+                    "0x1234567890123456789012345678901234567890",
+                    "0xBB9bc244D798123fDe783fCc1C72d3Bb8C189413",
+                ],
+            },
+            id="multiple-address-with-multiple-contract_address",
+        ),
+        pytest.param(
+            {
+                "contract_address": "0xBB9bc244D798123fDe783fCc1C72d3Bb8C189413",
+                "topics": [
+                    "0xb470a829ed7792f06947f0ca3730a570cb378329ddcf09f2b4efabd6326f51f6"
+                ],
+                "from_block": "latest",
+                "to_block": "latest",
+                "address": [
+                    "0xd3CdA913deB6f67967B99D67aCDFa1712C293601",
+                    "0x1234567890123456789012345678901234567890",
+                    "0xBB9bc244D798123fDe783fCc1C72d3Bb8C189413",
+                ],
+            },
+            {
+                "topics": [
+                    "0xb470a829ed7792f06947f0ca3730a570cb378329ddcf09f2b4efabd6326f51f6"
+                ],
+                "address": [
+                    "0xd3CdA913deB6f67967B99D67aCDFa1712C293601",
+                    "0x1234567890123456789012345678901234567890",
+                    "0xBB9bc244D798123fDe783fCc1C72d3Bb8C189413",
+                ],
+                "fromBlock": "latest",
+                "toBlock": "latest",
+            },
+            id="all-arguments-with-address-list",
+        ),
     ),
 )
 def test_construct_event_filter_params(w3, event_abi, fn_kwargs, expected):
-    _, actual = construct_event_filter_params(event_abi, w3.codec, **fn_kwargs)
-    assert actual == expected
+    _, filter_params = construct_event_filter_params(event_abi, w3.codec, **fn_kwargs)
+    # Ensure that the filter_params contains the expected keys
+    assert (
+        filter_params.keys() == expected.keys()
+    ), f"Keys don't match. Expected {set(expected.keys())}, got {set(filter_params.keys())}"  # noqa: E501
+    # Verify all values in filter_params match the expected values
+    for key, value in expected.items():
+        if isinstance(value, list) and isinstance(filter_params[key], list):
+            assert sorted(filter_params[key]) == sorted(
+                value
+            ), f"Expected {key}={value}, got {key}={filter_params.get(key)}"
+        else:
+            assert (
+                filter_params[key] == value
+            ), f"Expected {key}={value}, got {key}={filter_params.get(key)}"
 
 
-def hex_and_pad(i):
-    unpadded_hex_value = hex(i).rstrip("L")
-    return "0x" + unpadded_hex_value[2:].zfill(64)
+@pytest.mark.parametrize(
+    "fn_kwargs, expected_exception",
+    [
+        pytest.param(
+            {
+                "contract_address": "0xbb9bc244d798123fde783fcc1c72d3bb8c189413",
+                "topics": [
+                    "0xb470a829ed7792f06947f0ca3730a570cb378329ddcf09f2b4efabd6326f51f6"
+                ],
+                "address": [
+                    "0xd3cda913deb6f67967b99d67acdfa1712c293601",
+                    "0x1234567890123456789012345678901234567890",
+                    "0xBB9bc244D798123fDe783fCc1C72d3Bb8C189413",
+                ],
+                "from_block": 1,
+                "to_block": 2,
+            },
+            InvalidAddress,
+            id="invalid-checksum-address",
+        ),
+        pytest.param(
+            {
+                "contract_address": "0xbb9bc244d798123fde783fcc1c72d3bb8c189413",
+            },
+            InvalidAddress,
+            id="invalid-checksum-contract-address",
+        ),
+        pytest.param(
+            {"address": {"invalid": "0x1234567890123456789012345678901234567890"}},
+            Web3ValueError,
+            id="unsupported-type-exception",
+        ),
+    ],
+)
+def test_construct_event_filter_params_exceptions(
+    w3, event_abi, fn_kwargs, expected_exception
+):
+    with pytest.raises(expected_exception):
+        construct_event_filter_params(event_abi, w3.codec, **fn_kwargs)
 
 
 @pytest.mark.parametrize(
-    "event_abi,fn_kwargs,expected",
+    "fn_kwargs,expected",
     (
-        (EVENT_1_ABI, {}, [[]]),
-        (EVENT_1_ABI, {"argument_filters": {"arg0": 1}}, [[hex_and_pad(1)]]),
-        (EVENT_1_ABI, {"argument_filters": {"arg0": [1]}}, [[hex_and_pad(1)]]),
+        ({}, [[]]),
+        ({"argument_filters": {"arg0": 1}}, [[hex_and_pad(1)]]),
+        ({"argument_filters": {"arg0": [1]}}, [[hex_and_pad(1)]]),
         (
-            EVENT_1_ABI,
             {"argument_filters": {"arg0": [1, 2]}},
             [
                 [hex_and_pad(1)],
@@ -170,7 +297,7 @@ def hex_and_pad(i):
     ),
 )
 def test_construct_event_filter_params_for_data_filters(
-    event_abi, w3, fn_kwargs, expected
+    w3, event_abi, fn_kwargs, expected
 ):
     actual, _ = construct_event_filter_params(event_abi, w3.codec, **fn_kwargs)
     assert actual == expected

web3/_utils/filters.py

@@ -8,6 +8,7 @@
     List,
     Optional,
     Sequence,
+    Set,
     Tuple,
     Union,
 )
@@ -26,7 +27,6 @@
 )
 from eth_utils import (
     is_hex,
-    is_list_like,
     is_string,
     is_text,
 )
@@ -67,33 +67,37 @@
     from web3.eth import Eth  # noqa: F401
 
 
-def _validate_addresses_as_list(
-    address: Union[ChecksumAddress, List[ChecksumAddress]],
-) -> List[ChecksumAddress]:
+def _sanitize_addresses(
+    *address: Union[ChecksumAddress, List[ChecksumAddress]],
+) -> Union[ChecksumAddress, List[ChecksumAddress]]:
     """
-    Validates the address or list of addresses and returns a list of
-    ChecksumAddress.
+    Validates an address or list of addresses and returns a single
+    ChecksumAddress or a list of ChecksumAddresses.
     Raises Web3ValueError if the address is not valid.
 
     :param address: A single address or a list of addresses.
     :return: A list of ChecksumAddress.
     """
-    address_list: List[ChecksumAddress] = []
-    if address:
-        if isinstance(address, List):
-            address_list = address
+    address_set: Set[ChecksumAddress] = set()
+    for arg in address:
+        if not arg:
+            continue
+        elif isinstance(arg, str):
+            address_set.add(arg)
+        elif isinstance(arg, list):
+            address_set.update(arg)
         else:
-            address_list = [address]
+            raise Web3ValueError(
+                f"Unsupported type for `address` parameter: {type(address)}"
+            )
 
-        for addr in address_list:
-            try:
-                validate_address(addr)
-            except Web3ValidationError:
-                raise Web3ValueError(
-                    f"Unsupported type for `address` parameter: {type(address)}"
-                )
+    if not address_set:
+        return []
+    else:
+        for addr in address_set:
+            validate_address(addr)
 
-    return address_list
+        return list(address_set) if len(address_set) > 1 else address_set.pop()
 
 
 def construct_event_filter_params(
@@ -121,20 +125,9 @@ def construct_event_filter_params(
 
     filter_params["topics"] = topic_set
 
-    if address and contract_address:
-        filter_params["address"] = _validate_addresses_as_list(
-            address
-        ) + _validate_addresses_as_list(contract_address)
-    elif address and not contract_address:
-        if is_list_like(address):
-            filter_params["address"] = _validate_addresses_as_list(address)
-        else:
-            filter_params["address"] = address
-    elif contract_address:
-        if is_list_like(contract_address):
-            filter_params["address"] = _validate_addresses_as_list(contract_address)
-        else:
-            filter_params["address"] = contract_address
+    sanitized_addresses = _sanitize_addresses(address, contract_address)
+    if sanitized_addresses:
+        filter_params["address"] = sanitized_addresses
 
     if from_block is not None:
         filter_params["fromBlock"] = from_block