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 <mjladson@pm.me>
Co-authored-by: Jimmy Chen <jchen.tc@gmail.com>
Co-authored-by: Paul Hauner <paul@paulhauner.com>
This commit is contained in:
Eitan Seri-Levi
2024-01-25 00:09:47 +02:00
committed by GitHub
parent 612eaf2d41
commit f9e36c94ed
28 changed files with 715 additions and 21 deletions

View File

@@ -46,6 +46,8 @@ pub struct ValidatorSpecification {
pub fee_recipient: Option<Address>,
pub gas_limit: Option<u64>,
pub builder_proposals: Option<bool>,
pub builder_boost_factor: Option<u64>,
pub prefer_builder_proposals: Option<bool>,
pub enabled: Option<bool>,
}
@@ -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

View File

@@ -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<Address>,
pub builder_proposals: Option<bool>,
pub builder_boost_factor: Option<u64>,
pub prefer_builder_proposals: Option<bool>,
pub fee_recipient: Option<Address>,
pub gas_limit: Option<u64>,
pub bn_url: Option<SensitiveUrl>,
@@ -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,

View File

@@ -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<bool>,
pub builder_boost_factor: Option<u64>,
pub prefer_builder_proposals: Option<bool>,
pub fee_recipient: Option<Address>,
pub gas_limit: Option<u64>,
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()),