Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add eth_maxPriorityFeePerGas #201

Closed
wants to merge 2 commits into from

Conversation

lightclient
Copy link
Member

@lightclient lightclient commented May 30, 2021

This PR adds two new RPC methods:

  • eth_maxFeePerGas -- returns a maxFeePerGas estimate for quick inclusion
  • eth_maxPriorityFeePerGas -- returns a maxPriorityPerGas estimate for quick inclusion

An alternative to introducing two new methods is to add an optional flag to eth_gasPrice that denotes whether the caller is interested in a gasPrice estimate or a EIP-1559 fee estimate. I have a weak preference for the two methods.

I'll keep this as a draft until we feel there is good consensus on this issue.

cc: @tkstanczak, @ryanschneider, @MicahZoltu

@MicahZoltu
Copy link
Contributor

I'm actually weakly against these being available via JSON-RPC, at least under these names. eth_maxFeePerGas isn't something you should ask a computer for, it is something from the user. We want people to put down the max they are willing to pay. We don't want people just blindly letting a computer set it for them. If anything, this should take a transaction as input and just return the balance of the account minus any ETH attached to the transaction.

If we give people this method they will use it because they will think they should, when they should basically never use this method.

For eth_maxPriorityFeePerGas, I think we should return a time weighted histogram of minimum priority fees included in blocks. I don't think Ethereum clients should be making the final decision on what number to select, but they should make it easy for wallets to get the necessary data for decision making.

@lightclient
Copy link
Member Author

lightclient commented May 31, 2021

I could get on board with eth_maxFeePerGas not being an endpoint. It's quite simple to just ask for the latest header, multiply the header's base fee by some fixed amount, and finally add in the result of eth_maxPriorityFeePerGas (if desired). However, if you want to do more complicated things to keep a tight bound on the base fee -- things like increasing the fixed multiplier during periods of congestion, you'd really need this method or some way of returning to wallets how the base fee is actually moving.

Your argument against having eth_maxPriorityFeePerGas seems pretty similar to your argument against having eth_gasPrice. I think wallets very much rely on this functionality and we should consult them before not providing it (cc @GregTheGreek). I am also okay with potentially returning data to let the wallet make a good decision on the tip vs. just providing them the value.

@fvictorio
Copy link

I agree that it's not clear how eth_maxFeePerGas could be useful. It could return something like "the median of the previous block's maxFeePerGas values" but... that doesn't seem helpful to me.

With respect to eth_maxPriorityFeePerGas, I think the logical thing would be to make it behave as eth_gasPrice does now. The problem is that (as far as I know), eth_gasPrice is not very used, is it? (As a data point: in Hardhat we have always returned a fixed, hardcoded value, and in all this time only one or two users have complained about it).

So the alternative would be to return something else, like what Micah mentioned, but in that case I believe it should have a different name. Besides, it seems to me that deciding what exactly would be returned by that method will be easier once wallets start implementing strategies for setting the EIP-1559 values, so maybe the best thing to do would be to just wait until that happens, and then revisit this issue.

@GregTheGreek
Copy link
Contributor

@fvictorio eth_gasPrice is definitely used by everyone, no?

Unless a centralized api is calculating their own gas price oracle (GPO) then you have to query the node for the current gas price.

@fvictorio
Copy link

fvictorio commented May 31, 2021

Yeah, I regretted writing that after sending it. The way our users use the node is definitely not representative of the rest of the ecosystem.

I still think that an eth_getMaxPriorityFeePerGas should be as semantically similar to eth_gasPrice as possible, though.

@GregTheGreek
Copy link
Contributor

Right ok that makes sense. From my PoV it would be great if we can have some standard on how its implemented because as it stands now all the clients implement the internal GPO slightly differently which can cause some very under priced tx's.

@karalabe
Copy link
Member

karalabe commented Jun 1, 2021

Just hit this issue in Geth's 1559 RPC implementation (ethereum/go-ethereum#22964 (comment)).

My 2c is that eth.gasPrice should behave and return exactly what it does now. Otherwise we'll surely break someone's workflows if it suddenly becomes larger or smaller.

I'm voting for adding an eth.gasTipCap (tongue-in-cheek) that estimates the tip or the user (in reality, eth.gasPrice - baseFee). IMHO it is important for people to have at least a rough suggestion that they are free to accept or not. I don't think it's ok to rely on the user having to check ethgassation for even a baseline idea.

As for the fee cap, I think clients should default to something meaningful if the user doesn't specify it (e.g. estimated tip + 2x basefee), but I don't see a reason to make that into an endpoint if we already have a tip cuggestion (which is the hard part to estimate either way).

@timbeiko
Copy link
Collaborator

timbeiko commented Jun 1, 2021

My proposal if we have a priorityFee endpoint is to estimate a number based on the uncle risk of including transactions in a block and just return that (currently ~2 gwei due to MEV, see this post for more details). This number should be good enough as a "default" value for wallets.

A lot of wallets have been asking what is the default they should use, so providing that to them would be useful.

The returned value can be changed in a non-hard fork client release if the MEV per block changes significantly, which would cause uncle blocks to have a higher opportunity cost.

@lightclient
Copy link
Member Author

It appears everyone is onboard with eth_maxPriorityFeePerGas method and to not have eth_maxFeePerGas. I think we'll still need to work out what exactly this method will return, as there some different ideas, but I believe we should continue those discussions elsewhere and merge this PR.

@lightclient lightclient marked this pull request as ready for review June 1, 2021 16:59
@timbeiko
Copy link
Collaborator

timbeiko commented Jun 1, 2021

I feel like this is an appropriate discussion to have on this Friday's Gas API call given they will be the consumer of this to a large extent. @lightclient will merge this if no one objects to the API in the next day or so.

@lightclient lightclient changed the title Add eth_maxFeePerGas and eth_maxPriorityFeePerGas Add eth_maxPriorityFeePerGas Jun 4, 2021
@timbeiko
Copy link
Collaborator

@lightclient based on today's ACD conversation and going with feeHistory instead, are you OK closing this?

@lightclient
Copy link
Member Author

@timbeiko good with me!

@timbeiko timbeiko closed this Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants