fix: Min1GweiGasPriceFn to include GweiGasTip with gasFeeCap #23
Conversation
utils/utils.go
Outdated
| func Min1GweiGasPriceFn(suggestedGasTipCap *big.Int, suggestedGasPrice *big.Int, _ int, _ int) (GasFeeCap, GasTipCap) { | ||
| gasFeeCap, gasTipCap := DefaultGasPriceFn(suggestedGasTipCap, suggestedGasPrice, 0, 1) | ||
|
|
||
| GweiGasTip := big.NewInt(1_000_000_000) |
There was a problem hiding this comment.
Could you please rename this to minGasTip and have it as a configurable value so that we can easily change it from the config? Thank you.
utils/utils.go
Outdated
| GweiGasTip := big.NewInt(1_000_000_000) | ||
| if gasTipCap.Cmp(GweiGasTip) < 0 { | ||
| return gasFeeCap, GweiGasTip | ||
| return big.NewInt(0).Add(GweiGasTip, gasFeeCap), GweiGasTip |
There was a problem hiding this comment.
Also here, I think we should:
- replace
gasFeeCapwithminGasTip, since we don't need to add them up - update the
suggestedGasPriceusing the multiplier we added to the ERPC, having a configurable variable there as well.
Thank you.
There was a problem hiding this comment.
gasFeeCap is the max gas fee a transaction can use, which should be always greater than gasTipCap.
This was the reason the tx were deemed invalid.
The multiplier things is already there in DefaultGasPriceFn and HighPriorityGasPriceFn with 1.5 and 3 as the multipliers.
There was a problem hiding this comment.
I hadn't understood what this function was doing. It is actually getting the suggested gas price and suggested gas tip, and returning the maximum between 1Gwei and the suggested gas price, plus the suggested gas tip.
I'm not sure why Konrad had set it this way, but if we want to additionally increase the suggested gas fee cap if it is below the minimum effective priority fee following what we have in the ERPC, we should return the maximum of suggested gas price and 1Gwei and the maximum of suggested gas tip and that minimum priority fee.
I'd suggest rather creating a new function MinSuggestedGasFn and assign those recommended gas values, and maybe for now just keep everything under this function since we are not using the env file in the continuous tests.
There was a problem hiding this comment.
there is env file in continuous test, separately for chiado and mainnet.
The functionality explained in above comment sounds same as what this function is already doing, not sure what new functionality this new function would have.
There was a problem hiding this comment.
The update you made is not equivalent to what we want to achieve, i.e.:
- Suggested Gas Price should have max(suggested gas price, 1_000_000_000 WEI) -> this is to keep the logic from the initial function
- Suggest Gas Tip should have max(suggested gas tip cap, 900_000 WEI) -> this is the new logic we want to add to be conform with the ERPC
There was a problem hiding this comment.
we do not need SuggestedGasPrice fn as we are always sending type 2 transactions. The update in gas tip should be enough with adjusted gas price cap.
I removed the gasPriceCap * 1.5 as well in the SuggestGasTip function and renamed it to a more relevant name, to make it more in accordance with what we are aiming by this PR.
No description provided.