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 minimum gas price #1819

Merged
merged 6 commits into from Dec 7, 2019
Merged

Add minimum gas price #1819

merged 6 commits into from Dec 7, 2019

Conversation

@bowenwang1996
Copy link
Member

bowenwang1996 commented Dec 4, 2019

Add minimum gas price to genesis config so that when there are not enough transactions, gas price will not become zero.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Dec 4, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (staging@200f8d5). Click here to learn what that means.
The diff coverage is 97.01%.

Impacted file tree graph

@@            Coverage Diff             @@
##             staging    #1819   +/-   ##
==========================================
  Coverage           ?   78.28%           
==========================================
  Files              ?      170           
  Lines              ?    42845           
  Branches           ?        0           
==========================================
  Hits               ?    33543           
  Misses             ?     9302           
  Partials           ?        0
Impacted Files Coverage Δ
core/primitives/src/test_utils.rs 100% <ø> (ø)
near/tests/sync_nodes.rs 97.71% <ø> (ø)
chain/chain/src/test_utils.rs 77.12% <ø> (ø)
chain/client/tests/challenges.rs 92.79% <ø> (ø)
chain/chain/tests/simple_chain.rs 90.9% <ø> (ø)
chain/client/tests/process_blocks.rs 79.04% <100%> (ø)
chain/client/src/test_utils.rs 79.2% <100%> (ø)
chain/client/src/client.rs 88.21% <100%> (ø)
chain/chain/src/chain.rs 78.79% <100%> (ø)
near/tests/stake_nodes.rs 96.26% <100%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 200f8d5...e95aaf9. Read the comment docs.

Copy link
Collaborator

nearmax left a comment

Instead of adding a minimal gas price we should be using the initial gas price (or previous gas price, if suddenly we have no transactions for many blocks) until we have enough data to avoid the division by zero.

@bowenwang1996

This comment has been minimized.

Copy link
Member Author

bowenwang1996 commented Dec 4, 2019

Division by zero will not happen given how we compute the new gas price. I don't understand at which point we switch to use initial gas price or previous gas price.

@nearmax

This comment has been minimized.

Copy link
Collaborator

nearmax commented Dec 4, 2019

Division by zero will not happen given how we compute the new gas price.

I bad, what I meant is when there is not enough transactions we should fall back to either the gas price defined in the genesis, or the last price that was computed.

I don't understand at which point we switch to use initial gas price or previous gas price.

We would have to figure out the exact mechanics here and make sure this is not abusable. Adding one more parameter to our config obscures the problem, but does not solve it -- how do we determine the minimum gas price? What if it is too low, can it stall some shard by making it non-profitable to validate? Can anything bad happen if it is too hight? Also, when we have enough transactions to calculate the gas cost, can it drop below the minimum gas cost defined in this config?

@bowenwang1996

This comment has been minimized.

Copy link
Member Author

bowenwang1996 commented Dec 4, 2019

Gas price is computed every block, so not sure what you mean by "have enough transactions to compute gas cost". It seems that you are suggesting a dynamic approach to computing the minimum gas price, but some threshold has to be set (could be absolute or relative). I am also fine if we just use initial gas price as minimum gas price, but then we just need to take that into consideration when we set its value.

@nearmax

This comment has been minimized.

Copy link
Collaborator

nearmax commented Dec 4, 2019

Okay, I understand now our miscommunication. What you are saying is that when there are few transactions the gas price might drop too much, correct?

I actually expect the completely opposite thing to happen when there are few transactions. The blockchain without transactions is more expensive to run which will be reflected by the gas price going up. Here is the explanation:

Gas price is only relevant in the context of the fees. There are accountable and non-accountable expenses for the validators. Accountable expenses are fees. Non-accountable expenses are more complex things that we do not account for with our fees, e.g. cost of serializing and deserializing chunk/header, cost of catching up with the network, cost of helping someone catch up with the network. The cost of these non-accountable expenses is amortized across the participating nodes, which is one of the major contributors to the gas price. The more transactions we have per block the easier it is to pay for the non-accountable expenses with little gas increase. Now imagine we start receiving fewer transactions per block, actually, let's imagine we receive 1 transaction per block. The fees from this single transaction is going to be the only source of reward (not accounting for the inflation) to the validators, which means the gas price will need to go 10x or 100x or 1000x up to make this single transaction in each block to pay for not accountable expenses.

The side effect of gas price going up when there are fewer transactions is that the blockchain has some gas price threshold after each it enters the death spiral -- fewer people use it due to high prices, which leads to prices going even higher up.

@bowenwang1996

This comment has been minimized.

Copy link
Member Author

bowenwang1996 commented Dec 4, 2019

If we try to balance it this way, validators might have incentives to accept fewer transactions while still getting roughly the same amount of reward from executing transactions. Also the validator reward does not only come from transactions fees. When there are not enough transactions, they get reward mostly from inflation.

@nearmax

This comment has been minimized.

Copy link
Collaborator

nearmax commented Dec 4, 2019

If we try to balance it this way, validators might have incentives to accept fewer transactions while still getting roughly the same amount of reward from executing transactions.

It is hypothetical whether validators will do that and whether this is going to be enough to avoid the death spiral.

Also the validator reward does not only come from transactions fees. When there are not enough transactions, they get reward mostly from inflation.

So, hypothetically, inflation reward will stop the gas price going up into a death spiral. Do we have a formal way to analyse at what gas price it will stop growing up?

@bowenwang1996 Do you currently agree that the decrease of the transaction volume will cause the gas price to go up, instead of down?

@bowenwang1996

This comment has been minimized.

Copy link
Member Author

bowenwang1996 commented Dec 4, 2019

We unfortunately don't have any formal analysis on anything economics related, but I tend to believe that gas price should go up when transaction volume goes up so that validators get more reward when there are more transactions. It also makes sense that when there is more demand, the price is higher and vice versa.

@nearmax

This comment has been minimized.

Copy link
Collaborator

nearmax commented Dec 4, 2019

@evgenykuzyakov just reminded me that the gas price is not directly influenced by the validators and we actually implement it to go up or down depending on whether the load of the blockchain goes up or down. @bowenwang1996 AFAIR you have implemented it, do you remember whether we have implemented the gas price to go up or to go down depending on the load? Do you remember where in the code we have this logic?

@bowenwang1996

This comment has been minimized.

Copy link
Member Author

bowenwang1996 commented Dec 4, 2019

Yes gas price will currently go to zero if there are not enough transactions.

let new_gas_price = Self::compute_new_gas_price(

@nearmax

This comment has been minimized.

Copy link
Collaborator

nearmax commented Dec 4, 2019

Yes gas price will currently go to zero if there are not enough transactions.

let new_gas_price = Self::compute_new_gas_price(

I think there is a problem with the linked code that will cause gas_used to almost always go down independently on how much the blockchain is used. gas_limit there refers to the gas limit of the block. In all cases, except for one exception listed below gas_used <= gas_limit, which means numerator <= denominator, which means gas_price <= prev_gas_price, meaning it will always go down or stay the same.

The only exception comes from the fact that when there is a large load we can accidentally use more gas than the gas limit, by a tiny bit amount, which is also going to be an almost random amount. And since it will be: tiny, accidental, and random; it will not be a value we can rely on to correctly push the price of the gas upwards.

What am I missing?

@bowenwang1996

This comment has been minimized.

Copy link
Member Author

bowenwang1996 commented Dec 4, 2019

Gas price will go up when gas_used >= gas_limit/2. I just checked again and it looks fine to me.

@nearmax

This comment has been minimized.

Copy link
Collaborator

nearmax commented Dec 4, 2019

Gas price will go up when gas_used >= gas_limit/2. I just checked again and it looks fine to me.

So we have decided that we want our blocks to be half-full on average. This seems like a random constant that might be adjusted in the future (e.g. to make sure we utilize 80% of our costs). Should we make a parameter from it?

@bowenwang1996

This comment has been minimized.

Copy link
Member Author

bowenwang1996 commented Dec 5, 2019

Not sure whether it's a random constant @ilblackdragon.

@@ -152,6 +158,7 @@ pub struct ChainGenesis {
pub time: DateTime<Utc>,
pub gas_limit: Gas,
pub gas_price: Balance,
pub min_gas_price: Balance,

This comment has been minimized.

Copy link
@ilblackdragon

ilblackdragon Dec 5, 2019

Member

should we have a field of type BlockEconomicsConfig here?

@@ -86,6 +86,9 @@ pub const INITIAL_GAS_LIMIT: Gas = 10_000_000;
/// Initial gas price.
pub const INITIAL_GAS_PRICE: Balance = 100;

This comment has been minimized.

Copy link
@ilblackdragon

ilblackdragon Dec 5, 2019

Member

let's remove initial gas price. and for genesis use min_gas_price as the gas_price

@ilblackdragon

This comment has been minimized.

Copy link
Member

ilblackdragon commented Dec 5, 2019

@nearmax @bowenwang1996
This constant is connected with constant of how long we also waiting for prev block before considering to skip it. E.g if you decide for blocks to be 80% full, you are expected to wait for prev block only for 20% more time then expected block time and which can be affected by other lags.

50% balances this, with min_block_production_delay is 1s (https://github.com/nearprotocol/nearcore/blob/master/near/src/config.rs#L52) and max_block_production_delay is 2s (https://github.com/nearprotocol/nearcore/blob/master/near/src/config.rs#L55).

Even though we can adjust all the coefficients together to a different value, 50% both seems natural and also matches suggestions by Vitalik and few other folks working on similar model.

@nearmax

This comment has been minimized.

Copy link
Collaborator

nearmax commented Dec 6, 2019

Update: I derped, disregard.

@ilblackdragon FYI, on a different, but relevant note. In the economics paper it says:

We suggest that every block producers vote on gasLimit by providing a new value within +/-0.1% if the previous block has gasUsed > 0.9 * gasLimit.

This will almost never happen and if it will this event will be random. Our gasLimit is hard, meaning we define on the protocol, that as we process transactions/receipts as soon as we hit gasLimit with the last transaction/receipt we stop processing them. So in order to go 10% over the gasLimit we need to be lucky with getting this last transaction to be large enough (the order of transactions is defined in the protocol, so it will be hard for block producer to arrange that).

We need a replacement for the mechanism that determines when to vote on gasLimit.

@nearmax
nearmax approved these changes Dec 6, 2019
@bowenwang1996

This comment has been minimized.

Copy link
Member Author

bowenwang1996 commented Dec 6, 2019

I am confused as to why it is almost impossible to have gasUsed > 0.9 * gasLimit . I assume gasLimit will be a reasonably large value and transactions usually do not consume a ton of gas so it seems entirely plausible to have gasUsed > 0.9 * gasLimit if there are a lot of transactions.

@nearmax

This comment has been minimized.

Copy link
Collaborator

nearmax commented Dec 6, 2019

I am confused as to why it is almost impossible to have gasUsed > 0.9 * gasLimit . I assume gasLimit will be a reasonably large value and transactions usually do not consume a ton of gas so it seems entirely plausible to have gasUsed > 0.9 * gasLimit if there are a lot of transactions.

Oh yeah, you are right. I misunderstood.

@bowenwang1996 bowenwang1996 force-pushed the add-min-gas-price branch from 36ea89c to 422b9bf Dec 7, 2019
@bowenwang1996 bowenwang1996 merged commit 5e792f8 into staging Dec 7, 2019
1 check passed
1 check passed
gitlab-ci
Details
@bowenwang1996 bowenwang1996 deleted the add-min-gas-price branch Dec 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.