From e76c3c43456cd46a2e3088db5c36a0c6bd84cefd Mon Sep 17 00:00:00 2001 From: blockchaindevsh Date: Sun, 15 Sep 2024 13:59:23 +0800 Subject: [PATCH 1/3] remove UseInboxContract --- op-batcher/batcher/driver.go | 13 +++++-------- op-chain-ops/genesis/config.go | 3 --- op-node/cmd/genesis/cmd.go | 1 - op-node/rollup/derive/blob_data_source.go | 15 +++++---------- op-node/rollup/derive/calldata_source.go | 7 +++---- op-node/rollup/derive/data_source.go | 2 -- op-node/rollup/types.go | 3 --- 7 files changed, 13 insertions(+), 31 deletions(-) diff --git a/op-batcher/batcher/driver.go b/op-batcher/batcher/driver.go index 42d14a2291a9..7578233ebe9b 100644 --- a/op-batcher/batcher/driver.go +++ b/op-batcher/batcher/driver.go @@ -516,21 +516,18 @@ func (l *BatchSubmitter) sendTransaction(ctx context.Context, txdata txData, que if *candidate.To != l.RollupConfig.BatchInboxAddress { return fmt.Errorf("candidate.To is not inbox") } - if l.RollupConfig.UseInboxContract && l.inboxIsEOA == nil { + if l.inboxIsEOA == nil { var code []byte code, err = l.L1Client.CodeAt(ctx, *candidate.To, nil) if err != nil { return fmt.Errorf("CodeAt failed:%w", err) } isEOA := len(code) == 0 - if !isEOA { - return fmt.Errorf("UseInboxContract is enabled but BatchInboxAddress is an EOA") - } l.inboxIsEOA = &isEOA } - // Don't set GasLimit when UseInboxContract is enabled so that later on `EstimateGas` will be called - if !l.RollupConfig.UseInboxContract { + // Don't set GasLimit when inbox is contract so that later on `EstimateGas` will be called + if !*l.inboxIsEOA { intrinsicGas, err := core.IntrinsicGas(candidate.TxData, nil, false, true, true, false) if err != nil { // we log instead of return an error here because txmgr can do its own gas estimation @@ -573,8 +570,8 @@ func (l *BatchSubmitter) handleReceipt(r txmgr.TxReceipt[txID]) { if r.Err != nil { l.recordFailedTx(r.ID, r.Err) } else { - // check tx status if UseInboxContract - if l.RollupConfig.UseInboxContract && r.Receipt.Status == types.ReceiptStatusFailed { + // check tx status + if r.Receipt.Status == types.ReceiptStatusFailed { l.recordFailedTx(r.ID, ErrInboxTransactionFailed) return } diff --git a/op-chain-ops/genesis/config.go b/op-chain-ops/genesis/config.go index bfc7bb9a08a7..b06fd8118343 100644 --- a/op-chain-ops/genesis/config.go +++ b/op-chain-ops/genesis/config.go @@ -298,9 +298,6 @@ type DeployConfig struct { // IsSoulBackedByNative is a flag that indicates if the SoulGasToken is backed by native. // Only effective when UseSoulGasToken is true. IsSoulBackedByNative bool `json:"isSoulBackedByNative"` - - // UseInboxContract is a flag that indicates if the inbox is a contract - UseInboxContract bool `json:"use_inbox_contract"` } // Copy will deeply copy the DeployConfig. This does a JSON roundtrip to copy diff --git a/op-node/cmd/genesis/cmd.go b/op-node/cmd/genesis/cmd.go index 0ffc8c689f4a..6d3d687444a2 100644 --- a/op-node/cmd/genesis/cmd.go +++ b/op-node/cmd/genesis/cmd.go @@ -229,7 +229,6 @@ var Subcommands = cli.Commands{ if err != nil { return err } - rollupConfig.UseInboxContract = config.UseInboxContract if err := rollupConfig.Check(); err != nil { return fmt.Errorf("generated rollup config does not pass validation: %w", err) } diff --git a/op-node/rollup/derive/blob_data_source.go b/op-node/rollup/derive/blob_data_source.go index 42f43cd1bac6..1dee45da02df 100644 --- a/op-node/rollup/derive/blob_data_source.go +++ b/op-node/rollup/derive/blob_data_source.go @@ -73,12 +73,8 @@ func (ds *BlobDataSource) Next(ctx context.Context) (eth.Data, error) { return data, nil } -// getTxSucceedIfUseInboxContract returns nil if !useInboxContract; -// otherwise it returns a non-nil map which contains all successful tx hashes -func getTxSucceedIfUseInboxContract(ctx context.Context, useInboxContract bool, fetcher L1Fetcher, hash common.Hash) (txSucceeded map[common.Hash]bool, err error) { - if !useInboxContract { - return - } +// getTxSucceed returns a non-nil map which contains all successful tx hashes +func getTxSucceed(ctx context.Context, fetcher L1Fetcher, hash common.Hash) (txSucceeded map[common.Hash]bool, err error) { _, receipts, err := fetcher.FetchReceipts(ctx, hash) if err != nil { return nil, NewTemporaryError(fmt.Errorf("failed to fetch L1 block info and receipts: %w", err)) @@ -105,7 +101,7 @@ func (ds *BlobDataSource) open(ctx context.Context) ([]blobOrCalldata, error) { } return nil, NewTemporaryError(fmt.Errorf("failed to open blob data source: %w", err)) } - txSucceeded, err := getTxSucceedIfUseInboxContract(ctx, ds.dsCfg.useInboxContract, ds.fetcher, ds.ref.Hash) + txSucceeded, err := getTxSucceed(ctx, ds.fetcher, ds.ref.Hash) if err != nil { return nil, err } @@ -144,9 +140,8 @@ func dataAndHashesFromTxs(txs types.Transactions, config *DataSourceConfig, batc var hashes []eth.IndexedBlobHash blobIndex := 0 // index of each blob in the block's blob sidecar for _, tx := range txs { - // skip any non-batcher transactions or failed transactions if useInboxContract - // note: txSucceeded will only be nil when !useInboxContract - if !(isValidBatchTx(tx, config.l1Signer, config.batchInboxAddress, batcherAddr) && (txSucceeded == nil || txSucceeded[tx.Hash()])) { + // skip any non-batcher transactions or failed transactions + if !(isValidBatchTx(tx, config.l1Signer, config.batchInboxAddress, batcherAddr) && txSucceeded[tx.Hash()]) { blobIndex += len(tx.BlobHashes()) continue } diff --git a/op-node/rollup/derive/calldata_source.go b/op-node/rollup/derive/calldata_source.go index dac1a95a8706..48831b1dc133 100644 --- a/op-node/rollup/derive/calldata_source.go +++ b/op-node/rollup/derive/calldata_source.go @@ -44,7 +44,7 @@ func NewCalldataSource(ctx context.Context, log log.Logger, dsCfg DataSourceConf batcherAddr: batcherAddr, } } - txSucceed, err := getTxSucceedIfUseInboxContract(ctx, dsCfg.useInboxContract, fetcher, ref.Hash) + txSucceed, err := getTxSucceed(ctx, fetcher, ref.Hash) if err != nil { return &CalldataSource{ open: false, @@ -67,7 +67,7 @@ func NewCalldataSource(ctx context.Context, log log.Logger, dsCfg DataSourceConf func (ds *CalldataSource) Next(ctx context.Context) (eth.Data, error) { if !ds.open { if _, txs, err := ds.fetcher.InfoAndTxsByHash(ctx, ds.ref.Hash); err == nil { - txSucceeded, err := getTxSucceedIfUseInboxContract(ctx, ds.dsCfg.useInboxContract, ds.fetcher, ds.ref.Hash) + txSucceeded, err := getTxSucceed(ctx, ds.fetcher, ds.ref.Hash) if err != nil { return nil, err } @@ -94,8 +94,7 @@ func (ds *CalldataSource) Next(ctx context.Context) (eth.Data, error) { func DataFromEVMTransactions(dsCfg DataSourceConfig, batcherAddr common.Address, txs types.Transactions, log log.Logger, txSucceeded map[common.Hash]bool) []eth.Data { out := []eth.Data{} for _, tx := range txs { - // note: txSucceeded will only be nil when !useInboxContract - if isValidBatchTx(tx, dsCfg.l1Signer, dsCfg.batchInboxAddress, batcherAddr) && (txSucceeded == nil || txSucceeded[tx.Hash()]) { + if isValidBatchTx(tx, dsCfg.l1Signer, dsCfg.batchInboxAddress, batcherAddr) && txSucceeded[tx.Hash()] { out = append(out, tx.Data()) } } diff --git a/op-node/rollup/derive/data_source.go b/op-node/rollup/derive/data_source.go index 1645cad9029c..6cb43a94165f 100644 --- a/op-node/rollup/derive/data_source.go +++ b/op-node/rollup/derive/data_source.go @@ -56,7 +56,6 @@ func NewDataSourceFactory(log log.Logger, cfg *rollup.Config, fetcher L1Fetcher, l1Signer: cfg.L1Signer(), batchInboxAddress: cfg.BatchInboxAddress, plasmaEnabled: cfg.PlasmaEnabled(), - useInboxContract: cfg.UseInboxContract, } return &DataSourceFactory{ log: log, @@ -93,7 +92,6 @@ type DataSourceConfig struct { l1Signer types.Signer batchInboxAddress common.Address plasmaEnabled bool - useInboxContract bool } // isValidBatchTx returns true if: diff --git a/op-node/rollup/types.go b/op-node/rollup/types.go index 5d020c94ae38..526c970035f7 100644 --- a/op-node/rollup/types.go +++ b/op-node/rollup/types.go @@ -143,9 +143,6 @@ type Config struct { LegacyUsePlasma bool `json:"use_plasma,omitempty"` L2BlobConfig *L2BlobConfig `json:"l2_blob_config,omitempty"` - - // UseInboxContract is a flag that indicates if the inbox is a contract - UseInboxContract bool `json:"use_inbox_contract"` } type L2BlobConfig struct { From a4af765062bcbaa9138041662d04f424df2ec264 Mon Sep 17 00:00:00 2001 From: blockchaindevsh Date: Wed, 18 Sep 2024 07:11:15 +0800 Subject: [PATCH 2/3] optimize getTxSucceed to return a smaller map --- op-node/rollup/derive/blob_data_source.go | 8 ++++---- op-node/rollup/derive/calldata_source.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/op-node/rollup/derive/blob_data_source.go b/op-node/rollup/derive/blob_data_source.go index 1dee45da02df..05bd5d326388 100644 --- a/op-node/rollup/derive/blob_data_source.go +++ b/op-node/rollup/derive/blob_data_source.go @@ -73,8 +73,8 @@ func (ds *BlobDataSource) Next(ctx context.Context) (eth.Data, error) { return data, nil } -// getTxSucceed returns a non-nil map which contains all successful tx hashes -func getTxSucceed(ctx context.Context, fetcher L1Fetcher, hash common.Hash) (txSucceeded map[common.Hash]bool, err error) { +// getTxSucceed returns a non-nil map which contains all successful tx hashes to batch inbox +func getTxSucceed(ctx context.Context, fetcher L1Fetcher, hash common.Hash, batchInboxAddress common.Address) (txSucceeded map[common.Hash]bool, err error) { _, receipts, err := fetcher.FetchReceipts(ctx, hash) if err != nil { return nil, NewTemporaryError(fmt.Errorf("failed to fetch L1 block info and receipts: %w", err)) @@ -82,7 +82,7 @@ func getTxSucceed(ctx context.Context, fetcher L1Fetcher, hash common.Hash) (txS txSucceeded = make(map[common.Hash]bool) for _, receipt := range receipts { - if receipt.Status == types.ReceiptStatusSuccessful { + if receipt.Status == types.ReceiptStatusSuccessful && receipt.ContractAddress == batchInboxAddress { txSucceeded[receipt.TxHash] = true } } @@ -101,7 +101,7 @@ func (ds *BlobDataSource) open(ctx context.Context) ([]blobOrCalldata, error) { } return nil, NewTemporaryError(fmt.Errorf("failed to open blob data source: %w", err)) } - txSucceeded, err := getTxSucceed(ctx, ds.fetcher, ds.ref.Hash) + txSucceeded, err := getTxSucceed(ctx, ds.fetcher, ds.ref.Hash, ds.dsCfg.batchInboxAddress) if err != nil { return nil, err } diff --git a/op-node/rollup/derive/calldata_source.go b/op-node/rollup/derive/calldata_source.go index 48831b1dc133..cfff6e80d903 100644 --- a/op-node/rollup/derive/calldata_source.go +++ b/op-node/rollup/derive/calldata_source.go @@ -44,7 +44,7 @@ func NewCalldataSource(ctx context.Context, log log.Logger, dsCfg DataSourceConf batcherAddr: batcherAddr, } } - txSucceed, err := getTxSucceed(ctx, fetcher, ref.Hash) + txSucceed, err := getTxSucceed(ctx, fetcher, ref.Hash, dsCfg.batchInboxAddress) if err != nil { return &CalldataSource{ open: false, @@ -67,7 +67,7 @@ func NewCalldataSource(ctx context.Context, log log.Logger, dsCfg DataSourceConf func (ds *CalldataSource) Next(ctx context.Context) (eth.Data, error) { if !ds.open { if _, txs, err := ds.fetcher.InfoAndTxsByHash(ctx, ds.ref.Hash); err == nil { - txSucceeded, err := getTxSucceed(ctx, ds.fetcher, ds.ref.Hash) + txSucceeded, err := getTxSucceed(ctx, ds.fetcher, ds.ref.Hash, ds.dsCfg.batchInboxAddress) if err != nil { return nil, err } From 8613b745e599fe16c1253da707d0b9f962e80b31 Mon Sep 17 00:00:00 2001 From: blockchaindevsh Date: Sun, 22 Sep 2024 16:47:52 +0800 Subject: [PATCH 3/3] clear cache in recordFailedTx --- op-batcher/batcher/driver.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/op-batcher/batcher/driver.go b/op-batcher/batcher/driver.go index 7578233ebe9b..56a12e4eb5c0 100644 --- a/op-batcher/batcher/driver.go +++ b/op-batcher/batcher/driver.go @@ -8,6 +8,7 @@ import ( "math/big" _ "net/http/pprof" "sync" + "sync/atomic" "time" "github.com/ethereum-optimism/optimism/op-batcher/metrics" @@ -73,7 +74,7 @@ type BatchSubmitter struct { lastL1Tip eth.L1BlockRef state *channelManager - inboxIsEOA *bool + inboxIsEOA atomic.Pointer[bool] } // NewBatchSubmitter initializes the BatchSubmitter driver from a preconfigured DriverSetup @@ -516,18 +517,20 @@ func (l *BatchSubmitter) sendTransaction(ctx context.Context, txdata txData, que if *candidate.To != l.RollupConfig.BatchInboxAddress { return fmt.Errorf("candidate.To is not inbox") } - if l.inboxIsEOA == nil { + isEOAPointer := l.inboxIsEOA.Load() + if isEOAPointer == nil { var code []byte code, err = l.L1Client.CodeAt(ctx, *candidate.To, nil) if err != nil { return fmt.Errorf("CodeAt failed:%w", err) } isEOA := len(code) == 0 - l.inboxIsEOA = &isEOA + isEOAPointer = &isEOA + l.inboxIsEOA.Store(isEOAPointer) } // Don't set GasLimit when inbox is contract so that later on `EstimateGas` will be called - if !*l.inboxIsEOA { + if !*isEOAPointer { intrinsicGas, err := core.IntrinsicGas(candidate.TxData, nil, false, true, true, false) if err != nil { // we log instead of return an error here because txmgr can do its own gas estimation @@ -589,6 +592,7 @@ func (l *BatchSubmitter) recordL1Tip(l1tip eth.L1BlockRef) { } func (l *BatchSubmitter) recordFailedTx(id txID, err error) { + l.inboxIsEOA.Store(nil) l.Log.Warn("Transaction failed to send", logFields(id, err)...) l.state.TxFailed(id) }