faster syncing (#1348)

* maybe faster syncing

* 80-character lines

* remove instrumentation debugEchos; fix target attestation epoch in attestation pool validation

* use the epoch-granularity matching in attestation.addResolved(...)

beacon_chain/attestation_pool.nim

@@ -182,13 +182,6 @@ func updateLatestVotes(
     # # ForkChoice v2
     # pool.forkChoice_v2.process_attestation(validator, blck.root, target_epoch)
 
-func get_attesting_indices_seq(state: BeaconState,
-                               attestation_data: AttestationData,
-                               bits: CommitteeValidatorsBits,
-                               cache: var StateCache): seq[ValidatorIndex] =
-  toSeq(items(get_attesting_indices(
-    state, attestation_data, bits, cache)))
-
 func addUnresolved(pool: var AttestationPool, attestation: Attestation) =
   pool.unresolved[attestation.data.beacon_block_root] =
     UnresolvedAttestation(
@@ -227,24 +220,25 @@ proc addResolved(pool: var AttestationPool, blck: BlockRef, attestation: Attesta
   #   # Logging in isValidAttestationSlot
   #   return
 
-  # Get a temporary state at the (block, slot) targeted by the attestation
-  updateStateData(
-    pool.blockPool, pool.blockPool.tmpState,
-    BlockSlot(blck: blck, slot: attestation.data.slot))
-
-  template state(): BeaconState = pool.blockPool.tmpState.data.data
-
   # Check that the attestation is indeed valid
   # TODO: we might want to split checks that depend
   #       on the state and those that don't to cheaply
   #       discard invalid attestations before rewinding state.
-
+  if not isValidAttestationTargetEpoch(
-  if not isValidAttestationTargetEpoch(state, attestation.data):
+      attestation.data.target.epoch, attestation.data):
     notice "Invalid attestation",
       attestation = shortLog(attestation),
-      current_epoch = get_current_epoch(state)
+      current_epoch = attestation.data.slot.compute_epoch_at_slot
     return
 
+  # Get a temporary state at the (block, slot) targeted by the attestation
+  updateStateData(
+    pool.blockPool, pool.blockPool.tmpState,
+    BlockSlot(blck: blck, slot: attestation.data.slot),
+    true)
+
+  template state(): BeaconState = pool.blockPool.tmpState.data.data
+
   # TODO inefficient data structures..
 
   var cache = getEpochCache(blck, state)
@@ -255,8 +249,8 @@ proc addResolved(pool: var AttestationPool, blck: BlockRef, attestation: Attesta
     validation = Validation(
       aggregation_bits: attestation.aggregation_bits,
       aggregate_signature: attestation.signature)
-    participants = get_attesting_indices_seq(
+    participants = toSeq(items(get_attesting_indices(
-      state, attestation.data, validation.aggregation_bits, cache)
+      state, attestation.data, validation.aggregation_bits, cache)))
 
   var found = false
   for a in attestationsSeen.attestations.mitems():

beacon_chain/block_pool.nim

@@ -194,7 +194,9 @@ template withEpochState*(
 
   withEpochState(pool.dag, cache, blockSlot, body)
 
-proc updateStateData*(pool: BlockPool, state: var StateData, bs: BlockSlot) =
+proc updateStateData*(
+    pool: BlockPool, state: var StateData, bs: BlockSlot,
+    matchEpoch: bool = false) =
   ## Rewind or advance state such that it matches the given block and slot -
   ## this may include replaying from an earlier snapshot if blck is on a
   ## different branch or has advanced to a higher slot number than slot

beacon_chain/block_pools/candidate_chains.nim

@@ -30,7 +30,8 @@ proc putBlock*(
   dag.db.putBlock(signedBlock)
 
 proc updateStateData*(
-  dag: CandidateChains, state: var StateData, bs: BlockSlot) {.gcsafe.}
+  dag: CandidateChains, state: var StateData, bs: BlockSlot,
+  matchEpoch: bool = false) {.gcsafe.}
 
 template withState*(
     dag: CandidateChains, cache: var StateData, blockSlot: BlockSlot, body: untyped): untyped =
@@ -362,42 +363,26 @@ proc getState(
 
   true
 
-func getStateCacheIndex(dag: CandidateChains, blockRoot: Eth2Digest, slot: Slot): int =
+func getStateCacheIndex(
+    dag: CandidateChains, blockRoot: Eth2Digest, slot: Slot, matchEpoch: bool):
+    int =
   for i, cachedState in dag.cachedStates:
     let (cacheBlockRoot, cacheSlot, _) = cachedState
-    if cacheBlockRoot == blockRoot and cacheSlot == slot:
+    if cacheBlockRoot == blockRoot and (cacheSlot == slot or
+        (matchEpoch and
+          cacheSlot.compute_epoch_at_slot == slot.compute_epoch_at_slot)):
       return i
 
   -1
 
-func putStateCache(
+func putStateCache*(
     dag: CandidateChains, state: HashedBeaconState, blck: BlockRef) =
-  # Need to be able to efficiently access states for both attestation
+  # Efficiently access states for both attestation aggregation and to process
-  # aggregation and to process block proposals going back to the last
+  # block proposals going back to the last finalized slot.
-  # finalized slot. Ideally to avoid potential combinatiorial forking
+  let stateCacheIndex =
-  # storage and/or memory constraints could CoW, up to and including,
+    dag.getStateCacheIndex(blck.root, state.data.slot, false)
-  # in particular, hash_tree_root() which is expensive to do 30 times
-  # since the previous epoch, to efficiently state_transition back to
-  # desired slot. However, none of that's in place, so there are both
-  # expensive, repeated BeaconState copies as well as computationally
-  # time-consuming-near-end-of-epoch hash tree roots. The latter are,
-  # effectively, naïvely O(n^2) in slot number otherwise, so when the
-  # slots become in the mid-to-high-20s it's spending all its time in
-  # pointlessly repeated calculations of prefix-state-transitions. An
-  # intermediate time/memory workaround involves storing only mapping
-  # between BlockRefs, or BlockSlots, and the BeaconState tree roots,
-  # but that still involves tens of megabytes worth of copying, along
-  # with the concomitant memory allocator and GC load. Instead, use a
-  # more memory-intensive (but more conceptually straightforward, and
-  # faster) strategy to just store, for the most recent slots.
-  if state.data.slot mod 2 != 0:
-    return
-
-  let stateCacheIndex = dag.getStateCacheIndex(blck.root, state.data.slot)
   if stateCacheIndex == -1:
-    # Could use a deque or similar, but want simpler structure, and the data
+    const MAX_CACHE_SIZE = 18
-    # items are small and few.
-    const MAX_CACHE_SIZE = 16
 
     let cacheLen = dag.cachedStates.len
     doAssert cacheLen <= MAX_CACHE_SIZE
@@ -436,6 +421,7 @@ proc putState*(dag: CandidateChains, state: HashedBeaconState, blck: BlockRef) =
       if not rootWritten:
         dag.db.putStateRoot(blck.root, state.data.slot, state.root)
 
+  if state.data.slot mod 2 == 0:
     putStateCache(dag, state, blck)
 
 func getRef*(dag: CandidateChains, root: Eth2Digest): BlockRef =
@@ -542,8 +528,9 @@ proc skipAndUpdateState(
 
   ok
 
-proc rewindState(dag: CandidateChains, state: var StateData, bs: BlockSlot):
+proc rewindState(
-    seq[BlockRef] =
+    dag: CandidateChains, state: var StateData, bs: BlockSlot,
+    matchEpoch: bool): seq[BlockRef] =
   logScope:
     blockSlot = shortLog(bs)
     pcs = "replay_state"
@@ -580,7 +567,7 @@ proc rewindState(dag: CandidateChains, state: var StateData, bs: BlockSlot):
     # TODO investigate replacing with getStateCached, by refactoring whole
     # function. Empirically, this becomes pretty rare once good caches are
     # used in the front-end.
-    let idx = dag.getStateCacheIndex(parBs.blck.root, parBs.slot)
+    let idx = dag.getStateCacheIndex(parBs.blck.root, parBs.slot, matchEpoch)
     if idx >= 0:
       assign(state.data, dag.cachedStates[idx].state[])
       let ancestor = ancestors.pop()
@@ -629,7 +616,9 @@ proc rewindState(dag: CandidateChains, state: var StateData, bs: BlockSlot):
 
   ancestors
 
-proc getStateDataCached(dag: CandidateChains, state: var StateData, bs: BlockSlot): bool =
+proc getStateDataCached(
+    dag: CandidateChains, state: var StateData, bs: BlockSlot,
+    matchEpoch: bool): bool =
   # This pointedly does not run rewindState or state_transition, but otherwise
   # mostly matches updateStateData(...), because it's too expensive to run the
   # rewindState(...)/skipAndUpdateState(...)/state_transition(...) procs, when
@@ -639,7 +628,7 @@ proc getStateDataCached(dag: CandidateChains, state: var StateData, bs: BlockSlo
     # any given use case.
     doAssert state.data.data.slot <= bs.slot + 4
 
-  let idx = dag.getStateCacheIndex(bs.blck.root, bs.slot)
+  let idx = dag.getStateCacheIndex(bs.blck.root, bs.slot, matchEpoch)
   if idx >= 0:
     assign(state.data, dag.cachedStates[idx].state[])
     state.blck = bs.blck
@@ -665,7 +654,9 @@ template withEpochState*(
   dag.withState(cache, blockSlot):
     body
 
-proc updateStateData*(dag: CandidateChains, state: var StateData, bs: BlockSlot) =
+proc updateStateData*(
+    dag: CandidateChains, state: var StateData, bs: BlockSlot,
+    matchEpoch: bool = false) =
   ## Rewind or advance state such that it matches the given block and slot -
   ## this may include replaying from an earlier snapshot if blck is on a
   ## different branch or has advanced to a higher slot number than slot
@@ -681,10 +672,10 @@ proc updateStateData*(dag: CandidateChains, state: var StateData, bs: BlockSlot)
 
     return # State already at the right spot
 
-  if dag.getStateDataCached(state, bs):
+  if dag.getStateDataCached(state, bs, matchEpoch):
     return
 
-  let ancestors = rewindState(dag, state, bs)
+  let ancestors = rewindState(dag, state, bs, matchEpoch)
 
   # If we come this far, we found the state root. The last block on the stack
   # is the one that produced this particular state, so we can pop it
@@ -711,6 +702,8 @@ proc updateStateData*(dag: CandidateChains, state: var StateData, bs: BlockSlot)
 
   state.blck = bs.blck
 
+  dag.putStateCache(state.data, bs.blck)
+
 proc loadTailState*(dag: CandidateChains): StateData =
   ## Load the state associated with the current tail in the dag
   let stateRoot = dag.db.getBlock(dag.tail.root).get().message.state_root

beacon_chain/spec/beaconstate.nim

@@ -531,7 +531,7 @@ proc isValidAttestationSlot*(attestationSlot, stateSlot: Slot): bool =
 
 # TODO remove/merge with p2p-interface validation
 proc isValidAttestationTargetEpoch*(
-    state: BeaconState, data: AttestationData): bool =
+    state_epoch: Epoch, data: AttestationData): bool =
   # TODO what constitutes a valid attestation when it's about to be added to
   #      the pool? we're interested in attestations that will become viable
   #      for inclusion in blocks in the future and on any fork, so we need to
@@ -544,11 +544,6 @@ proc isValidAttestationTargetEpoch*(
   #      include an attestation in a block even if the corresponding validator
   #      was slashed in the same epoch - there's no penalty for doing this and
   #      the vote counting logic will take care of any ill effects (TODO verify)
-  # TODO re-enable check
-  #if not (data.crosslink.shard < SHARD_COUNT):
-  #  notice "Attestation shard too high",
-  #    attestation_shard = data.crosslink.shard
-  #  return
 
   # Without this check, we can't get a slot number for the attestation as
   # certain helpers will assert
@@ -558,8 +553,8 @@ proc isValidAttestationTargetEpoch*(
   #      of the attestation, we'll be safe!
   # TODO the above state selection logic should probably live here in the
   #      attestation pool
-  if not (data.target.epoch == get_previous_epoch(state) or
+  if not (data.target.epoch == get_previous_epoch(state_epoch) or
-      data.target.epoch == get_current_epoch(state)):
+      data.target.epoch == state_epoch):
     warn("Target epoch not current or previous epoch")
     return
 
@@ -601,7 +596,7 @@ proc check_attestation*(
       committee_count = committee_count_at_slot
     return
 
-  if not isValidAttestationTargetEpoch(state, data):
+  if not isValidAttestationTargetEpoch(state.slot.compute_epoch_at_slot, data):
     # Logging in isValidAttestationTargetEpoch
     return
 

beacon_chain/spec/validator.nim

@@ -101,14 +101,17 @@ func get_shuffled_active_validator_indices*(
     validator_indices
 
 # https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/beacon-chain.md#get_previous_epoch
-func get_previous_epoch*(state: BeaconState): Epoch =
+func get_previous_epoch*(current_epoch: Epoch): Epoch =
   # Return the previous epoch (unless the current epoch is ``GENESIS_EPOCH``).
-  let current_epoch = get_current_epoch(state)
   if current_epoch == GENESIS_EPOCH:
     current_epoch
   else:
     current_epoch - 1
 
+func get_previous_epoch*(state: BeaconState): Epoch =
+  # Return the previous epoch (unless the current epoch is ``GENESIS_EPOCH``).
+  get_previous_epoch(get_current_epoch(state))
+
 # https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/beacon-chain.md#compute_committee
 func compute_committee(indices: seq[ValidatorIndex], seed: Eth2Digest,
     index: uint64, count: uint64): seq[ValidatorIndex] =