From 1217d4ac74e4dfddc23391f41e50e51f7c64a772 Mon Sep 17 00:00:00 2001 From: Matthew Slipper Date: Wed, 2 Oct 2024 10:18:22 -0600 Subject: [PATCH] op-deployer: Custom gas price estimator (#12239) * op-deployer: Custom gas price estimator Sepolia's fees are super high and extremely volatile right now. As a result, it became difficult for users to deploy new chains using op-deployer. The OPCM's deploy transaction buys most of the gas in the block, and the default gas price estimation logic in the transaction manager wasn't aggressive enough for the transactions to land on chain in a timely manner. This PR adds the ability to customize the transaction manager with a custom `GasPriceEstimator` function that returns the tip, base fee, and blob fee. I extracted the original logic in the transaction manager into a default estimator that will be used if one isn't specified. For op-deployer, I built a custom estimator that pads the head block's base fee by 20%, and multiplies the suggested tip by 10. After testing this, I was able to get transactions onto Sepolia reliably. The algorithm is pretty simple and overpays for transactions by a lot, but for op-deployer's use case it's more important that transactions land quickly than it is they be cheap. Deployments are a one-time thing. * code review updates * better default support * specific test for extension --- .../deployer/broadcaster/gas_estimator.go | 38 ++++++++++++++ op-chain-ops/deployer/broadcaster/keyed.go | 4 +- .../deployer/integration_test/apply_test.go | 1 + op-service/txmgr/cli.go | 4 ++ op-service/txmgr/estimator.go | 33 +++++++++++++ op-service/txmgr/txmgr.go | 49 +++++++------------ op-service/txmgr/txmgr_test.go | 45 +++++++++++------ 7 files changed, 127 insertions(+), 47 deletions(-) create mode 100644 op-chain-ops/deployer/broadcaster/gas_estimator.go create mode 100644 op-service/txmgr/estimator.go diff --git a/op-chain-ops/deployer/broadcaster/gas_estimator.go b/op-chain-ops/deployer/broadcaster/gas_estimator.go new file mode 100644 index 000000000000..dc877bed0dc8 --- /dev/null +++ b/op-chain-ops/deployer/broadcaster/gas_estimator.go @@ -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 +} diff --git a/op-chain-ops/deployer/broadcaster/keyed.go b/op-chain-ops/deployer/broadcaster/keyed.go index 879d38b329b9..4768f31afc4a 100644 --- a/op-chain-ops/deployer/broadcaster/keyed.go +++ b/op-chain-ops/deployer/broadcaster/keyed.go @@ -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" @@ -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) diff --git a/op-chain-ops/deployer/integration_test/apply_test.go b/op-chain-ops/deployer/integration_test/apply_test.go index be4ef80e6374..184269618f0e 100644 --- a/op-chain-ops/deployer/integration_test/apply_test.go +++ b/op-chain-ops/deployer/integration_test/apply_test.go @@ -29,6 +29,7 @@ participants: - el_type: geth el_extra_params: - "--gcmode=archive" + - "--rpc.txfeecap=0" cl_type: lighthouse network_params: prefunded_accounts: '{ "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266": { "balance": "1000000ETH" } }' diff --git a/op-service/txmgr/cli.go b/op-service/txmgr/cli.go index fe65b6dd126e..2390933d79ca 100644 --- a/op-service/txmgr/cli.go +++ b/op-service/txmgr/cli.go @@ -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 { diff --git a/op-service/txmgr/estimator.go b/op-service/txmgr/estimator.go new file mode 100644 index 000000000000..c9968a1018a7 --- /dev/null +++ b/op-service/txmgr/estimator.go @@ -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 +} diff --git a/op-service/txmgr/txmgr.go b/op-service/txmgr/txmgr.go index 4e4c3e633f87..643337a147e7 100644 --- a/op-service/txmgr/txmgr.go +++ b/op-service/txmgr/txmgr.go @@ -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 @@ -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 } @@ -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) - // Enforce minimum base fee and tip cap minTipCap := m.cfg.MinTipCap.Load() minBaseFee := m.cfg.MinBaseFee.Load() @@ -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) - } return tip, baseFee, blobFee, nil } diff --git a/op-service/txmgr/txmgr_test.go b/op-service/txmgr/txmgr_test.go index 6bafa69464b6..0b246fd93238 100644 --- a/op-service/txmgr/txmgr_test.go +++ b/op-service/txmgr/txmgr_test.go @@ -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), @@ -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{ @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) @@ -1179,7 +1180,7 @@ 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") }, @@ -1187,7 +1188,7 @@ func TestIncreaseGasPrice(t *testing.T) { { 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") }, @@ -1195,7 +1196,7 @@ func TestIncreaseGasPrice(t *testing.T) { { 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") }, @@ -1203,13 +1204,25 @@ func TestIncreaseGasPrice(t *testing.T) { { 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