[batcher] Cleanup batcher channel inclusion block logic (#12363)

* Cleanup batcher channel inclusion block logic

* Add comment to isTimedOut

* Remove updateInclusionBlocks altogether

* Revert receiver variable rename

* Fix tests

* Fix isTimedOut for ChannelTimeouts of 1 (ensure that some txs have been confirmed)

* Added comment about confirmed txs to isFullySubmitted

op-batcher/batcher/channel.go

@@ -25,8 +25,6 @@ type channel struct {
 	// Set of confirmed txID -> inclusion block. For determining if the channel is timed out
 	confirmedTransactions map[string]eth.BlockID
 
-	// True if confirmed TX list is updated. Set to false after updated min/max inclusion blocks.
-	confirmedTxUpdated bool
 	// Inclusion block number of first confirmed TX
 	minInclusionBlock uint64
 	// Inclusion block number of last confirmed TX
@@ -42,6 +40,7 @@ func newChannel(log log.Logger, metr metrics.Metricer, cfg ChannelConfig, rollup
 		channelBuilder:        cb,
 		pendingTransactions:   make(map[string]txData),
 		confirmedTransactions: make(map[string]eth.BlockID),
+		minInclusionBlock:     math.MaxUint64,
 	}
 }
 
@@ -77,9 +76,12 @@ func (s *channel) TxConfirmed(id string, inclusionBlock eth.BlockID) (bool, []*t
 	}
 	delete(s.pendingTransactions, id)
 	s.confirmedTransactions[id] = inclusionBlock
-	s.confirmedTxUpdated = true
 	s.channelBuilder.FramePublished(inclusionBlock.Number)
 
+	// Update min/max inclusion blocks for timeout check
+	s.minInclusionBlock = min(s.minInclusionBlock, inclusionBlock.Number)
+	s.maxInclusionBlock = max(s.maxInclusionBlock, inclusionBlock.Number)
+
 	// If this channel timed out, put the pending blocks back into the local saved blocks
 	// and then reset this state so it can try to build a new channel.
 	if s.isTimedOut() {
@@ -102,43 +104,18 @@ func (s *channel) Timeout() uint64 {
 	return s.channelBuilder.Timeout()
 }
 
-// updateInclusionBlocks finds the first & last confirmed tx and saves its inclusion numbers
-func (s *channel) updateInclusionBlocks() {
-	if len(s.confirmedTransactions) == 0 || !s.confirmedTxUpdated {
-		return
-	}
-	// If there are confirmed transactions, find the first + last confirmed block numbers
-	min := uint64(math.MaxUint64)
-	max := uint64(0)
-	for _, inclusionBlock := range s.confirmedTransactions {
-		if inclusionBlock.Number < min {
-			min = inclusionBlock.Number
-		}
-		if inclusionBlock.Number > max {
-			max = inclusionBlock.Number
-		}
-	}
-	s.minInclusionBlock = min
-	s.maxInclusionBlock = max
-	s.confirmedTxUpdated = false
-}
-
-// pendingChannelIsTimedOut returns true if submitted channel has timed out.
+// isTimedOut returns true if submitted channel has timed out.
 // A channel has timed out if the difference in L1 Inclusion blocks between
 // the first & last included block is greater than or equal to the channel timeout.
 func (s *channel) isTimedOut() bool {
-	// Update min/max inclusion blocks for timeout check
-	s.updateInclusionBlocks()
 	// Prior to the granite hard fork activating, the use of the shorter ChannelTimeout here may cause the batcher
 	// to believe the channel timed out when it was valid. It would then resubmit the blocks needlessly.
 	// This wastes batcher funds but doesn't cause any problems for the chain progressing safe head.
-	return s.maxInclusionBlock-s.minInclusionBlock >= s.cfg.ChannelTimeout
+	return len(s.confirmedTransactions) > 0 && s.maxInclusionBlock-s.minInclusionBlock >= s.cfg.ChannelTimeout
 }
 
-// pendingChannelIsFullySubmitted returns true if the channel has been fully submitted.
+// isFullySubmitted returns true if the channel has been fully submitted (all transactions are confirmed).
 func (s *channel) isFullySubmitted() bool {
-	// Update min/max inclusion blocks for timeout check
-	s.updateInclusionBlocks()
 	return s.IsFull() && len(s.pendingTransactions)+s.PendingFrames() == 0
 }
 

op-batcher/batcher/channel_test.go

@@ -53,16 +53,19 @@ func TestChannelTimeout(t *testing.T) {
 	channel := m.currentChannel
 	require.NotNil(t, channel)
 
+	// add some pending txs, to be confirmed below
+	channel.pendingTransactions[zeroFrameTxID(0).String()] = txData{}
+	channel.pendingTransactions[zeroFrameTxID(1).String()] = txData{}
+	channel.pendingTransactions[zeroFrameTxID(2).String()] = txData{}
+
 	// There are no confirmed transactions so
 	// the pending channel cannot be timed out
 	timeout := channel.isTimedOut()
 	require.False(t, timeout)
 
-	// Manually set a confirmed transactions
-	// To avoid other methods clearing state
-	channel.confirmedTransactions[zeroFrameTxID(0).String()] = eth.BlockID{Number: 0}
-	channel.confirmedTransactions[zeroFrameTxID(1).String()] = eth.BlockID{Number: 99}
-	channel.confirmedTxUpdated = true
+	// Manually confirm transactions
+	channel.TxConfirmed(zeroFrameTxID(0).String(), eth.BlockID{Number: 0})
+	channel.TxConfirmed(zeroFrameTxID(1).String(), eth.BlockID{Number: 99})
 
 	// Since the ChannelTimeout is 100, the
 	// pending channel should not be timed out
@@ -71,10 +74,7 @@ func TestChannelTimeout(t *testing.T) {
 
 	// Add a confirmed transaction with a higher number
 	// than the ChannelTimeout
-	channel.confirmedTransactions[zeroFrameTxID(2).String()] = eth.BlockID{
-		Number: 101,
-	}
-	channel.confirmedTxUpdated = true
+	channel.TxConfirmed(zeroFrameTxID(2).String(), eth.BlockID{Number: 101})
 
 	// Now the pending channel should be timed out
 	timeout = channel.isTimedOut()