Skip to content

Commit

Permalink
txmgr: Simplify API
Browse files Browse the repository at this point in the history
  • Loading branch information
trianglesphere committed Mar 31, 2023
1 parent d236185 commit 0b72447
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 90 deletions.
8 changes: 3 additions & 5 deletions op-batcher/batcher/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,17 @@ func NewBatchSubmitterFromCLIConfig(cfg CLIConfig, l log.Logger, m metrics.Metri
return nil, fmt.Errorf("querying rollup config: %w", err)
}

txManagerConfig, err := txmgr.NewConfig(cfg.TxMgrConfig, l)
txManager, err := txmgr.NewSimpleTxManager("batcher", l, m, cfg.TxMgrConfig)
if err != nil {
return nil, err
}
txManager := txmgr.NewSimpleTxManager("batcher", l, m, txManagerConfig)

batcherCfg := Config{
L1Client: l1Client,
L2Client: l2Client,
RollupNode: rollupClient,
PollInterval: cfg.PollInterval,
NetworkTimeout: txManagerConfig.NetworkTimeout,
NetworkTimeout: cfg.TxMgrConfig.NetworkTimeout,
TxManager: txManager,
Rollup: rcfg,
Channel: ChannelConfig{
Expand Down Expand Up @@ -356,9 +355,8 @@ func (l *BatchSubmitter) sendTransaction(ctx context.Context, data []byte) (*typ

// Send the transaction through the txmgr
if receipt, err := l.txMgr.Send(ctx, txmgr.TxCandidate{
To: l.Rollup.BatchInboxAddress,
To: &l.Rollup.BatchInboxAddress,
TxData: data,
From: l.txMgr.From(),
GasLimit: intrinsicGas,
}); err != nil {
l.log.Warn("unable to publish tx", "err", err, "data_size", len(data))
Expand Down
19 changes: 2 additions & 17 deletions op-e2e/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
proposermetrics "github.com/ethereum-optimism/optimism/op-proposer/metrics"
l2os "github.com/ethereum-optimism/optimism/op-proposer/proposer"
oplog "github.com/ethereum-optimism/optimism/op-service/log"
"github.com/ethereum-optimism/optimism/op-service/txmgr"

"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
Expand Down Expand Up @@ -340,14 +339,7 @@ func TestMigration(t *testing.T) {
ApproxComprRatio: 0.4,
SubSafetyMargin: 4,
PollInterval: 50 * time.Millisecond,
TxMgrConfig: txmgr.CLIConfig{
L1RPCURL: forkedL1URL,
PrivateKey: hexPriv(secrets.Batcher),
NumConfirmations: 1,
ResubmissionTimeout: 5 * time.Second,
SafeAbortNonceTooLowCount: 3,
TxNotInMempoolTimeout: 2 * time.Minute,
},
TxMgrConfig: newTxMgrConfig(forkedL1URL, secrets.Batcher),
LogConfig: oplog.CLIConfig{
Level: "info",
Format: "text",
Expand All @@ -366,14 +358,7 @@ func TestMigration(t *testing.T) {
L2OOAddress: l2OS.Address.String(),
PollInterval: 50 * time.Millisecond,
AllowNonFinalized: true,
TxMgrConfig: txmgr.CLIConfig{
L1RPCURL: forkedL1URL,
PrivateKey: hexPriv(secrets.Proposer),
NumConfirmations: 1,
ResubmissionTimeout: 3 * time.Second,
SafeAbortNonceTooLowCount: 3,
TxNotInMempoolTimeout: 2 * time.Minute,
},
TxMgrConfig: newTxMgrConfig(forkedL1URL, secrets.Proposer),
LogConfig: oplog.CLIConfig{
Level: "info",
Format: "text",
Expand Down
43 changes: 19 additions & 24 deletions op-e2e/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,19 @@ var (
testingJWTSecret = [32]byte{123}
)

func newTxMgrConfig(l1Addr string, privKey *ecdsa.PrivateKey) txmgr.CLIConfig {
return txmgr.CLIConfig{
L1RPCURL: l1Addr,
PrivateKey: hexPriv(privKey),
NumConfirmations: 1,
SafeAbortNonceTooLowCount: 3,
ResubmissionTimeout: 3 * time.Second,
ReceiptQueryInterval: 50 * time.Millisecond,
NetworkTimeout: 2 * time.Second,
TxNotInMempoolTimeout: 2 * time.Minute,
}
}

func DefaultSystemConfig(t *testing.T) SystemConfig {
secrets, err := e2eutils.DefaultMnemonicConfig.Secrets()
require.NoError(t, err)
Expand Down Expand Up @@ -568,20 +581,11 @@ func (cfg SystemConfig) Start(_opts ...SystemConfigOption) (*System, error) {

// L2Output Submitter
sys.L2OutputSubmitter, err = l2os.NewL2OutputSubmitterFromCLIConfig(l2os.CLIConfig{
L1EthRpc: sys.Nodes["l1"].WSEndpoint(),
RollupRpc: sys.RollupNodes["sequencer"].HTTPEndpoint(),
L2OOAddress: predeploys.DevL2OutputOracleAddr.String(),
PollInterval: 50 * time.Millisecond,
TxMgrConfig: txmgr.CLIConfig{
L1RPCURL: sys.Nodes["l1"].WSEndpoint(),
PrivateKey: hexPriv(cfg.Secrets.Proposer),
NumConfirmations: 1,
SafeAbortNonceTooLowCount: 3,
ResubmissionTimeout: 3 * time.Second,
ReceiptQueryInterval: 50 * time.Millisecond,
NetworkTimeout: 2 * time.Second,
TxNotInMempoolTimeout: 2 * time.Minute,
},
L1EthRpc: sys.Nodes["l1"].WSEndpoint(),
RollupRpc: sys.RollupNodes["sequencer"].HTTPEndpoint(),
L2OOAddress: predeploys.DevL2OutputOracleAddr.String(),
PollInterval: 50 * time.Millisecond,
TxMgrConfig: newTxMgrConfig(sys.Nodes["l1"].WSEndpoint(), cfg.Secrets.Proposer),
AllowNonFinalized: cfg.NonFinalizedProposals,
LogConfig: oplog.CLIConfig{
Level: "info",
Expand All @@ -608,16 +612,7 @@ func (cfg SystemConfig) Start(_opts ...SystemConfigOption) (*System, error) {
ApproxComprRatio: 0.4,
SubSafetyMargin: 4,
PollInterval: 50 * time.Millisecond,
TxMgrConfig: txmgr.CLIConfig{
L1RPCURL: sys.Nodes["l1"].WSEndpoint(),
PrivateKey: hexPriv(cfg.Secrets.Batcher),
NumConfirmations: 1,
SafeAbortNonceTooLowCount: 3,
ResubmissionTimeout: 3 * time.Second,
ReceiptQueryInterval: 50 * time.Millisecond,
NetworkTimeout: 2 * time.Second,
TxNotInMempoolTimeout: 2 * time.Minute,
},
TxMgrConfig: newTxMgrConfig(sys.Nodes["l1"].WSEndpoint(), cfg.Secrets.Batcher),
LogConfig: oplog.CLIConfig{
Level: "info",
Format: "text",
Expand Down
8 changes: 3 additions & 5 deletions op-proposer/proposer/l2_output_submitter.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,10 @@ func NewL2OutputSubmitterConfigFromCLIConfig(cfg CLIConfig, l log.Logger, m metr
return nil, err
}

txManagerConfig, err := txmgr.NewConfig(cfg.TxMgrConfig, l)
txManager, err := txmgr.NewSimpleTxManager("proposer", l, m, cfg.TxMgrConfig)
if err != nil {
return nil, err
}
txManager := txmgr.NewSimpleTxManager("proposer", l, m, txManagerConfig)

// Connect to L1 and L2 providers. Perform these last since they are the most expensive.
ctx := context.Background()
Expand All @@ -173,7 +172,7 @@ func NewL2OutputSubmitterConfigFromCLIConfig(cfg CLIConfig, l log.Logger, m metr
return &Config{
L2OutputOracleAddr: l2ooAddress,
PollInterval: cfg.PollInterval,
NetworkTimeout: txManagerConfig.NetworkTimeout,
NetworkTimeout: cfg.TxMgrConfig.NetworkTimeout,
L1Client: l1Client,
RollupClient: rollupClient,
AllowNonFinalized: cfg.AllowNonFinalized,
Expand Down Expand Up @@ -329,9 +328,8 @@ func (l *L2OutputSubmitter) sendTransaction(ctx context.Context, output *eth.Out
}
receipt, err := l.txMgr.Send(ctx, txmgr.TxCandidate{
TxData: data,
To: l.l2ooContractAddr,
To: &l.l2ooContractAddr,
GasLimit: 0,
From: l.txMgr.From(),
})
if err != nil {
return err
Expand Down
13 changes: 7 additions & 6 deletions op-service/txmgr/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package txmgr
import (
"context"
"errors"
"fmt"
"math/big"
"time"

Expand Down Expand Up @@ -81,7 +82,7 @@ func CLIFlags(envPrefix string) []cli.Flag {
cli.DurationFlag{
Name: ResubmissionTimeoutFlagName,
Usage: "Duration we will wait before resubmitting a transaction to L1",
Value: 30 * time.Second,
Value: 48 * time.Second,
EnvVar: opservice.PrefixEnvVar(envPrefix, "RESUBMISSION_TIMEOUT"),
},
cli.DurationFlag{
Expand All @@ -105,7 +106,7 @@ func CLIFlags(envPrefix string) []cli.Flag {
cli.DurationFlag{
Name: ReceiptQueryIntervalFlagName,
Usage: "Frequency to poll for receipts",
Value: 30 * time.Second,
Value: 12 * time.Second,
EnvVar: opservice.PrefixEnvVar(envPrefix, "TXMGR_RECEIPT_QUERY_INTERVAL"),
},
}, client.CLIFlags(envPrefix)...)
Expand Down Expand Up @@ -177,21 +178,21 @@ func ReadCLIConfig(ctx *cli.Context) CLIConfig {

func NewConfig(cfg CLIConfig, l log.Logger) (Config, error) {
if err := cfg.Check(); err != nil {
return Config{}, err
return Config{}, fmt.Errorf("invalid config: %w", err)
}

ctx, cancel := context.WithTimeout(context.Background(), cfg.NetworkTimeout)
defer cancel()
l1, err := ethclient.DialContext(ctx, cfg.L1RPCURL)
if err != nil {
return Config{}, err
return Config{}, fmt.Errorf("could not dial eth client: %w", err)
}

ctx, cancel = context.WithTimeout(context.Background(), cfg.NetworkTimeout)
defer cancel()
chainID, err := l1.ChainID(ctx)
if err != nil {
return Config{}, err
return Config{}, fmt.Errorf("could not dial fetch L1 chain ID: %w", err)
}

// Allow backwards compatible ways of specifying the HD path
Expand All @@ -204,7 +205,7 @@ func NewConfig(cfg CLIConfig, l log.Logger) (Config, error) {

signerFactory, from, err := opcrypto.SignerFactoryFromConfig(l, cfg.PrivateKey, cfg.Mnemonic, hdPath, cfg.SignerCLIConfig)
if err != nil {
return Config{}, err
return Config{}, fmt.Errorf("could not init signer: %w", err)
}

return Config{
Expand Down
36 changes: 16 additions & 20 deletions op-service/txmgr/txmgr.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,22 +87,20 @@ type SimpleTxManager struct {
}

// NewSimpleTxManager initializes a new SimpleTxManager with the passed Config.
func NewSimpleTxManager(name string, l log.Logger, m metrics.TxMetricer, cfg Config) *SimpleTxManager {
if cfg.NumConfirmations == 0 {
panic("txmgr: NumConfirmations cannot be zero")
}
if cfg.NetworkTimeout == 0 {
cfg.NetworkTimeout = 2 * time.Second
func NewSimpleTxManager(name string, l log.Logger, m metrics.TxMetricer, cfg CLIConfig) (*SimpleTxManager, error) {
conf, err := NewConfig(cfg, l)
if err != nil {
return nil, err
}

return &SimpleTxManager{
chainID: cfg.ChainID,
chainID: conf.ChainID,
name: name,
cfg: cfg,
backend: cfg.Backend,
cfg: conf,
backend: conf.Backend,
l: l.New("service", name),
metr: m,
}
}, nil
}

func (m *SimpleTxManager) From() common.Address {
Expand All @@ -114,12 +112,10 @@ func (m *SimpleTxManager) From() common.Address {
type TxCandidate struct {
// TxData is the transaction data to be used in the constructed tx.
TxData []byte
// To is the recipient of the constructed tx.
To common.Address
// To is the recipient of the constructed tx. Nil means contract creation.
To *common.Address
// GasLimit is the gas limit to be used in the constructed tx.
GasLimit uint64
// From is the sender (or `from`) of the constructed tx.
From common.Address
}

// Send is used to publish a transaction with incrementally higher gas prices
Expand Down Expand Up @@ -159,30 +155,30 @@ func (m *SimpleTxManager) craftTx(ctx context.Context, candidate TxCandidate) (*
// Fetch the sender's nonce from the latest known block (nil `blockNumber`)
childCtx, cancel := context.WithTimeout(ctx, m.cfg.NetworkTimeout)
defer cancel()
nonce, err := m.backend.NonceAt(childCtx, candidate.From, nil)
nonce, err := m.backend.NonceAt(childCtx, m.cfg.From, nil)
if err != nil {
return nil, fmt.Errorf("failed to get nonce: %w", err)
}

rawTx := &types.DynamicFeeTx{
ChainID: m.chainID,
Nonce: nonce,
To: &candidate.To,
To: candidate.To,
GasTipCap: gasTipCap,
GasFeeCap: gasFeeCap,
Data: candidate.TxData,
}

m.l.Info("creating tx", "to", rawTx.To, "from", candidate.From)
m.l.Info("creating tx", "to", rawTx.To, "from", m.cfg.From)

// If the gas limit is set, we can use that as the gas
if candidate.GasLimit != 0 {
rawTx.Gas = candidate.GasLimit
} else {
// Calculate the intrinsic gas for the transaction
gas, err := m.backend.EstimateGas(ctx, ethereum.CallMsg{
From: candidate.From,
To: &candidate.To,
From: m.cfg.From,
To: candidate.To,
GasFeeCap: gasFeeCap,
GasTipCap: gasTipCap,
Data: rawTx.Data,
Expand All @@ -195,7 +191,7 @@ func (m *SimpleTxManager) craftTx(ctx context.Context, candidate TxCandidate) (*

ctx, cancel = context.WithTimeout(ctx, m.cfg.NetworkTimeout)
defer cancel()
return m.cfg.Signer(ctx, candidate.From, types.NewTx(rawTx))
return m.cfg.Signer(ctx, m.cfg.From, types.NewTx(rawTx))
}

// send submits the same transaction several times with increasing gas prices as necessary.
Expand Down
26 changes: 13 additions & 13 deletions op-service/txmgr/txmgr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,14 @@ func newTestHarnessWithConfig(t *testing.T, cfg Config) *testHarness {
g := newGasPricer(3)
backend := newMockBackend(g)
cfg.Backend = backend
mgr := NewSimpleTxManager("TEST", testlog.Logger(t, log.LvlCrit), &metrics.NoopTxMetrics{}, cfg)
mgr := &SimpleTxManager{
chainID: cfg.ChainID,
name: "TEST",
cfg: cfg,
backend: cfg.Backend,
l: testlog.Logger(t, log.LvlCrit),
metr: &metrics.NoopTxMetrics{},
}

return &testHarness{
cfg: cfg,
Expand All @@ -60,11 +67,9 @@ func newTestHarness(t *testing.T) *testHarness {
// createTxCandidate creates a mock [TxCandidate].
func (h testHarness) createTxCandidate() TxCandidate {
inbox := common.HexToAddress("0x42000000000000000000000000000000000000ff")
sender := common.HexToAddress("0xdeadbeef")
return TxCandidate{
To: inbox,
To: &inbox,
TxData: []byte{0x00, 0x01, 0x02},
From: sender,
GasLimit: uint64(1337),
}
}
Expand Down Expand Up @@ -593,18 +598,13 @@ func TestWaitMinedMultipleConfs(t *testing.T) {
require.Equal(t, txHash, receipt.TxHash)
}

// TestManagerPanicOnZeroConfs ensures that the NewSimpleTxManager will panic
// TestManagerErrsOnZeroConfs ensures that the NewSimpleTxManager will error
// when attempting to configure with NumConfirmations set to zero.
func TestManagerPanicOnZeroConfs(t *testing.T) {
func TestManagerErrsOnZeroConfs(t *testing.T) {
t.Parallel()

defer func() {
if r := recover(); r == nil {
t.Fatal("NewSimpleTxManager should panic when using zero conf")
}
}()

_ = newTestHarnessWithConfig(t, configWithNumConfs(0))
_, err := NewSimpleTxManager("TEST", testlog.Logger(t, log.LvlCrit), &metrics.NoopTxMetrics{}, CLIConfig{})
require.Error(t, err)
}

// failingBackend implements ReceiptSource, returning a failure on the
Expand Down

0 comments on commit 0b72447

Please sign in to comment.