Commit 93a26819 authored by Conner Fromknecht's avatar Conner Fromknecht

fix: properly set gas limit on BSS clearing txs

Fixes a bug where clearing txs are rejected on startup if the gas limit
is not set.
parent c4f7f3d4
---
'@eth-optimism/batch-submitter-service': patch
---
Fixes a bug where clearing txs are rejected on startup due to missing gas limit
...@@ -8,6 +8,7 @@ import ( ...@@ -8,6 +8,7 @@ import (
"strings" "strings"
"github.com/ethereum-optimism/optimism/go/bss-core/txmgr" "github.com/ethereum-optimism/optimism/go/bss-core/txmgr"
"github.com/ethereum/go-ethereum"
"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"
...@@ -159,7 +160,20 @@ func SignClearingTx( ...@@ -159,7 +160,20 @@ func SignClearingTx(
} }
gasFeeCap := txmgr.CalcGasFeeCap(head.BaseFee, gasTipCap) gasFeeCap := txmgr.CalcGasFeeCap(head.BaseFee, gasTipCap)
tx := CraftClearingTx(walletAddr, nonce, gasFeeCap, gasTipCap)
gasLimit, err := l1Client.EstimateGas(ctx, ethereum.CallMsg{
From: walletAddr,
To: &walletAddr,
GasFeeCap: gasFeeCap,
GasTipCap: gasTipCap,
Value: nil,
Data: nil,
})
if err != nil {
return nil, err
}
tx := CraftClearingTx(walletAddr, nonce, gasFeeCap, gasTipCap, gasLimit)
return types.SignTx( return types.SignTx(
tx, types.LatestSignerForChainID(chainID), privKey, tx, types.LatestSignerForChainID(chainID), privKey,
...@@ -173,11 +187,13 @@ func CraftClearingTx( ...@@ -173,11 +187,13 @@ func CraftClearingTx(
nonce uint64, nonce uint64,
gasFeeCap *big.Int, gasFeeCap *big.Int,
gasTipCap *big.Int, gasTipCap *big.Int,
gasLimit uint64,
) *types.Transaction { ) *types.Transaction {
return types.NewTx(&types.DynamicFeeTx{ return types.NewTx(&types.DynamicFeeTx{
To: &walletAddr, To: &walletAddr,
Nonce: nonce, Nonce: nonce,
Gas: gasLimit,
GasFeeCap: gasFeeCap, GasFeeCap: gasFeeCap,
GasTipCap: gasTipCap, GasTipCap: gasTipCap,
Value: nil, Value: nil,
......
...@@ -8,9 +8,10 @@ import ( ...@@ -8,9 +8,10 @@ import (
"testing" "testing"
"time" "time"
"github.com/ethereum-optimism/optimism/go/batch-submitter/drivers" "github.com/ethereum-optimism/optimism/go/bss-core/drivers"
"github.com/ethereum-optimism/optimism/go/bss-core/mock" "github.com/ethereum-optimism/optimism/go/bss-core/mock"
"github.com/ethereum-optimism/optimism/go/bss-core/txmgr" "github.com/ethereum-optimism/optimism/go/bss-core/txmgr"
"github.com/ethereum/go-ethereum"
"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"
...@@ -36,16 +37,18 @@ var ( ...@@ -36,16 +37,18 @@ var (
testGasTipCap = big.NewInt(4) testGasTipCap = big.NewInt(4)
testBlockNumber = uint64(5) testBlockNumber = uint64(5)
testBaseFee = big.NewInt(6) testBaseFee = big.NewInt(6)
testGasLimit = uint64(7)
) )
// TestCraftClearingTx asserts that CraftClearingTx produces the expected // TestCraftClearingTx asserts that CraftClearingTx produces the expected
// unsigned clearing transaction. // unsigned clearing transaction.
func TestCraftClearingTx(t *testing.T) { func TestCraftClearingTx(t *testing.T) {
tx := drivers.CraftClearingTx( tx := drivers.CraftClearingTx(
testWalletAddr, testNonce, testGasFeeCap, testGasTipCap, testWalletAddr, testNonce, testGasFeeCap, testGasTipCap, testGasLimit,
) )
require.Equal(t, &testWalletAddr, tx.To()) require.Equal(t, &testWalletAddr, tx.To())
require.Equal(t, testNonce, tx.Nonce()) require.Equal(t, testNonce, tx.Nonce())
require.Equal(t, testGasLimit, tx.Gas())
require.Equal(t, testGasFeeCap, tx.GasFeeCap()) require.Equal(t, testGasFeeCap, tx.GasFeeCap())
require.Equal(t, testGasTipCap, tx.GasTipCap()) require.Equal(t, testGasTipCap, tx.GasTipCap())
require.Equal(t, new(big.Int), tx.Value()) require.Equal(t, new(big.Int), tx.Value())
...@@ -64,6 +67,9 @@ func TestSignClearingTxEstimateGasSuccess(t *testing.T) { ...@@ -64,6 +67,9 @@ func TestSignClearingTxEstimateGasSuccess(t *testing.T) {
SuggestGasTipCap: func(_ context.Context) (*big.Int, error) { SuggestGasTipCap: func(_ context.Context) (*big.Int, error) {
return testGasTipCap, nil return testGasTipCap, nil
}, },
EstimateGas: func(_ context.Context, _ ethereum.CallMsg) (uint64, error) {
return testGasLimit, nil
},
}) })
expGasFeeCap := new(big.Int).Add( expGasFeeCap := new(big.Int).Add(
...@@ -131,6 +137,33 @@ func TestSignClearingTxHeaderByNumberFail(t *testing.T) { ...@@ -131,6 +137,33 @@ func TestSignClearingTxHeaderByNumberFail(t *testing.T) {
require.Nil(t, tx) require.Nil(t, tx)
} }
// TestSignClearingTxEstimateGasFail asserts that signing a clearing
// transaction will fail if the underlying call to EstimateGas fails.
func TestSignClearingTxEstimateGasFail(t *testing.T) {
errEstimateGas := errors.New("estimate gas")
l1Client := mock.NewL1Client(mock.L1ClientConfig{
EstimateGas: func(_ context.Context, _ ethereum.CallMsg) (uint64, error) {
return 0, errEstimateGas
},
HeaderByNumber: func(_ context.Context, _ *big.Int) (*types.Header, error) {
return &types.Header{
BaseFee: testBaseFee,
}, nil
},
SuggestGasTipCap: func(_ context.Context) (*big.Int, error) {
return testGasTipCap, nil
},
})
tx, err := drivers.SignClearingTx(
"TEST", context.Background(), testWalletAddr, testNonce, l1Client,
testPrivKey, testChainID,
)
require.Equal(t, errEstimateGas, err)
require.Nil(t, tx)
}
type clearPendingTxHarness struct { type clearPendingTxHarness struct {
l1Client *mock.L1Client l1Client *mock.L1Client
txMgr txmgr.TxManager txMgr txmgr.TxManager
...@@ -163,6 +196,11 @@ func newClearPendingTxHarnessWithNumConfs( ...@@ -163,6 +196,11 @@ func newClearPendingTxHarnessWithNumConfs(
return testGasTipCap, nil return testGasTipCap, nil
} }
} }
if l1ClientConfig.EstimateGas == nil {
l1ClientConfig.EstimateGas = func(_ context.Context, _ ethereum.CallMsg) (uint64, error) {
return testGasLimit, nil
}
}
l1Client := mock.NewL1Client(l1ClientConfig) l1Client := mock.NewL1Client(l1ClientConfig)
txMgr := txmgr.NewSimpleTxManager("test", txmgr.Config{ txMgr := txmgr.NewSimpleTxManager("test", txmgr.Config{
......
...@@ -4,6 +4,7 @@ import ( ...@@ -4,6 +4,7 @@ import (
"context" "context"
"math/big" "math/big"
"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/types"
) )
...@@ -11,6 +12,13 @@ import ( ...@@ -11,6 +12,13 @@ import (
// L1Client is an abstraction over an L1 Ethereum client functionality required // L1Client is an abstraction over an L1 Ethereum client functionality required
// by the batch submitter. // by the batch submitter.
type L1Client interface { type L1Client interface {
// EstimateGas tries to estimate the gas needed to execute a specific
// transaction based on the current pending state of the backend blockchain.
// There is no guarantee that this is the true gas limit requirement as
// other transactions may be added or removed by miners, but it should
// provide a basis for setting a reasonable default.
EstimateGas(context.Context, ethereum.CallMsg) (uint64, error)
// HeaderByNumber returns a block header from the current canonical chain. // HeaderByNumber returns a block header from the current canonical chain.
// If number is nil, the latest known header is returned. // If number is nil, the latest known header is returned.
HeaderByNumber(context.Context, *big.Int) (*types.Header, error) HeaderByNumber(context.Context, *big.Int) (*types.Header, error)
......
...@@ -5,6 +5,7 @@ import ( ...@@ -5,6 +5,7 @@ import (
"math/big" "math/big"
"sync" "sync"
"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/types"
) )
...@@ -15,6 +16,13 @@ type L1ClientConfig struct { ...@@ -15,6 +16,13 @@ type L1ClientConfig struct {
// BlockNumber returns the most recent block number. // BlockNumber returns the most recent block number.
BlockNumber func(context.Context) (uint64, error) BlockNumber func(context.Context) (uint64, error)
// EstimateGas tries to estimate the gas needed to execute a specific
// transaction based on the current pending state of the backend blockchain.
// There is no guarantee that this is the true gas limit requirement as
// other transactions may be added or removed by miners, but it should
// provide a basis for setting a reasonable default.
EstimateGas func(context.Context, ethereum.CallMsg) (uint64, error)
// HeaderByNumber returns a block header from the current canonical chain. // HeaderByNumber returns a block header from the current canonical chain.
// If number is nil, the latest known header is returned. // If number is nil, the latest known header is returned.
HeaderByNumber func(context.Context, *big.Int) (*types.Header, error) HeaderByNumber func(context.Context, *big.Int) (*types.Header, error)
...@@ -61,6 +69,18 @@ func (c *L1Client) BlockNumber(ctx context.Context) (uint64, error) { ...@@ -61,6 +69,18 @@ func (c *L1Client) BlockNumber(ctx context.Context) (uint64, error) {
return c.cfg.BlockNumber(ctx) return c.cfg.BlockNumber(ctx)
} }
// EstimateGas tries to estimate the gas needed to execute a specific
// transaction based on the current pending state of the backend blockchain.
// There is no guarantee that this is the true gas limit requirement as other
// transactions may be added or removed by miners, but it should provide a basis
// for setting a reasonable default.
func (c *L1Client) EstimateGas(ctx context.Context, msg ethereum.CallMsg) (uint64, error) {
c.mu.RLock()
defer c.mu.RUnlock()
return c.cfg.EstimateGas(ctx, msg)
}
// HeaderByNumber returns a block header from the current canonical chain. If // HeaderByNumber returns a block header from the current canonical chain. If
// number is nil, the latest known header is returned. // number is nil, the latest known header is returned.
func (c *L1Client) HeaderByNumber(ctx context.Context, blockNumber *big.Int) (*types.Header, error) { func (c *L1Client) HeaderByNumber(ctx context.Context, blockNumber *big.Int) (*types.Header, error) {
...@@ -113,6 +133,16 @@ func (c *L1Client) SetBlockNumberFunc( ...@@ -113,6 +133,16 @@ func (c *L1Client) SetBlockNumberFunc(
c.cfg.BlockNumber = f c.cfg.BlockNumber = f
} }
// SetEstimateGasFunc overwrites the mock EstimateGas method.
func (c *L1Client) SetEstimateGasFunc(
f func(context.Context, ethereum.CallMsg) (uint64, error)) {
c.mu.Lock()
defer c.mu.Unlock()
c.cfg.EstimateGas = f
}
// SetHeaderByNumberFunc overwrites the mock HeaderByNumber method. // SetHeaderByNumberFunc overwrites the mock HeaderByNumber method.
func (c *L1Client) SetHeaderByNumberFunc( func (c *L1Client) SetHeaderByNumberFunc(
f func(ctx context.Context, blockNumber *big.Int) (*types.Header, error)) { f func(ctx context.Context, blockNumber *big.Int) (*types.Header, error)) {
......
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