Implement dynamic gas fees for bridge transfers #784
Conversation
0xmovses
requested review from
l-monninger,
andyjsbell,
mzabaluev and
musitdev
as code owners
| @@ -650,11 +650,32 @@ impl GasMaster for EthClient { | |||
| } | |||
|
|
|||
| async fn get_priority_gas_fee_estimate(&self) -> GasMasterResult<u64> { | |||
There was a problem hiding this comment.
It's call priority gas estimate and you return the last block one. I thought you'll do a mean from the latest few blocks. If it's ok, perhaps it should better to call it get_last_block_priority_gas_fee for example.
| let percentage_adjustment = 0.2; // 20% adjustment hardcoded for now | ||
|
|
||
| // Apply the percentage adjustment to base gas price | ||
| let adjusted_base_fee = |
There was a problem hiding this comment.
This part seem a little strange. You multiply an u64 by a U256 and the result is an u64. I think you use U256 because of the precision, but the U256 result is transformed to an u64. Not sure but you should convert the base_gas_price in a U256 and do the calculus on u256 for all and do the conversion to u64 at the end.
Here you let the compiler decide when to do the conversion.
There was a problem hiding this comment.
More generally, we should use checked calculus functions like checked_mul or saturating_mul (if we don't care to detect overflow) for all our calculus.
| @@ -650,11 +650,32 @@ impl GasMaster for EthClient { | |||
| } | |||
|
|
|||
| async fn get_priority_gas_fee_estimate(&self) -> GasMasterResult<u64> { | |||
There was a problem hiding this comment.
Perhaps we should use u128 for all gas and amount value because of the decimal use in Eth for example, you can exceed the u64 limit. I got the issue when calculating the transaction fee for Eth. The code: let transaction_fee_wei = estimate_gas * gas_price; has exceeded u64 during my test.
Summary
protocol-units,networks,scripts,util,cicd, ormisc.Changelog
Testing
Outstanding issues