Add mutex to params/config (#8160)

* add mutex to params/config

* split config files into test/prod

* add tags checker

* add regression test

* remove debug info

* update bazel config

* go fmt

* make sure that conditional file is kept by gazelle

* update build tag: test -> develop

* gazelle

* remove redundant import

* update deps.md (per Nishant's suggestion)

Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>

DEPENDENCIES.md

@@ -67,3 +67,14 @@ bazel run //:gazelle -- update-repos -from_file=go.mod -to_macro=deps.bzl%prysm_
 The deps.bzl file should have been updated with the dependency and any transitive dependencies. 
 
 Do NOT add new `go_repository` to the WORKSPACE file. All dependencies should live in deps.bzl.
+
+## Running tests
+
+To enable conditional compilation and custom configuration for tests (where compiled code has more 
+debug info, while not being completely optimized), we rely on Go's build tags/constraints mechanism 
+(see official docs on [build constraints](https://golang.org/pkg/go/build/#hdr-Build_Constraints)). 
+Therefore, whenever using `go test`, do not forget to pass in extra build tag, eg:
+
+```bash
+go test ./beacon-chain/sync/initial-sync -tags develop 
+```

shared/params/BUILD.bazel

@@ -5,6 +5,8 @@ go_library(
     name = "go_default_library",
     srcs = [
         "config.go",
+        "config_utils_develop.go",  # keep
+        "config_utils_prod.go",
         "io_config.go",
         "loader.go",
         "mainnet_config.go",
@@ -29,6 +31,7 @@ go_test(
     name = "go_default_test",
     size = "small",
     srcs = [
+        "checktags_test.go",
         "config_test.go",
         "loader_test.go",
     ],
@@ -37,9 +40,12 @@ go_test(
         "@eth2_spec_tests_minimal//:test_data",
     ],
     embed = [":go_default_library"],
+    gotags = ["develop"],
+    race = "on",
     deps = [
         "//shared/testutil/assert:go_default_library",
         "//shared/testutil/require:go_default_library",
+        "@com_github_sirupsen_logrus//:go_default_library",
         "@io_bazel_rules_go//go/tools/bazel:go_default_library",
     ],
 )

shared/params/checktags_test.go

@@ -0,0 +1,11 @@
+// +build !develop
+
+package params
+
+import (
+	log "github.com/sirupsen/logrus"
+)
+
+func init() {
+	log.Fatal("Tests in this package require extra build tag: re-run with `-tags develop`")
+}

shared/params/config.go

@@ -3,8 +3,6 @@ package params
 
 import (
 	"time"
-
-	"github.com/mohae/deepcopy"
 )
 
 // BeaconChainConfig contains constant configs for node to participate in beacon chain.
@@ -126,27 +124,3 @@ type BeaconChainConfig struct {
 	// Weak subjectivity values.
 	SafetyDecay uint64 // SafetyDecay is defined as the loss in the 1/3 consensus safety margin of the casper FFG mechanism.
 }
-
-var beaconConfig = MainnetConfig()
-
-// BeaconConfig retrieves beacon chain config.
-func BeaconConfig() *BeaconChainConfig {
-	return beaconConfig
-}
-
-// OverrideBeaconConfig by replacing the config. The preferred pattern is to
-// call BeaconConfig(), change the specific parameters, and then call
-// OverrideBeaconConfig(c). Any subsequent calls to params.BeaconConfig() will
-// return this new configuration.
-func OverrideBeaconConfig(c *BeaconChainConfig) {
-	beaconConfig = c
-}
-
-// Copy returns a copy of the config object.
-func (c *BeaconChainConfig) Copy() *BeaconChainConfig {
-	config, ok := deepcopy.Copy(*c).(BeaconChainConfig)
-	if !ok {
-		config = *beaconConfig
-	}
-	return &config
-}

shared/params/config_test.go

@@ -11,7 +11,7 @@ import (
 // to make sure that previous test case has already been completed and check can be run.
 var testOverrideBeaconConfigExecuted bool
 
-func TestOverrideBeaconConfig(t *testing.T) {
+func TestConfig_OverrideBeaconConfig(t *testing.T) {
 	// Ensure that param modifications are safe.
 	params.SetupTestConfigCleanup(t)
 	cfg := params.BeaconConfig()
@@ -23,7 +23,7 @@ func TestOverrideBeaconConfig(t *testing.T) {
 	testOverrideBeaconConfigExecuted = true
 }
 
-func TestOverrideBeaconConfigTestTeardown(t *testing.T) {
+func TestConfig_OverrideBeaconConfigTestTeardown(t *testing.T) {
 	if !testOverrideBeaconConfigExecuted {
 		t.Skip("State leak can occur only if state mutating test has already completed")
 	}
@@ -32,3 +32,15 @@ func TestOverrideBeaconConfigTestTeardown(t *testing.T) {
 		t.Fatal("Parameter update has been leaked out of previous test")
 	}
 }
+
+func TestConfig_DataRace(t *testing.T) {
+	for i := 0; i < 10; i++ {
+		go func() {
+			cfg := params.BeaconConfig()
+			params.OverrideBeaconConfig(cfg)
+		}()
+		go func() uint64 {
+			return params.BeaconConfig().MaxDeposits
+		}()
+	}
+}

shared/params/config_utils_develop.go

@@ -0,0 +1,40 @@
+// +build develop
+
+package params
+
+import (
+	"sync"
+
+	"github.com/mohae/deepcopy"
+)
+
+var beaconConfig = MainnetConfig()
+var beaconConfigLock sync.RWMutex
+
+// BeaconConfig retrieves beacon chain config.
+func BeaconConfig() *BeaconChainConfig {
+	beaconConfigLock.RLock()
+	defer beaconConfigLock.RUnlock()
+	return beaconConfig
+}
+
+// OverrideBeaconConfig by replacing the config. The preferred pattern is to
+// call BeaconConfig(), change the specific parameters, and then call
+// OverrideBeaconConfig(c). Any subsequent calls to params.BeaconConfig() will
+// return this new configuration.
+func OverrideBeaconConfig(c *BeaconChainConfig) {
+	beaconConfigLock.Lock()
+	defer beaconConfigLock.Unlock()
+	beaconConfig = c
+}
+
+// Copy returns a copy of the config object.
+func (c *BeaconChainConfig) Copy() *BeaconChainConfig {
+	beaconConfigLock.RLock()
+	defer beaconConfigLock.RUnlock()
+	config, ok := deepcopy.Copy(*c).(BeaconChainConfig)
+	if !ok {
+		config = *beaconConfig
+	}
+	return &config
+}

shared/params/config_utils_prod.go

@@ -0,0 +1,31 @@
+// +build !develop
+
+package params
+
+import (
+	"github.com/mohae/deepcopy"
+)
+
+var beaconConfig = MainnetConfig()
+
+// BeaconConfig retrieves beacon chain config.
+func BeaconConfig() *BeaconChainConfig {
+	return beaconConfig
+}
+
+// OverrideBeaconConfig by replacing the config. The preferred pattern is to
+// call BeaconConfig(), change the specific parameters, and then call
+// OverrideBeaconConfig(c). Any subsequent calls to params.BeaconConfig() will
+// return this new configuration.
+func OverrideBeaconConfig(c *BeaconChainConfig) {
+	beaconConfig = c
+}
+
+// Copy returns a copy of the config object.
+func (c *BeaconChainConfig) Copy() *BeaconChainConfig {
+	config, ok := deepcopy.Copy(*c).(BeaconChainConfig)
+	if !ok {
+		config = *beaconConfig
+	}
+	return &config
+}