From d559f4ee89ad408a4e7b86d4192de5ca25abdab6 Mon Sep 17 00:00:00 2001
From: Dustin Brody <tersec@users.noreply.github.com>
Date: Wed, 15 Apr 2020 11:01:36 +0200
Subject: [PATCH] create distinct CommitteeIndex type

---
 beacon_chain/attestation_aggregation.nim         | 13 +++++--------
 beacon_chain/beacon_node.nim                     | 10 +++++-----
 beacon_chain/spec/beaconstate.nim                |  6 ++++--
 beacon_chain/spec/datatypes.nim                  |  4 ++++
 beacon_chain/spec/helpers.nim                    |  5 +++++
 beacon_chain/spec/validator.nim                  | 16 ++++++++++------
 research/state_sim.nim                           |  2 +-
 tests/mocking/mock_attestations.nim              |  4 ++--
 .../justification_finalization_helpers.nim       |  2 +-
 tests/test_attestation_pool.nim                  | 14 +++++++-------
 tests/test_state_transition.nim                  |  2 +-
 tests/testblockutil.nim                          |  5 +++--
 12 files changed, 48 insertions(+), 35 deletions(-)

diff --git a/beacon_chain/attestation_aggregation.nim b/beacon_chain/attestation_aggregation.nim
index 64e0d24f5..87bbe8ea8 100644
--- a/beacon_chain/attestation_aggregation.nim
+++ b/beacon_chain/attestation_aggregation.nim
@@ -12,9 +12,8 @@ import
   ./attestation_pool, ./beacon_node_types, ./ssz
 
 # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/validator.md#aggregation-selection
-func is_aggregator(state: BeaconState, slot: Slot, index: uint64,
+func is_aggregator(state: BeaconState, slot: Slot, index: CommitteeIndex,
     slot_signature: ValidatorSig): bool =
-  # TODO index is a CommitteeIndex, aka uint64
   var cache = get_empty_per_epoch_cache()
 
   let
@@ -23,10 +22,8 @@ func is_aggregator(state: BeaconState, slot: Slot, index: uint64,
   bytes_to_int(eth2hash(slot_signature.toRaw()).data[0..7]) mod modulo == 0
 
 proc aggregate_attestations*(
-    pool: AttestationPool, state: BeaconState, index: uint64,
+    pool: AttestationPool, state: BeaconState, index: CommitteeIndex,
     privkey: ValidatorPrivKey, trailing_distance: uint64): Option[AggregateAndProof] =
-  # TODO alias CommitteeIndex to actual type then convert various uint64's here
-
   doAssert state.slot >= trailing_distance
 
   # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#configuration
@@ -41,7 +38,7 @@ proc aggregate_attestations*(
   doAssert state.slot >= slot
 
   # TODO performance issue for future, via get_active_validator_indices(...)
-  doAssert index < get_committee_count_at_slot(state, slot)
+  doAssert index.uint64 < get_committee_count_at_slot(state, slot)
 
   # TODO for testing purposes, refactor this into the condition check
   # and just calculation
@@ -54,7 +51,7 @@ proc aggregate_attestations*(
   # TODO this won't actually match anything
   let attestation_data = AttestationData(
     slot: slot,
-    index: index,
+    index: index.uint64,
     beacon_block_root: get_block_root_at_slot(state, slot))
 
   # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/validator.md#construct-aggregate
@@ -65,7 +62,7 @@ proc aggregate_attestations*(
     if attestation.data == attestation_data:
       # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/validator.md#aggregateandproof
       return some(AggregateAndProof(
-        aggregator_index: index,
+        aggregator_index: index.uint64,
         aggregate: attestation,
         selection_proof: slot_signature))
 
diff --git a/beacon_chain/beacon_node.nim b/beacon_chain/beacon_node.nim
index 0268ec0b9..ff58e59c9 100644
--- a/beacon_chain/beacon_node.nim
+++ b/beacon_chain/beacon_node.nim
@@ -546,8 +546,8 @@ proc handleAttestations(node: BeaconNode, head: BlockRef, slot: Slot) =
     let committees_per_slot = get_committee_count_at_slot(state, slot)
 
     for committee_index in 0'u64..<committees_per_slot:
-      let
-        committee = get_beacon_committee(state, slot, committee_index, cache)
+      let committee = get_beacon_committee(
+        state, slot, committee_index.CommitteeIndex, cache)
 
       for index_in_committee, validatorIdx in committee:
         let validator = node.getAttachedValidator(state, validatorIdx)
@@ -617,8 +617,8 @@ proc broadcastAggregatedAttestations(
     committees_per_slot = get_committee_count_at_slot(state, slot)
   var cache = get_empty_per_epoch_cache()
   for committee_index in 0'u64..<committees_per_slot:
-    let
-      committee = get_beacon_committee(state, slot, committee_index, cache)
+    let committee = get_beacon_committee(
+      state, slot, committee_index.CommitteeIndex, cache)
 
     for index_in_committee, validatorIdx in committee:
       let validator = node.getAttachedValidator(state, validatorIdx)
@@ -629,7 +629,7 @@ proc broadcastAggregatedAttestations(
         # one isSome() with test.
         let option_aggregateandproof =
           aggregate_attestations(node.attestationPool, state,
-            committee_index,
+            committee_index.CommitteeIndex,
             # TODO https://github.com/status-im/nim-beacon-chain/issues/545
             # this assumes in-process private keys
             validator.privKey,
diff --git a/beacon_chain/spec/beaconstate.nim b/beacon_chain/spec/beaconstate.nim
index de12e1399..fefcefb9f 100644
--- a/beacon_chain/spec/beaconstate.nim
+++ b/beacon_chain/spec/beaconstate.nim
@@ -420,7 +420,8 @@ func get_attesting_indices*(state: BeaconState,
                             HashSet[ValidatorIndex] =
   # Return the set of attesting indices corresponding to ``data`` and ``bits``.
   result = initHashSet[ValidatorIndex]()
-  let committee = get_beacon_committee(state, data.slot, data.index, stateCache)
+  let committee = get_beacon_committee(
+    state, data.slot, data.index.CommitteeIndex, stateCache)
   for i, index in committee:
     if bits[i]:
       result.incl index
@@ -485,7 +486,8 @@ proc check_attestation*(
       state_slot = shortLog(stateSlot))
     return
 
-  let committee = get_beacon_committee(state, data.slot, data.index, stateCache)
+  let committee = get_beacon_committee(
+    state, data.slot, data.index.CommitteeIndex, stateCache)
   if attestation.aggregation_bits.len != committee.len:
     warn("Inconsistent aggregation and committee length",
       aggregation_bits_len = attestation.aggregation_bits.len,
diff --git a/beacon_chain/spec/datatypes.nim b/beacon_chain/spec/datatypes.nim
index d35bf0f2d..22da8542b 100644
--- a/beacon_chain/spec/datatypes.nim
+++ b/beacon_chain/spec/datatypes.nim
@@ -112,6 +112,7 @@ type
   # range-limit.
   ValidatorIndex* = distinct uint32
   Gwei* = uint64
+  CommitteeIndex* = distinct uint64
 
   BitList*[maxLen: static int] = distinct BitSeq
 
@@ -154,6 +155,9 @@ type
   # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/beacon-chain.md#AttestationData
   AttestationData* = object
     slot*: Slot
+
+    # TODO this is actually a CommitteeIndex; remove some conversions by
+    # allowing SSZ to directly handle this
     index*: uint64
 
     # LMD GHOST vote
diff --git a/beacon_chain/spec/helpers.nim b/beacon_chain/spec/helpers.nim
index 11e09a3dd..c6fac1f09 100644
--- a/beacon_chain/spec/helpers.nim
+++ b/beacon_chain/spec/helpers.nim
@@ -57,6 +57,11 @@ func get_active_validator_indices*(state: BeaconState, epoch: Epoch):
 # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/beacon-chain.md#get_committee_count_at_slot
 func get_committee_count_at_slot*(state: BeaconState, slot: Slot): uint64 =
   # Return the number of committees at ``slot``.
+
+  # TODO this is mostly used in for loops which have indexes which then need to
+  # be converted to CommitteeIndex types for get_beacon_committee(...); replace
+  # with better and more type-safe use pattern, probably beginning with using a
+  # CommitteeIndex return type here.
   let epoch = compute_epoch_at_slot(slot)
   let active_validator_indices = get_active_validator_indices(state, epoch)
   let committees_per_slot = clamp(
diff --git a/beacon_chain/spec/validator.nim b/beacon_chain/spec/validator.nim
index f6fa37eef..c786e11ef 100644
--- a/beacon_chain/spec/validator.nim
+++ b/beacon_chain/spec/validator.nim
@@ -112,7 +112,9 @@ func compute_committee(indices: seq[ValidatorIndex], seed: Eth2Digest,
     indices[stateCache.beacon_committee_cache[key][it]])
 
 # https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/specs/phase0/beacon-chain.md#get_beacon_committee
-func get_beacon_committee*(state: BeaconState, slot: Slot, index: uint64, cache: var StateCache): seq[ValidatorIndex] =
+func get_beacon_committee*(
+    state: BeaconState, slot: Slot, index: CommitteeIndex,
+    cache: var StateCache): seq[ValidatorIndex] =
   # Return the beacon committee at ``slot`` for ``index``.
   let
     epoch = compute_epoch_at_slot(slot)
@@ -132,7 +134,8 @@ func get_beacon_committee*(state: BeaconState, slot: Slot, index: uint64, cache:
   compute_committee(
     cache.active_validator_indices_cache[epoch],
     get_seed(state, epoch, DOMAIN_BEACON_ATTESTER),
-    (slot mod SLOTS_PER_EPOCH) * cache.committee_count_cache[epoch] + index,
+    (slot mod SLOTS_PER_EPOCH) * cache.committee_count_cache[epoch] +
+      index.uint64,
     cache.committee_count_cache[epoch] * SLOTS_PER_EPOCH,
     cache
   )
@@ -197,7 +200,7 @@ func get_beacon_proposer_index*(state: BeaconState, stateCache: var StateCache):
 # https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/specs/phase0/validator.md#validator-assignments
 func get_committee_assignment(
     state: BeaconState, epoch: Epoch, validator_index: ValidatorIndex):
-    Option[tuple[a: seq[ValidatorIndex], b: uint64, c: Slot]] {.used.} =
+    Option[tuple[a: seq[ValidatorIndex], b: CommitteeIndex, c: Slot]] {.used.} =
   # Return the committee assignment in the ``epoch`` for ``validator_index``.
   # ``assignment`` returned is a tuple of the following form:
   #     * ``assignment[0]`` is the list of validators in the committee
@@ -212,11 +215,12 @@ func get_committee_assignment(
   let start_slot = compute_start_slot_at_epoch(epoch)
   for slot in start_slot ..< start_slot + SLOTS_PER_EPOCH:
     for index in 0 ..< get_committee_count_at_slot(state, slot):
+      let idx = index.CommitteeIndex
       let committee =
-        get_beacon_committee(state, slot, index, cache)
+        get_beacon_committee(state, slot, idx, cache)
       if validator_index in committee:
-        return some((committee, index, slot))
-  none(tuple[a: seq[ValidatorIndex], b: uint64, c: Slot])
+        return some((committee, idx, slot))
+  none(tuple[a: seq[ValidatorIndex], b: CommitteeIndex, c: Slot])
 
 # https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/specs/phase0/validator.md#validator-assignments
 func is_proposer(
diff --git a/research/state_sim.nim b/research/state_sim.nim
index 27d089c74..a7995968a 100644
--- a/research/state_sim.nim
+++ b/research/state_sim.nim
@@ -146,7 +146,7 @@ cli do(slots = SLOTS_PER_EPOCH * 6,
         scass = withTimerRet(timers[tShuffle]):
           mapIt(
             0'u64 ..< get_committee_count_at_slot(state, target_slot),
-            get_beacon_committee(state, target_slot, it, cache))
+            get_beacon_committee(state, target_slot, it.CommitteeIndex, cache))
 
       for i, scas in scass:
         var
diff --git a/tests/mocking/mock_attestations.nim b/tests/mocking/mock_attestations.nim
index 28bcc3b6e..5281fcaee 100644
--- a/tests/mocking/mock_attestations.nim
+++ b/tests/mocking/mock_attestations.nim
@@ -86,7 +86,7 @@ proc mockAttestationImpl(
     beacon_committee = get_beacon_committee(
       state,
       result.data.slot,
-      result.data.index,
+      result.data.index.CommitteeIndex,
       cache
     )
     committee_size = beacon_committee.len
@@ -117,7 +117,7 @@ proc fillAggregateAttestation*(state: BeaconState, attestation: var Attestation)
   let beacon_committee = get_beacon_committee(
     state,
     attestation.data.slot,
-    attestation.data.index,
+    attestation.data.index.CommitteeIndex,
     cache
   )
   for i in 0 ..< beacon_committee.len:
diff --git a/tests/spec_epoch_processing/justification_finalization_helpers.nim b/tests/spec_epoch_processing/justification_finalization_helpers.nim
index 4970c6c36..83d91631f 100644
--- a/tests/spec_epoch_processing/justification_finalization_helpers.nim
+++ b/tests/spec_epoch_processing/justification_finalization_helpers.nim
@@ -45,7 +45,7 @@ proc addMockAttestations*(
       var cache = get_empty_per_epoch_cache()
 
       let committee = get_beacon_committee(
-                        state, slot.Slot, index, cache)
+                        state, slot.Slot, index.CommitteeIndex, cache)
 
       # Create a bitfield filled with the given count per attestation,
       # exactly on the right-most part of the committee field.
diff --git a/tests/test_attestation_pool.nim b/tests/test_attestation_pool.nim
index 879d22aac..055909faf 100644
--- a/tests/test_attestation_pool.nim
+++ b/tests/test_attestation_pool.nim
@@ -38,7 +38,7 @@ when const_preset == "minimal": # Too much stack space used on mainnet
       let
         # Create an attestation for slot 1!
         beacon_committee = get_beacon_committee(
-          state.data.data, state.data.data.slot, 0, cache)
+          state.data.data, state.data.data.slot, 0.CommitteeIndex, cache)
         attestation = makeAttestation(
           state.data.data, state.blck.root, beacon_committee[0], cache)
 
@@ -56,7 +56,7 @@ when const_preset == "minimal": # Too much stack space used on mainnet
       let
         # Create an attestation for slot 1!
         bc0 = get_beacon_committee(
-          state.data.data, state.data.data.slot, 0, cache)
+          state.data.data, state.data.data.slot, 0.CommitteeIndex, cache)
         attestation0 = makeAttestation(
           state.data.data, state.blck.root, bc0[0], cache)
 
@@ -64,7 +64,7 @@ when const_preset == "minimal": # Too much stack space used on mainnet
 
       let
         bc1 = get_beacon_committee(state.data.data,
-          state.data.data.slot, 0, cache)
+          state.data.data.slot, 0.CommitteeIndex, cache)
         attestation1 = makeAttestation(
           state.data.data, state.blck.root, bc1[0], cache)
 
@@ -84,7 +84,7 @@ when const_preset == "minimal": # Too much stack space used on mainnet
       let
         # Create an attestation for slot 1!
         bc0 = get_beacon_committee(
-          state.data.data, state.data.data.slot, 0, cache)
+          state.data.data, state.data.data.slot, 0.CommitteeIndex, cache)
         attestation0 = makeAttestation(
           state.data.data, state.blck.root, bc0[0], cache)
         attestation1 = makeAttestation(
@@ -106,7 +106,7 @@ when const_preset == "minimal": # Too much stack space used on mainnet
       var
         # Create an attestation for slot 1!
         bc0 = get_beacon_committee(
-          state.data.data, state.data.data.slot, 0, cache)
+          state.data.data, state.data.data.slot, 0.CommitteeIndex, cache)
         attestation0 = makeAttestation(
           state.data.data, state.blck.root, bc0[0], cache)
         attestation1 = makeAttestation(
@@ -129,7 +129,7 @@ when const_preset == "minimal": # Too much stack space used on mainnet
       var
         # Create an attestation for slot 1!
         bc0 = get_beacon_committee(state.data.data,
-          state.data.data.slot, 0, cache)
+          state.data.data.slot, 0.CommitteeIndex, cache)
         attestation0 = makeAttestation(
           state.data.data, state.blck.root, bc0[0], cache)
         attestation1 = makeAttestation(
@@ -185,7 +185,7 @@ when const_preset == "minimal": # Too much stack space used on mainnet
         b11Add = blockPool.add(b11Root, b11)
 
         bc1 = get_beacon_committee(
-          state.data.data, state.data.data.slot, 1, cache)
+          state.data.data, state.data.data.slot, 1.CommitteeIndex, cache)
         attestation0 = makeAttestation(state.data.data, b10Root, bc1[0], cache)
 
       pool.add(attestation0)
diff --git a/tests/test_state_transition.nim b/tests/test_state_transition.nim
index 4c232e5c5..95b5b46d2 100644
--- a/tests/test_state_transition.nim
+++ b/tests/test_state_transition.nim
@@ -85,7 +85,7 @@ suiteReport "Block processing" & preset():
     let
       # Create an attestation for slot 1 signed by the only attester we have!
       beacon_committee =
-        get_beacon_committee(state, state.slot, 0, cache)
+        get_beacon_committee(state, state.slot, 0.CommitteeIndex, cache)
       attestation = makeAttestation(
         state, previous_block_root, beacon_committee[0], cache)
 
diff --git a/tests/testblockutil.nim b/tests/testblockutil.nim
index 4bec609c6..313458df5 100644
--- a/tests/testblockutil.nim
+++ b/tests/testblockutil.nim
@@ -191,7 +191,7 @@ proc find_beacon_committee(
       slot = ((epoch_committee_index mod SLOTS_PER_EPOCH) +
         epoch.compute_start_slot_at_epoch.uint64).Slot
       index = epoch_committee_index div SLOTS_PER_EPOCH
-      committee = get_beacon_committee(state, slot, index, cache)
+      committee = get_beacon_committee(state, slot, index.CommitteeIndex, cache)
     if validator_index in committee:
       return (committee, slot, index)
   doAssert false
@@ -216,7 +216,8 @@ proc makeFullAttestations*(
 
   for index in 0..<count:
     let
-      committee = get_beacon_committee(state, slot, index, cache)
+      committee = get_beacon_committee(
+        state, slot, index.CommitteeIndex, cache)
       data = makeAttestationData(state, slot, index, beacon_block_root)
 
     doAssert committee.len() >= 1