From cdc3e9df9d7ee47039c5175e01fa1644a5cf0c15 Mon Sep 17 00:00:00 2001
From: Joshua Gutow <jgutow@optimism.io>
Date: Tue, 31 May 2022 13:50:17 -0700
Subject: [PATCH] opnode: Record errors from batch derivation

Errors were not being logged. Note that we keep processing in the
presence of batch derivation errors as we do not trust this L1 input
to always be valid.
---
 op-node/rollup/derive/payload_attributes.go | 15 ++++++++-------
 op-node/rollup/driver/step.go               | 10 +++++++---
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/op-node/rollup/derive/payload_attributes.go b/op-node/rollup/derive/payload_attributes.go
index 638378d98cf6f..e2b193967126b 100644
--- a/op-node/rollup/derive/payload_attributes.go
+++ b/op-node/rollup/derive/payload_attributes.go
@@ -224,32 +224,33 @@ func UserDeposits(receipts []*types.Receipt, depositContractAddr common.Address)
 	return out, errs
 }
 
-func BatchesFromEVMTransactions(config *rollup.Config, txLists []types.Transactions) ([]*BatchData, error) {
+func BatchesFromEVMTransactions(config *rollup.Config, txLists []types.Transactions) ([]*BatchData, []error) {
 	var out []*BatchData
+	var errs []error
 	l1Signer := config.L1Signer()
-	for _, txs := range txLists {
-		for _, tx := range txs {
+	for i, txs := range txLists {
+		for j, tx := range txs {
 			if to := tx.To(); to != nil && *to == config.BatchInboxAddress {
 				seqDataSubmitter, err := l1Signer.Sender(tx) // optimization: only derive sender if To is correct
 				if err != nil {
-					// TODO: log error
+					errs = append(errs, fmt.Errorf("invalid signature: tx list: %d, tx: %d, err: %w", i, j, err))
 					continue // bad signature, ignore
 				}
 				// some random L1 user might have sent a transaction to our batch inbox, ignore them
 				if seqDataSubmitter != config.BatchSenderAddress {
-					// TODO: log/record metric
+					errs = append(errs, fmt.Errorf("unauthorized batch submitter: tx list: %d, tx: %d", i, j))
 					continue // not an authorized batch submitter, ignore
 				}
 				batches, err := DecodeBatches(config, bytes.NewReader(tx.Data()))
 				if err != nil {
-					// TODO: log/record metric
+					errs = append(errs, fmt.Errorf("invalid batch: tx list: %d, tx: %d, err: %w", i, j, err))
 					continue
 				}
 				out = append(out, batches...)
 			}
 		}
 	}
-	return out, nil
+	return out, errs
 }
 
 func FilterBatches(config *rollup.Config, epoch rollup.Epoch, minL2Time uint64, maxL2Time uint64, batches []*BatchData) (out []*BatchData) {
diff --git a/op-node/rollup/driver/step.go b/op-node/rollup/driver/step.go
index c7282e71adecb..4a1de9c746c00 100644
--- a/op-node/rollup/driver/step.go
+++ b/op-node/rollup/driver/step.go
@@ -198,10 +198,14 @@ func (d *outputImpl) insertEpoch(ctx context.Context, l2Head eth.L2BlockRef, l2S
 	if err != nil {
 		return l2Head, l2SafeHead, false, fmt.Errorf("failed to fetch transactions from %s: %v", l1Input, err)
 	}
-	batches, err := derive.BatchesFromEVMTransactions(&d.Config, transactions)
-	if err != nil {
-		return l2Head, l2SafeHead, false, fmt.Errorf("failed to fetch create batches from transactions: %w", err)
+	batches, errs := derive.BatchesFromEVMTransactions(&d.Config, transactions)
+	// Some input to derive.BatchesFromEVMTransactions may be invalid and produce errors.
+	// We log the errors, but keep going as this process is designed to be resilient to these errors
+	// and we have defaults in case no valid (or partial) batches were submitted.
+	for i, err := range errs {
+		d.log.Error("Failed to decode batch", "err_idx", i, "err", err)
 	}
+
 	// Make batches contiguous
 	minL2Time := uint64(l2Info.Timestamp) + d.Config.BlockTime
 	maxL2Time := l1Info.Time() + d.Config.MaxSequencerDrift