Commit 0fb44b3c authored by Mark Tyneway's avatar Mark Tyneway Committed by GitHub

Merge pull request #1813 from ethereum-optimism/fix/gpo-gas-adding

feat: more accurate gas-oracle gas metering
parents 5b4d65d4 ab2378e3
---
'@eth-optimism/gas-oracle': patch
---
Meter gas usage based on gas used in block instead of assuming max gas usage per block
...@@ -2,6 +2,7 @@ package gasprices ...@@ -2,6 +2,7 @@ package gasprices
import ( import (
"errors" "errors"
"math/big"
"sync" "sync"
"github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/log"
...@@ -9,33 +10,26 @@ import ( ...@@ -9,33 +10,26 @@ import (
type GetLatestBlockNumberFn func() (uint64, error) type GetLatestBlockNumberFn func() (uint64, error)
type UpdateL2GasPriceFn func(uint64) error type UpdateL2GasPriceFn func(uint64) error
type GetGasUsedByBlockFn func(*big.Int) (uint64, error)
type GasPriceUpdater struct { type GasPriceUpdater struct {
mu *sync.RWMutex mu *sync.RWMutex
gasPricer *GasPricer gasPricer *GasPricer
epochStartBlockNumber uint64 epochStartBlockNumber uint64
averageBlockGasLimit float64 averageBlockGasLimit uint64
epochLengthSeconds uint64 epochLengthSeconds uint64
getLatestBlockNumberFn GetLatestBlockNumberFn getLatestBlockNumberFn GetLatestBlockNumberFn
getGasUsedByBlockFn GetGasUsedByBlockFn
updateL2GasPriceFn UpdateL2GasPriceFn updateL2GasPriceFn UpdateL2GasPriceFn
} }
func GetAverageGasPerSecond(
epochStartBlockNumber uint64,
latestBlockNumber uint64,
epochLengthSeconds uint64,
averageBlockGasLimit uint64,
) float64 {
blocksPassed := latestBlockNumber - epochStartBlockNumber
return float64(blocksPassed * averageBlockGasLimit / epochLengthSeconds)
}
func NewGasPriceUpdater( func NewGasPriceUpdater(
gasPricer *GasPricer, gasPricer *GasPricer,
epochStartBlockNumber uint64, epochStartBlockNumber uint64,
averageBlockGasLimit float64, averageBlockGasLimit uint64,
epochLengthSeconds uint64, epochLengthSeconds uint64,
getLatestBlockNumberFn GetLatestBlockNumberFn, getLatestBlockNumberFn GetLatestBlockNumberFn,
getGasUsedByBlockFn GetGasUsedByBlockFn,
updateL2GasPriceFn UpdateL2GasPriceFn, updateL2GasPriceFn UpdateL2GasPriceFn,
) (*GasPriceUpdater, error) { ) (*GasPriceUpdater, error) {
if averageBlockGasLimit < 1 { if averageBlockGasLimit < 1 {
...@@ -51,6 +45,7 @@ func NewGasPriceUpdater( ...@@ -51,6 +45,7 @@ func NewGasPriceUpdater(
epochLengthSeconds: epochLengthSeconds, epochLengthSeconds: epochLengthSeconds,
averageBlockGasLimit: averageBlockGasLimit, averageBlockGasLimit: averageBlockGasLimit,
getLatestBlockNumberFn: getLatestBlockNumberFn, getLatestBlockNumberFn: getLatestBlockNumberFn,
getGasUsedByBlockFn: getGasUsedByBlockFn,
updateL2GasPriceFn: updateL2GasPriceFn, updateL2GasPriceFn: updateL2GasPriceFn,
}, nil }, nil
} }
...@@ -63,16 +58,29 @@ func (g *GasPriceUpdater) UpdateGasPrice() error { ...@@ -63,16 +58,29 @@ func (g *GasPriceUpdater) UpdateGasPrice() error {
if err != nil { if err != nil {
return err return err
} }
if latestBlockNumber < uint64(g.epochStartBlockNumber) { if latestBlockNumber < g.epochStartBlockNumber {
return errors.New("Latest block number less than the last epoch's block number") return errors.New("Latest block number less than the last epoch's block number")
} }
averageGasPerSecond := GetAverageGasPerSecond(
g.epochStartBlockNumber, if latestBlockNumber == g.epochStartBlockNumber {
latestBlockNumber, log.Debug("latest block number is equal to epoch start block number", "number", latestBlockNumber)
uint64(g.epochLengthSeconds), return nil
uint64(g.averageBlockGasLimit), }
)
log.Debug("UpdateGasPrice", "averageGasPerSecond", averageGasPerSecond, "current-price", g.gasPricer.curPrice) // Accumulate the amount of gas that has been used in the epoch
totalGasUsed := uint64(0)
for i := g.epochStartBlockNumber + 1; i <= latestBlockNumber; i++ {
gasUsed, err := g.getGasUsedByBlockFn(new(big.Int).SetUint64(i))
log.Trace("fetching gas used", "height", i, "gas-used", gasUsed, "total-gas", totalGasUsed)
if err != nil {
return err
}
totalGasUsed += gasUsed
}
averageGasPerSecond := float64(totalGasUsed / g.epochLengthSeconds)
log.Debug("UpdateGasPrice", "average-gas-per-second", averageGasPerSecond, "current-price", g.gasPricer.curPrice)
_, err = g.gasPricer.CompleteEpoch(averageGasPerSecond) _, err = g.gasPricer.CompleteEpoch(averageGasPerSecond)
if err != nil { if err != nil {
return err return err
......
package gasprices package gasprices
import ( import (
"math/big"
"testing" "testing"
) )
...@@ -10,29 +11,12 @@ type MockEpoch struct { ...@@ -10,29 +11,12 @@ type MockEpoch struct {
postHook func(prevGasPrice uint64, gasPriceUpdater *GasPriceUpdater) postHook func(prevGasPrice uint64, gasPriceUpdater *GasPriceUpdater)
} }
func TestGetAverageGasPerSecond(t *testing.T) {
// Let's sanity check this function with some simple inputs.
// A 10 block epoch
epochStartBlockNumber := 10
latestBlockNumber := 20
// That lasts 10 seconds (1 block per second)
epochLengthSeconds := 10
// And each block has a gas limit of 1
averageBlockGasLimit := 1
// We expect a gas per second to be 1!
expectedGps := 1.0
gps := GetAverageGasPerSecond(uint64(epochStartBlockNumber), uint64(latestBlockNumber), uint64(epochLengthSeconds), uint64(averageBlockGasLimit))
if gps != expectedGps {
t.Fatalf("Gas per second not calculated correctly. Got: %v expected: %v", gps, expectedGps)
}
}
// Return a gas pricer that targets 3 blocks per epoch & 10% max change per epoch. // Return a gas pricer that targets 3 blocks per epoch & 10% max change per epoch.
func makeTestGasPricerAndUpdater(curPrice uint64) (*GasPricer, *GasPriceUpdater, func(uint64), error) { func makeTestGasPricerAndUpdater(curPrice uint64) (*GasPricer, *GasPriceUpdater, func(uint64), error) {
gpsTarget := 3300000.0 gpsTarget := 3300000.0
getGasTarget := func() float64 { return gpsTarget } getGasTarget := func() float64 { return gpsTarget }
epochLengthSeconds := uint64(10) epochLengthSeconds := uint64(10)
averageBlockGasLimit := 11000000.0 averageBlockGasLimit := uint64(11000000)
// Based on our 10 second epoch, we are targetting 3 blocks per epoch. // Based on our 10 second epoch, we are targetting 3 blocks per epoch.
gasPricer, err := NewGasPricer(curPrice, 1, getGasTarget, 10) gasPricer, err := NewGasPricer(curPrice, 1, getGasTarget, 10)
if err != nil { if err != nil {
...@@ -46,6 +30,12 @@ func makeTestGasPricerAndUpdater(curPrice uint64) (*GasPricer, *GasPriceUpdater, ...@@ -46,6 +30,12 @@ func makeTestGasPricerAndUpdater(curPrice uint64) (*GasPricer, *GasPriceUpdater,
return nil return nil
} }
// This is paramaterized based on 3 blocks per epoch, where each uses
// the average block gas limit plus an additional bit of gas added
getGasUsedByBlockFn := func(number *big.Int) (uint64, error) {
return averageBlockGasLimit*3/epochLengthSeconds + 1, nil
}
startBlock, _ := getLatestBlockNumber() startBlock, _ := getLatestBlockNumber()
gasUpdater, err := NewGasPriceUpdater( gasUpdater, err := NewGasPriceUpdater(
gasPricer, gasPricer,
...@@ -53,6 +43,7 @@ func makeTestGasPricerAndUpdater(curPrice uint64) (*GasPricer, *GasPriceUpdater, ...@@ -53,6 +43,7 @@ func makeTestGasPricerAndUpdater(curPrice uint64) (*GasPricer, *GasPriceUpdater,
averageBlockGasLimit, averageBlockGasLimit,
epochLengthSeconds, epochLengthSeconds,
getLatestBlockNumber, getLatestBlockNumber,
getGasUsedByBlockFn,
updateL2GasPrice, updateL2GasPrice,
) )
if err != nil { if err != nil {
...@@ -127,7 +118,7 @@ func TestUsageOfGasPriceUpdater(t *testing.T) { ...@@ -127,7 +118,7 @@ func TestUsageOfGasPriceUpdater(t *testing.T) {
postHook: func(prevGasPrice uint64, gasPriceUpdater *GasPriceUpdater) { postHook: func(prevGasPrice uint64, gasPriceUpdater *GasPriceUpdater) {
curPrice := gasPriceUpdater.gasPricer.curPrice curPrice := gasPriceUpdater.gasPricer.curPrice
if prevGasPrice >= curPrice { if prevGasPrice >= curPrice {
t.Fatalf("Expected gas price to increase.") t.Fatalf("Expected gas price to increase. Got %d, was %d", curPrice, prevGasPrice)
} }
}, },
}, },
...@@ -143,7 +134,7 @@ func TestUsageOfGasPriceUpdater(t *testing.T) { ...@@ -143,7 +134,7 @@ func TestUsageOfGasPriceUpdater(t *testing.T) {
postHook: func(prevGasPrice uint64, gasPriceUpdater *GasPriceUpdater) { postHook: func(prevGasPrice uint64, gasPriceUpdater *GasPriceUpdater) {
curPrice := gasPriceUpdater.gasPricer.curPrice curPrice := gasPriceUpdater.gasPricer.curPrice
if prevGasPrice != curPrice { if prevGasPrice != curPrice {
t.Fatalf("Expected gas price to stablize.") t.Fatalf("Expected gas price to stablize. Got %d, was %d", curPrice, prevGasPrice)
} }
}, },
}, },
......
...@@ -52,8 +52,10 @@ func (p *GasPricer) CalcNextEpochGasPrice(avgGasPerSecondLastEpoch float64) (uin ...@@ -52,8 +52,10 @@ func (p *GasPricer) CalcNextEpochGasPrice(avgGasPerSecondLastEpoch float64) (uin
} }
// The percent difference between our current average gas & our target gas // The percent difference between our current average gas & our target gas
proportionOfTarget := avgGasPerSecondLastEpoch / targetGasPerSecond proportionOfTarget := avgGasPerSecondLastEpoch / targetGasPerSecond
log.Trace("Calculating next epoch gas price", "proportionOfTarget", proportionOfTarget, log.Trace("Calculating next epoch gas price", "proportionOfTarget", proportionOfTarget,
"avgGasPerSecondLastEpoch", avgGasPerSecondLastEpoch, "targetGasPerSecond", targetGasPerSecond) "avgGasPerSecondLastEpoch", avgGasPerSecondLastEpoch, "targetGasPerSecond", targetGasPerSecond)
// The percent that we should adjust the gas price to reach our target gas // The percent that we should adjust the gas price to reach our target gas
proportionToChangeBy := 0.0 proportionToChangeBy := 0.0
if proportionOfTarget >= 1 { // If average avgGasPerSecondLastEpoch is GREATER than our target if proportionOfTarget >= 1 { // If average avgGasPerSecondLastEpoch is GREATER than our target
...@@ -61,10 +63,13 @@ func (p *GasPricer) CalcNextEpochGasPrice(avgGasPerSecondLastEpoch float64) (uin ...@@ -61,10 +63,13 @@ func (p *GasPricer) CalcNextEpochGasPrice(avgGasPerSecondLastEpoch float64) (uin
} else { } else {
proportionToChangeBy = math.Max(proportionOfTarget, 1-p.maxChangePerEpoch) proportionToChangeBy = math.Max(proportionOfTarget, 1-p.maxChangePerEpoch)
} }
updated := float64(max(1, p.curPrice)) * proportionToChangeBy updated := float64(max(1, p.curPrice)) * proportionToChangeBy
result := max(p.floorPrice, uint64(math.Ceil(updated))) result := max(p.floorPrice, uint64(math.Ceil(updated)))
log.Debug("Calculated next epoch gas price", "proportionToChangeBy", proportionToChangeBy, log.Debug("Calculated next epoch gas price", "proportionToChangeBy", proportionToChangeBy,
"proportionOfTarget", proportionOfTarget, "result", result) "proportionOfTarget", proportionOfTarget, "result", result)
return result, nil return result, nil
} }
......
...@@ -26,7 +26,7 @@ type Config struct { ...@@ -26,7 +26,7 @@ type Config struct {
floorPrice uint64 floorPrice uint64
targetGasPerSecond uint64 targetGasPerSecond uint64
maxPercentChangePerEpoch float64 maxPercentChangePerEpoch float64
averageBlockGasLimitPerEpoch float64 averageBlockGasLimitPerEpoch uint64
epochLengthSeconds uint64 epochLengthSeconds uint64
l2GasPriceSignificanceFactor float64 l2GasPriceSignificanceFactor float64
l1BaseFeeSignificanceFactor float64 l1BaseFeeSignificanceFactor float64
...@@ -52,7 +52,7 @@ func NewConfig(ctx *cli.Context) *Config { ...@@ -52,7 +52,7 @@ func NewConfig(ctx *cli.Context) *Config {
cfg.gasPriceOracleAddress = common.HexToAddress(addr) cfg.gasPriceOracleAddress = common.HexToAddress(addr)
cfg.targetGasPerSecond = ctx.GlobalUint64(flags.TargetGasPerSecondFlag.Name) cfg.targetGasPerSecond = ctx.GlobalUint64(flags.TargetGasPerSecondFlag.Name)
cfg.maxPercentChangePerEpoch = ctx.GlobalFloat64(flags.MaxPercentChangePerEpochFlag.Name) cfg.maxPercentChangePerEpoch = ctx.GlobalFloat64(flags.MaxPercentChangePerEpochFlag.Name)
cfg.averageBlockGasLimitPerEpoch = ctx.GlobalFloat64(flags.AverageBlockGasLimitPerEpochFlag.Name) cfg.averageBlockGasLimitPerEpoch = ctx.GlobalUint64(flags.AverageBlockGasLimitPerEpochFlag.Name)
cfg.epochLengthSeconds = ctx.GlobalUint64(flags.EpochLengthSecondsFlag.Name) cfg.epochLengthSeconds = ctx.GlobalUint64(flags.EpochLengthSecondsFlag.Name)
cfg.l2GasPriceSignificanceFactor = ctx.GlobalFloat64(flags.L2GasPriceSignificanceFactorFlag.Name) cfg.l2GasPriceSignificanceFactor = ctx.GlobalFloat64(flags.L2GasPriceSignificanceFactorFlag.Name)
cfg.floorPrice = ctx.GlobalUint64(flags.FloorPriceFlag.Name) cfg.floorPrice = ctx.GlobalUint64(flags.FloorPriceFlag.Name)
......
...@@ -271,6 +271,9 @@ func NewGasPriceOracle(cfg *Config) (*GasPriceOracle, error) { ...@@ -271,6 +271,9 @@ func NewGasPriceOracle(cfg *Config) (*GasPriceOracle, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
// getGasUsedByBlockFn is used by the GasPriceUpdater
// to fetch the amount of gas that a block has used
getGasUsedByBlockFn := wrapGetGasUsedByBlock(l2Client)
log.Info("Creating GasPriceUpdater", "epochStartBlockNumber", epochStartBlockNumber, log.Info("Creating GasPriceUpdater", "epochStartBlockNumber", epochStartBlockNumber,
"averageBlockGasLimitPerEpoch", cfg.averageBlockGasLimitPerEpoch, "averageBlockGasLimitPerEpoch", cfg.averageBlockGasLimitPerEpoch,
...@@ -282,6 +285,7 @@ func NewGasPriceOracle(cfg *Config) (*GasPriceOracle, error) { ...@@ -282,6 +285,7 @@ func NewGasPriceOracle(cfg *Config) (*GasPriceOracle, error) {
cfg.averageBlockGasLimitPerEpoch, cfg.averageBlockGasLimitPerEpoch,
cfg.epochLengthSeconds, cfg.epochLengthSeconds,
getLatestBlockNumberFn, getLatestBlockNumberFn,
getGasUsedByBlockFn,
updateL2GasPriceFn, updateL2GasPriceFn,
) )
......
...@@ -38,6 +38,19 @@ func wrapGetLatestBlockNumberFn(backend bind.ContractBackend) func() (uint64, er ...@@ -38,6 +38,19 @@ func wrapGetLatestBlockNumberFn(backend bind.ContractBackend) func() (uint64, er
} }
} }
// wrapGetGasUsedByBlock is used by the GasPriceUpdater to get
// the amount of gas used by a particular block. This is used to
// track gas usage over time
func wrapGetGasUsedByBlock(backend bind.ContractBackend) func(*big.Int) (uint64, error) {
return func(number *big.Int) (uint64, error) {
block, err := backend.HeaderByNumber(context.Background(), number)
if err != nil {
return 0, err
}
return block.GasUsed, nil
}
}
// DeployContractBackend represents the union of the // DeployContractBackend represents the union of the
// DeployBackend and the ContractBackend // DeployBackend and the ContractBackend
type DeployContractBackend interface { type DeployContractBackend interface {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment