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.

Sign up for GitHub

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

Disallow configuring filters with both manual and generated topic lists #976

Merged
merged 5 commits into from
Aug 2, 2018
Merged

Disallow configuring filters with both manual and generated topic lists #976

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c7807f7
Check in failing test for merging topic lists
dylanjw Jul 30, 2018
f69f30e
Create normalize_topic_list function
dylanjw Jul 30, 2018
bec2e4e
Disallow merging topic lists
dylanjw Jul 30, 2018
0f204ff
Check in topic filtering with options test
dylanjw Aug 1, 2018
4f55e98
Bump eth-tester version to include topic list bugfixes
dylanjw Aug 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions tests/core/filtering/test_contract_createFilter_topic_merging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import pytest


def test_merged_topic_list_event(
web3,
emitter,
emitter_event_ids,
wait_for_transaction):
manual_topics = [
'0xf16c999b533366ca5138d78e85da51611089cd05749f098d6c225d4cd42ee6ec', # event sig
'0x0000000000000000000000000000000000000000000000000000000000000457', # 1111
'0x0000000000000000000000000000000000000000000000000000000000000457', # 1111
'0x0000000000000000000000000000000000000000000000000000000000000457', # 1111
]
with pytest.raises(TypeError):
emitter.events.LogTripleWithIndex().createFilter(
fromBlock="latest",
topics=manual_topics,
argument_filters={'arg0': 2222, 'arg1': 2222, 'arg2': 2222})
5 changes: 2 additions & 3 deletions tests/core/utilities/test_construct_event_filter_params.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@
(EVENT_1_ABI, {}, {
"topics": ['0xb470a829ed7792f06947f0ca3730a570cb378329ddcf09f2b4efabd6326f51f6'],
}),
(EVENT_1_ABI, {'topics': ['should-be-preserved']}, {
(EVENT_1_ABI, {'topics': ['should-overwrite-topics']}, {
"topics": [
['should-be-preserved'],
['0xb470a829ed7792f06947f0ca3730a570cb378329ddcf09f2b4efabd6326f51f6'],
'should-overwrite-topics'
]
}),
(EVENT_1_ABI, {'contract_address': '0xd3CdA913deB6f67967B99D67aCDFa1712C293601'}, {
Expand Down
20 changes: 9 additions & 11 deletions tests/core/utilities/test_construct_event_topics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,48 +31,46 @@ def hex_and_pad(i):
(
EVENT_1_ABI,
{},
[[EVENT_1_TOPIC]],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API change doesn't have any downstream effects in the public API?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shouldn't be a bad effect. Because the topic list permutations were removed, construct_event_topics only needs to return a nested list when there are multiple options for a single argument filter.

There was a bug in how the topic list was being generated. This is an example of a correctly formated topic list to match logs with either of two values for the first indexed event arg:

[ event_sig_topic, [option1_for_arg1, option2_for_arg1], arg2_topic ]

whereas this is what we were generating to be equivalent to the above; we would pass full topic list permutation against each option:

[
    [ event_sig_topic, option1_for_arg1, arg2_topic ],
    [ event_sig_topic, option1_for_arg1, arg2_topic ]
]

This topic topic list actual means: any of [event_sig_topic, option1_for_arg1, arg2_topic ] in the first position and any of [event_sig_topic, option1_for_arg1, arg2_topic] in the second position. Node filters don't understand nested lists as full topic lists. The spec defines a single topic list that can have nested lists of options for each positional element.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can break this part out into another PR, and add a failing test.

This issue has been fixed in the new api: #953. Im fixing this function to make it easier to integrate the new filter builder api.

[EVENT_1_TOPIC],
),
(
EVENT_1_ABI,
{'arg0': 1},
[[EVENT_1_TOPIC]],
[EVENT_1_TOPIC],
),
(
EVENT_1_ABI,
{'arg0': 1, 'arg3': [1, 2]},
[[EVENT_1_TOPIC]],
[EVENT_1_TOPIC],
),
(
EVENT_1_ABI,
{'arg1': 1},
[
[EVENT_1_TOPIC, hex_and_pad(1), None, None],
EVENT_1_TOPIC, hex_and_pad(1)
],
),
(
EVENT_1_ABI,
{'arg1': [1, 2]},
[
[EVENT_1_TOPIC, hex_and_pad(1), None, None],
[EVENT_1_TOPIC, hex_and_pad(2), None, None],
EVENT_1_TOPIC, [hex_and_pad(1), hex_and_pad(2)],
],
),
(
EVENT_1_ABI,
{'arg1': [1], 'arg2': [2]},
[
[EVENT_1_TOPIC, hex_and_pad(1), hex_and_pad(2), None],
EVENT_1_TOPIC, hex_and_pad(1), hex_and_pad(2),
],
),
(
EVENT_1_ABI,
{'arg1': [1, 3], 'arg2': [2, 4]},
[
[EVENT_1_TOPIC, hex_and_pad(1), hex_and_pad(2), None],
[EVENT_1_TOPIC, hex_and_pad(1), hex_and_pad(4), None],
[EVENT_1_TOPIC, hex_and_pad(3), hex_and_pad(2), None],
[EVENT_1_TOPIC, hex_and_pad(3), hex_and_pad(4), None],
EVENT_1_TOPIC,
[hex_and_pad(1), hex_and_pad(3)],
[hex_and_pad(2), hex_and_pad(4)]
],
),
)
Expand Down
29 changes: 23 additions & 6 deletions web3/utils/events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
from web3.utils.normalizers import (
BASE_RETURN_NORMALIZERS,
)
from web3.utils.toolz import (
compose,
curry,
)

from .abi import (
exclude_indexed_event_inputs,
Expand Down Expand Up @@ -71,12 +75,7 @@ def construct_event_topic_set(event_abi, arguments=None):
for arg, arg_options in zipped_abi_and_args
]

topics = [
[event_topic] + list(permutation)
if any(value is not None for value in permutation)
else [event_topic]
for permutation in itertools.product(*encoded_args)
]
topics = list(normalize_topic_list([event_topic] + encoded_args))
return topics


Expand Down Expand Up @@ -221,3 +220,21 @@ def get_event_data(event_abi, log_entry):
}

return AttributeDict.recursive(event_data)


@to_tuple
def pop_singlets(seq):
yield from (i[0] if is_list_like(i) and len(i) == 1 else i for i in seq)


@curry
def remove_trailing_from_seq(seq, remove_value=None):
index = len(seq)
while index > 0 and seq[index - 1] == remove_value:
index -= 1
return seq[:index]


normalize_topic_list = compose(
remove_trailing_from_seq(remove_value=None),
pop_singlets,)
13 changes: 8 additions & 5 deletions web3/utils/filters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,14 @@ def construct_event_filter_params(event_abi,
toBlock=None,
address=None):
filter_params = {}

if topics is None:
topic_set = construct_event_topic_set(event_abi, argument_filters)
else:
topic_set = [topics] + construct_event_topic_set(event_abi, argument_filters)
topic_set = construct_event_topic_set(event_abi, argument_filters)

if topics is not None:
if len(topic_set) > 1:
raise TypeError(
"Merging the topics argument with topics generated "
"from argument_filters is not supported.")
topic_set = topics

if len(topic_set) == 1 and is_list_like(topic_set[0]):
filter_params['topics'] = topic_set[0]
Expand Down