From 09a42161ad3f330787735d9854384edf78914531 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Tue, 22 Jun 2021 15:55:19 +0700 Subject: [PATCH 1/5] Query validator indices in batch --- packages/validator/src/services/indices.ts | 53 +++++++++++++++++-- .../test/unit/services/indices.test.ts | 50 +++++++++++++++++ 2 files changed, 100 insertions(+), 3 deletions(-) create mode 100644 packages/validator/test/unit/services/indices.test.ts diff --git a/packages/validator/src/services/indices.ts b/packages/validator/src/services/indices.ts index 0846a7a90418..38619d9290f7 100644 --- a/packages/validator/src/services/indices.ts +++ b/packages/validator/src/services/indices.ts @@ -58,9 +58,21 @@ export class IndicesService { } // Query the remote BN to resolve a pubkey to a validator index. - const pubkeysHex = pubkeysToPoll.map((pubkey) => toHexString(pubkey)); - const validatorsState = await this.api.beacon.getStateValidators("head", {indices: pubkeysHex}); + // support up to 1000 pubkeys per poll + const pubkeysHex = pubkeysToPoll.map((pubkey) => toHexString(pubkey)).slice(0, MAX_PUBKEYS_PER_POLL); + const batches = pubkeysToBatches(pubkeysHex); + + const newIndicesArr = await Promise.all( + batches.map(async (batch) => { + const validatorIndicesArr = await Promise.all(batch.map(this.getIndicesPerHttpRequest)); + return validatorIndicesArr.flat(); + }) + ); + return newIndicesArr.flat(); + } + private getIndicesPerHttpRequest = async (pubkeysHex: string[]): Promise => { + const validatorsState = await this.api.beacon.getStateValidators("head", {indices: pubkeysHex}); const newIndices = []; for (const validatorState of validatorsState.data) { const pubkeyHex = toHexString(validatorState.validator.pubkey); @@ -71,7 +83,42 @@ export class IndicesService { newIndices.push(validatorState.index); } } - return newIndices; + }; +} + +type Batch = string[][]; +const PUBKEYS_PER_REQUEST = 40; +const REQUESTS_PER_BATCH = 5; +const MAX_PUBKEYS_PER_POLL = 5 * PUBKEYS_PER_REQUEST * REQUESTS_PER_BATCH; + +/** + * Divide pubkeys into batches, each batch contains at most 5 http requests, + * each request can work on at most 40 pubkeys. + * @param pubkeysHex + */ +export function pubkeysToBatches( + pubkeysHex: string[], + maxPubkeysPerRequest: number = PUBKEYS_PER_REQUEST, + maxRequesPerBatch: number = REQUESTS_PER_BATCH +): Batch[] { + if (!pubkeysHex || pubkeysHex.length === 0) { + return [[[]]]; + } + const batches: Batch[] = []; + + const pubkeysPerBatch = maxPubkeysPerRequest * maxRequesPerBatch; + let batch: Batch = []; + let pubkeysPerRequest: string[]; + for (let i = 0; i < pubkeysHex.length; i += maxPubkeysPerRequest) { + if (i % pubkeysPerBatch === 0) { + batch = []; + batches.push(batch); + } + if (i % maxPubkeysPerRequest === 0) { + pubkeysPerRequest = pubkeysHex.slice(i, Math.min(i + maxPubkeysPerRequest, pubkeysHex.length)); + batch.push(pubkeysPerRequest); + } } + return batches; } diff --git a/packages/validator/test/unit/services/indices.test.ts b/packages/validator/test/unit/services/indices.test.ts new file mode 100644 index 000000000000..ee69809db564 --- /dev/null +++ b/packages/validator/test/unit/services/indices.test.ts @@ -0,0 +1,50 @@ +import {expect} from "chai"; +import {pubkeysToBatches} from "../../../src/services/indices"; + +describe("pubkeysToBatches", function () { + const testCases: {pubkeys: string[]; expected: string[][][]}[] = [ + {pubkeys: [], expected: [[[]]]}, + {pubkeys: ["1"], expected: [[["1"]]]}, + {pubkeys: ["1", "2"], expected: [[["1", "2"]]]}, + {pubkeys: ["1", "2", "3"], expected: [[["1", "2"], ["3"]]]}, + { + pubkeys: ["1", "2", "3", "4"], + expected: [ + [ + ["1", "2"], + ["3", "4"], + ], + ], + }, + {pubkeys: ["1", "2", "3", "4", "5"], expected: [[["1", "2"], ["3", "4"], ["5"]]]}, + { + pubkeys: ["1", "2", "3", "4", "5", "6"], + expected: [ + [ + ["1", "2"], + ["3", "4"], + ["5", "6"], + ], + ], + }, + // new batch + { + pubkeys: ["1", "2", "3", "4", "5", "6", "7"], + expected: [ + [ + ["1", "2"], + ["3", "4"], + ["5", "6"], + ], + [["7"]], + ], + }, + ]; + const maxPubkeysPerRequest = 2; + const maxRequesPerBatch = 3; + for (const {pubkeys, expected} of testCases) { + it(`should work with ${pubkeys.length} pubkeys`, () => { + expect(pubkeysToBatches(pubkeys, maxPubkeysPerRequest, maxRequesPerBatch)).to.be.deep.equals(expected); + }); + } +}); From c6a0ea60bd17827cb506df86e651b8df9576643a Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Tue, 22 Jun 2021 16:15:33 +0700 Subject: [PATCH 2/5] chore: add log --- packages/validator/src/services/indices.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/validator/src/services/indices.ts b/packages/validator/src/services/indices.ts index 38619d9290f7..7d63123b5035 100644 --- a/packages/validator/src/services/indices.ts +++ b/packages/validator/src/services/indices.ts @@ -68,7 +68,9 @@ export class IndicesService { return validatorIndicesArr.flat(); }) ); - return newIndicesArr.flat(); + const newIndices = newIndicesArr.flat(); + this.logger.info("Discovered new validators", {count: newIndices.length}); + return newIndices; } private getIndicesPerHttpRequest = async (pubkeysHex: string[]): Promise => { From 47056169721a25cfdc5aba64febc6d97d3b58339 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Thu, 24 Jun 2021 07:55:50 +0700 Subject: [PATCH 3/5] Set PUBKEYS_PER_REQUEST to 10 --- packages/validator/src/services/indices.ts | 2 +- packages/validator/test/unit/services/indices.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/validator/src/services/indices.ts b/packages/validator/src/services/indices.ts index 7d63123b5035..1f13777900fb 100644 --- a/packages/validator/src/services/indices.ts +++ b/packages/validator/src/services/indices.ts @@ -90,7 +90,7 @@ export class IndicesService { } type Batch = string[][]; -const PUBKEYS_PER_REQUEST = 40; +const PUBKEYS_PER_REQUEST = 10; const REQUESTS_PER_BATCH = 5; const MAX_PUBKEYS_PER_POLL = 5 * PUBKEYS_PER_REQUEST * REQUESTS_PER_BATCH; diff --git a/packages/validator/test/unit/services/indices.test.ts b/packages/validator/test/unit/services/indices.test.ts index ee69809db564..be8346bea22d 100644 --- a/packages/validator/test/unit/services/indices.test.ts +++ b/packages/validator/test/unit/services/indices.test.ts @@ -41,10 +41,10 @@ describe("pubkeysToBatches", function () { }, ]; const maxPubkeysPerRequest = 2; - const maxRequesPerBatch = 3; + const maxRequestsPerBatch = 3; for (const {pubkeys, expected} of testCases) { it(`should work with ${pubkeys.length} pubkeys`, () => { - expect(pubkeysToBatches(pubkeys, maxPubkeysPerRequest, maxRequesPerBatch)).to.be.deep.equals(expected); + expect(pubkeysToBatches(pubkeys, maxPubkeysPerRequest, maxRequestsPerBatch)).to.be.deep.equals(expected); }); } }); From 3ce3cf6d23cb88b1b4fcd9ad00a95dd93121a96b Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Mon, 28 Jun 2021 13:18:03 +0700 Subject: [PATCH 4/5] Loop batches sequentially instead of Promise.all() --- packages/validator/src/services/indices.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/validator/src/services/indices.ts b/packages/validator/src/services/indices.ts index 1f13777900fb..2a6d84aad878 100644 --- a/packages/validator/src/services/indices.ts +++ b/packages/validator/src/services/indices.ts @@ -62,12 +62,11 @@ export class IndicesService { const pubkeysHex = pubkeysToPoll.map((pubkey) => toHexString(pubkey)).slice(0, MAX_PUBKEYS_PER_POLL); const batches = pubkeysToBatches(pubkeysHex); - const newIndicesArr = await Promise.all( - batches.map(async (batch) => { - const validatorIndicesArr = await Promise.all(batch.map(this.getIndicesPerHttpRequest)); - return validatorIndicesArr.flat(); - }) - ); + const newIndicesArr = []; + for (const batch of batches) { + const validatorIndicesArr = await Promise.all(batch.map(this.getIndicesPerHttpRequest)); + newIndicesArr.push(...validatorIndicesArr.flat()); + } const newIndices = newIndicesArr.flat(); this.logger.info("Discovered new validators", {count: newIndices.length}); return newIndices; From 906d49cffdf16139d9feb715982c838e66aed0af Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Mon, 28 Jun 2021 19:02:50 +0200 Subject: [PATCH 5/5] Review PR --- packages/validator/src/services/indices.ts | 58 +++++-------------- packages/validator/src/util/batch.ts | 16 +++++ .../test/unit/services/indices.test.ts | 50 ---------------- .../validator/test/unit/utils/batch.test.ts | 42 ++++++++++++++ 4 files changed, 72 insertions(+), 94 deletions(-) create mode 100644 packages/validator/src/util/batch.ts delete mode 100644 packages/validator/test/unit/services/indices.test.ts create mode 100644 packages/validator/test/unit/utils/batch.test.ts diff --git a/packages/validator/src/services/indices.ts b/packages/validator/src/services/indices.ts index 2a6d84aad878..3ae707610101 100644 --- a/packages/validator/src/services/indices.ts +++ b/packages/validator/src/services/indices.ts @@ -3,6 +3,13 @@ import {ILogger} from "@chainsafe/lodestar-utils"; import {toHexString} from "@chainsafe/ssz"; import {Api} from "@chainsafe/lodestar-api"; import {ValidatorStore} from "./validatorStore"; +import {batchItems} from "../util/batch"; + +/** + * URLs have a limitation on size, adding an unbounded num of pubkeys will break the request. + * For reasoning on the specific number see: https://github.com/ChainSafe/lodestar/pull/2730#issuecomment-866749083 + */ +const PUBKEYS_PER_REQUEST = 10; // To assist with readability type PubkeyHex = string; @@ -59,20 +66,19 @@ export class IndicesService { // Query the remote BN to resolve a pubkey to a validator index. // support up to 1000 pubkeys per poll - const pubkeysHex = pubkeysToPoll.map((pubkey) => toHexString(pubkey)).slice(0, MAX_PUBKEYS_PER_POLL); - const batches = pubkeysToBatches(pubkeysHex); + const pubkeysHex = pubkeysToPoll.map((pubkey) => toHexString(pubkey)); + const pubkeysHexBatches = batchItems(pubkeysHex, {batchSize: PUBKEYS_PER_REQUEST}); - const newIndicesArr = []; - for (const batch of batches) { - const validatorIndicesArr = await Promise.all(batch.map(this.getIndicesPerHttpRequest)); - newIndicesArr.push(...validatorIndicesArr.flat()); + const newIndices: number[] = []; + for (const pubkeysHexBatch of pubkeysHexBatches) { + const validatorIndicesArr = await this.fetchValidatorIndices(pubkeysHexBatch); + newIndices.push(...validatorIndicesArr); } - const newIndices = newIndicesArr.flat(); this.logger.info("Discovered new validators", {count: newIndices.length}); return newIndices; } - private getIndicesPerHttpRequest = async (pubkeysHex: string[]): Promise => { + private async fetchValidatorIndices(pubkeysHex: string[]): Promise { const validatorsState = await this.api.beacon.getStateValidators("head", {indices: pubkeysHex}); const newIndices = []; for (const validatorState of validatorsState.data) { @@ -85,41 +91,5 @@ export class IndicesService { } } return newIndices; - }; -} - -type Batch = string[][]; -const PUBKEYS_PER_REQUEST = 10; -const REQUESTS_PER_BATCH = 5; -const MAX_PUBKEYS_PER_POLL = 5 * PUBKEYS_PER_REQUEST * REQUESTS_PER_BATCH; - -/** - * Divide pubkeys into batches, each batch contains at most 5 http requests, - * each request can work on at most 40 pubkeys. - * @param pubkeysHex - */ -export function pubkeysToBatches( - pubkeysHex: string[], - maxPubkeysPerRequest: number = PUBKEYS_PER_REQUEST, - maxRequesPerBatch: number = REQUESTS_PER_BATCH -): Batch[] { - if (!pubkeysHex || pubkeysHex.length === 0) { - return [[[]]]; - } - const batches: Batch[] = []; - - const pubkeysPerBatch = maxPubkeysPerRequest * maxRequesPerBatch; - let batch: Batch = []; - let pubkeysPerRequest: string[]; - for (let i = 0; i < pubkeysHex.length; i += maxPubkeysPerRequest) { - if (i % pubkeysPerBatch === 0) { - batch = []; - batches.push(batch); - } - if (i % maxPubkeysPerRequest === 0) { - pubkeysPerRequest = pubkeysHex.slice(i, Math.min(i + maxPubkeysPerRequest, pubkeysHex.length)); - batch.push(pubkeysPerRequest); - } } - return batches; } diff --git a/packages/validator/src/util/batch.ts b/packages/validator/src/util/batch.ts new file mode 100644 index 000000000000..f0cfa9c7ee9d --- /dev/null +++ b/packages/validator/src/util/batch.ts @@ -0,0 +1,16 @@ +/** + * Divide pubkeys into batches, each batch contains at most 5 http requests, + * each request can work on at most 40 pubkeys. + */ +export function batchItems(items: T[], opts: {batchSize: number; maxBatches?: number}): T[][] { + const batches: T[][] = []; + const maxBatches = opts.maxBatches ?? Math.ceil(items.length / opts.batchSize); + + for (let i = 0; i < maxBatches; i++) { + const batch = items.slice(opts.batchSize * i, opts.batchSize * (i + 1)); + if (batch.length === 0) break; + batches.push(batch); + } + + return batches; +} diff --git a/packages/validator/test/unit/services/indices.test.ts b/packages/validator/test/unit/services/indices.test.ts deleted file mode 100644 index be8346bea22d..000000000000 --- a/packages/validator/test/unit/services/indices.test.ts +++ /dev/null @@ -1,50 +0,0 @@ -import {expect} from "chai"; -import {pubkeysToBatches} from "../../../src/services/indices"; - -describe("pubkeysToBatches", function () { - const testCases: {pubkeys: string[]; expected: string[][][]}[] = [ - {pubkeys: [], expected: [[[]]]}, - {pubkeys: ["1"], expected: [[["1"]]]}, - {pubkeys: ["1", "2"], expected: [[["1", "2"]]]}, - {pubkeys: ["1", "2", "3"], expected: [[["1", "2"], ["3"]]]}, - { - pubkeys: ["1", "2", "3", "4"], - expected: [ - [ - ["1", "2"], - ["3", "4"], - ], - ], - }, - {pubkeys: ["1", "2", "3", "4", "5"], expected: [[["1", "2"], ["3", "4"], ["5"]]]}, - { - pubkeys: ["1", "2", "3", "4", "5", "6"], - expected: [ - [ - ["1", "2"], - ["3", "4"], - ["5", "6"], - ], - ], - }, - // new batch - { - pubkeys: ["1", "2", "3", "4", "5", "6", "7"], - expected: [ - [ - ["1", "2"], - ["3", "4"], - ["5", "6"], - ], - [["7"]], - ], - }, - ]; - const maxPubkeysPerRequest = 2; - const maxRequestsPerBatch = 3; - for (const {pubkeys, expected} of testCases) { - it(`should work with ${pubkeys.length} pubkeys`, () => { - expect(pubkeysToBatches(pubkeys, maxPubkeysPerRequest, maxRequestsPerBatch)).to.be.deep.equals(expected); - }); - } -}); diff --git a/packages/validator/test/unit/utils/batch.test.ts b/packages/validator/test/unit/utils/batch.test.ts new file mode 100644 index 000000000000..71e51dc6e82a --- /dev/null +++ b/packages/validator/test/unit/utils/batch.test.ts @@ -0,0 +1,42 @@ +import {expect} from "chai"; +import {batchItems} from "../../../src/util/batch"; + +describe("util / batch", function () { + const testCases: {items: string[]; expected: string[][]}[] = [ + {items: [], expected: []}, + {items: ["1"], expected: [["1"]]}, + {items: ["1", "2"], expected: [["1", "2"]]}, + {items: ["1", "2", "3"], expected: [["1", "2"], ["3"]]}, + { + items: ["1", "2", "3", "4"], + expected: [ + ["1", "2"], + ["3", "4"], + ], + }, + {items: ["1", "2", "3", "4", "5"], expected: [["1", "2"], ["3", "4"], ["5"]]}, + { + items: ["1", "2", "3", "4", "5", "6"], + expected: [ + ["1", "2"], + ["3", "4"], + ["5", "6"], + ], + }, + // Ignore item 7 since it's over the max + { + items: ["1", "2", "3", "4", "5", "6", "7"], + expected: [ + ["1", "2"], + ["3", "4"], + ["5", "6"], + ], + }, + ]; + + for (const {items: pubkeys, expected} of testCases) { + it(`Batch ${pubkeys.length} items`, () => { + expect(batchItems(pubkeys, {batchSize: 2, maxBatches: 3})).to.deep.equal(expected); + }); + } +});