PR feedback

hwwhww · hwwhww · commit d0f392983a4c · 2018-12-21T17:32:00.000+08:00
diff --git a/eth/_utils/bls.py b/eth/_utils/bls.py
@@ -170,15 +170,18 @@ def privtopub(k: int) -> int:
 
 
 def verify(message: bytes, pubkey: int, signature: bytes, domain: int) -> bool:
-    final_exponentiation = final_exponentiate(
-        pairing(FQP_point_to_FQ2_point(decompress_G2(signature)), G1, False) *
-        pairing(
-            FQP_point_to_FQ2_point(hash_to_G2(message, domain)),
-            neg(decompress_G1(pubkey)),
-            False
+    try:
+        final_exponentiation = final_exponentiate(
+            pairing(FQP_point_to_FQ2_point(decompress_G2(signature)), G1, False) *
+            pairing(
+                FQP_point_to_FQ2_point(hash_to_G2(message, domain)),
+                neg(decompress_G1(pubkey)),
+                False
+            )
         )
-    )
-    return final_exponentiation == FQ12.one()
+        return final_exponentiation == FQ12.one()
+    except (ValidationError, ValueError, AssertionError):
+        return False
 
 
 def aggregate_signatures(signatures: Sequence[bytes]) -> Tuple[int, int]:
@@ -208,16 +211,19 @@ def verify_multiple(pubkeys: Sequence[int],
             )
         )
 
-    o = FQ12([1] + [0] * 11)
-    for m_pubs in set(messages):
-        # aggregate the pubs
-        group_pub = Z1
-        for i in range(len_msgs):
-            if messages[i] == m_pubs:
-                group_pub = add(group_pub, decompress_G1(pubkeys[i]))
-
-        o *= pairing(hash_to_G2(m_pubs, domain), group_pub, False)
-    o *= pairing(decompress_G2(signature), neg(G1), False)
-
-    final_exponentiation = final_exponentiate(o)
-    return final_exponentiation == FQ12.one()
+    try:
+        o = FQ12([1] + [0] * 11)
+        for m_pubs in set(messages):
+            # aggregate the pubs
+            group_pub = Z1
+            for i in range(len_msgs):
+                if messages[i] == m_pubs:
+                    group_pub = add(group_pub, decompress_G1(pubkeys[i]))
+
+            o *= pairing(hash_to_G2(m_pubs, domain), group_pub, False)
+        o *= pairing(decompress_G2(signature), neg(G1), False)
+
+        final_exponentiation = final_exponentiate(o)
+        return final_exponentiation == FQ12.one()
+    except (ValidationError, ValueError, AssertionError):
+        return False
diff --git a/eth/beacon/deposit_helpers.py b/eth/beacon/deposit_helpers.py
@@ -17,7 +17,9 @@
 )
 from eth.beacon.enums import (
     SignatureDomain,
-    ValidatorStatusCode,
+)
+from eth.beacon.exceptions import (
+    MinEmptyValidatorIndexNotFound,
 )
 from eth.beacon.types.deposit_input import DepositInput
 from eth.beacon.types.states import BeaconState
@@ -27,32 +29,32 @@
 )
 
 
-def min_empty_validator_index(validators: Sequence[ValidatorRecord],
-                              current_slot: int,
-                              zero_balance_validator_ttl: int) -> int:
+def get_min_empty_validator_index(validators: Sequence[ValidatorRecord],
+                                  current_slot: int,
+                                  zero_balance_validator_ttl: int) -> int:
     for index, validator in enumerate(validators):
         if (
                 validator.balance == 0 and
                 validator.latest_status_change_slot + zero_balance_validator_ttl <=
                 current_slot
         ):
             return index
-    return None
+    raise MinEmptyValidatorIndexNotFound()
 
 
 def validate_proof_of_possession(state: BeaconState,
                                  pubkey: int,
                                  proof_of_possession: bytes,
                                  withdrawal_credentials: Hash32,
-                                 randao_commitment: Hash32) -> bool:
+                                 randao_commitment: Hash32) -> None:
     deposit_input = DepositInput(
         pubkey=pubkey,
         withdrawal_credentials=withdrawal_credentials,
         randao_commitment=randao_commitment,
         proof_of_possession=EMPTY_SIGNATURE,
     )
 
-    if not bls.verify(
+    is_valid_signature = bls.verify(
         pubkey=pubkey,
         # TODO: change to hash_tree_root(deposit_input) when we have SSZ tree hashing
         message=deposit_input.root,
@@ -61,16 +63,48 @@ def validate_proof_of_possession(state: BeaconState,
             state.fork_data,
             state.slot,
             SignatureDomain.DOMAIN_DEPOSIT,
-        )
-    ):
+        ),
+    )
+
+    if not is_valid_signature:
         raise ValidationError(
             "BLS signature verification error"
         )
 
-    return True
+
+def add_pending_validator(state: BeaconState,
+                          validator: ValidatorRecord,
+                          zero_balance_validator_ttl: int) -> Tuple[BeaconState, int]:
+    """
+    Add a validator to the existing minimum empty validator index or
+    append to ``validator_registry``.
+    """
+    # Check if there's empty validator index in `validator_registry`
+    try:
+        index = get_min_empty_validator_index(
+            state.validator_registry,
+            state.slot,
+            zero_balance_validator_ttl,
+        )
+    except MinEmptyValidatorIndexNotFound:
+        index = None
+
+    if index is None:
+        # Append to the validator_registry
+        validator_registry = state.validator_registry + (validator,)
+        state = state.copy(
+            validator_registry=validator_registry,
+        )
+        index = len(state.validator_registry) - 1
+    else:
+        # Use the empty validator index
+        state = state.update_validator(index, validator)
+
+    return state, index
 
 
-def process_deposit(state: BeaconState,
+def process_deposit(*,
+                    state: BeaconState,
                     pubkey: int,
                     deposit: int,
                     proof_of_possession: bytes,
@@ -90,35 +124,19 @@ def process_deposit(state: BeaconState,
 
     validator_pubkeys = tuple([v.pubkey for v in state.validator_registry])
     if pubkey not in validator_pubkeys:
-        # Add new validator
-        validator = ValidatorRecord(
+        validator = ValidatorRecord.get_pending_validator(
             pubkey=pubkey,
             withdrawal_credentials=withdrawal_credentials,
             randao_commitment=randao_commitment,
-            randao_layers=0,
             balance=deposit,
-            status=ValidatorStatusCode.PENDING_ACTIVATION,
             latest_status_change_slot=state.slot,
-            exit_count=0,
         )
 
-        # Check if there's empty validator index
-        index = min_empty_validator_index(
-            state.validator_registry,
-            state.slot,
+        state, index = add_pending_validator(
+            state,
+            validator,
             zero_balance_validator_ttl,
         )
-        if index is None:
-            # Append to the validator_registry
-            with state.build_changeset() as state_changeset:
-                state_changeset.validator_registry = (
-                    state.validator_registry + (validator,)
-                )
-                state = state_changeset.commit()
-            index = len(state.validator_registry) - 1
-        else:
-            # Use the empty validator index
-            state = state.update_validator(index, validator)
     else:
         # Top-up - increase balance by deposit
         index = validator_pubkeys.index(pubkey)
@@ -134,9 +152,9 @@ def process_deposit(state: BeaconState,
             )
 
         # Update validator's balance and state
-        with validator.build_changeset() as validator_changeset:
-            validator_changeset.balance = validator.balance + deposit
-            validator = validator_changeset.commit()
+        validator = validator.copy(
+            balance=validator.balance + deposit,
+        )
         state = state.update_validator(index, validator)
 
     return state, index
diff --git a/eth/beacon/types/states.py b/eth/beacon/types/states.py
@@ -161,9 +161,9 @@ def num_crosslinks(self) -> int:
     def update_validator(self,
                          validator_index: int,
                          validator: ValidatorRecord) -> 'BeaconState':
-        with self.build_changeset() as state_changeset:
-            validator_registry = list(state_changeset.validator_registry)
-            validator_registry[validator_index] = validator
-            state_changeset.validator_registry = tuple(validator_registry)
-            self = state_changeset.commit()
+        validator_registry = list(self.validator_registry)
+        validator_registry[validator_index] = validator
+        self = self.copy(
+            validator_registry=tuple(validator_registry),
+        )
         return self
diff --git a/eth/beacon/types/validator_records.py b/eth/beacon/types/validator_records.py
@@ -68,3 +68,24 @@ def is_active(self) -> bool:
         Returns ``True`` if the validator is active.
         """
         return self.status in VALIDATOR_RECORD_ACTIVE_STATUSES
+
+    @classmethod
+    def get_pending_validator(cls,
+                              pubkey: int,
+                              withdrawal_credentials: Hash32,
+                              randao_commitment: Hash32,
+                              balance: int,
+                              latest_status_change_slot: int) -> 'ValidatorRecord':
+        """
+        Return a new pending ``ValidatorRecord`` with the given fields.
+        """
+        return cls(
+            pubkey=pubkey,
+            withdrawal_credentials=withdrawal_credentials,
+            randao_commitment=randao_commitment,
+            randao_layers=0,
+            balance=balance,
+            status=ValidatorStatusCode.PENDING_ACTIVATION,
+            latest_status_change_slot=latest_status_change_slot,
+            exit_count=0,
+        )
diff --git a/tests/beacon/test_deposit_helpers.py b/tests/beacon/test_deposit_helpers.py
@@ -10,17 +10,21 @@
 from eth.beacon.constants import (
     EMPTY_SIGNATURE,
 )
+from eth.beacon.deposit_helpers import (
+    add_pending_validator,
+    get_min_empty_validator_index,
+    process_deposit,
+    validate_proof_of_possession,
+)
 from eth.beacon.enums import (
     SignatureDomain,
 )
+from eth.beacon.exceptions import (
+    MinEmptyValidatorIndexNotFound,
+)
 from eth.beacon.helpers import (
     get_domain,
 )
-from eth.beacon.deposit_helpers import (
-    min_empty_validator_index,
-    process_deposit,
-    validate_proof_of_possession,
-)
 from eth.beacon.types.states import BeaconState
 from eth.beacon.types.deposit_input import DepositInput
 from eth.beacon.types.validator_records import ValidatorRecord
@@ -47,31 +51,89 @@ def make_deposit_input(pubkey, withdrawal_credentials, randao_commitment):
     "expected",
     (
         (0, 1, 1, 2, 0),
-        (1, 1, 1, 2, None),  # not (balance == 0)
-        (0, 1, 1, 1, None),  # not (validator.latest_status_change_slot + zero_balance_validator_ttl <= current_slot) # noqa: E501
+        (1, 1, 1, 2, MinEmptyValidatorIndexNotFound()),  # not (balance == 0)
+        (0, 1, 1, 1, MinEmptyValidatorIndexNotFound()),  # not (validator.latest_status_change_slot + zero_balance_validator_ttl <= current_slot) # noqa: E501
     ),
 )
-def test_min_empty_validator_index(sample_validator_record_params,
-                                   balance,
-                                   latest_status_change_slot,
-                                   zero_balance_validator_ttl,
-                                   current_slot,
-                                   expected):
+def test_get_min_empty_validator_index(sample_validator_record_params,
+                                       balance,
+                                       latest_status_change_slot,
+                                       zero_balance_validator_ttl,
+                                       current_slot,
+                                       expected):
     validators = [
         ValidatorRecord(**sample_validator_record_params).copy(
             balance=balance,
             latest_status_change_slot=latest_status_change_slot,
         )
         for _ in range(10)
     ]
+    if isinstance(expected, Exception):
+        with pytest.raises(MinEmptyValidatorIndexNotFound):
+            get_min_empty_validator_index(
+                validators=validators,
+                current_slot=current_slot,
+                zero_balance_validator_ttl=zero_balance_validator_ttl,
+            )
+    else:
+        result = get_min_empty_validator_index(
+            validators=validators,
+            current_slot=current_slot,
+            zero_balance_validator_ttl=zero_balance_validator_ttl,
+        )
+        assert result == expected
 
-    result = min_empty_validator_index(
-        validators=validators,
-        current_slot=current_slot,
-        zero_balance_validator_ttl=zero_balance_validator_ttl,
+
+@pytest.mark.parametrize(
+    "validator_registry_len,"
+    "min_empty_validator_index_result,"
+    "expected_index",
+    (
+        (10, 1, 1),
+        (10, 5, 5),
+        (10, None, 10),
+    ),
+)
+def test_add_pending_validator(monkeypatch,
+                               sample_beacon_state_params,
+                               sample_validator_record_params,
+                               validator_registry_len,
+                               min_empty_validator_index_result,
+                               expected_index):
+    from eth.beacon import deposit_helpers
+
+    def mock_get_min_empty_validator_index(validators,
+                                           current_slot,
+                                           zero_balance_validator_ttl):
+        if min_empty_validator_index_result is None:
+            raise MinEmptyValidatorIndexNotFound()
+        else:
+            return min_empty_validator_index_result
+
+    monkeypatch.setattr(
+        deposit_helpers,
+        'get_min_empty_validator_index',
+        mock_get_min_empty_validator_index
     )
 
-    assert result == expected
+    state = BeaconState(**sample_beacon_state_params).copy(
+        validator_registry=[
+            ValidatorRecord(**sample_validator_record_params).copy(
+                balance=100,
+            )
+            for _ in range(validator_registry_len)
+        ]
+    )
+    validator = ValidatorRecord(**sample_validator_record_params).copy(
+        balance=5566,
+    )
+    state, index = add_pending_validator(
+        state,
+        validator,
+        zero_balance_validator_ttl=0,  # it's for `get_min_empty_validator_index`
+    )
+    assert index == expected_index
+    assert state.validator_registry[index] == validator
 
 
 @pytest.mark.parametrize(
@@ -102,7 +164,7 @@ def test_validate_proof_of_possession(sample_beacon_state_params, pubkeys, privk
     if expected is True:
         proof_of_possession = sign_proof_of_possession(deposit_input, privkey, domain)
 
-        assert validate_proof_of_possession(
+        validate_proof_of_possession(
             state=state,
             pubkey=pubkey,
             proof_of_possession=proof_of_possession,
diff --git a/tests/beacon/types/test_validator_record.py b/tests/beacon/types/test_validator_record.py
