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

op-deployer: Custom gas price estimator #12239

Merged
merged 4 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions op-chain-ops/deployer/broadcaster/gas_estimator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package broadcaster

import (
"context"
"fmt"
"math/big"

"github.com/ethereum-optimism/optimism/op-service/txmgr"
)

var (
// baseFeePadFactor = 20% as a divisor
baseFeePadFactor = big.NewInt(5)
// tipMulFactor = 10 as a multiplier
tipMulFactor = big.NewInt(10)
// dummyBlobFee is a dummy value for the blob fee. Since this gas estimator will never
// post blobs, it's just set to 1.
dummyBlobFee = big.NewInt(1)
)

// DeployerGasPriceEstimator is a custom gas price estimator for use with op-deployer.
// It pads the base fee by 20% and multiplies the suggested tip by 10.
func DeployerGasPriceEstimator(ctx context.Context, client txmgr.ETHBackend) (*big.Int, *big.Int, *big.Int, error) {
chainHead, err := client.HeaderByNumber(ctx, nil)
if err != nil {
return nil, nil, nil, fmt.Errorf("failed to get block: %w", err)
}

tip, err := client.SuggestGasTipCap(ctx)
if err != nil {
return nil, nil, nil, fmt.Errorf("failed to get gas tip cap: %w", err)
}

baseFeePad := new(big.Int).Div(chainHead.BaseFee, baseFeePadFactor)
paddedBaseFee := new(big.Int).Add(chainHead.BaseFee, baseFeePad)
paddedTip := new(big.Int).Mul(tip, tipMulFactor)
return paddedTip, paddedBaseFee, dummyBlobFee, nil
}
4 changes: 3 additions & 1 deletion op-chain-ops/deployer/broadcaster/keyed.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import (
"math/big"
"time"

"github.com/ethereum-optimism/optimism/op-service/eth"

"github.com/ethereum-optimism/optimism/op-chain-ops/script"
opcrypto "github.com/ethereum-optimism/optimism/op-service/crypto"
"github.com/ethereum-optimism/optimism/op-service/eth"
"github.com/ethereum-optimism/optimism/op-service/txmgr"
"github.com/ethereum-optimism/optimism/op-service/txmgr/metrics"
"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -51,6 +52,7 @@ func NewKeyedBroadcaster(cfg KeyedBroadcasterOpts) (*KeyedBroadcaster, error) {
SafeAbortNonceTooLowCount: 3,
Signer: cfg.Signer,
From: cfg.From,
GasPriceEstimatorFn: DeployerGasPriceEstimator,
}

minTipCap, err := eth.GweiToWei(1.0)
Expand Down
1 change: 1 addition & 0 deletions op-chain-ops/deployer/integration_test/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ participants:
- el_type: geth
el_extra_params:
- "--gcmode=archive"
- "--rpc.txfeecap=0"
mslipper marked this conversation as resolved.
Show resolved Hide resolved
cl_type: lighthouse
network_params:
prefunded_accounts: '{ "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266": { "balance": "1000000ETH" } }'
Expand Down
4 changes: 4 additions & 0 deletions op-service/txmgr/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,10 @@ type Config struct {
// Signer is used to sign transactions when the gas price is increased.
Signer opcrypto.SignerFn
From common.Address

// GasPriceEstimatorFn is used to estimate the gas price for a transaction.
// If nil, DefaultGasPriceEstimatorFn is used.
GasPriceEstimatorFn GasPriceEstimatorFn
}

func (m *Config) Check() error {
Expand Down
33 changes: 33 additions & 0 deletions op-service/txmgr/estimator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package txmgr

import (
"context"
"errors"
"math/big"

"github.com/ethereum/go-ethereum/consensus/misc/eip4844"
)

type GasPriceEstimatorFn func(ctx context.Context, backend ETHBackend) (*big.Int, *big.Int, *big.Int, error)

func DefaultGasPriceEstimatorFn(ctx context.Context, backend ETHBackend) (*big.Int, *big.Int, *big.Int, error) {
tip, err := backend.SuggestGasTipCap(ctx)
if err != nil {
return nil, nil, nil, err
}

head, err := backend.HeaderByNumber(ctx, nil)
if err != nil {
return nil, nil, nil, err
}
if head.BaseFee == nil {
return nil, nil, nil, errors.New("txmgr does not support pre-london blocks that do not have a base fee")
}

var blobFee *big.Int
if head.ExcessBlobGas != nil {
blobFee = eip4844.CalcBlobFee(*head.ExcessBlobGas)
}

return tip, head.BaseFee, blobFee, nil
}
49 changes: 19 additions & 30 deletions op-service/txmgr/txmgr.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,10 @@ type SimpleTxManager struct {
name string
chainID *big.Int

backend ETHBackend
l log.Logger
metr metrics.TxMetricer
backend ETHBackend
l log.Logger
metr metrics.TxMetricer
gasPriceEstimatorFn GasPriceEstimatorFn

nonce *uint64
nonceLock sync.RWMutex
Expand All @@ -163,13 +164,15 @@ func NewSimpleTxManagerFromConfig(name string, l log.Logger, m metrics.TxMetrice
if err := conf.Check(); err != nil {
return nil, fmt.Errorf("invalid config: %w", err)
}

return &SimpleTxManager{
chainID: conf.ChainID,
name: name,
cfg: conf,
backend: conf.Backend,
l: l.New("service", name),
metr: m,
chainID: conf.ChainID,
name: name,
cfg: conf,
backend: conf.Backend,
l: l.New("service", name),
metr: m,
gasPriceEstimatorFn: conf.GasPriceEstimatorFn,
}, nil
}

Expand Down Expand Up @@ -876,27 +879,18 @@ func (m *SimpleTxManager) increaseGasPrice(ctx context.Context, tx *types.Transa
func (m *SimpleTxManager) SuggestGasPriceCaps(ctx context.Context) (*big.Int, *big.Int, *big.Int, error) {
cCtx, cancel := context.WithTimeout(ctx, m.cfg.NetworkTimeout)
defer cancel()
tip, err := m.backend.SuggestGasTipCap(cCtx)
if err != nil {
m.metr.RPCError()
return nil, nil, nil, fmt.Errorf("failed to fetch the suggested gas tip cap: %w", err)
} else if tip == nil {
return nil, nil, nil, errors.New("the suggested tip was nil")

estimatorFn := m.gasPriceEstimatorFn
if estimatorFn == nil {
estimatorFn = DefaultGasPriceEstimatorFn
}
cCtx, cancel = context.WithTimeout(ctx, m.cfg.NetworkTimeout)
defer cancel()
head, err := m.backend.HeaderByNumber(cCtx, nil)

tip, baseFee, blobFee, err := estimatorFn(cCtx, m.backend)
if err != nil {
m.metr.RPCError()
return nil, nil, nil, fmt.Errorf("failed to fetch the suggested base fee: %w", err)
} else if head.BaseFee == nil {
return nil, nil, nil, errors.New("txmgr does not support pre-london blocks that do not have a base fee")
return nil, nil, nil, fmt.Errorf("failed to get gas price estimates: %w", err)
}

baseFee := head.BaseFee
m.metr.RecordBaseFee(baseFee)
m.metr.RecordTipCap(tip)
Comment on lines -897 to -898
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to remove these metric updates? RecordTipCap is now unreferenced / dead code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure this needs to be added back, both metrics.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


// Enforce minimum base fee and tip cap
minTipCap := m.cfg.MinTipCap.Load()
minBaseFee := m.cfg.MinBaseFee.Load()
Expand All @@ -910,11 +904,6 @@ func (m *SimpleTxManager) SuggestGasPriceCaps(ctx context.Context) (*big.Int, *b
baseFee = new(big.Int).Set(minBaseFee)
}

var blobFee *big.Int
if head.ExcessBlobGas != nil {
blobFee = eip4844.CalcBlobFee(*head.ExcessBlobGas)
m.metr.RecordBlobBaseFee(blobFee)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this metric also needs to be added back.

}
return tip, baseFee, blobFee, nil
}

Expand Down
45 changes: 29 additions & 16 deletions op-service/txmgr/txmgr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1079,7 +1079,7 @@ func TestWaitMinedReturnsReceiptAfterFailure(t *testing.T) {
require.Equal(t, receipt.TxHash, txHash)
}

func doGasPriceIncrease(t *testing.T, txTipCap, txFeeCap, newTip, newBaseFee int64) (*types.Transaction, *types.Transaction, error) {
func doGasPriceIncrease(t *testing.T, txTipCap, txFeeCap, newTip, newBaseFee int64, estimator GasPriceEstimatorFn) (*types.Transaction, *types.Transaction, error) {
borkedBackend := failingBackend{
gasTip: big.NewInt(newTip),
baseFee: big.NewInt(newBaseFee),
Expand All @@ -1100,11 +1100,12 @@ func doGasPriceIncrease(t *testing.T, txTipCap, txFeeCap, newTip, newBaseFee int
cfg.MinBlobTxFee.Store(defaultMinBlobTxFee)

mgr := &SimpleTxManager{
cfg: &cfg,
name: "TEST",
backend: &borkedBackend,
l: testlog.Logger(t, log.LevelCrit),
metr: &metrics.NoopTxMetrics{},
cfg: &cfg,
name: "TEST",
backend: &borkedBackend,
l: testlog.Logger(t, log.LevelCrit),
metr: &metrics.NoopTxMetrics{},
gasPriceEstimatorFn: estimator,
}

tx := types.NewTx(&types.DynamicFeeTx{
Expand All @@ -1125,7 +1126,7 @@ func TestIncreaseGasPrice(t *testing.T) {
{
name: "bump at least 1",
run: func(t *testing.T) {
tx, newTx, err := doGasPriceIncrease(t, 1, 3, 1, 1)
tx, newTx, err := doGasPriceIncrease(t, 1, 3, 1, 1, DefaultGasPriceEstimatorFn)
require.True(t, newTx.GasFeeCap().Cmp(tx.GasFeeCap()) > 0, "new tx fee cap must be larger")
require.True(t, newTx.GasTipCap().Cmp(tx.GasTipCap()) > 0, "new tx tip must be larger")
require.NoError(t, err)
Expand All @@ -1134,7 +1135,7 @@ func TestIncreaseGasPrice(t *testing.T) {
{
name: "enforces min bump",
run: func(t *testing.T) {
tx, newTx, err := doGasPriceIncrease(t, 100, 1000, 101, 460)
tx, newTx, err := doGasPriceIncrease(t, 100, 1000, 101, 460, DefaultGasPriceEstimatorFn)
require.True(t, newTx.GasFeeCap().Cmp(tx.GasFeeCap()) > 0, "new tx fee cap must be larger")
require.True(t, newTx.GasTipCap().Cmp(tx.GasTipCap()) > 0, "new tx tip must be larger")
require.NoError(t, err)
Expand All @@ -1143,7 +1144,7 @@ func TestIncreaseGasPrice(t *testing.T) {
{
name: "enforces min bump on only tip increase",
run: func(t *testing.T) {
tx, newTx, err := doGasPriceIncrease(t, 100, 1000, 101, 440)
tx, newTx, err := doGasPriceIncrease(t, 100, 1000, 101, 440, DefaultGasPriceEstimatorFn)
require.True(t, newTx.GasFeeCap().Cmp(tx.GasFeeCap()) > 0, "new tx fee cap must be larger")
require.True(t, newTx.GasTipCap().Cmp(tx.GasTipCap()) > 0, "new tx tip must be larger")
require.NoError(t, err)
Expand All @@ -1152,7 +1153,7 @@ func TestIncreaseGasPrice(t *testing.T) {
{
name: "enforces min bump on only base fee increase",
run: func(t *testing.T) {
tx, newTx, err := doGasPriceIncrease(t, 100, 1000, 99, 460)
tx, newTx, err := doGasPriceIncrease(t, 100, 1000, 99, 460, DefaultGasPriceEstimatorFn)
require.True(t, newTx.GasFeeCap().Cmp(tx.GasFeeCap()) > 0, "new tx fee cap must be larger")
require.True(t, newTx.GasTipCap().Cmp(tx.GasTipCap()) > 0, "new tx tip must be larger")
require.NoError(t, err)
Expand All @@ -1161,7 +1162,7 @@ func TestIncreaseGasPrice(t *testing.T) {
{
name: "uses L1 values when larger",
run: func(t *testing.T) {
_, newTx, err := doGasPriceIncrease(t, 10, 100, 50, 200)
_, newTx, err := doGasPriceIncrease(t, 10, 100, 50, 200, DefaultGasPriceEstimatorFn)
require.True(t, newTx.GasFeeCap().Cmp(big.NewInt(450)) == 0, "new tx fee cap must be equal L1")
require.True(t, newTx.GasTipCap().Cmp(big.NewInt(50)) == 0, "new tx tip must be equal L1")
require.NoError(t, err)
Expand All @@ -1170,7 +1171,7 @@ func TestIncreaseGasPrice(t *testing.T) {
{
name: "uses L1 tip when larger and threshold FC",
run: func(t *testing.T) {
_, newTx, err := doGasPriceIncrease(t, 100, 2200, 120, 1050)
_, newTx, err := doGasPriceIncrease(t, 100, 2200, 120, 1050, DefaultGasPriceEstimatorFn)
require.True(t, newTx.GasTipCap().Cmp(big.NewInt(120)) == 0, "new tx tip must be equal L1")
require.True(t, newTx.GasFeeCap().Cmp(big.NewInt(2420)) == 0, "new tx fee cap must be equal to the threshold value")
require.NoError(t, err)
Expand All @@ -1179,37 +1180,49 @@ func TestIncreaseGasPrice(t *testing.T) {
{
name: "bumped fee above multiplier limit",
run: func(t *testing.T) {
_, _, err := doGasPriceIncrease(t, 1, 9999, 1, 1)
_, _, err := doGasPriceIncrease(t, 1, 9999, 1, 1, DefaultGasPriceEstimatorFn)
require.ErrorContains(t, err, "fee cap")
require.NotContains(t, err.Error(), "tip cap")
},
},
{
name: "bumped tip above multiplier limit",
run: func(t *testing.T) {
_, _, err := doGasPriceIncrease(t, 9999, 0, 0, 9999)
_, _, err := doGasPriceIncrease(t, 9999, 0, 0, 9999, DefaultGasPriceEstimatorFn)
require.ErrorContains(t, err, "tip cap")
require.NotContains(t, err.Error(), "fee cap")
},
},
{
name: "bumped fee and tip above multiplier limit",
run: func(t *testing.T) {
_, _, err := doGasPriceIncrease(t, 9999, 9999, 1, 1)
_, _, err := doGasPriceIncrease(t, 9999, 9999, 1, 1, DefaultGasPriceEstimatorFn)
require.ErrorContains(t, err, "tip cap")
require.ErrorContains(t, err, "fee cap")
},
},
{
name: "uses L1 FC when larger and threshold tip",
run: func(t *testing.T) {
_, newTx, err := doGasPriceIncrease(t, 100, 2200, 100, 2000)
_, newTx, err := doGasPriceIncrease(t, 100, 2200, 100, 2000, DefaultGasPriceEstimatorFn)
require.True(t, newTx.GasTipCap().Cmp(big.NewInt(110)) == 0, "new tx tip must be equal the threshold value")
t.Log("Vals:", newTx.GasFeeCap())
require.True(t, newTx.GasFeeCap().Cmp(big.NewInt(4110)) == 0, "new tx fee cap must be equal L1")
require.NoError(t, err)
},
},
{
name: "supports extension through custom estimator",
run: func(t *testing.T) {
estimator := func(ctx context.Context, backend ETHBackend) (*big.Int, *big.Int, *big.Int, error) {
return big.NewInt(100), big.NewInt(3000), big.NewInt(100), nil
}
_, newTx, err := doGasPriceIncrease(t, 70, 2000, 80, 2100, estimator)
require.NoError(t, err)
require.True(t, newTx.GasFeeCap().Cmp(big.NewInt(6100)) == 0)
require.True(t, newTx.GasTipCap().Cmp(big.NewInt(100)) == 0)
},
},
}
for _, test := range tests {
test := test
Expand Down