fix: address PR feedback for fee manager/keeper separation

Co-Authored-By: Tejas Badadare <[email protected]>

Author: devin-ai-integration[bot]

apps/fortuna/src/command/run.rs

@@ -3,7 +3,7 @@ use {
         api::{self, ApiBlockChainState, BlockchainState, ChainId},
         chain::ethereum::InstrumentedPythContract,
         command::register_provider::CommitmentMetadata,
-        config::{Commitment, Config, EthereumConfig, ProviderConfig, RunOptions},
+        config::{Commitment, Config, EthereumConfig, KeeperConfig, ProviderConfig, RunOptions},
         eth_utils::traced_client::RpcMetrics,
         history::History,
         keeper::{self, keeper_metrics::KeeperMetrics},
@@ -100,9 +100,6 @@ pub async fn run(opts: &RunOptions) -> Result<()> {
         tracing::info!("Not starting keeper service: no keeper private key specified. Please add one to the config if you would like to run the keeper service.")
     }
 
-    let keeper_replica_config = config.keeper.replica_config.clone();
-    let keeper_run_config = config.keeper.run_config.clone();
-
     let chains: Arc<RwLock<HashMap<ChainId, ApiBlockChainState>>> = Arc::new(RwLock::new(
         config
             .chains
@@ -115,28 +112,16 @@ pub async fn run(opts: &RunOptions) -> Result<()> {
         keeper_metrics.add_chain(chain_id.clone(), config.provider.address);
         let keeper_metrics = keeper_metrics.clone();
         let keeper_private_key_option = keeper_private_key_option.clone();
-        let keeper_replica_config = keeper_replica_config.clone();
-        let keeper_run_config = keeper_run_config.clone();
         let chains = chains.clone();
         let secret_copy = secret.clone();
         let rpc_metrics = rpc_metrics.clone();
         let provider_config = config.provider.clone();
         let history = history.clone();
-        let fee_manager_private_key = config.keeper.fee_manager_private_key.clone();
-        let known_keeper_addresses = config.keeper.known_keeper_addresses.clone();
+        let keeper_config_base = config.keeper.clone();
         spawn(async move {
             loop {
                 let keeper_config = if keeper_private_key_option.is_some() {
-                    Some(crate::config::KeeperConfig {
-                        private_key: crate::config::SecretString {
-                            value: keeper_private_key_option.clone(),
-                            file: None,
-                        },
-                        fee_manager_private_key: fee_manager_private_key.clone(),
-                        known_keeper_addresses: known_keeper_addresses.clone(),
-                        replica_config: keeper_replica_config.clone(),
-                        run_config: keeper_run_config.clone(),
-                    })
+                    Some(keeper_config_base.clone())
                 } else {
                     None
                 };
@@ -195,7 +180,7 @@ async fn setup_chain_and_run_keeper(
     chain_id: &ChainId,
     chain_config: EthereumConfig,
     keeper_metrics: Arc<KeeperMetrics>,
-    keeper_config: Option<crate::config::KeeperConfig>,
+    keeper_config: Option<KeeperConfig>,
     chains: Arc<RwLock<HashMap<ChainId, ApiBlockChainState>>>,
     secret_copy: &str,
     history: Arc<History>,

apps/fortuna/src/config.rs

@@ -385,7 +385,7 @@ pub struct KeeperConfig {
 
     /// The fee manager's private key for fee manager operations.
     /// This key is used to withdraw fees from the contract as the fee manager.
-    /// Multiple replicas can share the same fee manager private key.
+    /// Multiple replicas can share the same fee manager private key but different keeper keys (`private_key`).
     #[serde(default)]
     pub fee_manager_private_key: Option<SecretString>,
 

apps/fortuna/src/keeper.rs

@@ -126,8 +126,17 @@ pub async fn run_keeper_threads(
         let fee_manager_private_key = keeper_config
             .fee_manager_private_key
             .as_ref()
-            .and_then(|key| key.load().ok())
-            .flatten();
+            .ok_or_else(|| {
+                anyhow::anyhow!(
+                    "Fee manager private key is required when fee withdrawal is enabled"
+                )
+            })?
+            .load()?
+            .ok_or_else(|| {
+                anyhow::anyhow!(
+                    "Fee manager private key value is required when fee withdrawal is enabled"
+                )
+            })?;
         spawn(
             withdraw_fees_wrapper(
                 contract.clone(),

apps/fortuna/src/keeper/fee.rs

@@ -8,10 +8,10 @@ use {
     anyhow::{anyhow, Result},
     ethers::{
         middleware::Middleware,
-        signers::Signer,
-        types::{Address, U256},
+        signers::{LocalWallet, Signer},
+        types::{Address, TransactionRequest, U256},
     },
-    std::sync::Arc,
+    std::{str::FromStr, sync::Arc},
     tokio::time::{self, Duration},
     tracing::{self, Instrument},
 };
@@ -57,7 +57,7 @@ pub async fn withdraw_fees_wrapper(
     provider_address: Address,
     poll_interval: Duration,
     min_balance: U256,
-    fee_manager_private_key: Option<String>,
+    fee_manager_private_key: String,
     known_keeper_addresses: Vec<Address>,
 ) {
     loop {
@@ -82,17 +82,12 @@ pub async fn withdraw_fees_if_necessary(
     contract: Arc<InstrumentedSignablePythContract>,
     provider_address: Address,
     min_balance: U256,
-    fee_manager_private_key: Option<String>,
+    fee_manager_private_key: String,
     known_keeper_addresses: Vec<Address>,
 ) -> Result<()> {
     let provider = contract.provider();
     let wallet = contract.wallet();
 
-    let keeper_balance = provider
-        .get_balance(wallet.address(), None)
-        .await
-        .map_err(|e| anyhow!("Error while getting balance. error: {:?}", e))?;
-
     if !should_withdraw_fees(
         Arc::new(provider.clone()),
         wallet.address(),
@@ -103,6 +98,11 @@ pub async fn withdraw_fees_if_necessary(
         return Ok(());
     }
 
+    let keeper_balance = provider
+        .get_balance(wallet.address(), None)
+        .await
+        .map_err(|e| anyhow!("Error while getting balance. error: {:?}", e))?;
+
     let provider_info = contract
         .get_provider_info(provider_address)
         .call()
@@ -114,16 +114,59 @@ pub async fn withdraw_fees_if_necessary(
     if keeper_balance < min_balance && U256::from(fees) > min_balance {
         tracing::info!("Claiming accrued fees...");
 
-        if let Some(_fee_manager_key) = fee_manager_private_key {
-            let contract_call = contract.withdraw_as_fee_manager(provider_address, fees);
-            send_and_confirm(contract_call).await?;
-        } else {
-            if provider_info.fee_manager != wallet.address() {
-                return Err(anyhow!("Fee manager for provider {:?} is not the keeper wallet and no fee manager private key is configured. Fee manager: {:?} Keeper: {:?}", provider_address, provider_info.fee_manager, wallet.address()));
-            }
+        let contract_call = contract.withdraw_as_fee_manager(provider_address, fees);
+        send_and_confirm(contract_call).await?;
+
+        let fee_manager_wallet = LocalWallet::from_str(&fee_manager_private_key)
+            .map_err(|e| anyhow!("Invalid fee manager private key: {:?}", e))?;
+        let fee_manager_address = fee_manager_wallet.address();
+        let keeper_address = wallet.address();
+
+        if fee_manager_address != keeper_address {
+            tracing::info!(
+                "Transferring withdrawn fees from fee manager {:?} to keeper {:?}",
+                fee_manager_address,
+                keeper_address
+            );
+
+            let transfer_amount = U256::from(fees);
+            let chain_id = provider
+                .get_chainid()
+                .await
+                .map_err(|e| anyhow!("Failed to get chain ID: {:?}", e))?;
+
+            let gas_price = provider
+                .get_gas_price()
+                .await
+                .map_err(|e| anyhow!("Failed to get gas price: {:?}", e))?;
+
+            let nonce = provider
+                .get_transaction_count(fee_manager_address, None)
+                .await
+                .map_err(|e| anyhow!("Failed to get nonce: {:?}", e))?;
+
+            let tx = TransactionRequest::new()
+                .to(keeper_address)
+                .value(transfer_amount)
+                .from(fee_manager_address)
+                .gas(21000) // Standard ETH transfer gas limit
+                .gas_price(gas_price)
+                .nonce(nonce)
+                .chain_id(chain_id.as_u64());
+
+            let typed_tx = tx.into();
+            let signature = fee_manager_wallet
+                .sign_transaction(&typed_tx)
+                .await
+                .map_err(|e| anyhow!("Failed to sign transfer transaction: {:?}", e))?;
+
+            let signed_tx = typed_tx.rlp_signed(&signature);
+            let tx_hash = provider
+                .send_raw_transaction(signed_tx)
+                .await
+                .map_err(|e| anyhow!("Failed to send transfer transaction: {:?}", e))?;
 
-            let contract_call = contract.withdraw_as_fee_manager(provider_address, fees);
-            send_and_confirm(contract_call).await?;
+            tracing::info!("Transfer transaction sent: {:?}", tx_hash);
         }
     } else if keeper_balance < min_balance {
         // NOTE: This log message triggers a grafana alert. If you want to change the text, please change the alert also.