From d71b1bc86e557e7ee14dcc033662561e763cd014 Mon Sep 17 00:00:00 2001 From: Olga Date: Tue, 11 Feb 2025 13:08:00 +0800 Subject: [PATCH 1/5] fix op-batcher pack over MaxRLPBytesChannel --- op-node/rollup/derive/channel_out_test.go | 18 ++++++++++++++++++ op-node/rollup/derive/span_channel_out.go | 1 + 2 files changed, 19 insertions(+) diff --git a/op-node/rollup/derive/channel_out_test.go b/op-node/rollup/derive/channel_out_test.go index 328be3b7c5ad..cfc351020a0e 100644 --- a/op-node/rollup/derive/channel_out_test.go +++ b/op-node/rollup/derive/channel_out_test.go @@ -491,3 +491,21 @@ func testSpanChannelOut_MaxBlocksPerSpanBatch(t *testing.T, tt maxBlocksTest) { require.Equalf(t, batch0, batch, "iteration %d", i) } } + +func TestSpanChannelOut_ExceedMaxRLPBytesPerChannel(t *testing.T) { + + largeBatchSize := int(rollup.NewChainSpec(&rollupCfg).MaxRLPBytesPerChannel(0)) + cout, singularBatches := SpanChannelAndBatches(t, 100, 1, Zlib) + + cout.rlp[0] = bytes.NewBuffer(make([]byte, largeBatchSize)) + cout.rlp[1] = bytes.NewBuffer(make([]byte, largeBatchSize)) + cout.sealedRLPBytes = largeBatchSize + + for _, batch := range singularBatches { + err := cout.addSingularBatch(batch, 1) + require.ErrorIs(t, err, ErrTooManyRLPBytes) + } + + require.Equal(t, cout.activeRLP().Len(), largeBatchSize) + require.Greater(t, cout.inactiveRLP().Len(), largeBatchSize) +} diff --git a/op-node/rollup/derive/span_channel_out.go b/op-node/rollup/derive/span_channel_out.go index b363cd4958dd..f92da823c403 100644 --- a/op-node/rollup/derive/span_channel_out.go +++ b/op-node/rollup/derive/span_channel_out.go @@ -178,6 +178,7 @@ func (co *SpanChannelOut) addSingularBatch(batch *SingularBatch, seqNum uint64) // the Fjord activation. maxRLPBytesPerChannel := co.chainSpec.MaxRLPBytesPerChannel(batch.Timestamp) if active.Len() > int(maxRLPBytesPerChannel) { + co.swapRLP() return fmt.Errorf("could not take %d bytes as replacement of channel of %d bytes, max is %d. err: %w", active.Len(), co.inactiveRLP().Len(), maxRLPBytesPerChannel, ErrTooManyRLPBytes) } From 07337467d265ceca1144f538578d389b3870d4d7 Mon Sep 17 00:00:00 2001 From: Olga Date: Tue, 11 Feb 2025 13:21:31 +0800 Subject: [PATCH 2/5] add test cases from different CompressionAlgo --- op-node/rollup/derive/channel_out_test.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/op-node/rollup/derive/channel_out_test.go b/op-node/rollup/derive/channel_out_test.go index cfc351020a0e..2eb3d8fe53a4 100644 --- a/op-node/rollup/derive/channel_out_test.go +++ b/op-node/rollup/derive/channel_out_test.go @@ -493,9 +493,17 @@ func testSpanChannelOut_MaxBlocksPerSpanBatch(t *testing.T, tt maxBlocksTest) { } func TestSpanChannelOut_ExceedMaxRLPBytesPerChannel(t *testing.T) { + for _, algo := range CompressionAlgos { + t.Run("testSpanChannelOut_ExceedMaxRLPBytesPerChannel_"+algo.String(), func(t *testing.T) { + testSpanChannelOut_ExceedMaxRLPBytesPerChannel(t, algo) + }) + } +} + +func testSpanChannelOut_ExceedMaxRLPBytesPerChannel(t *testing.T, algo CompressionAlgo) { largeBatchSize := int(rollup.NewChainSpec(&rollupCfg).MaxRLPBytesPerChannel(0)) - cout, singularBatches := SpanChannelAndBatches(t, 100, 1, Zlib) + cout, singularBatches := SpanChannelAndBatches(t, 100, 1, algo) cout.rlp[0] = bytes.NewBuffer(make([]byte, largeBatchSize)) cout.rlp[1] = bytes.NewBuffer(make([]byte, largeBatchSize)) From 7c17aeec012938459673b443f699ce6057805f2b Mon Sep 17 00:00:00 2001 From: Olga Date: Fri, 14 Feb 2025 10:38:39 +0800 Subject: [PATCH 3/5] add fresh compression logic and corresponding comments --- op-node/rollup/derive/span_channel_out.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/op-node/rollup/derive/span_channel_out.go b/op-node/rollup/derive/span_channel_out.go index f92da823c403..c8e13ec8a5f8 100644 --- a/op-node/rollup/derive/span_channel_out.go +++ b/op-node/rollup/derive/span_channel_out.go @@ -178,7 +178,13 @@ func (co *SpanChannelOut) addSingularBatch(batch *SingularBatch, seqNum uint64) // the Fjord activation. maxRLPBytesPerChannel := co.chainSpec.MaxRLPBytesPerChannel(batch.Timestamp) if active.Len() > int(maxRLPBytesPerChannel) { + + // if active size exceeds MaxRLPBytesPerChannel we revert the last batch + // by switching the RLP buffer and doing a fresh compression co.swapRLP() + if err = co.compress(); err != nil { + return err + } return fmt.Errorf("could not take %d bytes as replacement of channel of %d bytes, max is %d. err: %w", active.Len(), co.inactiveRLP().Len(), maxRLPBytesPerChannel, ErrTooManyRLPBytes) } From 63cf3d701d5ed3aaec83a156da7c2733b81b66f2 Mon Sep 17 00:00:00 2001 From: Olga Date: Fri, 14 Feb 2025 10:44:57 +0800 Subject: [PATCH 4/5] refactor: enhance MaxRLPBytesPerChannel test --- op-node/rollup/derive/channel_out_test.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/op-node/rollup/derive/channel_out_test.go b/op-node/rollup/derive/channel_out_test.go index 2eb3d8fe53a4..5122746150cb 100644 --- a/op-node/rollup/derive/channel_out_test.go +++ b/op-node/rollup/derive/channel_out_test.go @@ -492,15 +492,15 @@ func testSpanChannelOut_MaxBlocksPerSpanBatch(t *testing.T, tt maxBlocksTest) { } } -func TestSpanChannelOut_ExceedMaxRLPBytesPerChannel(t *testing.T) { +func TestSpanChannelOut_MaxRLPBytesPerChannel(t *testing.T) { for _, algo := range CompressionAlgos { - t.Run("testSpanChannelOut_ExceedMaxRLPBytesPerChannel_"+algo.String(), func(t *testing.T) { - testSpanChannelOut_ExceedMaxRLPBytesPerChannel(t, algo) + t.Run("testSpanChannelOut_MaxRLPBytesPerChannel_"+algo.String(), func(t *testing.T) { + testSpanChannelOut_MaxRLPBytesPerChannel(t, algo) }) } } -func testSpanChannelOut_ExceedMaxRLPBytesPerChannel(t *testing.T, algo CompressionAlgo) { +func testSpanChannelOut_MaxRLPBytesPerChannel(t *testing.T, algo CompressionAlgo) { largeBatchSize := int(rollup.NewChainSpec(&rollupCfg).MaxRLPBytesPerChannel(0)) cout, singularBatches := SpanChannelAndBatches(t, 100, 1, algo) @@ -509,11 +509,9 @@ func testSpanChannelOut_ExceedMaxRLPBytesPerChannel(t *testing.T, algo Compressi cout.rlp[1] = bytes.NewBuffer(make([]byte, largeBatchSize)) cout.sealedRLPBytes = largeBatchSize - for _, batch := range singularBatches { - err := cout.addSingularBatch(batch, 1) - require.ErrorIs(t, err, ErrTooManyRLPBytes) - } + err := cout.addSingularBatch(singularBatches[0], 1) + require.ErrorIs(t, err, ErrTooManyRLPBytes) - require.Equal(t, cout.activeRLP().Len(), largeBatchSize) + require.Equal(t, cout.activeRLP().Len(), largeBatchSize, "active RLP should be equal to the max RLP limit") require.Greater(t, cout.inactiveRLP().Len(), largeBatchSize) } From 2ee7a3099220b2ace9c10adbb81634f331d70cdf Mon Sep 17 00:00:00 2001 From: Olga Date: Fri, 14 Feb 2025 23:14:00 +0800 Subject: [PATCH 5/5] refactor: rename variable and add required messages --- op-node/rollup/derive/channel_out_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/op-node/rollup/derive/channel_out_test.go b/op-node/rollup/derive/channel_out_test.go index 5122746150cb..e82653cfd985 100644 --- a/op-node/rollup/derive/channel_out_test.go +++ b/op-node/rollup/derive/channel_out_test.go @@ -502,16 +502,16 @@ func TestSpanChannelOut_MaxRLPBytesPerChannel(t *testing.T) { func testSpanChannelOut_MaxRLPBytesPerChannel(t *testing.T, algo CompressionAlgo) { - largeBatchSize := int(rollup.NewChainSpec(&rollupCfg).MaxRLPBytesPerChannel(0)) + maxRLPBytesPerChannel := int(rollup.NewChainSpec(&rollupCfg).MaxRLPBytesPerChannel(0)) cout, singularBatches := SpanChannelAndBatches(t, 100, 1, algo) - cout.rlp[0] = bytes.NewBuffer(make([]byte, largeBatchSize)) - cout.rlp[1] = bytes.NewBuffer(make([]byte, largeBatchSize)) - cout.sealedRLPBytes = largeBatchSize + cout.rlp[0] = bytes.NewBuffer(make([]byte, maxRLPBytesPerChannel)) + cout.rlp[1] = bytes.NewBuffer(make([]byte, maxRLPBytesPerChannel)) + cout.sealedRLPBytes = maxRLPBytesPerChannel err := cout.addSingularBatch(singularBatches[0], 1) - require.ErrorIs(t, err, ErrTooManyRLPBytes) + require.ErrorIs(t, err, ErrTooManyRLPBytes, "error should be ErrTooManyRLPBytes") - require.Equal(t, cout.activeRLP().Len(), largeBatchSize, "active RLP should be equal to the max RLP limit") - require.Greater(t, cout.inactiveRLP().Len(), largeBatchSize) + require.Equal(t, cout.activeRLP().Len(), maxRLPBytesPerChannel, "active RLP should be equal to the max RLP limit") + require.Greater(t, cout.inactiveRLP().Len(), maxRLPBytesPerChannel, "inactive RLP should be greater than max RLP limit") }