From 494ffb63ceebca62e89c5752a7f4a0703c35d8ff Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Fri, 24 Apr 2020 09:16:11 +0200 Subject: [PATCH] eh fixes (#926) * work around improbable exceptions in metrics / chronos * fix unnecessary lookup in block pool --- beacon_chain/attestation_aggregation.nim | 2 ++ beacon_chain/attestation_pool.nim | 9 +++++++++ beacon_chain/beacon_node.nim | 7 +++++++ beacon_chain/block_pool.nim | 22 +++++++++++++++++----- beacon_chain/time.nim | 10 +++++++++- 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/beacon_chain/attestation_aggregation.nim b/beacon_chain/attestation_aggregation.nim index 87bbe8ea8..905fc32bd 100644 --- a/beacon_chain/attestation_aggregation.nim +++ b/beacon_chain/attestation_aggregation.nim @@ -5,6 +5,8 @@ # * Apache v2 license (license terms in the root directory or at http://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. +{.push raises: [Defect].} + import options, ./spec/[beaconstate, datatypes, crypto, digest, helpers, validator, diff --git a/beacon_chain/attestation_pool.nim b/beacon_chain/attestation_pool.nim index b98fd9873..6eedd2547 100644 --- a/beacon_chain/attestation_pool.nim +++ b/beacon_chain/attestation_pool.nim @@ -1,3 +1,12 @@ +# beacon_chain +# Copyright (c) 2018-2020 Status Research & Development GmbH +# Licensed and distributed under either of +# * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT). +# * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +{.push raises: [Defect].} + import deques, sequtils, tables, options, chronicles, stew/[bitseqs, byteutils], json_serialization/std/sets, diff --git a/beacon_chain/beacon_node.nim b/beacon_chain/beacon_node.nim index e4fb50bf4..658fcd58f 100644 --- a/beacon_chain/beacon_node.nim +++ b/beacon_chain/beacon_node.nim @@ -1,3 +1,10 @@ +# beacon_chain +# Copyright (c) 2018-2020 Status Research & Development GmbH +# Licensed and distributed under either of +# * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT). +# * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). +# at your option. This file may not be copied, modified, or distributed except according to those terms. + import # Standard library os, tables, random, strutils, times, diff --git a/beacon_chain/block_pool.nim b/beacon_chain/block_pool.nim index 6e397bfa0..16d3c5787 100644 --- a/beacon_chain/block_pool.nim +++ b/beacon_chain/block_pool.nim @@ -1,3 +1,12 @@ +# beacon_chain +# Copyright (c) 2018-2020 Status Research & Development GmbH +# Licensed and distributed under either of +# * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT). +# * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +{.push raises: [Defect].} + import bitops, chronicles, options, tables, ssz, beacon_chain_db, state_transition, extras, kvstore, @@ -336,13 +345,13 @@ proc add*( logScope: pcs = "block_addition" # Already seen this block?? - if blockRoot in pool.blocks: + pool.blocks.withValue(blockRoot, blockRef): debug "Block already exists", blck = shortLog(blck), blockRoot = shortLog(blockRoot), cat = "filtering" - return pool.blocks[blockRoot] + return blockRef[] pool.missing.del(blockRoot) @@ -547,7 +556,7 @@ func checkMissing*(pool: var BlockPool): seq[FetchRecord] = proc skipAndUpdateState( state: var HashedBeaconState, slot: Slot, - afterUpdate: proc (state: HashedBeaconState)) = + afterUpdate: proc (state: HashedBeaconState) {.gcsafe.}) = while state.data.slot < slot: # Process slots one at a time in case afterUpdate needs to see empty states process_slots(state, state.data.slot + 1) @@ -555,7 +564,7 @@ proc skipAndUpdateState( proc skipAndUpdateState( state: var HashedBeaconState, signedBlock: SignedBeaconBlock, flags: UpdateFlags, - afterUpdate: proc (state: HashedBeaconState)): bool = + afterUpdate: proc (state: HashedBeaconState) {.gcsafe.}): bool = skipAndUpdateState(state, signedBlock.message.slot - 1, afterUpdate) @@ -845,7 +854,10 @@ proc updateHead*(pool: BlockPool, newHead: BlockRef) = cat = "fork_choice" # A reasonable criterion for "reorganizations of the chain" - beacon_reorgs_total.inc() + try: + beacon_reorgs_total.inc() + except Exception as e: # TODO https://github.com/status-im/nim-metrics/pull/22 + trace "Couldn't update metrics", msg = e.msg else: info "Updated head block", stateRoot = shortLog(pool.headState.data.root), diff --git a/beacon_chain/time.nim b/beacon_chain/time.nim index 122988d88..fd8599bc9 100644 --- a/beacon_chain/time.nim +++ b/beacon_chain/time.nim @@ -1,3 +1,5 @@ +{.push raises: [Defect].} + import chronos, spec/datatypes @@ -79,7 +81,13 @@ func saturate*(d: tuple[inFuture: bool, offset: Duration]): Duration = if d.inFuture: d.offset else: seconds(0) proc addTimer*(fromNow: Duration, cb: CallbackFunc, udata: pointer = nil) = - discard setTimer(Moment.now() + fromNow, cb, udata) + try: + discard setTimer(Moment.now() + fromNow, cb, udata) + except Exception as e: + # TODO https://github.com/status-im/nim-chronos/issues/94 + # shouldn't happen because we should have initialized chronos by now + # https://github.com/nim-lang/Nim/issues/10288 - sigh + raiseAssert e.msg func shortLog*(d: Duration): string = let dd = int64(d.milliseconds())