call out inefficiencies and quirks for future consideration

beacon_chain/spec/beaconstate.nim

@@ -114,16 +114,26 @@ func append_to_recent_block_hashes*(old_block_hashes: seq[Eth2Digest],
 proc get_attestation_participants*(state: BeaconState,
                                    attestation_data: AttestationSignedData,
                                    attester_bitfield: seq[byte]): seq[int] =
+  ## Attestation participants in the attestation data are called out in a
+  ## bit field that corresponds to the committee of the shard at the time - this
+  ## function converts it to list of indices in to BeaconState.validators
+  ## Returns empty list if the shard is not found
+  # XXX Linear search through shard list? borderline ok, it's a small list
+  # XXX bitfield type needed, once bit order settles down
+  # XXX iterator candidate
   let
     sncs_for_slot = get_shards_and_committees_for_slot(
       state, attestation_data.slot)
 
   for snc in sncs_for_slot:
-      if snc.shard == attestation_data.shard:
+    if snc.shard != attestation_data.shard:
+      continue
+
+    # XXX investigate functional library / approach to help avoid loop bugs
     assert len(attester_bitfield) == ceil_div8(len(snc.committee))
     for i, vindex in snc.committee:
       let
         bit = (attester_bitfield[i div 8] shr (7 - (i mod 8))) mod 2
       if bit == 1:
           result.add(vindex)
-          return
+    return # found the shard, we're done

beacon_chain/spec/datatypes.nim

@@ -17,6 +17,11 @@
 #
 # How wrong the code is:
 # https://github.com/ethereum/eth2.0-specs/compare/126a7abfa86448091a0e037f52966b6a9531a857...master
+#
+# These datatypes are used as specifications for serialization - thus should not
+# be altered outside of what the spec says. Likewise, they should not be made
+# `ref` - this can be achieved by wrapping them in higher-level
+# types / composition
 
 import
   intsets, eth_common, math,

beacon_chain/spec/helpers.nim

@@ -76,4 +76,4 @@ func get_new_recent_block_hashes*(old_block_hashes: seq[Eth2Digest],
   for _ in 0 ..< min(d, old_block_hashes.len):
     result.add parent_hash
 
-func ceil_div8*(v: int): int = (v + 7) div 8
+func ceil_div8*(v: int): int = (v + 7) div 8 # XXX use a proper bitarray!

beacon_chain/spec/validator.nim

@@ -26,6 +26,8 @@ func get_new_validators*(current_validators: seq[ValidatorRecord],
                          status: ValidatorStatusCodes,
                          current_slot: uint64
                          ): tuple[validators: seq[ValidatorRecord], index: int] =
+  # XXX Spec candidate: inefficient API
+  #
   # Check that validator really did register
   # let signed_message = signed_message = bytes32(pubkey) + withdrawal_credentials + randao_commitment
   # assert BLSVerify(pub=pubkey,