cache beacon committee size calculation (#1363)

* cache beacon committee size calculation

this fixes a bug in get_validator_churn_limit as well

* fix

* make committee counts consistently uint64

mixing feels like the worst of the two worlds

beacon_chain/attestation_aggregation.nim

@@ -42,13 +42,13 @@ proc aggregate_attestations*(
   doAssert slot + ATTESTATION_PROPAGATION_SLOT_RANGE >= state.slot
   doAssert state.slot >= slot
 
+  var cache = StateCache()
   # TODO performance issue for future, via get_active_validator_indices(...)
-  doAssert index.uint64 < get_committee_count_at_slot(state, slot)
+  doAssert index.uint64 < get_committee_count_per_slot(state, slot, cache)
 
   # TODO for testing purposes, refactor this into the condition check
   # and just calculation
   # https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/validator.md#aggregation-selection
-  var cache = StateCache()
   if not is_aggregator(state, slot, index, slot_signature, cache):
     return none(AggregateAndProof)
 
@@ -171,8 +171,7 @@ proc isValidAttestation*(
       epochInfo = blck.getEpochInfo(state)
       requiredSubnetIndex =
         compute_subnet_for_attestation(
-          get_committee_count_at_slot(
+          get_committee_count_per_slot(epochInfo),
-            epochInfo.shuffled_active_validator_indices.len.uint64),
           attestation.data.slot, attestation.data.index.CommitteeIndex)
 
     if requiredSubnetIndex != topicCommitteeIndex:

beacon_chain/block_pool.nim

@@ -8,7 +8,7 @@
 import
   extras, beacon_chain_db,
   stew/results,
-  spec/[crypto, datatypes, digest, presets]
+  spec/[crypto, datatypes, digest, helpers, presets]
 
 
 import
@@ -212,3 +212,9 @@ proc isValidBeaconBlock*(
     current_slot: Slot, flags: UpdateFlags): Result[void, BlockError] =
   isValidBeaconBlock(
     pool.dag, pool.quarantine, signed_beacon_block, current_slot, flags)
+
+func count_active_validators*(epochInfo: EpochRef): uint64 =
+  epochInfo.shuffled_active_validator_indices.len.uint64
+
+func get_committee_count_per_slot*(epochInfo: EpochRef): uint64 =
+  get_committee_count_per_slot(count_active_validators(epochInfo))

beacon_chain/mainchain_monitor.nim

@@ -506,8 +506,8 @@ proc createBeaconStateAux(preset: RuntimePreset,
                                              eth1Block.voteData.block_hash,
                                              eth1Block.timestamp.uint64,
                                              deposits, {})
-  let activeValidators = count_active_validator_indices(result[], GENESIS_EPOCH)
+  let activeValidators = count_active_validators(result[], GENESIS_EPOCH, StateCache())
-  eth1Block.knownGoodDepositsCount = some activeValidators.uint64
+  eth1Block.knownGoodDepositsCount = some activeValidators
 
 proc createBeaconState(m: MainchainMonitor, eth1Block: Eth1Block): BeaconStateRef =
   createBeaconStateAux(

beacon_chain/spec/beaconstate.nim

@@ -115,8 +115,10 @@ func compute_activation_exit_epoch*(epoch: Epoch): Epoch =
 # https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/beacon-chain.md#get_validator_churn_limit
 func get_validator_churn_limit(state: BeaconState, cache: var StateCache): uint64 =
   # Return the validator churn limit for the current epoch.
-  max(MIN_PER_EPOCH_CHURN_LIMIT,
+  max(
-      len(cache.shuffled_active_validator_indices).uint64 div CHURN_LIMIT_QUOTIENT)
+    MIN_PER_EPOCH_CHURN_LIMIT,
+    count_active_validators(
+      state, state.get_current_epoch(), cache) div CHURN_LIMIT_QUOTIENT)
 
 # https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/beacon-chain.md#initiate_validator_exit
 func initiate_validator_exit*(state: var BeaconState,
@@ -588,12 +590,11 @@ proc check_attestation*(
     attestation = shortLog(attestation)
   trace "process_attestation: beginning"
 
-  let committee_count_at_slot =
+  let committees_per_slot =
-    get_committee_count_at_slot(get_shuffled_active_validator_indices(
+    get_committee_count_per_slot(state, data.slot, stateCache)
-      state, state.get_current_epoch(), stateCache).len.uint64).uint64
+  if not (data.index < committees_per_slot):
-  if not (data.index < committee_count_at_slot):
     warn "Data index exceeds committee count",
-      committee_count = committee_count_at_slot
+      committee_count = committees_per_slot
     return
 
   if not isValidAttestationTargetEpoch(state.get_current_epoch(), data):

beacon_chain/spec/helpers.nim

@@ -11,7 +11,7 @@
 
 import
   # Standard lib
-  math,
+  std/[math, sequtils, tables],
   # Third-party
   stew/endians2,
   # Internal
@@ -57,10 +57,16 @@ func is_active_validator*(validator: Validator, epoch: Epoch): bool =
   validator.activation_epoch <= epoch and epoch < validator.exit_epoch
 
 # https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/beacon-chain.md#get_active_validator_indices
-func count_active_validator_indices*(state: BeaconState, epoch: Epoch): int =
+func count_active_validators*(state: BeaconState,
-  for val in state.validators:
+                              epoch: Epoch,
-    if is_active_validator(val, epoch):
+                              cache: StateCache): uint64 =
-      result += 1
+  if epoch in cache.shuffled_active_validator_indices:
+    try:
+      cache.shuffled_active_validator_indices[epoch].len.uint64
+    except KeyError:
+      raiseAssert "just checked"
+  else:
+    countIt(state.validators, is_active_validator(it, epoch)).uint64
 
 # https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/beacon-chain.md#get_active_validator_indices
 func get_active_validator_indices*(state: BeaconState, epoch: Epoch):
@@ -70,13 +76,15 @@ func get_active_validator_indices*(state: BeaconState, epoch: Epoch):
     if is_active_validator(val, epoch):
       result.add idx.ValidatorIndex
 
-# https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/beacon-chain.md#get_committee_count_at_slot
+# https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/beacon-chain.md#get_committee_count_per_slot
-func get_committee_count_at_slot*(num_active_validators: uint64): uint64 =
+func get_committee_count_per_slot*(num_active_validators: uint64): uint64 =
   clamp(
     num_active_validators div SLOTS_PER_EPOCH div TARGET_COMMITTEE_SIZE,
-    1, MAX_COMMITTEES_PER_SLOT).uint64
+    1'u64, MAX_COMMITTEES_PER_SLOT)
 
-func get_committee_count_at_slot*(state: BeaconState, slot: Slot): uint64 =
+func get_committee_count_per_slot*(state: BeaconState,
+                                   epoch: Epoch,
+                                   cache: StateCache): uint64 =
   # Return the number of committees at ``slot``.
 
   # TODO this is mostly used in for loops which have indexes which then need to
@@ -84,12 +92,16 @@ func get_committee_count_at_slot*(state: BeaconState, slot: Slot): uint64 =
   # with better and more type-safe use pattern, probably beginning with using a
   # CommitteeIndex return type here.
   let
-    epoch = compute_epoch_at_slot(slot)
+    active_validator_count = count_active_validators(state, epoch, cache)
-    active_validator_count = count_active_validator_indices(state, epoch)
+  result = get_committee_count_per_slot(active_validator_count)
-  result = get_committee_count_at_slot(active_validator_count.uint64)
 
   # Otherwise, get_beacon_committee(...) cannot access some committees.
-  doAssert (SLOTS_PER_EPOCH * MAX_COMMITTEES_PER_SLOT).uint64 >= result
+  doAssert (SLOTS_PER_EPOCH * MAX_COMMITTEES_PER_SLOT) >= uint64(result)
+
+func get_committee_count_per_slot*(state: BeaconState,
+                                   slot: Slot,
+                                   cache: StateCache): uint64 =
+  get_committee_count_per_slot(state, slot.compute_epoch_at_slot, cache)
 
 # https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/beacon-chain.md#get_current_epoch
 func get_current_epoch*(state: BeaconState): Epoch =

beacon_chain/spec/network.nim

@@ -84,9 +84,11 @@ func getAttestationTopic*(forkDigest: ForkDigest, subnetIndex: uint64):
   except ValueError as e:
     raiseAssert e.msg
 
-func getAttestationTopic*(forkDigest: ForkDigest, attestation: Attestation, num_active_validators: uint64): string =
+func getAttestationTopic*(forkDigest: ForkDigest,
+                          attestation: Attestation,
+                          num_active_validators: uint64): string =
   getAttestationTopic(
     forkDigest,
     compute_subnet_for_attestation(
-      get_committee_count_at_slot(num_active_validators),
+      get_committee_count_per_slot(num_active_validators),
       attestation.data.slot, attestation.data.index.CommitteeIndex))

beacon_chain/spec/validator.nim

@@ -154,14 +154,14 @@ func get_beacon_committee*(
       get_shuffled_active_validator_indices(state, epoch)
 
   try:
-    let committee_count = get_committee_count_at_slot(
+    let committees_per_slot = get_committee_count_per_slot(
       cache.shuffled_active_validator_indices[epoch].len.uint64)
     compute_committee(
       cache.shuffled_active_validator_indices[epoch],
       get_seed(state, epoch, DOMAIN_BEACON_ATTESTER),
-      (slot mod SLOTS_PER_EPOCH) * committee_count +
+      (slot mod SLOTS_PER_EPOCH) * committees_per_slot +
         index.uint64,
-      committee_count * SLOTS_PER_EPOCH
+      committees_per_slot * SLOTS_PER_EPOCH
     )
   except KeyError:
     raiseAssert "values are added to cache before using them"
@@ -289,7 +289,7 @@ 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):
+    for index in 0'u64 ..< get_committee_count_per_slot(state, slot, cache):
       let idx = index.CommitteeIndex
       let committee = get_beacon_committee(state, slot, idx, cache)
       if validator_index in committee:
@@ -306,16 +306,17 @@ func get_committee_assignments*(
   var cache = StateCache()
   let start_slot = compute_start_slot_at_epoch(epoch)
 
-  # get_committee_count_at_slot is constant throughout an epoch
+  # get_committee_count_per_slot is constant throughout an epoch
-  let committee_count_at_slot = get_committee_count_at_slot(state, start_slot)
+  let committees_per_slot =
+    get_committee_count_per_slot(state, start_slot, cache)
 
   for slot in start_slot ..< start_slot + SLOTS_PER_EPOCH:
-    for index in 0 ..< committee_count_at_slot:
+    for index in 0'u64 ..< committees_per_slot:
       let idx = index.CommitteeIndex
       if not disjoint(validator_indices,
           get_beacon_committee(state, slot, idx, cache).toHashSet):
         result.add(
-          (compute_subnet_for_attestation(committee_count_at_slot, slot, idx),
+          (compute_subnet_for_attestation(committees_per_slot, slot, idx),
             slot))
 
 # https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/validator.md#validator-assignments

beacon_chain/validator_api.nim

@@ -237,7 +237,7 @@ proc installValidatorApiHandlers*(rpcServer: RpcServer, node: BeaconNode) =
 
       proc forSlot(slot: Slot, res: var seq[BeaconStatesCommitteesTuple]) =
         if index == 0: # TODO this means if the parameter is missing (its optional)
-          let committees_per_slot = get_committee_count_at_slot(state, slot)
+          let committees_per_slot = get_committee_count_per_slot(state, slot, cache)
           for committee_index in 0'u64..<committees_per_slot:
             res.add(getCommittee(slot, committee_index.CommitteeIndex))
         else:

beacon_chain/validator_duties.nim

@@ -123,7 +123,7 @@ proc sendAttestation*(node: BeaconNode, attestation: Attestation) =
       BlockSlot(blck: attestationBlck, slot: attestation.data.slot)):
     node.sendAttestation(
       attestation,
-      blck.getEpochInfo(state).shuffled_active_validator_indices.len.uint64)
+      count_active_validators(blck.getEpochInfo(state)))
 
 proc createAndSendAttestation(node: BeaconNode,
                               fork: Fork,
@@ -331,13 +331,9 @@ proc handleAttestations(node: BeaconNode, head: BlockRef, slot: Slot) =
   node.blockPool.withState(node.blockPool.tmpState, attestationHead):
     var cache = getEpochCache(attestationHead.blck, state)
     let
-      committees_per_slot = get_committee_count_at_slot(state, slot)
+      committees_per_slot = get_committee_count_per_slot(state, slot, cache)
       num_active_validators =
-        try:
+        count_active_validators(state, slot.compute_epoch_at_slot, cache)
-          cache.shuffled_active_validator_indices[
-            slot.compute_epoch_at_slot].len.uint64
-        except KeyError:
-          raiseAssert "getEpochCache(...) didn't fill cache"
 
     for committee_index in 0'u64..<committees_per_slot:
       let committee = get_beacon_committee(
@@ -394,9 +390,10 @@ proc broadcastAggregatedAttestations(
 
   let bs = BlockSlot(blck: aggregationHead, slot: aggregationSlot)
   node.blockPool.withState(node.blockPool.tmpState, bs):
+    var cache = getEpochCache(aggregationHead, state)
     let
-      committees_per_slot = get_committee_count_at_slot(state, aggregationSlot)
+      committees_per_slot =
-    var cache = StateCache()
+        get_committee_count_per_slot(state, aggregationSlot, cache)
     for committee_index in 0'u64..<committees_per_slot:
       let committee = get_beacon_committee(
         state, aggregationSlot, committee_index.CommitteeIndex, cache)

research/block_sim.nim

@@ -24,6 +24,7 @@ import
   ../beacon_chain/[
     attestation_pool, block_pool, beacon_node_types, beacon_chain_db,
     interop, validator_pool],
+  ../beacon_chain/block_pools/candidate_chains,
   eth/db/[kvstore, kvstore_sqlite3],
   ../beacon_chain/ssz/[merkleization, ssz_serialization],
   ./simutils
@@ -67,8 +68,8 @@ cli do(slots = SLOTS_PER_EPOCH * 6,
       attestationHead = blockPool.head.blck.atSlot(slot)
 
     blockPool.withState(blockPool.tmpState, attestationHead):
-      var cache = StateCache()
+      var cache = getEpochCache(attestationHead.blck, state)
-      let committees_per_slot = get_committee_count_at_slot(state, slot)
+      let committees_per_slot = get_committee_count_per_slot(state, slot, cache)
 
       for committee_index in 0'u64..<committees_per_slot:
         let committee = get_beacon_committee(

research/state_sim.nim

@@ -107,12 +107,12 @@ cli do(slots = SLOTS_PER_EPOCH * 6,
       # some variation
       let
         target_slot = state[].data.slot + MIN_ATTESTATION_INCLUSION_DELAY - 1
-        commitee_count = get_committee_count_at_slot(state[].data, target_slot)
+        committees_per_slot = get_committee_count_per_slot(state[].data, target_slot, cache)
 
       let
         scass = withTimerRet(timers[tShuffle]):
           mapIt(
-            0 ..< commitee_count.int,
+            0 ..< committees_per_slot.int,
             get_beacon_committee(state[].data, target_slot, it.CommitteeIndex, cache))
 
       for i, scas in scass:

tests/spec_epoch_processing/justification_finalization_helpers.nim

@@ -39,14 +39,13 @@ proc addMockAttestations*(
     get_shuffled_active_validator_indices(state, epoch)
   var remaining_balance = state.get_total_active_balance(cache).int64 * 2 div 3
 
-  let start_slot = compute_start_slot_at_epoch(epoch)
+  let
+    start_slot = compute_start_slot_at_epoch(epoch)
+    committees_per_slot = get_committee_count_per_slot(state, epoch, cache)
 
   # for-loop of distinct type is broken: https://github.com/nim-lang/Nim/issues/12074
   for slot in start_slot.uint64 ..< start_slot.uint64 + SLOTS_PER_EPOCH:
-    for index in 0 ..< get_committee_count_at_slot(state, slot.Slot):
+    for index in 0'u64 ..< committees_per_slot:
-      # TODO: can we move cache out of the loops
-      var cache = StateCache()
-
       let committee = get_beacon_committee(
                         state, slot.Slot, index.CommitteeIndex, cache)
 

tests/test_attestation_pool.nim

@@ -291,11 +291,9 @@ suiteReport "Attestation pool processing" & preset():
     doAssert: b10Add_clone.error == Duplicate
 
   wrappedTimedTest "Trying to add a duplicate block from an old pruned epoch is tagged as an error":
-    var cache = StateCache()
-
     blockpool[].addFlags {skipBLSValidation}
     pool.forkChoice_v2.proto_array.prune_threshold = 1
-
+    var cache = StateCache()
     let
       b10 = makeTestBlock(state.data, blockPool[].tail.root, cache)
       b10Add = blockpool[].addRawBlock(b10) do (
@@ -326,8 +324,10 @@ suiteReport "Attestation pool processing" & preset():
       let start_slot = compute_start_slot_at_epoch(Epoch epoch)
       for slot in start_slot ..< start_slot + SLOTS_PER_EPOCH:
 
-        let new_block = makeTestBlock(state.data, block_root, cache, attestations = attestations)
+        let new_block = makeTestBlock(
-        let block_ok = state_transition(defaultRuntimePreset, state.data, new_block, {skipBLSValidation}, noRollback)
+          state.data, block_root, cache, attestations = attestations)
+        let block_ok = state_transition(
+          defaultRuntimePreset, state.data, new_block, {skipBLSValidation}, noRollback)
         doAssert: block_ok
 
         block_root = new_block.root
@@ -344,7 +344,7 @@ suiteReport "Attestation pool processing" & preset():
         blockPool[].updateHead(head)
 
         attestations.setlen(0)
-        for index in 0 ..< get_committee_count_at_slot(state.data.data, slot.Slot):
+        for index in 0'u64 ..< get_committee_count_per_slot(state.data.data, slot.Slot, cache):
           let committee = get_beacon_committee(
               state.data.data, state.data.data.slot, index.CommitteeIndex, cache)
 

tests/testblockutil.nim

@@ -189,8 +189,8 @@ proc find_beacon_committee(
     state: BeaconState, validator_index: ValidatorIndex,
     cache: var StateCache): auto =
   let epoch = compute_epoch_at_slot(state.slot)
-  for epoch_committee_index in 0'u64 ..< get_committee_count_at_slot(
+  for epoch_committee_index in 0'u64 ..< get_committee_count_per_slot(
-      state, epoch.compute_start_slot_at_epoch) * SLOTS_PER_EPOCH:
+      state, epoch, cache) * SLOTS_PER_EPOCH:
     let
       slot = ((epoch_committee_index mod SLOTS_PER_EPOCH) +
         epoch.compute_start_slot_at_epoch.uint64).Slot
@@ -216,9 +216,9 @@ proc makeFullAttestations*(
   # Create attestations in which the full committee participates for each shard
   # that should be attested to during a particular slot
   let
-    count = get_committee_count_at_slot(state, slot)
+    committees_per_slot = get_committee_count_per_slot(state, slot, cache)
 
-  for index in 0..<count:
+  for index in 0'u64..<committees_per_slot:
     let
       committee = get_beacon_committee(
         state, slot, index.CommitteeIndex, cache)