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-service/txmgr: Add threshold to gas price increase limiter #8699

Merged
merged 1 commit into from
Dec 20, 2023
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
31 changes: 31 additions & 0 deletions op-service/txmgr/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"math"
"math/big"
"time"

Expand All @@ -13,6 +14,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"
)

Expand All @@ -27,6 +29,7 @@ const (
NumConfirmationsFlagName = "num-confirmations"
SafeAbortNonceTooLowCountFlagName = "safe-abort-nonce-too-low-count"
FeeLimitMultiplierFlagName = "fee-limit-multiplier"
FeeLimitThresholdFlagName = "txmgr.fee-limit-threshold"
ResubmissionTimeoutFlagName = "resubmission-timeout"
NetworkTimeoutFlagName = "network-timeout"
TxSendTimeoutFlagName = "txmgr.send-timeout"
Expand All @@ -53,6 +56,7 @@ type DefaultFlagValues struct {
NumConfirmations uint64
SafeAbortNonceTooLowCount uint64
FeeLimitMultiplier uint64
FeeLimitThresholdGwei float64
ResubmissionTimeout time.Duration
NetworkTimeout time.Duration
TxSendTimeout time.Duration
Expand All @@ -65,6 +69,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,
Expand All @@ -75,6 +80,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,
Expand Down Expand Up @@ -125,6 +131,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",
Expand Down Expand Up @@ -169,6 +181,7 @@ type CLIConfig struct {
NumConfirmations uint64
SafeAbortNonceTooLowCount uint64
FeeLimitMultiplier uint64
FeeLimitThresholdGwei float64
ResubmissionTimeout time.Duration
ReceiptQueryInterval time.Duration
NetworkTimeout time.Duration
Expand All @@ -182,6 +195,7 @@ func NewCLIConfig(l1RPCURL string, defaults DefaultFlagValues) CLIConfig {
NumConfirmations: defaults.NumConfirmations,
SafeAbortNonceTooLowCount: defaults.SafeAbortNonceTooLowCount,
FeeLimitMultiplier: defaults.FeeLimitMultiplier,
FeeLimitThresholdGwei: defaults.FeeLimitThresholdGwei,
ResubmissionTimeout: defaults.ResubmissionTimeout,
NetworkTimeout: defaults.NetworkTimeout,
TxSendTimeout: defaults.TxSendTimeout,
Expand Down Expand Up @@ -234,6 +248,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),
Expand Down Expand Up @@ -274,10 +289,21 @@ 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)
}

// convert float GWei value into integer Wei value
feeLimitThreshold, _ := new(big.Float).Mul(
big.NewFloat(cfg.FeeLimitThresholdGwei),
big.NewFloat(params.GWei)).
Int(nil)
sebastianst marked this conversation as resolved.
Show resolved Hide resolved

return Config{
Backend: l1,
ResubmissionTimeout: cfg.ResubmissionTimeout,
FeeLimitMultiplier: cfg.FeeLimitMultiplier,
FeeLimitThreshold: feeLimitThreshold,
ChainID: chainID,
TxSendTimeout: cfg.TxSendTimeout,
TxNotInMempoolTimeout: cfg.TxNotInMempoolTimeout,
Expand All @@ -302,6 +328,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

Expand Down
30 changes: 22 additions & 8 deletions op-service/txmgr/txmgr.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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 {
sebastianst marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand Down
43 changes: 34 additions & 9 deletions op-service/txmgr/txmgr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
},
Expand All @@ -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)
}
Expand Down