Add support for updating validator graffiti (#4417)

## Issue Addressed

#4386 

## Proposed Changes

The original proposal described in the issue adds a new endpoint to support updating validator graffiti, but I realized we already have an endpoint that we use for updating various validator fields in memory and in the validator definitions file, so I think that would be the best place to add this to.

### API endpoint

`PATCH lighthouse/validators/{validator_pubkey}` 

This endpoint updates the graffiti in both the [ validator definition file](https://lighthouse-book.sigmaprime.io/graffiti.html#2-setting-the-graffiti-in-the-validator_definitionsyml) and the in memory `InitializedValidators`. In the next block proposal, the new graffiti will be used.

Note that the [`--graffiti-file`](https://lighthouse-book.sigmaprime.io/graffiti.html#1-using-the---graffiti-file-flag-on-the-validator-client) flag has a priority over the validator definitions file, so if the caller attempts to update the graffiti while the `--graffiti-file` flag is present, the endpoint will return an error (Bad request 400).

## Tasks

- [x] Add graffiti update support to `PATCH lighthouse/validators/{validator_pubkey}` 
- [x] Return error if user tries to update graffiti while the `--graffiti-flag` is set
- [x] Update Lighthouse book
This commit is contained in:
Jimmy Chen
2023-06-22 02:14:57 +00:00
parent 3cac6d9ed5
commit 33c942ff03
8 changed files with 143 additions and 11 deletions

View File

@@ -357,7 +357,7 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
.and(warp::path("graffiti"))
.and(warp::path::end())
.and(validator_store_filter.clone())
.and(graffiti_file_filter)
.and(graffiti_file_filter.clone())
.and(graffiti_flag_filter)
.and(signer.clone())
.and(log_filter.clone())
@@ -617,18 +617,27 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
.and(warp::path::end())
.and(warp::body::json())
.and(validator_store_filter.clone())
.and(graffiti_file_filter)
.and(signer.clone())
.and(task_executor_filter.clone())
.and_then(
|validator_pubkey: PublicKey,
body: api_types::ValidatorPatchRequest,
validator_store: Arc<ValidatorStore<T, E>>,
graffiti_file: Option<GraffitiFile>,
signer,
task_executor: TaskExecutor| {
blocking_signed_json_task(signer, move || {
if body.graffiti.is_some() && graffiti_file.is_some() {
return Err(warp_utils::reject::custom_bad_request(
"Unable to update graffiti as the \"--graffiti-file\" flag is set"
.to_string(),
));
}
let maybe_graffiti = body.graffiti.clone().map(Into::into);
let initialized_validators_rw_lock = validator_store.initialized_validators();
let mut initialized_validators = initialized_validators_rw_lock.write();
match (
initialized_validators.is_enabled(&validator_pubkey),
initialized_validators.validator(&validator_pubkey.compress()),
@@ -641,7 +650,8 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
if Some(is_enabled) == body.enabled
&& initialized_validator.get_gas_limit() == body.gas_limit
&& initialized_validator.get_builder_proposals()
== body.builder_proposals =>
== body.builder_proposals
&& initialized_validator.get_graffiti() == maybe_graffiti =>
{
Ok(())
}
@@ -654,6 +664,7 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
body.enabled,
body.gas_limit,
body.builder_proposals,
body.graffiti,
),
)
.map_err(|e| {

View File

@@ -28,12 +28,14 @@ use slot_clock::{SlotClock, TestingSlotClock};
use std::future::Future;
use std::marker::PhantomData;
use std::net::{IpAddr, Ipv4Addr};
use std::str::FromStr;
use std::sync::Arc;
use std::time::Duration;
use task_executor::TaskExecutor;
use tempfile::{tempdir, TempDir};
use tokio::runtime::Runtime;
use tokio::sync::oneshot;
use types::graffiti::GraffitiString;
const PASSWORD_BYTES: &[u8] = &[42, 50, 37];
pub const TEST_DEFAULT_FEE_RECIPIENT: Address = Address::repeat_byte(42);
@@ -533,7 +535,7 @@ impl ApiTester {
let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index];
self.client
.patch_lighthouse_validators(&validator.voting_pubkey, Some(enabled), None, None)
.patch_lighthouse_validators(&validator.voting_pubkey, Some(enabled), None, None, None)
.await
.unwrap();
@@ -575,7 +577,13 @@ impl ApiTester {
let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index];
self.client
.patch_lighthouse_validators(&validator.voting_pubkey, None, Some(gas_limit), None)
.patch_lighthouse_validators(
&validator.voting_pubkey,
None,
Some(gas_limit),
None,
None,
)
.await
.unwrap();
@@ -602,6 +610,7 @@ impl ApiTester {
None,
None,
Some(builder_proposals),
None,
)
.await
.unwrap();
@@ -620,6 +629,34 @@ impl ApiTester {
self
}
pub async fn set_graffiti(self, index: usize, graffiti: &str) -> Self {
let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index];
let graffiti_str = GraffitiString::from_str(graffiti).unwrap();
self.client
.patch_lighthouse_validators(
&validator.voting_pubkey,
None,
None,
None,
Some(graffiti_str),
)
.await
.unwrap();
self
}
pub async fn assert_graffiti(self, index: usize, graffiti: &str) -> Self {
let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index];
let graffiti_str = GraffitiString::from_str(graffiti).unwrap();
assert_eq!(
self.validator_store.graffiti(&validator.voting_pubkey),
Some(graffiti_str.into())
);
self
}
}
struct HdValidatorScenario {
@@ -723,7 +760,13 @@ fn routes_with_invalid_auth() {
.await
.test_with_invalid_auth(|client| async move {
client
.patch_lighthouse_validators(&PublicKeyBytes::empty(), Some(false), None, None)
.patch_lighthouse_validators(
&PublicKeyBytes::empty(),
Some(false),
None,
None,
None,
)
.await
})
.await
@@ -931,6 +974,41 @@ fn validator_builder_proposals() {
});
}
#[test]
fn validator_graffiti() {
let runtime = build_runtime();
let weak_runtime = Arc::downgrade(&runtime);
runtime.block_on(async {
ApiTester::new(weak_runtime)
.await
.create_hd_validators(HdValidatorScenario {
count: 2,
specify_mnemonic: false,
key_derivation_path_offset: 0,
disabled: vec![],
})
.await
.assert_enabled_validators_count(2)
.assert_validators_count(2)
.set_graffiti(0, "Mr F was here")
.await
.assert_graffiti(0, "Mr F was here")
.await
// Test setting graffiti while the validator is disabled
.set_validator_enabled(0, false)
.await
.assert_enabled_validators_count(1)
.assert_validators_count(2)
.set_graffiti(0, "Mr F was here again")
.await
.set_validator_enabled(0, true)
.await
.assert_enabled_validators_count(2)
.assert_graffiti(0, "Mr F was here again")
.await
});
}
#[test]
fn keystore_validator_creation() {
let runtime = build_runtime();

View File

@@ -468,7 +468,7 @@ fn import_and_delete_conflicting_web3_signer_keystores() {
for pubkey in &pubkeys {
tester
.client
.patch_lighthouse_validators(pubkey, Some(false), None, None)
.patch_lighthouse_validators(pubkey, Some(false), None, None, None)
.await
.unwrap();
}