From d8c788575363069ee7202c04f0b73ddfa6c5b794 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Sat, 5 Jan 2019 12:55:52 -0600 Subject: [PATCH 1/7] Increase safety of code by avoiding mutation --- tests/beacon/test_helpers.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/tests/beacon/test_helpers.py b/tests/beacon/test_helpers.py index b7e218b781..9a0c7ff26e 100644 --- a/tests/beacon/test_helpers.py +++ b/tests/beacon/test_helpers.py @@ -3,6 +3,8 @@ import pytest import random +import cytoolz + from hypothesis import ( given, strategies as st, @@ -60,6 +62,10 @@ ) +class UnreachableCodePathError(Exception): + pass + + @pytest.fixture() def sample_block(sample_beacon_block_params): return SerenityBeaconBlock(**sample_beacon_block_params) @@ -822,22 +828,27 @@ def _correct_slashable_vote_data_params(params, validators, messages, privkeys): def _corrupt_signature(params): - params = copy.deepcopy(params) message = bytes.fromhex("deadbeefcafe") privkey = 42 domain = SignatureDomain.DOMAIN_ATTESTATION - params["aggregate_signature"] = bls.sign(message, privkey, domain) - return params + corrupt_signature = bls.sign(message, privkey, domain) + + return cytoolz.assoc(params, "aggregate_signature", corrupt_signature) def _corrupt_vote_count(params): - params = copy.deepcopy(params) key = "custody_bit_0_indices" for i in itertools.count(): if i not in params[key]: - params[key].append(i) - break - return params + new_vote_count = params[key] + [i] + return cytoolz.assoc( + params, + key, + new_vote_count, + ) + else: + msg = "list of ``custody_bit_0_indices`` should not exhaust ``itertools.count``" + raise UnreachableCodePathError(msg) def _create_slashable_vote_data_messages(params): From 9fe6b38d21f2e81c2d80acdb1423c51f9ff4734a Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Sat, 5 Jan 2019 12:10:42 -0600 Subject: [PATCH 2/7] Fixes bug with calculating the domain for the BLS signature The previous implementation was using the `domain_type` as the `domain` when it should in fact be the value calculated by `get_domain`. --- tests/beacon/test_helpers.py | 55 +++++++++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 14 deletions(-) diff --git a/tests/beacon/test_helpers.py b/tests/beacon/test_helpers.py index 9a0c7ff26e..526f4463e8 100644 --- a/tests/beacon/test_helpers.py +++ b/tests/beacon/test_helpers.py @@ -785,19 +785,24 @@ def test_verify_vote_count(max_casper_votes, sample_slashable_vote_data_params, assert verify_vote_count(votes, max_casper_votes) -def _get_indices_and_signatures(num_validators, message, privkeys): +def _get_indices_and_signatures(num_validators, message, privkeys, fork_data, slot): num_indices = 5 assert num_validators >= num_indices indices = random.sample(range(num_validators), num_indices) privkeys = [privkeys[i] for i in indices] - domain = SignatureDomain.DOMAIN_ATTESTATION + domain_type = SignatureDomain.DOMAIN_ATTESTATION + domain = get_domain( + fork_data=fork_data, + slot=slot, + domain_type=domain_type, + ) signatures = tuple( map(lambda key: bls.sign(message, key, domain), privkeys) ) return (indices, signatures) -def _correct_slashable_vote_data_params(params, validators, messages, privkeys): +def _correct_slashable_vote_data_params(params, validators, messages, privkeys, fork_data): valid_params = copy.deepcopy(params) num_validators = len(validators) @@ -807,6 +812,8 @@ def _correct_slashable_vote_data_params(params, validators, messages, privkeys): num_validators, messages[0], privkeys, + fork_data, + params["data"].slot, ) valid_params[key] = poc_0_indices @@ -816,6 +823,8 @@ def _correct_slashable_vote_data_params(params, validators, messages, privkeys): num_validators, messages[1], privkeys, + fork_data, + params["data"].slot, ) valid_params[key] = poc_1_indices @@ -827,10 +836,15 @@ def _correct_slashable_vote_data_params(params, validators, messages, privkeys): return valid_params -def _corrupt_signature(params): +def _corrupt_signature(params, fork_data): message = bytes.fromhex("deadbeefcafe") privkey = 42 - domain = SignatureDomain.DOMAIN_ATTESTATION + domain_type = SignatureDomain.DOMAIN_ATTESTATION + domain = get_domain( + fork_data=fork_data, + slot=params["data"].slot, + domain_type=domain_type, + ) corrupt_signature = bls.sign(message, privkey, domain) return cytoolz.assoc(params, "aggregate_signature", corrupt_signature) @@ -860,7 +874,8 @@ def _create_slashable_vote_data_messages(params): def test_verify_slashable_vote_data_signature(privkeys, sample_beacon_state_params, genesis_validators, - sample_slashable_vote_data_params): + sample_slashable_vote_data_params, + sample_fork_data_params): sample_beacon_state_params["validator_registry"] = genesis_validators state = BeaconState(**sample_beacon_state_params) @@ -868,16 +883,18 @@ def test_verify_slashable_vote_data_signature(privkeys, # touch disjoint subsets of the provided params messages = _create_slashable_vote_data_messages(sample_slashable_vote_data_params) + fork_data = ForkData(**sample_fork_data_params) valid_params = _correct_slashable_vote_data_params( sample_slashable_vote_data_params, genesis_validators, messages, privkeys, + fork_data, ) valid_votes = SlashableVoteData(**valid_params) assert verify_slashable_vote_data_signature(state, valid_votes) - invalid_params = _corrupt_signature(valid_params) + invalid_params = _corrupt_signature(valid_params, fork_data) invalid_votes = SlashableVoteData(**invalid_params) assert not verify_slashable_vote_data_signature(state, invalid_votes) @@ -893,22 +910,27 @@ def _run_verify_slashable_vote(params, state, max_casper_votes, should_succeed): @pytest.mark.parametrize( ( - 'param_mapper,' - 'should_succeed' + 'param_mapper', + 'should_succeed', + 'needs_fork_data', ), [ - (lambda params: params, True), - (lambda params: _corrupt_vote_count(params), False), - (lambda params: _corrupt_signature(params), False), - (lambda params: _corrupt_vote_count(_corrupt_signature(params)), False), + (lambda params: params, True, False), + (_corrupt_vote_count, False, False), + (_corrupt_signature, False, True), + (lambda params, fork_data: _corrupt_vote_count( + _corrupt_signature(params, fork_data) + ), False, True), ], ) def test_verify_slashable_vote_data(param_mapper, should_succeed, + needs_fork_data, privkeys, sample_beacon_state_params, genesis_validators, sample_slashable_vote_data_params, + sample_fork_data_params, max_casper_votes): sample_beacon_state_params["validator_registry"] = genesis_validators state = BeaconState(**sample_beacon_state_params) @@ -917,13 +939,18 @@ def test_verify_slashable_vote_data(param_mapper, # touch disjoint subsets of the provided params messages = _create_slashable_vote_data_messages(sample_slashable_vote_data_params) + fork_data = ForkData(**sample_fork_data_params) params = _correct_slashable_vote_data_params( sample_slashable_vote_data_params, genesis_validators, messages, privkeys, + fork_data, ) - params = param_mapper(params) + if needs_fork_data: + params = param_mapper(params, fork_data) + else: + params = param_mapper(params) _run_verify_slashable_vote(params, state, max_casper_votes, should_succeed) From 3fd91a107c6725c084a0b40892502b20b9dfa74a Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Mon, 7 Jan 2019 10:43:48 -0600 Subject: [PATCH 3/7] Remove specialized exception in favor of built-in type --- tests/beacon/test_helpers.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/beacon/test_helpers.py b/tests/beacon/test_helpers.py index 526f4463e8..4de9853881 100644 --- a/tests/beacon/test_helpers.py +++ b/tests/beacon/test_helpers.py @@ -62,10 +62,6 @@ ) -class UnreachableCodePathError(Exception): - pass - - @pytest.fixture() def sample_block(sample_beacon_block_params): return SerenityBeaconBlock(**sample_beacon_block_params) @@ -861,8 +857,7 @@ def _corrupt_vote_count(params): new_vote_count, ) else: - msg = "list of ``custody_bit_0_indices`` should not exhaust ``itertools.count``" - raise UnreachableCodePathError(msg) + raise Exception("Unreachable code path") def _create_slashable_vote_data_messages(params): From 79720659005080b0746d7b187bc7a59c39791aaf Mon Sep 17 00:00:00 2001 From: Piper Merriam Date: Mon, 7 Jan 2019 12:02:22 -0700 Subject: [PATCH 4/7] Use eth_utils for cytoolz import. --- tests/beacon/test_helpers.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/beacon/test_helpers.py b/tests/beacon/test_helpers.py index 4de9853881..8cd9401b17 100644 --- a/tests/beacon/test_helpers.py +++ b/tests/beacon/test_helpers.py @@ -3,8 +3,6 @@ import pytest import random -import cytoolz - from hypothesis import ( given, strategies as st, @@ -14,6 +12,7 @@ denoms, ValidationError, ) +from eth_utils.toolz import assoc from eth.constants import ( ZERO_HASH32, @@ -843,7 +842,7 @@ def _corrupt_signature(params, fork_data): ) corrupt_signature = bls.sign(message, privkey, domain) - return cytoolz.assoc(params, "aggregate_signature", corrupt_signature) + return assoc(params, "aggregate_signature", corrupt_signature) def _corrupt_vote_count(params): @@ -851,7 +850,7 @@ def _corrupt_vote_count(params): for i in itertools.count(): if i not in params[key]: new_vote_count = params[key] + [i] - return cytoolz.assoc( + return assoc( params, key, new_vote_count, From 06ec7960c9ddea07894ffb4ec1727f48ee7a3eb8 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Wed, 9 Jan 2019 12:53:02 -0600 Subject: [PATCH 5/7] Avoid mutation on fixture params --- tests/beacon/test_helpers.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/tests/beacon/test_helpers.py b/tests/beacon/test_helpers.py index 8cd9401b17..98636adfaa 100644 --- a/tests/beacon/test_helpers.py +++ b/tests/beacon/test_helpers.py @@ -870,8 +870,17 @@ def test_verify_slashable_vote_data_signature(privkeys, genesis_validators, sample_slashable_vote_data_params, sample_fork_data_params): - sample_beacon_state_params["validator_registry"] = genesis_validators - state = BeaconState(**sample_beacon_state_params) + beacon_state_params_with_genesis_validators = assoc( + sample_beacon_state_params, + "validator_registry", + genesis_validators, + ) + beacon_state_params_with_fork_data = assoc( + beacon_state_params_with_genesis_validators, + "fork_data", + ForkData(**sample_fork_data_params), + ) + state = BeaconState(**beacon_state_params_with_fork_data) # NOTE: we can do this before "correcting" the params as they # touch disjoint subsets of the provided params @@ -926,8 +935,17 @@ def test_verify_slashable_vote_data(param_mapper, sample_slashable_vote_data_params, sample_fork_data_params, max_casper_votes): - sample_beacon_state_params["validator_registry"] = genesis_validators - state = BeaconState(**sample_beacon_state_params) + beacon_state_params_with_genesis_validators = assoc( + sample_beacon_state_params, + "validator_registry", + genesis_validators, + ) + beacon_state_params_with_fork_data = assoc( + beacon_state_params_with_genesis_validators, + "fork_data", + ForkData(**sample_fork_data_params), + ) + state = BeaconState(**beacon_state_params_with_fork_data) # NOTE: we can do this before "correcting" the params as they # touch disjoint subsets of the provided params From e15e3e662067d6ae8c219b9fb3ecfb1039949ff4 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Wed, 9 Jan 2019 12:54:34 -0600 Subject: [PATCH 6/7] Ensure a target number of validators rather than rely on the fixture --- tests/beacon/test_helpers.py | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/tests/beacon/test_helpers.py b/tests/beacon/test_helpers.py index 98636adfaa..e43e6fb649 100644 --- a/tests/beacon/test_helpers.py +++ b/tests/beacon/test_helpers.py @@ -797,11 +797,9 @@ def _get_indices_and_signatures(num_validators, message, privkeys, fork_data, sl return (indices, signatures) -def _correct_slashable_vote_data_params(params, validators, messages, privkeys, fork_data): +def _correct_slashable_vote_data_params(num_validators, params, messages, privkeys, fork_data): valid_params = copy.deepcopy(params) - num_validators = len(validators) - key = "custody_bit_0_indices" (poc_0_indices, poc_0_signatures) = _get_indices_and_signatures( num_validators, @@ -865,7 +863,16 @@ def _create_slashable_vote_data_messages(params): return votes.messages -def test_verify_slashable_vote_data_signature(privkeys, +@pytest.mark.parametrize( + ( + 'num_validators', + ), + [ + (40,), + ] +) +def test_verify_slashable_vote_data_signature(num_validators, + privkeys, sample_beacon_state_params, genesis_validators, sample_slashable_vote_data_params, @@ -888,8 +895,8 @@ def test_verify_slashable_vote_data_signature(privkeys, fork_data = ForkData(**sample_fork_data_params) valid_params = _correct_slashable_vote_data_params( + num_validators, sample_slashable_vote_data_params, - genesis_validators, messages, privkeys, fork_data, @@ -911,6 +918,14 @@ def _run_verify_slashable_vote(params, state, max_casper_votes, should_succeed): assert not result +@pytest.mark.parametrize( + ( + 'num_validators', + ), + [ + (40,), + ] +) @pytest.mark.parametrize( ( 'param_mapper', @@ -926,7 +941,8 @@ def _run_verify_slashable_vote(params, state, max_casper_votes, should_succeed): ), False, True), ], ) -def test_verify_slashable_vote_data(param_mapper, +def test_verify_slashable_vote_data(num_validators, + param_mapper, should_succeed, needs_fork_data, privkeys, @@ -953,8 +969,8 @@ def test_verify_slashable_vote_data(param_mapper, fork_data = ForkData(**sample_fork_data_params) params = _correct_slashable_vote_data_params( + num_validators, sample_slashable_vote_data_params, - genesis_validators, messages, privkeys, fork_data, From 569849cd60d734c95be81dbcefce2740fc415c5c Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Wed, 9 Jan 2019 12:55:55 -0600 Subject: [PATCH 7/7] Use fork data from the state rather than derived from the params This change is more in line with how this functionality will ultimately be used so we want to route our query through the `BeaconState`. --- tests/beacon/test_helpers.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/beacon/test_helpers.py b/tests/beacon/test_helpers.py index e43e6fb649..6ed9c245e0 100644 --- a/tests/beacon/test_helpers.py +++ b/tests/beacon/test_helpers.py @@ -893,18 +893,17 @@ def test_verify_slashable_vote_data_signature(num_validators, # touch disjoint subsets of the provided params messages = _create_slashable_vote_data_messages(sample_slashable_vote_data_params) - fork_data = ForkData(**sample_fork_data_params) valid_params = _correct_slashable_vote_data_params( num_validators, sample_slashable_vote_data_params, messages, privkeys, - fork_data, + state.fork_data, ) valid_votes = SlashableVoteData(**valid_params) assert verify_slashable_vote_data_signature(state, valid_votes) - invalid_params = _corrupt_signature(valid_params, fork_data) + invalid_params = _corrupt_signature(valid_params, state.fork_data) invalid_votes = SlashableVoteData(**invalid_params) assert not verify_slashable_vote_data_signature(state, invalid_votes) @@ -967,16 +966,15 @@ def test_verify_slashable_vote_data(num_validators, # touch disjoint subsets of the provided params messages = _create_slashable_vote_data_messages(sample_slashable_vote_data_params) - fork_data = ForkData(**sample_fork_data_params) params = _correct_slashable_vote_data_params( num_validators, sample_slashable_vote_data_params, messages, privkeys, - fork_data, + state.fork_data, ) if needs_fork_data: - params = param_mapper(params, fork_data) + params = param_mapper(params, state.fork_data) else: params = param_mapper(params) _run_verify_slashable_vote(params, state, max_casper_votes, should_succeed)