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

EIP-2250 : Gas Price Range #2250

Closed
wants to merge 8 commits into from
Closed

Conversation

tomhschmidt
Copy link

When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md

We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met:

  • The PR edits only existing draft PRs.
  • The build passes.
  • Your Github username or email address is listed in the 'author' header of all affected PRs, inside .
  • If matching on email address, the email address is the one publicly listed on your GitHub profile.

@tomhschmidt tomhschmidt changed the title EIP-X : Gas Price Range EIP-2250 : Gas Price Range Aug 26, 2019
@BrendanChou
Copy link

Other possibilities might be:

  • Soft lock (showing a warning) rather than a hard lock on the gas prices
  • A way for a DAPP to indicate to a wallet that transaction data may no longer be valid for whatever reason and so the user should cancel rather than continue on (although this could maybe be solved by the wallet itself by running the transaction in its own EVM and seeing if it will revert)

@tomhschmidt
Copy link
Author

Other possibilities might be:

  • Soft lock (showing a warning) rather than a hard lock on the gas prices
  • A way for a DAPP to indicate to a wallet that transaction data may no longer be valid for whatever reason and so the user should cancel rather than continue on (although this could maybe be solved by the wallet itself by running the transaction in its own EVM and seeing if it will revert)

Good ideas! I've been trying to find inspiration from other EIPs that create a standard around "proposals" or "suggestions" to figure out questions like these, but couldn't find anything.

IMO wallets are responsible for interpreting these variables and turning them into an experience that's best for their users. I could see arguments for and against each of these implementations depending on the user / wallet / situation, but at least now, the dApp can give the wallet signal that a given gas price suggestion should not be broken.

@fengtality
Copy link

Great idea! This may be simplistic, but since it seems like you're just adding a couple new parameters, can you not just override eth_sendTransaction?

@tomhschmidt
Copy link
Author

Great idea! This may be simplistic, but since it seems like you're just adding a couple new parameters, can you not just override eth_sendTransaction?

This was how I initially designed the spec, but it's a bit tricky in that it breaks backwards compatibility. The other issue is that the params passed into eth_sendTransaction get encoded, hashed, and signed, and we probably don't want the gas price range params to be part of the transaction itself, so passing them into a wrapper fn that can strip them out before calling eth_sendTransaction seems a bit cleaner.

@fengtality
Copy link

This was how I initially designed the spec, but it's a bit tricky in that it breaks backwards compatibility. The other issue is that the params passed into eth_sendTransaction get encoded, hashed, and signed, and we probably don't want the gas price range params to be part of the transaction itself, so passing them into a wrapper fn that can strip them out before calling eth_sendTransaction seems a bit cleaner.

Makes sense. If this is accepted, which libraries/products need to roll out support for this in order for dApps to make it available to users? (i.e. web3.js, Metamask, etc)

@tomhschmidt
Copy link
Author

This was how I initially designed the spec, but it's a bit tricky in that it breaks backwards compatibility. The other issue is that the params passed into eth_sendTransaction get encoded, hashed, and signed, and we probably don't want the gas price range params to be part of the transaction itself, so passing them into a wrapper fn that can strip them out before calling eth_sendTransaction seems a bit cleaner.

Makes sense. If this is accepted, which libraries/products need to roll out support for this in order for dApps to make it available to users? (i.e. web3.js, Metamask, etc)

Anything that uses the Ethereum JSON RPC spec would need to be updated to support this new functionality (parity and geth nodes, web3.js/py, ethers) and anything that has these things as dependencies.

Practically, I think the best implementation would involve just web3 and ethers implementing proposeTransaction for wallets and just stripping out the extra params and calling sendTransaction directly when interacting with unlocked nodes directly. Certainly, nodes can implement proposeTransaction, but I think it would just be a pure alias on sendTransaction

@MicahZoltu
Copy link
Contributor

I'm strongly against this EIP as written. Gas prices are the domain of the signing tool IMO, not of the dapp. We have seen many problems with dapps hard-coding gas prices and gas price assumptions into contracts and UIs over time and gas price estimation is a hard problem that we shouldn't expect dapp authors to solve reliably and have those solutions remain relevant as gas prices and gas pricing algorithms evolve over time.

I do think there is value in the dapp being able to express time sensitivity to the signing tool so it can make appropriate recommendations to the user (e.g., "You have set the gas price to 'slow', but the dapp says this transaction is time sensitive, are you sure you want to do this?"), but that is very different from the dapp suggestion and/or hard-coding gas prices.

I would like to see the ecosystem evolve to a point where dapps don't have any concern/knowledge about gas prices and instead they focus on building the dapp. Signing tools, on the other hand, should be enhanced to do a better job with gas price suggestion, selection, and information.

@tomhschmidt
Copy link
Author

tomhschmidt commented Sep 5, 2019

I'm strongly against this EIP as written. Gas prices are the domain of the signing tool IMO, not of the dapp. We have seen many problems with dapps hard-coding gas prices and gas price assumptions into contracts and UIs over time and gas price estimation is a hard problem that we shouldn't expect dapp authors to solve reliably and have those solutions remain relevant as gas prices and gas pricing algorithms evolve over time.

I do think there is value in the dapp being able to express time sensitivity to the signing tool so it can make appropriate recommendations to the user (e.g., "You have set the gas price to 'slow', but the dapp says this transaction is time sensitive, are you sure you want to do this?"), but that is very different from the dapp suggestion and/or hard-coding gas prices.

I would like to see the ecosystem evolve to a point where dapps don't have any concern/knowledge about gas prices and instead they focus on building the dapp. Signing tools, on the other hand, should be enhanced to do a better job with gas price suggestion, selection, and information.

Thanks for the feedback, Micah! I'm glad we agree that dapps should be able to express time sensitivity to the signing tool / wallet and happy to figure out the best way to do this. I think there may have been a small misunderstanding in your post.

dapps hard-coding gas prices and gas price assumptions into contracts

Agreed! Definitely not what we want and not necessary what this EIP would allow. I think the important thing to always remember is that this EIP simply allows a dApp to signal time sensitivity to a wallet, but it's up to the wallet to decide what to do with this information. For certain dapps or users, the time sensitivity could be immutable, but for others, could simply be strongly recommended.

I agree with your desire to make dApp development easier and remove gas price estimation burden. I think the question then is what level of abstraction do we want to expose in the JSON RPC interface. I think best practice is to make the JSON RPC interface fairly raw with minimal opinions or assumptions, but allow others to build helper and utility functions / libraries on top of it (such as ethers.js).

To that end, instead of forcing dApp developers to use a convenience function for getting gas price bounds given a time priority, it might make more sense to direct more devs to using something like Ethereal's gas price estimator function. Ethereal allows a dev to get an appropriate gas price given a number of blocks that they want the transaction to get confirmed within. This could be abstracted out even further (to your point) so that a dev can simply supply an amount of time or a "slow"/"medium"/"fast" value and get a gas price back which can be used as a bound for the transaction proposal. Allowing the ability to set gas price bounds raw is still helpful to expose for developers who want to do so because they have their own gas pricing function, gas price is plays an essential role in their dapp, etc.

In short, I think we just need to make it clearer to devs that they should be delegating gas price estimation to e.g. Ethereal, ETHGasStation, etc. and build more convenience functions there, but ultimately still spitting out a raw gas price range for this EIP.

Let me know your thoughts!

@MicahZoltu
Copy link
Contributor

I think the important thing to always remember is that this EIP simply allows a dApp to signal time sensitivity to a wallet, but it's up to the wallet to decide what to do with this information.

This doesn't appear to be true in the current iteration of this EIP. This EIP is proposing the following:

gasPrice: QUANTITY - (optional, default: To-Be-Determined) Integer of the gasPrice used for each paid gas
minGasPrice: QUANTITY (optional, default: 0) Integer of the minimum gas price to be allowed for this transaction. Must be greater or equal to and 0 and less than or equal to maxGasPrice if it is set. Must also be less than or equal to gasPrice.
maxGasPrice: QUANTITY (optional) Integer of the maximum gas price to be allowed for this transaction. Must be greater than or equal to minGasPrice if it is set. Must also be greater than or equal to gasPrice.

That isn't a time sensitivity signal, that is a min/max gas price. While the signer could try to derive time sensitivity from that information, I think the dapp should instead give direct information about time sensitivity rather than derived information that the signer then has to guess what is meant.

@tomhschmidt
Copy link
Author

Thinking about this a bit more and after getting some inspiration from Uniswap's "deadline" parameter , might be better to just revive #599. To @MicahZoltu, it's more deterministic than gasPrice.

@Arctek
Copy link

Arctek commented Dec 27, 2019

There is also #1681, which proposes using a timestamp instead of the block number.

That said there is the issue in 0x v3 where the protocol fee that must be sent with the transaction is calculated from the gas price (this is going to affect Augur as well as a result).
Also AFAIK Bancor has also had this type of functionality when you use their website to make trades.

So in this case you don't want to set minimum/maximum gas price, you want to set a single value that the wallet UI isn't allowed to change.

I don't know if this calls for a new EIP or not, but unless the dApp can signal this to the signer I don't see how this can be derived?

At least in the case of 0x we probably need one of the temporal protection EIPs as well as a way to set a fixed gas price.
As you would lock the gas price, but set a sane expiry on the transaction i.e. 2 minutes (or by block number).

@MicahZoltu
Copy link
Contributor

@Arctek IMO, the problem here is 0x, not the underlying communication protocol between app and signer. The 0x fee should not depend on gas price for a number of reasons, including third party transaction submission, signers changing gas price after transaction construction, changes in gas pricing algorithms by Miners, Miners being able to submit transactions for 0 gas price, etc.

@abandeali1
Copy link

We have seen many problems with dapps hard-coding gas prices and gas price assumptions into contracts and UIs over time and gas price estimation is a hard problem that we shouldn't expect dapp authors to solve reliably and have those solutions remain relevant as gas prices and gas pricing algorithms evolve over time.

There is no requirement to use this RPC method or any of its optional fields. I don’t see any harm in allowing dapps to choose to tackle this problem if they need to for any reason.

@Arctek IMO, the problem here is 0x, not the underlying communication protocol between app and signer. The 0x fee should not depend on gas price for a number of reasons, including third party transaction submission, signers changing gas price after transaction construction, changes in gas pricing algorithms by Miners, Miners being able to submit transactions for 0 gas price, etc.

Fee markets have a huge impact on the economics of any transactional smart contract. I can't see any practical argument for willfully ignoring them, especially since we already have the ability to add constraints to them. I'd argue that the main reason most contracts currently do not interact with gas prices is because of the lack of tools available to communicate these requirements to signers (though many prominent contracts still do, including 0x, Kyber, Bancor, etc). Making it easier to do so would unlock a lot of new functionality for smart contracts and would have numerous benefits for end users as a result. I actually think that this should be the primary goal of this EIP, and allowing dapps to estimate gas on behalf of their users should be a secondary benefit (though I do think that this is very useful for that too).

This EIP is also pretty exclusive from #599 and #1681 because it can prevent users from making a mistake in the first place rather than reacting to a mistake. This is a very important distinction from a UX perspective. With #599 or #1681 users could easily underestimate gas price relative to a transaction's expiry and would not realize this until the transaction is no longer valid. Sure - they wouldn't actually waste gas, but a lot could change in the amount of time it takes for them to realize this and resubmit the transaction. Longer wait times and manual resubmissions are also terrible UX.

@tomhschmidt I'd be happy to take over this EIP as an author and continue to push it forward!

@github-actions
Copy link

github-actions bot commented Sep 8, 2020

There has been no activity on this pull request for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Sep 8, 2020
@github-actions
Copy link

This pull request was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.

@github-actions github-actions bot closed this Sep 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants