Closed kclowes closed 2 years ago
Hi @kclowes, good excuse to dig into some more web3.py code. I will put together a PR for this.
Awesome, thanks @broper2!
Hi @kclowes, wanted to run something by you before submitting PR. I do think the underlying data used in eth_maxPriorityFeePerGas implementation is available in eth_feeHistory, and we could add a simple function to estimate a reasonable "tip" default.
However, as I read about this more, I found this discussion: https://github.com/ethereum/pm/issues/328#issuecomment-853510466
Seems we may be able to leverage relation between gas_price and get_block('latest')['baseFeePerGas'] to get an exact value of what eth_maxPriorityFeePerGas would return. The relationship (web3.eth.gas_price - web3.eth.get_block('latest')['baseFeePerGas']) == web3.eth.max_priority_fee seems to hold in some quick functional testing I have done.
I don't claim to be ETH expert, so please tell me if I am missing something, or if this looks ok as a no-impact change. Thanks!
That looks good to me, but @fselmo has done way more of the dynamic fee work so I'll defer to him. Do you have any thoughts @fselmo? Thanks for looking into it, @broper2!
Seems we may be able to leverage relation between gas_price and get_block('latest')['baseFeePerGas'] to get an exact value of what eth_maxPriorityFeePerGas would return. The relationship (web3.eth.gas_price - web3.eth.get_block('latest')['baseFeePerGas']) == web3.eth.max_priority_fee seems to hold in some quick functional testing I have done.
I don't think this is good enough. Some testing on my end suggests that at every new block transition (most likely) there is inconsistency in the values for eth_maxPriorityFeePerGas
and (eth_gasPrice
- latest_base_fee
), therefore this is simply not good enough to hold.
I did some quick-and-dirty testing calling the below method on an infura connection multiple times until I got False
and the inequality printed. It seems to happen about once every few seconds or so and then re-stabilizes to True
which makes me think it's at the new block transition but I haven't really inspected it too much. Only enough to know this is not ideal.
def max_prio_calc():
gp = w3.eth.gas_price
bf = w3.eth.get_block('latest')['baseFeePerGas']
mpf = w3.eth.max_priority_fee
print(f'gas_price: {gp} | base_fee: {bf} | max_priority_fee: {mpf}')
theory_holds = mpf == gp - bf
print(theory_holds)
if not theory_holds:
print(f'{mpf} != {gp - bf}')
e.g.
>>> max_prio_calc()
gas_price: 78216696028 | base_fee: 77168141801 | max_priority_fee: 1250000000
False
1250000000 != 1048554227
Again, I haven't looked into this too much yet (other than testing the suggestion) but I would be good with building a calculation from scratch that is equal to what geth
is doing or some other method that is more robust using eth_feeHistory
. I also wouldn't mind putting the responsibility of specifying the maxPriorityFeePerGas
on the user if the eth_maxPriorityFeePerGas
RPC method isn't available (though this is less ideal).
Thoughts?
Yeah its a good point. From reading about this, I am not convinced we will be able to replicate behavior of RPC endpoint eth_maxPriorityFeePerGas perfectly either way, but if that is accepted I can put together a calculation and see if it gets closer over number of tries (I am actually guessing that it will not be as good, but don't mind trying).
If the goal is really to never deviate from what eth_maxPriorityFeePerGas returns, I think it may make better sense to put responsibility on user.
Let me know what you think @fselmo
@broper2 yeah I hear you... I don't necessarily care that we replicate what geth is doing... something else that is reasonable would work well here too. The reason I don't like the eth_gasPrice
approach is that sometimes it doesn't make sense at all and thus isn't reliable. I just ran it for less than a minute and I got a negative value because the base_fee
was higher than the gas_price
at that moment.
e.g.
>>> max_prio_calc()
gas_price: 67953109912 | base_fee: 74750863316 | max_priority_fee: 1500000000
False
1500000000 != -6797753404
Also, the more I think about it, the more I don't think I'm ok with putting the responsibility on the user. The barrier to entry is still pretty high for web3 and one of our main goals is to make the user experience better and make development more inviting.
In the end I think this ended up being a bit more complicated of a task than we expected but thank you for starting the conversation on it. Let me know if you are still interested in pursuing this. No sweat if not. I wouldn't mind scooping this up after the holidays. I would probably start looking at what geth is doing and any other examples (if they exist) and look to implement a similar internal approach for when the eth_maxPriorityFeePerGas
method is not available.
Good catch with the negative value, should have considered that. And no problem, I'll give it a go creating an estimate to this endpoint in the next few days. I am just trying to dig into some web3 code so don't mind the added complexity at all.
When I was researching this before, I came across this Alchemy blog which gives a lot of info on best way to do exactly this calculation. Will circle back when I have that PR updated. Thanks @fselmo
https://docs.alchemy.com/alchemy/guides/eip-1559/gas-estimator
Since
eth_maxPriorityFeePerGas
is Geth-specific, we should stop using it to fill in gas values. We should useeth_feeHistory
instead. I think it still makes sense to keep the RPC endpoint inweb3/eth.py
, but all places we calleth_maxPriorityFeePerGas
should be changed.Comment from #2237:
Originally posted by @guoyiteng in https://github.com/ethereum/web3.py/issues/2237#issuecomment-993133677