Repository: ethereum/eth-tester (language: python)

Issue:
Supply log topic as bytes
### What was wrong?

It's pretty natural to want to supply a log topic as a 32-byte `bytes` value instead of a hex string.

Right now, only hex-encoded strings of 32 bytes are supported.

### How can it be fixed?

Add tests that 32-byte values will be accepted, like these, but with a `decode_hex()`:
https://github.com/ethereum/eth-tester/blob/035dde2c4ebc10a205748b37b940cb519251d9da/tests/core/validation/test_inbound_validation.py#L160-L165

Add tests that 31 or 33-byte values are not accepted.

Add tests that the logs are correctly found when a bytes-value is supplied to `create_log_filter()`: https://github.com/ethereum/eth-tester/blob/2098d83b41141b4102fe9152308529d7c0a67988/eth_tester/utils/backend_testing.py#L1145

Implement any conversions needed to make ^ pass.



Discussion / hints:

Fix patch:
diff --git a/newsfragments/185.feature.rst b/newsfragments/185.feature.rst
new file mode 100644
index 00000000..60662af8
--- /dev/null
+++ b/newsfragments/185.feature.rst
@@ -0,0 +1,1 @@
+Allow filter topics to be passed as `bytes` in addition to `hex` values.
\ No newline at end of file


Test patch:
diff --git a/eth_tester/normalization/inbound.py b/eth_tester/normalization/inbound.py
index a813d010..5da5c5cf 100644
--- a/eth_tester/normalization/inbound.py
+++ b/eth_tester/normalization/inbound.py
@@ -27,6 +27,8 @@
 )
 
 from eth_tester.validation.inbound import (
+    is_32byte_hex_string,
+    is_32_bytes,
     is_valid_topic_array,
 )
 
@@ -34,8 +36,12 @@
 def normalize_topic(topic):
     if topic is None:
         return None
-    else:
+    if is_32byte_hex_string(topic):
         return decode_hex(topic)
+    if is_32_bytes(topic):
+        return topic
+
+    raise TypeError(f"Topic is not in a recognized format: {topic}")
 
 
 @to_tuple
diff --git a/eth_tester/utils/backend_testing.py b/eth_tester/utils/backend_testing.py
index 9b3d609e..1cd1c977 100644
--- a/eth_tester/utils/backend_testing.py
+++ b/eth_tester/utils/backend_testing.py
@@ -55,6 +55,9 @@
     _deploy_gas_burner,
     _make_call_gas_burner_transaction,
 )
+from eth_utils import (
+    encode_hex,
+)
 
 PK_A = "0x58d23b55bc9cdce1f18c2500f40ff4ab7245df9a89505e9b1fa4851f623d241d"
 PK_A_ADDRESS = "0xdc544d1aa88ff8bbd2f2aec754b1f1e99e1812fd"
@@ -1505,6 +1508,15 @@ def test_pending_transaction_filter(self, eth_tester):
                 ],
                 0,
             ],
+            [
+                [
+                    (
+                        b"\xf7\x0f\xe6\x89\xe2\x90\xd8\xce+*8\x8a\xc2\x8d\xb3o\xbb\x0e"
+                        b"\x16\xa6\xd8\x9ch\x04\xc4a\xf6Z\x1b@\xbb\x15"
+                    )
+                ],
+                1,
+            ],
         ),
         ids=[
             "filter None",
@@ -1513,6 +1525,7 @@ def test_pending_transaction_filter(self, eth_tester):
             "filter Event and None",
             "filter Event and argument",
             "filter Event and wrong argument",
+            "filter Event only bytes",
         ],
     )
     def test_log_filter_picks_up_new_logs(self, eth_tester, filter_topics, expected):
@@ -1551,6 +1564,45 @@ def test_log_filter_picks_up_new_logs(self, eth_tester, filter_topics, expected)
         assert len(specific_logs_all) == expected
         assert len(specific_direct_logs_all) == expected
 
+    @pytest.mark.parametrize(
+        "filter_topics",
+        (
+            "not a list",
+            {},
+            1,
+            [1],
+            [1, 2],
+            [1, None],
+            [None, 1],
+            [encode_hex(b"\x00" * 30 + b"\x01")],
+            [encode_hex(b"\x00" * 32 + b"\x01")],
+        ),
+        ids=[
+            "filter string",
+            "filter dict",
+            "filter int",
+            "filter int in list",
+            "filter multiple ints in list",
+            "filter int and None in list",
+            "filter None and int in list",
+            "filter bytes with less than 32 bytes",
+            "filter bytes with more than 32 bytes",
+        ],
+    )
+    def test_log_filter_invalid_topics_throws_error(self, eth_tester, filter_topics):
+        self.skip_if_no_evm_execution()
+
+        emitter_address = _deploy_emitter(eth_tester)
+        _call_emitter(
+            eth_tester,
+            emitter_address,
+            "logSingle",
+            [EMITTER_ENUM["LogSingleWithIndex"], 1],
+        )
+
+        with pytest.raises(ValidationError):
+            eth_tester.create_log_filter(from_block=0, topics=filter_topics)
+
     def test_log_filter_includes_old_logs(self, eth_tester):
         """
         Cases to test:
diff --git a/eth_tester/validation/inbound.py b/eth_tester/validation/inbound.py
index 80269169..4fd2a69a 100644
--- a/eth_tester/validation/inbound.py
+++ b/eth_tester/validation/inbound.py
@@ -10,6 +10,7 @@
 from eth_typing import HexStr
 from eth_utils import (
     is_boolean,
+    is_bytes,
     is_checksum_address,
     is_checksum_formatted_address,
     is_dict,
@@ -47,12 +48,16 @@
 )
 
 
+def is_32_bytes(value):
+    return is_bytes(value) and len(value) == 32
+
+
 def is_32byte_hex_string(value):
     return is_text(value) and is_hex(value) and len(remove_0x_prefix(value)) == 64
 
 
 def is_topic(value):
-    return value is None or is_32byte_hex_string(value)
+    return value is None or is_32byte_hex_string(value) or is_32_bytes(value)
 
 
 def validate_32_byte_hex_value(value, name):
@@ -155,8 +160,10 @@ def validate_filter_params(from_block, to_block, address, topics):
         validate_account(address)
 
     invalid_topics_message = (
-        "Topics must be one of `None`, an array of 32 byte hexadecimal encoded "
-        "strings, or an array of arrays of 32 byte hexadecimal strings"
+        "Topics must be one of `None` or an array of topics. Each topic must be 32 "
+        "bytes, represented as a bytestring or its hex string equivalent. A "
+        'filter query of topics using "OR" can be achieved using a sub-array of '
+        "topics. See https://eth.wiki/json-rpc/API#eth_newfilter for more details."
     )
     # topics
     if topics is None:
diff --git a/tests/core/validation/test_inbound_validation.py b/tests/core/validation/test_inbound_validation.py
index fdf3e1c3..e03aa11d 100644
--- a/tests/core/validation/test_inbound_validation.py
+++ b/tests/core/validation/test_inbound_validation.py
@@ -142,6 +142,8 @@ def test_filter_id_input_validation(validator, filter_id, is_valid):
 ADDRESS_B = encode_hex(b"\x00" * 19 + b"\x02")
 TOPIC_A = encode_hex(b"\x00" * 31 + b"\x01")
 TOPIC_B = encode_hex(b"\x00" * 31 + b"\x02")
+TOPIC_C = encode_hex(b"\x00" * 30 + b"\x01")
+TOPIC_D = encode_hex(b"\x00" * 32 + b"\x01")
 
 
 def _yield_key_value_if_value_not_none(key, value):
@@ -175,6 +177,27 @@ def _yield_key_value_if_value_not_none(key, value):
         (_make_filter_params(topics=[[TOPIC_A], [TOPIC_B]]), True),
         (_make_filter_params(topics=[TOPIC_A, [TOPIC_B, TOPIC_A]]), True),
         (_make_filter_params(topics=[[TOPIC_A], [TOPIC_B, None]]), True),
+        (_make_filter_params(topics=[decode_hex(TOPIC_A)]), True),
+        (_make_filter_params(topics=[decode_hex(TOPIC_A), decode_hex(TOPIC_B)]), True),
+        (_make_filter_params(topics=[decode_hex(TOPIC_A), None]), True),
+        (
+            _make_filter_params(topics=[[decode_hex(TOPIC_A)], [decode_hex(TOPIC_B)]]),
+            True,
+        ),
+        (
+            _make_filter_params(
+                topics=[decode_hex(TOPIC_A), [decode_hex(TOPIC_B), decode_hex(TOPIC_A)]]
+            ),
+            True,
+        ),
+        (
+            _make_filter_params(
+                topics=[[decode_hex(TOPIC_A)], [decode_hex(TOPIC_B), None]]
+            ),
+            True,
+        ),
+        (_make_filter_params(topics=[decode_hex(TOPIC_C)]), False),
+        (_make_filter_params(topics=[decode_hex(TOPIC_D)]), False),
         (_make_filter_params(topics=[ADDRESS_A]), False),
         (_make_filter_params(topics=[ADDRESS_A, TOPIC_B]), False),
         (_make_filter_params(topics=[[ADDRESS_A], [TOPIC_B]]), False),