Implement feerecipient API for keymanager (#3213)

## Issue Addressed

* #3173 

## Proposed Changes

Moved all `fee_recipient_file` related logic inside the `ValidatorStore` as it makes more sense to have this all together there. I tested this with the validators I have on `mainnet-shadow-fork-5` and everything appeared to work well. Only technicality is that I can't get the method to return `401` when the authorization header is not specified (it returns `400` instead). Fixing this is probably quite difficult given that none of `warp`'s rejections have code `401`.. I don't really think this matters too much though as long as it fails.

book/src/suggested-fee-recipient.md

@@ -26,14 +26,9 @@ Lighthouse BN also provides a method for defining this value, should the VC not
 Assuming trustworthy nodes, the priority for the four methods is:
 
 1. `validator_definitions.yml`
-1. `--suggested-fee-recipient-file`
 1. `--suggested-fee-recipient` provided to the VC.
 1. `--suggested-fee-recipient` provided to the BN.
 
-Users may configure the fee recipient via `validator_definitions.yml` or via the
-`--suggested-fee-recipient-file` flag. The value in `validator_definitions.yml` will always take
-precedence.
-
 ### 1. Setting the fee recipient in the `validator_definitions.yml`
 
 Users can set the fee recipient in `validator_definitions.yml` with the `suggested_fee_recipient`
@@ -56,36 +51,111 @@ Below is an example of the validator_definitions.yml with `suggested_fee_recipie
   suggested_fee_recipient: "0xa2e334e71511686bcfe38bb3ee1ad8f6babcc03d"
 ```
 
-### 2. Using the "--suggested-fee-recipient-file" flag on the validator client
+### 2. Using the "--suggested-fee-recipient" flag on the validator client
-
-Users can specify a file with the `--suggested-fee-recipient-file` flag. This option is useful for dynamically
-changing fee recipients. This file is reloaded each time a validator is chosen to propose a block.
-
-Usage:
-`lighthouse vc --suggested-fee-recipient-file fee_recipient.txt`
-
-The file should contain key value pairs corresponding to validator public keys and their associated
-fee recipient. The file can optionally contain a `default` key for the default case.
-
-The following example sets the default and the values for the validators with pubkeys `0x87a5` and
-`0xa556`:
-
-```
-default: 0x6cc8dcbca744a6e4ffedb98e1d0df903b10abd21
-0x87a580d31d7bc69069b55f5a01995a610dd391a26dc9e36e81057a17211983a79266800ab8531f21f1083d7d84085007: 0x6cc8dcbca744a6e4ffedb98e1d0df903b10abd21
-0xa5566f9ec3c6e1fdf362634ebec9ef7aceb0e460e5079714808388e5d48f4ae1e12897fed1bea951c17fa389d511e477: 0xa2e334e71511686bcfe38bb3ee1ad8f6babcc03d
-```
-
-Lighthouse will first search for the fee recipient corresponding to the public key of the proposing
-validator, if there are no matches for the public key, then it uses the address corresponding to the
-default key (if present).
-
-### 3. Using the "--suggested-fee-recipient" flag on the validator client
 
 The `--suggested-fee-recipient` can be provided to the VC to act as a default value for all
 validators where a `suggested_fee_recipient` is not loaded from another method.
 
-### 4. Using the "--suggested-fee-recipient" flag on the beacon node
+### 3. Using the "--suggested-fee-recipient" flag on the beacon node
 
 The `--suggested-fee-recipient` can be provided to the BN to act as a default value when the
 validator client does not transmit a `suggested_fee_recipient` to the BN.
+
+## Setting the fee recipient dynamically using the keymanager API
+
+When the [validator client API](api-vc.md) is enabled, the
+[standard keymanager API](https://ethereum.github.io/keymanager-APIs/) includes an endpoint
+for setting the fee recipient dynamically for a given public key. When used, the fee recipient
+will be saved in `validator_definitions.yml` so that it persists across restarts of the validator
+client.
+
+| Property | Specification |
+| --- | --- |
+Path | `/eth/v1/validator/{pubkey}/feerecipient`
+Method | POST
+Required Headers | [`Authorization`](./api-vc-auth-header.md)
+Typical Responses | 202, 404
+
+#### Example Request Body
+```json
+{
+    "ethaddress": "0x1D4E51167DBDC4789a014357f4029ff76381b16c"
+}
+```
+
+```bash
+DATADIR=$HOME/.lighthouse/mainnet
+PUBKEY=0xa9735061c84fc0003657e5bd38160762b7ef2d67d280e00347b1781570088c32c06f15418c144949f5d736b1d3a6c591
+FEE_RECIPIENT=0x1D4E51167DBDC4789a014357f4029ff76381b16c
+
+curl -X POST \
+    -H "Authorization: Bearer $(cat ${DATADIR}/validators/api-token.txt)" \
+    -H "Content-Type: application/json" \
+    -d "{ \"ethaddress\": \"${FEE_RECIPIENT}\" }" \
+    http://localhost:5062/eth/v1/validator/${PUBKEY}/feerecipient | jq
+```
+
+#### Successful Response (202)
+```json
+null
+```
+
+### Querying the fee recipient
+
+The same path with a `GET` request can be used to query the fee recipient for a given public key at any time.
+
+| Property | Specification |
+| --- | --- |
+Path | `/eth/v1/validator/{pubkey}/feerecipient`
+Method | GET
+Required Headers | [`Authorization`](./api-vc-auth-header.md)
+Typical Responses | 200, 404
+
+```bash
+DATADIR=$HOME/.lighthouse/mainnet
+PUBKEY=0xa9735061c84fc0003657e5bd38160762b7ef2d67d280e00347b1781570088c32c06f15418c144949f5d736b1d3a6c591
+
+curl -X GET \
+    -H "Authorization: Bearer $(cat ${DATADIR}/validators/api-token.txt)" \
+    -H "Content-Type: application/json" \
+    http://localhost:5062/eth/v1/validator/${PUBKEY}/feerecipient | jq
+```
+
+#### Successful Response (200)
+```json
+{
+  "data": {
+    "pubkey": "0xa9735061c84fc0003657e5bd38160762b7ef2d67d280e00347b1781570088c32c06f15418c144949f5d736b1d3a6c591",
+    "ethaddress": "0x1d4e51167dbdc4789a014357f4029ff76381b16c"
+  }
+}
+```
+
+### Removing the fee recipient
+
+The same path with a `DELETE` request can be used to remove the fee recipient for a given public key at any time.
+This is useful if you want the fee recipient to fall back to the validator client (or beacon node) default.
+
+| Property | Specification |
+| --- | --- |
+Path | `/eth/v1/validator/{pubkey}/feerecipient`
+Method | DELETE
+Required Headers | [`Authorization`](./api-vc-auth-header.md)
+Typical Responses | 204, 404
+
+```bash
+DATADIR=$HOME/.lighthouse/mainnet
+PUBKEY=0xa9735061c84fc0003657e5bd38160762b7ef2d67d280e00347b1781570088c32c06f15418c144949f5d736b1d3a6c591
+
+curl -X DELETE \
+    -H "Authorization: Bearer $(cat ${DATADIR}/validators/api-token.txt)" \
+    -H "Content-Type: application/json" \
+    http://localhost:5062/eth/v1/validator/${PUBKEY}/feerecipient | jq
+```
+
+#### Successful Response (204)
+```json
+null
+```
+
+

common/eth2/src/lighthouse_vc/http_client.rs

@@ -303,11 +303,11 @@ impl ValidatorClientHttpClient {
     }
 
     /// Perform a HTTP DELETE request.
-    async fn delete_with_unsigned_response<T: Serialize, U: IntoUrl, V: DeserializeOwned>(
+    async fn delete_with_raw_response<T: Serialize, U: IntoUrl>(
         &self,
         url: U,
         body: &T,
-    ) -> Result<V, Error> {
+    ) -> Result<Response, Error> {
         let response = self
             .client
             .delete(url)
@@ -316,7 +316,16 @@ impl ValidatorClientHttpClient {
             .send()
             .await
             .map_err(Error::Reqwest)?;
-        let response = ok_or_error(response).await?;
+        ok_or_error(response).await
+    }
+
+    /// Perform a HTTP DELETE request.
+    async fn delete_with_unsigned_response<T: Serialize, U: IntoUrl, V: DeserializeOwned>(
+        &self,
+        url: U,
+        body: &T,
+    ) -> Result<V, Error> {
+        let response = self.delete_with_raw_response(url, body).await?;
         Ok(response.json().await?)
     }
 
@@ -486,6 +495,18 @@ impl ValidatorClientHttpClient {
         Ok(url)
     }
 
+    fn make_fee_recipient_url(&self, pubkey: &PublicKeyBytes) -> Result<Url, Error> {
+        let mut url = self.server.full.clone();
+        url.path_segments_mut()
+            .map_err(|()| Error::InvalidUrl(self.server.clone()))?
+            .push("eth")
+            .push("v1")
+            .push("validator")
+            .push(&pubkey.to_string())
+            .push("feerecipient");
+        Ok(url)
+    }
+
     /// `GET lighthouse/auth`
     pub async fn get_auth(&self) -> Result<AuthResponse, Error> {
         let mut url = self.server.full.clone();
@@ -543,14 +564,44 @@ impl ValidatorClientHttpClient {
         let url = self.make_remotekeys_url()?;
         self.delete_with_unsigned_response(url, req).await
     }
+
+    /// `GET /eth/v1/validator/{pubkey}/feerecipient`
+    pub async fn get_fee_recipient(
+        &self,
+        pubkey: &PublicKeyBytes,
+    ) -> Result<GetFeeRecipientResponse, Error> {
+        let url = self.make_fee_recipient_url(pubkey)?;
+        self.get(url)
+            .await
+            .map(|generic: GenericResponse<GetFeeRecipientResponse>| generic.data)
+    }
+
+    /// `POST /eth/v1/validator/{pubkey}/feerecipient`
+    pub async fn post_fee_recipient(
+        &self,
+        pubkey: &PublicKeyBytes,
+        req: &UpdateFeeRecipientRequest,
+    ) -> Result<Response, Error> {
+        let url = self.make_fee_recipient_url(pubkey)?;
+        self.post_with_raw_response(url, req).await
+    }
+
+    /// `POST /eth/v1/validator/{pubkey}/feerecipient`
+    pub async fn delete_fee_recipient(&self, pubkey: &PublicKeyBytes) -> Result<Response, Error> {
+        let url = self.make_fee_recipient_url(pubkey)?;
+        self.delete_with_raw_response(url, &()).await
+    }
 }
 
-/// Returns `Ok(response)` if the response is a `200 OK` response. Otherwise, creates an
+/// Returns `Ok(response)` if the response is a `200 OK` response or a
-/// appropriate error message.
+/// `202 Accepted` response. Otherwise, creates an appropriate error message.
 async fn ok_or_error(response: Response) -> Result<Response, Error> {
     let status = response.status();
 
-    if status == StatusCode::OK {
+    if status == StatusCode::OK
+        || status == StatusCode::ACCEPTED
+        || status == StatusCode::NO_CONTENT
+    {
         Ok(response)
     } else if let Ok(message) = response.json().await {
         Err(Error::ServerMessage(message))

common/eth2/src/lighthouse_vc/std_types.rs

@@ -2,7 +2,13 @@ use account_utils::ZeroizeString;
 use eth2_keystore::Keystore;
 use serde::{Deserialize, Serialize};
 use slashing_protection::interchange::Interchange;
-use types::PublicKeyBytes;
+use types::{Address, PublicKeyBytes};
+
+#[derive(Debug, Deserialize, Serialize, PartialEq)]
+pub struct GetFeeRecipientResponse {
+    pub pubkey: PublicKeyBytes,
+    pub ethaddress: Address,
+}
 
 #[derive(Debug, Deserialize, Serialize, PartialEq)]
 pub struct AuthResponse {

common/eth2/src/lighthouse_vc/types.rs

@@ -97,3 +97,8 @@ pub struct Web3SignerValidatorRequest {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub client_identity_password: Option<String>,
 }
+
+#[derive(Debug, Clone, PartialEq, Deserialize, Serialize)]
+pub struct UpdateFeeRecipientRequest {
+    pub ethaddress: Address,
+}

common/warp_utils/src/reject.rs

@@ -205,8 +205,13 @@ pub async fn handle_rejection(err: warp::Rejection) -> Result<impl warp::Reply,
         code = StatusCode::FORBIDDEN;
         message = format!("FORBIDDEN: Invalid auth token: {}", e.0);
     } else if let Some(e) = err.find::<warp::reject::MissingHeader>() {
-        code = StatusCode::BAD_REQUEST;
+        if e.name().eq("Authorization") {
-        message = format!("BAD_REQUEST: missing {} header", e.name());
+            code = StatusCode::UNAUTHORIZED;
+            message = "UNAUTHORIZED: missing Authorization header".to_string();
+        } else {
+            code = StatusCode::BAD_REQUEST;
+            message = format!("BAD_REQUEST: missing {} header", e.name());
+        }
     } else if let Some(e) = err.find::<warp::reject::InvalidHeader>() {
         code = StatusCode::BAD_REQUEST;
         message = format!("BAD_REQUEST: invalid {} header", e.name());

lighthouse/tests/validator_client.rs

@@ -249,66 +249,6 @@ fn fee_recipient_flag() {
             )
         });
 }
-#[test]
-fn fee_recipient_file_flag() {
-    let dir = TempDir::new().expect("Unable to create temporary directory");
-    let mut file =
-        File::create(dir.path().join("fee_recipient.txt")).expect("Unable to create file");
-    let new_key = Keypair::random();
-    let pubkeybytes = PublicKeyBytes::from(new_key.pk);
-    let contents = "default:0x00000000219ab540356cbb839cbe05303d7705fa";
-    file.write_all(contents.as_bytes())
-        .expect("Unable to write to file");
-    CommandLineTest::new()
-        .flag(
-            "suggested-fee-recipient-file",
-            dir.path().join("fee_recipient.txt").as_os_str().to_str(),
-        )
-        .run()
-        .with_config(|config| {
-            // Public key not present so load default.
-            assert_eq!(
-                config
-                    .fee_recipient_file
-                    .clone()
-                    .unwrap()
-                    .load_fee_recipient(&pubkeybytes)
-                    .unwrap(),
-                Some(Address::from_str("0x00000000219ab540356cbb839cbe05303d7705fa").unwrap())
-            )
-        });
-}
-#[test]
-fn fee_recipient_file_with_pk_flag() {
-    let dir = TempDir::new().expect("Unable to create temporary directory");
-    let mut file =
-        File::create(dir.path().join("fee_recipient.txt")).expect("Unable to create file");
-    let new_key = Keypair::random();
-    let pubkeybytes = PublicKeyBytes::from(new_key.pk);
-    let contents = format!(
-        "{}:0x00000000219ab540356cbb839cbe05303d7705fa",
-        pubkeybytes.to_string()
-    );
-    file.write_all(contents.as_bytes())
-        .expect("Unable to write to file");
-    CommandLineTest::new()
-        .flag(
-            "suggested-fee-recipient-file",
-            dir.path().join("fee_recipient.txt").as_os_str().to_str(),
-        )
-        .run()
-        .with_config(|config| {
-            assert_eq!(
-                config
-                    .fee_recipient_file
-                    .clone()
-                    .unwrap()
-                    .load_fee_recipient(&pubkeybytes)
-                    .unwrap(),
-                Some(Address::from_str("0x00000000219ab540356cbb839cbe05303d7705fa").unwrap())
-            )
-        });
-}
 
 // Tests for HTTP flags.
 #[test]

testing/web3signer_tests/src/lib.rs

@@ -310,6 +310,7 @@ mod tests {
                 spec,
                 None,
                 slot_clock,
+                None,
                 executor,
                 log.clone(),
             );

validator_client/src/cli.rs

@@ -136,14 +136,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
                 .value_name("FEE-RECIPIENT")
                 .takes_value(true)
         )
-        .arg(
-            Arg::with_name("suggested-fee-recipient-file")
-                .long("suggested-fee-recipient-file")
-                .help("The fallback address provided to the BN if nothing suitable is found \
-                           in the validator definitions.")
-                .value_name("FEE-RECIPIENT-FILE")
-                .takes_value(true)
-        )
         /* REST API related arguments */
         .arg(
             Arg::with_name("http")

validator_client/src/config.rs

@@ -1,4 +1,3 @@
-use crate::fee_recipient_file::FeeRecipientFile;
 use crate::graffiti_file::GraffitiFile;
 use crate::{http_api, http_metrics};
 use clap::ArgMatches;
@@ -44,8 +43,6 @@ pub struct Config {
     pub graffiti_file: Option<GraffitiFile>,
     /// Fallback fallback address.
     pub fee_recipient: Option<Address>,
-    /// Fee recipient file to load per validator suggested-fee-recipients.
-    pub fee_recipient_file: Option<FeeRecipientFile>,
     /// Configuration for the HTTP REST API.
     pub http_api: http_api::Config,
     /// Configuration for the HTTP REST API.
@@ -86,7 +83,6 @@ impl Default for Config {
             graffiti: None,
             graffiti_file: None,
             fee_recipient: None,
-            fee_recipient_file: None,
             http_api: <_>::default(),
             http_metrics: <_>::default(),
             monitoring_api: None,
@@ -206,19 +202,6 @@ impl Config {
             }
         }
 
-        if let Some(fee_recipient_file_path) = cli_args.value_of("suggested-fee-recipient-file") {
-            let mut fee_recipient_file = FeeRecipientFile::new(fee_recipient_file_path.into());
-            fee_recipient_file
-                .read_fee_recipient_file()
-                .map_err(|e| format!("Error reading suggested-fee-recipient file: {:?}", e))?;
-            config.fee_recipient_file = Some(fee_recipient_file);
-            info!(
-                log,
-                "Successfully loaded suggested-fee-recipient file";
-                "path" => fee_recipient_file_path
-            );
-        }
-
         if let Some(input_fee_recipient) =
             parse_optional::<Address>(cli_args, "suggested-fee-recipient")?
         {

validator_client/src/fee_recipient_file.rs

@@ -1,184 +0,0 @@
-use serde_derive::{Deserialize, Serialize};
-use std::collections::HashMap;
-use std::fs::File;
-use std::io::{prelude::*, BufReader};
-use std::path::PathBuf;
-use std::str::FromStr;
-
-use bls::PublicKeyBytes;
-use types::Address;
-
-#[derive(Debug)]
-#[allow(clippy::enum_variant_names)]
-pub enum Error {
-    InvalidFile(std::io::Error),
-    InvalidLine(String),
-    InvalidPublicKey(String),
-    InvalidFeeRecipient(String),
-}
-
-/// Struct to load validator fee-recipients from file.
-/// The fee-recipient file is expected to have the following structure
-///
-/// default: 0x00000000219ab540356cbb839cbe05303d7705fa
-/// public_key1: fee-recipient1
-/// public_key2: fee-recipient2
-/// ...
-#[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct FeeRecipientFile {
-    fee_recipient_path: PathBuf,
-    fee_recipients: HashMap<PublicKeyBytes, Address>,
-    default: Option<Address>,
-}
-
-impl FeeRecipientFile {
-    pub fn new(fee_recipient_path: PathBuf) -> Self {
-        Self {
-            fee_recipient_path,
-            fee_recipients: HashMap::new(),
-            default: None,
-        }
-    }
-
-    /// Returns the fee-recipient corresponding to the given public key if present, else returns the
-    /// default fee-recipient.
-    ///
-    /// Returns an error if loading from the fee-recipient file fails.
-    pub fn get_fee_recipient(&self, public_key: &PublicKeyBytes) -> Result<Option<Address>, Error> {
-        Ok(self
-            .fee_recipients
-            .get(public_key)
-            .copied()
-            .or(self.default))
-    }
-
-    /// Loads the fee-recipient file and populates the default fee-recipient and `fee_recipients` hashmap.
-    /// Returns the fee-recipient corresponding to the given public key if present, else returns the
-    /// default fee-recipient.
-    ///
-    /// Returns an error if loading from the fee-recipient file fails.
-    pub fn load_fee_recipient(
-        &mut self,
-        public_key: &PublicKeyBytes,
-    ) -> Result<Option<Address>, Error> {
-        self.read_fee_recipient_file()?;
-        Ok(self
-            .fee_recipients
-            .get(public_key)
-            .copied()
-            .or(self.default))
-    }
-
-    /// Reads from a fee-recipient file with the specified format and populates the default value
-    /// and the hashmap.
-    ///
-    /// Returns an error if the file does not exist, or if the format is invalid.
-    pub fn read_fee_recipient_file(&mut self) -> Result<(), Error> {
-        let file = File::open(self.fee_recipient_path.as_path()).map_err(Error::InvalidFile)?;
-        let reader = BufReader::new(file);
-
-        let lines = reader.lines();
-
-        self.default = None;
-        self.fee_recipients.clear();
-
-        for line in lines {
-            let line = line.map_err(|e| Error::InvalidLine(e.to_string()))?;
-            let (pk_opt, fee_recipient) = read_line(&line)?;
-            match pk_opt {
-                Some(pk) => {
-                    self.fee_recipients.insert(pk, fee_recipient);
-                }
-                None => self.default = Some(fee_recipient),
-            }
-        }
-        Ok(())
-    }
-}
-
-/// Parses a line from the fee-recipient file.
-///
-/// `Ok((None, fee_recipient))` represents the fee-recipient for the default key.
-/// `Ok((Some(pk), fee_recipient))` represents fee-recipient for the public key `pk`.
-/// Returns an error if the line is in the wrong format or does not contain a valid public key or fee-recipient.
-fn read_line(line: &str) -> Result<(Option<PublicKeyBytes>, Address), Error> {
-    if let Some(i) = line.find(':') {
-        let (key, value) = line.split_at(i);
-        // Note: `value.len() >=1` so `value[1..]` is safe
-        let fee_recipient = Address::from_str(value[1..].trim())
-            .map_err(|e| Error::InvalidFeeRecipient(e.to_string()))?;
-        if key == "default" {
-            Ok((None, fee_recipient))
-        } else {
-            let pk = PublicKeyBytes::from_str(key).map_err(Error::InvalidPublicKey)?;
-            Ok((Some(pk), fee_recipient))
-        }
-    } else {
-        Err(Error::InvalidLine(format!("Missing delimiter: {}", line)))
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use bls::Keypair;
-    use std::io::LineWriter;
-    use tempfile::TempDir;
-
-    const DEFAULT_FEE_RECIPIENT: &str = "0x00000000219ab540356cbb839cbe05303d7705fa";
-    const CUSTOM_FEE_RECIPIENT1: &str = "0x4242424242424242424242424242424242424242";
-    const CUSTOM_FEE_RECIPIENT2: &str = "0x0000000000000000000000000000000000000001";
-    const PK1: &str = "0x800012708dc03f611751aad7a43a082142832b5c1aceed07ff9b543cf836381861352aa923c70eeb02018b638aa306aa";
-    const PK2: &str = "0x80001866ce324de7d80ec73be15e2d064dcf121adf1b34a0d679f2b9ecbab40ce021e03bb877e1a2fe72eaaf475e6e21";
-
-    // Create a fee-recipient file in the required format and return a path to the file.
-    fn create_fee_recipient_file() -> PathBuf {
-        let temp = TempDir::new().unwrap();
-        let pk1 = PublicKeyBytes::deserialize(&hex::decode(&PK1[2..]).unwrap()).unwrap();
-        let pk2 = PublicKeyBytes::deserialize(&hex::decode(&PK2[2..]).unwrap()).unwrap();
-
-        let file_name = temp.into_path().join("fee_recipient.txt");
-
-        let file = File::create(&file_name).unwrap();
-        let mut fee_recipient_file = LineWriter::new(file);
-        fee_recipient_file
-            .write_all(format!("default: {}\n", DEFAULT_FEE_RECIPIENT).as_bytes())
-            .unwrap();
-        fee_recipient_file
-            .write_all(format!("{}: {}\n", pk1.as_hex_string(), CUSTOM_FEE_RECIPIENT1).as_bytes())
-            .unwrap();
-        fee_recipient_file
-            .write_all(format!("{}: {}\n", pk2.as_hex_string(), CUSTOM_FEE_RECIPIENT2).as_bytes())
-            .unwrap();
-        fee_recipient_file.flush().unwrap();
-        file_name
-    }
-
-    #[test]
-    fn test_load_fee_recipient() {
-        let fee_recipient_file_path = create_fee_recipient_file();
-        let mut gf = FeeRecipientFile::new(fee_recipient_file_path);
-
-        let pk1 = PublicKeyBytes::deserialize(&hex::decode(&PK1[2..]).unwrap()).unwrap();
-        let pk2 = PublicKeyBytes::deserialize(&hex::decode(&PK2[2..]).unwrap()).unwrap();
-
-        // Read once
-        gf.read_fee_recipient_file().unwrap();
-
-        assert_eq!(
-            gf.load_fee_recipient(&pk1).unwrap().unwrap(),
-            Address::from_str(CUSTOM_FEE_RECIPIENT1).unwrap()
-        );
-        assert_eq!(
-            gf.load_fee_recipient(&pk2).unwrap().unwrap(),
-            Address::from_str(CUSTOM_FEE_RECIPIENT2).unwrap()
-        );
-
-        // Random pk should return the default fee-recipient
-        let random_pk = Keypair::random().pk.compress();
-        assert_eq!(
-            gf.load_fee_recipient(&random_pk).unwrap().unwrap(),
-            Address::from_str(DEFAULT_FEE_RECIPIENT).unwrap()
-        );
-    }
-}

validator_client/src/http_api/mod.rs

@@ -9,10 +9,11 @@ use account_utils::{
     mnemonic_from_phrase,
     validator_definitions::{SigningDefinition, ValidatorDefinition},
 };
+pub use api_secret::ApiSecret;
 use create_validator::{create_validators_mnemonic, create_validators_web3signer};
 use eth2::lighthouse_vc::{
-    std_types::AuthResponse,
+    std_types::{AuthResponse, GetFeeRecipientResponse},
-    types::{self as api_types, PublicKey, PublicKeyBytes},
+    types::{self as api_types, GenericResponse, PublicKey, PublicKeyBytes},
 };
 use lighthouse_version::version_with_platform;
 use serde::{Deserialize, Serialize};
@@ -35,8 +36,6 @@ use warp::{
     Filter,
 };
 
-pub use api_secret::ApiSecret;
-
 #[derive(Debug)]
 pub enum Error {
     Warp(warp::Error),
@@ -562,6 +561,123 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
     let std_keystores = eth_v1.and(warp::path("keystores")).and(warp::path::end());
     let std_remotekeys = eth_v1.and(warp::path("remotekeys")).and(warp::path::end());
 
+    // GET /eth/v1/validator/{pubkey}/feerecipient
+    let get_fee_recipient = eth_v1
+        .and(warp::path("validator"))
+        .and(warp::path::param::<PublicKey>())
+        .and(warp::path("feerecipient"))
+        .and(warp::path::end())
+        .and(validator_store_filter.clone())
+        .and(signer.clone())
+        .and_then(
+            |validator_pubkey: PublicKey, validator_store: Arc<ValidatorStore<T, E>>, signer| {
+                blocking_signed_json_task(signer, move || {
+                    if validator_store
+                        .initialized_validators()
+                        .read()
+                        .is_enabled(&validator_pubkey)
+                        .is_none()
+                    {
+                        return Err(warp_utils::reject::custom_not_found(format!(
+                            "no validator found with pubkey {:?}",
+                            validator_pubkey
+                        )));
+                    }
+                    validator_store
+                        .get_fee_recipient(&PublicKeyBytes::from(&validator_pubkey))
+                        .map(|fee_recipient| {
+                            GenericResponse::from(GetFeeRecipientResponse {
+                                pubkey: PublicKeyBytes::from(validator_pubkey.clone()),
+                                ethaddress: fee_recipient,
+                            })
+                        })
+                        .ok_or_else(|| {
+                            warp_utils::reject::custom_server_error(
+                                "no fee recipient set".to_string(),
+                            )
+                        })
+                })
+            },
+        );
+
+    // POST /eth/v1/validator/{pubkey}/feerecipient
+    let post_fee_recipient = eth_v1
+        .and(warp::path("validator"))
+        .and(warp::path::param::<PublicKey>())
+        .and(warp::body::json())
+        .and(warp::path("feerecipient"))
+        .and(warp::path::end())
+        .and(validator_store_filter.clone())
+        .and(signer.clone())
+        .and_then(
+            |validator_pubkey: PublicKey,
+             request: api_types::UpdateFeeRecipientRequest,
+             validator_store: Arc<ValidatorStore<T, E>>,
+             signer| {
+                blocking_signed_json_task(signer, move || {
+                    if validator_store
+                        .initialized_validators()
+                        .read()
+                        .is_enabled(&validator_pubkey)
+                        .is_none()
+                    {
+                        return Err(warp_utils::reject::custom_not_found(format!(
+                            "no validator found with pubkey {:?}",
+                            validator_pubkey
+                        )));
+                    }
+                    validator_store
+                        .initialized_validators()
+                        .write()
+                        .set_validator_fee_recipient(&validator_pubkey, request.ethaddress)
+                        .map_err(|e| {
+                            warp_utils::reject::custom_server_error(format!(
+                                "Error persisting fee recipient: {:?}",
+                                e
+                            ))
+                        })
+                })
+            },
+        )
+        .map(|reply| warp::reply::with_status(reply, warp::http::StatusCode::ACCEPTED));
+
+    // DELETE /eth/v1/validator/{pubkey}/feerecipient
+    let delete_fee_recipient = eth_v1
+        .and(warp::path("validator"))
+        .and(warp::path::param::<PublicKey>())
+        .and(warp::path("feerecipient"))
+        .and(warp::path::end())
+        .and(validator_store_filter.clone())
+        .and(signer.clone())
+        .and_then(
+            |validator_pubkey: PublicKey, validator_store: Arc<ValidatorStore<T, E>>, signer| {
+                blocking_signed_json_task(signer, move || {
+                    if validator_store
+                        .initialized_validators()
+                        .read()
+                        .is_enabled(&validator_pubkey)
+                        .is_none()
+                    {
+                        return Err(warp_utils::reject::custom_not_found(format!(
+                            "no validator found with pubkey {:?}",
+                            validator_pubkey
+                        )));
+                    }
+                    validator_store
+                        .initialized_validators()
+                        .write()
+                        .delete_validator_fee_recipient(&validator_pubkey)
+                        .map_err(|e| {
+                            warp_utils::reject::custom_server_error(format!(
+                                "Error persisting fee recipient removal: {:?}",
+                                e
+                            ))
+                        })
+                })
+            },
+        )
+        .map(|reply| warp::reply::with_status(reply, warp::http::StatusCode::NO_CONTENT));
+
     // GET /eth/v1/keystores
     let get_std_keystores = std_keystores
         .and(signer.clone())
@@ -647,6 +763,7 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
                         .or(get_lighthouse_spec)
                         .or(get_lighthouse_validators)
                         .or(get_lighthouse_validators_pubkey)
+                        .or(get_fee_recipient)
                         .or(get_std_keystores)
                         .or(get_std_remotekeys),
                 )
@@ -655,11 +772,16 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
                         .or(post_validators_keystore)
                         .or(post_validators_mnemonic)
                         .or(post_validators_web3signer)
+                        .or(post_fee_recipient)
                         .or(post_std_keystores)
                         .or(post_std_remotekeys),
                 ))
                 .or(warp::patch().and(patch_validators))
-                .or(warp::delete().and(delete_std_keystores.or(delete_std_remotekeys))),
+                .or(warp::delete().and(
+                    delete_fee_recipient
+                        .or(delete_std_keystores)
+                        .or(delete_std_remotekeys),
+                )),
         )
         // The auth route is the only route that is allowed to be accessed without the API token.
         .or(warp::get().and(get_auth))

validator_client/src/http_api/tests.rs

@@ -36,6 +36,7 @@ use tokio::runtime::Runtime;
 use tokio::sync::oneshot;
 
 const PASSWORD_BYTES: &[u8] = &[42, 50, 37];
+pub const TEST_DEFAULT_FEE_RECIPIENT: Address = Address::repeat_byte(42);
 
 type E = MainnetEthSpec;
 
@@ -102,6 +103,7 @@ impl ApiTester {
             spec,
             Some(Arc::new(DoppelgangerService::new(log.clone()))),
             slot_clock,
+            Some(TEST_DEFAULT_FEE_RECIPIENT),
             executor.clone(),
             log.clone(),
         ));
@@ -185,7 +187,7 @@ impl ApiTester {
         missing_token_client.send_authorization_header(false);
         match func(missing_token_client).await {
             Err(ApiError::ServerMessage(ApiErrorMessage {
-                code: 400, message, ..
+                code: 401, message, ..
             })) if message.contains("missing Authorization header") => (),
             Err(other) => panic!("expected missing header error, got {:?}", other),
             Ok(_) => panic!("expected missing header error, got Ok"),

validator_client/src/http_api/tests/keystores.rs

@@ -1,5 +1,7 @@
 use super::*;
 use account_utils::random_password_string;
+use bls::PublicKeyBytes;
+use eth2::lighthouse_vc::types::UpdateFeeRecipientRequest;
 use eth2::lighthouse_vc::{
     http_client::ValidatorClientHttpClient as HttpClient,
     std_types::{KeystoreJsonStr as Keystore, *},
@@ -9,6 +11,7 @@ use itertools::Itertools;
 use rand::{rngs::SmallRng, Rng, SeedableRng};
 use slashing_protection::interchange::{Interchange, InterchangeMetadata};
 use std::{collections::HashMap, path::Path};
+use types::Address;
 
 fn new_keystore(password: ZeroizeString) -> Keystore {
     let keypair = Keypair::random();
@@ -585,6 +588,185 @@ fn import_invalid_slashing_protection() {
     })
 }
 
+#[test]
+fn check_get_set_fee_recipient() {
+    run_test(|tester: ApiTester| async move {
+        let _ = &tester;
+        let password = random_password_string();
+        let keystores = (0..3)
+            .map(|_| new_keystore(password.clone()))
+            .collect::<Vec<_>>();
+        let all_pubkeys = keystores.iter().map(keystore_pubkey).collect::<Vec<_>>();
+
+        let import_res = tester
+            .client
+            .post_keystores(&ImportKeystoresRequest {
+                keystores: keystores.clone(),
+                passwords: vec![password.clone(); keystores.len()],
+                slashing_protection: None,
+            })
+            .await
+            .unwrap();
+
+        // All keystores should be imported.
+        check_keystore_import_response(&import_res, all_imported(keystores.len()));
+
+        // Check that GET lists all the imported keystores.
+        let get_res = tester.client.get_keystores().await.unwrap();
+        check_keystore_get_response(&get_res, &keystores);
+
+        // Before setting anything, every fee recipient should be set to TEST_DEFAULT_FEE_RECIPIENT
+        for pubkey in &all_pubkeys {
+            let get_res = tester
+                .client
+                .get_fee_recipient(pubkey)
+                .await
+                .expect("should get fee recipient");
+            assert_eq!(
+                get_res,
+                GetFeeRecipientResponse {
+                    pubkey: pubkey.clone(),
+                    ethaddress: TEST_DEFAULT_FEE_RECIPIENT,
+                }
+            );
+        }
+
+        use std::str::FromStr;
+        let fee_recipient_public_key_1 =
+            Address::from_str("0x25c4a76E7d118705e7Ea2e9b7d8C59930d8aCD3b").unwrap();
+        let fee_recipient_public_key_2 =
+            Address::from_str("0x0000000000000000000000000000000000000001").unwrap();
+        let fee_recipient_override =
+            Address::from_str("0x0123456789abcdef0123456789abcdef01234567").unwrap();
+
+        // set the fee recipient for pubkey[1] using the API
+        tester
+            .client
+            .post_fee_recipient(
+                &all_pubkeys[1],
+                &UpdateFeeRecipientRequest {
+                    ethaddress: fee_recipient_public_key_1.clone(),
+                },
+            )
+            .await
+            .expect("should update fee recipient");
+        // now everything but pubkey[1] should be TEST_DEFAULT_FEE_RECIPIENT
+        for (i, pubkey) in all_pubkeys.iter().enumerate() {
+            let get_res = tester
+                .client
+                .get_fee_recipient(pubkey)
+                .await
+                .expect("should get fee recipient");
+            let expected = if i == 1 {
+                fee_recipient_public_key_1.clone()
+            } else {
+                TEST_DEFAULT_FEE_RECIPIENT
+            };
+            assert_eq!(
+                get_res,
+                GetFeeRecipientResponse {
+                    pubkey: pubkey.clone(),
+                    ethaddress: expected,
+                }
+            );
+        }
+
+        // set the fee recipient for pubkey[2] using the API
+        tester
+            .client
+            .post_fee_recipient(
+                &all_pubkeys[2],
+                &UpdateFeeRecipientRequest {
+                    ethaddress: fee_recipient_public_key_2.clone(),
+                },
+            )
+            .await
+            .expect("should update fee recipient");
+        // now everything but pubkey[1] & pubkey[2] should be fee_recipient_file_default
+        for (i, pubkey) in all_pubkeys.iter().enumerate() {
+            let get_res = tester
+                .client
+                .get_fee_recipient(pubkey)
+                .await
+                .expect("should get fee recipient");
+            let expected = if i == 1 {
+                fee_recipient_public_key_1.clone()
+            } else if i == 2 {
+                fee_recipient_public_key_2.clone()
+            } else {
+                TEST_DEFAULT_FEE_RECIPIENT
+            };
+            assert_eq!(
+                get_res,
+                GetFeeRecipientResponse {
+                    pubkey: pubkey.clone(),
+                    ethaddress: expected,
+                }
+            );
+        }
+
+        // should be able to override previous fee_recipient
+        tester
+            .client
+            .post_fee_recipient(
+                &all_pubkeys[1],
+                &UpdateFeeRecipientRequest {
+                    ethaddress: fee_recipient_override.clone(),
+                },
+            )
+            .await
+            .expect("should update fee recipient");
+        for (i, pubkey) in all_pubkeys.iter().enumerate() {
+            let get_res = tester
+                .client
+                .get_fee_recipient(pubkey)
+                .await
+                .expect("should get fee recipient");
+            let expected = if i == 1 {
+                fee_recipient_override.clone()
+            } else if i == 2 {
+                fee_recipient_public_key_2.clone()
+            } else {
+                TEST_DEFAULT_FEE_RECIPIENT
+            };
+            assert_eq!(
+                get_res,
+                GetFeeRecipientResponse {
+                    pubkey: pubkey.clone(),
+                    ethaddress: expected,
+                }
+            );
+        }
+
+        // delete fee recipient for pubkey[1] using the API
+        tester
+            .client
+            .delete_fee_recipient(&all_pubkeys[1])
+            .await
+            .expect("should delete fee recipient");
+        // now everything but pubkey[2] should be TEST_DEFAULT_FEE_RECIPIENT
+        for (i, pubkey) in all_pubkeys.iter().enumerate() {
+            let get_res = tester
+                .client
+                .get_fee_recipient(pubkey)
+                .await
+                .expect("should get fee recipient");
+            let expected = if i == 2 {
+                fee_recipient_public_key_2.clone()
+            } else {
+                TEST_DEFAULT_FEE_RECIPIENT
+            };
+            assert_eq!(
+                get_res,
+                GetFeeRecipientResponse {
+                    pubkey: pubkey.clone(),
+                    ethaddress: expected,
+                }
+            );
+        }
+    })
+}
+
 fn all_indices(count: usize) -> Vec<usize> {
     (0..count).collect()
 }

validator_client/src/initialized_validators.rs

@@ -617,6 +617,78 @@ impl InitializedValidators {
         Ok(())
     }
 
+    /// Sets the `InitializedValidator` and `ValidatorDefinition` `suggested_fee_recipient` values.
+    ///
+    /// ## Notes
+    ///
+    /// Setting a validator `fee_recipient` will cause `self.definitions` to be updated and saved to
+    /// disk.
+    ///
+    /// Saves the `ValidatorDefinitions` to file, even if no definitions were changed.
+    pub fn set_validator_fee_recipient(
+        &mut self,
+        voting_public_key: &PublicKey,
+        fee_recipient: Address,
+    ) -> Result<(), Error> {
+        if let Some(def) = self
+            .definitions
+            .as_mut_slice()
+            .iter_mut()
+            .find(|def| def.voting_public_key == *voting_public_key)
+        {
+            def.suggested_fee_recipient = Some(fee_recipient);
+        }
+
+        if let Some(val) = self
+            .validators
+            .get_mut(&PublicKeyBytes::from(voting_public_key))
+        {
+            val.suggested_fee_recipient = Some(fee_recipient);
+        }
+
+        self.definitions
+            .save(&self.validators_dir)
+            .map_err(Error::UnableToSaveDefinitions)?;
+
+        Ok(())
+    }
+
+    /// Removes the `InitializedValidator` and `ValidatorDefinition` `suggested_fee_recipient` values.
+    ///
+    /// ## Notes
+    ///
+    /// Removing a validator `fee_recipient` will cause `self.definitions` to be updated and saved to
+    /// disk. The fee_recipient for the validator will then fall back to the process level default if
+    /// it is set.
+    ///
+    /// Saves the `ValidatorDefinitions` to file, even if no definitions were changed.
+    pub fn delete_validator_fee_recipient(
+        &mut self,
+        voting_public_key: &PublicKey,
+    ) -> Result<(), Error> {
+        if let Some(def) = self
+            .definitions
+            .as_mut_slice()
+            .iter_mut()
+            .find(|def| def.voting_public_key == *voting_public_key)
+        {
+            def.suggested_fee_recipient = None;
+        }
+
+        if let Some(val) = self
+            .validators
+            .get_mut(&PublicKeyBytes::from(voting_public_key))
+        {
+            val.suggested_fee_recipient = None;
+        }
+
+        self.definitions
+            .save(&self.validators_dir)
+            .map_err(Error::UnableToSaveDefinitions)?;
+
+        Ok(())
+    }
+
     /// Tries to decrypt the key cache.
     ///
     /// Returns the decrypted cache if decryption was successful, or an error if a required password

validator_client/src/lib.rs

@@ -5,7 +5,6 @@ mod check_synced;
 mod cli;
 mod config;
 mod duties_service;
-mod fee_recipient_file;
 mod graffiti_file;
 mod http_metrics;
 mod key_cache;
@@ -360,6 +359,7 @@ impl<T: EthSpec> ProductionValidatorClient<T> {
             context.eth2_config.spec.clone(),
             doppelganger_service.clone(),
             slot_clock.clone(),
+            config.fee_recipient,
             context.executor.clone(),
             log.clone(),
         ));
@@ -426,8 +426,6 @@ impl<T: EthSpec> ProductionValidatorClient<T> {
             .validator_store(validator_store.clone())
             .beacon_nodes(beacon_nodes.clone())
             .runtime_context(context.service_context("preparation".into()))
-            .fee_recipient(config.fee_recipient)
-            .fee_recipient_file(config.fee_recipient_file.clone())
             .build()?;
 
         let sync_committee_service = SyncCommitteeService::new(

validator_client/src/preparation_service.rs

@@ -1,8 +1,5 @@
 use crate::beacon_node_fallback::{BeaconNodeFallback, RequireSynced};
-use crate::{
+use crate::validator_store::{DoppelgangerStatus, ValidatorStore};
-    fee_recipient_file::FeeRecipientFile,
-    validator_store::{DoppelgangerStatus, ValidatorStore},
-};
 use bls::PublicKeyBytes;
 use environment::RuntimeContext;
 use parking_lot::RwLock;
@@ -31,8 +28,6 @@ pub struct PreparationServiceBuilder<T: SlotClock + 'static, E: EthSpec> {
     slot_clock: Option<T>,
     beacon_nodes: Option<Arc<BeaconNodeFallback<T, E>>>,
     context: Option<RuntimeContext<E>>,
-    fee_recipient: Option<Address>,
-    fee_recipient_file: Option<FeeRecipientFile>,
 }
 
 impl<T: SlotClock + 'static, E: EthSpec> PreparationServiceBuilder<T, E> {
@@ -42,8 +37,6 @@ impl<T: SlotClock + 'static, E: EthSpec> PreparationServiceBuilder<T, E> {
             slot_clock: None,
             beacon_nodes: None,
             context: None,
-            fee_recipient: None,
-            fee_recipient_file: None,
         }
     }
 
@@ -67,16 +60,6 @@ impl<T: SlotClock + 'static, E: EthSpec> PreparationServiceBuilder<T, E> {
         self
     }
 
-    pub fn fee_recipient(mut self, fee_recipient: Option<Address>) -> Self {
-        self.fee_recipient = fee_recipient;
-        self
-    }
-
-    pub fn fee_recipient_file(mut self, fee_recipient_file: Option<FeeRecipientFile>) -> Self {
-        self.fee_recipient_file = fee_recipient_file;
-        self
-    }
-
     pub fn build(self) -> Result<PreparationService<T, E>, String> {
         Ok(PreparationService {
             inner: Arc::new(Inner {
@@ -92,8 +75,6 @@ impl<T: SlotClock + 'static, E: EthSpec> PreparationServiceBuilder<T, E> {
                 context: self
                     .context
                     .ok_or("Cannot build PreparationService without runtime_context")?,
-                fee_recipient: self.fee_recipient,
-                fee_recipient_file: self.fee_recipient_file,
                 validator_registration_cache: RwLock::new(HashMap::new()),
             }),
         })
@@ -106,8 +87,6 @@ pub struct Inner<T, E: EthSpec> {
     slot_clock: T,
     beacon_nodes: Arc<BeaconNodeFallback<T, E>>,
     context: RuntimeContext<E>,
-    fee_recipient: Option<Address>,
-    fee_recipient_file: Option<FeeRecipientFile>,
     // Used to track unpublished validator registration changes.
     validator_registration_cache:
         RwLock<HashMap<ValidatorRegistrationKey, SignedValidatorRegistrationData>>,
@@ -301,23 +280,6 @@ impl<T: SlotClock + 'static, E: EthSpec> PreparationService<T, E> {
     {
         let log = self.context.log();
 
-        let fee_recipient_file = self
-            .fee_recipient_file
-            .clone()
-            .map(|mut fee_recipient_file| {
-                fee_recipient_file
-                    .read_fee_recipient_file()
-                    .map_err(|e| {
-                        error!(
-                            log,
-                            "Error loading fee-recipient file";
-                            "error" => ?e
-                        );
-                    })
-                    .unwrap_or(());
-                fee_recipient_file
-            });
-
         let all_pubkeys: Vec<_> = self
             .validator_store
             .voting_pubkeys(DoppelgangerStatus::ignored);
@@ -327,22 +289,7 @@ impl<T: SlotClock + 'static, E: EthSpec> PreparationService<T, E> {
             .filter_map(|pubkey| {
                 // Ignore fee recipients for keys without indices, they are inactive.
                 let validator_index = self.validator_store.validator_index(&pubkey)?;
-
+                let fee_recipient = self.validator_store.get_fee_recipient(&pubkey);
-                // If there is a `suggested_fee_recipient` in the validator definitions yaml
-                // file, use that value.
-                let fee_recipient = self
-                    .validator_store
-                    .suggested_fee_recipient(&pubkey)
-                    .or_else(|| {
-                        // If there's nothing in the validator defs file, check the fee
-                        // recipient file.
-                        fee_recipient_file
-                            .as_ref()?
-                            .get_fee_recipient(&pubkey)
-                            .ok()?
-                    })
-                    // If there's nothing in the file, try the process-level default value.
-                    .or(self.fee_recipient);
 
                 if let Some(fee_recipient) = fee_recipient {
                     Some(map_fn(pubkey, validator_index, fee_recipient))

validator_client/src/validator_store.rs

@@ -86,6 +86,7 @@ pub struct ValidatorStore<T, E: EthSpec> {
     log: Logger,
     doppelganger_service: Option<Arc<DoppelgangerService>>,
     slot_clock: T,
+    fee_recipient_process: Option<Address>,
     task_executor: TaskExecutor,
     _phantom: PhantomData<E>,
 }
@@ -101,6 +102,7 @@ impl<T: SlotClock + 'static, E: EthSpec> ValidatorStore<T, E> {
         spec: ChainSpec,
         doppelganger_service: Option<Arc<DoppelgangerService>>,
         slot_clock: T,
+        fee_recipient_process: Option<Address>,
         task_executor: TaskExecutor,
         log: Logger,
     ) -> Self {
@@ -113,6 +115,7 @@ impl<T: SlotClock + 'static, E: EthSpec> ValidatorStore<T, E> {
             log,
             doppelganger_service,
             slot_clock,
+            fee_recipient_process,
             task_executor,
             _phantom: PhantomData,
         }
@@ -356,7 +359,21 @@ impl<T: SlotClock + 'static, E: EthSpec> ValidatorStore<T, E> {
         self.validators.read().graffiti(validator_pubkey)
     }
 
-    pub fn suggested_fee_recipient(&self, validator_pubkey: &PublicKeyBytes) -> Option<Address> {
+    /// Returns the fee recipient for the given public key. The priority order for fetching
+    /// the fee recipient is:
+    /// 1. validator_definitions.yml
+    /// 2. process level fee recipient
+    pub fn get_fee_recipient(&self, validator_pubkey: &PublicKeyBytes) -> Option<Address> {
+        // If there is a `suggested_fee_recipient` in the validator definitions yaml
+        // file, use that value.
+        self.suggested_fee_recipient(validator_pubkey)
+            // If there's nothing in the file, try the process-level default value.
+            .or(self.fee_recipient_process)
+    }
+
+    /// Returns the suggested_fee_recipient from `validator_definitions.yml` if any.
+    /// This has been pulled into a private function so the read lock is dropped easily
+    fn suggested_fee_recipient(&self, validator_pubkey: &PublicKeyBytes) -> Option<Address> {
         self.validators
             .read()
             .suggested_fee_recipient(validator_pubkey)