From 21c4ce8fd4e6e76bd67e82980da56af981b47495 Mon Sep 17 00:00:00 2001
From: tersec <tersec@users.noreply.github.com>
Date: Mon, 16 Nov 2020 17:10:51 +0100
Subject: [PATCH] remove superfluous TODOs/not-really-TODOs, type conversion,
 imports (#2025)

---
 beacon_chain/block_pools/block_pools_types.nim | 2 +-
 beacon_chain/block_pools/spec_cache.nim        | 2 --
 beacon_chain/eth2_network.nim                  | 2 +-
 beacon_chain/nimbus_beacon_node.nim            | 2 +-
 beacon_chain/nimbus_validator_client.nim       | 2 +-
 beacon_chain/rpc/beacon_api.nim                | 4 ++--
 beacon_chain/ssz/bitseqs.nim                   | 2 +-
 beacon_chain/ssz/types.nim                     | 4 ++--
 beacon_chain/validator_pool.nim                | 2 +-
 tests/mocking/mock_deposits.nim                | 2 +-
 tests/test_interop.nim                         | 2 +-
 tests/testblockutil.nim                        | 2 +-
 12 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/beacon_chain/block_pools/block_pools_types.nim b/beacon_chain/block_pools/block_pools_types.nim
index f7e556b13..11c92a2ba 100644
--- a/beacon_chain/block_pools/block_pools_types.nim
+++ b/beacon_chain/block_pools/block_pools_types.nim
@@ -162,7 +162,7 @@ type
     parent*: BlockRef ##\
     ## Not nil, except for the tail
 
-    slot*: Slot # TODO could calculate this by walking to root, but..
+    slot*: Slot # could calculate this by walking to root, but..
 
     epochRefs*: seq[EpochRef] ##\
     ## Cached information about the epochs starting at this block.
diff --git a/beacon_chain/block_pools/spec_cache.nim b/beacon_chain/block_pools/spec_cache.nim
index 02669e71a..fe3a0bb50 100644
--- a/beacon_chain/block_pools/spec_cache.nim
+++ b/beacon_chain/block_pools/spec_cache.nim
@@ -139,7 +139,6 @@ proc is_valid_indexed_attestation*(
 
   # Verify aggregate signature
   if skipBLSValidation notin flags:
-     # TODO: fuse loops with blsFastAggregateVerify
     let pubkeys = mapIt(
       indexed_attestation.attesting_indices, epochRef.validator_keys[it])
     if not verify_attestation_signature(
@@ -164,7 +163,6 @@ proc is_valid_indexed_attestation*(
 
   # Verify aggregate signature
   if skipBLSValidation notin flags:
-     # TODO: fuse loops with blsFastAggregateVerify
     let pubkeys = mapIt(
       attesting_indices, epochRef.validator_keys[it])
     if not verify_attestation_signature(
diff --git a/beacon_chain/eth2_network.nim b/beacon_chain/eth2_network.nim
index 73fe94676..53a70c76a 100644
--- a/beacon_chain/eth2_network.nim
+++ b/beacon_chain/eth2_network.nim
@@ -494,7 +494,7 @@ template errorMsgLit(x: static string): ErrorMsg =
   const val = ErrorMsg toBytes(x)
   val
 
-proc formatErrorMsg(msg: ErrorMSg): string =
+func formatErrorMsg(msg: ErrorMSg): string =
   let candidate = string.fromBytes(asSeq(msg))
   for c in candidate:
     # TODO UTF-8 - but let's start with ASCII
diff --git a/beacon_chain/nimbus_beacon_node.nim b/beacon_chain/nimbus_beacon_node.nim
index e6386aaa5..c93d6b681 100644
--- a/beacon_chain/nimbus_beacon_node.nim
+++ b/beacon_chain/nimbus_beacon_node.nim
@@ -29,7 +29,7 @@ import
   beacon_node_common, beacon_node_types, beacon_node_status,
   block_pools/[chain_dag, quarantine, clearance, block_pools_types],
   nimbus_binary_common, network_metadata,
-  eth1_monitor, version, ssz/[merkleization], merkle_minimal,
+  eth1_monitor, version, ssz/merkleization,
   sync_protocol, request_manager, keystore_management, interop, statusbar,
   sync_manager, validator_duties, filepath,
   validator_slashing_protection, ./eth2_processor
diff --git a/beacon_chain/nimbus_validator_client.nim b/beacon_chain/nimbus_validator_client.nim
index b986d7736..d6f414095 100644
--- a/beacon_chain/nimbus_validator_client.nim
+++ b/beacon_chain/nimbus_validator_client.nim
@@ -201,7 +201,7 @@ proc onSlotStart(vc: ValidatorClient, lastSlot, scheduledSlot: Slot) {.gcsafe, a
 
           # TODO I don't like these (u)int64-to-int conversions...
           let attestation = await validator.produceAndSignAttestation(
-            ad, a.committee_length.int, a.validator_committee_index.int,
+            ad, a.committee_length.int, a.validator_committee_index,
             vc.fork, vc.beaconGenesis.genesis_validators_root)
 
           discard await vc.client.post_v1_beacon_pool_attestations(attestation)
diff --git a/beacon_chain/rpc/beacon_api.nim b/beacon_chain/rpc/beacon_api.nim
index 9556f57d8..4b9b3f7ee 100644
--- a/beacon_chain/rpc/beacon_api.nim
+++ b/beacon_chain/rpc/beacon_api.nim
@@ -187,7 +187,7 @@ proc installBeaconApiHandlers*(rpcServer: RpcServer, node: BeaconNode) =
       proc forSlot(slot: Slot, res: var seq[BeaconStatesCommitteesTuple]) =
         let committees_per_slot =
           get_committee_count_per_slot(state, slot.epoch, cache)
-        if index == 0: # TODO this means if the parameter is missing (its optional)
+        if index == 0: # parameter is missing (it's optional)
           for committee_index in 0'u64..<committees_per_slot:
             res.add(getCommittee(slot, committee_index.CommitteeIndex))
         else:
@@ -195,7 +195,7 @@ proc installBeaconApiHandlers*(rpcServer: RpcServer, node: BeaconNode) =
             raise newException(ValueError, "Committee index out of bounds")
           res.add(getCommittee(slot, index.CommitteeIndex))
 
-      if slot == 0: # TODO this means if the parameter is missing (its optional)
+      if slot == 0: # parameter is missing (it's optional)
         for i in 0 ..< SLOTS_PER_EPOCH:
           forSlot(compute_start_slot_at_epoch(epoch.Epoch) + i, result)
       else:
diff --git a/beacon_chain/ssz/bitseqs.nim b/beacon_chain/ssz/bitseqs.nim
index f56175868..75e4d785a 100644
--- a/beacon_chain/ssz/bitseqs.nim
+++ b/beacon_chain/ssz/bitseqs.nim
@@ -93,7 +93,7 @@ template loopOverWords(lhs, rhs: BitSeq,
       var rhsWord: WordType
     var firstByteOfLastWord, lastByteOfLastWord: int
 
-    # TODO: Returing a `var` value from an iterator is always safe due to
+    # TODO: Returning a `var` value from an iterator is always safe due to
     # the way inlining works, but currently the compiler reports an error
     # when a local variable escapes. We have to cheat it with this location
     # obfuscation through pointers:
diff --git a/beacon_chain/ssz/types.nim b/beacon_chain/ssz/types.nim
index e7f516869..7b53c7a16 100644
--- a/beacon_chain/ssz/types.nim
+++ b/beacon_chain/ssz/types.nim
@@ -131,8 +131,8 @@ template bytes*(x: BitList): auto = seq[byte](x)
 template `[]`*(x: BitList, idx: auto): auto = BitSeq(x)[idx]
 template `[]=`*(x: var BitList, idx: auto, val: bool) = BitSeq(x)[idx] = val
 template `==`*(a, b: BitList): bool = BitSeq(a) == BitSeq(b)
-template setBit*(x: var BitList, idx: int) = setBit(BitSeq(x), idx)
-template clearBit*(x: var BitList, idx: int) = clearBit(BitSeq(x), idx)
+template setBit*(x: var BitList, idx: Natural) = setBit(BitSeq(x), idx)
+template clearBit*(x: var BitList, idx: Natural) = clearBit(BitSeq(x), idx)
 template overlaps*(a, b: BitList): bool = overlaps(BitSeq(a), BitSeq(b))
 template combine*(a: var BitList, b: BitList) = combine(BitSeq(a), BitSeq(b))
 template isSubsetOf*(a, b: BitList): bool = isSubsetOf(BitSeq(a), BitSeq(b))
diff --git a/beacon_chain/validator_pool.nim b/beacon_chain/validator_pool.nim
index 3a327bba5..59bcadfaa 100644
--- a/beacon_chain/validator_pool.nim
+++ b/beacon_chain/validator_pool.nim
@@ -74,7 +74,7 @@ proc signAttestation*(v: AttachedValidator,
 
 proc produceAndSignAttestation*(validator: AttachedValidator,
                                 attestationData: AttestationData,
-                                committeeLen: int, indexInCommittee: int,
+                                committeeLen: int, indexInCommittee: Natural,
                                 fork: Fork, genesis_validators_root: Eth2Digest):
                                 Future[Attestation] {.async.} =
   let validatorSignature = await validator.signAttestation(attestationData,
diff --git a/tests/mocking/mock_deposits.nim b/tests/mocking/mock_deposits.nim
index 8043e314d..ec26532ec 100644
--- a/tests/mocking/mock_deposits.nim
+++ b/tests/mocking/mock_deposits.nim
@@ -10,7 +10,7 @@
 
 import
   # Standard library
-  math, random,
+  math,
 
   # Specs
   ../../beacon_chain/spec/[datatypes, crypto, digest,
diff --git a/tests/test_interop.nim b/tests/test_interop.nim
index 2ae93bf94..2dda68dcd 100644
--- a/tests/test_interop.nim
+++ b/tests/test_interop.nim
@@ -2,7 +2,7 @@
 
 import
   unittest, stint, ./testutil, stew/byteutils,
-  ../beacon_chain/[interop, merkle_minimal, ssz],
+  ../beacon_chain/[interop, ssz],
   ../beacon_chain/spec/[beaconstate, crypto, datatypes, presets]
 
 # Interop test yaml, found here:
diff --git a/tests/testblockutil.nim b/tests/testblockutil.nim
index 051987bc7..e00389d9d 100644
--- a/tests/testblockutil.nim
+++ b/tests/testblockutil.nim
@@ -7,7 +7,7 @@
 
 import
   options, stew/endians2,
-  ../beacon_chain/[extras, merkle_minimal, validator_pool],
+  ../beacon_chain/[extras, validator_pool],
   ../beacon_chain/ssz/merkleization,
   ../beacon_chain/spec/[beaconstate, crypto, datatypes, digest, presets,
                         helpers, validator, signatures, state_transition]