search

ethereum-optimism / optimism

Optimism is Ethereum, scaled.
https://optimism.io
MIT License
5.68k stars 3.32k forks source link

executionPrice is set via the gasPrice #536

Closed K-Ho closed 3 years ago

K-Ho commented 3 years ago

Describe the bug The gasPrice in OE is currently harcoded to 1 gwei and estimateGas really does a calculateFeeInGwei. Within estimateGas, we pull the executionPrice using geth's built-in suggestPrice which will suggest a gasPrice based on historical gasPrices. This means that when we switch fees on, suggestPrice will always return 1 gwei and the L2 executionPrice would be stuck at a constant 1 gwei. See: https://github.com/ethereum-optimism/optimism/blob/ba2e04320b80b1c03bc517fa17e29c615a73e413/l2geth/internal/ethapi/api.go#L1022

Expected behavior I would expect executionPrice to be set based on the congestion of the chain

Additional context The executionPrice should be set based on congestion of the chain using some formula. While OE is still under capacity, I suggest we either hardcode executionPrice to 0 or set it based on an env var (so if we receive a huge spike in usage, we can easily increase executionPrice)

gakonst commented 3 years ago

Given that we do not have a notion of a mempool, I am supportive of setting the executionPrice via env var now (initially set to 0).

tynes commented 3 years ago

MVP via config is fine for executionPrice but having downtime to change config options is no fun.

Ideally we can set that via IPC so that we don't have to restart the Sequencer to update that value

@smartcontracts Would this solve the problem that users are facing for the hackathon?

gakonst commented 3 years ago

@tynes Agreed we can do it over IPC (since all namespaces are exposed over IPC). Probably better that way instead of config option. We should add it to the rollup_private namespace then, do you agree?

K-Ho commented 3 years ago

Currently going to set executionPrice to 0, but let's get set something up before end of May. Ideally, the executionPrice is set in an L2 contract cc @tynes @karlfloersch

tynes commented 3 years ago

It was decided that in the short term, placing the data in a contract is not needed for Synthetix Exchange. It is something that is required for Uniswap V3