Skip to content

Commit

Permalink
op-batcher: prevent over-assessment of DA type (ethereum-optimism#12115)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
geoknee authored Sep 25, 2024
1 parent 10a16aa commit 616a078
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 6 deletions.
12 changes: 10 additions & 2 deletions op-batcher/batcher/channel_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
19 changes: 15 additions & 4 deletions op-batcher/batcher/channel_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
})
Expand Down

0 comments on commit 616a078

Please sign in to comment.