From e661f7d0c78fad815cd37c2f6bd12730a0cec680 Mon Sep 17 00:00:00 2001 From: tersec Date: Mon, 1 Mar 2021 19:55:25 +0000 Subject: [PATCH] prevent uint64 to int64-induced RangeError/RangeDefects in metrics (#2358) * prevent uint64 to int64-induced RangeError/RangeDefects in metrics * remove redundant min(foo, int64.high) * adjust spacing to be consistent --- beacon_chain/block_pools/chain_dag.nim | 22 +++++++++++----------- beacon_chain/eth1_monitor.nim | 6 ------ beacon_chain/nimbus_beacon_node.nim | 8 +++++--- beacon_chain/spec/datatypes.nim | 6 ++++++ beacon_chain/validator_duties.nim | 2 +- 5 files changed, 23 insertions(+), 21 deletions(-) diff --git a/beacon_chain/block_pools/chain_dag.nim b/beacon_chain/block_pools/chain_dag.nim index 27fce844a..7d138f97e 100644 --- a/beacon_chain/block_pools/chain_dag.nim +++ b/beacon_chain/block_pools/chain_dag.nim @@ -885,32 +885,32 @@ proc updateHead*( finalized = shortLog(dag.headState.data.data.finalized_checkpoint) # https://github.com/ethereum/eth2.0-metrics/blob/master/metrics.md#additional-metrics - if dag.headState.data.data.eth1_data.deposit_count < high(int64).uint64: - beacon_pending_deposits.set( - dag.headState.data.data.eth1_data.deposit_count.int64 - - dag.headState.data.data.eth1_deposit_index.int64) - beacon_processed_deposits_total.set( - dag.headState.data.data.eth1_deposit_index.int64) + # both non-negative, so difference can't overflow or underflow int64 + beacon_pending_deposits.set( + dag.headState.data.data.eth1_data.deposit_count.toGaugeValue - + dag.headState.data.data.eth1_deposit_index.toGaugeValue) + beacon_processed_deposits_total.set( + dag.headState.data.data.eth1_deposit_index.toGaugeValue) beacon_head_root.set newHead.root.toGaugeValue - beacon_head_slot.set newHead.slot.int64 + beacon_head_slot.set newHead.slot.toGaugeValue if lastHead.slot.epoch != newHead.slot.epoch: # Epoch updated - in theory, these could happen when the wall clock # changes epoch, even if there is no new block / head, but we'll delay # updating them until a block confirms the change beacon_current_justified_epoch.set( - dag.headState.data.data.current_justified_checkpoint.epoch.int64) + dag.headState.data.data.current_justified_checkpoint.epoch.toGaugeValue) beacon_current_justified_root.set( dag.headState.data.data.current_justified_checkpoint.root.toGaugeValue) beacon_previous_justified_epoch.set( - dag.headState.data.data.previous_justified_checkpoint.epoch.int64) + dag.headState.data.data.previous_justified_checkpoint.epoch.toGaugeValue) beacon_previous_justified_root.set( dag.headState.data.data.previous_justified_checkpoint.root.toGaugeValue) let epochRef = getEpochRef(dag, newHead, newHead.slot.epoch) beacon_active_validators.set( - epochRef.shuffled_active_validator_indices.lenu64().int64) + epochRef.shuffled_active_validator_indices.lenu64().toGaugeValue) if finalizedHead != dag.finalizedHead: block: # Remove states, walking slot by slot @@ -977,7 +977,7 @@ proc updateHead*( heads = dag.heads.len beacon_finalized_epoch.set( - dag.headState.data.data.finalized_checkpoint.epoch.int64) + dag.headState.data.data.finalized_checkpoint.epoch.toGaugeValue) beacon_finalized_root.set( dag.headState.data.data.finalized_checkpoint.root.toGaugeValue) diff --git a/beacon_chain/eth1_monitor.nim b/beacon_chain/eth1_monitor.nim index 9b26363c1..f2a4e2b9b 100644 --- a/beacon_chain/eth1_monitor.nim +++ b/beacon_chain/eth1_monitor.nim @@ -260,12 +260,6 @@ proc fixupWeb3Urls*(web3Url: var string) = web3Url = "ws://" & normalizedUrl.substr(pos) warn "Only WebSocket web3 providers are supported. Rewriting URL", web3Url -func toGaugeValue(x: uint64): int64 = - if x > uint64(int64.high): - int64.high - else: - int64(x) - template toGaugeValue(x: Quantity): int64 = toGaugeValue(distinctBase x) diff --git a/beacon_chain/nimbus_beacon_node.nim b/beacon_chain/nimbus_beacon_node.nim index 057fe403f..0cd2ef263 100644 --- a/beacon_chain/nimbus_beacon_node.nim +++ b/beacon_chain/nimbus_beacon_node.nim @@ -963,10 +963,12 @@ proc onSlotStart( # Check before any re-scheduling of onSlotStart() checkIfShouldStopAtEpoch(wallSlot, node.config.stopAtEpoch) - beacon_slot.set wallSlot.int64 - beacon_current_epoch.set wallSlot.epoch.int64 + beacon_slot.set wallSlot.toGaugeValue + beacon_current_epoch.set wallSlot.epoch.toGaugeValue - finalization_delay.set wallSlot.epoch.int64 - finalizedEpoch.int64 + # both non-negative, so difference can't overflow or underflow int64 + finalization_delay.set( + wallSlot.epoch.toGaugeValue - finalizedEpoch.toGaugeValue) if node.config.verifyFinalization: verifyFinalization(node, wallSlot) diff --git a/beacon_chain/spec/datatypes.nim b/beacon_chain/spec/datatypes.nim index e3b1f23b0..96520dd2b 100644 --- a/beacon_chain/spec/datatypes.nim +++ b/beacon_chain/spec/datatypes.nim @@ -852,6 +852,12 @@ template lenu64*(x: untyped): untyped = func `$`*(v: ForkDigest | Version): string = toHex(array[4, byte](v)) +func toGaugeValue*(x: uint64 | Epoch | Slot): int64 = + if x > uint64(int64.high): + int64.high + else: + int64(x) + # TODO where's borrow support when you need it func `==`*(a, b: ForkDigest | Version): bool = array[4, byte](a) == array[4, byte](b) diff --git a/beacon_chain/validator_duties.nim b/beacon_chain/validator_duties.nim index 8df9604a4..423d80e02 100644 --- a/beacon_chain/validator_duties.nim +++ b/beacon_chain/validator_duties.nim @@ -589,7 +589,7 @@ proc updateValidatorMetrics*(node: BeaconNode) = total += balance node.attachedValidatorBalanceTotal = total - attached_validator_balance_total.set(min(total, int64.high.uint64).int64) + attached_validator_balance_total.set(total.toGaugeValue) else: discard