Merge pull request #5306 from ethereum-optimism/jg/cleanup_txmgr_apis

txmgr: Simplify API

op-batcher/batcher/driver.go

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

op-e2e/migration_test.go

@@ -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"
@@ -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",
@@ -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",

op-e2e/setup.go

@@ -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)
@@ -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",
@@ -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",

op-proposer/proposer/l2_output_submitter.go

@@ -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()
@@ -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,
@@ -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

op-service/txmgr/cli.go

@@ -3,6 +3,7 @@ package txmgr
 import (
 	"context"
 	"errors"
+	"fmt"
 	"math/big"
 	"time"
 
@@ -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{
@@ -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)...)
@@ -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
@@ -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{

op-service/txmgr/txmgr.go

@@ -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 {
@@ -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
@@ -159,7 +155,7 @@ 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)
 	}
@@ -167,13 +163,13 @@ func (m *SimpleTxManager) craftTx(ctx context.Context, candidate TxCandidate) (*
 	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 {
@@ -181,8 +177,8 @@ func (m *SimpleTxManager) craftTx(ctx context.Context, candidate TxCandidate) (*
 	} 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,
@@ -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.

op-service/txmgr/txmgr_test.go

@@ -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,
@@ -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),
 	}
 }
@@ -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