From d8e406b6acb31b26ae8f242dfa699a4c6880ce51 Mon Sep 17 00:00:00 2001 From: chonghe <44791194+chong-he@users.noreply.github.com> Date: Mon, 15 Jun 2026 14:03:13 +0800 Subject: [PATCH] Append client version info to user graffiti by default (#9313) Closes issue #9287 Co-Authored-By: Tan Chee Keong Co-Authored-By: chonghe <44791194+chong-he@users.noreply.github.com> Co-Authored-By: Michael Sproul --- .../beacon_chain/src/graffiti_calculator.rs | 6 +- beacon_node/execution_layer/src/engine_api.rs | 8 +- beacon_node/http_api/tests/tests.rs | 101 +++++++++++++++--- book/src/help_vc.md | 8 +- book/src/validator_graffiti.md | 38 ++++++- common/eth2/src/lib.rs | 15 +-- common/eth2/src/types.rs | 4 +- lighthouse/tests/validator_client.rs | 53 +++++++++ validator_client/src/cli.rs | 9 +- validator_client/src/config.rs | 2 +- 10 files changed, 203 insertions(+), 41 deletions(-) diff --git a/beacon_node/beacon_chain/src/graffiti_calculator.rs b/beacon_node/beacon_chain/src/graffiti_calculator.rs index 403873cc00..705121824c 100644 --- a/beacon_node/beacon_chain/src/graffiti_calculator.rs +++ b/beacon_node/beacon_chain/src/graffiti_calculator.rs @@ -62,7 +62,7 @@ impl GraffitiSettings { validator_graffiti .map(|graffiti| Self::Specified { graffiti, - policy: policy.unwrap_or(GraffitiPolicy::PreserveUserGraffiti), + policy: policy.unwrap_or_default(), }) .unwrap_or(Self::Unspecified) } @@ -480,9 +480,9 @@ mod tests { // 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 + // There is a space between the user graffiti and client version info // as defined in calculate_graffiti function in engine_api.rs - format!("{} {}", append_graffiti_string, graffiti) + format!("{} {}", graffiti, append_graffiti_string) }; let expected_graffiti_prefix_bytes = expected_graffiti_string.as_bytes(); diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index d9dd9aaf4c..079a82ff98 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -832,16 +832,16 @@ impl ClientVersionV1 { // 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) + format!("{} {}", graffiti_str, append_graffiti_full) // user-specified graffiti is between 20-23 characters } else if (20..24).contains(&graffiti_length) { - format!("{} {}", append_graffiti_one_byte, graffiti_str) + format!("{} {}", graffiti_str, append_graffiti_one_byte) // user-specified graffiti is between 24-27 characters } else if (24..28).contains(&graffiti_length) { - format!("{} {}", append_graffiti_no_commit, graffiti_str) + format!("{} {}", graffiti_str, append_graffiti_no_commit) // user-specified graffiti is between 28-29 characters } else if (28..30).contains(&graffiti_length) { - format!("{} {}", append_graffiti_only_el, graffiti_str) + format!("{} {}", graffiti_str, append_graffiti_only_el) // if user-specified graffiti is between 30-32 characters, append nothing } else { return graffiti; diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 455a957337..843e055827 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -7967,8 +7967,8 @@ impl ApiTester { let graffiti = Some(Graffiti::from([0; GRAFFITI_BYTES_LEN])); let builder_boost_factor = None; - // Default case where GraffitiPolicy is None - let default_path = self + // When GraffitiPolicy is None + let no_graffiti_policy_path = self .client .get_validator_blocks_v3_path( slot, @@ -7981,13 +7981,30 @@ impl ApiTester { .await .unwrap(); + // Default case where GraffitiPolicy is AppendClientVersions + let default_path = self + .client + .get_validator_blocks_v3_path( + slot, + &randao_reveal, + graffiti.as_ref(), + SkipRandaoVerification::Yes, + builder_boost_factor, + Some(GraffitiPolicy::AppendClientVersions), + ) + .await + .unwrap(); + + let query_none_path = no_graffiti_policy_path.query().unwrap_or(""); let query_default_path = default_path.query().unwrap_or(""); - // When GraffitiPolicy is None, the HTTP API query path should not contain "graffiti_policy" + // When GraffitiPolicy is AppendClientVersions (default GraffitiPolicy), 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 ); + // The HTTP API query path for GraffiliPolicy is None should be the same as the default (GraffitiPolicy = AppendClientVersions) + assert_eq!(query_none_path, query_default_path); let preserve_path = self .client @@ -8003,36 +8020,86 @@ impl ApiTester { .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" + // When GraffitiPolicy is set to PreserveUserGraffiti, the HTTP API query path should contain "graffiti_policy" assert!( - !query_preserve_path.contains("graffiti_policy"), + 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); + self + } - let append_path = self + async fn get_validator_blocks_v4_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; + + // When GraffitiPolicy is None + let no_graffiti_policy_path = self .client - .get_validator_blocks_v3_path( + .get_validator_blocks_v4_path( slot, &randao_reveal, graffiti.as_ref(), - SkipRandaoVerification::No, + SkipRandaoVerification::Yes, + None, + builder_boost_factor, + None, + ) + .await + .unwrap(); + + // Default case where GraffitiPolicy is AppendClientVersions + let default_path = self + .client + .get_validator_blocks_v4_path( + slot, + &randao_reveal, + graffiti.as_ref(), + SkipRandaoVerification::Yes, + None, 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" + let query_none_path = no_graffiti_policy_path.query().unwrap_or(""); + let query_default_path = default_path.query().unwrap_or(""); + // When GraffitiPolicy is AppendClientVersions (default GraffitiPolicy), the HTTP API query path should not contain "graffiti_policy" assert!( - query_append_path.contains("graffiti_policy"), - "URL should contain graffiti_policy=AppendClientVersions parameter. URL is: {}", - query_append_path + !query_default_path.contains("graffiti_policy"), + "URL should not contain graffiti_policy parameter (same as PreserveUserGraffiti). URL is: {}", + query_default_path ); + // The HTTP API query path for GraffiliPolicy is None should be the same as the default (GraffitiPolicy = AppendClientVersions) + assert_eq!(query_none_path, query_default_path); + + let preserve_path = self + .client + .get_validator_blocks_v4_path( + slot, + &randao_reveal, + graffiti.as_ref(), + SkipRandaoVerification::Yes, + None, + 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 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 + ); + self } } @@ -9531,10 +9598,12 @@ async fn get_beacon_rewards_attestations_fulu() { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn get_validator_blocks_v3_http_api_path() { +async fn get_validator_blocks_http_api_path() { ApiTester::new() .await .get_validator_blocks_v3_path_graffiti_policy() + .await + .get_validator_blocks_v4_path_graffiti_policy() .await; } diff --git a/book/src/help_vc.md b/book/src/help_vc.md index 4647780ea8..f1a342197c 100644 --- a/book/src/help_vc.md +++ b/book/src/help_vc.md @@ -227,10 +227,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. + --graffiti-append [] + Client version info will be appended to user custom graffiti, with a + space in between. This should only be set to false when using a + Lighthouse beacon node. [default: true] [possible values: true, false] -h, --help Prints help information --http diff --git a/book/src/validator_graffiti.md b/book/src/validator_graffiti.md index 9908d056da..801d73d676 100644 --- a/book/src/validator_graffiti.md +++ b/book/src/validator_graffiti.md @@ -60,7 +60,7 @@ Usage: `lighthouse vc --graffiti example` ## 4. Using the "--graffiti" flag on the beacon node -Users can also specify a common graffiti using the `--graffiti` flag on the beacon node as a common graffiti for all validators. +Users can also specify a common graffiti using the `--graffiti` flag on the beacon node as a common graffiti for all validators. Usage: `lighthouse bn --graffiti fortytwo` @@ -93,3 +93,39 @@ curl -X PATCH "http://localhost:5062/lighthouse/validators/0xb0148e6348264131bf4 ``` A `null` response indicates that the request is successful. + +## Automatically append client version info to user graffiti + +> Note: this feature only works when a Lighthouse validator client is connected to a Lighthouse beacon node. + +In the interest of obtaining client diversity data, Lighthouse will by default automatically append client version info +to user graffiti in proposed blocks. + +For example, you set the graffiti in the validator client as `This is my graffiti`. You are using Lighthouse (`LH`) v8.1.3 +with commit hash `176cce5` and Reth (`RH`) v2.2.0 with commit hash `88505c7`. The appended graffiti will include: + +- Execution layer client code +- First two bytes of the execution layer commit hash +- Consensus layer client code +- First two bytes of the consensus layer commit hash + +When the user graffiti is less than 20 characters, as in the above example, the appended graffiti when proposing a block +will be: `This is my graffiti RH8850LH176c`. + +Given that the total size of the graffiti is 32 bytes, if the appended graffiti exceeds the size, the appended +client version info will automatically be shortened. Some examples are as follows, where the last part of the graffiti is the +appended client version info. + +When the user graffiti is between 20-23 characters: +`This is my graffiti yo RH88LH17` + +When the user graffiti is between 24-27 characters: +`This is my graffiti string RHLH` + +When the user graffiti is between 28-29 characters: +`This is my graffiti string yo RH` + +When the user graffiti is between 30-32 characters, no client version info will be appended: +`This is my graffiti string yo yo` + +To opt out from this, when using a Lighthouse beacon node, use the flag `--graffiti-append false` on the validator client. This will retain your own graffiti when proposing a block, without appending any client version info. diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index 572f9522ee..46f553542a 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -2393,12 +2393,12 @@ 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 + // Only append the HTTP URL request if the graffiti_policy is PreserveUserGraffiti + // If AppendClientVersions (default), then we do not modify the HTTP URL request // so that the default case is compliant to the spec - if let Some(GraffitiPolicy::AppendClientVersions) = graffiti_policy { + if let Some(GraffitiPolicy::PreserveUserGraffiti) = graffiti_policy { path.query_pairs_mut() - .append_pair("graffiti_policy", "AppendClientVersions"); + .append_pair("graffiti_policy", "PreserveUserGraffiti"); } Ok(path) @@ -2600,9 +2600,12 @@ impl BeaconNodeHttpClient { .append_pair("builder_boost_factor", &builder_booster_factor.to_string()); } - if let Some(GraffitiPolicy::AppendClientVersions) = graffiti_policy { + // Only append the HTTP URL request if the graffiti_policy is PreserveUserGraffiti + // If AppendClientVersions (default), then we do not modify the HTTP URL request + // so that the default case is compliant to the spec + if let Some(GraffitiPolicy::PreserveUserGraffiti) = graffiti_policy { path.query_pairs_mut() - .append_pair("graffiti_policy", "AppendClientVersions"); + .append_pair("graffiti_policy", "PreserveUserGraffiti"); } Ok(path) diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index 449ea88685..049ffba657 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -758,10 +758,10 @@ pub struct ProposerData { pub slot: Slot, } -#[derive(Clone, Copy, Serialize, Deserialize, Default, Debug)] +#[derive(Clone, Copy, Serialize, Deserialize, Default, Debug, PartialEq)] pub enum GraffitiPolicy { - #[default] PreserveUserGraffiti, + #[default] AppendClientVersions, } diff --git a/lighthouse/tests/validator_client.rs b/lighthouse/tests/validator_client.rs index 6fd5a6538c..945e363ab5 100644 --- a/lighthouse/tests/validator_client.rs +++ b/lighthouse/tests/validator_client.rs @@ -2,6 +2,7 @@ use beacon_node_fallback::{ApiTopic, beacon_node_health::BeaconNodeSyncDistanceT use crate::exec::CommandLineTestExec; use bls::{Keypair, PublicKeyBytes}; +use eth2::types::GraffitiPolicy; use initialized_validators::DEFAULT_WEB3SIGNER_KEEP_ALIVE; use sensitive_url::SensitiveUrl; use std::fs::File; @@ -261,6 +262,58 @@ fn graffiti_file_with_pk_flag() { }); } +// Tests for graffiti-append flag +#[test] +fn graffiti_append_default() { + CommandLineTest::new().run().with_config(|config| { + assert_eq!( + config.graffiti_policy, + Some(GraffitiPolicy::AppendClientVersions) + ); + }); +} + +#[test] +fn graffiti_append_true_flag() { + CommandLineTest::new() + .flag("graffiti-append", Some("true")) + .run() + .with_config(|config| { + assert_eq!( + config.graffiti_policy, + Some(GraffitiPolicy::AppendClientVersions) + ); + }); +} + +#[test] +fn graffiti_append_false_flag() { + CommandLineTest::new() + .flag("graffiti-append", Some("false")) + .run() + .with_config(|config| { + assert_eq!( + config.graffiti_policy, + Some(GraffitiPolicy::PreserveUserGraffiti) + ); + }); +} + +// Retain previous behaviour: `--graffiti-append` with no value is the same as +// `--graffiti-append true`. +#[test] +fn graffiti_append_no_value() { + CommandLineTest::new() + .flag("graffiti-append", None) + .run() + .with_config(|config| { + assert_eq!( + config.graffiti_policy, + Some(GraffitiPolicy::AppendClientVersions) + ); + }); +} + // Tests for suggested-fee-recipient flags. #[test] fn fee_recipient_flag() { diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index 0eb0e9e5dd..e5fe1580da 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -152,13 +152,14 @@ pub struct ValidatorClient { #[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.", + num_args = 0..=1, + help = "Client version info will be appended to user custom graffiti, with a space in between. \ + This should only be set to false when using a Lighthouse beacon node.", display_order = 0, + default_value = "true", help_heading = FLAG_HEADER )] - pub graffiti_append: bool, + pub graffiti_append: Option, #[clap( long, diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index d68a78b705..418cd385da 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -239,7 +239,7 @@ impl Config { } } - config.graffiti_policy = if validator_client_config.graffiti_append { + config.graffiti_policy = if validator_client_config.graffiti_append.unwrap_or(true) { Some(GraffitiPolicy::AppendClientVersions) } else { Some(GraffitiPolicy::PreserveUserGraffiti)