remove Nim 1.2 workarounds and `TODO`s (#4533)

* clean up some Nim 1.2 workarounds

* re-add notes about JS backend

* another proc/noSideEffect -> func

* revert ncli/ncli_common.nim changes; 19969 evidently wasn't backported to 1.6

beacon_chain/beacon_chain_db.nim

@@ -1128,8 +1128,6 @@ proc getStateOnlyMutableValidators(
   ## not found at all, rollback will not be called
   # TODO rollback is needed to deal with bug - use `noRollback` to ignore:
   #      https://github.com/nim-lang/Nim/issues/14126
-  # TODO RVO is inefficient for large objects:
-  #      https://github.com/nim-lang/Nim/issues/13879
 
   case store.getSnappySSZ(key, toBeaconStateNoImmutableValidators(output))
   of GetResult.found:
@@ -1169,8 +1167,6 @@ proc getStateOnlyMutableValidators(
   ## not found at all, rollback will not be called
   # TODO rollback is needed to deal with bug - use `noRollback` to ignore:
   #      https://github.com/nim-lang/Nim/issues/14126
-  # TODO RVO is inefficient for large objects:
-  #      https://github.com/nim-lang/Nim/issues/13879
 
   case store.getSZSSZ(key, toBeaconStateNoImmutableValidators(output))
   of GetResult.found:
@@ -1208,8 +1204,6 @@ proc getStateOnlyMutableValidators(
   ## not found at all, rollback will not be called
   # TODO rollback is needed to deal with bug - use `noRollback` to ignore:
   #      https://github.com/nim-lang/Nim/issues/14126
-  # TODO RVO is inefficient for large objects:
-  #      https://github.com/nim-lang/Nim/issues/13879
 
   case store.getSZSSZ(key, toBeaconStateNoImmutableValidators(output))
   of GetResult.found:
@@ -1270,8 +1264,6 @@ proc getState*(
   ## not found at all, rollback will not be called
   # TODO rollback is needed to deal with bug - use `noRollback` to ignore:
   #      https://github.com/nim-lang/Nim/issues/14126
-  # TODO RVO is inefficient for large objects:
-  #      https://github.com/nim-lang/Nim/issues/13879
   type T = type(output)
 
   if not getStateOnlyMutableValidators(
@@ -1293,8 +1285,6 @@ proc getState*(
   ## not found at all, rollback will not be called
   # TODO rollback is needed to deal with bug - use `noRollback` to ignore:
   #      https://github.com/nim-lang/Nim/issues/14126
-  # TODO RVO is inefficient for large objects:
-  #      https://github.com/nim-lang/Nim/issues/13879
   type T = type(output)
   getStateOnlyMutableValidators(
     db.immutableValidators, db.statesNoVal[T.toFork], key.data, output,

beacon_chain/consensus_object_pools/attestation_pool.nim

@@ -664,11 +664,7 @@ proc getAttestationsForBlock*(pool: var AttestationPool,
           # Fast path for when all remaining candidates fit
           if candidates.lenu64 < MAX_ATTESTATIONS: candidates.len - 1
           else: maxIndex(candidates)
-        # TODO slot not used; replace with _ when
-        # https://github.com/nim-lang/Nim/issues/15972 and
-        # https://github.com/nim-lang/Nim/issues/16217 are
-        # fixed in Status's Nim.
-        (_, slot, entry, j) = candidates[candidate]
+        (_, _, entry, j) = candidates[candidate]
 
       candidates.del(candidate) # careful, `del` reorders candidates
 

beacon_chain/eth1/eth1_monitor.nim

@@ -642,7 +642,7 @@ proc forkchoiceUpdated*(
         withdrawals: mapIt(withdrawals.get, it.asEngineWithdrawal))))
 
 # TODO can't be defined within exchangeTransitionConfiguration
-proc `==`(x, y: Quantity): bool {.borrow, noSideEffect.}
+func `==`(x, y: Quantity): bool {.borrow.}
 
 type
   EtcStatus {.pure.} = enum

beacon_chain/spec/beacon_time.nim

@@ -50,46 +50,44 @@ const
 
   NANOSECONDS_PER_SLOT = SECONDS_PER_SLOT * 1_000_000_000'u64
 
-# TODO when https://github.com/nim-lang/Nim/issues/14440 lands in Status's Nim,
-# switch proc {.noSideEffect.} to func.
 template ethTimeUnit*(typ: type) {.dirty.} =
-  proc `+`*(x: typ, y: uint64): typ {.borrow, noSideEffect.}
-  proc `-`*(x: typ, y: uint64): typ {.borrow, noSideEffect.}
-  proc `-`*(x: uint64, y: typ): typ {.borrow, noSideEffect.}
+  func `+`*(x: typ, y: uint64): typ {.borrow.}
+  func `-`*(x: typ, y: uint64): typ {.borrow.}
+  func `-`*(x: uint64, y: typ): typ {.borrow.}
 
   # Not closed over type in question (Slot or Epoch)
-  proc `mod`*(x: typ, y: uint64): uint64 {.borrow, noSideEffect.}
-  proc `div`*(x: typ, y: uint64): uint64 {.borrow, noSideEffect.}
-  proc `div`*(x: uint64, y: typ): uint64 {.borrow, noSideEffect.}
-  proc `-`*(x: typ, y: typ): uint64 {.borrow, noSideEffect.}
+  func `mod`*(x: typ, y: uint64): uint64 {.borrow.}
+  func `div`*(x: typ, y: uint64): uint64 {.borrow.}
+  func `div`*(x: uint64, y: typ): uint64 {.borrow.}
+  func `-`*(x: typ, y: typ): uint64 {.borrow.}
 
   iterator countdown*(a, b: typ, step: Positive = 1): typ =
     # otherwise we use the signed version that breaks at the boundary
     for i in countdown(distinctBase(a), distinctBase(b), step):
       yield typ(i)
 
-  proc `*`*(x: typ, y: uint64): uint64 {.borrow, noSideEffect.}
+  func `*`*(x: typ, y: uint64): uint64 {.borrow.}
 
-  proc `+=`*(x: var typ, y: typ) {.borrow, noSideEffect.}
-  proc `+=`*(x: var typ, y: uint64) {.borrow, noSideEffect.}
-  proc `-=`*(x: var typ, y: typ) {.borrow, noSideEffect.}
-  proc `-=`*(x: var typ, y: uint64) {.borrow, noSideEffect.}
+  func `+=`*(x: var typ, y: typ) {.borrow.}
+  func `+=`*(x: var typ, y: uint64) {.borrow.}
+  func `-=`*(x: var typ, y: typ) {.borrow.}
+  func `-=`*(x: var typ, y: uint64) {.borrow.}
 
   # Comparison operators
-  proc `<`*(x: typ, y: typ): bool {.borrow, noSideEffect.}
-  proc `<`*(x: typ, y: uint64): bool {.borrow, noSideEffect.}
-  proc `<`*(x: uint64, y: typ): bool {.borrow, noSideEffect.}
-  proc `<=`*(x: typ, y: typ): bool {.borrow, noSideEffect.}
-  proc `<=`*(x: typ, y: uint64): bool {.borrow, noSideEffect.}
-  proc `<=`*(x: uint64, y: typ): bool {.borrow, noSideEffect.}
+  func `<`*(x: typ, y: typ): bool {.borrow.}
+  func `<`*(x: typ, y: uint64): bool {.borrow.}
+  func `<`*(x: uint64, y: typ): bool {.borrow.}
+  func `<=`*(x: typ, y: typ): bool {.borrow.}
+  func `<=`*(x: typ, y: uint64): bool {.borrow.}
+  func `<=`*(x: uint64, y: typ): bool {.borrow.}
 
-  proc `==`*(x: typ, y: typ): bool {.borrow, noSideEffect.}
-  proc `==`*(x: typ, y: uint64): bool {.borrow, noSideEffect.}
-  proc `==`*(x: uint64, y: typ): bool {.borrow, noSideEffect.}
+  func `==`*(x: typ, y: typ): bool {.borrow.}
+  func `==`*(x: typ, y: uint64): bool {.borrow.}
+  func `==`*(x: uint64, y: typ): bool {.borrow.}
 
   # Nim integration
-  proc `$`*(x: typ): string {.borrow, noSideEffect.}
-  proc hash*(x: typ): Hash {.borrow, noSideEffect.}
+  func `$`*(x: typ): string {.borrow.}
+  func hash*(x: typ): Hash {.borrow.}
 
   template asUInt64*(v: typ): uint64 = distinctBase(v)
   template shortLog*(v: typ): auto = distinctBase(v)

beacon_chain/spec/helpers.nim

@@ -136,9 +136,8 @@ func compute_domain*(
     genesis_validators_root: Eth2Digest = ZERO_HASH): Eth2Domain =
   ## Return the domain for the ``domain_type`` and ``fork_version``.
   #
-  # TODO Can't be used as part of a const/static expression:
+  # TODO toOpenArray can't be used from JavaScript backend
   # https://github.com/nim-lang/Nim/issues/15952
-  # https://github.com/nim-lang/Nim/issues/19969
   let fork_data_root =
     compute_fork_data_root(fork_version, genesis_validators_root)
   result[0..3] = domain_type.data