Fix nil batchFetcher dereference in FillInBatchGasFields

arbnode/inbox_tracker.go

@@ -338,16 +338,15 @@ func (t *InboxTracker) legacyGetDelayedMessageAndAccumulator(ctx context.Context
 	}
 	var acc common.Hash
 	copy(acc[:], data[:32])
-	msg, err := arbostypes.ParseIncomingL1Message(bytes.NewReader(data[32:]), nil)
+	batchFetcher := func(batchNum uint64) ([]byte, error) {
+		data, _, err := t.txStreamer.inboxReader.GetSequencerMessageBytes(ctx, batchNum)
+		return data, err
+	}
+	msg, err := arbostypes.ParseIncomingL1Message(bytes.NewReader(data[32:]), batchFetcher)
 	if err != nil {
 		return nil, common.Hash{}, err
 	}
 
-	err = msg.FillInBatchGasFields(func(batchNum uint64) ([]byte, error) {
-		data, _, err := t.txStreamer.inboxReader.GetSequencerMessageBytes(ctx, batchNum)
-		return data, err
-	})
-
 	return msg, acc, err
 }
 

arbnode/transaction_streamer.go

@@ -382,9 +382,13 @@ func (s *TransactionStreamer) addMessagesAndReorg(batch ethdb.Batch, msgIdxOfFir
 					continue
 				}
 				msgBlockNum := new(big.Int).SetUint64(oldMessage.Message.Header.BlockNumber)
-				delayedInBlock, err := s.delayedBridge.LookupMessagesInRange(s.GetContext(), msgBlockNum, msgBlockNum, nil)
+				batchFetcher := func(batchNum uint64, parentChainBlockNumber uint64) ([]byte, error) {
+					data, _, err := s.inboxReader.GetSequencerMessageBytesForParentBlock(s.GetContext(), batchNum, parentChainBlockNumber)
+					return data, err
+				}
+				delayedInBlock, err := s.delayedBridge.LookupMessagesInRange(s.GetContext(), msgBlockNum, msgBlockNum, batchFetcher)
 				if err != nil {
-					log.Error("reorg-resequence: failed to serialize old delayed message from database", "err", err)
+					log.Error("reorg-resequence: failed to look up old delayed message from L1", "err", err)
 					continue
 				}
 				messageFound := false
@@ -516,7 +520,7 @@ func (s *TransactionStreamer) GetMessage(msgIdx arbutil.MessageIndex) (*arbostyp
 	}
 
 	if s.inboxReader != nil {
-		err = message.Message.FillInBatchGasFields(func(batchNum uint64) ([]byte, error) {
+		batchFetcher := func(batchNum uint64) ([]byte, error) {
 			ctx, err := s.GetContextSafe()
 			if err != nil {
 				return nil, err
@@ -533,10 +537,13 @@ func (s *TransactionStreamer) GetMessage(msgIdx arbutil.MessageIndex) (*arbostyp
 			}
 
 			return data, err
-		})
+		}
+		err = message.Message.FillInBatchGasFields(batchFetcher)
 		if err != nil {
 			return nil, err
 		}
+	} else if message.Message.Header.Kind == arbostypes.L1MessageType_BatchPostingReport {
+		log.Warn("GetMessage: cannot fill in batch gas fields for BatchPostingReport without inboxReader", "msgIdx", msgIdx)
 	}
 	return &message, nil
 }

arbos/arbostypes/incomingmessage.go

@@ -36,6 +36,12 @@ const (
 
 const MaxL2MessageSize = 256 * 1024
 
+var (
+	ErrNilBatchFetcher         = errors.New("batch fetcher is nil")
+	ErrBatchHashMismatch       = errors.New("batch data hash mismatch")
+	ErrParseBatchPostingReport = errors.New("failed to parse batch posting report")
+)
+
 func ValidateMaxTxDataSize(maxTxDataSize uint64) error {
 	// tighter limit https://github.com/OffchainLabs/nitro/commit/ed015e752d7d24e59ec9e6f894fe1a26ffa19036
 	// The default block gas limit can fit 1523 txs
@@ -189,11 +195,20 @@ func LegacyCostForStats(stats *BatchDataStats) uint64 {
 }
 
 func (msg *L1IncomingMessage) FillInBatchGasFields(batchFetcher FallibleBatchFetcher) error {
+	if batchFetcher == nil {
+		if msg.Header.Kind == L1MessageType_BatchPostingReport {
+			return fmt.Errorf("%w: cannot fill in batch gas fields for BatchPostingReport", ErrNilBatchFetcher)
+		}
+		return nil
+	}
 	return msg.FillInBatchGasFieldsWithParentBlock(FromFallibleBatchFetcher(batchFetcher), msg.Header.BlockNumber)
 }
 
 func (msg *L1IncomingMessage) FillInBatchGasFieldsWithParentBlock(batchFetcher FallibleBatchFetcherWithParentBlock, parentChainBlockNumber uint64) error {
-	if batchFetcher == nil || msg.Header.Kind != L1MessageType_BatchPostingReport {
+	if msg.Header.Kind != L1MessageType_BatchPostingReport {
+		return nil
+	}
+	if batchFetcher == nil {
 		return nil
 	}
 	if msg.BatchDataStats != nil && msg.LegacyBatchGasCost != nil {
@@ -202,7 +217,7 @@ func (msg *L1IncomingMessage) FillInBatchGasFieldsWithParentBlock(batchFetcher F
 	if msg.BatchDataStats == nil {
 		_, _, batchHash, batchNum, _, _, err := ParseBatchPostingReportMessageFields(bytes.NewReader(msg.L2msg))
 		if err != nil {
-			return fmt.Errorf("failed to parse batch posting report: %w", err)
+			return fmt.Errorf("%w: %w", ErrParseBatchPostingReport, err)
 		}
 		batchData, err := batchFetcher(batchNum, parentChainBlockNumber)
 		if err != nil {
@@ -220,12 +235,12 @@ func (msg *L1IncomingMessage) FillInBatchGasFieldsWithParentBlock(batchFetcher F
 			// missing BatchDataStats and fail there, since it does know the arbos
 			// version. In practice, any node that supports arbos50 populates both
 			// fields together, so this fallback path should not be reached.
-			log.Warn("Failed reading batch data for filling message - leaving BatchDataStats empty")
+			log.Warn("Failed reading batch data for filling message - leaving BatchDataStats empty", "batchNum", batchNum, "err", err)
 			return nil
 		} else {
 			gotHash := crypto.Keccak256Hash(batchData)
 			if gotHash != batchHash {
-				return fmt.Errorf("batch fetcher returned incorrect data hash %v (wanted %v for batch %v)", gotHash, batchHash, batchNum)
+				return fmt.Errorf("%w: got %v, wanted %v for batch %v", ErrBatchHashMismatch, gotHash, batchHash, batchNum)
 			}
 			msg.BatchDataStats = GetDataStats(batchData)
 		}