From f9e36c94edf69c87fafb912f81c124714855a84f Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Thu, 25 Jan 2024 00:09:47 +0200 Subject: [PATCH] Expose additional builder booster related flags in the vc (#5086) * expose builder booster flags in vc, enable options in validator endpoints, update tests * resolve failing test * fix issues related to CreateConfig and MoveConfig * remove unneeded val, change how boost factor flag logic in the vc, add some additional documentation * fix typos * fix typos * assume builder-proosals flag if one of other two vc builder flags are present * fmt * typo * typo * Fix CLI help text * Prioritise per validator builder boost configurations over CLI flags. * Add http test for builder boost factor with process defaults. * Fix issue with PATCH request * Add prefer builder proposals * Add more builder boost factor tests. --------- Co-authored-by: Mac L Co-authored-by: Jimmy Chen Co-authored-by: Paul Hauner --- account_manager/src/validator/import.rs | 2 + book/src/api-vc-endpoints.md | 2 +- book/src/builders.md | 18 +- book/src/help_vc.md | 6 + book/src/help_vm_create.md | 10 +- book/src/help_vm_move.md | 8 +- .../src/validator_definitions.rs | 13 + common/eth2/src/lighthouse_vc/http_client.rs | 5 + common/eth2/src/lighthouse_vc/types.rs | 24 ++ lighthouse/tests/account_manager.rs | 8 + lighthouse/tests/validator_client.rs | 26 ++ lighthouse/tests/validator_manager.rs | 47 ++++ testing/web3signer_tests/src/lib.rs | 4 + validator_client/src/block_service.rs | 39 ++- validator_client/src/cli.rs | 18 ++ validator_client/src/config.rs | 12 + .../src/http_api/create_validator.rs | 2 + validator_client/src/http_api/keystores.rs | 2 + validator_client/src/http_api/mod.rs | 12 + validator_client/src/http_api/remotekeys.rs | 2 + validator_client/src/http_api/test_utils.rs | 22 +- validator_client/src/http_api/tests.rs | 232 +++++++++++++++++- .../src/http_api/tests/keystores.rs | 4 +- .../src/initialized_validators.rs | 43 ++++ validator_client/src/validator_store.rs | 89 ++++++- validator_manager/src/common.rs | 6 + validator_manager/src/create_validators.rs | 39 +++ validator_manager/src/move_validators.rs | 41 +++- 28 files changed, 715 insertions(+), 21 deletions(-) diff --git a/account_manager/src/validator/import.rs b/account_manager/src/validator/import.rs index 339d9a291..bf000385f 100644 --- a/account_manager/src/validator/import.rs +++ b/account_manager/src/validator/import.rs @@ -284,6 +284,8 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin suggested_fee_recipient, None, None, + None, + None, ) .map_err(|e| format!("Unable to create new validator definition: {:?}", e))?; diff --git a/book/src/api-vc-endpoints.md b/book/src/api-vc-endpoints.md index f41625ad8..d3b2edafe 100644 --- a/book/src/api-vc-endpoints.md +++ b/book/src/api-vc-endpoints.md @@ -427,7 +427,7 @@ Example Response Body ## `PATCH /lighthouse/validators/:voting_pubkey` -Update some values for the validator with `voting_pubkey`. Possible fields: `enabled`, `gas_limit`, `builder_proposals`, +Update some values for the validator with `voting_pubkey`. Possible fields: `enabled`, `gas_limit`, `builder_proposals`, `builder_boost_factor`, `prefer_builder_proposals` and `graffiti`. The following example updates a validator from `enabled: true` to `enabled: false`. ### HTTP Specification diff --git a/book/src/builders.md b/book/src/builders.md index e48cc0a88..014e43211 100644 --- a/book/src/builders.md +++ b/book/src/builders.md @@ -31,6 +31,18 @@ blinded blocks, you should use the following flag: lighthouse vc --builder-proposals ``` With the `--builder-proposals` flag, the validator client will ask for blinded blocks for all validators it manages. + +``` +lighthouse vc --prefer-builder-proposals +``` +With the `--prefer-builder-proposals` flag, the validator client will always prefer blinded blocks, regardless of the payload value, for all validators it manages. + +``` +lighthouse vc --builder-boost-factor +``` +With the `--builder-boost-factor` flag, a percentage multiplier is applied to the builder's payload value when choosing between a +builder payload header and payload from the paired execution node. + In order to configure whether a validator queries for blinded blocks check out [this section.](#validator-client-configuration) ## Multiple builders @@ -46,9 +58,9 @@ relays, run one of the following services and configure lighthouse to use it wit In the validator client you can configure gas limit and fee recipient on a per-validator basis. If no gas limit is configured, Lighthouse will use a default gas limit of 30,000,000, which is the current default value used in execution engines. You can also enable or disable use of external builders on a per-validator basis rather than using -`--builder-proposals`, which enables external builders for all validators. In order to manage these configurations -per-validator, you can either make updates to the `validator_definitions.yml` file or you can use the HTTP requests -described below. +`--builder-proposals`, `--builder-boost-factor` or `--prefer-builder-proposals`, which apply builder related preferences for all validators. +In order to manage these configurations per-validator, you can either make updates to the `validator_definitions.yml` file +or you can use the HTTP requests described below. Both the gas limit and fee recipient will be passed along as suggestions to connected builders. If there is a discrepancy in either, it will *not* keep you from proposing a block with the builder. This is because the bounds on gas limit are diff --git a/book/src/help_vc.md b/book/src/help_vc.md index 62b64efd4..bc6deec1e 100644 --- a/book/src/help_vc.md +++ b/book/src/help_vc.md @@ -56,6 +56,9 @@ FLAGS: machine. Note that logs can often contain sensitive information about your validator and so this flag should be used with caution. For Windows users, the log file permissions will be inherited from the parent folder. --metrics Enable the Prometheus metrics HTTP server. Disabled by default. + --prefer-builder-proposals + If this flag is set, Lighthouse will always prefer blocks constructed by builders, regardless of payload + value. --produce-block-v3 Enable block production via the block v3 endpoint for this validator client. This should only be enabled when paired with a beacon node that has this endpoint implemented. This flag will be enabled by default in @@ -80,6 +83,9 @@ OPTIONS: Comma-separated list of beacon API topics to broadcast to all beacon nodes. Possible values are: none, attestations, blocks, subscriptions, sync-committee. Default (when flag is omitted) is to broadcast subscriptions only. + --builder-boost-factor + Defines the boost factor, a percentage multiplier to apply to the builder's payload value when choosing + between a builder payload header and payload from the local execution node. --builder-registration-timestamp-override This flag takes a unix timestamp value that will be used to override the timestamp used in the builder api registration diff --git a/book/src/help_vm_create.md b/book/src/help_vm_create.md index 505ea8638..71db3cc59 100644 --- a/book/src/help_vm_create.md +++ b/book/src/help_vm_create.md @@ -42,6 +42,9 @@ OPTIONS: A HTTP(S) address of a beacon node using the beacon-API. If this value is provided, an error will be raised if any validator key here is already known as a validator by that beacon node. This helps prevent the same validator being created twice and therefore slashable conditions. + --builder-boost-factor + Defines the boost factor, a percentage multiplier to apply to the builder's payload value when choosing + between a builder payload header and payload from the local execution node. --builder-proposals When provided, all created validators will attempt to create blocks via builder rather than the local EL. [possible values: true, false] @@ -93,13 +96,18 @@ OPTIONS: --logfile-max-size The maximum size (in MB) each log file can grow to before rotating. If set to 0, background file logging is disabled. [default: 200] - --mnemonic-path If present, the mnemonic will be read in from this file. + --mnemonic-path + If present, the mnemonic will be read in from this file. + --network Name of the Eth2 chain Lighthouse will sync and follow. [possible values: mainnet, prater, goerli, gnosis, chiado, sepolia, holesky] --output-path The path to a directory where the validator and (optionally) deposits files will be created. The directory will be created if it does not exist. + --prefer-builder-proposals + If this flag is set, Lighthouse will always prefer blocks constructed by builders, regardless of payload + value. [possible values: true, false] --safe-slots-to-import-optimistically Used to coordinate manual overrides of the SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY parameter. This flag should only be used if the user has a clear understanding that the broad Ethereum community has elected to override diff --git a/book/src/help_vm_move.md b/book/src/help_vm_move.md index dea440dca..a89af437a 100644 --- a/book/src/help_vm_move.md +++ b/book/src/help_vm_move.md @@ -26,10 +26,13 @@ FLAGS: -V, --version Prints version information OPTIONS: + --builder-boost-factor + Defines the boost factor, a percentage multiplier to apply to the builder's payload value when choosing + between a builder payload header and payload from the local execution node. --builder-proposals When provided, all created validators will attempt to create blocks via builder rather than the local EL. [possible values: true, false] - --count The number of validators to move. + --count The number of validators to move. -d, --datadir Used to specify a custom root data directory for lighthouse keys and databases. Defaults to $HOME/.lighthouse/{network} where network is the value of the `network` flag Note: Users should specify @@ -75,6 +78,9 @@ OPTIONS: --network Name of the Eth2 chain Lighthouse will sync and follow. [possible values: mainnet, prater, goerli, gnosis, chiado, sepolia, holesky] + --prefer-builder-proposals + If this flag is set, Lighthouse will always prefer blocks constructed by builders, regardless of payload + value. [possible values: true, false] --safe-slots-to-import-optimistically Used to coordinate manual overrides of the SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY parameter. This flag should only be used if the user has a clear understanding that the broad Ethereum community has elected to override diff --git a/common/account_utils/src/validator_definitions.rs b/common/account_utils/src/validator_definitions.rs index 61c65a29b..f228ce5fd 100644 --- a/common/account_utils/src/validator_definitions.rs +++ b/common/account_utils/src/validator_definitions.rs @@ -157,6 +157,12 @@ pub struct ValidatorDefinition { #[serde(skip_serializing_if = "Option::is_none")] pub builder_proposals: Option, #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub builder_boost_factor: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub prefer_builder_proposals: Option, + #[serde(default)] pub description: String, #[serde(flatten)] pub signing_definition: SigningDefinition, @@ -169,6 +175,7 @@ impl ValidatorDefinition { /// ## Notes /// /// This function does not check the password against the keystore. + #[allow(clippy::too_many_arguments)] pub fn new_keystore_with_password>( voting_keystore_path: P, voting_keystore_password_storage: PasswordStorage, @@ -176,6 +183,8 @@ impl ValidatorDefinition { suggested_fee_recipient: Option
, gas_limit: Option, builder_proposals: Option, + builder_boost_factor: Option, + prefer_builder_proposals: Option, ) -> Result { let voting_keystore_path = voting_keystore_path.as_ref().into(); let keystore = @@ -196,6 +205,8 @@ impl ValidatorDefinition { suggested_fee_recipient, gas_limit, builder_proposals, + builder_boost_factor, + prefer_builder_proposals, signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, voting_keystore_password_path, @@ -344,6 +355,8 @@ impl ValidatorDefinitions { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, voting_keystore_password_path, diff --git a/common/eth2/src/lighthouse_vc/http_client.rs b/common/eth2/src/lighthouse_vc/http_client.rs index 2e6756c63..83aeea4bf 100644 --- a/common/eth2/src/lighthouse_vc/http_client.rs +++ b/common/eth2/src/lighthouse_vc/http_client.rs @@ -483,12 +483,15 @@ impl ValidatorClientHttpClient { } /// `PATCH lighthouse/validators/{validator_pubkey}` + #[allow(clippy::too_many_arguments)] pub async fn patch_lighthouse_validators( &self, voting_pubkey: &PublicKeyBytes, enabled: Option, gas_limit: Option, builder_proposals: Option, + builder_boost_factor: Option, + prefer_builder_proposals: Option, graffiti: Option, ) -> Result<(), Error> { let mut path = self.server.full.clone(); @@ -505,6 +508,8 @@ impl ValidatorClientHttpClient { enabled, gas_limit, builder_proposals, + builder_boost_factor, + prefer_builder_proposals, graffiti, }, ) diff --git a/common/eth2/src/lighthouse_vc/types.rs b/common/eth2/src/lighthouse_vc/types.rs index 230293f1b..d903d7b73 100644 --- a/common/eth2/src/lighthouse_vc/types.rs +++ b/common/eth2/src/lighthouse_vc/types.rs @@ -32,6 +32,12 @@ pub struct ValidatorRequest { #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub builder_proposals: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub builder_boost_factor: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub prefer_builder_proposals: Option, #[serde(with = "serde_utils::quoted_u64")] pub deposit_gwei: u64, } @@ -86,6 +92,12 @@ pub struct ValidatorPatchRequest { #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub graffiti: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub builder_boost_factor: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub prefer_builder_proposals: Option, } #[derive(Clone, PartialEq, Serialize, Deserialize)] @@ -105,6 +117,12 @@ pub struct KeystoreValidatorsPostRequest { #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub builder_proposals: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub builder_boost_factor: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub prefer_builder_proposals: Option, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -135,6 +153,12 @@ pub struct Web3SignerValidatorRequest { pub client_identity_path: Option, #[serde(skip_serializing_if = "Option::is_none")] pub client_identity_password: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub builder_boost_factor: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub prefer_builder_proposals: Option, } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] diff --git a/lighthouse/tests/account_manager.rs b/lighthouse/tests/account_manager.rs index 63d79fceb..f82e3ec71 100644 --- a/lighthouse/tests/account_manager.rs +++ b/lighthouse/tests/account_manager.rs @@ -492,6 +492,8 @@ fn validator_import_launchpad() { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, voting_public_key: keystore.public_key().unwrap(), signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, @@ -614,6 +616,8 @@ fn validator_import_launchpad_no_password_then_add_password() { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, voting_public_key: keystore.public_key().unwrap(), signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, @@ -640,6 +644,8 @@ fn validator_import_launchpad_no_password_then_add_password() { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, voting_public_key: keystore.public_key().unwrap(), signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path: dst_keystore_dir.join(KEYSTORE_NAME), @@ -742,6 +748,8 @@ fn validator_import_launchpad_password_file() { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, voting_keystore_password_path: None, diff --git a/lighthouse/tests/validator_client.rs b/lighthouse/tests/validator_client.rs index 025188fed..701aed07e 100644 --- a/lighthouse/tests/validator_client.rs +++ b/lighthouse/tests/validator_client.rs @@ -464,6 +464,32 @@ fn builder_proposals_flag() { .with_config(|config| assert!(config.builder_proposals)); } #[test] +fn builder_boost_factor_flag() { + CommandLineTest::new() + .flag("builder-boost-factor", Some("150")) + .run() + .with_config(|config| assert_eq!(config.builder_boost_factor, Some(150))); +} +#[test] +fn no_builder_boost_factor_flag() { + CommandLineTest::new() + .run() + .with_config(|config| assert_eq!(config.builder_boost_factor, None)); +} +#[test] +fn prefer_builder_proposals_flag() { + CommandLineTest::new() + .flag("prefer-builder-proposals", None) + .run() + .with_config(|config| assert!(config.prefer_builder_proposals)); +} +#[test] +fn no_prefer_builder_proposals_flag() { + CommandLineTest::new() + .run() + .with_config(|config| assert!(!config.prefer_builder_proposals)); +} +#[test] fn no_builder_registration_timestamp_override_flag() { CommandLineTest::new() .run() diff --git a/lighthouse/tests/validator_manager.rs b/lighthouse/tests/validator_manager.rs index e0a1e92d6..fab1cfebf 100644 --- a/lighthouse/tests/validator_manager.rs +++ b/lighthouse/tests/validator_manager.rs @@ -122,6 +122,8 @@ pub fn validator_create_defaults() { specify_voting_keystore_password: false, eth1_withdrawal_address: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, fee_recipient: None, gas_limit: None, bn_url: None, @@ -143,6 +145,8 @@ pub fn validator_create_misc_flags() { .flag("--specify-voting-keystore-password", None) .flag("--eth1-withdrawal-address", Some(EXAMPLE_ETH1_ADDRESS)) .flag("--builder-proposals", Some("true")) + .flag("--prefer-builder-proposals", Some("true")) + .flag("--builder-boost-factor", Some("150")) .flag("--suggested-fee-recipient", Some(EXAMPLE_ETH1_ADDRESS)) .flag("--gas-limit", Some("1337")) .flag("--beacon-node", Some("http://localhost:1001")) @@ -159,6 +163,8 @@ pub fn validator_create_misc_flags() { specify_voting_keystore_password: true, eth1_withdrawal_address: Some(Address::from_str(EXAMPLE_ETH1_ADDRESS).unwrap()), builder_proposals: Some(true), + builder_boost_factor: Some(150), + prefer_builder_proposals: Some(true), fee_recipient: Some(Address::from_str(EXAMPLE_ETH1_ADDRESS).unwrap()), gas_limit: Some(1337), bn_url: Some(SensitiveUrl::parse("http://localhost:1001").unwrap()), @@ -244,6 +250,8 @@ pub fn validator_move_defaults() { dest_vc_token_path: PathBuf::from("./2.json"), validators: Validators::All, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, fee_recipient: None, gas_limit: None, password_source: PasswordSource::Interactive { @@ -280,6 +288,8 @@ pub fn validator_move_misc_flags_0() { PublicKeyBytes::from_str(EXAMPLE_PUBKEY_1).unwrap(), ]), builder_proposals: Some(true), + builder_boost_factor: None, + prefer_builder_proposals: None, fee_recipient: Some(Address::from_str(EXAMPLE_ETH1_ADDRESS).unwrap()), gas_limit: Some(1337), password_source: PasswordSource::Interactive { stdin_inputs: true }, @@ -297,6 +307,7 @@ pub fn validator_move_misc_flags_1() { .flag("--dest-vc-token", Some("./2.json")) .flag("--validators", Some(&format!("{}", EXAMPLE_PUBKEY_0))) .flag("--builder-proposals", Some("false")) + .flag("--prefer-builder-proposals", Some("false")) .assert_success(|config| { let expected = MoveConfig { src_vc_url: SensitiveUrl::parse("http://localhost:1").unwrap(), @@ -307,6 +318,40 @@ pub fn validator_move_misc_flags_1() { PublicKeyBytes::from_str(EXAMPLE_PUBKEY_0).unwrap() ]), builder_proposals: Some(false), + builder_boost_factor: None, + prefer_builder_proposals: Some(false), + fee_recipient: None, + gas_limit: None, + password_source: PasswordSource::Interactive { + stdin_inputs: cfg!(windows) || false, + }, + }; + assert_eq!(expected, config); + }); +} + +#[test] +pub fn validator_move_misc_flags_2() { + CommandLineTest::validators_move() + .flag("--src-vc-url", Some("http://localhost:1")) + .flag("--src-vc-token", Some("./1.json")) + .flag("--dest-vc-url", Some("http://localhost:2")) + .flag("--dest-vc-token", Some("./2.json")) + .flag("--validators", Some(&format!("{}", EXAMPLE_PUBKEY_0))) + .flag("--builder-proposals", Some("false")) + .flag("--builder-boost-factor", Some("100")) + .assert_success(|config| { + let expected = MoveConfig { + src_vc_url: SensitiveUrl::parse("http://localhost:1").unwrap(), + src_vc_token_path: PathBuf::from("./1.json"), + dest_vc_url: SensitiveUrl::parse("http://localhost:2").unwrap(), + dest_vc_token_path: PathBuf::from("./2.json"), + validators: Validators::Specific(vec![ + PublicKeyBytes::from_str(EXAMPLE_PUBKEY_0).unwrap() + ]), + builder_proposals: Some(false), + builder_boost_factor: Some(100), + prefer_builder_proposals: None, fee_recipient: None, gas_limit: None, password_source: PasswordSource::Interactive { @@ -333,6 +378,8 @@ pub fn validator_move_count() { dest_vc_token_path: PathBuf::from("./2.json"), validators: Validators::Count(42), builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, fee_recipient: None, gas_limit: None, password_source: PasswordSource::Interactive { diff --git a/testing/web3signer_tests/src/lib.rs b/testing/web3signer_tests/src/lib.rs index 463de0c8b..6f3536fe4 100644 --- a/testing/web3signer_tests/src/lib.rs +++ b/testing/web3signer_tests/src/lib.rs @@ -391,6 +391,8 @@ mod tests { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, description: String::default(), signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path: signer_rig.keystore_path.clone(), @@ -409,6 +411,8 @@ mod tests { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, description: String::default(), signing_definition: SigningDefinition::Web3Signer(Web3SignerDefinition { url: signer_rig.url.to_string(), diff --git a/validator_client/src/block_service.rs b/validator_client/src/block_service.rs index e0c98660e..445d4f1a5 100644 --- a/validator_client/src/block_service.rs +++ b/validator_client/src/block_service.rs @@ -325,14 +325,7 @@ impl BlockService { if self.validator_store.produce_block_v3() { for validator_pubkey in proposers { - let builder_proposals = self - .validator_store - .get_builder_proposals(&validator_pubkey); - // Translate `builder_proposals` to a boost factor. Builder proposals set to `true` - // requires no boost factor, it just means "use a builder proposal if the BN returns - // one". On the contrary, `builder_proposals: false` indicates a preference for - // local payloads, so we set the builder boost factor to 0. - let builder_boost_factor = if !builder_proposals { Some(0) } else { None }; + let builder_boost_factor = self.get_builder_boost_factor(&validator_pubkey); let service = self.clone(); let log = log.clone(); self.inner.context.executor.spawn( @@ -853,6 +846,36 @@ impl BlockService { Ok::<_, BlockError>(unsigned_block) } + + /// Returns the builder boost factor of the given public key. + /// The priority order for fetching this value is: + /// + /// 1. validator_definitions.yml + /// 2. process level flag + fn get_builder_boost_factor(&self, validator_pubkey: &PublicKeyBytes) -> Option { + // Apply per validator configuration first. + let validator_builder_boost_factor = self + .validator_store + .determine_validator_builder_boost_factor(validator_pubkey); + + // Fallback to process-wide configuration if needed. + let maybe_builder_boost_factor = validator_builder_boost_factor.or_else(|| { + self.validator_store + .determine_default_builder_boost_factor() + }); + + if let Some(builder_boost_factor) = maybe_builder_boost_factor { + // if builder boost factor is set to 100 it should be treated + // as None to prevent unnecessary calculations that could + // lead to loss of information. + if builder_boost_factor == 100 { + return None; + } + return Some(builder_boost_factor); + } + + None + } } pub enum UnsignedBlock { diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index cd3ad494d..e6d19bc89 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -349,4 +349,22 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .default_value("500") .takes_value(true), ) + .arg( + Arg::with_name("builder-boost-factor") + .long("builder-boost-factor") + .value_name("UINT64") + .help("Defines the boost factor, \ + a percentage multiplier to apply to the builder's payload value \ + when choosing between a builder payload header and payload from \ + the local execution node.") + .conflicts_with("prefer-builder-proposals") + .takes_value(true), + ) + .arg( + Arg::with_name("prefer-builder-proposals") + .long("prefer-builder-proposals") + .help("If this flag is set, Lighthouse will always prefer blocks \ + constructed by builders, regardless of payload value.") + .takes_value(false), + ) } diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index 4b7da7642..a919afb01 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -77,6 +77,10 @@ pub struct Config { pub validator_registration_batch_size: usize, /// Enables block production via the block v3 endpoint. This configuration option can be removed post deneb. pub produce_block_v3: bool, + /// Specifies the boost factor, a percentage multiplier to apply to the builder's payload value. + pub builder_boost_factor: Option, + /// If true, Lighthouse will prefer builder proposals, if available. + pub prefer_builder_proposals: bool, } impl Default for Config { @@ -118,6 +122,8 @@ impl Default for Config { enable_latency_measurement_service: true, validator_registration_batch_size: 500, produce_block_v3: false, + builder_boost_factor: None, + prefer_builder_proposals: false, } } } @@ -346,6 +352,10 @@ impl Config { config.produce_block_v3 = true; } + if cli_args.is_present("prefer-builder-proposals") { + config.prefer_builder_proposals = true; + } + config.gas_limit = cli_args .value_of("gas-limit") .map(|gas_limit| { @@ -365,6 +375,8 @@ impl Config { ); } + config.builder_boost_factor = parse_optional(cli_args, "builder-boost-factor")?; + config.enable_latency_measurement_service = parse_optional(cli_args, "latency-measurement-service")?.unwrap_or(true); diff --git a/validator_client/src/http_api/create_validator.rs b/validator_client/src/http_api/create_validator.rs index 52336afa5..afa5d4fed 100644 --- a/validator_client/src/http_api/create_validator.rs +++ b/validator_client/src/http_api/create_validator.rs @@ -148,6 +148,8 @@ pub async fn create_validators_mnemonic, T: 'static + SlotClock, request.suggested_fee_recipient, request.gas_limit, request.builder_proposals, + request.builder_boost_factor, + request.prefer_builder_proposals, ) .await .map_err(|e| { diff --git a/validator_client/src/http_api/keystores.rs b/validator_client/src/http_api/keystores.rs index c2d9b4d67..074c57834 100644 --- a/validator_client/src/http_api/keystores.rs +++ b/validator_client/src/http_api/keystores.rs @@ -224,6 +224,8 @@ fn import_single_keystore( None, None, None, + None, + None, )) .map_err(|e| format!("failed to initialize validator: {:?}", e))?; diff --git a/validator_client/src/http_api/mod.rs b/validator_client/src/http_api/mod.rs index c65beb739..dcf66d2fb 100644 --- a/validator_client/src/http_api/mod.rs +++ b/validator_client/src/http_api/mod.rs @@ -565,6 +565,8 @@ pub fn serve( let suggested_fee_recipient = body.suggested_fee_recipient; let gas_limit = body.gas_limit; let builder_proposals = body.builder_proposals; + let builder_boost_factor = body.builder_boost_factor; + let prefer_builder_proposals = body.prefer_builder_proposals; let validator_def = { if let Some(handle) = task_executor.handle() { @@ -577,6 +579,8 @@ pub fn serve( suggested_fee_recipient, gas_limit, builder_proposals, + builder_boost_factor, + prefer_builder_proposals, )) .map_err(|e| { warp_utils::reject::custom_server_error(format!( @@ -625,6 +629,8 @@ pub fn serve( suggested_fee_recipient: web3signer.suggested_fee_recipient, gas_limit: web3signer.gas_limit, builder_proposals: web3signer.builder_proposals, + builder_boost_factor: web3signer.builder_boost_factor, + prefer_builder_proposals: web3signer.prefer_builder_proposals, description: web3signer.description, signing_definition: SigningDefinition::Web3Signer( Web3SignerDefinition { @@ -691,8 +697,12 @@ pub fn serve( (Some(is_enabled), Some(initialized_validator)) if Some(is_enabled) == body.enabled && initialized_validator.get_gas_limit() == body.gas_limit + && initialized_validator.get_builder_boost_factor() + == body.builder_boost_factor && initialized_validator.get_builder_proposals() == body.builder_proposals + && initialized_validator.get_prefer_builder_proposals() + == body.prefer_builder_proposals && initialized_validator.get_graffiti() == maybe_graffiti => { Ok(()) @@ -706,6 +716,8 @@ pub fn serve( body.enabled, body.gas_limit, body.builder_proposals, + body.builder_boost_factor, + body.prefer_builder_proposals, body.graffiti, ), ) diff --git a/validator_client/src/http_api/remotekeys.rs b/validator_client/src/http_api/remotekeys.rs index 991dfb8bf..053bbcb4b 100644 --- a/validator_client/src/http_api/remotekeys.rs +++ b/validator_client/src/http_api/remotekeys.rs @@ -125,6 +125,8 @@ fn import_single_remotekey( suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, description: String::from("Added by remotekey API"), signing_definition: SigningDefinition::Web3Signer(Web3SignerDefinition { url, diff --git a/validator_client/src/http_api/test_utils.rs b/validator_client/src/http_api/test_utils.rs index 916c098cd..7b0cb51ec 100644 --- a/validator_client/src/http_api/test_utils.rs +++ b/validator_client/src/http_api/test_utils.rs @@ -315,6 +315,8 @@ impl ApiTester { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, deposit_gwei: E::default_spec().max_effective_balance, }) .collect::>(); @@ -447,6 +449,8 @@ impl ApiTester { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, }; self.client @@ -467,6 +471,8 @@ impl ApiTester { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, }; let response = self @@ -511,6 +517,8 @@ impl ApiTester { request_timeout_ms: None, client_identity_path: None, client_identity_password: None, + builder_boost_factor: None, + prefer_builder_proposals: None, } }) .collect(); @@ -534,7 +542,15 @@ impl ApiTester { let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; self.client - .patch_lighthouse_validators(&validator.voting_pubkey, Some(enabled), None, None, None) + .patch_lighthouse_validators( + &validator.voting_pubkey, + Some(enabled), + None, + None, + None, + None, + None, + ) .await .unwrap(); @@ -582,6 +598,8 @@ impl ApiTester { Some(gas_limit), None, None, + None, + None, ) .await .unwrap(); @@ -610,6 +628,8 @@ impl ApiTester { None, Some(builder_proposals), None, + None, + None, ) .await .unwrap(); diff --git a/validator_client/src/http_api/tests.rs b/validator_client/src/http_api/tests.rs index 7de3cea21..f7db76e4a 100644 --- a/validator_client/src/http_api/tests.rs +++ b/validator_client/src/http_api/tests.rs @@ -52,6 +52,12 @@ struct ApiTester { impl ApiTester { pub async fn new() -> Self { + let mut config = Config::default(); + config.fee_recipient = Some(TEST_DEFAULT_FEE_RECIPIENT); + Self::new_with_config(config).await + } + + pub async fn new_with_config(mut config: Config) -> Self { let log = test_logger(); let validator_dir = tempdir().unwrap(); @@ -70,10 +76,8 @@ impl ApiTester { let api_secret = ApiSecret::create_or_open(validator_dir.path()).unwrap(); let api_pubkey = api_secret.api_token(); - let mut config = Config::default(); config.validator_dir = validator_dir.path().into(); config.secrets_dir = secrets_dir.path().into(); - config.fee_recipient = Some(TEST_DEFAULT_FEE_RECIPIENT); let spec = E::default_spec(); @@ -271,6 +275,8 @@ impl ApiTester { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, deposit_gwei: E::default_spec().max_effective_balance, }) .collect::>(); @@ -404,6 +410,8 @@ impl ApiTester { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, }; self.client @@ -424,6 +432,8 @@ impl ApiTester { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, }; let response = self @@ -462,6 +472,8 @@ impl ApiTester { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, voting_public_key: kp.pk, url: format!("http://signer_{}.com/", i), root_certificate_path: None, @@ -518,7 +530,15 @@ impl ApiTester { let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; self.client - .patch_lighthouse_validators(&validator.voting_pubkey, Some(enabled), None, None, None) + .patch_lighthouse_validators( + &validator.voting_pubkey, + Some(enabled), + None, + None, + None, + None, + None, + ) .await .unwrap(); @@ -566,6 +586,8 @@ impl ApiTester { Some(gas_limit), None, None, + None, + None, ) .await .unwrap(); @@ -594,6 +616,50 @@ impl ApiTester { None, Some(builder_proposals), None, + None, + None, + ) + .await + .unwrap(); + + self + } + + pub async fn set_builder_boost_factor(self, index: usize, builder_boost_factor: u64) -> Self { + let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; + + self.client + .patch_lighthouse_validators( + &validator.voting_pubkey, + None, + None, + None, + Some(builder_boost_factor), + None, + None, + ) + .await + .unwrap(); + + self + } + + pub async fn set_prefer_builder_proposals( + self, + index: usize, + prefer_builder_proposals: bool, + ) -> Self { + let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; + + self.client + .patch_lighthouse_validators( + &validator.voting_pubkey, + None, + None, + None, + None, + Some(prefer_builder_proposals), + None, ) .await .unwrap(); @@ -613,6 +679,64 @@ impl ApiTester { self } + pub async fn assert_builder_boost_factor( + self, + index: usize, + builder_boost_factor: Option, + ) -> Self { + let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; + + assert_eq!( + self.validator_store + .get_builder_boost_factor(&validator.voting_pubkey), + builder_boost_factor + ); + + self + } + + pub async fn assert_validator_derived_builder_boost_factor( + self, + index: usize, + builder_boost_factor: Option, + ) -> Self { + let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; + + assert_eq!( + self.validator_store + .determine_validator_builder_boost_factor(&validator.voting_pubkey), + builder_boost_factor + ); + + self + } + + pub fn assert_default_builder_boost_factor(self, builder_boost_factor: Option) -> Self { + assert_eq!( + self.validator_store + .determine_default_builder_boost_factor(), + builder_boost_factor + ); + + self + } + + pub async fn assert_prefer_builder_proposals( + self, + index: usize, + prefer_builder_proposals: bool, + ) -> Self { + let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; + + assert_eq!( + self.validator_store + .get_prefer_builder_proposals(&validator.voting_pubkey), + prefer_builder_proposals + ); + + self + } + pub async fn set_graffiti(self, index: usize, graffiti: &str) -> Self { let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; let graffiti_str = GraffitiString::from_str(graffiti).unwrap(); @@ -622,6 +746,8 @@ impl ApiTester { None, None, None, + None, + None, Some(graffiti_str), ) .await @@ -741,6 +867,8 @@ async fn routes_with_invalid_auth() { gas_limit: <_>::default(), builder_proposals: <_>::default(), deposit_gwei: <_>::default(), + builder_boost_factor: <_>::default(), + prefer_builder_proposals: <_>::default(), }]) .await }) @@ -771,6 +899,8 @@ async fn routes_with_invalid_auth() { suggested_fee_recipient: <_>::default(), gas_limit: <_>::default(), builder_proposals: <_>::default(), + builder_boost_factor: <_>::default(), + prefer_builder_proposals: <_>::default(), }) .await }) @@ -783,6 +913,8 @@ async fn routes_with_invalid_auth() { None, None, None, + None, + None, ) .await }) @@ -980,6 +1112,100 @@ async fn validator_builder_proposals() { .await; } +#[tokio::test] +async fn validator_builder_boost_factor() { + ApiTester::new() + .await + .create_hd_validators(HdValidatorScenario { + count: 2, + specify_mnemonic: false, + key_derivation_path_offset: 0, + disabled: vec![], + }) + .await + .assert_enabled_validators_count(2) + .assert_validators_count(2) + .set_builder_boost_factor(0, 120) + .await + // Test setting builder proposals while the validator is disabled + .set_validator_enabled(0, false) + .await + .assert_enabled_validators_count(1) + .assert_validators_count(2) + .set_builder_boost_factor(0, 80) + .await + .set_validator_enabled(0, true) + .await + .assert_enabled_validators_count(2) + .assert_builder_boost_factor(0, Some(80)) + .await; +} + +/// Verifies the builder boost factors translated from the `builder_proposals`, +/// `prefer_builder_proposals` and `builder_boost_factor` values. +#[tokio::test] +async fn validator_derived_builder_boost_factor_with_process_defaults() { + let config = Config { + builder_proposals: true, + prefer_builder_proposals: false, + builder_boost_factor: Some(80), + ..Config::default() + }; + ApiTester::new_with_config(config) + .await + .create_hd_validators(HdValidatorScenario { + count: 3, + specify_mnemonic: false, + key_derivation_path_offset: 0, + disabled: vec![], + }) + .await + .assert_default_builder_boost_factor(Some(80)) + .assert_validator_derived_builder_boost_factor(0, None) + .await + .set_builder_proposals(0, false) + .await + .assert_validator_derived_builder_boost_factor(0, Some(0)) + .await + .set_builder_boost_factor(1, 120) + .await + .assert_validator_derived_builder_boost_factor(1, Some(120)) + .await + .set_prefer_builder_proposals(2, true) + .await + .assert_validator_derived_builder_boost_factor(2, Some(u64::MAX)) + .await; +} + +#[tokio::test] +async fn prefer_builder_proposals_validator() { + ApiTester::new() + .await + .create_hd_validators(HdValidatorScenario { + count: 2, + specify_mnemonic: false, + key_derivation_path_offset: 0, + disabled: vec![], + }) + .await + .assert_enabled_validators_count(2) + .assert_validators_count(2) + .set_prefer_builder_proposals(0, false) + .await + // Test setting builder proposals while the validator is disabled + .set_validator_enabled(0, false) + .await + .assert_enabled_validators_count(1) + .assert_validators_count(2) + .set_prefer_builder_proposals(0, true) + .await + .set_validator_enabled(0, true) + .await + .assert_enabled_validators_count(2) + .assert_prefer_builder_proposals(0, true) + .await; +} + #[tokio::test] async fn validator_graffiti() { ApiTester::new() diff --git a/validator_client/src/http_api/tests/keystores.rs b/validator_client/src/http_api/tests/keystores.rs index f301af1c2..fe58393bb 100644 --- a/validator_client/src/http_api/tests/keystores.rs +++ b/validator_client/src/http_api/tests/keystores.rs @@ -43,6 +43,8 @@ fn web3signer_validator_with_pubkey(pubkey: PublicKey) -> Web3SignerValidatorReq suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, voting_public_key: pubkey, url: web3_signer_url(), root_certificate_path: None, @@ -468,7 +470,7 @@ async fn import_and_delete_conflicting_web3_signer_keystores() { for pubkey in &pubkeys { tester .client - .patch_lighthouse_validators(pubkey, Some(false), None, None, None) + .patch_lighthouse_validators(pubkey, Some(false), None, None, None, None, None) .await .unwrap(); } diff --git a/validator_client/src/initialized_validators.rs b/validator_client/src/initialized_validators.rs index b65dad4c4..7e4331dc8 100644 --- a/validator_client/src/initialized_validators.rs +++ b/validator_client/src/initialized_validators.rs @@ -131,6 +131,8 @@ pub struct InitializedValidator { suggested_fee_recipient: Option
, gas_limit: Option, builder_proposals: Option, + builder_boost_factor: Option, + prefer_builder_proposals: Option, /// The validators index in `state.validators`, to be updated by an external service. index: Option, } @@ -159,6 +161,14 @@ impl InitializedValidator { self.gas_limit } + pub fn get_builder_boost_factor(&self) -> Option { + self.builder_boost_factor + } + + pub fn get_prefer_builder_proposals(&self) -> Option { + self.prefer_builder_proposals + } + pub fn get_builder_proposals(&self) -> Option { self.builder_proposals } @@ -335,6 +345,8 @@ impl InitializedValidator { suggested_fee_recipient: def.suggested_fee_recipient, gas_limit: def.gas_limit, builder_proposals: def.builder_proposals, + builder_boost_factor: def.builder_boost_factor, + prefer_builder_proposals: def.prefer_builder_proposals, index: None, }) } @@ -815,6 +827,22 @@ impl InitializedValidators { .and_then(|v| v.builder_proposals) } + /// Returns the `builder_boost_factor` for a given public key specified in the + /// `ValidatorDefinitions`. + pub fn builder_boost_factor(&self, public_key: &PublicKeyBytes) -> Option { + self.validators + .get(public_key) + .and_then(|v| v.builder_boost_factor) + } + + /// Returns the `prefer_builder_proposals` for a given public key specified in the + /// `ValidatorDefinitions`. + pub fn prefer_builder_proposals(&self, public_key: &PublicKeyBytes) -> Option { + self.validators + .get(public_key) + .and_then(|v| v.prefer_builder_proposals) + } + /// Returns an `Option` of a reference to an `InitializedValidator` for a given public key specified in the /// `ValidatorDefinitions`. pub fn validator(&self, public_key: &PublicKeyBytes) -> Option<&InitializedValidator> { @@ -835,12 +863,15 @@ impl InitializedValidators { /// or `InitializedValidator`. The same logic applies to `builder_proposals` and `graffiti`. /// /// Saves the `ValidatorDefinitions` to file, even if no definitions were changed. + #[allow(clippy::too_many_arguments)] pub async fn set_validator_definition_fields( &mut self, voting_public_key: &PublicKey, enabled: Option, gas_limit: Option, builder_proposals: Option, + builder_boost_factor: Option, + prefer_builder_proposals: Option, graffiti: Option, ) -> Result<(), Error> { if let Some(def) = self @@ -862,6 +893,12 @@ impl InitializedValidators { if let Some(graffiti) = graffiti.clone() { def.graffiti = Some(graffiti); } + if let Some(builder_boost_factor) = builder_boost_factor { + def.builder_boost_factor = Some(builder_boost_factor); + } + if let Some(prefer_builder_proposals) = prefer_builder_proposals { + def.prefer_builder_proposals = Some(prefer_builder_proposals); + } } self.update_validators().await?; @@ -880,6 +917,12 @@ impl InitializedValidators { if let Some(graffiti) = graffiti { val.graffiti = Some(graffiti.into()); } + if let Some(builder_boost_factor) = builder_boost_factor { + val.builder_boost_factor = Some(builder_boost_factor); + } + if let Some(prefer_builder_proposals) = prefer_builder_proposals { + val.prefer_builder_proposals = Some(prefer_builder_proposals); + } } self.definitions diff --git a/validator_client/src/validator_store.rs b/validator_client/src/validator_store.rs index 19726c2ae..c913b9906 100644 --- a/validator_client/src/validator_store.rs +++ b/validator_client/src/validator_store.rs @@ -98,6 +98,8 @@ pub struct ValidatorStore { gas_limit: Option, builder_proposals: bool, produce_block_v3: bool, + prefer_builder_proposals: bool, + builder_boost_factor: Option, task_executor: TaskExecutor, _phantom: PhantomData, } @@ -130,6 +132,8 @@ impl ValidatorStore { gas_limit: config.gas_limit, builder_proposals: config.builder_proposals, produce_block_v3: config.produce_block_v3, + prefer_builder_proposals: config.prefer_builder_proposals, + builder_boost_factor: config.builder_boost_factor, task_executor, _phantom: PhantomData, } @@ -178,6 +182,8 @@ impl ValidatorStore { suggested_fee_recipient: Option
, gas_limit: Option, builder_proposals: Option, + builder_boost_factor: Option, + prefer_builder_proposals: Option, ) -> Result { let mut validator_def = ValidatorDefinition::new_keystore_with_password( voting_keystore_path, @@ -186,6 +192,8 @@ impl ValidatorStore { suggested_fee_recipient, gas_limit, builder_proposals, + builder_boost_factor, + prefer_builder_proposals, ) .map_err(|e| format!("failed to create validator definitions: {:?}", e))?; @@ -474,7 +482,7 @@ impl ValidatorStore { .unwrap_or(DEFAULT_GAS_LIMIT) } - /// Returns a `bool` for the given public key that denotes whther this validator should use the + /// Returns a `bool` for the given public key that denotes whether this validator should use the /// builder API. The priority order for fetching this value is: /// /// 1. validator_definitions.yml @@ -487,12 +495,91 @@ impl ValidatorStore { ) } + /// Returns a `u64` for the given public key that denotes the builder boost factor. The priority order for fetching this value is: + /// + /// 1. validator_definitions.yml + /// 2. process level flag + pub fn get_builder_boost_factor(&self, validator_pubkey: &PublicKeyBytes) -> Option { + self.validators + .read() + .builder_boost_factor(validator_pubkey) + .or(self.builder_boost_factor) + } + + /// Returns a `bool` for the given public key that denotes whether this validator should prefer a + /// builder payload. The priority order for fetching this value is: + /// + /// 1. validator_definitions.yml + /// 2. process level flag + pub fn get_prefer_builder_proposals(&self, validator_pubkey: &PublicKeyBytes) -> bool { + self.validators + .read() + .prefer_builder_proposals(validator_pubkey) + .unwrap_or(self.prefer_builder_proposals) + } + fn get_builder_proposals_defaulting(&self, builder_proposals: Option) -> bool { builder_proposals // If there's nothing in the file, try the process-level default value. .unwrap_or(self.builder_proposals) } + /// Translate the per validator `builder_proposals`, `builder_boost_factor` and + /// `prefer_builder_proposals` to a boost factor, if available. + /// - If `prefer_builder_proposals` is true, set boost factor to `u64::MAX` to indicate a + /// preference for builder payloads. + /// - If `builder_boost_factor` is a value other than None, return its value as the boost factor. + /// - If `builder_proposals` is set to false, set boost factor to 0 to indicate a preference for + /// local payloads. + /// - Else return `None` to indicate no preference between builder and local payloads. + pub fn determine_validator_builder_boost_factor( + &self, + validator_pubkey: &PublicKeyBytes, + ) -> Option { + let validator_prefer_builder_proposals = self + .validators + .read() + .prefer_builder_proposals(validator_pubkey); + + if matches!(validator_prefer_builder_proposals, Some(true)) { + return Some(u64::MAX); + } + + self.validators + .read() + .builder_boost_factor(validator_pubkey) + .or_else(|| { + if matches!( + self.validators.read().builder_proposals(validator_pubkey), + Some(false) + ) { + return Some(0); + } + None + }) + } + + /// Translate the process-wide `builder_proposals`, `builder_boost_factor` and + /// `prefer_builder_proposals` configurations to a boost factor. + /// - If `prefer_builder_proposals` is true, set boost factor to `u64::MAX` to indicate a + /// preference for builder payloads. + /// - If `builder_boost_factor` is a value other than None, return its value as the boost factor. + /// - If `builder_proposals` is set to false, set boost factor to 0 to indicate a preference for + /// local payloads. + /// - Else return `None` to indicate no preference between builder and local payloads. + pub fn determine_default_builder_boost_factor(&self) -> Option { + if self.prefer_builder_proposals { + return Some(u64::MAX); + } + self.builder_boost_factor.or({ + if self.builder_proposals { + Some(0) + } else { + None + } + }) + } + pub async fn sign_block>( &self, validator_pubkey: PublicKeyBytes, diff --git a/validator_manager/src/common.rs b/validator_manager/src/common.rs index 6a3f93a3f..871c53620 100644 --- a/validator_manager/src/common.rs +++ b/validator_manager/src/common.rs @@ -46,6 +46,8 @@ pub struct ValidatorSpecification { pub fee_recipient: Option
, pub gas_limit: Option, pub builder_proposals: Option, + pub builder_boost_factor: Option, + pub prefer_builder_proposals: Option, pub enabled: Option, } @@ -64,6 +66,8 @@ impl ValidatorSpecification { gas_limit, builder_proposals, enabled, + builder_boost_factor, + prefer_builder_proposals, } = self; let voting_public_key = voting_keystore @@ -136,6 +140,8 @@ impl ValidatorSpecification { enabled, gas_limit, builder_proposals, + builder_boost_factor, + prefer_builder_proposals, None, // Grafitti field is not maintained between validator moves. ) .await diff --git a/validator_manager/src/create_validators.rs b/validator_manager/src/create_validators.rs index 8ea740ff5..8ab3303d3 100644 --- a/validator_manager/src/create_validators.rs +++ b/validator_manager/src/create_validators.rs @@ -25,6 +25,8 @@ pub const ETH1_WITHDRAWAL_ADDRESS_FLAG: &str = "eth1-withdrawal-address"; pub const GAS_LIMIT_FLAG: &str = "gas-limit"; pub const FEE_RECIPIENT_FLAG: &str = "suggested-fee-recipient"; pub const BUILDER_PROPOSALS_FLAG: &str = "builder-proposals"; +pub const BUILDER_BOOST_FACTOR_FLAG: &str = "builder-boost-factor"; +pub const PREFER_BUILDER_PROPOSALS_FLAG: &str = "prefer-builder-proposals"; pub const BEACON_NODE_FLAG: &str = "beacon-node"; pub const FORCE_BLS_WITHDRAWAL_CREDENTIALS: &str = "force-bls-withdrawal-credentials"; @@ -183,6 +185,30 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { address. This is not recommended.", ), ) + .arg( + Arg::with_name(BUILDER_BOOST_FACTOR_FLAG) + .long(BUILDER_BOOST_FACTOR_FLAG) + .takes_value(true) + .value_name("UINT64") + .required(false) + .help( + "Defines the boost factor, \ + a percentage multiplier to apply to the builder's payload value \ + when choosing between a builder payload header and payload from \ + the local execution node.", + ), + ) + .arg( + Arg::with_name(PREFER_BUILDER_PROPOSALS_FLAG) + .long(PREFER_BUILDER_PROPOSALS_FLAG) + .help( + "If this flag is set, Lighthouse will always prefer blocks \ + constructed by builders, regardless of payload value.", + ) + .required(false) + .possible_values(&["true", "false"]) + .takes_value(true), + ) } /// The CLI arguments are parsed into this struct before running the application. This step of @@ -199,6 +225,8 @@ pub struct CreateConfig { pub specify_voting_keystore_password: bool, pub eth1_withdrawal_address: Option
, pub builder_proposals: Option, + pub builder_boost_factor: Option, + pub prefer_builder_proposals: Option, pub fee_recipient: Option
, pub gas_limit: Option, pub bn_url: Option, @@ -223,6 +251,11 @@ impl CreateConfig { ETH1_WITHDRAWAL_ADDRESS_FLAG, )?, builder_proposals: clap_utils::parse_optional(matches, BUILDER_PROPOSALS_FLAG)?, + builder_boost_factor: clap_utils::parse_optional(matches, BUILDER_BOOST_FACTOR_FLAG)?, + prefer_builder_proposals: clap_utils::parse_optional( + matches, + PREFER_BUILDER_PROPOSALS_FLAG, + )?, fee_recipient: clap_utils::parse_optional(matches, FEE_RECIPIENT_FLAG)?, gas_limit: clap_utils::parse_optional(matches, GAS_LIMIT_FLAG)?, bn_url: clap_utils::parse_optional(matches, BEACON_NODE_FLAG)?, @@ -254,6 +287,8 @@ impl ValidatorsAndDeposits { gas_limit, bn_url, force_bls_withdrawal_credentials, + builder_boost_factor, + prefer_builder_proposals, } = config; // Since Capella, it really doesn't make much sense to use BLS @@ -456,6 +491,8 @@ impl ValidatorsAndDeposits { fee_recipient, gas_limit, builder_proposals, + builder_boost_factor, + prefer_builder_proposals, // Allow the VC to choose a default "enabled" state. Since "enabled" is not part of // the standard API, leaving this as `None` means we are not forced to use the // non-standard API. @@ -585,6 +622,8 @@ pub mod tests { specify_voting_keystore_password: false, eth1_withdrawal_address: junk_execution_address(), builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, fee_recipient: None, gas_limit: None, bn_url: None, diff --git a/validator_manager/src/move_validators.rs b/validator_manager/src/move_validators.rs index fa886e8f9..5826f2756 100644 --- a/validator_manager/src/move_validators.rs +++ b/validator_manager/src/move_validators.rs @@ -32,6 +32,8 @@ pub const VALIDATORS_FLAG: &str = "validators"; pub const GAS_LIMIT_FLAG: &str = "gas-limit"; pub const FEE_RECIPIENT_FLAG: &str = "suggested-fee-recipient"; pub const BUILDER_PROPOSALS_FLAG: &str = "builder-proposals"; +pub const BUILDER_BOOST_FACTOR_FLAG: &str = "builder-boost-factor"; +pub const PREFER_BUILDER_PROPOSALS_FLAG: &str = "prefer-builder-proposals"; const NO_VALIDATORS_MSG: &str = "No validators present on source validator client"; @@ -170,6 +172,30 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .long(STDIN_INPUTS_FLAG) .help("If present, read all user inputs from stdin instead of tty."), ) + .arg( + Arg::with_name(BUILDER_BOOST_FACTOR_FLAG) + .long(BUILDER_BOOST_FACTOR_FLAG) + .takes_value(true) + .value_name("UINT64") + .required(false) + .help( + "Defines the boost factor, \ + a percentage multiplier to apply to the builder's payload value \ + when choosing between a builder payload header and payload from \ + the local execution node.", + ), + ) + .arg( + Arg::with_name(PREFER_BUILDER_PROPOSALS_FLAG) + .long(PREFER_BUILDER_PROPOSALS_FLAG) + .help( + "If this flag is set, Lighthouse will always prefer blocks \ + constructed by builders, regardless of payload value.", + ) + .required(false) + .possible_values(&["true", "false"]) + .takes_value(true), + ) } #[derive(Clone, PartialEq, Debug, Serialize, Deserialize)] @@ -187,6 +213,8 @@ pub struct MoveConfig { pub dest_vc_token_path: PathBuf, pub validators: Validators, pub builder_proposals: Option, + pub builder_boost_factor: Option, + pub prefer_builder_proposals: Option, pub fee_recipient: Option
, pub gas_limit: Option, pub password_source: PasswordSource, @@ -221,6 +249,11 @@ impl MoveConfig { dest_vc_token_path: clap_utils::parse_required(matches, DEST_VC_TOKEN_FLAG)?, validators, builder_proposals: clap_utils::parse_optional(matches, BUILDER_PROPOSALS_FLAG)?, + builder_boost_factor: clap_utils::parse_optional(matches, BUILDER_BOOST_FACTOR_FLAG)?, + prefer_builder_proposals: clap_utils::parse_optional( + matches, + PREFER_BUILDER_PROPOSALS_FLAG, + )?, fee_recipient: clap_utils::parse_optional(matches, FEE_RECIPIENT_FLAG)?, gas_limit: clap_utils::parse_optional(matches, GAS_LIMIT_FLAG)?, password_source: PasswordSource::Interactive { @@ -253,6 +286,8 @@ async fn run<'a>(config: MoveConfig) -> Result<(), String> { fee_recipient, gas_limit, mut password_source, + builder_boost_factor, + prefer_builder_proposals, } = config; // Moving validators between the same VC is unlikely to be useful and probably indicates a user @@ -488,13 +523,15 @@ async fn run<'a>(config: MoveConfig) -> Result<(), String> { let keystore_derivation_path = voting_keystore.0.path(); - let validator_specification = ValidatorSpecification { + let validator_specification: ValidatorSpecification = ValidatorSpecification { voting_keystore, voting_keystore_password, slashing_protection: Some(InterchangeJsonStr(slashing_protection)), fee_recipient, gas_limit, builder_proposals, + builder_boost_factor, + prefer_builder_proposals, // Allow the VC to choose a default "enabled" state. Since "enabled" is not part of // the standard API, leaving this as `None` means we are not forced to use the // non-standard API. @@ -758,6 +795,8 @@ mod test { dest_vc_token_path: dest_vc_token_path.clone(), validators: validators.clone(), builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, fee_recipient: None, gas_limit: None, password_source: PasswordSource::Testing(self.passwords.clone()),