-
Notifications
You must be signed in to change notification settings - Fork 1k
Append client version info to user graffiti by default #9313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: unstable
Are you sure you want to change the base?
Changes from 13 commits
78a3ba4
f50cf4e
6cab7f5
bbc0603
0b19d28
322a309
4af8aa8
7a86942
1ad7d99
8861721
0d172c2
bfc4ed4
a3eb614
4da58f9
70667fc
a8ec07b
5f826cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,38 @@ 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 | ||
|
|
||
| In the interest of obtaining client diversity data, Lighthouse will by default automatically append client version info | ||
| to user graffiti in the proposed blocks. | ||
|
chong-he marked this conversation as resolved.
Outdated
|
||
|
|
||
| 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, 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. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should highlight that this ONLY works with Lighthouse BNs
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Revised the documentation to make this clear |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,43 @@ 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")) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also need to test
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a test to prevent regression |
||
| .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) | ||
| ); | ||
| }); | ||
| } | ||
|
|
||
| // Tests for suggested-fee-recipient flags. | ||
| #[test] | ||
| fn fee_recipient_flag() { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -152,13 +152,12 @@ pub struct ValidatorClient { | |
|
|
||
| #[clap( | ||
| long, | ||
| requires = "graffiti", | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove this |
||
| help = "When used, client version info will be prepended to user custom graffiti, with a space in between. \ | ||
| help = "When used, client version info will be appended to user custom graffiti, with a space in between. \ | ||
| This should only be used with a Lighthouse beacon node.", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should tweak this to say:
And something about how the default behaviour is to append.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a default value to the flag in the cli to make it clearer |
||
| display_order = 0, | ||
| help_heading = FLAG_HEADER | ||
| )] | ||
| pub graffiti_append: bool, | ||
| pub graffiti_append: Option<bool>, | ||
|
|
||
| #[clap( | ||
| long, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should leave as is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at this, I thought I wanted to be consistent with other variables of using
SkipRandaoVerification::Yes, that's probably why I changed to yes. As this is a test that is not related to RANDAO, I guess it is ok to useYeshere?(i.e., the previous
Nois probably not quite right)