[Merged by Bors] - Allow truncation of pubkey cache on creation

[michaelsproul]

Issue Addressed

Closes #1680

Proposed Changes

This PR fixes a race condition in beacon node start-up whereby the pubkey cache could be created by the beacon chain builder before the PersistedBeaconChain was stored to disk. When the node restarted, it would find the persisted chain missing, and attempt to start from scratch, creating a new pubkey cache in the process. This call to ValidatorPubkeyCache::new would fail if the file already existed (which it did). I changed the behaviour so that pubkey cache initialization now doesn't care whether there's a file already in existence (it's only a cache after all). Instead it will truncate and recreate the file in the race scenario described.

[michaelsproul]

Marking this WIP while we work out a fix for this case raised by @paulhauner:

1. User inits client, including cache and persisted head
2. Cache gets mistakenly deleted
3. This patch silently recreates the cache without complete information, leading to errors down the line (?)

[michaelsproul]

@paulhauner I think the case mentioned above is actually OK, because the beacon chain won't even try to re-initialise the cache if the head exists and it's missing. I tested deleting the pubkey cache and this error occurs:

Sep 30 14:29:50.954 WARN Ethereum 2.0 is pre-release. This software is experimental.
Sep 30 14:29:50.954 INFO Lighthouse started                      version: Lighthouse/v0.2.12-c31cbb627
Sep 30 14:29:50.955 INFO Configured for testnet                  name: medalla
Sep 30 14:29:50.956 INFO Data directory initialised              datadir: /home/michael/.lighthouse/medalla
Sep 30 14:29:51.048 INFO Starting beacon chain                   method: resume, service: beacon
Sep 30 14:29:51.098 CRIT Failed to start beacon node             reason: Unable to open persisted pubkey cache: ValidatorPubkeyCacheFileError("Io(Os { code: 2, kind: NotFound, message: \"No such file or directory\" })")
Sep 30 14:29:51.098 INFO Internal shutdown received              reason: Failed to start beacon node
Sep 30 14:29:51.098 INFO Shutting down..

My change is only relevant in the case where ValidatorPubkeyCache::new is called, which is here:

lighthouse/beacon_node/beacon_chain/src/builder.rs

Lines 532 to 535 in a1a6b01

	let validator_pubkey_cache = self.validator_pubkey_cache.map(Ok).unwrap_or_else(|| {
	ValidatorPubkeyCache::new(&canonical_head.beacon_state, pubkey_cache_path)
	.map_err(|e| format!("Unable to init validator pubkey cache: {:?}", e))
	})?;

And it only happens if self.validator_pubkey_cache is None, which can't happen in the resume_from_db case, because we trip the error on line 316:

lighthouse/beacon_node/beacon_chain/src/builder.rs

Lines 316 to 324 in a1a6b01

	let pubkey_cache = ValidatorPubkeyCache::load_from_file(pubkey_cache_path)
	.map_err(|e| format!("Unable to open persisted pubkey cache: {:?}", e))?;
	self.genesis_block_root = Some(chain.genesis_block_root);
	self.head_tracker = Some(
	HeadTracker::from_ssz_container(&chain.ssz_head_tracker)
	.map_err(|e| format!("Failed to decode head tracker for database: {:?}", e))?,
	);
	self.validator_pubkey_cache = Some(pubkey_cache);

[paulhauner]

@paulhauner I think the case mentioned above is actually OK

Nice, sorry for the distraction.

Merge it!

[michaelsproul]

No problemo~

bors r+

[bors]

Pull request successfully merged into master.

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