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.

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

node independent filter middleware #732

Merged
merged 11 commits into from
Sep 27, 2018
10 changes: 9 additions & 1 deletion tests/core/filtering/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,15 @@
)


@pytest.fixture(params=[True, False], ids=["local_filter_middleware", "node_based_filter"])
@pytest.fixture()
def tester_snapshot(web3):
return web3.providers[0].ethereum_tester.take_snapshot()


@pytest.fixture(
scope='function',
params=[True, False],
ids=["local_filter_middleware", "node_based_filter"])
def web3(request):
use_filter_middleware = request.param
provider = EthereumTesterProvider()
Expand Down
14 changes: 14 additions & 0 deletions tests/core/filtering/test_contract_data_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ def array_values(draw):
return (matching, non_matching)


def clear_chain_state(web3, snapshot):
"""Clear chain state
Hypothesis doesn't allow function scoped fixtures to re-run between test runs
so chain state needs to be explicitly cleared
"""
web3.providers[0].ethereum_tester.revert_to_snapshot(snapshot)


@pytest.mark.parametrize('api_style', ('v4', 'build_filter'))
@given(vals=dynamic_values())
@settings(max_examples=5, deadline=None)
Expand All @@ -65,7 +73,9 @@ def test_data_filters_with_dynamic_arguments(
create_filter,
emitter,
api_style,
tester_snapshot,
vals):
clear_chain_state(web3, tester_snapshot)

if api_style == 'build_filter':
filter_builder = emitter.events.LogDynamicArgs.build_filter()
Expand Down Expand Up @@ -104,7 +114,9 @@ def test_data_filters_with_fixed_arguments(
create_filter,
api_style,
vals,
tester_snapshot,
request):
clear_chain_state(web3, tester_snapshot)

if api_style == 'build_filter':
filter_builder = emitter.events.LogQuadrupleArg.build_filter()
Expand Down Expand Up @@ -156,7 +168,9 @@ def test_data_filters_with_list_arguments(
create_filter,
api_style,
vals,
tester_snapshot,
request):
clear_chain_state(web3, tester_snapshot)

matching, non_matching = vals

Expand Down
14 changes: 14 additions & 0 deletions tests/core/filtering/test_contract_topic_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ def array_values(draw):
return (matching, non_matching)


def clear_chain_state(web3, snapshot):
"""Clear chain state
Hypothesis doesn't allow function scoped fixtures to re-run between test runs
so chain state needs to be explicitly cleared
"""
web3.providers[0].ethereum_tester.revert_to_snapshot(snapshot)


@pytest.mark.parametrize('api_style', ('v4', 'build_filter'))
@given(vals=dynamic_values())
@settings(max_examples=5, deadline=None)
Expand All @@ -68,7 +76,9 @@ def test_topic_filters_with_dynamic_arguments(
emitter_event_ids,
create_filter,
api_style,
tester_snapshot,
vals):
clear_chain_state(web3, tester_snapshot)

if api_style == 'build_filter':
filter_builder = emitter.events.LogDynamicArgs.build_filter()
Expand Down Expand Up @@ -110,7 +120,9 @@ def test_topic_filters_with_fixed_arguments(
call_as_instance,
create_filter,
api_style,
tester_snapshot,
vals):
clear_chain_state(web3, tester_snapshot)

if api_style == 'build_filter':
filter_builder = emitter.events.LogQuadrupleWithIndex.build_filter()
Expand Down Expand Up @@ -162,7 +174,9 @@ def test_topic_filters_with_list_arguments(
call_as_instance,
create_filter,
api_style,
tester_snapshot,
vals):
clear_chain_state(web3, tester_snapshot)

matching, non_matching = vals

Expand Down
16 changes: 16 additions & 0 deletions tests/core/filtering/test_filter_reaches_every_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,19 @@ def test_filtering_sequential_blocks(
emitter.functions.logNoArgs(which=1).transact()
assert web3.eth.blockNumber == initial_block_number + 100
assert len(filter_.get_new_entries()) == 100


def test_filtering_starting_block_range(
web3,
emitter,
Emitter,
wait_for_transaction):
for i in range(10):
emitter.functions.logNoArgs(which=1).transact()
builder = emitter.events.LogNoArguments.build_filter()
filter_ = builder.deploy(web3)
initial_block_number = web3.eth.blockNumber
for i in range(10):
emitter.functions.logNoArgs(which=1).transact()
assert web3.eth.blockNumber == initial_block_number + 10
assert len(filter_.get_new_entries()) == 10
4 changes: 4 additions & 0 deletions tests/core/middleware/test_filter_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ def w3(w3_base, result_generator_middleware):


@pytest.mark.parametrize("start, stop, expected", [
(2, 7, [
(2, 6),
(7, 7)
]),
(0, 12, [
(0, 4),
(5, 9),
Expand Down
8 changes: 4 additions & 4 deletions web3/middleware/filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ def gen_bounded_segments(start, stop, step):
yield (start, stop)
return
for segment in zip(
range(start, stop - step, step),
range(start + step, stop, step)):
range(start, stop - step + 1, step),
range(start + step, stop + 1, step)):
yield segment
else:
Copy link
Collaborator

@carver carver Sep 26, 2018

Choose a reason for hiding this comment

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

This doesn't need to be in an else clause right? Seems like it should always run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its a for/else loop, so it runs at the end of the for loop. It would work the same to remove the else clause, but probably safer to keep it in case anything is added to this method.

Copy link
Member

Choose a reason for hiding this comment

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

for/else is typically reserved for loops that have a break somewhere in them, in order to only run the else in whatever case the break does not get triggered (or inversely, having the ability to break, thus skipping the else code.

remainder = (stop - start) % step
Expand Down Expand Up @@ -195,7 +195,7 @@ def __init__(
self.topics = topics
self.web3 = web3
Copy link
Member

Choose a reason for hiding this comment

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

We've been using w3 as the variable name for a Web3 instance. Maybe switch to use this convention.

if from_block is None or from_block == 'latest':
self._from_block = web3.eth.blockNumber
self._from_block = web3.eth.blockNumber + 1
else:
self._from_block = from_block
self._to_block = to_block
Expand Down Expand Up @@ -228,7 +228,7 @@ def to_block(self):
return to_block

def _get_filter_changes(self):
for start, stop in iter_latest_block_ranges(self.web3, self.from_block, self._to_block):
for start, stop in iter_latest_block_ranges(self.web3, self._from_block, self._to_block):
if None in (start, stop):
yield []

Expand Down