diff --git a/scripts/local_testnet/validator_client.sh b/scripts/local_testnet/validator_client.sh index aab44045a..b68993637 100755 --- a/scripts/local_testnet/validator_client.sh +++ b/scripts/local_testnet/validator_client.sh @@ -15,4 +15,4 @@ exec lighthouse \ --datadir $VALIDATORS_DIR \ --secrets-dir $SECRETS_DIR \ --testnet-dir $TESTNET_DIR \ - --auto-register + --init-slashing-protection diff --git a/testing/node_test_rig/src/lib.rs b/testing/node_test_rig/src/lib.rs index e2391c0f8..2c2a0e81f 100644 --- a/testing/node_test_rig/src/lib.rs +++ b/testing/node_test_rig/src/lib.rs @@ -106,6 +106,14 @@ pub fn testing_client_config() -> ClientConfig { client_config } +pub fn testing_validator_config() -> ValidatorConfig { + ValidatorConfig { + init_slashing_protection: true, + disable_auto_discover: false, + ..ValidatorConfig::default() + } +} + /// Contains the directories for a `LocalValidatorClient`. /// /// This struct is separate to `LocalValidatorClient` to allow for pre-computation of validator diff --git a/testing/simulator/src/eth1_sim.rs b/testing/simulator/src/eth1_sim.rs index 75f2256a1..acf4fe845 100644 --- a/testing/simulator/src/eth1_sim.rs +++ b/testing/simulator/src/eth1_sim.rs @@ -4,8 +4,8 @@ use eth1::http::Eth1NetworkId; use eth1_test_rig::GanacheEth1Instance; use futures::prelude::*; use node_test_rig::{ - environment::EnvironmentBuilder, testing_client_config, ClientGenesis, ValidatorConfig, - ValidatorFiles, + environment::EnvironmentBuilder, testing_client_config, testing_validator_config, + ClientGenesis, ValidatorFiles, }; use rayon::prelude::*; use std::net::{IpAddr, Ipv4Addr}; @@ -128,14 +128,7 @@ pub fn run_eth1_sim(matches: &ArgMatches) -> Result<(), String> { */ for (i, files) in validator_files.into_iter().enumerate() { network - .add_validator_client( - ValidatorConfig { - disable_auto_discover: false, - ..ValidatorConfig::default() - }, - i, - files, - ) + .add_validator_client(testing_validator_config(), i, files) .await?; } diff --git a/testing/simulator/src/no_eth1_sim.rs b/testing/simulator/src/no_eth1_sim.rs index 621b8ef3b..afcd96986 100644 --- a/testing/simulator/src/no_eth1_sim.rs +++ b/testing/simulator/src/no_eth1_sim.rs @@ -2,8 +2,8 @@ use crate::{checks, LocalNetwork}; use clap::ArgMatches; use futures::prelude::*; use node_test_rig::{ - environment::EnvironmentBuilder, testing_client_config, ClientGenesis, ValidatorConfig, - ValidatorFiles, + environment::EnvironmentBuilder, testing_client_config, testing_validator_config, + ClientGenesis, ValidatorFiles, }; use rayon::prelude::*; use std::net::{IpAddr, Ipv4Addr}; @@ -99,14 +99,7 @@ pub fn run_no_eth1_sim(matches: &ArgMatches) -> Result<(), String> { let add_validators_fut = async { for (i, files) in validator_files.into_iter().enumerate() { network - .add_validator_client( - ValidatorConfig { - disable_auto_discover: false, - ..ValidatorConfig::default() - }, - i, - files, - ) + .add_validator_client(testing_validator_config(), i, files) .await?; } diff --git a/validator_client/slashing_protection/src/slashing_database.rs b/validator_client/slashing_protection/src/slashing_database.rs index df0b38ecf..1bfdcf60d 100644 --- a/validator_client/slashing_protection/src/slashing_database.rs +++ b/validator_client/slashing_protection/src/slashing_database.rs @@ -173,6 +173,17 @@ impl SlashingDatabase { Ok(()) } + /// Check that all of the given validators are registered. + pub fn check_validator_registrations<'a>( + &self, + mut public_keys: impl Iterator, + ) -> Result<(), NotSafe> { + let mut conn = self.conn_pool.get()?; + let txn = conn.transaction()?; + public_keys + .try_for_each(|public_key| self.get_validator_id_in_txn(&txn, public_key).map(|_| ())) + } + /// Get the database-internal ID for a validator. /// /// This is NOT the same as a validator index, and depends on the ordering that validators diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index 0651bf536..c78979880 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -52,14 +52,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .conflicts_with("datadir") .requires("validators-dir"), ) - .arg(Arg::with_name("auto-register").long("auto-register").help( - "If present, the validator client will register any new signing keys with \ - the slashing protection database so that they may be used. WARNING: \ - enabling the same signing key on multiple validator clients WILL lead to \ - that validator getting slashed. Only use this flag the first time you run \ - the validator client, or if you're certain there are no other \ - nodes using the same key. Automatically enabled unless `--strict` is specified", - )) .arg( Arg::with_name("delete-lockfiles") .long("delete-lockfiles") @@ -73,14 +65,15 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { ) ) .arg( - Arg::with_name("strict-slashing-protection") - .long("strict-slashing-protection") - .help( - "If present, do not create a new slashing database. This is to ensure that users \ - do not accidentally get slashed in case their slashing protection db ends up in the \ - wrong directory during directory restructure and vc creates a new empty db and \ - re-registers all validators." - ) + Arg::with_name("init-slashing-protection") + .long("init-slashing-protection") + .help( + "If present, do not require the slashing protection database to exist before \ + running. You SHOULD NOT use this flag unless you're certain that a new \ + slashing protection database is required. Usually, your database \ + will have been initialized when you imported your validator keys. If you \ + misplace your database and then run with this flag you risk being slashed." + ) ) .arg( Arg::with_name("disable-auto-discover") diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index d51f44e44..f26eaf9e0 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -32,8 +32,8 @@ pub struct Config { pub delete_lockfiles: bool, /// If true, don't scan the validators dir for new keystores. pub disable_auto_discover: bool, - /// If true, don't re-register existing validators in definitions.yml for slashing protection. - pub strict_slashing_protection: bool, + /// If true, re-register existing validators in definitions.yml for slashing protection. + pub init_slashing_protection: bool, /// Graffiti to be inserted everytime we create a block. pub graffiti: Option, /// Configuration for the HTTP REST API. @@ -58,7 +58,7 @@ impl Default for Config { allow_unsynced_beacon_node: false, delete_lockfiles: false, disable_auto_discover: false, - strict_slashing_protection: false, + init_slashing_protection: false, graffiti: None, http_api: <_>::default(), } @@ -122,7 +122,7 @@ impl Config { config.allow_unsynced_beacon_node = cli_args.is_present("allow-unsynced"); config.delete_lockfiles = cli_args.is_present("delete-lockfiles"); config.disable_auto_discover = cli_args.is_present("disable-auto-discover"); - config.strict_slashing_protection = cli_args.is_present("strict-slashing-protection"); + config.init_slashing_protection = cli_args.is_present("init-slashing-protection"); if let Some(input_graffiti) = cli_args.value_of("graffiti") { let graffiti_bytes = input_graffiti.as_bytes(); diff --git a/validator_client/src/http_api/tests.rs b/validator_client/src/http_api/tests.rs index e9344b5f4..eef3aa8ae 100644 --- a/validator_client/src/http_api/tests.rs +++ b/validator_client/src/http_api/tests.rs @@ -17,6 +17,7 @@ use eth2::{ }; use eth2_keystore::KeystoreBuilder; use parking_lot::RwLock; +use slashing_protection::{SlashingDatabase, SLASHING_PROTECTION_FILENAME}; use slot_clock::TestingSlotClock; use std::marker::PhantomData; use std::net::Ipv4Addr; @@ -65,15 +66,17 @@ impl ApiTester { .build() .unwrap(); + let slashing_db_path = config.validator_dir.join(SLASHING_PROTECTION_FILENAME); + let slashing_protection = SlashingDatabase::open_or_create(&slashing_db_path).unwrap(); + let validator_store: ValidatorStore = ValidatorStore::new( initialized_validators, - &config, + slashing_protection, Hash256::repeat_byte(42), E::default_spec(), fork_service.clone(), log.clone(), - ) - .unwrap(); + ); let initialized_validators = validator_store.initialized_validators(); diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index 3b7dc6b07..5591a6cbf 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -28,6 +28,7 @@ use futures::channel::mpsc; use http_api::ApiSecret; use initialized_validators::InitializedValidators; use notifier::spawn_notifier; +use slashing_protection::{SlashingDatabase, SLASHING_PROTECTION_FILENAME}; use slog::{error, info, Logger}; use slot_clock::SlotClock; use slot_clock::SystemTimeSlotClock; @@ -106,6 +107,44 @@ impl ProductionValidatorClient { .await .map_err(|e| format!("Unable to initialize validators: {:?}", e))?; + // Initialize slashing protection. + let slashing_db_path = config.validator_dir.join(SLASHING_PROTECTION_FILENAME); + let slashing_protection = if config.init_slashing_protection { + SlashingDatabase::open_or_create(&slashing_db_path).map_err(|e| { + format!( + "Failed to open or create slashing protection database: {:?}", + e + ) + }) + } else { + SlashingDatabase::open(&slashing_db_path).map_err(|e| { + format!( + "Failed to open slashing protection database: {:?}.\n\ + Ensure that `slashing_protection.sqlite` is in {:?} folder", + e, config.validator_dir + ) + }) + }?; + + // Check validator registration with slashing protection, or auto-register all validators. + if config.init_slashing_protection { + slashing_protection + .register_validators(validators.iter_voting_pubkeys()) + .map_err(|e| format!("Error while registering slashing protection: {:?}", e))?; + } else { + slashing_protection + .check_validator_registrations(validators.iter_voting_pubkeys()) + .map_err(|e| { + format!( + "One or more validators not found in slashing protection database.\n\ + Ensure you haven't misplaced your slashing protection database, or \ + carefully consider running with --init-slashing-protection (see --help). \ + Error: {:?}", + e + ) + })?; + } + info!( log, "Initialized validators"; @@ -157,12 +196,12 @@ impl ProductionValidatorClient { let validator_store: ValidatorStore = ValidatorStore::new( validators, - &config, + slashing_protection, genesis_validators_root, context.eth2_config.spec.clone(), fork_service.clone(), log.clone(), - )?; + ); info!( log, @@ -170,8 +209,6 @@ impl ProductionValidatorClient { "voting_validators" => validator_store.num_voting_validators() ); - validator_store.register_all_validators_for_slashing_protection()?; - let duties_service = DutiesServiceBuilder::new() .slot_clock(slot_clock.clone()) .validator_store(validator_store.clone()) diff --git a/validator_client/src/validator_store.rs b/validator_client/src/validator_store.rs index 08583f84d..66b75874a 100644 --- a/validator_client/src/validator_store.rs +++ b/validator_client/src/validator_store.rs @@ -1,9 +1,7 @@ -use crate::{ - config::Config, fork_service::ForkService, initialized_validators::InitializedValidators, -}; +use crate::{fork_service::ForkService, initialized_validators::InitializedValidators}; use account_utils::{validator_definitions::ValidatorDefinition, ZeroizeString}; use parking_lot::RwLock; -use slashing_protection::{NotSafe, Safe, SlashingDatabase, SLASHING_PROTECTION_FILENAME}; +use slashing_protection::{NotSafe, Safe, SlashingDatabase}; use slog::{crit, error, warn, Logger}; use slot_clock::SlotClock; use std::marker::PhantomData; @@ -56,32 +54,13 @@ pub struct ValidatorStore { impl ValidatorStore { pub fn new( validators: InitializedValidators, - config: &Config, + slashing_protection: SlashingDatabase, genesis_validators_root: Hash256, spec: ChainSpec, fork_service: ForkService, log: Logger, - ) -> Result { - let slashing_db_path = config.validator_dir.join(SLASHING_PROTECTION_FILENAME); - let slashing_protection = if config.strict_slashing_protection { - // Don't create a new slashing database if `strict_slashing_protection` is turned on. - SlashingDatabase::open(&slashing_db_path).map_err(|e| { - format!( - "Failed to open slashing protection database: {:?}. - Ensure that `slashing_protection.sqlite` is in {:?} folder", - e, config.validator_dir - ) - })? - } else { - SlashingDatabase::open_or_create(&slashing_db_path).map_err(|e| { - format!( - "Failed to open or create slashing protection database: {:?}", - e - ) - })? - }; - - Ok(Self { + ) -> Self { + Self { validators: Arc::new(RwLock::new(validators)), slashing_protection, genesis_validators_root, @@ -90,7 +69,7 @@ impl ValidatorStore { temp_dir: None, fork_service, _phantom: PhantomData, - }) + } } pub fn initialized_validators(&self) -> Arc> { @@ -130,16 +109,6 @@ impl ValidatorStore { Ok(validator_def) } - /// Register all known validators with the slashing protection database. - /// - /// Registration is required to protect against a lost or missing slashing database, - /// such as when relocating validator keys to a new machine. - pub fn register_all_validators_for_slashing_protection(&self) -> Result<(), String> { - self.slashing_protection - .register_validators(self.validators.read().iter_voting_pubkeys()) - .map_err(|e| format!("Error while registering validators: {:?}", e)) - } - pub fn voting_pubkeys(&self) -> Vec { self.validators .read() @@ -235,7 +204,7 @@ impl ValidatorStore { warn!( self.log, "Not signing block for unregistered validator"; - "msg" => "Carefully consider running with --auto-register (see --help)", + "msg" => "Carefully consider running with --init-slashing-protection (see --help)", "public_key" => format!("{:?}", pk) ); None @@ -314,7 +283,7 @@ impl ValidatorStore { warn!( self.log, "Not signing attestation for unregistered validator"; - "msg" => "Carefully consider running with --auto-register (see --help)", + "msg" => "Carefully consider running with --init-slashing-protection (see --help)", "public_key" => format!("{:?}", pk) ); None