[DO NOT MERGE] feat(bolt-sidecar): use pricing model to validate priority fee #599
Conversation
bolt-sidecar/src/state/execution.rs
Outdated
| impl From<pricing::PricingError> for ValidationError { | ||
| fn from(err: pricing::PricingError) -> Self { | ||
| ValidationError::Internal(format!("Pricing error: {}", err)) | ||
| } | ||
| } |
There was a problem hiding this comment.
I think some validation in validate_request() is duplicated in calculate_min_priority_fee() so we might not need this after some deduplication
| let min_priority_fee = | ||
| pricing.calculate_min_priority_fee(tx.gas_limit(), preconfirmed_gas)?; |
There was a problem hiding this comment.
Can this function return an u128?
There was a problem hiding this comment.
Yes it could, but why is it necessary?
For 30M gas the expected result is [613_499_092..19_175_357_339]
There was a problem hiding this comment.
Because on the line below you do a conversion to an u128, but it's not a strong requirement tho
mempirate
changed the title
estensen
changed the title
| # Min priority fee to accept for a commitment | ||
| BOLT_SIDECAR_MIN_PRIORITY_FEE=2000000000 # 2 Gwei = 2 * 10^9 wei | ||
| # Min profit per gas to accept a commitment | ||
| BOLT_SIDECAR_MIN_PROFIT=2000000000 # 2 Gwei = 2 * 10^9 wei |
There was a problem hiding this comment.
This needs to be calculated and based on profit a proposer needs to exceed the "risk free" return of liquid staking on the collateral
| tx.effective_tip_per_gas(max_base_fee).map_or(false, |tip| tip >= min_priority_fee) | ||
| }) | ||
| /// Returns an error if min priority fee cannot be calculated. | ||
| pub fn validate_min_priority_fee( |
There was a problem hiding this comment.
I would move the bulk logic of this function to ExecutionState, and just have this validate that the effective_tip_per_gas is greater than the result of running the pricing algo. So the function can actually remain the same as before.
| template_committed_gas, | ||
| self.limits.min_inclusion_profit, | ||
| max_basefee, | ||
| )? { | ||
| return Err(ValidationError::MaxPriorityFeePerGasTooLow); |
There was a problem hiding this comment.
We should return the value of the minimum fee in this error, so it can be propagated to any consumers
| @@ -777,11 +777,11 @@ Options: | |||
| [env: BOLT_SIDECAR_MAX_COMMITTED_GAS=] | |||
| [default: 10000000] | |||
| --min-priority-fee <MIN_PRIORITY_FEE> | |||
| Min priority fee to accept for a commitment | |||
| --min-inclusion_profit <MIN_INCLUSION_PROFIT> | |||
There was a problem hiding this comment.
Should be --min-inclusion-profit (- instead of _)
| let small_sum_fee_avg = small_fee_sum / 10; | ||
|
|
||
| assert!( | ||
| (big_fee as f64 - small_sum_fee_avg as f64).abs() < 1_000.0, |
| @@ -222,6 +230,7 @@ impl<C: StateFetcher> ExecutionState<C> { | |||
| kzg_settings: EnvKzgSettings::default(), | |||
| // TODO: add a way to configure these values from CLI | |||
| validation_params: ValidationParams::new(gas_limit), | |||
| pricing: PreconfPricing::new(gas_limit), | |||
There was a problem hiding this comment.
Nit but can we rename this to pricer: InclusionPricer? Less ambiguous if we have different preconf types in the future.
Implement pricing model pioneered by Nethermind Research.
The pricing model defines a pricing curve to ensure proposers don't give up any profits when preconfirming a transaciton. Proposers would want profit for the extra work they do and for the "risk free" rate they could get on the 1 ETH collateral they put down with liquid staking. We're currently researching what this profit should be, but for now we default it to 2 GWEI but make it configurable.
In many cases the proposer will outsource the pricing to the Firewall, but this will PR will make sure proposers don't accept a preconf where they lose money.