From 616a078ea1159c9fa0436a81219e946e28adb10a Mon Sep 17 00:00:00 2001
From: George Knee <georgeknee@googlemail.com>
Date: Wed, 25 Sep 2024 19:22:30 +0100
Subject: [PATCH] op-batcher: prevent over-assessment of DA type (#12115)

* test: assert that default config doesn't change prematurely

* test: use a better system to ensure we are not over assessing

* return io.EOF from getReadyChannel

when the current channel has no tx data

also improve godoc
---
 op-batcher/batcher/channel_manager.go      | 12 ++++++++++--
 op-batcher/batcher/channel_manager_test.go | 19 +++++++++++++++----
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/op-batcher/batcher/channel_manager.go b/op-batcher/batcher/channel_manager.go
index 3bfff303db4b..23e8f7843696 100644
--- a/op-batcher/batcher/channel_manager.go
+++ b/op-batcher/batcher/channel_manager.go
@@ -193,7 +193,11 @@ func (s *channelManager) TxData(l1Head eth.BlockID) (txData, error) {
 }
 
 // getReadyChannel returns the next channel ready to submit data, or an error.
-// It adds blocks from the block queue to the current channel and generates frames for it.
+// It will create a new channel if necessary.
+// If there is no data ready to send, it adds blocks from the block queue
+// to the current channel and generates frames for it.
+// Always returns nil and the io.EOF sentinel error when
+// there is no channel with txData
 func (s *channelManager) getReadyChannel(l1Head eth.BlockID) (*channel, error) {
 	var firstWithTxData *channel
 	for _, ch := range s.channelQueue {
@@ -239,7 +243,11 @@ func (s *channelManager) getReadyChannel(l1Head eth.BlockID) (*channel, error) {
 		return nil, err
 	}
 
-	return s.currentChannel, nil
+	if s.currentChannel.HasTxData() {
+		return s.currentChannel, nil
+	}
+
+	return nil, io.EOF
 }
 
 // ensureChannelWithSpace ensures currentChannel is populated with a channel that has
diff --git a/op-batcher/batcher/channel_manager_test.go b/op-batcher/batcher/channel_manager_test.go
index 5df5feacf4bf..c129cd9cde99 100644
--- a/op-batcher/batcher/channel_manager_test.go
+++ b/op-batcher/batcher/channel_manager_test.go
@@ -491,9 +491,11 @@ func TestChannelManager_ChannelCreation(t *testing.T) {
 type FakeDynamicEthChannelConfig struct {
 	DynamicEthChannelConfig
 	chooseBlobs bool
+	assessments int
 }
 
 func (f *FakeDynamicEthChannelConfig) ChannelConfig() ChannelConfig {
+	f.assessments++
 	if f.chooseBlobs {
 		return f.blobConfig
 	}
@@ -537,13 +539,21 @@ func TestChannelManager_TxData(t *testing.T) {
 		name                            string
 		chooseBlobsWhenChannelCreated   bool
 		chooseBlobsWhenChannelSubmitted bool
+
+		// * One when the channelManager was created
+		// * One when the channel is about to be submitted
+		// * Potentially one more if the replacement channel is about to be submitted,
+		//   this only happens when going from calldata->blobs because
+		//   the channel is no longer ready to send until more data
+		//   is added.
+		numExpectedAssessments int
 	}
 
 	tt := []TestCase{
-		{"blobs->blobs", true, true},
-		{"calldata->calldata", false, false},
-		{"blobs->calldata", true, false},
-		{"calldata->blobs", false, true},
+		{"blobs->blobs", true, true, 2},
+		{"calldata->calldata", false, false, 2},
+		{"blobs->calldata", true, false, 2},
+		{"calldata->blobs", false, true, 3},
 	}
 
 	for _, tc := range tt {
@@ -590,6 +600,7 @@ func TestChannelManager_TxData(t *testing.T) {
 				}
 			}
 
+			require.Equal(t, tc.numExpectedAssessments, cfg.assessments)
 			require.Equal(t, tc.chooseBlobsWhenChannelSubmitted, data.asBlob)
 			require.Equal(t, tc.chooseBlobsWhenChannelSubmitted, m.defaultCfg.UseBlobs)
 		})