[Tests] SHA256 + official shuffling vectors (#263)

* Change digests to SHA2-256 (from Keccak256)

* Fix sha256 digest initialization - pass shuffling test

* Add comments + remove old shuffling tests

* Remove stale tree hashing tests

* Small toOpenArray optim + notes on in-place hashing

* Revert "Revert "Update test repo""

This reverts commit f385467.

* Revert "Revert "Fix SIGFPE on shuffling for 0""

This reverts commit 226d380.

* Revert "Revert "Implement shuffling test (pending typedesc and shuffling algo fix)""

This reverts commit 813cb6f.

* withEth2hash templates now needs ctx.init()

* Use init-update-finish to avoid burnMem

beacon_chain/spec/digest.nim

@@ -7,54 +7,57 @@
 
 # Serenity hash function / digest
 #
-# https://github.com/ethereum/eth2.0-specs/blob/v0.6.0/specs/core/0_beacon-chain.md#hash
+# https://github.com/ethereum/eth2.0-specs/blob/v0.6.1/specs/core/0_beacon-chain.md#hash
 #
-# In Phase 0 the beacon chain is deployed with the same hash function as
-# Ethereum 1.0, i.e. Keccak-256 (also incorrectly known as SHA3).
+# In Phase 0 the beacon chain is deployed with SHA256 (SHA2-256).
+# Note that is is different from Keccak256 (often mistakenly called SHA3-256)
+# and SHA3-256.
 #
-# Note: We aim to migrate to a S[T/N]ARK-friendly hash function in a future
-# Ethereum 2.0 deployment phase.
+# In Eth1.0, the default hash function is Keccak256 and SHA256 is available as a precompiled contract.
 #
-# https://crypto.stackexchange.com/questions/15727/what-are-the-key-differences-between-the-draft-sha-3-standard-and-the-keccak-sub
-#
-# In our code base, to enable a smooth transition, we call this function
-# `eth2hash`, and it outputs a `Eth2Digest`. Easy to sed :)
+# In our code base, to enable a smooth transition
+# (already did Blake2b --> Keccak256 --> SHA2-256),
+# we call this function `eth2hash`, and it outputs a `Eth2Digest`. Easy to sed :)
 
 import
-  nimcrypto/[keccak, hash], eth/common/eth_types_json_serialization,
+  nimcrypto/[sha2, hash], eth/common/eth_types_json_serialization,
   hashes
 
 export
   eth_types_json_serialization, hash.`$`
 
 type
   Eth2Digest* = MDigest[32 * 8] ## `hash32` from spec
-  Eth2Hash* = keccak256 ## Context for hash function
+  Eth2Hash* = sha256            ## Context for hash function
 
 func shortLog*(x: Eth2Digest): string =
   # result = is needed to fix https://github.com/status-im/nim-beacon-chain/issues/209
   result = ($x)[0..7]
 
-func eth2hash*(v: openArray[byte]): Eth2Digest =
-  var ctx: keccak256 # use explicit type so we can rely on init being useless
-  # We can avoid this step for Keccak/SHA3 digests because `ctx` is already
-  # empty, but if digest will be changed next line must be enabled.
-  # ctx.init()
+# TODO: expose an in-place digest function
+#       when hashing in loop or into a buffer
+#       See: https://github.com/cheatfate/nimcrypto/blob/b90ba3abd/nimcrypto/sha2.nim#L570
+func eth2hash*(v: openArray[byte]): Eth2Digest {.inline.} =
+  # We use the init-update-finish interface to avoid
+  # the expensive burning/clearing memory (20~30% perf)
+  # TODO: security implication?
+  var ctx: sha256
+  ctx.init()
   ctx.update(v)
   result = ctx.finish()
 
 template withEth2Hash*(body: untyped): Eth2Digest =
   ## This little helper will init the hash function and return the sliced
   ## hash:
   ## let hashOfData = withHash: h.update(data)
-  var h  {.inject.}: keccak256
-  # TODO no need, as long as using keccak256: h.init()
+  var h  {.inject.}: sha256
+  h.init()
   body
   var res = h.finish()
   res
 
 func hash*(x: Eth2Digest): Hash =
-  ## Hash for Keccak digests for Nim hash tables
+  ## Hash for digests for Nim hash tables
   # Stub for BeaconChainDB
 
   # We just slice the first 4 or 8 bytes of the block hash

beacon_chain/spec/validator.nim

@@ -12,8 +12,9 @@ import
   ../ssz, ../beacon_node_types,
   ./crypto, ./datatypes, ./digest, ./helpers
 
-# https://github.com/ethereum/eth2.0-specs/blob/v0.5.0/specs/core/0_beacon-chain.md#get_shuffling
-# https://github.com/ethereum/eth2.0-specs/blob/v0.5.0/specs/core/0_beacon-chain.md#get_permuted_index
+# TODO: Proceed to renaming and signature changes
+# https://github.com/ethereum/eth2.0-specs/blob/v0.6.1/specs/core/0_beacon-chain.md#get_shuffled_index
+# https://github.com/ethereum/eth2.0-specs/blob/v0.6.1/specs/core/0_beacon-chain.md#compute_committee
 func get_shuffled_seq*(seed: Eth2Digest,
                        list_size: uint64,
                        ): seq[ValidatorIndex] =
@@ -25,8 +26,16 @@ func get_shuffled_seq*(seed: Eth2Digest,
   ##
   ## Invert the inner/outer loops from the spec, essentially. Most useful
   ## hash result re-use occurs within a round.
+
+  # Empty size -> empty list.
+  if list_size == 0:
+    return
+
   var
     # Share these buffers.
+    # TODO: Redo to follow spec.
+    #       We can have an "Impl" private version that takes buffer as parameters
+    #       so that we avoid alloc on repeated calls from compute_committee
     pivot_buffer: array[(32+1), byte]
     source_buffer: array[(32+1+4), byte]
     shuffled_active_validator_indices = mapIt(
@@ -44,7 +53,7 @@ func get_shuffled_seq*(seed: Eth2Digest,
     source_buffer[32] = round_bytes1
 
     # Only one pivot per round.
-    let pivot = bytes_to_int(eth2hash(pivot_buffer).data[0..7]) mod list_size
+    let pivot = bytes_to_int(eth2hash(pivot_buffer).data.toOpenArray(0, 7)) mod list_size
 
     ## Only need to run, per round, position div 256 hashes, so precalculate
     ## them. This consumes memory, but for low-memory devices, it's possible

beacon_chain/ssz.nim

@@ -10,7 +10,7 @@
 
 import
   endians, typetraits, options, algorithm, math,
-  faststreams/input_stream, serialization, eth/common, nimcrypto/keccak,
+  faststreams/input_stream, serialization, eth/common, nimcrypto/sha2,
   ./spec/[bitfield, crypto, datatypes, digest]
 
 # ################### Helper functions ###################################

tests/all_tests.nim

@@ -5,7 +5,7 @@
 #   * Apache v2 license (license terms in the root directory or at http://www.apache.org/licenses/LICENSE-2.0).
 # at your option. This file may not be copied, modified, or distributed except according to those terms.
 
-import
+import # Unit test
   ./test_attestation_pool,
   ./test_beacon_chain_db,
   ./test_beacon_node,
@@ -15,8 +15,10 @@ import
   ./test_helpers,
   ./test_ssz,
   ./test_state_transition,
-  ./test_sync_protocol,
-  ./test_validator
+  ./test_sync_protocol
+  # ./test_validator # Empty!
 
-  # TODO - re-enable once official test fixtures arrive
+import # Official fixtures
+  # TODO - re-enable
   #./official/test_fixture_state
+  ./official/test_fixture_shuffling

tests/official/state_test_utils.nim → tests/official/fixtures_utils.nim

@@ -12,14 +12,15 @@ export nimcrypto.toHex
 type
   # TODO: use ref object to avoid allocating
   #       so much on the stack - pending https://github.com/status-im/nim-json-serialization/issues/3
-  StateTest* = object
+  StateTests* = object
     title*: string
     summary*: string
     test_suite*: string
     fork*: string
-    test_cases*: seq[TestCase]
+    test_cases*: seq[StateTestCase]
 
   TestConstants* = object
+    # TODO - 0.5.1 constants
     SHARD_COUNT*: int
     TARGET_COMMITTEE_SIZE*: int
     MAX_BALANCE_CHURN_QUOTIENT*: int
@@ -66,13 +67,30 @@ type
     DOMAIN_VOLUNTARY_EXIT*: SignatureDomain
     DOMAIN_TRANSFER*: SignatureDomain
 
-  TestCase* = object
+  StateTestCase* = object
     name*: string
     config*: TestConstants
     verify_signatures*: bool
     initial_state*: BeaconState
     blocks*: seq[BeaconBlock]
     expected_state*: BeaconState
+
+  ShufflingTests* = object
+    title*: string
+    summary*: string
+    forks_timeline*: string
+    forks*: seq[string]
+    config*: string
+    runner*: string
+    handler*: string
+    test_cases*: seq[ShufflingTestCase]
+
+  ShufflingTestCase* = object
+    seed*: Eth2Digest
+    count*: uint64
+    shuffled*: seq[ValidatorIndex]
+
+  Tests* = StateTests or ShufflingTests
 
 # #######################
 # Default init
@@ -89,9 +107,27 @@ proc readValue*[N: static int](r: var JsonReader, a: var array[N, byte]) {.inlin
   #       if so export that to nim-eth
   hexToByteArray(r.readValue(string), a)
 
-proc parseStateTests*(jsonPath: string): StateTest =
+proc readValue*(r: var JsonReader, a: var ValidatorIndex) {.inline.} =
+  a = r.readValue(uint32)
+
+# TODO: cannot pass a typedesc
+# proc parseTests*(jsonPath: string, T: typedesc[Tests]): T =
+#   # TODO: due to generic early symbol resolution
+#   #       we cannot use a generic proc
+#   #       Otherwise we get:
+#   #       "Error: undeclared identifier: 'ReaderType'"
+#   #       Templates, even untyped don't work
+#   try:
+#     result = Json.loadFile(jsonPath, T)
+#   except SerializationError as err:
+#     writeStackTrace()
+#     stderr.write "Json load issue for file \"", jsonPath, "\"\n"
+#     stderr.write err.formatMsg(jsonPath), "\n"
+#     quit 1
+
+proc parseTests*(jsonPath: string): ShufflingTests =
   try:
-    result = Json.loadFile(jsonPath, StateTest)
+    result = Json.loadFile(jsonPath, ShufflingTests)
   except SerializationError as err:
     writeStackTrace()
     stderr.write "Json load issue for file \"", jsonPath, "\"\n"

tests/official/test_fixture_shuffling.nim

@@ -0,0 +1,30 @@
+# beacon_chain
+# Copyright (c) 2018 Status Research & Development GmbH
+# Licensed and distributed under either of
+#   * MIT license (license terms in the root directory or at http://opensource.org/licenses/MIT).
+#   * Apache v2 license (license terms in the root directory or at http://www.apache.org/licenses/LICENSE-2.0).
+# at your option. This file may not be copied, modified, or distributed except according to those terms.
+
+import
+  # Standard libs
+  ospaths, strutils, json, unittest,
+  # Third parties
+
+  # Beacon chain internals
+  ../../beacon_chain/spec/validator,
+  # Test utilities
+  ./fixtures_utils
+
+const TestFolder = currentSourcePath.rsplit(DirSep, 1)[0]
+const TestsPath = "fixtures" / "json_tests" / "shuffling" / "core" / "shuffling_full.json"
+
+var shufflingTests: ShufflingTests
+
+suite "Official - Shuffling tests":
+  test "Parsing the official shuffling tests":
+    shufflingTests = parseTests(TestFolder / TestsPath)
+
+  test "Shuffling a sequence of N validators":
+    for t in shufflingTests.test_cases:
+      let implResult = get_shuffled_seq(t.seed, t.count)
+      check: implResult == t.shuffled

tests/official/test_fixture_state.nim

@@ -14,17 +14,17 @@ import
   ../../beacon_chain/spec/[datatypes, crypto, digest, beaconstate],
   ../../beacon_chain/[ssz, state_transition],
   # Test utilities
-  ./state_test_utils
+  ./fixtures_utils
 
 const TestFolder = currentSourcePath.rsplit(DirSep, 1)[0]
 const TestsPath = "fixtures" / "json_tests" / "state" / "sanity-check_default-config_100-vals.json"
 
 
-var stateTests: StateTest
+var stateTests: StateTests
 suite "Official - State tests": # Initializing a beacon state from the deposits
   # Source: https://github.com/ethereum/eth2.0-specs/blob/2baa242ac004b0475604c4c4ef4315e14f56c5c7/tests/phase0/test_sanity.py#L55-L460
   test "Parsing the official state tests into Nimbus beacon types":
-    stateTests = parseStateTests(TestFolder / TestsPath)
+    stateTests = parseTests(TestFolder / TestsPath, StateTests) # TODO pending typedesc fix in fixture_utils.nim
     doAssert $stateTests.test_cases[0].name == "test_empty_block_transition"
 
   test "[For information - Non-blocking] Block root signing":

tests/test_ssz.nim

@@ -78,18 +78,18 @@ suite "Simple serialization":
     slot: 42.Slot, signature: sign(SigKey.random(), 0'u64, ""))
   SSZ.roundripTest BeaconState(slot: 42.Slot)
 
-suite "Tree hashing":
-  # TODO The test values are taken from an earlier version of SSZ and have
-  #      nothing to do with upstream - needs verification and proper test suite
+# suite "Tree hashing":
+#   # TODO The test values are taken from an earlier version of SSZ and have
+#   #      nothing to do with upstream - needs verification and proper test suite
 
-  test "Hash BeaconBlock":
-    let vr = BeaconBlock()
-    check:
-      $hash_tree_root(vr) ==
-        "8951C9C64ABA469EBA78F5D9F9A0666FB697B8C4D86901445777E4445D0B1543"
+#   test "Hash BeaconBlock":
+#     let vr = BeaconBlock()
+#     check:
+#       $hash_tree_root(vr) ==
+#         "8951C9C64ABA469EBA78F5D9F9A0666FB697B8C4D86901445777E4445D0B1543"
 
-  test "Hash BeaconState":
-    let vr = BeaconState()
-    check:
-      $hash_tree_root(vr) ==
-        "66F9BF92A690F1FBD36488D98BE70DA6C84100EDF935BC6D0B30FF14A2976455"
+#   test "Hash BeaconState":
+#     let vr = BeaconState()
+#     check:
+#       $hash_tree_root(vr) ==
+#         "66F9BF92A690F1FBD36488D98BE70DA6C84100EDF935BC6D0B30FF14A2976455"