From ee8af34b7778873be3256fb0f5cc83b028750820 Mon Sep 17 00:00:00 2001 From: Roman Dvorkin <121502696+rdvorkin@users.noreply.github.com> Date: Thu, 12 Oct 2023 15:30:40 +0300 Subject: [PATCH 1/7] fix: fixed chunkIntoN to chunk correctly (#6018) * FIX: fixed chunkIntoN to chunk correctly chunkIntoN was chunking into N chunks, however it is used to chunk eth_getCode and eth_getProof responses into chunks of 2, so the desired action should be to chunk into chunks of length N https://github.com/ChainSafe/lodestar/blob/unstable/packages/prover/src/utils/evm.ts#L105 Because the current tests work on chunking array of length 4 into 2 the previous behavior was working and passing tests. When there are 6 requests the current behavior no longer works * Fix strings in test --- packages/prover/src/utils/conversion.ts | 9 ++++----- packages/prover/test/unit/utils/conversion.test.ts | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 5 deletions(-) create mode 100644 packages/prover/test/unit/utils/conversion.test.ts diff --git a/packages/prover/src/utils/conversion.ts b/packages/prover/src/utils/conversion.ts index b26b1852ed74..1f143baf1bda 100644 --- a/packages/prover/src/utils/conversion.ts +++ b/packages/prover/src/utils/conversion.ts @@ -97,12 +97,11 @@ export function cleanObject | unknown[]>(obj: } /** - * Convert an array to array of chunks + * Convert an array to array of chunks of length N * @example - * chunkIntoN([1,2,3,4], 2) - * => [[1,2], [3,4]] + * chunkIntoN([1,2,3,4,5,6], 2) + * => [[1,2], [3,4], [5,6]] */ export function chunkIntoN(arr: T, n: number): T[] { - const size = Math.ceil(arr.length / n); - return Array.from({length: n}, (v, i) => arr.slice(i * size, i * size + size)) as T[]; + return Array.from({length: Math.ceil(arr.length / n)}, (_, i) => arr.slice(i * n, i * n + n)) as T[]; } diff --git a/packages/prover/test/unit/utils/conversion.test.ts b/packages/prover/test/unit/utils/conversion.test.ts new file mode 100644 index 000000000000..0685e02c34f6 --- /dev/null +++ b/packages/prover/test/unit/utils/conversion.test.ts @@ -0,0 +1,14 @@ +import {expect} from "chai"; +import {chunkIntoN} from "../../../src/utils/conversion.js"; + +describe("utils/conversion", () => { + describe("chunkIntoN", () => { + it("should return true if splitting into chunks correctly", async () => { + expect(chunkIntoN([1, 2, 3, 4, 5, 6], 2)).to.be.deep.eq([ + [1, 2], + [3, 4], + [5, 6], + ]); + }); + }); +}); From b34291ddeb7009878a033835567518e4af27da6c Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Thu, 12 Oct 2023 14:42:04 +0200 Subject: [PATCH 2/7] Add more test cases --- .../prover/test/unit/utils/conversion.test.ts | 90 +++++++++++++++++-- 1 file changed, 83 insertions(+), 7 deletions(-) diff --git a/packages/prover/test/unit/utils/conversion.test.ts b/packages/prover/test/unit/utils/conversion.test.ts index 0685e02c34f6..0b70df87d51c 100644 --- a/packages/prover/test/unit/utils/conversion.test.ts +++ b/packages/prover/test/unit/utils/conversion.test.ts @@ -3,12 +3,88 @@ import {chunkIntoN} from "../../../src/utils/conversion.js"; describe("utils/conversion", () => { describe("chunkIntoN", () => { - it("should return true if splitting into chunks correctly", async () => { - expect(chunkIntoN([1, 2, 3, 4, 5, 6], 2)).to.be.deep.eq([ - [1, 2], - [3, 4], - [5, 6], - ]); - }); + const testCases = [ + { + title: "even number of chunks", + input: { + data: [1, 2, 3, 4, 5, 6], + n: 2, + }, + output: [ + [1, 2], + [3, 4], + [5, 6], + ], + }, + { + title: "even number of chunks with additional element", + input: { + data: [1, 2, 3, 4, 5, 6, 7], + n: 2, + }, + output: [[1, 2], [3, 4], [5, 6], [7]], + }, + { + title: "odd number of chunks", + input: { + data: [1, 2, 3, 4, 5, 6], + n: 3, + }, + output: [ + [1, 2, 3], + [4, 5, 6], + ], + }, + { + title: "odd number of chunks with additional element", + input: { + data: [1, 2, 3, 4, 5, 6, 7], + n: 3, + }, + output: [[1, 2, 3], [4, 5, 6], [7]], + }, + { + title: "data less than chunk size", + input: { + data: [1], + n: 3, + }, + output: [[1]], + }, + { + title: "data 1 less than chunk size", + input: { + data: [1, 2], + n: 3, + }, + output: [[1, 2]], + }, + { + title: "data 1 extra than chunk size", + input: { + data: [1, 2, 3, 4], + n: 3, + }, + output: [[1, 2, 3], [4]], + }, + { + title: "when data have different order", + input: { + data: [6, 5, 4, 3, 2, 1], + n: 2, + }, + output: [ + [6, 5], + [4, 3], + [2, 1], + ], + }, + ]; + + for (const {title, input, output} of testCases) { + it(`should split the chunks correctly for "${title}"`, async () => { + expect(chunkIntoN(input.data, input.n)).to.be.deep.eq(output); + }); + } }); }); From 19838a47933bdca5652484c8cd071e081b19a831 Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Thu, 12 Oct 2023 17:29:22 +0200 Subject: [PATCH 3/7] Update test title --- .../prover/test/unit/utils/conversion.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/prover/test/unit/utils/conversion.test.ts b/packages/prover/test/unit/utils/conversion.test.ts index 0b70df87d51c..0f6e5b3e28e1 100644 --- a/packages/prover/test/unit/utils/conversion.test.ts +++ b/packages/prover/test/unit/utils/conversion.test.ts @@ -5,7 +5,7 @@ describe("utils/conversion", () => { describe("chunkIntoN", () => { const testCases = [ { - title: "even number of chunks", + title: "when even number of chunks", input: { data: [1, 2, 3, 4, 5, 6], n: 2, @@ -17,7 +17,7 @@ describe("utils/conversion", () => { ], }, { - title: "even number of chunks with additional element", + title: "when even number of chunks with additional element", input: { data: [1, 2, 3, 4, 5, 6, 7], n: 2, @@ -25,7 +25,7 @@ describe("utils/conversion", () => { output: [[1, 2], [3, 4], [5, 6], [7]], }, { - title: "odd number of chunks", + title: "when odd number of chunks", input: { data: [1, 2, 3, 4, 5, 6], n: 3, @@ -36,7 +36,7 @@ describe("utils/conversion", () => { ], }, { - title: "odd number of chunks with additional element", + title: "when odd number of chunks with additional element", input: { data: [1, 2, 3, 4, 5, 6, 7], n: 3, @@ -44,7 +44,7 @@ describe("utils/conversion", () => { output: [[1, 2, 3], [4, 5, 6], [7]], }, { - title: "data less than chunk size", + title: "when data less than chunk size", input: { data: [1], n: 3, @@ -52,7 +52,7 @@ describe("utils/conversion", () => { output: [[1]], }, { - title: "data 1 less than chunk size", + title: "when data 1 less than chunk size", input: { data: [1, 2], n: 3, @@ -60,7 +60,7 @@ describe("utils/conversion", () => { output: [[1, 2]], }, { - title: "data 1 extra than chunk size", + title: "when data 1 extra than chunk size", input: { data: [1, 2, 3, 4], n: 3, @@ -82,7 +82,7 @@ describe("utils/conversion", () => { ]; for (const {title, input, output} of testCases) { - it(`should split the chunks correctly for "${title}"`, async () => { + it(title, async () => { expect(chunkIntoN(input.data, input.n)).to.be.deep.eq(output); }); } From 100c4473a67d6e28496ae3e79b32e6ddf06a2776 Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Thu, 12 Oct 2023 18:04:30 +0200 Subject: [PATCH 4/7] Update packages/prover/test/unit/utils/conversion.test.ts Co-authored-by: Nico Flaig --- packages/prover/test/unit/utils/conversion.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/prover/test/unit/utils/conversion.test.ts b/packages/prover/test/unit/utils/conversion.test.ts index 0f6e5b3e28e1..2ec5e35dc684 100644 --- a/packages/prover/test/unit/utils/conversion.test.ts +++ b/packages/prover/test/unit/utils/conversion.test.ts @@ -5,7 +5,7 @@ describe("utils/conversion", () => { describe("chunkIntoN", () => { const testCases = [ { - title: "when even number of chunks", + title: "even number of chunks", input: { data: [1, 2, 3, 4, 5, 6], n: 2, From 173dce3e8d002f8bfdfa9aaf24c13b7b5b8b76b8 Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Thu, 12 Oct 2023 18:07:46 +0200 Subject: [PATCH 5/7] Update the test description --- .../prover/test/unit/utils/conversion.test.ts | 34 ++++++++----------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/packages/prover/test/unit/utils/conversion.test.ts b/packages/prover/test/unit/utils/conversion.test.ts index 2ec5e35dc684..61f886dd4f2d 100644 --- a/packages/prover/test/unit/utils/conversion.test.ts +++ b/packages/prover/test/unit/utils/conversion.test.ts @@ -17,7 +17,7 @@ describe("utils/conversion", () => { ], }, { - title: "when even number of chunks with additional element", + title: "even number of chunks with additional element", input: { data: [1, 2, 3, 4, 5, 6, 7], n: 2, @@ -25,7 +25,7 @@ describe("utils/conversion", () => { output: [[1, 2], [3, 4], [5, 6], [7]], }, { - title: "when odd number of chunks", + title: "odd number of chunks", input: { data: [1, 2, 3, 4, 5, 6], n: 3, @@ -36,7 +36,7 @@ describe("utils/conversion", () => { ], }, { - title: "when odd number of chunks with additional element", + title: "odd number of chunks with additional element", input: { data: [1, 2, 3, 4, 5, 6, 7], n: 3, @@ -44,7 +44,7 @@ describe("utils/conversion", () => { output: [[1, 2, 3], [4, 5, 6], [7]], }, { - title: "when data less than chunk size", + title: "data less than chunk size", input: { data: [1], n: 3, @@ -52,7 +52,7 @@ describe("utils/conversion", () => { output: [[1]], }, { - title: "when data 1 less than chunk size", + title: "data 1 less than chunk size", input: { data: [1, 2], n: 3, @@ -60,31 +60,27 @@ describe("utils/conversion", () => { output: [[1, 2]], }, { - title: "when data 1 extra than chunk size", + title: "data 1 extra than chunk size", input: { data: [1, 2, 3, 4], n: 3, }, output: [[1, 2, 3], [4]], }, - { - title: "when data have different order", - input: { - data: [6, 5, 4, 3, 2, 1], - n: 2, - }, - output: [ - [6, 5], - [4, 3], - [2, 1], - ], - }, ]; for (const {title, input, output} of testCases) { - it(title, async () => { + it(`should chunkify data when "${title}"`, async () => { expect(chunkIntoN(input.data, input.n)).to.be.deep.eq(output); }); + + it("should not change the order of elements", () => { + expect(chunkIntoN([6, 5, 4, 3, 2, 1], 2)).to.be.deep.eq([ + [6, 5], + [4, 3], + [2, 1], + ]); + }); } }); }); From a4ec47a2d417e625a80caa70e98813afa7d7f3e4 Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Thu, 12 Oct 2023 18:12:59 +0200 Subject: [PATCH 6/7] Update the test description --- .../prover/test/unit/utils/conversion.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/prover/test/unit/utils/conversion.test.ts b/packages/prover/test/unit/utils/conversion.test.ts index 61f886dd4f2d..557e40c0f1c1 100644 --- a/packages/prover/test/unit/utils/conversion.test.ts +++ b/packages/prover/test/unit/utils/conversion.test.ts @@ -73,14 +73,14 @@ describe("utils/conversion", () => { it(`should chunkify data when "${title}"`, async () => { expect(chunkIntoN(input.data, input.n)).to.be.deep.eq(output); }); - - it("should not change the order of elements", () => { - expect(chunkIntoN([6, 5, 4, 3, 2, 1], 2)).to.be.deep.eq([ - [6, 5], - [4, 3], - [2, 1], - ]); - }); } + + it("should not change the order of elements", () => { + expect(chunkIntoN([6, 5, 4, 3, 2, 1], 2)).to.be.deep.eq([ + [6, 5], + [4, 3], + [2, 1], + ]); + }); }); }); From 7a6cd92a72cb7a60a90f4d3457c51c7f3118e2ee Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Thu, 12 Oct 2023 18:13:47 +0200 Subject: [PATCH 7/7] Update the test description --- packages/prover/test/unit/utils/conversion.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/prover/test/unit/utils/conversion.test.ts b/packages/prover/test/unit/utils/conversion.test.ts index 557e40c0f1c1..50ed03a89450 100644 --- a/packages/prover/test/unit/utils/conversion.test.ts +++ b/packages/prover/test/unit/utils/conversion.test.ts @@ -70,7 +70,7 @@ describe("utils/conversion", () => { ]; for (const {title, input, output} of testCases) { - it(`should chunkify data when "${title}"`, async () => { + it(`should chunkify data when ${title}`, async () => { expect(chunkIntoN(input.data, input.n)).to.be.deep.eq(output); }); }