all: switch gas limits from big.Int to uint64 #15466
Conversation
core/block_validator.go
Outdated
| contrib := new(big.Int).Mul(parent.GasUsed(), big.NewInt(3)) | ||
| contrib = contrib.Div(contrib, big.NewInt(2)) | ||
| contrib = contrib.Div(contrib, params.GasLimitBoundDivisor) | ||
| contrib := ((parent.GasUsed() / 2) * 3) / params.GasLimitBoundDivisor // div-mul order change is fine due to last 10 bit shift |
There was a problem hiding this comment.
Instead of changing the order, you could do
(x/2 * 3) / 1024
eq (x/2 + x) / 1024
Which could be further reduced to
(x >> 1 + x) >> 10
or x >> 11 + x >> 10
Where it's trivial to see that the last expression is overflow-safe.
If you want to maintain the params, you could do
parent.gasUsed / params.GasLimitBoundDivisor + (parent.gasUsed / 2 ) / params.GasLimitBoundDivisor
There was a problem hiding this comment.
Fixed to your first suggestion.
There was a problem hiding this comment.
So that means that there will be errors if gasUsed ever goes above 2/3 of a full uint64?
Which I think we can live with
| i.Add(i, amount) | ||
| func (gp *GasPool) AddGas(amount uint64) *GasPool { | ||
| if uint64(*gp) > math.MaxUint64-amount { | ||
| panic("gas pool pushed above uint64") |
There was a problem hiding this comment.
I'm not sure a panic is right here - perhaps can be used to cause DoS over RPC, by performing calls with very large gas values?
karalabe
force-pushed
the
uint64-gas-limit
branch
from
35a275a to
74907b6
Compare
|
Maybe I don't see the big picture. But wouldn't be better to create a type alias for |
|
@ricardohsd |
|
@fjl that's true. Thanks for the explanation. |
karalabe
force-pushed
the
uint64-gas-limit
branch
from
74907b6 to
6f69cdd
Compare
This PR continues the uint64 gas rewrite from a long time ago, by transitioning all remaining types (headers, blocks, receipts, transactions) to 64 bit gas fields. This is mostly needed to unify our APIs (old APIs use big.Ints, new ones use uint64s).
I also need this PR to revamp gas estimation (currently it's not automatic if no gas limit is provided), but that turned out to require yet again a large API overhaul, so I'll just submit this massive PR first, and follow up with the API reorgs afterwards.