diff --git a/op-batcher/Makefile b/op-batcher/Makefile index 3235b3b772995..7ec5761e58163 100644 --- a/op-batcher/Makefile +++ b/op-batcher/Makefile @@ -20,6 +20,7 @@ lint: golangci-lint run -E goimports,sqlclosecheck,bodyclose,asciicheck,misspell,errorlint -e "errors.As" -e "errors.Is" fuzz: + go test -run NOTAREALTEST -v -fuzztime 10s -fuzz FuzzChannelConfig_CheckTimeout ./batcher go test -run NOTAREALTEST -v -fuzztime 10s -fuzz FuzzDurationZero ./batcher go test -run NOTAREALTEST -v -fuzztime 10s -fuzz FuzzDurationTimeoutMaxChannelDuration ./batcher go test -run NOTAREALTEST -v -fuzztime 10s -fuzz FuzzDurationTimeoutZeroMaxChannelDuration ./batcher diff --git a/op-batcher/batcher/channel_builder.go b/op-batcher/batcher/channel_builder.go index 77d7c0d8574fa..9fdf0dfcba39d 100644 --- a/op-batcher/batcher/channel_builder.go +++ b/op-batcher/batcher/channel_builder.go @@ -12,8 +12,6 @@ import ( ) var ( - ErrZeroMaxFrameSize = errors.New("max frame size cannot be zero") - ErrSmallMaxFrameSize = errors.New("max frame size cannot be less than 23") ErrInvalidChannelTimeout = errors.New("channel timeout is less than the safety margin") ErrInputTargetReached = errors.New("target amount of input data reached") ErrMaxFrameIndex = errors.New("max frame index reached (uint16)") @@ -83,15 +81,15 @@ func (cc *ChannelConfig) Check() error { // will infinitely loop when trying to create frames in the // [channelBuilder.OutputFrames] function. if cc.MaxFrameSize == 0 { - return ErrZeroMaxFrameSize + return errors.New("max frame size cannot be zero") } - // If the [MaxFrameSize] is set to < 23, the channel out - // will underflow the maxSize variable in the [derive.ChannelOut]. + // If the [MaxFrameSize] is less than [FrameV0OverHeadSize], the channel + // out will underflow the maxSize variable in the [derive.ChannelOut]. // Since it is of type uint64, it will wrap around to a very large // number, making the frame size extremely large. - if cc.MaxFrameSize < 23 { - return ErrSmallMaxFrameSize + if cc.MaxFrameSize < derive.FrameV0OverHeadSize { + return fmt.Errorf("max frame size %d is less than the minimum 23", cc.MaxFrameSize) } return nil diff --git a/op-batcher/batcher/channel_builder_test.go b/op-batcher/batcher/channel_builder_test.go index 03e8c4d9b5915..7653964235294 100644 --- a/op-batcher/batcher/channel_builder_test.go +++ b/op-batcher/batcher/channel_builder_test.go @@ -3,6 +3,7 @@ package batcher import ( "bytes" "errors" + "fmt" "math" "math/big" "math/rand" @@ -32,27 +33,79 @@ var defaultTestChannelConfig = ChannelConfig{ ApproxComprRatio: 0.4, } -// TestConfigValidation tests the validation of the [ChannelConfig] struct. -func TestConfigValidation(t *testing.T) { - // Construct a valid config. - validChannelConfig := defaultTestChannelConfig - require.NoError(t, validChannelConfig.Check()) - - // Set the config to have a zero max frame size. - validChannelConfig.MaxFrameSize = 0 - require.ErrorIs(t, validChannelConfig.Check(), ErrZeroMaxFrameSize) - - // Set the config to have a max frame size less than 23. - validChannelConfig.MaxFrameSize = 22 - require.ErrorIs(t, validChannelConfig.Check(), ErrSmallMaxFrameSize) - - // Reset the config and test the Timeout error. - // NOTE: We should be fuzzing these values with the constraint that - // SubSafetyMargin > ChannelTimeout to ensure validation. - validChannelConfig = defaultTestChannelConfig - validChannelConfig.ChannelTimeout = 0 - validChannelConfig.SubSafetyMargin = 1 - require.ErrorIs(t, validChannelConfig.Check(), ErrInvalidChannelTimeout) +// TestChannelConfig_Check tests the [ChannelConfig] [Check] function. +func TestChannelConfig_Check(t *testing.T) { + type test struct { + input ChannelConfig + assertion func(error) + } + + // Construct test cases that test the boundary conditions + zeroChannelConfig := defaultTestChannelConfig + zeroChannelConfig.MaxFrameSize = 0 + timeoutChannelConfig := defaultTestChannelConfig + timeoutChannelConfig.ChannelTimeout = 0 + timeoutChannelConfig.SubSafetyMargin = 1 + tests := []test{ + { + input: defaultTestChannelConfig, + assertion: func(output error) { + require.NoError(t, output) + }, + }, + { + input: timeoutChannelConfig, + assertion: func(output error) { + require.ErrorIs(t, output, ErrInvalidChannelTimeout) + }, + }, + { + input: zeroChannelConfig, + assertion: func(output error) { + require.EqualError(t, output, "max frame size cannot be zero") + }, + }, + } + for i := 1; i < derive.FrameV0OverHeadSize; i++ { + smallChannelConfig := defaultTestChannelConfig + smallChannelConfig.MaxFrameSize = uint64(i) + expectedErr := fmt.Sprintf("max frame size %d is less than the minimum 23", i) + tests = append(tests, test{ + input: smallChannelConfig, + assertion: func(output error) { + require.EqualError(t, output, expectedErr) + }, + }) + } + + // Run the table tests + for _, test := range tests { + test.assertion(test.input.Check()) + } +} + +// FuzzChannelConfig_CheckTimeout tests the [ChannelConfig] [Check] function +// with fuzzing to make sure that a [ErrInvalidChannelTimeout] is thrown when +// the [ChannelTimeout] is less than the [SubSafetyMargin]. +func FuzzChannelConfig_CheckTimeout(f *testing.F) { + for i := range [10]int{} { + f.Add(uint64(i+1), uint64(i)) + } + f.Fuzz(func(t *testing.T, channelTimeout uint64, subSafetyMargin uint64) { + // We only test where [ChannelTimeout] is less than the [SubSafetyMargin] + // So we cannot have [ChannelTimeout] be [math.MaxUint64] + if channelTimeout == math.MaxUint64 { + channelTimeout = math.MaxUint64 - 1 + } + if subSafetyMargin <= channelTimeout { + subSafetyMargin = channelTimeout + 1 + } + + channelConfig := defaultTestChannelConfig + channelConfig.ChannelTimeout = channelTimeout + channelConfig.SubSafetyMargin = subSafetyMargin + require.ErrorIs(t, channelConfig.Check(), ErrInvalidChannelTimeout) + }) } // addMiniBlock adds a minimal valid L2 block to the channel builder using the