Commit d39144d5 authored by Joshua Gutow's avatar Joshua Gutow

op-service/txmgr: Return previous tx if no updates

Return the same transaction if we don't update any fields. We do this
because Ethereum signatures are not deterministic and therefore the
transaction hash will change wwhen we re-sign the tx. We don't want
to do that because we want to see ErrAlreadyKnown instead of
ErrReplacementUnderpriced.
parent 480fe38f
...@@ -122,6 +122,11 @@ func (m *SimpleTxManager) IncreaseGasPrice(ctx context.Context, tx *types.Transa ...@@ -122,6 +122,11 @@ func (m *SimpleTxManager) IncreaseGasPrice(ctx context.Context, tx *types.Transa
gasTipCap = tip gasTipCap = tip
} }
// Return the same transaction if we don't update any fields.
// We do this because ethereum signatures are not deterministic and therefore the transaction hash will change
// when we re-sign the tx. We don't want to do that because we want to see ErrAlreadyKnown instead of ErrReplacementUnderpriced
var reusedTip, reusedFeeCap bool
// new = old * (100 + priceBump) / 100 // new = old * (100 + priceBump) / 100
// Enforce a min priceBump on the tip. Do this before the feeCap is calculated // Enforce a min priceBump on the tip. Do this before the feeCap is calculated
thresholdTip := new(big.Int).Mul(priceBumpPercent, tx.GasTipCap()) thresholdTip := new(big.Int).Mul(priceBumpPercent, tx.GasTipCap())
...@@ -129,6 +134,7 @@ func (m *SimpleTxManager) IncreaseGasPrice(ctx context.Context, tx *types.Transa ...@@ -129,6 +134,7 @@ func (m *SimpleTxManager) IncreaseGasPrice(ctx context.Context, tx *types.Transa
if tx.GasTipCapIntCmp(gasTipCap) >= 0 { if tx.GasTipCapIntCmp(gasTipCap) >= 0 {
m.l.Debug("Reusing the previous tip", "previous", tx.GasTipCap(), "suggested", gasTipCap) m.l.Debug("Reusing the previous tip", "previous", tx.GasTipCap(), "suggested", gasTipCap)
gasTipCap = tx.GasTipCap() gasTipCap = tx.GasTipCap()
reusedTip = true
} else if thresholdTip.Cmp(gasTipCap) > 0 { } else if thresholdTip.Cmp(gasTipCap) > 0 {
m.l.Debug("Overriding the tip to enforce a price bump", "previous", tx.GasTipCap(), "suggested", gasTipCap, "new", thresholdTip) m.l.Debug("Overriding the tip to enforce a price bump", "previous", tx.GasTipCap(), "suggested", gasTipCap, "new", thresholdTip)
gasTipCap = thresholdTip gasTipCap = thresholdTip
...@@ -150,11 +156,16 @@ func (m *SimpleTxManager) IncreaseGasPrice(ctx context.Context, tx *types.Transa ...@@ -150,11 +156,16 @@ func (m *SimpleTxManager) IncreaseGasPrice(ctx context.Context, tx *types.Transa
if tx.GasFeeCapIntCmp(gasFeeCap) >= 0 { if tx.GasFeeCapIntCmp(gasFeeCap) >= 0 {
m.l.Debug("Reusing the previous fee cap", "previous", tx.GasFeeCap(), "suggested", gasFeeCap) m.l.Debug("Reusing the previous fee cap", "previous", tx.GasFeeCap(), "suggested", gasFeeCap)
gasFeeCap = tx.GasFeeCap() gasFeeCap = tx.GasFeeCap()
reusedFeeCap = true
} else if thresholdFeeCap.Cmp(gasFeeCap) > 0 { } else if thresholdFeeCap.Cmp(gasFeeCap) > 0 {
m.l.Debug("Overriding the fee cap to enforce a price bump", "previous", tx.GasFeeCap(), "suggested", gasFeeCap, "new", thresholdFeeCap) m.l.Debug("Overriding the fee cap to enforce a price bump", "previous", tx.GasFeeCap(), "suggested", gasFeeCap, "new", thresholdFeeCap)
gasFeeCap = thresholdFeeCap gasFeeCap = thresholdFeeCap
} }
if reusedTip && reusedFeeCap {
return tx, nil
}
rawTx := &types.DynamicFeeTx{ rawTx := &types.DynamicFeeTx{
ChainID: tx.ChainId(), ChainID: tx.ChainId(),
Nonce: tx.Nonce(), Nonce: tx.Nonce(),
......
...@@ -4,6 +4,7 @@ import ( ...@@ -4,6 +4,7 @@ import (
"context" "context"
"errors" "errors"
"math/big" "math/big"
"math/rand"
"sync" "sync"
"testing" "testing"
"time" "time"
...@@ -11,9 +12,12 @@ import ( ...@@ -11,9 +12,12 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/ethereum-optimism/optimism/op-node/testlog" "github.com/ethereum-optimism/optimism/op-node/testlog"
"github.com/ethereum-optimism/optimism/op-node/testutils"
opcrypto "github.com/ethereum-optimism/optimism/op-service/crypto"
"github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core"
"github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/log"
) )
...@@ -727,3 +731,42 @@ func TestIncreaseGasPriceUseLargeIncrease(t *testing.T) { ...@@ -727,3 +731,42 @@ func TestIncreaseGasPriceUseLargeIncrease(t *testing.T) {
require.True(t, newTx.GasFeeCap().Cmp(feeCap) == 0, "new tx fee cap must be equal L1") require.True(t, newTx.GasFeeCap().Cmp(feeCap) == 0, "new tx fee cap must be equal L1")
require.True(t, newTx.GasTipCap().Cmp(borkedBackend.gasTip) == 0, "new tx tip must be equal L1") require.True(t, newTx.GasTipCap().Cmp(borkedBackend.gasTip) == 0, "new tx tip must be equal L1")
} }
// TestIncreaseGasPriceReusesTransaction asserts that if the L1 basefee & tip remain the
// same, the transaction is returned with the same signature values. The means that the error
// when submitting the transaction to the network is ErrAlreadyKnown instead of ErrReplacementUnderpriced
func TestIncreaseGasPriceReusesTransaction(t *testing.T) {
t.Parallel()
borkedBackend := failingBackend{
gasTip: big.NewInt(10),
baseFee: big.NewInt(45),
}
pk := testutils.InsecureRandomKey(rand.New(rand.NewSource(123)))
signer := opcrypto.PrivateKeySignerFn(pk, big.NewInt(10))
mgr := &SimpleTxManager{
Config: Config{
ResubmissionTimeout: time.Second,
ReceiptQueryInterval: 50 * time.Millisecond,
NumConfirmations: 1,
SafeAbortNonceTooLowCount: 3,
Signer: func(ctx context.Context, from common.Address, tx *types.Transaction) (*types.Transaction, error) {
return signer(from, tx)
},
From: crypto.PubkeyToAddress(pk.PublicKey),
},
name: "TEST",
backend: &borkedBackend,
l: testlog.Logger(t, log.LvlCrit),
}
tx := types.NewTx(&types.DynamicFeeTx{
GasTipCap: big.NewInt(10),
GasFeeCap: big.NewInt(100),
})
ctx := context.Background()
newTx, err := mgr.IncreaseGasPrice(ctx, tx)
require.NoError(t, err)
require.Equal(t, tx.Hash(), newTx.Hash())
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment