[Merged by Bors] - VC: accept unknown fields in chain spec

[michaelsproul]

Issue Addressed

Closes #2274

Proposed Changes

• Modify the YamlConfig to collect unknown fields into an extra_fields map, instead of failing hard.
• Log a debug message if there are extra fields returned to the VC from one of its BNs.

This restores Lighthouse's compatibility with Teku beacon nodes (and therefore Infura)

[paulhauner]

IIRC, we added deny_unknown_fields because we realised we were silently ignoring some parameters in ef_tests. Perhaps we could retain this functionality by doing something like:

#[cfg_attr(feature = "strict_yaml_config", serde(deny_unknown_fields))]

And then turning that on in ef_tests. What are your thoughts? I'm not fully set on doing this.

[michaelsproul]

IIRC, we added deny_unknown_fields because we realised we were silently ignoring some parameters in ef_tests.

Ah yeah, good idea! I think most errors would be caught by the test below (which used not exist), but it doesn't hurt to be extra-strict in testing:

lighthouse/testing/ef_tests/tests/tests.rs

Lines 7 to 20 in b34a79d

	// Check that the config from the Eth2.0 spec tests matches our minimal/mainnet config.
	fn config_test<E: EthSpec + TypeName>() {
	let config_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
	.join("eth2.0-spec-tests")
	.join("tests")
	.join(E::name())
	.join("config")
	.join("phase0.yaml");
	let yaml_config = YamlConfig::from_file(&config_path).expect("config file loads OK");
	let spec = E::default_spec();
	let yaml_from_spec = YamlConfig::from_spec::<E>(&spec);
	assert_eq!(yaml_config.apply_to_chain_spec::<E>(&spec), Some(spec));
	assert_eq!(yaml_from_spec, yaml_config);
	}

I'll add the cfg

[michaelsproul]

Actually, it can be more lightweight than that. We can just assert that extra_fields is empty.

[michaelsproul]

Done. Let me know if that soln suits you @paulhauner 😊

[michaelsproul]

There were a large number of places where we violated these lints, so I think we should leave them for another time. Particularly so we don't introduce unnecessary noise to conflict with other larger refactors that are ongoing (e.g. #2279).

[paulhauner]

Nice! The extra_fields trick is cool, I didn't know we could do that!

EDIT: looks like CI is a little unhappy.

[michaelsproul]

bors r+

[bors]

Pull request successfully merged into unstable.

Build succeeded:

• arbitrary-check
• cargo-audit
• cargo-fmt
• cargo-udeps
• check-benchmarks
• check-consensus
• clippy
• debug-tests-ubuntu
• dockerfile-ubuntu
• ef-tests-ubuntu
• eth1-simulator-ubuntu
• no-eth1-simulator-ubuntu
• release-tests-ubuntu
• state-transition-vectors-ubuntu