[fortuna] Automated fee adjustment based on gas prices #1708
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
apps/fortuna/src/keeper.rs
Outdated
| @@ -940,3 +959,195 @@ pub async fn withdraw_fees_if_necessary( | |||
|
|
|||
| Ok(()) | |||
| } | |||
|
|
|||
| #[tracing::instrument(name = "adjust_fee", skip_all, fields())] | |||
There was a problem hiding this comment.
no need to have fields() if you specify skip_all
| @@ -940,3 +959,195 @@ pub async fn withdraw_fees_if_necessary( | |||
|
|
|||
| Ok(()) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
maybe move this logic to a separate module? we can probably have a separate module for each of these maintenance threads
There was a problem hiding this comment.
i would rather do this separately and refactor all of the similar functions here so we don't end up in an inconsistent state
apps/fortuna/src/keeper.rs
Outdated
| .try_into() | ||
| .map_err(|e| anyhow!("gas price doesn't fit into 128 bits. error: {:?}", e))?; | ||
|
|
||
| Ok(gas_used * gas_price) |
There was a problem hiding this comment.
nit (OCD): in this branch it's gas_used * gas_price, in the other branch it's gas_price * gas_used. You can also just return the gas_price from both branch and do the multiplication outside
apps/fortuna/src/keeper.rs
Outdated
| /// - either the fee is increasing or the keeper is earning a profit -- i.e., fees only decrease when the keeper is profitable | ||
| /// - at least one random number has been requested since the last fee update | ||
| /// | ||
| /// These conditions are designed to prevent the |
There was a problem hiding this comment.
This sentence seems a bit unfinished
apps/fortuna/src/keeper.rs
Outdated
| gas_limit: u64, | ||
| min_profit_pct: u64, | ||
| max_profit_pct: u64, | ||
| min_fee: u128, |
There was a problem hiding this comment.
maybe rename this to fixed_min_fee to avoid confusion
| provider_fee, | ||
| target_fee | ||
| ); | ||
| let contract_call = contract.set_provider_fee_as_fee_manager(provider_address, target_fee); |
There was a problem hiding this comment.
I think the what we set the value to should be something between target_fee(which can be renamed to target_fee_min) and target_fee_max. Otherwise, small changes in one direction can trigger updates continuously.
| # on-chain fees to make between [min_profit_pct, max_profit_pct] of the max callback | ||
| # cost in profit per transaction. | ||
| min_profit_pct: 0 | ||
| target_profit_pct: 20 |
There was a problem hiding this comment.
I'm not sure if we want this level of customisation. We can set this as the average of min_profit_pct and max_profit_pct.
| contract.clone(), | ||
| chain_state.provider_address.clone(), | ||
| ADJUST_FEE_INTERVAL, | ||
| chain_eth_config.legacy_tx, |
There was a problem hiding this comment.
maybe just pass the whole config object down?
| /// The maximum percentage profit to earn as a function of the callback cost. | ||
| /// For example, 100 means a profit of 100% over the cost of the callback. | ||
| /// The fee will be lowered if it is more profitable than specified here. | ||
| /// Must be larger than min_profit_pct. |
There was a problem hiding this comment.
we should probably check this min_profit_pct < target_profit_pct < max_profit_pct inequality somewhere. I can imagine a misconfiguration resulting in constant fee updates and draining the fee manager because of gas usage.
This should save us from having to keep dialing up/down the blast fees. I've tried to set up the logic so that it ensures the provider is always profitable and doesn't adjust the fees too often, but we'll probably have to tune this. See inline comment explaining the update logic.