From 188c21f1903b24344a19a4742749d93d49a4382f Mon Sep 17 00:00:00 2001 From: Sebastian Stammler Date: Wed, 20 Dec 2023 11:14:11 +0100 Subject: [PATCH] op-service/txmgr: Add threshold to gas price increase limiter --- op-service/txmgr/cli.go | 25 ++++++++++++++++++++ op-service/txmgr/txmgr.go | 30 +++++++++++++++++------- op-service/txmgr/txmgr_test.go | 43 +++++++++++++++++++++++++++------- 3 files changed, 81 insertions(+), 17 deletions(-) diff --git a/op-service/txmgr/cli.go b/op-service/txmgr/cli.go index 30f4225ecc459..5df9e2036c814 100644 --- a/op-service/txmgr/cli.go +++ b/op-service/txmgr/cli.go @@ -13,6 +13,7 @@ import ( "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" ) @@ -27,6 +28,7 @@ const ( NumConfirmationsFlagName = "num-confirmations" SafeAbortNonceTooLowCountFlagName = "safe-abort-nonce-too-low-count" FeeLimitMultiplierFlagName = "fee-limit-multiplier" + FeeLimitThresholdFlagName = "fee-limit-threshold" ResubmissionTimeoutFlagName = "resubmission-timeout" NetworkTimeoutFlagName = "network-timeout" TxSendTimeoutFlagName = "txmgr.send-timeout" @@ -53,6 +55,7 @@ type DefaultFlagValues struct { NumConfirmations uint64 SafeAbortNonceTooLowCount uint64 FeeLimitMultiplier uint64 + FeeLimitThresholdGwei float64 ResubmissionTimeout time.Duration NetworkTimeout time.Duration TxSendTimeout time.Duration @@ -65,6 +68,7 @@ var ( NumConfirmations: uint64(10), SafeAbortNonceTooLowCount: uint64(3), FeeLimitMultiplier: uint64(5), + FeeLimitThresholdGwei: 100.0, ResubmissionTimeout: 48 * time.Second, NetworkTimeout: 10 * time.Second, TxSendTimeout: 0 * time.Second, @@ -75,6 +79,7 @@ var ( NumConfirmations: uint64(3), SafeAbortNonceTooLowCount: uint64(3), FeeLimitMultiplier: uint64(5), + FeeLimitThresholdGwei: 100.0, ResubmissionTimeout: 24 * time.Second, NetworkTimeout: 10 * time.Second, TxSendTimeout: 2 * time.Minute, @@ -125,6 +130,12 @@ func CLIFlagsWithDefaults(envPrefix string, defaults DefaultFlagValues) []cli.Fl Value: defaults.FeeLimitMultiplier, EnvVars: prefixEnvVars("TXMGR_FEE_LIMIT_MULTIPLIER"), }, + &cli.Float64Flag{ + Name: FeeLimitThresholdFlagName, + Usage: "The minimum threshold (in GWei) at which fee bumping starts to be capped. Allows arbitrary fee bumps below this threshold.", + Value: defaults.FeeLimitThresholdGwei, + EnvVars: prefixEnvVars("TXMGR_FEE_LIMIT_THRESHOLD"), + }, &cli.DurationFlag{ Name: ResubmissionTimeoutFlagName, Usage: "Duration we will wait before resubmitting a transaction to L1", @@ -169,6 +180,7 @@ type CLIConfig struct { NumConfirmations uint64 SafeAbortNonceTooLowCount uint64 FeeLimitMultiplier uint64 + FeeLimitThresholdGwei float64 ResubmissionTimeout time.Duration ReceiptQueryInterval time.Duration NetworkTimeout time.Duration @@ -234,6 +246,7 @@ func ReadCLIConfig(ctx *cli.Context) CLIConfig { NumConfirmations: ctx.Uint64(NumConfirmationsFlagName), SafeAbortNonceTooLowCount: ctx.Uint64(SafeAbortNonceTooLowCountFlagName), FeeLimitMultiplier: ctx.Uint64(FeeLimitMultiplierFlagName), + FeeLimitThresholdGwei: ctx.Float64(FeeLimitThresholdFlagName), ResubmissionTimeout: ctx.Duration(ResubmissionTimeoutFlagName), ReceiptQueryInterval: ctx.Duration(ReceiptQueryIntervalFlagName), NetworkTimeout: ctx.Duration(NetworkTimeoutFlagName), @@ -274,10 +287,17 @@ func NewConfig(cfg CLIConfig, l log.Logger) (Config, error) { return Config{}, fmt.Errorf("could not init signer: %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) + return Config{ Backend: l1, ResubmissionTimeout: cfg.ResubmissionTimeout, FeeLimitMultiplier: cfg.FeeLimitMultiplier, + FeeLimitThreshold: feeLimitThreshold, ChainID: chainID, TxSendTimeout: cfg.TxSendTimeout, TxNotInMempoolTimeout: cfg.TxNotInMempoolTimeout, @@ -302,6 +322,11 @@ type Config struct { // The multiplier applied to fee suggestions to put a hard limit on fee increases. FeeLimitMultiplier uint64 + // Minimum threshold (in Wei) at which the FeeLimitMultiplier takes effect. + // On low-fee networks, like test networks, this allows for arbitrary fee bumps + // below this threshold. + FeeLimitThreshold *big.Int + // ChainID is the chain ID of the L1 chain. ChainID *big.Int diff --git a/op-service/txmgr/txmgr.go b/op-service/txmgr/txmgr.go index e77a43923db7a..252f63d2c189d 100644 --- a/op-service/txmgr/txmgr.go +++ b/op-service/txmgr/txmgr.go @@ -513,15 +513,10 @@ func (m *SimpleTxManager) increaseGasPrice(ctx context.Context, tx *types.Transa } bumpedTip, bumpedFee := updateFees(tx.GasTipCap(), tx.GasFeeCap(), tip, basefee, m.l) - // Make sure increase is at most [FeeLimitMultiplier] the suggested values - maxTip := new(big.Int).Mul(tip, big.NewInt(int64(m.cfg.FeeLimitMultiplier))) - if bumpedTip.Cmp(maxTip) > 0 { - return nil, fmt.Errorf("bumped tip cap %v is over %dx multiple of the suggested value", bumpedTip, m.cfg.FeeLimitMultiplier) - } - maxFee := calcGasFeeCap(new(big.Int).Mul(basefee, big.NewInt(int64(m.cfg.FeeLimitMultiplier))), maxTip) - if bumpedFee.Cmp(maxFee) > 0 { - return nil, fmt.Errorf("bumped fee cap %v is over %dx multiple of the suggested value", bumpedFee, m.cfg.FeeLimitMultiplier) + if err := m.checkLimits(tip, basefee, bumpedTip, bumpedFee); err != nil { + return nil, err } + rawTx := &types.DynamicFeeTx{ ChainID: tx.ChainId(), Nonce: tx.Nonce(), @@ -589,6 +584,25 @@ func (m *SimpleTxManager) suggestGasPriceCaps(ctx context.Context) (*big.Int, *b return tip, head.BaseFee, nil } +func (m *SimpleTxManager) checkLimits(tip, basefee, bumpedTip, bumpedFee *big.Int) error { + // If below threshold, don't apply multiplier limit + if thr := m.cfg.FeeLimitThreshold; thr != nil && thr.Cmp(bumpedFee) == 1 { + return nil + } + + // Make sure increase is at most [FeeLimitMultiplier] the suggested values + feeLimitMult := big.NewInt(int64(m.cfg.FeeLimitMultiplier)) + maxTip := new(big.Int).Mul(tip, feeLimitMult) + if bumpedTip.Cmp(maxTip) > 0 { + return fmt.Errorf("bumped tip cap %v is over %dx multiple of the suggested value", bumpedTip, m.cfg.FeeLimitMultiplier) + } + maxFee := calcGasFeeCap(new(big.Int).Mul(basefee, feeLimitMult), maxTip) + if bumpedFee.Cmp(maxFee) > 0 { + return fmt.Errorf("bumped fee cap %v is over %dx multiple of the suggested value", bumpedFee, m.cfg.FeeLimitMultiplier) + } + return nil +} + // calcThresholdValue returns x * priceBumpPercent / 100 func calcThresholdValue(x *big.Int) *big.Int { threshold := new(big.Int).Mul(priceBumpPercent, x) diff --git a/op-service/txmgr/txmgr_test.go b/op-service/txmgr/txmgr_test.go index e04966402946e..dd47cfeb233a1 100644 --- a/op-service/txmgr/txmgr_test.go +++ b/op-service/txmgr/txmgr_test.go @@ -19,6 +19,7 @@ import ( "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/params" ) type sendTransactionFunc func(ctx context.Context, tx *types.Transaction) error @@ -246,7 +247,6 @@ func (*mockBackend) ChainID(ctx context.Context) (*big.Int, error) { // receipt containing the txHash and the gasFeeCap used in the GasUsed to make // the value accessible from our test framework. func (b *mockBackend) TransactionReceipt(ctx context.Context, txHash common.Hash) (*types.Receipt, error) { - b.mu.RLock() defer b.mu.RUnlock() @@ -714,8 +714,8 @@ func (b *failingBackend) BlockNumber(ctx context.Context) (uint64, error) { // TransactionReceipt for the failingBackend returns errRpcFailure on the first // invocation, and a receipt containing the passed TxHash on the second. func (b *failingBackend) TransactionReceipt( - ctx context.Context, txHash common.Hash) (*types.Receipt, error) { - + ctx context.Context, txHash common.Hash, +) (*types.Receipt, error) { if !b.returnSuccessReceipt { b.returnSuccessReceipt = true return nil, errRpcFailure @@ -894,9 +894,32 @@ func TestIncreaseGasPrice(t *testing.T) { } } -// TestIncreaseGasPriceNotExponential asserts that if the L1 basefee & tip remain the -// same, repeated calls to IncreaseGasPrice do not continually increase the gas price. -func TestIncreaseGasPriceNotExponential(t *testing.T) { +// TestIncreaseGasPriceLimits asserts that if the L1 basefee & tip remain the +// same, repeated calls to IncreaseGasPrice eventually hit a limit. +func TestIncreaseGasPriceLimits(t *testing.T) { + t.Run("no-threshold", func(t *testing.T) { + testIncreaseGasPriceLimit(t, gasPriceLimitTest{ + expTipCap: 36, + expFeeCap: 493, // just below 5*100 + }) + }) + t.Run("with-threshold", func(t *testing.T) { + testIncreaseGasPriceLimit(t, gasPriceLimitTest{ + thr: big.NewInt(params.GWei), + expTipCap: 61_265_017, + expFeeCap: 957_582_949, // just below 1 gwei + }) + }) +} + +type gasPriceLimitTest struct { + thr *big.Int + expTipCap, expFeeCap int64 +} + +// testIncreaseGasPriceLimit runs a gas bumping test that increases the gas price until it hits an error. +// It starts with a tx that has a tip cap of 10 wei and fee cap of 100 wei. +func testIncreaseGasPriceLimit(t *testing.T, lt gasPriceLimitTest) { t.Parallel() borkedTip := int64(10) @@ -913,6 +936,7 @@ func TestIncreaseGasPriceNotExponential(t *testing.T) { NumConfirmations: 1, SafeAbortNonceTooLowCount: 3, FeeLimitMultiplier: 5, + FeeLimitThreshold: lt.thr, Signer: func(ctx context.Context, from common.Address, tx *types.Transaction) (*types.Transaction, error) { return tx, nil }, @@ -937,10 +961,11 @@ func TestIncreaseGasPriceNotExponential(t *testing.T) { } tx = newTx } + lastTip, lastFee := tx.GasTipCap(), tx.GasFeeCap() - require.Equal(t, lastTip.Int64(), int64(36)) - require.Equal(t, lastFee.Int64(), int64(493)) - // Confirm that fees stop rising + // Confirm that fees only rose until expected threshold + require.Equal(t, lt.expTipCap, lastTip.Int64()) + require.Equal(t, lt.expFeeCap, lastFee.Int64()) _, err := mgr.increaseGasPrice(ctx, tx) require.Error(t, err) }