From 104909cb8da66d7ce59e2e1dc79b6b7bce58af07 Mon Sep 17 00:00:00 2001 From: Sebastian Stammler Date: Wed, 3 Jan 2024 17:09:59 +0100 Subject: [PATCH] txmgr: Add min basefee and tip cap parameters to enforce fee minima (#8799) * txmgr: Add min basefee and tip cap parameters to enforce fee minima * op-service: Move GweiToWei into package eth --- op-service/eth/ether.go | 29 ++++++++++++++ op-service/eth/ether_test.go | 71 ++++++++++++++++++++++++++++++++++ op-service/txmgr/cli.go | 54 +++++++++++++++++++++----- op-service/txmgr/txmgr.go | 16 +++++++- op-service/txmgr/txmgr_test.go | 71 +++++++++++++++++++++++++++++++++- 5 files changed, 228 insertions(+), 13 deletions(-) create mode 100644 op-service/eth/ether.go create mode 100644 op-service/eth/ether_test.go diff --git a/op-service/eth/ether.go b/op-service/eth/ether.go new file mode 100644 index 000000000000..ad0d9152e647 --- /dev/null +++ b/op-service/eth/ether.go @@ -0,0 +1,29 @@ +package eth + +import ( + "errors" + "fmt" + "math" + "math/big" + + "github.com/ethereum/go-ethereum/accounts/abi" + "github.com/ethereum/go-ethereum/params" +) + +func GweiToWei(gwei float64) (*big.Int, error) { + if math.IsNaN(gwei) || math.IsInf(gwei, 0) { + return nil, fmt.Errorf("invalid gwei value: %v", gwei) + } + + // convert float GWei value into integer Wei value + wei, _ := new(big.Float).Mul( + big.NewFloat(gwei), + big.NewFloat(params.GWei)). + Int(nil) + + if wei.Cmp(abi.MaxUint256) == 1 { + return nil, errors.New("gwei value larger than max uint256") + } + + return wei, nil +} diff --git a/op-service/eth/ether_test.go b/op-service/eth/ether_test.go new file mode 100644 index 000000000000..f313619a4521 --- /dev/null +++ b/op-service/eth/ether_test.go @@ -0,0 +1,71 @@ +package eth + +import ( + "math" + "math/big" + "testing" + + "github.com/ethereum/go-ethereum/accounts/abi" + "github.com/stretchr/testify/require" +) + +func TestGweiToWei(t *testing.T) { + maxUint256p1, _ := new(big.Int).Add(abi.MaxUint256, big.NewInt(1)).Float64() + for _, tt := range []struct { + desc string + gwei float64 + wei *big.Int + err bool + }{ + { + desc: "zero", + gwei: 0, + wei: new(big.Int), + }, + { + desc: "one-wei", + gwei: 0.000000001, + wei: big.NewInt(1), + }, + { + desc: "one-gwei", + gwei: 1.0, + wei: big.NewInt(1e9), + }, + { + desc: "one-ether", + gwei: 1e9, + wei: big.NewInt(1e18), + }, + { + desc: "err-pos-inf", + gwei: math.Inf(1), + err: true, + }, + { + desc: "err-neg-inf", + gwei: math.Inf(-1), + err: true, + }, + { + desc: "err-nan", + gwei: math.NaN(), + err: true, + }, + { + desc: "err-too-large", + gwei: maxUint256p1, + err: true, + }, + } { + t.Run(tt.desc, func(t *testing.T) { + wei, err := GweiToWei(tt.gwei) + if tt.err { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tt.wei, wei) + } + }) + } +} diff --git a/op-service/txmgr/cli.go b/op-service/txmgr/cli.go index c83a3313bf00..009e5d0f8844 100644 --- a/op-service/txmgr/cli.go +++ b/op-service/txmgr/cli.go @@ -4,17 +4,16 @@ import ( "context" "errors" "fmt" - "math" "math/big" "time" opservice "github.com/ethereum-optimism/optimism/op-service" opcrypto "github.com/ethereum-optimism/optimism/op-service/crypto" + "github.com/ethereum-optimism/optimism/op-service/eth" opsigner "github.com/ethereum-optimism/optimism/op-service/signer" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/ethclient" "github.com/ethereum/go-ethereum/log" - "github.com/ethereum/go-ethereum/params" "github.com/urfave/cli/v2" ) @@ -30,6 +29,8 @@ const ( SafeAbortNonceTooLowCountFlagName = "safe-abort-nonce-too-low-count" FeeLimitMultiplierFlagName = "fee-limit-multiplier" FeeLimitThresholdFlagName = "txmgr.fee-limit-threshold" + MinBasefeeFlagName = "txmgr.min-basefee" + MinTipCapFlagName = "txmgr.min-tip-cap" ResubmissionTimeoutFlagName = "resubmission-timeout" NetworkTimeoutFlagName = "network-timeout" TxSendTimeoutFlagName = "txmgr.send-timeout" @@ -137,6 +138,16 @@ func CLIFlagsWithDefaults(envPrefix string, defaults DefaultFlagValues) []cli.Fl Value: defaults.FeeLimitThresholdGwei, EnvVars: prefixEnvVars("TXMGR_FEE_LIMIT_THRESHOLD"), }, + &cli.Float64Flag{ + Name: MinBasefeeFlagName, + Usage: "Enforces a minimum basefee (in GWei) to assume when determining tx fees. Off by default.", + EnvVars: prefixEnvVars("TXMGR_MIN_BASEFEE"), + }, + &cli.Float64Flag{ + Name: MinTipCapFlagName, + Usage: "Enforces a minimum tip cap (in GWei) to use when determining tx fees. Off by default.", + EnvVars: prefixEnvVars("TXMGR_MIN_TIP_CAP"), + }, &cli.DurationFlag{ Name: ResubmissionTimeoutFlagName, Usage: "Duration we will wait before resubmitting a transaction to L1", @@ -182,6 +193,8 @@ type CLIConfig struct { SafeAbortNonceTooLowCount uint64 FeeLimitMultiplier uint64 FeeLimitThresholdGwei float64 + MinBasefeeGwei float64 + MinTipCapGwei float64 ResubmissionTimeout time.Duration ReceiptQueryInterval time.Duration NetworkTimeout time.Duration @@ -218,6 +231,10 @@ func (m CLIConfig) Check() error { if m.FeeLimitMultiplier == 0 { return errors.New("must provide FeeLimitMultiplier") } + if m.MinBasefeeGwei < m.MinTipCapGwei { + return fmt.Errorf("minBasefee smaller than minTipCap, have %f < %f", + m.MinBasefeeGwei, m.MinTipCapGwei) + } if m.ResubmissionTimeout == 0 { return errors.New("must provide ResubmissionTimeout") } @@ -249,6 +266,8 @@ func ReadCLIConfig(ctx *cli.Context) CLIConfig { SafeAbortNonceTooLowCount: ctx.Uint64(SafeAbortNonceTooLowCountFlagName), FeeLimitMultiplier: ctx.Uint64(FeeLimitMultiplierFlagName), FeeLimitThresholdGwei: ctx.Float64(FeeLimitThresholdFlagName), + MinBasefeeGwei: ctx.Float64(MinBasefeeFlagName), + MinTipCapGwei: ctx.Float64(MinTipCapFlagName), ResubmissionTimeout: ctx.Duration(ResubmissionTimeoutFlagName), ReceiptQueryInterval: ctx.Duration(ReceiptQueryIntervalFlagName), NetworkTimeout: ctx.Duration(NetworkTimeoutFlagName), @@ -289,21 +308,28 @@ func NewConfig(cfg CLIConfig, l log.Logger) (Config, error) { return Config{}, fmt.Errorf("could not init signer: %w", err) } - if thr := cfg.FeeLimitThresholdGwei; math.IsNaN(thr) || math.IsInf(thr, 0) { - return Config{}, fmt.Errorf("invalid fee limit threshold: %v", thr) + feeLimitThreshold, err := eth.GweiToWei(cfg.FeeLimitThresholdGwei) + if err != nil { + return Config{}, fmt.Errorf("invalid fee limit threshold: %w", err) + } + + minBasefee, err := eth.GweiToWei(cfg.MinBasefeeGwei) + if err != nil { + return Config{}, fmt.Errorf("invalid min basefee: %w", err) } - // convert float GWei value into integer Wei value - feeLimitThreshold, _ := new(big.Float).Mul( - big.NewFloat(cfg.FeeLimitThresholdGwei), - big.NewFloat(params.GWei)). - Int(nil) + minTipCap, err := eth.GweiToWei(cfg.MinTipCapGwei) + if err != nil { + return Config{}, fmt.Errorf("invalid min tip cap: %w", err) + } return Config{ Backend: l1, ResubmissionTimeout: cfg.ResubmissionTimeout, FeeLimitMultiplier: cfg.FeeLimitMultiplier, FeeLimitThreshold: feeLimitThreshold, + MinBasefee: minBasefee, + MinTipCap: minTipCap, ChainID: chainID, TxSendTimeout: cfg.TxSendTimeout, TxNotInMempoolTimeout: cfg.TxNotInMempoolTimeout, @@ -333,6 +359,12 @@ type Config struct { // below this threshold. FeeLimitThreshold *big.Int + // Minimum basefee (in Wei) to assume when determining tx fees. + MinBasefee *big.Int + + // Minimum tip cap (in Wei) to enforce when determining tx fees. + MinTipCap *big.Int + // ChainID is the chain ID of the L1 chain. ChainID *big.Int @@ -380,6 +412,10 @@ func (m Config) Check() error { if m.FeeLimitMultiplier == 0 { return errors.New("must provide FeeLimitMultiplier") } + if m.MinBasefee != nil && m.MinTipCap != nil && m.MinBasefee.Cmp(m.MinTipCap) == -1 { + return fmt.Errorf("minBasefee smaller than minTipCap, have %v < %v", + m.MinBasefee, m.MinTipCap) + } if m.ResubmissionTimeout == 0 { return errors.New("must provide ResubmissionTimeout") } diff --git a/op-service/txmgr/txmgr.go b/op-service/txmgr/txmgr.go index 80181dd5e10f..defabdbd599a 100644 --- a/op-service/txmgr/txmgr.go +++ b/op-service/txmgr/txmgr.go @@ -594,9 +594,21 @@ func (m *SimpleTxManager) suggestGasPriceCaps(ctx context.Context) (*big.Int, *b } else if head.BaseFee == nil { return nil, nil, errors.New("txmgr does not support pre-london blocks that do not have a basefee") } - m.metr.RecordBasefee(head.BaseFee) + basefee := head.BaseFee + m.metr.RecordBasefee(basefee) m.metr.RecordTipCap(tip) - return tip, head.BaseFee, nil + + // Enforce minimum basefee and tip cap + if minTipCap := m.cfg.MinTipCap; minTipCap != nil && tip.Cmp(minTipCap) == -1 { + m.l.Debug("Enforcing min tip cap", "minTipCap", m.cfg.MinTipCap, "origTipCap", tip) + tip = new(big.Int).Set(m.cfg.MinTipCap) + } + if minBasefee := m.cfg.MinBasefee; minBasefee != nil && basefee.Cmp(minBasefee) == -1 { + m.l.Debug("Enforcing min basefee", "minBasefee", m.cfg.MinBasefee, "origBasefee", basefee) + basefee = new(big.Int).Set(m.cfg.MinBasefee) + } + + return tip, basefee, nil } func (m *SimpleTxManager) checkLimits(tip, basefee, bumpedTip, bumpedFee *big.Int) error { diff --git a/op-service/txmgr/txmgr_test.go b/op-service/txmgr/txmgr_test.go index a6b406ba35ad..cab9f66ca398 100644 --- a/op-service/txmgr/txmgr_test.go +++ b/op-service/txmgr/txmgr_test.go @@ -871,7 +871,7 @@ func TestIncreaseGasPrice(t *testing.T) { }, }, { - name: "enforces min bump on only tip incrase", + name: "enforces min bump on only tip increase", run: func(t *testing.T) { tx, newTx := doGasPriceIncrease(t, 100, 1000, 101, 440) require.True(t, newTx.GasFeeCap().Cmp(tx.GasFeeCap()) > 0, "new tx fee cap must be larger") @@ -879,7 +879,7 @@ func TestIncreaseGasPrice(t *testing.T) { }, }, { - name: "enforces min bump on only basefee incrase", + name: "enforces min bump on only basefee increase", run: func(t *testing.T) { tx, newTx := doGasPriceIncrease(t, 100, 1000, 99, 460) require.True(t, newTx.GasFeeCap().Cmp(tx.GasFeeCap()) > 0, "new tx fee cap must be larger") @@ -1053,3 +1053,70 @@ func TestNonceReset(t *testing.T) { // internal nonce tracking should be reset every 3rd tx require.Equal(t, []uint64{0, 0, 1, 2, 0, 1, 2, 0}, nonces) } + +func TestMinFees(t *testing.T) { + for _, tt := range []struct { + desc string + minBasefee *big.Int + minTipCap *big.Int + expectMinBasefee bool + expectMinTipCap bool + }{ + { + desc: "no-mins", + }, + { + desc: "high-min-basefee", + minBasefee: big.NewInt(10_000_000), + expectMinBasefee: true, + }, + { + desc: "high-min-tipcap", + minTipCap: big.NewInt(1_000_000), + expectMinTipCap: true, + }, + { + desc: "high-mins", + minBasefee: big.NewInt(10_000_000), + minTipCap: big.NewInt(1_000_000), + expectMinBasefee: true, + expectMinTipCap: true, + }, + { + desc: "low-min-basefee", + minBasefee: big.NewInt(1), + }, + { + desc: "low-min-tipcap", + minTipCap: big.NewInt(1), + }, + { + desc: "low-mins", + minBasefee: big.NewInt(1), + minTipCap: big.NewInt(1), + }, + } { + t.Run(tt.desc, func(t *testing.T) { + require := require.New(t) + conf := configWithNumConfs(1) + conf.MinBasefee = tt.minBasefee + conf.MinTipCap = tt.minTipCap + h := newTestHarnessWithConfig(t, conf) + + tip, basefee, err := h.mgr.suggestGasPriceCaps(context.TODO()) + require.NoError(err) + + if tt.expectMinBasefee { + require.Equal(tt.minBasefee, basefee, "expect suggested basefee to equal MinBasefee") + } else { + require.Equal(h.gasPricer.baseBaseFee, basefee, "expect suggested basefee to equal mock basefee") + } + + if tt.expectMinTipCap { + require.Equal(tt.minTipCap, tip, "expect suggested tip to equal MinTipCap") + } else { + require.Equal(h.gasPricer.baseGasTipFee, tip, "expect suggested tip to equal mock tip") + } + }) + } +}