Skip to content

Commit

Permalink
op-service/txmgr: Add threshold to gas price increase limiter
Browse files Browse the repository at this point in the history
  • Loading branch information
sebastianst committed Dec 20, 2023
1 parent c04cefe commit 89b16aa
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 17 deletions.
25 changes: 25 additions & 0 deletions op-service/txmgr/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -27,6 +28,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 +55,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 +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,
Expand All @@ -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,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -169,6 +180,7 @@ type CLIConfig struct {
NumConfirmations uint64
SafeAbortNonceTooLowCount uint64
FeeLimitMultiplier uint64
FeeLimitThresholdGwei float64
ResubmissionTimeout time.Duration
ReceiptQueryInterval time.Duration
NetworkTimeout time.Duration
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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,
Expand All @@ -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

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 {
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

0 comments on commit 89b16aa

Please sign in to comment.