From bfdaf8a3f5b3229ee1f49340e555fb3fcd14c746 Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Mon, 9 Aug 2021 13:40:06 -0500 Subject: [PATCH] Fix config loader tests (#9348) * don't mutate mainnet config * add missing minimal config params * Ensure all yaml fields were set and are correct * gofmt * Add sanity check that some empty yaml wasn't given * Gazelle and deepsource feedback --- shared/params/BUILD.bazel | 1 + shared/params/loader.go | 2 +- shared/params/loader_test.go | 51 +++++++++++++++++++++++++++++++-- shared/params/minimal_config.go | 7 +++++ shared/params/values.go | 2 ++ 5 files changed, 59 insertions(+), 4 deletions(-) diff --git a/shared/params/BUILD.bazel b/shared/params/BUILD.bazel index 59487a743..bef9e31cb 100644 --- a/shared/params/BUILD.bazel +++ b/shared/params/BUILD.bazel @@ -49,6 +49,7 @@ go_test( "//shared/testutil/assert:go_default_library", "//shared/testutil/require:go_default_library", "@com_github_sirupsen_logrus//:go_default_library", + "@in_gopkg_yaml_v2//:go_default_library", "@io_bazel_rules_go//go/tools/bazel:go_default_library", ], ) diff --git a/shared/params/loader.go b/shared/params/loader.go index 313fd1ac4..88312c198 100644 --- a/shared/params/loader.go +++ b/shared/params/loader.go @@ -29,7 +29,7 @@ func LoadChainConfigFile(chainConfigFileName string) { } } yamlFile = []byte(strings.Join(lines, "\n")) - conf := MainnetConfig() + conf := MainnetConfig().Copy() if err := yaml.UnmarshalStrict(yamlFile, conf); err != nil { if _, ok := err.(*yaml.TypeError); !ok { log.WithError(err).Fatal("Failed to parse chain config yaml file.") diff --git a/shared/params/loader_test.go b/shared/params/loader_test.go index fc690f309..0d8176ce9 100644 --- a/shared/params/loader_test.go +++ b/shared/params/loader_test.go @@ -3,17 +3,19 @@ package params import ( "io/ioutil" "path" + "reflect" "strings" "testing" "github.com/bazelbuild/rules_go/go/tools/bazel" "github.com/prysmaticlabs/prysm/shared/testutil/assert" "github.com/prysmaticlabs/prysm/shared/testutil/require" + "gopkg.in/yaml.v2" ) func TestLoadConfigFileMainnet(t *testing.T) { // See https://media.githubusercontent.com/media/ethereum/eth2.0-spec-tests/master/tests/minimal/config/phase0.yaml - assertVals := func(name string, c1, c2 *BeaconChainConfig) { + assertVals := func(name string, fields []string, c1, c2 *BeaconChainConfig) { // Misc params. assert.Equal(t, c1.MaxCommitteesPerSlot, c2.MaxCommitteesPerSlot, "%s: MaxCommitteesPerSlot", name) assert.Equal(t, c1.TargetCommitteeSize, c2.TargetCommitteeSize, "%s: TargetCommitteeSize", name) @@ -97,18 +99,43 @@ func TestLoadConfigFileMainnet(t *testing.T) { assert.Equal(t, c1.DomainVoluntaryExit, c2.DomainVoluntaryExit, "%s: DomainVoluntaryExit", name) assert.Equal(t, c1.DomainSelectionProof, c2.DomainSelectionProof, "%s: DomainSelectionProof", name) assert.Equal(t, c1.DomainAggregateAndProof, c2.DomainAggregateAndProof, "%s: DomainAggregateAndProof", name) + + // Ensure all fields from the yaml file exist, were set, and correctly match the expected value. + ft1 := reflect.TypeOf(*c1) + for _, field := range fields { + var found bool + for i := 0; i < ft1.NumField(); i++ { + v, ok := ft1.Field(i).Tag.Lookup("yaml") + if ok && v == field { + found = true + v1 := reflect.ValueOf(*c1).Field(i).Interface() + v2 := reflect.ValueOf(*c2).Field(i).Interface() + if reflect.ValueOf(v1).Kind() == reflect.Slice { + assert.DeepEqual(t, v1, v2, "%s: %s", name, field) + } else { + assert.Equal(t, v1, v2, "%s: %s", name, field) + } + break + } + } + if !found { + t.Errorf("No struct tag found `yaml:%s`", field) + } + } } t.Run("mainnet", func(t *testing.T) { mainnetConfigFile := ConfigFilePath(t, "mainnet") LoadChainConfigFile(mainnetConfigFile) - assertVals("mainnet", MainnetConfig(), BeaconConfig()) + fields := fieldsFromYaml(t, mainnetConfigFile) + assertVals("mainnet", fields, MainnetConfig(), BeaconConfig()) }) t.Run("minimal", func(t *testing.T) { minimalConfigFile := ConfigFilePath(t, "minimal") LoadChainConfigFile(minimalConfigFile) - assertVals("minimal", MinimalSpecConfig(), BeaconConfig()) + fields := fieldsFromYaml(t, minimalConfigFile) + assertVals("minimal", fields, MinimalSpecConfig(), BeaconConfig()) }) } @@ -210,3 +237,21 @@ func ConfigFilePath(t *testing.T, config string) string { configFilePath := path.Join(filepath, "config", "phase0.yaml") return configFilePath } + +func fieldsFromYaml(t *testing.T, fp string) []string { + yamlFile, err := ioutil.ReadFile(fp) + require.NoError(t, err) + m := make(map[string]interface{}) + require.NoError(t, yaml.Unmarshal(yamlFile, &m)) + + var keys []string + for k := range m { + keys = append(keys, k) + } + + if len(keys) == 0 { + t.Errorf("No fields loaded from yaml file %s", fp) + } + + return keys +} diff --git a/shared/params/minimal_config.go b/shared/params/minimal_config.go index 1daf6540c..875311866 100644 --- a/shared/params/minimal_config.go +++ b/shared/params/minimal_config.go @@ -91,5 +91,12 @@ func MinimalSpecConfig() *BeaconChainConfig { minimalConfig.InactivityScoreBias = 4 minimalConfig.EpochsPerSyncCommitteePeriod = 8 + // Ethereum PoW parameters. + minimalConfig.DepositChainID = 5 // Chain ID of eth1 goerli. + minimalConfig.DepositNetworkID = 5 // Network ID of eth1 goerli. + minimalConfig.DepositContractAddress = "0x1234567890123456789012345678901234567890" + + minimalConfig.ConfigName = ConfigNames[Minimal] + return minimalConfig } diff --git a/shared/params/values.go b/shared/params/values.go index 31f33ea61..ad4e87f10 100644 --- a/shared/params/values.go +++ b/shared/params/values.go @@ -2,6 +2,7 @@ package params const ( Mainnet ConfigName = iota + Minimal EndToEnd Pyrmont Toledo @@ -11,6 +12,7 @@ const ( // ConfigNames provides network configuration names. var ConfigNames = map[ConfigName]string{ Mainnet: "mainnet", + Minimal: "minimal", EndToEnd: "end-to-end", Pyrmont: "pyrmont", Toledo: "toledo",