stop discarding future epochs; remove a StateCache() construction (#1610)

* stop discarding non-existent future epochs during epoch state transitions; remove a pointless StateCache() construction in advance_slots()

* update nbench to pass StateCache to process_slots()

beacon_chain/beacon_node_common.nim

@@ -67,6 +67,7 @@ proc updateHead*(node: BeaconNode, wallSlot: Slot): BlockRef =
   ## Can return `nil`
   node.processor[].updateHead(wallSlot)
 
+# TODO stew/sequtils2
 template findIt*(s: openarray, predicate: untyped): int =
   var res = -1
   for i, it {.inject.} in s:

beacon_chain/block_pools/chain_dag.nim

@@ -372,7 +372,7 @@ proc init*(T: type ChainDAGRef,
 
   var cache: StateCache
   res.updateStateData(res.headState, headRef.atSlot(headRef.slot), cache)
-    # We presently save states on the epoch boundary - it means that the latest
+  # We presently save states on the epoch boundary - it means that the latest
   # state we loaded might be older than head block - nonetheless, it will be
   # from the same epoch as the head, thus the finalized and justified slots are
   # the same - these only change on epoch boundaries.

beacon_chain/block_pools/quarantine.nim

@@ -40,6 +40,7 @@ func checkMissing*(quarantine: var QuarantineRef): seq[FetchRecord] =
     if countOnes(v.tries.uint64) == 1:
       result.add(FetchRecord(root: k))
 
+# TODO stew/sequtils2
 template anyIt(s, pred: untyped): bool =
   # https://github.com/nim-lang/Nim/blob/version-1-2/lib/pure/collections/sequtils.nim#L682-L704
   # without the items(...)

beacon_chain/fork_choice/proto_array.nim

@@ -352,7 +352,7 @@ func maybe_update_best_child_and_descendant(
        parent_index: Index,
        child_index: Index): Result[void, ForkChoiceError] =
   ## Observe the parent at `parent_index` with respect to the child at `child_index` and
-  ## potentiatlly modify the `parent.best_child` and `parent.best_descendant` values
+  ## potentially modify the `parent.best_child` and `parent.best_descendant` values
   ##
   ## There are four scenarios:
   ##

beacon_chain/spec/datatypes.nim

@@ -437,6 +437,8 @@ type
     data*: BeaconState
     root*: Eth2Digest # hash_tree_root(data)
 
+  # This doesn't know about forks or branches in the DAG. It's for straight,
+  # linear chunks of the chain.
   StateCache* = object
     shuffled_active_validator_indices*:
       Table[Epoch, seq[ValidatorIndex]]

beacon_chain/spec/state_transition.nim

@@ -124,15 +124,6 @@ func process_slot*(state: var HashedBeaconState) {.nbench.} =
   state.data.block_roots[state.data.slot mod SLOTS_PER_HISTORICAL_ROOT] =
     hash_tree_root(state.data.latest_block_header)
 
-func clear_epoch_from_cache(cache: var StateCache, epoch: Epoch) =
-  cache.shuffled_active_validator_indices.del epoch
-  let
-    start_slot = epoch.compute_start_slot_at_epoch
-    end_slot = (epoch + 1).compute_start_slot_at_epoch
-
-  for i in start_slot ..< end_slot:
-    cache.beacon_proposer_indices.del i
-
 # https://github.com/ethereum/eth2.0-specs/blob/v0.12.2/specs/phase0/beacon-chain.md#beacon-chain-state-transition-function
 proc advance_slot*(
     state: var HashedBeaconState, updateFlags: UpdateFlags,
@@ -146,8 +137,12 @@ proc advance_slot*(
     # Note: Genesis epoch = 0, no need to test if before Genesis
     beacon_previous_validators.set(get_epoch_validator_count(state.data))
     process_epoch(state.data, updateFlags, epochCache)
-    clear_epoch_from_cache(
+
-      epochCache, (state.data.slot + 1).compute_epoch_at_slot)
+    # Current EpochRef system doesn't look ahead within individual BlockRefs
+    doAssert (state.data.slot + 1).compute_epoch_at_slot notin
+      epochCache.shuffled_active_validator_indices
+    doAssert state.data.slot + 1 notin epochCache.beacon_proposer_indices
+
   state.data.slot += 1
   if is_epoch_transition:
     beacon_current_validators.set(get_epoch_validator_count(state.data))
@@ -156,7 +151,7 @@ proc advance_slot*(
 
 # https://github.com/ethereum/eth2.0-specs/blob/v0.12.2/specs/phase0/beacon-chain.md#beacon-chain-state-transition-function
 proc process_slots*(state: var HashedBeaconState, slot: Slot,
-    updateFlags: UpdateFlags = {}): bool {.nbench.} =
+    cache: var StateCache, updateFlags: UpdateFlags = {}): bool {.nbench.} =
   # TODO this function is not _really_ necessary: when replaying states, we
   #      advance slots one by one before calling `state_transition` - this way,
   #      we avoid the state root calculation - as such, instead of advancing
@@ -173,7 +168,6 @@ proc process_slots*(state: var HashedBeaconState, slot: Slot,
       return false
 
   # Catch up to the target slot
-  var cache = StateCache()
   while state.data.slot < slot:
     advance_slot(state, updateFlags, cache)
 
@@ -204,7 +198,9 @@ proc state_transition*(
   ## before the state has been updated, `rollback` will not be called.
   doAssert not rollback.isNil, "use noRollback if it's ok to mess up state"
 
-  if not process_slots(state, signedBlock.message.slot, flags):
+  # This only fails if it hasn't changed stateCache, so it can't create a false
+  # not-followed future history in stateCache.
+  if not process_slots(state, signedBlock.message.slot, stateCache, flags):
     rollback(state)
     return false
 

nbench/scenarios.nim

@@ -163,6 +163,7 @@ proc runFullTransition*(dir, preState, blocksPrefix: string, blocksQty: int, ski
     echo "State transition status: ", if success: "SUCCESS ✓" else: "FAILURE ⚠️"
 
 proc runProcessSlots*(dir, preState: string, numSlots: uint64) =
+  var cache = StateCache()
   let prePath = dir / preState & ".ssz"
 
   echo "Running: ", prePath
@@ -172,7 +173,7 @@ proc runProcessSlots*(dir, preState: string, numSlots: uint64) =
   state.root = hash_tree_root(state.data)
 
   # Shouldn't necessarily assert, because nbench can run test suite
-  discard process_slots(state[], state.data.slot + numSlots)
+  discard process_slots(state[], state.data.slot + numSlots, cache)
 
 template processEpochScenarioImpl(
            dir, preState: string,

tests/mocking/mock_attestations.nim

@@ -131,10 +131,12 @@ func fillAggregateAttestation*(state: BeaconState, attestation: var Attestation)
     attestation.aggregation_bits[i] = true
 
 proc add*(state: var HashedBeaconState, attestation: Attestation, slot: Slot) =
-  var signedBlock = mockBlockForNextSlot(state.data)
+  var
+    signedBlock = mockBlockForNextSlot(state.data)
+    cache = StateCache()
   signedBlock.message.slot = slot
   signedBlock.message.body.attestations.add attestation
-  doAssert process_slots(state, slot)
+  doAssert process_slots(state, slot, cache)
   signMockBlock(state.data, signedBlock)
 
   let success = state_transition(

tests/mocking/mock_state.nim

@@ -14,10 +14,12 @@ import
 
 proc nextEpoch*(state: var HashedBeaconState) =
   ## Transition to the start of the next epoch
+  var cache = StateCache()
   let slot =
     state.data.slot + SLOTS_PER_EPOCH - (state.data.slot mod SLOTS_PER_EPOCH)
-  doAssert process_slots(state, slot)
+  doAssert process_slots(state, slot, cache)
 
 proc nextSlot*(state: var HashedBeaconState) =
   ## Transition to the next slot
-  doAssert process_slots(state, state.data.slot + 1)
+  var cache = StateCache()
+  doAssert process_slots(state, state.data.slot + 1, cache)

tests/official/test_fixture_sanity_slots.nim

@@ -34,11 +34,12 @@ proc runTest(identifier: string) =
         preState = newClone(parseTest(testDir/"pre.ssz", SSZ, BeaconState))
         hashedPreState = (ref HashedBeaconState)(
           data: preState[], root: hash_tree_root(preState[]))
+        cache = StateCache()
       let postState = newClone(parseTest(testDir/"post.ssz", SSZ, BeaconState))
 
       check:
         process_slots(
-          hashedPreState[], hashedPreState.data.slot + num_slots)
+          hashedPreState[], hashedPreState.data.slot + num_slots, cache)
 
         hashedPreState.root == postState[].hash_tree_root()
       let newPreState = newClone(hashedPreState.data)

tests/spec_epoch_processing/epoch_utils.nim

@@ -12,11 +12,12 @@ import
 
 proc processSlotsUntilEndCurrentEpoch(state: var HashedBeaconState) =
   # Process all slots until the end of the last slot of the current epoch
+  var cache = StateCache()
   let slot =
     state.data.slot + SLOTS_PER_EPOCH - (state.data.slot mod SLOTS_PER_EPOCH)
 
   # Transition to slot before the epoch state transition
-  discard process_slots(state, slot - 1)
+  discard process_slots(state, slot - 1, cache)
 
   # For the last slot of the epoch,
   # only process_slot without process_epoch

tests/test_attestation_pool.nim

@@ -59,12 +59,12 @@ suiteReport "Attestation pool processing" & preset():
       quarantine = QuarantineRef()
       pool = newClone(AttestationPool.init(chainDag, quarantine))
       state = newClone(chainDag.headState)
+      cache = StateCache()
     # Slot 0 is a finalized slot - won't be making attestations for it..
     check:
-      process_slots(state.data, state.data.data.slot + 1)
+      process_slots(state.data, state.data.data.slot + 1, cache)
 
   wrappedTimedTest "Can add and retrieve simple attestation" & preset():
-    var cache = StateCache()
     let
       # Create an attestation for slot 1!
       beacon_committee = get_beacon_committee(
@@ -76,7 +76,7 @@ suiteReport "Attestation pool processing" & preset():
       attestation, [beacon_committee[0]].toHashSet(), attestation.data.slot)
 
     check:
-      process_slots(state.data, MIN_ATTESTATION_INCLUSION_DELAY.Slot + 1)
+      process_slots(state.data, MIN_ATTESTATION_INCLUSION_DELAY.Slot + 1, cache)
 
     let attestations = pool[].getAttestationsForBlock(state.data.data)
 
@@ -93,7 +93,7 @@ suiteReport "Attestation pool processing" & preset():
         state.data.data, state.blck.root, bc0[0], cache)
 
     check:
-      process_slots(state.data, state.data.data.slot + 1)
+      process_slots(state.data, state.data.data.slot + 1, cache)
 
     let
       bc1 = get_beacon_committee(state.data.data,
@@ -107,7 +107,8 @@ suiteReport "Attestation pool processing" & preset():
     pool[].addAttestation(
       attestation0, [bc0[0]].toHashSet, attestation1.data.slot)
 
-    discard process_slots(state.data, MIN_ATTESTATION_INCLUSION_DELAY.Slot + 1)
+    discard process_slots(
+      state.data, MIN_ATTESTATION_INCLUSION_DELAY.Slot + 1, cache)
 
     let attestations = pool[].getAttestationsForBlock(state.data.data)
 
@@ -131,7 +132,7 @@ suiteReport "Attestation pool processing" & preset():
       attestation1, [bc0[1]].toHashSet, attestation1.data.slot)
 
     check:
-      process_slots(state.data, MIN_ATTESTATION_INCLUSION_DELAY.Slot + 1)
+      process_slots(state.data, MIN_ATTESTATION_INCLUSION_DELAY.Slot + 1, cache)
 
     let attestations = pool[].getAttestationsForBlock(state.data.data)
 
@@ -158,7 +159,7 @@ suiteReport "Attestation pool processing" & preset():
       attestation1, [bc0[1]].toHashSet, attestation1.data.slot)
 
     check:
-      process_slots(state.data, MIN_ATTESTATION_INCLUSION_DELAY.Slot + 1)
+      process_slots(state.data, MIN_ATTESTATION_INCLUSION_DELAY.Slot + 1, cache)
 
     let attestations = pool[].getAttestationsForBlock(state.data.data)
 
@@ -184,7 +185,7 @@ suiteReport "Attestation pool processing" & preset():
       attestation0, [bc0[0]].toHashSet, attestation0.data.slot)
 
     check:
-      process_slots(state.data, MIN_ATTESTATION_INCLUSION_DELAY.Slot + 1)
+      process_slots(state.data, MIN_ATTESTATION_INCLUSION_DELAY.Slot + 1, cache)
 
     let attestations = pool[].getAttestationsForBlock(state.data.data)
 
@@ -398,9 +399,10 @@ suiteReport "Attestation validation " & preset():
       quarantine = QuarantineRef()
       pool = newClone(AttestationPool.init(chainDag, quarantine))
       state = newClone(loadTailState(chainDag))
+      cache = StateCache()
     # Slot 0 is a finalized slot - won't be making attestations for it..
     check:
-      process_slots(state.data, state.data.data.slot + 1)
+      process_slots(state.data, state.data.data.slot + 1, cache)
 
   wrappedTimedTest "Validation sanity":
     chainDag.updateFlags.incl {skipBLSValidation}

tests/test_block_pool.nim

@@ -164,7 +164,7 @@ suiteReport "Block pool processing" & preset():
 
     # Skip one slot to get a gap
     check:
-      process_slots(stateData.data, stateData.data.data.slot + 1)
+      process_slots(stateData.data, stateData.data.data.slot + 1, cache)
 
     let
       b4 = addTestBlock(stateData.data, b2.root, cache)
@@ -328,7 +328,7 @@ suiteReport "chain DAG finalization tests" & preset():
       tmpState = assignClone(dag.headState.data)
     check:
       process_slots(
-        tmpState[], tmpState.data.slot + (5 * SLOTS_PER_EPOCH).uint64)
+        tmpState[], tmpState.data.slot + (5 * SLOTS_PER_EPOCH).uint64, cache)
 
     let lateBlock = addTestBlock(tmpState[], dag.head.root, cache)
     block:
@@ -337,6 +337,9 @@ suiteReport "chain DAG finalization tests" & preset():
 
     assign(tmpState[], dag.headState.data)
 
+    # StateCache is designed to only hold a single linear history at a time
+    cache = StateCache()
+
     for i in 0 ..< (SLOTS_PER_EPOCH * 6):
       if i == 1:
         # There are 2 heads now because of the fork at slot 1
@@ -412,6 +415,9 @@ suiteReport "chain DAG finalization tests" & preset():
     check:
       dag.heads.len() == 1
 
+    # The loop creates multiple branches, which StateCache isn't suitable for
+    cache = StateCache()
+
     advance_slot(prestate[], {}, cache)
 
     # create another block, orphaning the head
@@ -448,7 +454,7 @@ suiteReport "chain DAG finalization tests" & preset():
     # Advance past epoch so that the epoch transition is gapped
     check:
       process_slots(
-        dag.headState.data, Slot(SLOTS_PER_EPOCH * 6 + 2) )
+        dag.headState.data, Slot(SLOTS_PER_EPOCH * 6 + 2), cache)
 
     var blck = makeTestBlock(
       dag.headState.data, dag.head.root, cache,

tests/test_state_transition.nim

@@ -27,17 +27,18 @@ suiteReport "Block processing" & preset():
     genesisRoot = hash_tree_root(genesisBlock.message)
 
   setup:
-    var state = newClone(genesisState[])
+    var
+      state = newClone(genesisState[])
+      cache = StateCache()
 
   timedTest "Passes from genesis state, no block" & preset():
     check:
-      process_slots(state[], state.data.slot + 1)
+      process_slots(state[], state.data.slot + 1, cache)
       state.data.slot == genesisState.data.slot + 1
 
   timedTest "Passes from genesis state, empty block" & preset():
     var
       previous_block_root = genesisBlock.root
-      cache = StateCache()
       new_block = makeTestBlock(state[], previous_block_root, cache)
 
     let block_ok = state_transition(defaultRuntimePreset, state[], new_block, {}, noRollback)
@@ -49,7 +50,7 @@ suiteReport "Block processing" & preset():
 
   timedTest "Passes through epoch update, no block" & preset():
     check:
-      process_slots(state[], Slot(SLOTS_PER_EPOCH))
+      process_slots(state[], Slot(SLOTS_PER_EPOCH), cache)
       state.data.slot == genesisState.data.slot + SLOTS_PER_EPOCH
 
   timedTest "Passes through epoch update, empty block" & preset():
@@ -77,7 +78,7 @@ suiteReport "Block processing" & preset():
 
     # Slot 0 is a finalized slot - won't be making attestations for it..
     check:
-      process_slots(state[], state.data.slot + 1)
+      process_slots(state[], state.data.slot + 1, cache)
 
     let
       # Create an attestation for slot 1 signed by the only attester we have!
@@ -90,7 +91,7 @@ suiteReport "Block processing" & preset():
     # to let the attestation propagate properly to interested participants
     check:
       process_slots(
-        state[], GENESIS_SLOT + MIN_ATTESTATION_INCLUSION_DELAY + 1)
+        state[], GENESIS_SLOT + MIN_ATTESTATION_INCLUSION_DELAY + 1, cache)
 
     let
       new_block = makeTestBlock(state[], previous_block_root, cache,