diff --git a/beacon_chain/beacon_node.nim b/beacon_chain/beacon_node.nim index fda717d1a..6321cf4bd 100644 --- a/beacon_chain/beacon_node.nim +++ b/beacon_chain/beacon_node.nim @@ -303,12 +303,13 @@ proc storeBlock( let blck = node.blockPool.add(blockRoot, signedBlock) if blck.isErr: if blck.error == Invalid and node.config.dumpEnabled: + dump(node.config.dumpDir / "invalid", signedBlock, blockRoot) + let parent = node.blockPool.getRef(signedBlock.message.parent_root) if parent != nil: - node.blockPool.withState( - node.blockPool.tmpState, parent.atSlot(signedBlock.message.slot - 1)): + let parentBs = parent.atSlot(signedBlock.message.slot - 1) + node.blockPool.withState(node.blockPool.tmpState, parentBs): dump(node.config.dumpDir / "invalid", hashedState, parent) - dump(node.config.dumpDir / "invalid", signedBlock, blockRoot) return err(blck.error) @@ -1082,6 +1083,7 @@ programMain: if config.dumpEnabled: createDir(config.dumpDir) createDir(config.dumpDir / "incoming") + createDir(config.dumpDir / "invalid") var node = waitFor BeaconNode.init(config) diff --git a/beacon_chain/block_pools/candidate_chains.nim b/beacon_chain/block_pools/candidate_chains.nim index 7b4eef987..2da450573 100644 --- a/beacon_chain/block_pools/candidate_chains.nim +++ b/beacon_chain/block_pools/candidate_chains.nim @@ -313,12 +313,50 @@ proc getState( func getStateCacheIndex(dag: CandidateChains, blockRoot: Eth2Digest, slot: Slot): int = for i, cachedState in dag.cachedStates: - let (cacheBlockRoot, cacheSlot, state) = cachedState + let (cacheBlockRoot, cacheSlot, _) = cachedState if cacheBlockRoot == blockRoot and cacheSlot == slot: return i -1 +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. + 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 + # items are small and few. + const MAX_CACHE_SIZE = 32 + + let cacheLen = dag.cachedStates.len + doAssert cacheLen <= MAX_CACHE_SIZE + + let entry = + if dag.cachedStates.len == MAX_CACHE_SIZE: dag.cachedStates.pop().state + else: (ref HashedBeaconState)() + entry[] = state + + insert(dag.cachedStates, (blck.root, state.data.slot, entry)) + trace "CandidateChains.putState(): state cache updated", + cacheLen, root = shortLog(blck.root), slot = state.data.slot + proc putState*(dag: CandidateChains, state: HashedBeaconState, blck: BlockRef) = # TODO we save state at every epoch start but never remove them - we also # potentially save multiple states per slot if reorgs happen, meaning @@ -344,35 +382,7 @@ proc putState*(dag: CandidateChains, state: HashedBeaconState, blck: BlockRef) = if not rootWritten: dag.db.putStateRoot(blck.root, state.data.slot, state.root) - # 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. - 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 - # items are small and few. - const MAX_CACHE_SIZE = 32 - insert(dag.cachedStates, (blck.root, state.data.slot, newClone(state))) - while dag.cachedStates.len > MAX_CACHE_SIZE: - discard dag.cachedStates.pop() - let cacheLen = dag.cachedStates.len - trace "CandidateChains.putState(): state cache updated", cacheLen - doAssert cacheLen > 0 and cacheLen <= MAX_CACHE_SIZE + putStateCache(dag, state, blck) func getRef*(dag: CandidateChains, root: Eth2Digest): BlockRef = ## Retrieve a resolved block reference, if available diff --git a/beacon_chain/block_pools/clearance.nim b/beacon_chain/block_pools/clearance.nim index 2e7e35645..79d8f083d 100644 --- a/beacon_chain/block_pools/clearance.nim +++ b/beacon_chain/block_pools/clearance.nim @@ -266,7 +266,9 @@ proc isValidBeaconBlock*( # The block is not from a future slot # TODO allow `MAXIMUM_GOSSIP_CLOCK_DISPARITY` leniency, especially towards # seemingly future slots. - if not (signed_beacon_block.message.slot <= current_slot): + # TODO using +1 here while this is being sorted - should queue these until + # they're within the DISPARITY limit + if not (signed_beacon_block.message.slot <= current_slot + 1): debug "isValidBeaconBlock: block is from a future slot", signed_beacon_block_message_slot = signed_beacon_block.message.slot, current_slot = current_slot diff --git a/beacon_chain/spec/beaconstate.nim b/beacon_chain/spec/beaconstate.nim index dfec56815..cbb722988 100644 --- a/beacon_chain/spec/beaconstate.nim +++ b/beacon_chain/spec/beaconstate.nim @@ -174,7 +174,7 @@ proc slash_validator*(state: var BeaconState, slashed_index: ValidatorIndex, validator.slashed = true validator.withdrawable_epoch = max(validator.withdrawable_epoch, epoch + EPOCHS_PER_SLASHINGS_VECTOR) - state.slashings[epoch mod EPOCHS_PER_SLASHINGS_VECTOR] += + state.slashings[int(epoch mod EPOCHS_PER_SLASHINGS_VECTOR)] += validator.effective_balance decrease_balance(state, slashed_index, validator.effective_balance div MIN_SLASHING_PENALTY_QUOTIENT) diff --git a/beacon_chain/spec/state_transition_epoch.nim b/beacon_chain/spec/state_transition_epoch.nim index 6bc05e1e2..467872f85 100644 --- a/beacon_chain/spec/state_transition_epoch.nim +++ b/beacon_chain/spec/state_transition_epoch.nim @@ -566,7 +566,7 @@ func process_final_updates*(state: var BeaconState) {.nbench.}= MAX_EFFECTIVE_BALANCE) # Reset slashings - state.slashings[next_epoch mod EPOCHS_PER_SLASHINGS_VECTOR] = 0.Gwei + state.slashings[int(next_epoch mod EPOCHS_PER_SLASHINGS_VECTOR)] = 0.Gwei # Set randao mix state.randao_mixes[next_epoch mod EPOCHS_PER_HISTORICAL_VECTOR] = diff --git a/beacon_chain/sszdump.nim b/beacon_chain/sszdump.nim index dbd1a7151..1268fd591 100644 --- a/beacon_chain/sszdump.nim +++ b/beacon_chain/sszdump.nim @@ -1,24 +1,38 @@ +{.push raises: [Defect].} + import - os, strformat, + os, strformat, chronicles, ssz/ssz_serialization, beacon_node_types, ./spec/[crypto, datatypes, digest] +# Dump errors are generally not fatal where used currently - the code calling +# these functions, like most code, is not exception safe +template logErrors(body: untyped) = + try: + body + except CatchableError as err: + notice "Failed to write SSZ", dir, msg = err.msg + proc dump*(dir: string, v: AttestationData, validator: ValidatorPubKey) = - SSZ.saveFile(dir / &"att-{v.slot}-{v.index}-{shortLog(validator)}.ssz", v) + logErrors: + SSZ.saveFile(dir / &"att-{v.slot}-{v.index}-{shortLog(validator)}.ssz", v) proc dump*(dir: string, v: SignedBeaconBlock, root: Eth2Digest) = - SSZ.saveFile(dir / &"block-{v.message.slot}-{shortLog(root)}.ssz", v) + logErrors: + SSZ.saveFile(dir / &"block-{v.message.slot}-{shortLog(root)}.ssz", v) proc dump*(dir: string, v: SignedBeaconBlock, blck: BlockRef) = dump(dir, v, blck.root) proc dump*(dir: string, v: HashedBeaconState, blck: BlockRef) = - SSZ.saveFile( - dir / &"state-{v.data.slot}-{shortLog(blck.root)}-{shortLog(v.root)}.ssz", - v.data) + logErrors: + SSZ.saveFile( + dir / &"state-{v.data.slot}-{shortLog(blck.root)}-{shortLog(v.root)}.ssz", + v.data) proc dump*(dir: string, v: HashedBeaconState) = - SSZ.saveFile( - dir / &"state-{v.data.slot}-{shortLog(v.root)}.ssz", - v.data) + logErrors: + SSZ.saveFile( + dir / &"state-{v.data.slot}-{shortLog(v.root)}.ssz", + v.data) diff --git a/ncli/ncli_hash_tree_root.nim b/ncli/ncli_hash_tree_root.nim index 2a91a4074..24260880a 100644 --- a/ncli/ncli_hash_tree_root.nim +++ b/ncli/ncli_hash_tree_root.nim @@ -17,13 +17,17 @@ cli do(kind: string, file: string): echo "Unknown file type: ", ext quit 1 ) - echo hash_tree_root(v[]).data.toHex() + when t is SignedBeaconBlock: + echo hash_tree_root(v.message).data.toHex() + else: + echo hash_tree_root(v[]).data.toHex() let ext = splitFile(file).ext case kind of "attester_slashing": printit(AttesterSlashing) of "attestation": printit(Attestation) + of "signed_block": printit(SignedBeaconBlock) of "block": printit(BeaconBlock) of "block_body": printit(BeaconBlockBody) of "block_header": printit(BeaconBlockHeader) diff --git a/ncli/ncli_pretty.nim b/ncli/ncli_pretty.nim index 80462da11..b6dac732b 100644 --- a/ncli/ncli_pretty.nim +++ b/ncli/ncli_pretty.nim @@ -22,6 +22,7 @@ cli do(kind: string, file: string): case kind of "attester_slashing": printit(AttesterSlashing) of "attestation": printit(Attestation) + of "signed_block": printit(SignedBeaconBlock) of "block": printit(BeaconBlock) of "block_body": printit(BeaconBlockBody) of "block_header": printit(BeaconBlockHeader) diff --git a/tests/test_block_pool.nim b/tests/test_block_pool.nim index 18cd986b7..53f1509be 100644 --- a/tests/test_block_pool.nim +++ b/tests/test_block_pool.nim @@ -13,6 +13,9 @@ import ../beacon_chain/spec/[datatypes, digest, validator], ../beacon_chain/[beacon_node_types, block_pool, state_transition, ssz] +when isMainModule: + import chronicles # or some random compile error happens... + suiteReport "BlockRef and helpers" & preset(): timedTest "isAncestorOf sanity" & preset(): let @@ -367,3 +370,4 @@ when const_preset == "minimal": # These require some minutes in mainnet hash_tree_root(pool.headState.data.data) hash_tree_root(pool2.justifiedState.data.data) == hash_tree_root(pool.justifiedState.data.data) +