From e4f4cb15137d08f6cf1cd95da908b6734a56f4c9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C8=98tefan=20Talpalaru?= <stefantalpalaru@yahoo.com>
Date: Sun, 14 Feb 2021 21:04:54 +0100
Subject: [PATCH] remove "beacon_chain.nimble" (#2316)

Tests have been moved to the Makefile and the test binaries are built in
parallel.

The Nim compilation scheme has been moved to a script and adapted to
work with different binaries being built in parallel from the same main
source file (think minimal/mainnet tests).
---
 .gitignore                     |   3 -
 Makefile                       | 167 +++++++++++++++++++++++++++++----
 beacon_chain.nimble            | 100 --------------------
 config.nims                    |  10 +-
 scripts/compile_nim_program.sh |  23 +++++
 5 files changed, 177 insertions(+), 126 deletions(-)
 delete mode 100644 beacon_chain.nimble
 create mode 100755 scripts/compile_nim_program.sh

diff --git a/.gitignore b/.gitignore
index 6ff1b566e..7f5040936 100644
--- a/.gitignore
+++ b/.gitignore
@@ -24,9 +24,6 @@ build/
 *.exe
 *.dll
 
-# a symlink that can't be added to the repo because of Windows
-/beacon_chain.nims
-
 /scripts/testnet*.sh
 
 # State sim # TODO - move in another folder
diff --git a/Makefile b/Makefile
index 896a6b8f7..0ca98d9db 100644
--- a/Makefile
+++ b/Makefile
@@ -37,7 +37,6 @@ endif
 # TODO re-enable ncli_query if/when it works again
 TOOLS := \
 	nimbus_beacon_node \
-	block_sim \
 	deposit_contract \
 	inspector \
 	logtrace \
@@ -47,9 +46,9 @@ TOOLS := \
 	ncli_db \
 	process_dashboard \
 	stack_sizes \
-	state_sim \
 	nimbus_validator_client \
 	nimbus_signing_process
+.PHONY: $(TOOLS)
 
 # bench_bls_sig_agggregation TODO reenable after bls v0.10.1 changes
 
@@ -67,7 +66,6 @@ TOOLS_CSV := $(subst $(SPACE),$(COMMA),$(TOOLS))
 	deps \
 	update \
 	test \
-	$(TOOLS) \
 	clean_eth2_network_simulation_all \
 	eth2_network_simulation \
 	clean-testnet0 \
@@ -120,41 +118,172 @@ DEPOSITS_DELAY := 0
 
 #- "--define:release" cannot be added to "config.nims"
 #- disable Nim's default parallelisation because it starts too many processes for too little gain
-NIM_PARAMS := $(NIM_PARAMS) -d:release --parallelBuild:1
+NIM_PARAMS += -d:release --parallelBuild:1
 
 ifeq ($(USE_LIBBACKTRACE), 0)
 # Blame Jacek for the lack of line numbers in your stack traces ;-)
-NIM_PARAMS := $(NIM_PARAMS) --stacktrace:on --excessiveStackTrace:on --linetrace:off -d:disable_libbacktrace
+NIM_PARAMS += --stacktrace:on --excessiveStackTrace:on --linetrace:off -d:disable_libbacktrace
 endif
 
-deps: | deps-common nat-libs beacon_chain.nims build/generate_makefile
+deps: | deps-common nat-libs build/generate_makefile
 ifneq ($(USE_LIBBACKTRACE), 0)
 deps: | libbacktrace
 endif
 
-#- deletes and recreates "beacon_chain.nims" which on Windows is a copy instead of a proper symlink
 #- deletes binaries that might need to be rebuilt after a Git pull
 update: | update-common
-	rm -f beacon_chain.nims && \
-		"$(MAKE)" beacon_chain.nims $(HANDLE_OUTPUT)
 	rm -f build/generate_makefile
 
-# symlink
-beacon_chain.nims:
-	ln -s beacon_chain.nimble $@
-
 # nim-libbacktrace
 libbacktrace:
 	+ "$(MAKE)" -C vendor/nim-libbacktrace --no-print-directory BUILD_CXX_LIB=0
 
-# Windows 10 with WSL enabled, but no distro installed, fails if "../../nimble.sh" is executed directly
-# in a Makefile recipe but works when prefixing it with `bash`. No idea how the PATH is overridden.
+# test suite
+TEST_BINARIES := \
+	test_fixture_const_sanity_check_minimal \
+	test_fixture_const_sanity_check_mainnet \
+	test_fixture_ssz_generic_types \
+	test_fixture_ssz_consensus_objects \
+	all_fixtures_require_ssz \
+	test_official_interchange_vectors \
+	proto_array \
+	fork_choice \
+	all_tests \
+	test_keystore \
+	state_sim \
+	block_sim
+.PHONY: $(TEST_BINARIES)
+
+test_fixture_const_sanity_check_minimal: | build deps
+	+ echo -e $(BUILD_MSG) "build/$@" && \
+		MAKE="$(MAKE)" $(ENV_SCRIPT) scripts/compile_nim_program.sh \
+			$@ \
+			"tests/official/test_fixture_const_sanity_check.nim" \
+			$(NIM_PARAMS) -d:const_preset=minimal -d:chronicles_sinks="json[file]" \
+			$(HANDLE_OUTPUT) && \
+		echo -e $(BUILD_END_MSG) "build/$@"
+
+test_fixture_const_sanity_check_mainnet: | build deps
+	+ echo -e $(BUILD_MSG) "build/$@" && \
+		MAKE="$(MAKE)" $(ENV_SCRIPT) scripts/compile_nim_program.sh \
+			$@ \
+			"tests/official/test_fixture_const_sanity_check.nim" \
+			$(NIM_PARAMS) -d:const_preset=mainnet -d:chronicles_sinks="json[file]" \
+			$(HANDLE_OUTPUT) && \
+		echo -e $(BUILD_END_MSG) "build/$@"
+
+# Generic SSZ test, doesn't use consensus objects minimal/mainnet presets
+test_fixture_ssz_generic_types: | build deps
+	+ echo -e $(BUILD_MSG) "build/$@" && \
+		MAKE="$(MAKE)" $(ENV_SCRIPT) scripts/compile_nim_program.sh \
+			$@ \
+			"tests/official/$@.nim" \
+			$(NIM_PARAMS) -d:chronicles_log_level=TRACE -d:chronicles_sinks="json[file]" \
+			$(HANDLE_OUTPUT) && \
+		echo -e $(BUILD_END_MSG) "build/$@"
+
+# Consensus object SSZ tests
+test_fixture_ssz_consensus_objects: | build deps
+	+ echo -e $(BUILD_MSG) "build/$@" && \
+		MAKE="$(MAKE)" $(ENV_SCRIPT) scripts/compile_nim_program.sh \
+			$@ \
+			"tests/official/$@.nim" \
+			$(NIM_PARAMS) -d:chronicles_log_level=TRACE -d:chronicles_sinks="json[file]" \
+			$(HANDLE_OUTPUT) && \
+		echo -e $(BUILD_END_MSG) "build/$@"
+
+# EF tests
+all_fixtures_require_ssz: | build deps
+	+ echo -e $(BUILD_MSG) "build/$@" && \
+		MAKE="$(MAKE)" $(ENV_SCRIPT) scripts/compile_nim_program.sh \
+			$@ \
+			"tests/official/$@.nim" \
+			$(NIM_PARAMS) -d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:chronicles_sinks="json[file]" \
+			$(HANDLE_OUTPUT) && \
+		echo -e $(BUILD_END_MSG) "build/$@"
+
+# EIP-3076 - Slashing interchange
+test_official_interchange_vectors: | build deps
+	+ echo -e $(BUILD_MSG) "build/$@" && \
+		MAKE="$(MAKE)" $(ENV_SCRIPT) scripts/compile_nim_program.sh \
+			$@ \
+			"tests/slashing_protection/$@.nim" \
+			$(NIM_PARAMS) -d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:chronicles_sinks="json[file]" \
+			$(HANDLE_OUTPUT) && \
+		echo -e $(BUILD_END_MSG) "build/$@"
+
+# Mainnet config
+proto_array: | build deps
+	+ echo -e $(BUILD_MSG) "build/$@" && \
+		MAKE="$(MAKE)" $(ENV_SCRIPT) scripts/compile_nim_program.sh \
+			$@ \
+			"beacon_chain/fork_choice/$@.nim" \
+			$(NIM_PARAMS) -d:const_preset=mainnet -d:chronicles_sinks="json[file]" \
+			$(HANDLE_OUTPUT) && \
+		echo -e $(BUILD_END_MSG) "build/$@"
+
+fork_choice: | build deps
+	+ echo -e $(BUILD_MSG) "build/$@" && \
+		MAKE="$(MAKE)" $(ENV_SCRIPT) scripts/compile_nim_program.sh \
+			$@ \
+			"beacon_chain/fork_choice/$@.nim" \
+			$(NIM_PARAMS) -d:const_preset=mainnet -d:chronicles_sinks="json[file]" \
+			$(HANDLE_OUTPUT) && \
+		echo -e $(BUILD_END_MSG) "build/$@"
+
+all_tests: | build deps
+	+ echo -e $(BUILD_MSG) "build/$@" && \
+		MAKE="$(MAKE)" $(ENV_SCRIPT) scripts/compile_nim_program.sh \
+			$@ \
+			"tests/$@.nim" \
+			$(NIM_PARAMS) -d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:chronicles_sinks="json[file]" \
+			$(HANDLE_OUTPUT) && \
+		echo -e $(BUILD_END_MSG) "build/$@"
+
+# TODO `test_keystore` is extracted from the rest of the tests because it uses conflicting BLST headers
+test_keystore: | build deps
+	+ echo -e $(BUILD_MSG) "build/$@" && \
+		MAKE="$(MAKE)" $(ENV_SCRIPT) scripts/compile_nim_program.sh \
+			$@ \
+			"tests/$@.nim" \
+			$(NIM_PARAMS) -d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:chronicles_sinks="json[file]" \
+			$(HANDLE_OUTPUT) && \
+		echo -e $(BUILD_END_MSG) "build/$@"
+
+# State and block sims; getting to 4th epoch triggers consensus checks
+state_sim: | build deps
+	+ echo -e $(BUILD_MSG) "build/$@" && \
+		MAKE="$(MAKE)" $(ENV_SCRIPT) scripts/compile_nim_program.sh \
+			$@ \
+			"research/$@.nim" \
+			$(NIM_PARAMS) -d:const_preset=mainnet \
+			$(HANDLE_OUTPUT) && \
+		echo -e $(BUILD_END_MSG) "build/$@"
+
+block_sim: | build deps
+	+ echo -e $(BUILD_MSG) "build/$@" && \
+		MAKE="$(MAKE)" $(ENV_SCRIPT) scripts/compile_nim_program.sh \
+			$@ \
+			"research/$@.nim" \
+			$(NIM_PARAMS) -d:const_preset=mainnet \
+			$(HANDLE_OUTPUT) && \
+		echo -e $(BUILD_END_MSG) "build/$@"
+
 DISABLE_TEST_FIXTURES_SCRIPT := 0
-test: | build deps
+# This parameter passing scheme is ugly, but short.
+test: | $(TEST_BINARIES)
 ifeq ($(DISABLE_TEST_FIXTURES_SCRIPT), 0)
 	V=$(V) scripts/setup_official_tests.sh
 endif
-	+ $(ENV_SCRIPT) nim test $(NIM_PARAMS) beacon_chain.nims && rm -f 0000-*.json
+	for TEST_BINARY in $(TEST_BINARIES); do \
+		PARAMS=""; \
+		if [[ "$${TEST_BINARY}" == "state_sim" ]]; then PARAMS="--validators=3000 --slots=128"; \
+		elif [[ "$${TEST_BINARY}" == "block_sim" ]]; then PARAMS="--validators=3000 --slots=128"; \
+		fi; \
+		echo -e "\nRunning $${TEST_BINARY} $${PARAMS}\n"; \
+		build/$${TEST_BINARY} $${PARAMS} || { echo -e "\n$${TEST_BINARY} $${PARAMS} failed; Aborting."; exit 1; }; \
+		done; \
+		rm -rf 0000-*.json t_slashprot_migration.* *.log block_sim_db
 
 # It's OK to only build this once. `make update` deletes the binary, forcing a rebuild.
 ifneq ($(USE_LIBBACKTRACE), 0)
@@ -173,9 +302,7 @@ build/generate_makefile: tools/generate_makefile.nim | deps-common
 $(TOOLS): | build deps
 	+ for D in $(TOOLS_DIRS); do [ -e "$${D}/$@.nim" ] && TOOL_DIR="$${D}" && break; done && \
 		echo -e $(BUILD_MSG) "build/$@" && \
-		$(ENV_SCRIPT) nim c --compileOnly -o:build/$@ $(NIM_PARAMS) "$${TOOL_DIR}/$@.nim" && \
-		build/generate_makefile "nimcache/release/$@/$@.json" "nimcache/release/$@/$@.makefile" && \
-		"$(MAKE)" -f "nimcache/release/$@/$@.makefile" --no-print-directory build $(HANDLE_OUTPUT) && \
+		MAKE="$(MAKE)" $(ENV_SCRIPT) scripts/compile_nim_program.sh $@ "$${TOOL_DIR}/$@.nim" $(NIM_PARAMS) $(HANDLE_OUTPUT) && \
 		echo -e $(BUILD_END_MSG) "build/$@"
 
 clean_eth2_network_simulation_data:
diff --git a/beacon_chain.nimble b/beacon_chain.nimble
deleted file mode 100644
index 9f8392910..000000000
--- a/beacon_chain.nimble
+++ /dev/null
@@ -1,100 +0,0 @@
-mode = ScriptMode.Verbose
-
-import
-  beacon_chain/version as ver
-
-packageName   = "beacon_chain"
-version       = versionAsStr
-author        = "Status Research & Development GmbH"
-description   = "Eth2.0 research implementation of the beacon chain"
-license       = "MIT or Apache License 2.0"
-installDirs   = @["beacon_chain", "research"]
-skipDirs      = @["nfuzz"]
-bin           = @[
-  "beacon_chain/nimbus_beacon_node",
-  "research/serialized_sizes",
-  "nbench/nbench",
-  ]
-
-### Dependencies
-requires "nim >= 0.19.0",
-  "blscurve",
-  "chronicles",
-  "chronos",
-  "confutils",
-  "eth",
-  "json_rpc",
-  "json_serialization",
-  "libbacktrace",
-  "libp2p",
-  "metrics",
-  "nimcrypto",
-  "normalize",
-  "serialization",
-  "stew",
-  "testutils",
-  "prompt",
-  "unicodedb",
-  "web3",
-  "yaml",
-  "zxcvbn"
-
-### Helper functions
-proc buildAndRunBinary(name: string, srcDir = "./", params = "", cmdParams = "", lang = "c") =
-  if not dirExists "build":
-    mkDir "build"
-  # allow something like "nim test --verbosity:0 --hints:off beacon_chain.nims"
-  var extra_params = params
-  for i in 2..<paramCount():
-    extra_params &= " " & paramStr(i)
-  exec "nim " & lang & " --out:./build/" & name & " -r " & extra_params & " " & srcDir & name & ".nim" & " " & cmdParams
-
-task moduleTests, "Run all module tests":
-  buildAndRunBinary "nimbus_beacon_node", "beacon_chain/",
-                    "-d:chronicles_log_level=TRACE " &
-                    "-d:const_preset=minimal " &
-                    "-d:testutils_test_build"
-
-### tasks
-task test, "Run all tests":
-  # We're enabling the TRACE log level so we're sure that those rarely used
-  # pieces of code get tested regularly. Increased test output verbosity is the
-  # price we pay for that.
-
-  buildAndRunBinary "test_fixture_const_sanity_check", "tests/official/", """-d:const_preset=minimal -d:chronicles_sinks="json[file]""""
-  buildAndRunBinary "test_fixture_const_sanity_check", "tests/official/", """-d:const_preset=mainnet -d:chronicles_sinks="json[file]""""
-
-  # Generic SSZ test, doesn't use consensus objects minimal/mainnet presets
-  buildAndRunBinary "test_fixture_ssz_generic_types", "tests/official/", """-d:chronicles_log_level=TRACE -d:chronicles_sinks="json[file]""""
-
-  # Consensus object SSZ tests
-  buildAndRunBinary "test_fixture_ssz_consensus_objects", "tests/official/", """-d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:chronicles_sinks="json[file]""""
-
-  # EF tests
-  buildAndRunBinary "all_fixtures_require_ssz", "tests/official/", """-d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:chronicles_sinks="json[file]""""
-
-  # EIP-3076 - Slashing interchange
-  buildAndRunBinary "test_official_interchange_vectors", "tests/slashing_protection/", """-d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:chronicles_sinks="json[file]""""
-
-  # Mainnet config
-  buildAndRunBinary "proto_array", "beacon_chain/fork_choice/", """-d:const_preset=mainnet -d:chronicles_sinks="json[file]""""
-  buildAndRunBinary "fork_choice", "beacon_chain/fork_choice/", """-d:const_preset=mainnet -d:chronicles_sinks="json[file]""""
-  buildAndRunBinary "all_tests", "tests/", """-d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:chronicles_sinks="json[file]""""
-  # TODO `test_keystore` is extracted from the rest of the tests because it uses conflicting BLST headers
-  buildAndRunBinary "test_keystore", "tests/", """-d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:chronicles_sinks="json[file]""""
-
-  # As BLST has a pure C fallback as of Dec 2020 undergoing audit and formal verification
-  # we skip Miracl checks as they are costly in CI time.
-  # # Check Miracl/Milagro fallback on select tests
-  # buildAndRunBinary "test_interop", "tests/", """-d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:BLS_FORCE_BACKEND=miracl -d:chronicles_sinks="json[file]""""
-  # buildAndRunBinary "test_process_attestation", "tests/spec_block_processing/", """-d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:BLS_FORCE_BACKEND=miracl -d:chronicles_sinks="json[file]""""
-  # buildAndRunBinary "test_process_deposits", "tests/spec_block_processing/", """-d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:BLS_FORCE_BACKEND=miracl -d:chronicles_sinks="json[file]""""
-  # buildAndRunBinary "all_fixtures_require_ssz", "tests/official/", """-d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:BLS_FORCE_BACKEND=miracl -d:chronicles_sinks="json[file]""""
-  # buildAndRunBinary "test_attestation_pool", "tests/", """-d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:BLS_FORCE_BACKEND=miracl -d:chronicles_sinks="json[file]""""
-  # buildAndRunBinary "test_block_pool", "tests/", """-d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:BLS_FORCE_BACKEND=miracl -d:chronicles_sinks="json[file]""""
-
-  # State and block sims; getting to 4th epoch triggers consensus checks
-  buildAndRunBinary "state_sim", "research/", "-d:const_preset=mainnet -d:chronicles_log_level=INFO", "--validators=3000 --slots=128"
-  # buildAndRunBinary "state_sim", "research/", "-d:const_preset=mainnet -d:BLS_FORCE_BACKEND=miracl -d:chronicles_log_level=INFO", "--validators=3000 --slots=128"
-  buildAndRunBinary "block_sim", "research/", "-d:const_preset=mainnet", "--validators=3000 --slots=128"
-  # buildAndRunBinary "block_sim", "research/", "-d:const_preset=mainnet -d:BLS_FORCE_BACKEND=miracl", "--validators=3000 --slots=128"
diff --git a/config.nims b/config.nims
index 34f5dfb62..b479ecd65 100644
--- a/config.nims
+++ b/config.nims
@@ -1,7 +1,11 @@
-when defined(release):
-  let nimCachePath = "nimcache/release/" & projectName()
+const nimCachePathOverride {.strdefine.} = ""
+when nimCachePathOverride == "":
+  when defined(release):
+    let nimCachePath = "nimcache/release/" & projectName()
+  else:
+    let nimCachePath = "nimcache/debug/" & projectName()
 else:
-  let nimCachePath = "nimcache/debug/" & projectName()
+  let nimCachePath = nimCachePathOverride
 switch("nimcache", nimCachePath)
 
 # `-flto` gives a significant improvement in processing speed, specially hash tree and state transition (basically any CPU-bound code implemented in nim)
diff --git a/scripts/compile_nim_program.sh b/scripts/compile_nim_program.sh
new file mode 100755
index 000000000..b9ffbbe9f
--- /dev/null
+++ b/scripts/compile_nim_program.sh
@@ -0,0 +1,23 @@
+#!/usr/bin/env bash
+
+set -e
+
+cd "$(dirname "${BASH_SOURCE[0]}")"/..
+
+BINARY="$1"
+SOURCE="$2"
+shift 2
+
+# According to the Nim compiler, the project name comes from the main source
+# file, not the output binary.
+PROJECT_NAME="$(basename ${SOURCE%.nim})"
+
+# The default nimcache dir is "nimcache/release/${PROJECT_NAME}" which doesn't
+# allow building different binaries from the same main source file, in
+# parallel.
+# We can't use '--nimcache:...' here, because the same path is being used by
+# LTO on macOS, in "config.nims"
+nim c --compileOnly -o:build/${BINARY} "$@" -d:nimCachePathOverride=nimcache/release/${BINARY} "${SOURCE}"
+build/generate_makefile "nimcache/release/${BINARY}/${PROJECT_NAME}.json" "nimcache/release/${BINARY}/${BINARY}.makefile"
+"${MAKE}" -f "nimcache/release/${BINARY}/${BINARY}.makefile" --no-print-directory build
+