diff --git a/account_manager/src/validator/import.rs b/account_manager/src/validator/import.rs index f43dfcdb8f6..4c7140df39c 100644 --- a/account_manager/src/validator/import.rs +++ b/account_manager/src/validator/import.rs @@ -273,9 +273,15 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin eprintln!("Successfully imported keystore."); num_imported_keystores += 1; - let validator_def = - ValidatorDefinition::new_keystore_with_password(&dest_keystore, password_opt, None) - .map_err(|e| format!("Unable to create new validator definition: {:?}", e))?; + let graffiti = None; + let suggested_fee_recipient = None; + let validator_def = ValidatorDefinition::new_keystore_with_password( + &dest_keystore, + password_opt, + graffiti, + suggested_fee_recipient, + ) + .map_err(|e| format!("Unable to create new validator definition: {:?}", e))?; defs.push(validator_def); diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 4c2960c9d6a..5150ab492ba 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -108,7 +108,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { Arg::with_name("network-load") .long("network-load") .value_name("INTEGER") - .help("Lighthouse's network can be tuned for bandwidth/performance. Setting this to a high value, will increase the bandwidth lighthouse uses, increasing the likelihood of redundant information in exchange for faster communication. This can increase profit of validators marginally by receiving messages faster on the network. Lower values decrease bandwidth usage, but makes communication slower which can lead to validator performance reduction. Values are in the range [1,5].") + .help("Lighthouse's network can be tuned for bandwidth/performance. Setting this to a high value, will increase the bandwidth lighthouse uses, increasing the likelihood of redundant information in exchange for faster communication. This can increase profit of validators marginally by receiving messages faster on the network. Lower values decrease bandwidth usage, but makes communication slower which can lead to validator performance reduction. Values are in the range [1,5].") .default_value("3") .set(clap::ArgSettings::Hidden) .takes_value(true), @@ -409,9 +409,9 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .takes_value(true) ) .arg( - Arg::with_name("fee-recipient") - .long("fee-recipient") - .value_name("FEE-RECIPIENT") + Arg::with_name("suggested-fee-recipient") + .long("suggested-fee-recipient") + .value_name("SUGGESTED-FEE-RECIPIENT") .help("Once the merge has happened, this address will receive transaction fees \ collected from any blocks produced by this node. Defaults to a junk \ address whilst the merge is in development stages. THE DEFAULT VALUE \ diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 20408229311..7487acbde09 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -14,12 +14,7 @@ use std::net::{IpAddr, Ipv4Addr, ToSocketAddrs}; use std::net::{TcpListener, UdpSocket}; use std::path::{Path, PathBuf}; use std::str::FromStr; -use types::{Address, Checkpoint, Epoch, EthSpec, Hash256, PublicKeyBytes, GRAFFITI_BYTES_LEN}; - -// TODO(merge): remove this default value. It's just there to make life easy during -// early testnets. -const DEFAULT_SUGGESTED_FEE_RECIPIENT: [u8; 20] = - [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1]; +use types::{Checkpoint, Epoch, EthSpec, Hash256, PublicKeyBytes, GRAFFITI_BYTES_LEN}; /// Gets the fully-initialized global client. /// @@ -253,12 +248,8 @@ pub fn get_config( client_config.execution_endpoints = Some(client_config.eth1.endpoints.clone()); } - client_config.suggested_fee_recipient = Some( - clap_utils::parse_optional(cli_args, "fee-recipient")? - // TODO(merge): remove this default value. It's just there to make life easy during - // early testnets. - .unwrap_or_else(|| Address::from(DEFAULT_SUGGESTED_FEE_RECIPIENT)), - ); + client_config.suggested_fee_recipient = + clap_utils::parse_optional(cli_args, "suggested-fee-recipient")?; if let Some(freezer_dir) = cli_args.value_of("freezer-dir") { client_config.freezer_db_path = Some(PathBuf::from(freezer_dir)); diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index 7552d42306c..ee0ecb86a58 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -21,6 +21,7 @@ * [Voluntary Exits](./voluntary-exit.md) * [Validator Monitoring](./validator-monitoring.md) * [Doppelganger Protection](./validator-doppelganger.md) + * [Suggested Fee Recipient](./suggested-fee-recipient.md) * [APIs](./api.md) * [Beacon Node API](./api-bn.md) * [/lighthouse](./api-lighthouse.md) diff --git a/book/src/suggested-fee-recipient.md b/book/src/suggested-fee-recipient.md new file mode 100644 index 00000000000..6513495fe46 --- /dev/null +++ b/book/src/suggested-fee-recipient.md @@ -0,0 +1,91 @@ +# Suggested Fee Recipient + +*Note: these documents are not relevant until the Bellatrix (Merge) upgrade has occurred.* + +## Fee recipient trust assumptions + +During post-merge block production, the Beacon Node (BN) will provide a `suggested_fee_recipient` to +the execution node. This is a 20-byte Ethereum address which the EL might choose to set as the +coinbase and the recipient of other fees or rewards. + +There is no guarantee that an execution node will use the `suggested_fee_recipient` to collect fees, +it may use any address it chooses. It is assumed that an honest execution node *will* use the +`suggested_fee_recipient`, but users should note this trust assumption. + +The `suggested_fee_recipient` can be provided to the VC, who will transmit it to the BN. The also BN +has a choice regarding the fee recipient it passes to the execution node, creating another +noteworthy trust assumption. + +To be sure *you* control your fee recipient value, run your own BN and execution node (don't use +third-party services). + +The Lighthouse VC provides three methods for setting the `suggested_fee_recipient` (also known +simply as the "fee recipient") to be passed to the execution layer during block production. The +Lighthouse BN also provides a method for defining this value, should the VC not transmit a value. + +Assuming trustworthy nodes, the priority for the four methods is: + +1. `validator_definitions.yml` +1. `--suggested-fee-recipient-file` +1. `--suggested-fee-recipient` provided to the VC. +1. `--suggested-fee-recipient` provided to the BN. + +Users may configure the fee recipient via `validator_definitions.yml` or via the +`--suggested-fee-recipient-file` flag. The value in `validator_definitions.yml` will always take +precedence. + +### 1. Setting the fee recipient in the `validator_definitions.yml` + +Users can set the fee recipient in `validator_definitions.yml` with the `suggested_fee_recipient` +key. This option is recommended for most users, where each validator has a fixed fee recipient. + +Below is an example of the validator_definitions.yml with `suggested_fee_recipient` values: + +``` +--- +- enabled: true + voting_public_key: "0x87a580d31d7bc69069b55f5a01995a610dd391a26dc9e36e81057a17211983a79266800ab8531f21f1083d7d84085007" + type: local_keystore + voting_keystore_path: /home/paul/.lighthouse/validators/0x87a580d31d7bc69069b55f5a01995a610dd391a26dc9e36e81057a17211983a79266800ab8531f21f1083d7d84085007/voting-keystore.json + voting_keystore_password_path: /home/paul/.lighthouse/secrets/0x87a580d31d7bc69069b55f5a01995a610dd391a26dc9e36e81057a17211983a79266800ab8531f21f1083d7d84085007 + suggested_fee_recipient: "0x6cc8dcbca744a6e4ffedb98e1d0df903b10abd21" +- enabled: false + voting_public_key: "0xa5566f9ec3c6e1fdf362634ebec9ef7aceb0e460e5079714808388e5d48f4ae1e12897fed1bea951c17fa389d511e477" + type: local_keystore voting_keystore_path: /home/paul/.lighthouse/validators/0xa5566f9ec3c6e1fdf362634ebec9ef7aceb0e460e5079714808388e5d48f4ae1e12897fed1bea951c17fa389d511e477/voting-keystore.json + voting_keystore_password: myStrongpa55word123&$ + suggested_fee_recipient: "0xa2e334e71511686bcfe38bb3ee1ad8f6babcc03d" +``` + +### 2. Using the "--suggested-fee-recipient-file" flag on the validator client + +Users can specify a file with the `--suggested-fee-recipient-file` flag. This option is useful for dynamically +changing fee recipients. This file is reloaded each time a validator is chosen to propose a block. + +Usage: +`lighthouse vc --suggested-fee-recipient-file fee_recipient.txt` + +The file should contain key value pairs corresponding to validator public keys and their associated +fee recipient. The file can optionally contain a `default` key for the default case. + +The following example sets the default and the values for the validators with pubkeys `0x87a5` and +`0xa556`: + +``` +default: 0x6cc8dcbca744a6e4ffedb98e1d0df903b10abd21 +0x87a580d31d7bc69069b55f5a01995a610dd391a26dc9e36e81057a17211983a79266800ab8531f21f1083d7d84085007: 0x6cc8dcbca744a6e4ffedb98e1d0df903b10abd21 +0xa5566f9ec3c6e1fdf362634ebec9ef7aceb0e460e5079714808388e5d48f4ae1e12897fed1bea951c17fa389d511e477: 0xa2e334e71511686bcfe38bb3ee1ad8f6babcc03d +``` + +Lighthouse will first search for the fee recipient corresponding to the public key of the proposing +validator, if there are no matches for the public key, then it uses the address corresponding to the +default key (if present). + +### 3. Using the "--suggested-fee-recipient" flag on the validator client + +The `--suggested-fee-recipient` can be provided to the VC to act as a default value for all +validators where a `suggested_fee_recipient` is not loaded from another method. + +### 4. Using the "--suggested-fee-recipient" flag on the beacon node + +The `--suggested-fee-recipient` can be provided to the BN to act as a default value when the +validator client does not transmit a `suggested_fee_recipient` to the BN. diff --git a/common/account_utils/src/validator_definitions.rs b/common/account_utils/src/validator_definitions.rs index 418c0fb3c68..b8f0d494637 100644 --- a/common/account_utils/src/validator_definitions.rs +++ b/common/account_utils/src/validator_definitions.rs @@ -13,7 +13,7 @@ use std::collections::HashSet; use std::fs::{self, OpenOptions}; use std::io; use std::path::{Path, PathBuf}; -use types::{graffiti::GraffitiString, PublicKey}; +use types::{graffiti::GraffitiString, Address, PublicKey}; use validator_dir::VOTING_KEYSTORE_FILE; /// The file name for the serialized `ValidatorDefinitions` struct. @@ -90,6 +90,9 @@ pub struct ValidatorDefinition { #[serde(skip_serializing_if = "Option::is_none")] pub graffiti: Option, #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub suggested_fee_recipient: Option
, + #[serde(default)] pub description: String, #[serde(flatten)] pub signing_definition: SigningDefinition, @@ -106,6 +109,7 @@ impl ValidatorDefinition { voting_keystore_path: P, voting_keystore_password: Option, graffiti: Option, + suggested_fee_recipient: Option
, ) -> Result { let voting_keystore_path = voting_keystore_path.as_ref().into(); let keystore = @@ -117,6 +121,7 @@ impl ValidatorDefinition { voting_public_key, description: keystore.description().unwrap_or("").to_string(), graffiti, + suggested_fee_recipient, signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, voting_keystore_password_path: None, @@ -262,6 +267,7 @@ impl ValidatorDefinitions { voting_public_key, description: keystore.description().unwrap_or("").to_string(), graffiti: None, + suggested_fee_recipient: None, signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, voting_keystore_password_path, @@ -458,4 +464,45 @@ mod tests { Some(GraffitiString::from_str("mrfwashere").unwrap()) ); } + + #[test] + fn suggested_fee_recipient_checks() { + let no_suggested_fee_recipient = r#"--- + description: "" + enabled: true + type: local_keystore + voting_keystore_path: "" + voting_public_key: "0xaf3c7ddab7e293834710fca2d39d068f884455ede270e0d0293dc818e4f2f0f975355067e8437955cb29aec674e5c9e7" + "#; + let def: ValidatorDefinition = serde_yaml::from_str(no_suggested_fee_recipient).unwrap(); + assert!(def.suggested_fee_recipient.is_none()); + + let invalid_suggested_fee_recipient = r#"--- + description: "" + enabled: true + type: local_keystore + suggested_fee_recipient: "foopy" + voting_keystore_path: "" + voting_public_key: "0xaf3c7ddab7e293834710fca2d39d068f884455ede270e0d0293dc818e4f2f0f975355067e8437955cb29aec674e5c9e7" + "#; + + let def: Result = + serde_yaml::from_str(invalid_suggested_fee_recipient); + assert!(def.is_err()); + + let valid_suggested_fee_recipient = r#"--- + description: "" + enabled: true + type: local_keystore + suggested_fee_recipient: "0xa2e334e71511686bcfe38bb3ee1ad8f6babcc03d" + voting_keystore_path: "" + voting_public_key: "0xaf3c7ddab7e293834710fca2d39d068f884455ede270e0d0293dc818e4f2f0f975355067e8437955cb29aec674e5c9e7" + "#; + + let def: ValidatorDefinition = serde_yaml::from_str(valid_suggested_fee_recipient).unwrap(); + assert_eq!( + def.suggested_fee_recipient, + Some(Address::from_str("0xa2e334e71511686bcfe38bb3ee1ad8f6babcc03d").unwrap()) + ); + } } diff --git a/common/eth2/src/lighthouse_vc/types.rs b/common/eth2/src/lighthouse_vc/types.rs index 9e311c9d6ba..bf4767f700a 100644 --- a/common/eth2/src/lighthouse_vc/types.rs +++ b/common/eth2/src/lighthouse_vc/types.rs @@ -22,6 +22,9 @@ pub struct ValidatorRequest { #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub graffiti: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub suggested_fee_recipient: Option
, #[serde(with = "eth2_serde_utils::quoted_u64")] pub deposit_gwei: u64, } @@ -42,6 +45,9 @@ pub struct CreatedValidator { #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub graffiti: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub suggested_fee_recipient: Option
, pub eth1_deposit_tx_data: String, #[serde(with = "eth2_serde_utils::quoted_u64")] pub deposit_gwei: u64, @@ -64,6 +70,7 @@ pub struct KeystoreValidatorsPostRequest { pub enable: bool, pub keystore: Keystore, pub graffiti: Option, + pub suggested_fee_recipient: Option
, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -73,6 +80,9 @@ pub struct Web3SignerValidatorRequest { #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub graffiti: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub suggested_fee_recipient: Option
, pub voting_public_key: PublicKey, pub url: String, #[serde(default)] diff --git a/lighthouse/tests/account_manager.rs b/lighthouse/tests/account_manager.rs index 96be44fcad8..fcc1d2aee22 100644 --- a/lighthouse/tests/account_manager.rs +++ b/lighthouse/tests/account_manager.rs @@ -493,6 +493,7 @@ fn validator_import_launchpad() { enabled: false, description: "".into(), graffiti: None, + suggested_fee_recipient: None, voting_public_key: keystore.public_key().unwrap(), signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, @@ -612,6 +613,7 @@ fn validator_import_launchpad_no_password_then_add_password() { enabled: true, description: "".into(), graffiti: None, + suggested_fee_recipient: None, voting_public_key: keystore.public_key().unwrap(), signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, @@ -635,6 +637,7 @@ fn validator_import_launchpad_no_password_then_add_password() { enabled: true, description: "".into(), graffiti: None, + suggested_fee_recipient: None, voting_public_key: keystore.public_key().unwrap(), signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path: dst_keystore_dir.join(KEYSTORE_NAME), @@ -734,6 +737,7 @@ fn validator_import_launchpad_password_file() { description: "".into(), voting_public_key: keystore.public_key().unwrap(), graffiti: None, + suggested_fee_recipient: None, signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, voting_keystore_password_path: None, diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index 6d03cafe10b..f630ed8e73b 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -212,7 +212,7 @@ fn merge_fee_recipient_flag() { CommandLineTest::new() .flag("merge", None) .flag( - "fee-recipient", + "suggested-fee-recipient", Some("0x00000000219ab540356cbb839cbe05303d7705fa"), ) .run_with_zero_port() diff --git a/lighthouse/tests/validator_client.rs b/lighthouse/tests/validator_client.rs index 68da7c706ba..e682471c469 100644 --- a/lighthouse/tests/validator_client.rs +++ b/lighthouse/tests/validator_client.rs @@ -220,12 +220,12 @@ fn graffiti_file_with_pk_flag() { }); } -// Tests for fee-recipient flags. +// Tests for suggested-fee-recipient flags. #[test] fn fee_recipient_flag() { CommandLineTest::new() .flag( - "fee-recipient", + "suggested-fee-recipient", Some("0x00000000219ab540356cbb839cbe05303d7705fa"), ) .run() @@ -248,7 +248,7 @@ fn fee_recipient_file_flag() { .expect("Unable to write to file"); CommandLineTest::new() .flag( - "fee-recipient-file", + "suggested-fee-recipient-file", dir.path().join("fee_recipient.txt").as_os_str().to_str(), ) .run() @@ -280,7 +280,7 @@ fn fee_recipient_file_with_pk_flag() { .expect("Unable to write to file"); CommandLineTest::new() .flag( - "fee-recipient-file", + "suggested-fee-recipient-file", dir.path().join("fee_recipient.txt").as_os_str().to_str(), ) .run() diff --git a/testing/web3signer_tests/src/lib.rs b/testing/web3signer_tests/src/lib.rs index 6d6ee9933eb..4c08ceb7b22 100644 --- a/testing/web3signer_tests/src/lib.rs +++ b/testing/web3signer_tests/src/lib.rs @@ -336,6 +336,7 @@ mod tests { enabled: true, voting_public_key: validator_pubkey.clone(), graffiti: None, + suggested_fee_recipient: None, description: String::default(), signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path: signer_rig.keystore_path.clone(), @@ -351,6 +352,7 @@ mod tests { enabled: true, voting_public_key: validator_pubkey.clone(), graffiti: None, + suggested_fee_recipient: None, description: String::default(), signing_definition: SigningDefinition::Web3Signer { url: signer_rig.url.to_string(), diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index c1e0fa28734..db078c3c56e 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -127,22 +127,20 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .takes_value(true) .conflicts_with("graffiti") ) - // This overwrites the fee-recipient configured in the beacon node. .arg( - Arg::with_name("fee-recipient") - .long("fee-recipient") + Arg::with_name("suggested-fee-recipient") + .long("suggested-fee-recipient") .help("Specify the address, that will receive transaction fees collected from any \ blocks produced by this validator client.") .value_name("FEE-RECIPIENT") .takes_value(true) ) .arg( - Arg::with_name("fee-recipient-file") - .long("fee-recipient-file") + Arg::with_name("suggested-fee-recipient-file") + .long("suggested-fee-recipient-file") .help("Specify a fee-recipient file to load validator fee-recipients from.") .value_name("FEE-RECIPIENT-FILE") .takes_value(true) - .conflicts_with("fee-recipient") ) /* REST API related arguments */ .arg( diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index f6e210be914..9758537b9bb 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -45,7 +45,7 @@ pub struct Config { /// fee_recipient address, that will receive transaction fees collected from any blocks /// produced by this node. pub fee_recipient: Option
, - /// Fee recipient file to load per validator fee-recipients. + /// Fee recipient file to load per validator suggested-fee-recipients. pub fee_recipient_file: Option, /// Configuration for the HTTP REST API. pub http_api: http_api::Config, @@ -205,16 +205,22 @@ impl Config { } } - if let Some(fee_recipient_file_path) = cli_args.value_of("fee-recipient-file") { + if let Some(fee_recipient_file_path) = cli_args.value_of("suggested-fee-recipient-file") { let mut fee_recipient_file = FeeRecipientFile::new(fee_recipient_file_path.into()); fee_recipient_file .read_fee_recipient_file() - .map_err(|e| format!("Error reading fee-recipient file: {:?}", e))?; + .map_err(|e| format!("Error reading suggested-fee-recipient file: {:?}", e))?; config.fee_recipient_file = Some(fee_recipient_file); - info!(log, "Successfully loaded fee_recipient file"; "path" => fee_recipient_file_path); + info!( + log, + "Successfully loaded suggested-fee-recipient file"; + "path" => fee_recipient_file_path + ); } - if let Some(input_fee_recipient) = parse_optional::
(cli_args, "fee-recipient")? { + if let Some(input_fee_recipient) = + parse_optional::
(cli_args, "suggested-fee-recipient")? + { config.fee_recipient = Some(input_fee_recipient); } diff --git a/validator_client/src/http_api/create_validator.rs b/validator_client/src/http_api/create_validator.rs index 3c4901e6145..a8e4fd26290 100644 --- a/validator_client/src/http_api/create_validator.rs +++ b/validator_client/src/http_api/create_validator.rs @@ -139,6 +139,7 @@ pub async fn create_validators_mnemonic, T: 'static + SlotClock, voting_password_string, request.enable, request.graffiti.clone(), + request.suggested_fee_recipient, ) .await .map_err(|e| { @@ -152,6 +153,7 @@ pub async fn create_validators_mnemonic, T: 'static + SlotClock, enabled: request.enable, description: request.description.clone(), graffiti: request.graffiti.clone(), + suggested_fee_recipient: request.suggested_fee_recipient, voting_pubkey, eth1_deposit_tx_data: eth2_serde_utils::hex::encode(ð1_deposit_data.rlp), deposit_gwei: request.deposit_gwei, @@ -170,6 +172,7 @@ pub async fn create_validators_web3signer( enabled: request.enable, voting_public_key: request.voting_public_key.clone(), graffiti: request.graffiti.clone(), + suggested_fee_recipient: request.suggested_fee_recipient, description: request.description.clone(), signing_definition: SigningDefinition::Web3Signer { url: request.url.clone(), diff --git a/validator_client/src/http_api/mod.rs b/validator_client/src/http_api/mod.rs index 5e0f3443a25..29398423428 100644 --- a/validator_client/src/http_api/mod.rs +++ b/validator_client/src/http_api/mod.rs @@ -386,6 +386,7 @@ pub fn serve( drop(validator_dir); let voting_password = body.password.clone(); let graffiti = body.graffiti.clone(); + let suggested_fee_recipient = body.suggested_fee_recipient; let validator_def = { if let Some(runtime) = runtime.upgrade() { @@ -395,6 +396,7 @@ pub fn serve( voting_password, body.enable, graffiti, + suggested_fee_recipient, )) .map_err(|e| { warp_utils::reject::custom_server_error(format!( diff --git a/validator_client/src/http_api/tests.rs b/validator_client/src/http_api/tests.rs index c9ef869be50..1d860c4b492 100644 --- a/validator_client/src/http_api/tests.rs +++ b/validator_client/src/http_api/tests.rs @@ -263,6 +263,7 @@ impl ApiTester { enable: !s.disabled.contains(&i), description: format!("boi #{}", i), graffiti: None, + suggested_fee_recipient: None, deposit_gwei: E::default_spec().max_effective_balance, }) .collect::>(); @@ -393,6 +394,7 @@ impl ApiTester { .into(), keystore, graffiti: None, + suggested_fee_recipient: None, }; self.client @@ -410,6 +412,7 @@ impl ApiTester { .into(), keystore, graffiti: None, + suggested_fee_recipient: None, }; let response = self @@ -445,6 +448,7 @@ impl ApiTester { enable: s.enabled, description: format!("{}", i), graffiti: None, + suggested_fee_recipient: None, voting_public_key: kp.pk, url: format!("http://signer_{}.com/", i), root_certificate_path: None, @@ -570,6 +574,7 @@ fn routes_with_invalid_auth() { enable: <_>::default(), description: <_>::default(), graffiti: <_>::default(), + suggested_fee_recipient: <_>::default(), deposit_gwei: <_>::default(), }]) .await @@ -598,6 +603,7 @@ fn routes_with_invalid_auth() { enable: <_>::default(), keystore, graffiti: <_>::default(), + suggested_fee_recipient: <_>::default(), }) .await }) diff --git a/validator_client/src/initialized_validators.rs b/validator_client/src/initialized_validators.rs index 72e651f7d18..df5f87817d7 100644 --- a/validator_client/src/initialized_validators.rs +++ b/validator_client/src/initialized_validators.rs @@ -25,7 +25,7 @@ use std::io::{self, Read}; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; -use types::{Graffiti, Keypair, PublicKey, PublicKeyBytes}; +use types::{Address, Graffiti, Keypair, PublicKey, PublicKeyBytes}; use url::{ParseError, Url}; use crate::key_cache; @@ -95,6 +95,7 @@ impl From for Error { pub struct InitializedValidator { signing_method: Arc, graffiti: Option, + suggested_fee_recipient: Option
, /// The validators index in `state.validators`, to be updated by an external service. index: Option, } @@ -257,6 +258,7 @@ impl InitializedValidator { Ok(Self { signing_method: Arc::new(signing_method), graffiti: def.graffiti.map(Into::into), + suggested_fee_recipient: def.suggested_fee_recipient, index: None, }) } @@ -422,6 +424,14 @@ impl InitializedValidators { self.validators.get(public_key).and_then(|v| v.graffiti) } + /// Returns the `suggested_fee_recipient` for a given public key specified in the + /// `ValidatorDefinitions`. + pub fn suggested_fee_recipient(&self, public_key: &PublicKeyBytes) -> Option
{ + self.validators + .get(public_key) + .and_then(|v| v.suggested_fee_recipient) + } + /// Sets the `InitializedValidator` and `ValidatorDefinition` `enabled` values. /// /// ## Notes diff --git a/validator_client/src/preparation_service.rs b/validator_client/src/preparation_service.rs index 0875c00b31c..d0f942c523c 100644 --- a/validator_client/src/preparation_service.rs +++ b/validator_client/src/preparation_service.rs @@ -134,28 +134,28 @@ impl PreparationService { ); let executor = self.context.executor.clone(); + let spec = spec.clone(); let interval_fut = async move { loop { - if let Some(duration_to_next_epoch) = - self.slot_clock.duration_to_next_epoch(E::slots_per_epoch()) - { - sleep(duration_to_next_epoch).await; - self.prepare_proposers_and_publish() - .await - .map_err(|e| { - error!( - log, - "Error during proposer preparation"; - "error" => format!("{:?}", e), - ) - }) - .unwrap_or(()); + // Poll the endpoint immediately to ensure fee recipients are received. + self.prepare_proposers_and_publish(&spec) + .await + .map_err(|e| { + error!( + log, + "Error during proposer preparation"; + "error" => format!("{:?}", e), + ) + }) + .unwrap_or(()); + + if let Some(duration_to_next_slot) = self.slot_clock.duration_to_next_slot() { + sleep(duration_to_next_slot).await; } else { error!(log, "Failed to read slot clock"); // If we can't read the slot clock, just wait another slot. sleep(slot_duration).await; - continue; } } }; @@ -165,8 +165,8 @@ impl PreparationService { } /// Prepare proposer preparations and send to beacon node - async fn prepare_proposers_and_publish(&self) -> Result<(), String> { - let preparation_data = self.collect_preparation_data().unwrap(); + async fn prepare_proposers_and_publish(&self, spec: &ChainSpec) -> Result<(), String> { + let preparation_data = self.collect_preparation_data(spec).unwrap(); if !preparation_data.is_empty() { self.publish_preparation_data(preparation_data).await?; } @@ -174,7 +174,10 @@ impl PreparationService { Ok(()) } - fn collect_preparation_data(&self) -> Result, String> { + fn collect_preparation_data( + &self, + spec: &ChainSpec, + ) -> Result, String> { let log = self.context.log(); let fee_recipient_file = self @@ -202,18 +205,41 @@ impl PreparationService { .filter_map(|pubkey| { let validator_index = self.validator_store.validator_index(&pubkey); if let Some(validator_index) = validator_index { - let fee_recipient = fee_recipient_file - .as_ref() - .and_then(|g| match g.get_fee_recipient(&pubkey) { - Ok(g) => g, - Err(_e) => None, - }) - .or(self.fee_recipient); + let fee_recipient = if let Some(from_validator_defs) = + self.validator_store.suggested_fee_recipient(&pubkey) + { + // If there is a `suggested_fee_recipient` in the validator definitions yaml + // file, use that value. + Some(from_validator_defs) + } else { + // If there's nothing in the validator defs file, check the fee recipient + // file. + fee_recipient_file + .as_ref() + .and_then(|f| match f.get_fee_recipient(&pubkey) { + Ok(f) => f, + Err(_e) => None, + }) + // If there's nothing in the file, try the process-level default value. + .or(self.fee_recipient) + }; - fee_recipient.map(|fee_recipient| ProposerPreparationData { - validator_index, - fee_recipient, - }) + if let Some(fee_recipient) = fee_recipient { + Some(ProposerPreparationData { + validator_index, + fee_recipient, + }) + } else { + if spec.bellatrix_fork_epoch.is_some() { + error!( + log, + "Validator is missing fee recipient"; + "msg" => "update validator_definitions.yml", + "pubkey" => ?pubkey + ); + } + None + } } else { None } diff --git a/validator_client/src/validator_store.rs b/validator_client/src/validator_store.rs index d7efa806aef..18a3259bbc1 100644 --- a/validator_client/src/validator_store.rs +++ b/validator_client/src/validator_store.rs @@ -15,7 +15,7 @@ use std::path::Path; use std::sync::Arc; use task_executor::TaskExecutor; use types::{ - attestation::Error as AttestationError, graffiti::GraffitiString, AggregateAndProof, + attestation::Error as AttestationError, graffiti::GraffitiString, Address, AggregateAndProof, Attestation, BeaconBlock, ChainSpec, ContributionAndProof, Domain, Epoch, EthSpec, Fork, Graffiti, Hash256, Keypair, PublicKeyBytes, SelectionProof, Signature, SignedAggregateAndProof, SignedBeaconBlock, SignedContributionAndProof, Slot, SyncAggregatorSelectionData, @@ -146,11 +146,13 @@ impl ValidatorStore { password: ZeroizeString, enable: bool, graffiti: Option, + suggested_fee_recipient: Option
, ) -> Result { let mut validator_def = ValidatorDefinition::new_keystore_with_password( voting_keystore_path, Some(password), graffiti.map(Into::into), + suggested_fee_recipient, ) .map_err(|e| format!("failed to create validator definitions: {:?}", e))?; @@ -349,6 +351,12 @@ impl ValidatorStore { self.validators.read().graffiti(validator_pubkey) } + pub fn suggested_fee_recipient(&self, validator_pubkey: &PublicKeyBytes) -> Option
{ + self.validators + .read() + .suggested_fee_recipient(validator_pubkey) + } + pub async fn sign_block( &self, validator_pubkey: PublicKeyBytes,