From 87c92ea05fcebb8d7d2631c273785fec43eab5ba Mon Sep 17 00:00:00 2001
From: cheatfate <eugene.kabanov@status.im>
Date: Wed, 11 Nov 2020 15:39:36 +0200
Subject: [PATCH] Address review comments. Remove `-8.0` bucket because `-4.0`
 is a limit.

---
 beacon_chain/eth2_processor.nim   | 9 +++++----
 beacon_chain/time.nim             | 3 +++
 beacon_chain/validator_duties.nim | 8 +++-----
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/beacon_chain/eth2_processor.nim b/beacon_chain/eth2_processor.nim
index 7f247e2b8..4e0489f47 100644
--- a/beacon_chain/eth2_processor.nim
+++ b/beacon_chain/eth2_processor.nim
@@ -144,7 +144,8 @@ proc storeBlock(
   if blck.isErr:
     return err(blck.error[1])
 
-  beacon_store_block_duration_seconds.observe((Moment.now() - start).milliseconds.float64 / 1000)
+  let duration = (Moment.now() - start).toFloatSeconds()
+  beacon_store_block_duration_seconds.observe(duration)
   return ok()
 
 proc processAttestation(
@@ -271,7 +272,7 @@ proc blockValidator*(
     return blck.error[0]
 
   beacon_blocks_received.inc()
-  beacon_block_delay.observe(float(milliseconds(delay)) / 1000.0)
+  beacon_block_delay.observe(delay.toFloatSeconds())
 
   # Block passed validation - enqueue it for processing. The block processing
   # queue is effectively unbounded as we use a freestanding task to enqueue
@@ -312,7 +313,7 @@ proc attestationValidator*(
     return v.error[0]
 
   beacon_attestations_received.inc()
-  beacon_attestation_delay.observe(float(milliseconds(delay)) / 1000.0)
+  beacon_attestation_delay.observe(delay.toFloatSeconds())
 
   while self.attestationsQueue.full():
     let dropped = self.attestationsQueue.popFirst()
@@ -355,7 +356,7 @@ proc aggregateValidator*(
     return v.error[0]
 
   beacon_aggregates_received.inc()
-  beacon_aggregate_delay.observe(float(milliseconds(delay)) / 1000.0)
+  beacon_aggregate_delay.observe(delay.toFloatSeconds())
 
   while self.aggregatesQueue.full():
     let dropped = self.aggregatesQueue.popFirst()
diff --git a/beacon_chain/time.nim b/beacon_chain/time.nim
index 5a39d59a4..78becdabd 100644
--- a/beacon_chain/time.nim
+++ b/beacon_chain/time.nim
@@ -124,5 +124,8 @@ func shortLog*(d: Duration): string =
       tmp &= $frac & "m"
     tmp
 
+func toFloatSeconds*(d: Duration): float =
+  float(milliseconds(d)) / 1000.0
+
 func `$`*(v: BeaconTime): string = $Duration(v)
 func shortLog*(v: BeaconTime): Duration = Duration(v)
diff --git a/beacon_chain/validator_duties.nim b/beacon_chain/validator_duties.nim
index 5a214a344..eee38a531 100644
--- a/beacon_chain/validator_duties.nim
+++ b/beacon_chain/validator_duties.nim
@@ -28,7 +28,7 @@ import
   validator_slashing_protection
 
 # Metrics for tracking attestation and beacon block loss
-const delayBuckets = [-Inf, -8.0, -4.0, -2.0, -1.0, -0.5, -0.1, -0.05,
+const delayBuckets = [-Inf, -4.0, -2.0, -1.0, -0.5, -0.1, -0.05,
                       0.05, 0.1, 0.5, 1.0, 2.0, 4.0, 8.0, Inf]
 
 declareCounter beacon_attestations_sent,
@@ -174,11 +174,9 @@ proc createAndSendAttestation(node: BeaconNode,
 
   let (delayStr, delayMillis) =
     if wallTime < deadline:
-      ("-" & $(deadline - wallTime),
-       -float(milliseconds(deadline - wallTime)) / 1000.0)
+      ("-" & $(deadline - wallTime), -toFloatSeconds(deadline - wallTime))
     else:
-      ($(wallTime - deadline),
-       float(milliseconds(wallTime - deadline)) / 1000.0)
+      ($(wallTime - deadline), toFloatSeconds(wallTime - deadline))
 
   notice "Attestation sent", attestation = shortLog(attestation),
                              validator = shortLog(validator), delay = delayStr,