Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

op-batcher: prevent over-assessment of DA type #12115

Merged
merged 3 commits into from
Sep 25, 2024
Merged

op-batcher: prevent over-assessment of DA type #12115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9f97b44
test: assert that default config doesn't change prematurely
geoknee Sep 25, 2024
41cdbfb
test: use a better system to ensure we are not over assessing
geoknee Sep 25, 2024
e2d46fd
return io.EOF from getReadyChannel
geoknee Sep 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions op-batcher/batcher/channel_manager.go
View file
Open in desktop
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
View file
Open in desktop
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