From 4671eabc26cfd361dbdb8a7f73461fff5d3dfb71 Mon Sep 17 00:00:00 2001 From: shuse2 Date: Thu, 19 Oct 2023 15:54:27 +0200 Subject: [PATCH] :nail_care: Remove unnecessary exceptions --- framework/src/engine/bft/method.ts | 11 +-- framework/src/engine/bft/module.ts | 8 +-- framework/src/engine/consensus/consensus.ts | 8 +-- framework/src/engine/generator/generator.ts | 1 - .../src/schema/application_config_schema.ts | 14 ---- framework/src/testing/fixtures/config.ts | 3 - framework/src/types.ts | 1 - .../__snapshots__/application.spec.ts.snap | 3 - .../unit/engine/bft/bft_processing.spec.ts | 7 +- framework/test/unit/engine/bft/method.spec.ts | 67 +++---------------- framework/test/unit/engine/bft/module.spec.ts | 2 +- .../unit/engine/consensus/consensus.spec.ts | 3 - .../application_config_schema.spec.ts.snap | 16 ----- 13 files changed, 14 insertions(+), 130 deletions(-) diff --git a/framework/src/engine/bft/method.ts b/framework/src/engine/bft/method.ts index e4cb960c60f..e91203210ce 100644 --- a/framework/src/engine/bft/method.ts +++ b/framework/src/engine/bft/method.ts @@ -49,16 +49,14 @@ export interface BlockHeaderAsset { export class BFTMethod { private _batchSize!: number; private _blockTime!: number; - private _shuffleValidatorsFromHeight!: number; public blockTime(): number { return this._blockTime; } - public init(batchSize: number, blockTime: number, shuffleValidatorsFromHeight: number) { + public init(batchSize: number, blockTime: number) { this._batchSize = batchSize; this._blockTime = blockTime; - this._shuffleValidatorsFromHeight = shuffleValidatorsFromHeight; } public areHeadersContradicting(bftHeader1: BlockHeader, bftHeader2: BlockHeader): boolean { @@ -175,7 +173,6 @@ export class BFTMethod { precommitThreshold: bigint, certificateThreshold: bigint, validators: Validator[], - height: number, ): Promise { if (validators.length > this._batchSize) { throw new Error( @@ -230,12 +227,6 @@ export class BFTMethod { const validatorsHash = computeValidatorsHash(validatorsWithBFTWeight, certificateThreshold); - // Ensure that validator list is not shuffled before the configured block height, - // to be able to sync with the new version - if (height < this._shuffleValidatorsFromHeight) { - sortValidatorsByBLSKey(validators); - } - const bftParams: BFTParameters = { prevoteThreshold: (BigInt(2) * aggregateBFTWeight) / BigInt(3) + BigInt(1), precommitThreshold, diff --git a/framework/src/engine/bft/module.ts b/framework/src/engine/bft/module.ts index 32c2b50f0ef..756c22b8bf2 100644 --- a/framework/src/engine/bft/module.ts +++ b/framework/src/engine/bft/module.ts @@ -38,13 +38,9 @@ export class BFTModule { private _maxLengthBlockBFTInfos!: number; // eslint-disable-next-line @typescript-eslint/require-await - public async init( - batchSize: number, - blockTime: number, - shuffleValidatorsFromHeight: number, - ): Promise { + public async init(batchSize: number, blockTime: number): Promise { this._batchSize = batchSize; - this.method.init(this._batchSize, blockTime, shuffleValidatorsFromHeight); + this.method.init(this._batchSize, blockTime); this._maxLengthBlockBFTInfos = 3 * this._batchSize; } diff --git a/framework/src/engine/consensus/consensus.ts b/framework/src/engine/consensus/consensus.ts index 5b863bcccfd..6030b233ed8 100644 --- a/framework/src/engine/consensus/consensus.ts +++ b/framework/src/engine/consensus/consensus.ts @@ -183,11 +183,7 @@ export class Consensus { blockExecutor, mechanisms: [blockSyncMechanism, fastChainSwitchMechanism], }); - await this._bft.init( - this._genesisConfig.bftBatchSize, - this._genesisConfig.blockTime, - this._genesisConfig.exceptions.shuffleValidatorsFromHeight, - ); + await this._bft.init(this._genesisConfig.bftBatchSize, this._genesisConfig.blockTime); this._network.registerEndpoint(NETWORK_LEGACY_GET_BLOCKS_FROM_ID, async ({ data, peerId }) => this._legacyEndpoint.handleRPCGetLegacyBlocksFromID(data, peerId), @@ -1032,7 +1028,6 @@ export class Consensus { afterResult.preCommitThreshold, afterResult.certificateThreshold, afterResult.nextValidators, - block.header.height, ); this.events.emit(CONSENSUS_EVENT_VALIDATORS_CHANGED, { preCommitThreshold: afterResult.preCommitThreshold, @@ -1086,7 +1081,6 @@ export class Consensus { result.preCommitThreshold, result.certificateThreshold, result.nextValidators, - genesisBlock.header.height, ); this.events.emit(CONSENSUS_EVENT_VALIDATORS_CHANGED, { preCommitThreshold: result.preCommitThreshold, diff --git a/framework/src/engine/generator/generator.ts b/framework/src/engine/generator/generator.ts index 450ac3995b0..931971a16af 100644 --- a/framework/src/engine/generator/generator.ts +++ b/framework/src/engine/generator/generator.ts @@ -596,7 +596,6 @@ export class Generator { afterResult.preCommitThreshold, afterResult.certificateThreshold, afterResult.nextValidators, - height, ); } diff --git a/framework/src/schema/application_config_schema.ts b/framework/src/schema/application_config_schema.ts index b4ffa4b075b..f55061f6ebd 100644 --- a/framework/src/schema/application_config_schema.ts +++ b/framework/src/schema/application_config_schema.ts @@ -279,17 +279,6 @@ export const applicationConfigSchema = { minimum: 1, description: 'Minimum block height which can be certified', }, - exceptions: { - type: 'object', - required: ['shuffleValidatorsFromHeight'], - properties: { - shuffleValidatorsFromHeight: { - type: 'integer', - minimum: 0, - description: 'Block height from which the validator list will be shuffled', - }, - }, - }, }, additionalProperties: false, }, @@ -362,9 +351,6 @@ export const applicationConfigSchema = { bftBatchSize: BFT_BATCH_SIZE, maxTransactionsSize: MAX_TRANSACTIONS_SIZE, minimumCertifyHeight: 1, - exceptions: { - shuffleValidatorsFromHeight: 0, - }, }, generator: { keys: {}, diff --git a/framework/src/testing/fixtures/config.ts b/framework/src/testing/fixtures/config.ts index f47e44a41eb..928271cba34 100644 --- a/framework/src/testing/fixtures/config.ts +++ b/framework/src/testing/fixtures/config.ts @@ -34,9 +34,6 @@ export const defaultConfig: ApplicationConfig = { blockTime: 10, chainID: '10000000', maxTransactionsSize: 15 * 1024, // Kilo Bytes - exceptions: { - shuffleValidatorsFromHeight: 0, - }, }, network: { version: '1.0', diff --git a/framework/src/types.ts b/framework/src/types.ts index 48f84cb2ff9..7a90042fe67 100644 --- a/framework/src/types.ts +++ b/framework/src/types.ts @@ -65,7 +65,6 @@ export interface GenesisConfig { blockTime: number; bftBatchSize: number; minimumCertifyHeight: number; - exceptions: { shuffleValidatorsFromHeight: number }; } export interface TransactionPoolConfig { diff --git a/framework/test/unit/__snapshots__/application.spec.ts.snap b/framework/test/unit/__snapshots__/application.spec.ts.snap index 41903f4a6a5..b1396361a55 100644 --- a/framework/test/unit/__snapshots__/application.spec.ts.snap +++ b/framework/test/unit/__snapshots__/application.spec.ts.snap @@ -12,9 +12,6 @@ exports[`Application #constructor should set internal variables 1`] = ` }, "blockTime": 10, "chainID": "10000000", - "exceptions": { - "shuffleValidatorsFromHeight": 0, - }, "maxTransactionsSize": 15360, "minimumCertifyHeight": 1, }, diff --git a/framework/test/unit/engine/bft/bft_processing.spec.ts b/framework/test/unit/engine/bft/bft_processing.spec.ts index 8934a853765..9c84437b61d 100644 --- a/framework/test/unit/engine/bft/bft_processing.spec.ts +++ b/framework/test/unit/engine/bft/bft_processing.spec.ts @@ -39,7 +39,6 @@ describe('BFT processing', () => { scenario11ValidatorsPartialSwitch, ]; const blockTime = 10; - const shuffleValidatorsFromHeight = 0; for (const scenario of bftScenarios) { // eslint-disable-next-line no-loop-func @@ -50,11 +49,7 @@ describe('BFT processing', () => { beforeAll(async () => { bftModule = new BFTModule(); - await bftModule.init( - scenario.config.activeValidators, - blockTime, - shuffleValidatorsFromHeight, - ); + await bftModule.init(scenario.config.activeValidators, blockTime); db = new InMemoryDatabase(); stateStore = new StateStore(db); diff --git a/framework/test/unit/engine/bft/method.spec.ts b/framework/test/unit/engine/bft/method.spec.ts index 3ab01467b42..2a947325618 100644 --- a/framework/test/unit/engine/bft/method.spec.ts +++ b/framework/test/unit/engine/bft/method.spec.ts @@ -34,7 +34,6 @@ import { } from '../../../../src/engine/bft/schemas'; import { createFakeBlockHeader } from '../../../../src/testing'; import { computeValidatorsHash } from '../../../../src/engine'; -import { sortValidatorsByBLSKey } from '../../../../src/engine/bft/utils'; describe('BFT Method', () => { let bftMethod: BFTMethod; @@ -44,7 +43,7 @@ describe('BFT Method', () => { beforeEach(() => { bftMethod = new BFTMethod(); validatorsMethod = { getValidatorKeys: jest.fn() }; - bftMethod.init(103, 10, 0); + bftMethod.init(103, 10); }); describe('areHeadersContradicting', () => { @@ -708,7 +707,6 @@ describe('BFT Method', () => { const generatorAddress = utils.getRandomBytes(20); const params20 = createParam(); const params30 = createParam(); - const height = 8; const validators = [ { @@ -805,7 +803,6 @@ describe('BFT Method', () => { blsKey: utils.getRandomBytes(48), generatorKey: utils.getRandomBytes(32), })), - height, ), ).rejects.toThrow('Invalid validators size.'); }); @@ -820,13 +817,7 @@ describe('BFT Method', () => { validatorsAddressNotUnique[8].address = validatorsAddressNotUnique[12].address; await expect( - bftMethod.setBFTParameters( - stateStore, - BigInt(68), - BigInt(68), - validatorsAddressNotUnique, - height, - ), + bftMethod.setBFTParameters(stateStore, BigInt(68), BigInt(68), validatorsAddressNotUnique), ).rejects.toThrow('Provided validator addresses are not unique.'); }); @@ -840,13 +831,7 @@ describe('BFT Method', () => { validatorsBLSKeysNotUnique[13].blsKey = validatorsBLSKeysNotUnique[7].blsKey; await expect( - bftMethod.setBFTParameters( - stateStore, - BigInt(68), - BigInt(68), - validatorsBLSKeysNotUnique, - height, - ), + bftMethod.setBFTParameters(stateStore, BigInt(68), BigInt(68), validatorsBLSKeysNotUnique), ).rejects.toThrow('Provided validator BLS keys are not unique.'); }); @@ -861,13 +846,7 @@ describe('BFT Method', () => { validatorsInvalidBLSKeys[13].blsKey = Buffer.alloc(48, 0); await expect( - bftMethod.setBFTParameters( - stateStore, - BigInt(68), - BigInt(68), - validatorsInvalidBLSKeys, - height, - ), + bftMethod.setBFTParameters(stateStore, BigInt(68), BigInt(68), validatorsInvalidBLSKeys), ).not.toReject(); }); @@ -883,32 +862,31 @@ describe('BFT Method', () => { blsKey: utils.getRandomBytes(48), generatorKey: utils.getRandomBytes(32), })), - height, ), ).rejects.toThrow('BFT Weight must be 0 or greater.'); }); it('should throw when less than 1/3 of aggregateBFTWeight for precommitThreshold is given', async () => { await expect( - bftMethod.setBFTParameters(stateStore, BigInt(34), BigInt(68), validators, height), + bftMethod.setBFTParameters(stateStore, BigInt(34), BigInt(68), validators), ).rejects.toThrow('Invalid precommitThreshold input.'); }); it('should throw when precommitThreshold is given is greater than aggregateBFTWeight', async () => { await expect( - bftMethod.setBFTParameters(stateStore, BigInt(104), BigInt(68), validators, height), + bftMethod.setBFTParameters(stateStore, BigInt(104), BigInt(68), validators), ).rejects.toThrow('Invalid precommitThreshold input.'); }); it('should throw when less than 1/3 of aggregateBFTWeight for certificateThreshold is given', async () => { await expect( - bftMethod.setBFTParameters(stateStore, BigInt(68), BigInt(34), validators, height), + bftMethod.setBFTParameters(stateStore, BigInt(68), BigInt(34), validators), ).rejects.toThrow('Invalid certificateThreshold input.'); }); it('should throw when certificateThreshold is given is greater than aggregateBFTWeight', async () => { await expect( - bftMethod.setBFTParameters(stateStore, BigInt(68), BigInt(104), validators, height), + bftMethod.setBFTParameters(stateStore, BigInt(68), BigInt(104), validators), ).rejects.toThrow('Invalid certificateThreshold input.'); }); @@ -932,7 +910,6 @@ describe('BFT Method', () => { precommitThreshold, certificateThreshold, validators, - height, ); const bftParams = await bftParamsStore.getWithSchema( @@ -944,29 +921,6 @@ describe('BFT Method', () => { expect(bftParams.validators).toEqual(shuffledValidators); }); - it('should store validators in order of BLS keys if block height is earlier than the configured exception', async () => { - const sortedValidators = [...validators]; - sortValidatorsByBLSKey(sortedValidators); - - bftMethod.init(103, 10, 88); - - await bftMethod.setBFTParameters( - stateStore, - precommitThreshold, - certificateThreshold, - validators, - height, - ); - - const bftParams = await bftParamsStore.getWithSchema( - utils.intToBuffer(104, 4), - bftParametersSchema, - ); - - expect(bftParams.validators).toHaveLength(3); - expect(bftParams.validators).toEqual(sortedValidators); - }); - it('should store BFT parameters with height maxHeightPrevoted + 1 if blockBFTInfo does not exist', async () => { await votesStore.setWithSchema( EMPTY_KEY, @@ -991,7 +945,6 @@ describe('BFT Method', () => { precommitThreshold, certificateThreshold, validators, - height, ); await expect( @@ -1008,7 +961,6 @@ describe('BFT Method', () => { precommitThreshold, certificateThreshold, validators, - height, ); await expect( @@ -1025,7 +977,6 @@ describe('BFT Method', () => { precommitThreshold, certificateThreshold, validators, - height, ); const bftParams = await bftParamsStore.getWithSchema( @@ -1041,7 +992,6 @@ describe('BFT Method', () => { precommitThreshold, certificateThreshold, validators, - height, ); const voteState = await votesStore.getWithSchema(EMPTY_KEY, bftVotesSchema); @@ -1060,7 +1010,6 @@ describe('BFT Method', () => { precommitThreshold, certificateThreshold, validators, - height, ); const voteState = await votesStore.getWithSchema(EMPTY_KEY, bftVotesSchema); diff --git a/framework/test/unit/engine/bft/module.spec.ts b/framework/test/unit/engine/bft/module.spec.ts index b011c34ff08..8621f0204f8 100644 --- a/framework/test/unit/engine/bft/module.spec.ts +++ b/framework/test/unit/engine/bft/module.spec.ts @@ -32,7 +32,7 @@ describe('bft module', () => { describe('init', () => { it('should initialize config with given value', async () => { - await expect(bftModule.init(20, 10, 0)).toResolve(); + await expect(bftModule.init(20, 10)).toResolve(); expect(bftModule['_batchSize']).toBe(20); }); diff --git a/framework/test/unit/engine/consensus/consensus.spec.ts b/framework/test/unit/engine/consensus/consensus.spec.ts index e97de35b269..19adf9d4d19 100644 --- a/framework/test/unit/engine/consensus/consensus.spec.ts +++ b/framework/test/unit/engine/consensus/consensus.spec.ts @@ -127,9 +127,6 @@ describe('consensus', () => { bft, genesisConfig: { blockTime: 10, - exceptions: { - shuffleValidatorsFromHeight: 0, - }, } as any, }); dbMock = { diff --git a/framework/test/unit/schema/__snapshots__/application_config_schema.spec.ts.snap b/framework/test/unit/schema/__snapshots__/application_config_schema.spec.ts.snap index 1b4aae257e2..e52e162ac40 100644 --- a/framework/test/unit/schema/__snapshots__/application_config_schema.spec.ts.snap +++ b/framework/test/unit/schema/__snapshots__/application_config_schema.spec.ts.snap @@ -14,9 +14,6 @@ exports[`schema/application_config_schema.js application config schema must matc "fromFile": "./config/genesis_block.blob", }, "blockTime": 10, - "exceptions": { - "shuffleValidatorsFromHeight": 0, - }, "maxTransactionsSize": 15360, "minimumCertifyHeight": 1, }, @@ -128,19 +125,6 @@ exports[`schema/application_config_schema.js application config schema must matc "format": "hex", "type": "string", }, - "exceptions": { - "properties": { - "shuffleValidatorsFromHeight": { - "description": "Block height from which the validator list will be shuffled", - "minimum": 0, - "type": "integer", - }, - }, - "required": [ - "shuffleValidatorsFromHeight", - ], - "type": "object", - }, "maxTransactionsSize": { "description": "Maximum number of transactions allowed per block", "maximum": 30720,