BM-1951: refactor DynamicGasFiller and update defaults #1339
Conversation
github-actions
bot
changed the title
| // Using default, as it is requires a lot of duplication to override. | ||
| assert!(alloy::providers::utils::EIP1559_FEE_ESTIMATION_REWARD_PERCENTILE == 20.0); | ||
| assert!(alloy::providers::utils::EIP1559_BASE_FEE_MULTIPLIER == 2); |
There was a problem hiding this comment.
This is very unfortunate that Alloy doesn't enable customizing these easily (and EIP1559_FEE_ESTIMATION_PAST_BLOCKS), as this would give us the clearest way to create these priority modes.
As written we are applying a % multiplier increase to the median of the 20th percentile of priority fees from the past 10 blocks, which is extremely kind of hard to reason about...
I think the ideal design would be for the priority modes to specify the percentile (and possibly num blocks to consider) when generating the values.
Agree too much duplication and code needed for a quick patch fix, but for 1.2 something to consider
| #[serde(rename_all = "snake_case")] | ||
| #[non_exhaustive] | ||
| pub enum PriorityMode { | ||
| /// Low gas price/priority fee. |
There was a problem hiding this comment.
I think its worth explaining what the logic does today a bit more, so that provers understand what customizing it does, focusing in on priority_gas_fee, as given we set max_fee_per_gas with a 2x multiplier on base fee, i don't see how it can ever get hit.
Something like, "By default transactions are sent using the median of the 20th percentile priority_gas_fee from the past 10 blocks. The various priority modes then apply a multiplier on top of this value to compute the final priority_gas_fee for sending the transaction."
"gas price" is basically deprecated at this point, so I think it makes sense to just talk about the impact on max_fee_per_gas and priority_fee_per_gas these modes have.
| /// Configuration for a priority mode. | ||
| #[derive(Clone, Debug)] | ||
| struct PriorityModeConfig { | ||
| /// The base percentage increase applied to every transaction (e.g., 0 = no change, 10 = 10% increase). |
There was a problem hiding this comment.
Specially, this is applied to max_priority_fee_per_gas and max_fee_per_gas
austinabell
requested review from
a team,
Wollac,
capossele,
ec2,
willemolding and
zeroecco
as code owners
After a few iterations, decided to try to keep it as simple as possible, but has tradeoffs like not being able to configure priority fee separately from base fee. Deprecated the ability to set an additional gas to the lock transaction because it previously conflicted with the dynamic gas filler and static amounts don't make too much sense.
lockin_priority_gasforgas_priority_modeconfig