Skip to content

Commit

Permalink
expose builder booster flags in vc, enable options in validator endpo…
Browse files Browse the repository at this point in the history
…ints, update tests
  • Loading branch information
eserilev committed Jan 19, 2024
1 parent 47b28c4 commit 03d68f5
Show file tree
Hide file tree
Showing 24 changed files with 477 additions and 13 deletions.
2 changes: 2 additions & 0 deletions account_manager/src/validator/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))?;

Expand Down
8 changes: 8 additions & 0 deletions book/src/help_vc.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ FLAGS:
--builder-proposals
If this flag is set, Lighthouse will query the Beacon Node for only block headers during proposals and will
sign over headers. Useful for outsourcing execution payload construction during proposals.
--prefer-builder-proposals
If this flag is set, Lighthouse will always prefer blocks constructed by builders, if available.
--disable-auto-discover
If present, do not attempt to discover new validators in the validators-dir. Validators will need to be
manually added to the validator_definitions.yml file.
Expand Down Expand Up @@ -203,4 +205,10 @@ OPTIONS:
--validators-dir <VALIDATORS_DIR>
The directory which contains the validator keystores, deposit data for each validator along with the common
slashing protection database and the validator_definitions.yml
--builder-boost-factor <INTEGER>
Percentage multiplier to apply to the builder's payload value when choosing between a
builder payload header and payload from the paired execution node. This parameter is only
relevant if the beacon node is connected to a builder, deems it safe to produce a builder
payload, and receives valid responses from both the builder endpoint _and_ the paired
execution node.
```
13 changes: 13 additions & 0 deletions common/account_utils/src/validator_definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,12 @@ pub struct ValidatorDefinition {
#[serde(skip_serializing_if = "Option::is_none")]
pub builder_proposals: Option<bool>,
#[serde(default)]
#[serde(skip_serializing_if = "Option::is_none")]
pub builder_boost_factor: Option<u64>,
#[serde(default)]
#[serde(skip_serializing_if = "Option::is_none")]
pub prefer_builder_proposals: Option<bool>,
#[serde(default)]
pub description: String,
#[serde(flatten)]
pub signing_definition: SigningDefinition,
Expand All @@ -169,13 +175,16 @@ impl ValidatorDefinition {
/// ## Notes
///
/// This function does not check the password against the keystore.
#[allow(clippy::too_many_arguments)]
pub fn new_keystore_with_password<P: AsRef<Path>>(
voting_keystore_path: P,
voting_keystore_password_storage: PasswordStorage,
graffiti: Option<GraffitiString>,
suggested_fee_recipient: Option<Address>,
gas_limit: Option<u64>,
builder_proposals: Option<bool>,
builder_boost_factor: Option<u64>,
prefer_builder_proposals: Option<bool>,
) -> Result<Self, Error> {
let voting_keystore_path = voting_keystore_path.as_ref().into();
let keystore =
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions common/eth2/src/lighthouse_vc/http_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool>,
gas_limit: Option<u64>,
builder_proposals: Option<bool>,
builder_boost_factor: Option<u64>,
prefer_builder_proposals: Option<bool>,
graffiti: Option<GraffitiString>,
) -> Result<(), Error> {
let mut path = self.server.full.clone();
Expand All @@ -505,6 +508,8 @@ impl ValidatorClientHttpClient {
enabled,
gas_limit,
builder_proposals,
builder_boost_factor,
prefer_builder_proposals,
graffiti,
},
)
Expand Down
24 changes: 24 additions & 0 deletions common/eth2/src/lighthouse_vc/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ pub struct ValidatorRequest {
#[serde(default)]
#[serde(skip_serializing_if = "Option::is_none")]
pub builder_proposals: Option<bool>,
#[serde(default)]
#[serde(skip_serializing_if = "Option::is_none")]
pub builder_boost_factor: Option<u64>,
#[serde(default)]
#[serde(skip_serializing_if = "Option::is_none")]
pub prefer_builder_proposals: Option<bool>,
#[serde(with = "serde_utils::quoted_u64")]
pub deposit_gwei: u64,
}
Expand Down Expand Up @@ -86,6 +92,12 @@ pub struct ValidatorPatchRequest {
#[serde(default)]
#[serde(skip_serializing_if = "Option::is_none")]
pub graffiti: Option<GraffitiString>,
#[serde(default)]
#[serde(skip_serializing_if = "Option::is_none")]
pub builder_boost_factor: Option<u64>,
#[serde(default)]
#[serde(skip_serializing_if = "Option::is_none")]
pub prefer_builder_proposals: Option<bool>,
}

#[derive(Clone, PartialEq, Serialize, Deserialize)]
Expand All @@ -105,6 +117,12 @@ pub struct KeystoreValidatorsPostRequest {
#[serde(default)]
#[serde(skip_serializing_if = "Option::is_none")]
pub builder_proposals: Option<bool>,
#[serde(default)]
#[serde(skip_serializing_if = "Option::is_none")]
pub builder_boost_factor: Option<u64>,
#[serde(default)]
#[serde(skip_serializing_if = "Option::is_none")]
pub prefer_builder_proposals: Option<bool>,
}

#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
Expand Down Expand Up @@ -135,6 +153,12 @@ pub struct Web3SignerValidatorRequest {
pub client_identity_path: Option<PathBuf>,
#[serde(skip_serializing_if = "Option::is_none")]
pub client_identity_password: Option<String>,
#[serde(default)]
#[serde(skip_serializing_if = "Option::is_none")]
pub builder_boost_factor: Option<u64>,
#[serde(default)]
#[serde(skip_serializing_if = "Option::is_none")]
pub prefer_builder_proposals: Option<bool>,
}

#[derive(Debug, Clone, PartialEq, Deserialize, Serialize)]
Expand Down
8 changes: 8 additions & 0 deletions lighthouse/tests/account_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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),
Expand Down Expand Up @@ -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,
Expand Down
26 changes: 26 additions & 0 deletions lighthouse/tests/validator_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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("100"))
.run()
.with_config(|config| assert_eq!(config.builder_boost_factor, Some(100)));
}
#[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()
Expand Down
15 changes: 15 additions & 0 deletions lighthouse/tests/validator_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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("--builder-boost-factor", Some("100"))
.flag("--prefer-builder-payload", Some("true"))
.flag("--suggested-fee-recipient", Some(EXAMPLE_ETH1_ADDRESS))
.flag("--gas-limit", Some("1337"))
.flag("--beacon-node", Some("http://localhost:1001"))
Expand All @@ -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(100),
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()),
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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: Some(100),
prefer_builder_proposals: Some(true),
fee_recipient: Some(Address::from_str(EXAMPLE_ETH1_ADDRESS).unwrap()),
gas_limit: Some(1337),
password_source: PasswordSource::Interactive { stdin_inputs: true },
Expand All @@ -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(),
Expand All @@ -307,6 +318,8 @@ 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 {
Expand All @@ -333,6 +346,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 {
Expand Down
4 changes: 4 additions & 0 deletions testing/web3signer_tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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(),
Expand Down
46 changes: 38 additions & 8 deletions validator_client/src/block_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,14 +325,7 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockService<T, E> {

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(
Expand Down Expand Up @@ -852,6 +845,43 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockService<T, E> {

Ok::<_, BlockError>(unsigned_block)
}

/// translate `builder_proposals``, `builder_boost_factor`` and `prefer_builder_proposals`` to a
/// boost factor. if `builder_proposals` is set to false, set boost factor to 0 to indicate a
/// preference for local payloads. if builder_boost_factor is a value other than none, return
/// its value as the boost factor. if `prefer_builder_proposals` is true, set boost factor to
/// u64::MAX to indicate a preference for builder payloads. else return None to indicate no preference
/// between builder and local payloads
fn get_builder_boost_factor(&self, validator_pubkey: &PublicKeyBytes) -> Option<u64> {
let builder_proposals = self.validator_store.get_builder_proposals(validator_pubkey);

let builder_boost_factor = self
.validator_store
.get_builder_boost_factor(validator_pubkey);

let prefer_builder_proposals = self
.validator_store
.get_prefer_builder_proposals(validator_pubkey);

if !builder_proposals {
return Some(0);
}

if let Some(builder_boost_factor) = 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 Some(builder_boost_factor);
}
}

if prefer_builder_proposals {
return Some(u64::MAX);
}

None
}
}

pub enum UnsignedBlock<E: EthSpec> {
Expand Down
19 changes: 19 additions & 0 deletions validator_client/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,4 +349,23 @@ 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.")
.default_value("100")
.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, if available.")
.takes_value(false),
)
}
Loading

0 comments on commit 03d68f5

Please sign in to comment.