diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 25b2aa30cb3..2107f06e1e3 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -33,7 +33,7 @@ use crate::events::ServerSentEventHandler; use crate::execution_payload::{NotifyExecutionLayer, PreparePayloadHandle, get_execution_payload}; use crate::fetch_blobs::EngineGetBlobsOutput; use crate::fork_choice_signal::{ForkChoiceSignalRx, ForkChoiceSignalTx, ForkChoiceWaitResult}; -use crate::graffiti_calculator::GraffitiCalculator; +use crate::graffiti_calculator::{GraffitiCalculator, GraffitiSettings}; use crate::kzg_utils::reconstruct_blobs; use crate::light_client_finality_update_verification::{ Error as LightClientFinalityUpdateError, VerifiedLightClientFinalityUpdate, @@ -4493,7 +4493,7 @@ impl BeaconChain { self: &Arc, randao_reveal: Signature, slot: Slot, - validator_graffiti: Option, + graffiti_settings: GraffitiSettings, verification: ProduceBlockVerification, builder_boost_factor: Option, block_production_version: BlockProductionVersion, @@ -4527,7 +4527,7 @@ impl BeaconChain { state_root_opt, slot, randao_reveal, - validator_graffiti, + graffiti_settings, verification, builder_boost_factor, block_production_version, @@ -5060,7 +5060,7 @@ impl BeaconChain { state_root_opt: Option, produce_at_slot: Slot, randao_reveal: Signature, - validator_graffiti: Option, + graffiti_settings: GraffitiSettings, verification: ProduceBlockVerification, builder_boost_factor: Option, block_production_version: BlockProductionVersion, @@ -5071,7 +5071,7 @@ impl BeaconChain { let chain = self.clone(); let graffiti = self .graffiti_calculator - .get_graffiti(validator_graffiti) + .get_graffiti(graffiti_settings) .await; let span = Span::current(); let mut partial_beacon_block = self diff --git a/beacon_node/beacon_chain/src/graffiti_calculator.rs b/beacon_node/beacon_chain/src/graffiti_calculator.rs index 56808e0e67e..85470715c9f 100644 --- a/beacon_node/beacon_chain/src/graffiti_calculator.rs +++ b/beacon_node/beacon_chain/src/graffiti_calculator.rs @@ -1,5 +1,6 @@ use crate::BeaconChain; use crate::BeaconChainTypes; +use eth2::types::GraffitiPolicy; use execution_layer::{CommitPrefix, ExecutionLayer, http::ENGINE_GET_CLIENT_VERSION_V1}; use logging::crit; use serde::{Deserialize, Serialize}; @@ -48,6 +49,25 @@ impl Debug for GraffitiOrigin { } } +pub enum GraffitiSettings { + Unspecified, + Specified { + graffiti: Graffiti, + policy: GraffitiPolicy, + }, +} + +impl GraffitiSettings { + pub fn new(validator_graffiti: Option, policy: Option) -> Self { + validator_graffiti + .map(|graffiti| Self::Specified { + graffiti, + policy: policy.unwrap_or(GraffitiPolicy::PreserveUserGraffiti), + }) + .unwrap_or(Self::Unspecified) + } +} + pub struct GraffitiCalculator { pub beacon_graffiti: GraffitiOrigin, execution_layer: Option>, @@ -73,11 +93,19 @@ impl GraffitiCalculator { /// 2. Graffiti specified by the user via beacon node CLI options. /// 3. The EL & CL client version string, applicable when the EL supports version specification. /// 4. The default lighthouse version string, used if the EL lacks version specification support. - pub async fn get_graffiti(&self, validator_graffiti: Option) -> Graffiti { - if let Some(graffiti) = validator_graffiti { - return graffiti; + pub async fn get_graffiti(&self, graffiti_settings: GraffitiSettings) -> Graffiti { + match graffiti_settings { + GraffitiSettings::Specified { graffiti, policy } => match policy { + GraffitiPolicy::PreserveUserGraffiti => graffiti, + GraffitiPolicy::AppendClientVersions => { + self.calculate_combined_graffiti(Some(graffiti)).await + } + }, + GraffitiSettings::Unspecified => self.calculate_combined_graffiti(None).await, } + } + async fn calculate_combined_graffiti(&self, validator_graffiti: Option) -> Graffiti { match self.beacon_graffiti { GraffitiOrigin::UserSpecified(graffiti) => graffiti, GraffitiOrigin::Calculated(default_graffiti) => { @@ -133,7 +161,7 @@ impl GraffitiCalculator { CommitPrefix("00000000".to_string()) }); - engine_version.calculate_graffiti(lighthouse_commit_prefix) + engine_version.calculate_graffiti(lighthouse_commit_prefix, validator_graffiti) } } } @@ -224,8 +252,10 @@ async fn engine_version_cache_refresh_service( #[cfg(test)] mod tests { use crate::ChainConfig; + use crate::graffiti_calculator::GraffitiSettings; use crate::test_utils::{BeaconChainHarness, EphemeralHarnessType, test_spec}; use bls::Keypair; + use eth2::types::GraffitiPolicy; use execution_layer::EngineCapabilities; use execution_layer::test_utils::{DEFAULT_CLIENT_VERSION, DEFAULT_ENGINE_CAPABILITIES}; use std::sync::Arc; @@ -281,8 +311,12 @@ mod tests { let version_bytes = std::cmp::min(lighthouse_version::VERSION.len(), GRAFFITI_BYTES_LEN); // grab the slice of the graffiti that corresponds to the lighthouse version - let graffiti_slice = - &harness.chain.graffiti_calculator.get_graffiti(None).await.0[..version_bytes]; + let graffiti_slice = &harness + .chain + .graffiti_calculator + .get_graffiti(GraffitiSettings::Unspecified) + .await + .0[..version_bytes]; // convert graffiti bytes slice to ascii for easy debugging if this test should fail let graffiti_str = @@ -303,7 +337,12 @@ mod tests { let spec = Arc::new(test_spec::()); let harness = get_harness(VALIDATOR_COUNT, spec, None); - let found_graffiti_bytes = harness.chain.graffiti_calculator.get_graffiti(None).await.0; + let found_graffiti_bytes = harness + .chain + .graffiti_calculator + .get_graffiti(GraffitiSettings::Unspecified) + .await + .0; let mock_commit = DEFAULT_CLIENT_VERSION.commit.clone(); let expected_graffiti_string = format!( @@ -352,7 +391,10 @@ mod tests { let found_graffiti = harness .chain .graffiti_calculator - .get_graffiti(Some(Graffiti::from(graffiti_bytes))) + .get_graffiti(GraffitiSettings::new( + Some(Graffiti::from(graffiti_bytes)), + Some(GraffitiPolicy::PreserveUserGraffiti), + )) .await; assert_eq!( @@ -360,4 +402,98 @@ mod tests { "0x6e6963652067726166666974692062726f000000000000000000000000000000" ); } + + #[tokio::test] + async fn check_append_el_version_graffiti_various_length() { + let spec = Arc::new(test_spec::()); + let harness = get_harness(VALIDATOR_COUNT, spec, None); + + let graffiti_vec = vec![ + // less than 20 characters, example below is 19 characters + "This is my graffiti", + // 20-23 characters, example below is 22 characters + "This is my graffiti yo", + // 24-27 characters, example below is 26 characters + "This is my graffiti string", + // 28-29 characters, example below is 29 characters + "This is my graffiti string yo", + // 30-32 characters, example below is 32 characters + "This is my graffiti string yo yo", + ]; + + for graffiti in graffiti_vec { + let mut graffiti_bytes = [0; GRAFFITI_BYTES_LEN]; + graffiti_bytes[..graffiti.len()].copy_from_slice(graffiti.as_bytes()); + + // To test appending client version info with user specified graffiti + let policy = GraffitiPolicy::AppendClientVersions; + let found_graffiti_bytes = harness + .chain + .graffiti_calculator + .get_graffiti(GraffitiSettings::Specified { + graffiti: Graffiti::from(graffiti_bytes), + policy, + }) + .await + .0; + + let mock_commit = DEFAULT_CLIENT_VERSION.commit.clone(); + + let graffiti_length = graffiti.len(); + let append_graffiti_string = match graffiti_length { + 0..=19 => format!( + "{}{}{}{}", + DEFAULT_CLIENT_VERSION.code, + mock_commit + .strip_prefix("0x") + .unwrap_or("&mock_commit") + .get(0..4) + .expect("should get first 2 bytes in hex"), + "LH", + lighthouse_version::COMMIT_PREFIX + .get(0..4) + .expect("should get first 2 bytes in hex") + ), + 20..=23 => format!( + "{}{}{}{}", + DEFAULT_CLIENT_VERSION.code, + mock_commit + .strip_prefix("0x") + .unwrap_or("&mock_commit") + .get(0..2) + .expect("should get first 2 bytes in hex"), + "LH", + lighthouse_version::COMMIT_PREFIX + .get(0..2) + .expect("should get first 2 bytes in hex") + ), + 24..=27 => format!("{}{}", DEFAULT_CLIENT_VERSION.code, "LH",), + 28..=29 => DEFAULT_CLIENT_VERSION.code.to_string(), + // when user graffiti length is 30-32 characters, append nothing + 30..=32 => String::new(), + _ => panic!( + "graffiti length should be less than or equal to GRAFFITI_BYTES_LEN (32 characters)" + ), + }; + + let expected_graffiti_string = if append_graffiti_string.is_empty() { + // for the case of empty append_graffiti_string, i.e., user-specified graffiti is 30-32 characters + graffiti.to_string() + } else { + // There is a space between the client version info and user graffiti + // as defined in calculate_graffiti function in engine_api.rs + format!("{} {}", append_graffiti_string, graffiti) + }; + + let expected_graffiti_prefix_bytes = expected_graffiti_string.as_bytes(); + let expected_graffiti_prefix_len = + std::cmp::min(expected_graffiti_prefix_bytes.len(), GRAFFITI_BYTES_LEN); + + let found_graffiti_string = + std::str::from_utf8(&found_graffiti_bytes[..expected_graffiti_prefix_len]) + .expect("bytes should convert nicely to ascii"); + + assert_eq!(expected_graffiti_string, found_graffiti_string); + } + } } diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 54fa6b5ff09..500c0b22d61 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -2,6 +2,7 @@ use crate::blob_verification::GossipVerifiedBlob; use crate::block_verification_types::{AsBlock, RpcBlock}; use crate::custody_context::NodeCustodyType; use crate::data_column_verification::CustodyDataColumn; +use crate::graffiti_calculator::GraffitiSettings; use crate::kzg_utils::build_data_column_sidecars; use crate::observed_operations::ObservationOutcome; pub use crate::persisted_beacon_chain::PersistedBeaconChain; @@ -23,7 +24,7 @@ use bls::get_withdrawal_credentials; use bls::{ AggregateSignature, Keypair, PublicKey, PublicKeyBytes, SecretKey, Signature, SignatureBytes, }; -use eth2::types::SignedBlockContentsTuple; +use eth2::types::{GraffitiPolicy, SignedBlockContentsTuple}; use execution_layer::test_utils::generate_genesis_header; use execution_layer::{ ExecutionLayer, @@ -943,6 +944,8 @@ where // BeaconChain errors out with `DuplicateFullyImported`. Vary the graffiti so that we produce // different blocks each time. let graffiti = Graffiti::from(self.rng.lock().random::<[u8; 32]>()); + let graffiti_settings = + GraffitiSettings::new(Some(graffiti), Some(GraffitiPolicy::PreserveUserGraffiti)); let randao_reveal = self.sign_randao_reveal(&state, proposer_index, slot); @@ -956,7 +959,7 @@ where None, slot, randao_reveal, - Some(graffiti), + graffiti_settings, ProduceBlockVerification::VerifyRandao, builder_boost_factor, BlockProductionVersion::V3, @@ -1000,6 +1003,8 @@ where // BeaconChain errors out with `DuplicateFullyImported`. Vary the graffiti so that we produce // different blocks each time. let graffiti = Graffiti::from(self.rng.lock().random::<[u8; 32]>()); + let graffiti_settings = + GraffitiSettings::new(Some(graffiti), Some(GraffitiPolicy::PreserveUserGraffiti)); let randao_reveal = self.sign_randao_reveal(&state, proposer_index, slot); @@ -1010,7 +1015,7 @@ where None, slot, randao_reveal, - Some(graffiti), + graffiti_settings, ProduceBlockVerification::VerifyRandao, None, BlockProductionVersion::FullV2, @@ -1059,6 +1064,8 @@ where // BeaconChain errors out with `DuplicateFullyImported`. Vary the graffiti so that we produce // different blocks each time. let graffiti = Graffiti::from(self.rng.lock().random::<[u8; 32]>()); + let graffiti_settings = + GraffitiSettings::new(Some(graffiti), Some(GraffitiPolicy::PreserveUserGraffiti)); let randao_reveal = self.sign_randao_reveal(&state, proposer_index, slot); @@ -1071,7 +1078,7 @@ where None, slot, randao_reveal, - Some(graffiti), + graffiti_settings, ProduceBlockVerification::VerifyRandao, None, BlockProductionVersion::FullV2, diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index b0cc4dd8241..88567ac6e12 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -763,8 +763,12 @@ pub struct ClientVersionV1 { } impl ClientVersionV1 { - pub fn calculate_graffiti(&self, lighthouse_commit_prefix: CommitPrefix) -> Graffiti { - let graffiti_string = format!( + pub fn calculate_graffiti( + &self, + lighthouse_commit_prefix: CommitPrefix, + validator_graffiti: Option, + ) -> Graffiti { + let append_graffiti_full = format!( "{}{}LH{}", self.code, self.commit @@ -778,6 +782,53 @@ impl ClientVersionV1 { .unwrap_or("0000") .to_lowercase(), ); + + // Implement the special case here: + // https://hackmd.io/@wmoBhF17RAOH2NZ5bNXJVg/BJX2c9gja#SPECIAL-CASE-the-flexible-standard + let append_graffiti_one_byte = format!( + "{}{}LH{}", + self.code, + self.commit + .0 + .get(..2) + .unwrap_or(self.commit.0.as_str()) + .to_lowercase(), + lighthouse_commit_prefix + .0 + .get(..2) + .unwrap_or("00") + .to_lowercase(), + ); + + let append_graffiti_no_commit = format!("{}LH", self.code); + let append_graffiti_only_el = format!("{}", self.code); + + let graffiti_string = if let Some(graffiti) = validator_graffiti { + let graffiti_length = graffiti.as_utf8_lossy().len(); + let graffiti_str = graffiti.as_utf8_lossy(); + + // 12 characters for append_graffiti_full, plus one character for spacing + // that leaves user specified graffiti to be 32-12-1 = 19 characters max, i.e., <20 + if graffiti_length < 20 { + format!("{} {}", append_graffiti_full, graffiti_str) + // user-specified graffiti is between 20-23 characters + } else if (20..24).contains(&graffiti_length) { + format!("{} {}", append_graffiti_one_byte, graffiti_str) + // user-specified graffiti is between 24-27 characters + } else if (24..28).contains(&graffiti_length) { + format!("{} {}", append_graffiti_no_commit, graffiti_str) + // user-specified graffiti is between 28-29 characters + } else if (28..30).contains(&graffiti_length) { + format!("{} {}", append_graffiti_only_el, graffiti_str) + // if user-specified graffiti is between 30-32 characters, append nothing + } else { + return graffiti; + } + } else { + // if no validator_graffiti (user doesn't specify), use the full client version info graffiti + append_graffiti_full + }; + let mut graffiti_bytes = [0u8; GRAFFITI_BYTES_LEN]; let bytes_to_copy = std::cmp::min(graffiti_string.len(), GRAFFITI_BYTES_LEN); graffiti_bytes[..bytes_to_copy] diff --git a/beacon_node/http_api/src/produce_block.rs b/beacon_node/http_api/src/produce_block.rs index 472ec0b65e4..3bd0cec7e33 100644 --- a/beacon_node/http_api/src/produce_block.rs +++ b/beacon_node/http_api/src/produce_block.rs @@ -6,6 +6,7 @@ use crate::{ add_ssz_content_type_header, beacon_response, inconsistent_fork_rejection, }, }; +use beacon_chain::graffiti_calculator::GraffitiSettings; use beacon_chain::{ BeaconBlockResponseWrapper, BeaconChain, BeaconChainTypes, ProduceBlockVerification, }; @@ -68,11 +69,13 @@ pub async fn produce_block_v3( query.builder_boost_factor }; + let graffiti_settings = GraffitiSettings::new(query.graffiti, query.graffiti_policy); + let block_response_type = chain .produce_block_with_verification( randao_reveal, slot, - query.graffiti, + graffiti_settings, randao_verification, builder_boost_factor, BlockProductionVersion::V3, @@ -148,11 +151,13 @@ pub async fn produce_blinded_block_v2( })?; let randao_verification = get_randao_verification(&query, randao_reveal.is_infinity())?; + let graffiti_settings = GraffitiSettings::new(query.graffiti, query.graffiti_policy); + let block_response_type = chain .produce_block_with_verification( randao_reveal, slot, - query.graffiti, + graffiti_settings, randao_verification, None, BlockProductionVersion::BlindedV2, @@ -182,12 +187,13 @@ pub async fn produce_block_v2( })?; let randao_verification = get_randao_verification(&query, randao_reveal.is_infinity())?; + let graffiti_settings = GraffitiSettings::new(query.graffiti, query.graffiti_policy); let block_response_type = chain .produce_block_with_verification( randao_reveal, slot, - query.graffiti, + graffiti_settings, randao_verification, None, BlockProductionVersion::FullV2, diff --git a/beacon_node/http_api/tests/interactive_tests.rs b/beacon_node/http_api/tests/interactive_tests.rs index 0119a7645c2..ce61c821b57 100644 --- a/beacon_node/http_api/tests/interactive_tests.rs +++ b/beacon_node/http_api/tests/interactive_tests.rs @@ -640,7 +640,7 @@ pub async fn proposer_boost_re_org_test( .into(); let (unsigned_block_type, _) = tester .client - .get_validator_blocks_v3::(slot_c, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot_c, &randao_reveal, None, None, None) .await .unwrap(); diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index f8eba0ee2b7..ed7abead18a 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -3681,7 +3681,7 @@ impl ApiTester { let (response, metadata) = self .client - .get_validator_blocks_v3_ssz::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3_ssz::(slot, &randao_reveal, None, None, None) .await .unwrap(); @@ -4646,7 +4646,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -4673,7 +4673,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, Some(0)) + .get_validator_blocks_v3::(slot, &randao_reveal, None, Some(0), None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -4701,7 +4701,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, Some(u64::MAX)) + .get_validator_blocks_v3::(slot, &randao_reveal, None, Some(u64::MAX), None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -4858,7 +4858,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -4939,7 +4939,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5034,7 +5034,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5125,7 +5125,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5216,7 +5216,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5305,7 +5305,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5366,7 +5366,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5437,7 +5437,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5552,7 +5552,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5573,7 +5573,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5708,7 +5708,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5739,7 +5739,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5821,7 +5821,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5895,7 +5895,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5964,7 +5964,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -6033,7 +6033,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -6100,7 +6100,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -6174,7 +6174,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -6864,6 +6864,82 @@ impl ApiTester { } self } + + async fn get_validator_blocks_v3_path_graffiti_policy(self) -> Self { + let slot = self.chain.slot().unwrap(); + let epoch = self.chain.epoch().unwrap(); + let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; + let graffiti = Some(Graffiti::from([0; GRAFFITI_BYTES_LEN])); + let builder_boost_factor = None; + + // Default case where GraffitiPolicy is None + let default_path = self + .client + .get_validator_blocks_v3_path( + slot, + &randao_reveal, + graffiti.as_ref(), + SkipRandaoVerification::Yes, + builder_boost_factor, + None, + ) + .await + .unwrap(); + + let query_default_path = default_path.query().unwrap_or(""); + // When GraffitiPolicy is None, the HTTP API query path should not contain "graffiti_policy" + assert!( + !query_default_path.contains("graffiti_policy"), + "URL should not contain graffiti_policy parameter (same as PreserveUserGraffiti). URL is: {}", + query_default_path + ); + + let preserve_path = self + .client + .get_validator_blocks_v3_path( + slot, + &randao_reveal, + graffiti.as_ref(), + SkipRandaoVerification::Yes, + builder_boost_factor, + Some(GraffitiPolicy::PreserveUserGraffiti), + ) + .await + .unwrap(); + + let query_preserve_path = preserve_path.query().unwrap_or(""); + // When GraffitiPolicy is set to PreserveUserGraffiti, the HTTP API query path should not contain "graffiti_policy" + assert!( + !query_preserve_path.contains("graffiti_policy"), + "URL should not contain graffiti_policy parameter when using PreserveUserGraffiti. URL is: {}", + query_preserve_path + ); + + // The HTTP API query path for PreserveUserGraffiti should be the same as the default + assert_eq!(query_default_path, query_preserve_path); + + let append_path = self + .client + .get_validator_blocks_v3_path( + slot, + &randao_reveal, + graffiti.as_ref(), + SkipRandaoVerification::No, + builder_boost_factor, + Some(GraffitiPolicy::AppendClientVersions), + ) + .await + .unwrap(); + + let query_append_path = append_path.query().unwrap_or(""); + // When GraffitiPolicy is AppendClientVersions, the HTTP API query path should contain "graffiti_policy" + assert!( + query_append_path.contains("graffiti_policy"), + "URL should contain graffiti_policy=AppendClientVersions parameter. URL is: {}", + query_append_path + ); + self + } } async fn poll_events, eth2::Error>> + Unpin, E: EthSpec>( @@ -8054,3 +8130,11 @@ async fn get_beacon_rewards_blocks_electra() { .test_beacon_block_rewards_electra() .await; } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn get_validator_blocks_v3_http_api_path() { + ApiTester::new() + .await + .get_validator_blocks_v3_path_graffiti_policy() + .await; +} diff --git a/book/src/help_vc.md b/book/src/help_vc.md index b19ff0ba388..2a9936d1d2f 100644 --- a/book/src/help_vc.md +++ b/book/src/help_vc.md @@ -221,6 +221,10 @@ Flags: automatically enabled for <= 64 validators. Enabling this metric for higher validator counts will lead to higher volume of prometheus metrics being collected. + --graffiti-append + When used, client version info will be prepended to user custom + graffiti, with a space in between. This should only be used with a + Lighthouse beacon node. -h, --help Prints help information --http diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index 820d817d9d8..8746e3c063c 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -2207,6 +2207,7 @@ impl BeaconNodeHttpClient { graffiti: Option<&Graffiti>, skip_randao_verification: SkipRandaoVerification, builder_booster_factor: Option, + graffiti_policy: Option, ) -> Result { let mut path = self.eth_path(V3)?; @@ -2234,6 +2235,14 @@ impl BeaconNodeHttpClient { .append_pair("builder_boost_factor", &builder_booster_factor.to_string()); } + // Only append the HTTP URL request if the graffiti_policy is to AppendClientVersions + // If PreserveUserGraffiti (default), then the HTTP URL request does not contain graffiti_policy + // so that the default case is compliant to the spec + if let Some(GraffitiPolicy::AppendClientVersions) = graffiti_policy { + path.query_pairs_mut() + .append_pair("graffiti_policy", "AppendClientVersions"); + } + Ok(path) } @@ -2244,6 +2253,7 @@ impl BeaconNodeHttpClient { randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, builder_booster_factor: Option, + graffiti_policy: Option, ) -> Result<(JsonProduceBlockV3Response, ProduceBlockV3Metadata), Error> { self.get_validator_blocks_v3_modular( slot, @@ -2251,6 +2261,7 @@ impl BeaconNodeHttpClient { graffiti, SkipRandaoVerification::No, builder_booster_factor, + graffiti_policy, ) .await } @@ -2263,6 +2274,7 @@ impl BeaconNodeHttpClient { graffiti: Option<&Graffiti>, skip_randao_verification: SkipRandaoVerification, builder_booster_factor: Option, + graffiti_policy: Option, ) -> Result<(JsonProduceBlockV3Response, ProduceBlockV3Metadata), Error> { let path = self .get_validator_blocks_v3_path( @@ -2271,6 +2283,7 @@ impl BeaconNodeHttpClient { graffiti, skip_randao_verification, builder_booster_factor, + graffiti_policy, ) .await?; @@ -2313,6 +2326,7 @@ impl BeaconNodeHttpClient { randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, builder_booster_factor: Option, + graffiti_policy: Option, ) -> Result<(ProduceBlockV3Response, ProduceBlockV3Metadata), Error> { self.get_validator_blocks_v3_modular_ssz::( slot, @@ -2320,6 +2334,7 @@ impl BeaconNodeHttpClient { graffiti, SkipRandaoVerification::No, builder_booster_factor, + graffiti_policy, ) .await } @@ -2332,6 +2347,7 @@ impl BeaconNodeHttpClient { graffiti: Option<&Graffiti>, skip_randao_verification: SkipRandaoVerification, builder_booster_factor: Option, + graffiti_policy: Option, ) -> Result<(ProduceBlockV3Response, ProduceBlockV3Metadata), Error> { let path = self .get_validator_blocks_v3_path( @@ -2340,6 +2356,7 @@ impl BeaconNodeHttpClient { graffiti, skip_randao_verification, builder_booster_factor, + graffiti_policy, ) .await?; diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index aace8f936c9..b1a61ce00cc 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -752,12 +752,20 @@ pub struct ProposerData { pub slot: Slot, } +#[derive(Clone, Copy, Serialize, Deserialize, Default, Debug)] +pub enum GraffitiPolicy { + #[default] + PreserveUserGraffiti, + AppendClientVersions, +} + #[derive(Clone, Deserialize)] pub struct ValidatorBlocksQuery { pub randao_reveal: SignatureBytes, pub graffiti: Option, pub skip_randao_verification: SkipRandaoVerification, pub builder_boost_factor: Option, + pub graffiti_policy: Option, } #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Deserialize)] diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index 477781d3e88..3e1c46097f0 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -150,6 +150,16 @@ pub struct ValidatorClient { )] pub graffiti: Option, + #[clap( + long, + requires = "graffiti", + help = "When used, client version info will be prepended to user custom graffiti, with a space in between. \ + This should only be used with a Lighthouse beacon node.", + display_order = 0, + help_heading = FLAG_HEADER + )] + pub graffiti_append: bool, + #[clap( long, value_name = "GRAFFITI-FILE", diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index 04d69dc9dc1..1a286a74dc1 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -7,7 +7,7 @@ use directory::{ DEFAULT_HARDCODED_NETWORK, DEFAULT_ROOT_DIR, DEFAULT_SECRET_DIR, DEFAULT_VALIDATOR_DIR, get_network_dir, }; -use eth2::types::Graffiti; +use eth2::types::{Graffiti, GraffitiPolicy}; use graffiti_file::GraffitiFile; use initialized_validators::Config as InitializedValidatorsConfig; use lighthouse_validator_store::Config as ValidatorStoreConfig; @@ -55,6 +55,8 @@ pub struct Config { pub graffiti: Option, /// Graffiti file to load per validator graffitis. pub graffiti_file: Option, + /// GraffitiPolicy to append client version info + pub graffiti_policy: Option, /// Configuration for the HTTP REST API. pub http_api: validator_http_api::Config, /// Configuration for the HTTP REST API. @@ -119,6 +121,7 @@ impl Default for Config { long_timeouts_multiplier: 1, graffiti: None, graffiti_file: None, + graffiti_policy: None, http_api: <_>::default(), http_metrics: <_>::default(), beacon_node_fallback: <_>::default(), @@ -233,6 +236,12 @@ impl Config { } } + config.graffiti_policy = if validator_client_config.graffiti_append { + Some(GraffitiPolicy::AppendClientVersions) + } else { + Some(GraffitiPolicy::PreserveUserGraffiti) + }; + if let Some(input_fee_recipient) = validator_client_config.suggested_fee_recipient { config.validator_store.fee_recipient = Some(input_fee_recipient); } diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index 71bdde10b02..23541cf6e28 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -486,7 +486,8 @@ impl ProductionValidatorClient { .executor(context.executor.clone()) .chain_spec(context.eth2_config.spec.clone()) .graffiti(config.graffiti) - .graffiti_file(config.graffiti_file.clone()); + .graffiti_file(config.graffiti_file.clone()) + .graffiti_policy(config.graffiti_policy); // If we have proposer nodes, add them to the block service builder. if proposer_nodes_num > 0 { diff --git a/validator_client/validator_services/src/block_service.rs b/validator_client/validator_services/src/block_service.rs index 23658af03fd..625f8db7cb9 100644 --- a/validator_client/validator_services/src/block_service.rs +++ b/validator_client/validator_services/src/block_service.rs @@ -1,5 +1,6 @@ use beacon_node_fallback::{ApiTopic, BeaconNodeFallback, Error as FallbackError, Errors}; use bls::PublicKeyBytes; +use eth2::types::GraffitiPolicy; use eth2::{BeaconNodeHttpClient, StatusCode}; use graffiti_file::{GraffitiFile, determine_graffiti}; use logging::crit; @@ -50,6 +51,7 @@ pub struct BlockServiceBuilder { chain_spec: Option>, graffiti: Option, graffiti_file: Option, + graffiti_policy: Option, } impl BlockServiceBuilder { @@ -63,6 +65,7 @@ impl BlockServiceBuilder { chain_spec: None, graffiti: None, graffiti_file: None, + graffiti_policy: None, } } @@ -106,6 +109,11 @@ impl BlockServiceBuilder { self } + pub fn graffiti_policy(mut self, graffiti_policy: Option) -> Self { + self.graffiti_policy = graffiti_policy; + self + } + pub fn build(self) -> Result, String> { Ok(BlockService { inner: Arc::new(Inner { @@ -127,6 +135,7 @@ impl BlockServiceBuilder { proposer_nodes: self.proposer_nodes, graffiti: self.graffiti, graffiti_file: self.graffiti_file, + graffiti_policy: self.graffiti_policy, }), }) } @@ -192,6 +201,7 @@ pub struct Inner { chain_spec: Arc, graffiti: Option, graffiti_file: Option, + graffiti_policy: Option, } /// Attempts to produce attestations for any block producer(s) at the start of the epoch. @@ -466,6 +476,7 @@ impl BlockService { randao_reveal_ref, graffiti.as_ref(), builder_boost_factor, + self_ref.graffiti_policy, ) .await }) @@ -492,6 +503,7 @@ impl BlockService { randao_reveal_ref, graffiti.as_ref(), builder_boost_factor, + self_ref.graffiti_policy, ) .await .map_err(|e| {