Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

txmgr: Simplify API #5306

Merged
merged 2 commits into from
Apr 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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