From 4a9a7be2710d59c3bc383366dbf14c43463ad040 Mon Sep 17 00:00:00 2001 From: tersec Date: Wed, 22 Jul 2020 07:51:45 +0000 Subject: [PATCH] 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 | 32 ++++----- beacon_chain/block_pool.nim | 4 +- beacon_chain/block_pools/candidate_chains.nim | 69 +++++++++---------- beacon_chain/spec/beaconstate.nim | 13 ++-- beacon_chain/spec/validator.nim | 7 +- 5 files changed, 56 insertions(+), 69 deletions(-) diff --git a/beacon_chain/attestation_pool.nim b/beacon_chain/attestation_pool.nim index 01541e2d8..b057d0f01 100644 --- a/beacon_chain/attestation_pool.nim +++ b/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(state, attestation.data): + if not isValidAttestationTargetEpoch( + 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( - state, attestation.data, validation.aggregation_bits, cache) + participants = toSeq(items(get_attesting_indices( + state, attestation.data, validation.aggregation_bits, cache))) var found = false for a in attestationsSeen.attestations.mitems(): diff --git a/beacon_chain/block_pool.nim b/beacon_chain/block_pool.nim index 31607bef6..492f65306 100644 --- a/beacon_chain/block_pool.nim +++ b/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 diff --git a/beacon_chain/block_pools/candidate_chains.nim b/beacon_chain/block_pools/candidate_chains.nim index 5197991a6..51f672df2 100644 --- a/beacon_chain/block_pools/candidate_chains.nim +++ b/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 - # aggregation and to process block proposals going back to the last - # finalized slot. Ideally to avoid potential combinatiorial forking - # storage and/or memory constraints could CoW, up to and including, - # 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) + # Efficiently access states for both attestation aggregation and to process + # block proposals going back to the last finalized slot. + let stateCacheIndex = + dag.getStateCacheIndex(blck.root, state.data.slot, false) if stateCacheIndex == -1: - # Could use a deque or similar, but want simpler structure, and the data - # items are small and few. - const MAX_CACHE_SIZE = 16 + const MAX_CACHE_SIZE = 18 let cacheLen = dag.cachedStates.len doAssert cacheLen <= MAX_CACHE_SIZE @@ -436,7 +421,8 @@ proc putState*(dag: CandidateChains, state: HashedBeaconState, blck: BlockRef) = if not rootWritten: dag.db.putStateRoot(blck.root, state.data.slot, state.root) - putStateCache(dag, state, blck) + if state.data.slot mod 2 == 0: + putStateCache(dag, state, blck) func getRef*(dag: CandidateChains, root: Eth2Digest): BlockRef = ## Retrieve a resolved block reference, if available @@ -542,8 +528,9 @@ proc skipAndUpdateState( ok -proc rewindState(dag: CandidateChains, state: var StateData, bs: BlockSlot): - seq[BlockRef] = +proc rewindState( + 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 diff --git a/beacon_chain/spec/beaconstate.nim b/beacon_chain/spec/beaconstate.nim index e33e6e521..8b7671172 100644 --- a/beacon_chain/spec/beaconstate.nim +++ b/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 - data.target.epoch == get_current_epoch(state)): + if not (data.target.epoch == get_previous_epoch(state_epoch) or + 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 diff --git a/beacon_chain/spec/validator.nim b/beacon_chain/spec/validator.nim index 7474967dc..12a0ecd35 100644 --- a/beacon_chain/spec/validator.nim +++ b/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] =