From 6cbb5b96ff0d852f84edcf9b7bfc7e030efe508f Mon Sep 17 00:00:00 2001 From: Franco NG Date: Thu, 14 Sep 2023 12:19:26 +0200 Subject: [PATCH 1/2] Code improvements and remove shuffleValidatorList from PoA --- .../poa/commands/update_generator_key.ts | 10 ---- framework/src/modules/poa/module.ts | 45 ++++++++--------- framework/src/modules/poa/types.ts | 8 +-- framework/src/modules/poa/utils.ts | 43 ---------------- framework/src/modules/pos/module.ts | 2 +- framework/src/modules/pos/utils.ts | 13 ++--- framework/src/modules/validators/method.ts | 2 +- .../test/unit/modules/poa/module.spec.ts | 5 +- framework/test/unit/modules/poa/utils.spec.ts | 50 ------------------- 9 files changed, 34 insertions(+), 144 deletions(-) delete mode 100644 framework/src/modules/poa/utils.ts delete mode 100644 framework/test/unit/modules/poa/utils.spec.ts diff --git a/framework/src/modules/poa/commands/update_generator_key.ts b/framework/src/modules/poa/commands/update_generator_key.ts index d3f7f4f994d..4a94d28b776 100644 --- a/framework/src/modules/poa/commands/update_generator_key.ts +++ b/framework/src/modules/poa/commands/update_generator_key.ts @@ -12,7 +12,6 @@ * Removal or modification of this copyright notice is prohibited. */ -import { validator } from '@liskhq/lisk-validator'; import { BaseCommand } from '../../base_command'; import { updateGeneratorKeySchema } from '../schemas'; import { COMMAND_UPDATE_KEY } from '../constants'; @@ -41,15 +40,6 @@ export class UpdateGeneratorKeyCommand extends BaseCommand { public async verify( context: CommandVerifyContext, ): Promise { - try { - validator.validate(updateGeneratorKeySchema, context.params); - } catch (err) { - return { - status: VerifyStatus.FAIL, - error: err as Error, - }; - } - const validatorExists = await this.stores .get(ValidatorStore) .has(context, context.transaction.senderAddress); diff --git a/framework/src/modules/poa/module.ts b/framework/src/modules/poa/module.ts index 02049d574d4..6c29f886611 100644 --- a/framework/src/modules/poa/module.ts +++ b/framework/src/modules/poa/module.ts @@ -19,7 +19,13 @@ import { BaseModule, ModuleInitArgs, ModuleMetadata } from '../base_module'; import { PoAMethod } from './method'; import { PoAEndpoint } from './endpoint'; import { AuthorityUpdateEvent } from './events/authority_update'; -import { ChainPropertiesStore, ValidatorStore, NameStore, SnapshotStore } from './stores'; +import { + ChainPropertiesStore, + ValidatorStore, + NameStore, + SnapshotStore, + Validator, +} from './stores'; import { BlockAfterExecuteContext, GenesisBlockExecuteContext } from '../../state_machine'; import { MODULE_NAME_POA, @@ -30,7 +36,7 @@ import { MAX_UINT64, defaultConfig, } from './constants'; -import { shuffleValidatorList } from './utils'; +import { shuffleValidatorList } from '../pos/utils'; import { NextValidatorsSetter, MethodContext } from '../../state_machine/types'; import { configSchema, @@ -165,7 +171,7 @@ export class PoAModule extends BaseModule { previousLengthValidators, ); - const nextValidators = shuffleValidatorList(randomSeed, snapshot1.validators); + const nextValidators = shuffleValidatorList(randomSeed, snapshot1.validators); await this._validatorsMethod.setValidatorsParams( context as MethodContext, @@ -206,23 +212,18 @@ export class PoAModule extends BaseModule { throw new Error('`address` property of all entries in validators must be distinct.'); } - const sortedValidatorsByAddress = [...validatorAddresses].sort((a, b) => a.compare(b)); - for (let i = 0; i < validators.length; i += 1) { - // Check that entries in the validators array are ordered lexicographically according to address. - if (!validatorAddresses[i].equals(sortedValidatorsByAddress[i])) { - throw new Error('`validators` must be ordered lexicographically by address.'); - } + if (!objects.isBufferArrayOrdered(validatorAddresses)) { + throw new Error('`validators` must be ordered lexicographically by address.'); + } - if (!/^[a-z0-9!@$&_.]+$/g.test(validators[i].name)) { + for (const poaValidator of validators) { + if (!/^[a-z0-9!@$&_.]+$/g.test(poaValidator.name)) { throw new Error('`name` property is invalid. Must contain only characters a-z0-9!@$&_.'); } } const { activeValidators, threshold } = snapshotSubstore; const activeValidatorAddresses = activeValidators.map(v => v.address); - const sortedActiveValidatorsByAddress = [...activeValidatorAddresses].sort((a, b) => - a.compare(b), - ); const validatorAddressesString = validatorAddresses.map(a => a.toString('hex')); let totalWeight = BigInt(0); @@ -231,25 +232,21 @@ export class PoAModule extends BaseModule { throw new Error('`address` properties in `activeValidators` must be distinct.'); } - for (let i = 0; i < activeValidators.length; i += 1) { - // Check that entries in the snapshotSubstore.activeValidators array are ordered lexicographically according to address. - if (!activeValidators[i].address.equals(sortedActiveValidatorsByAddress[i])) { - throw new Error( - '`activeValidators` must be ordered lexicographically by address property.', - ); - } - + if (!objects.isBufferArrayOrdered(activeValidatorAddresses)) { + throw new Error('`activeValidators` must be ordered lexicographically by address property.'); + } + for (const activeValidator of activeValidators) { // Check that for every element activeValidator in the snapshotSubstore.activeValidators array, there is an entry validator in the validators array with validator.address == activeValidator.address. - if (!validatorAddressesString.includes(activeValidators[i].address.toString('hex'))) { + if (!validatorAddressesString.includes(activeValidator.address.toString('hex'))) { throw new Error('`activeValidator` address is missing from validators array.'); } // Check that the weight property of every entry in the snapshotSubstore.activeValidators array is a positive integer. - if (activeValidators[i].weight <= BigInt(0)) { + if (activeValidator.weight <= BigInt(0)) { throw new Error('`activeValidators` weight must be positive integer.'); } - totalWeight += activeValidators[i].weight; + totalWeight += activeValidator.weight; } if (totalWeight > MAX_UINT64) { diff --git a/framework/src/modules/poa/types.ts b/framework/src/modules/poa/types.ts index 134ca1c4567..439d7538b4c 100644 --- a/framework/src/modules/poa/types.ts +++ b/framework/src/modules/poa/types.ts @@ -27,8 +27,8 @@ export type ModuleConfigJSON = JSONObject; export interface RegisterAuthorityParams { name: string; blsKey: Buffer; - generatorKey: Buffer; proofOfPossession: Buffer; + generatorKey: Buffer; } export interface UpdateAuthorityParams { @@ -42,12 +42,6 @@ export interface UpdateAuthorityParams { aggregationBits: Buffer; } -export interface ValidatorWeightWithRoundHash { - readonly address: Buffer; - weight: bigint; - roundHash: Buffer; -} - export interface ValidatorsMethod { setValidatorGeneratorKey( methodContext: MethodContext, diff --git a/framework/src/modules/poa/utils.ts b/framework/src/modules/poa/utils.ts deleted file mode 100644 index dc7fdfd8b01..00000000000 --- a/framework/src/modules/poa/utils.ts +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright © 2023 Lisk Foundation - * - * See the LICENSE file at the top-level directory of this distribution - * for licensing information. - * - * Unless otherwise agreed in a custom licensing agreement with the Lisk Foundation, - * no part of this software, including this file, may be copied, modified, - * propagated, or distributed except according to the terms contained in the - * LICENSE file. - * - * Removal or modification of this copyright notice is prohibited. - */ - -import { utils } from '@liskhq/lisk-cryptography'; -import { ValidatorWeightWithRoundHash } from './types'; -import { Validator } from './stores'; - -// Same as pos/utils/shuffleValidatorList -export const shuffleValidatorList = ( - roundSeed: Buffer, - validators: Validator[], -): ValidatorWeightWithRoundHash[] => { - const validatorsWithRoundHash: ValidatorWeightWithRoundHash[] = []; - for (const validator of validators) { - const seedSource = Buffer.concat([roundSeed, validator.address]); - validatorsWithRoundHash.push({ - ...validator, - roundHash: utils.hash(seedSource), - }); - } - - validatorsWithRoundHash.sort((validator1, validator2) => { - const diff = validator1.roundHash.compare(validator2.roundHash); - if (diff !== 0) { - return diff; - } - - return validator1.address.compare(validator2.address); - }); - - return validatorsWithRoundHash; -}; diff --git a/framework/src/modules/pos/module.ts b/framework/src/modules/pos/module.ts index 06520f17aa1..b1b9eedf193 100644 --- a/framework/src/modules/pos/module.ts +++ b/framework/src/modules/pos/module.ts @@ -690,7 +690,7 @@ export class PoSModule extends BaseModule { } // Update the validators - const shuffledValidators = shuffleValidatorList(randomSeed1, validators); + const shuffledValidators = shuffleValidatorList(randomSeed1, validators); let aggregateBFTWeight = BigInt(0); const bftValidators: { address: Buffer; bftWeight: bigint }[] = []; for (const v of shuffledValidators) { diff --git a/framework/src/modules/pos/utils.ts b/framework/src/modules/pos/utils.ts index c91b08b57ec..a2d61af78ec 100644 --- a/framework/src/modules/pos/utils.ts +++ b/framework/src/modules/pos/utils.ts @@ -99,16 +99,17 @@ export const pickStandByValidator = ( return -1; }; -export const shuffleValidatorList = ( - previousRoundSeed1: Buffer, - addresses: ValidatorWeight[], -): ValidatorWeight[] => { +export const shuffleValidatorList = ( + roundSeed: Buffer, + addresses: T[], +): (T & { roundHash: Buffer })[] => { const validatorList = [...addresses].map(validator => ({ ...validator, - })) as { address: Buffer; roundHash: Buffer; weight: bigint }[]; + roundHash: Buffer.from([]), + })) as (T & { roundHash: Buffer })[]; for (const validator of validatorList) { - const seedSource = Buffer.concat([previousRoundSeed1, validator.address]); + const seedSource = Buffer.concat([roundSeed, validator.address]); validator.roundHash = utils.hash(seedSource); } diff --git a/framework/src/modules/validators/method.ts b/framework/src/modules/validators/method.ts index 0fa0cc242c0..24a33a1e05c 100644 --- a/framework/src/modules/validators/method.ts +++ b/framework/src/modules/validators/method.ts @@ -363,7 +363,7 @@ export class ValidatorsMethod extends BaseMethod { throw new Error(`BLS public key must be ${BLS_PUBLIC_KEY_LENGTH} bytes long.`); } if (args.proofOfPossession && args.proofOfPossession.length !== BLS_POP_LENGTH) { - throw new Error(`Proof of possesion must be ${BLS_POP_LENGTH} bytes long.`); + throw new Error(`Proof of Possession must be ${BLS_POP_LENGTH} bytes long.`); } if (args.generatorKey && args.generatorKey.length !== ED25519_PUBLIC_KEY_LENGTH) { throw new Error(`Generator key must be ${ED25519_PUBLIC_KEY_LENGTH} bytes long.`); diff --git a/framework/test/unit/modules/poa/module.spec.ts b/framework/test/unit/modules/poa/module.spec.ts index fd7ed1c6928..f9f06f27ce1 100644 --- a/framework/test/unit/modules/poa/module.spec.ts +++ b/framework/test/unit/modules/poa/module.spec.ts @@ -54,8 +54,9 @@ import { ChainPropertiesStore, SnapshotObject, ChainProperties, + Validator, } from '../../../../src/modules/poa/stores'; -import { shuffleValidatorList } from '../../../../src/modules/poa/utils'; +import { shuffleValidatorList } from '../../../../src/modules/pos/utils'; describe('PoA module', () => { let poaModule: PoAModule; @@ -306,7 +307,7 @@ describe('PoA module', () => { for (const validator of snapshot1.validators) { validators.push(validator); } - const nextValidators = shuffleValidatorList(randomSeed, validators); + const nextValidators = shuffleValidatorList(randomSeed, validators); await poaModule.afterTransactionsExecute(context); expect(poaModule.stores.get(SnapshotStore).set).toHaveBeenCalledWith( context, diff --git a/framework/test/unit/modules/poa/utils.spec.ts b/framework/test/unit/modules/poa/utils.spec.ts deleted file mode 100644 index 64ba1cd35ad..00000000000 --- a/framework/test/unit/modules/poa/utils.spec.ts +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright © 2023 Lisk Foundation - * - * See the LICENSE file at the top-level directory of this distribution - * for licensing information. - * - * Unless otherwise agreed in a custom licensing agreement with the Lisk Foundation, - * no part of this software, including this file, may be copied, modified, - * propagated, or distributed except according to the terms contained in the - * LICENSE file. - * - * Removal or modification of this copyright notice is prohibited. - */ - -import { address as cryptoAddress } from '@liskhq/lisk-cryptography'; -import { shuffleValidatorList } from '../../../../src/modules/poa/utils'; -import * as validatorShufflingScenario from '../../../fixtures/pos_validator_shuffling/uniformly_shuffled_validator_list.json'; - -// Same as pos/utils/shuffleValidatorList -describe('utils', () => { - describe('shuffleValidatorList', () => { - const { previousRoundSeed1 } = validatorShufflingScenario.testCases.input; - const validatorsList = [...validatorShufflingScenario.testCases.input.validatorList].map( - address => ({ - address: Buffer.from(address, 'hex'), - weight: BigInt(1), - }), - ); - it('should return a list of uniformly shuffled list of validators', () => { - const shuffledValidatorList = shuffleValidatorList( - Buffer.from(previousRoundSeed1, 'hex'), - validatorsList, - ); - const lisk32Addresses = validatorsList.map(a => - cryptoAddress.getLisk32AddressFromAddress(a.address), - ); - - expect(shuffledValidatorList).toHaveLength(validatorsList.length); - shuffledValidatorList.forEach(validator => - expect(lisk32Addresses).toContain( - cryptoAddress.getLisk32AddressFromAddress(validator.address), - ), - ); - - expect(shuffledValidatorList.map(b => b.address.toString('hex'))).toEqual( - validatorShufflingScenario.testCases.output.validatorList, - ); - }); - }); -}); From cbf4a6e81fc6599675de059a62b5b72caa18159d Mon Sep 17 00:00:00 2001 From: Franco NG Date: Thu, 14 Sep 2023 12:50:52 +0200 Subject: [PATCH 2/2] Revert changes that has already been resolved in #8967 at update_generator_key --- .../src/modules/poa/commands/update_generator_key.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/framework/src/modules/poa/commands/update_generator_key.ts b/framework/src/modules/poa/commands/update_generator_key.ts index 4a94d28b776..d3f7f4f994d 100644 --- a/framework/src/modules/poa/commands/update_generator_key.ts +++ b/framework/src/modules/poa/commands/update_generator_key.ts @@ -12,6 +12,7 @@ * Removal or modification of this copyright notice is prohibited. */ +import { validator } from '@liskhq/lisk-validator'; import { BaseCommand } from '../../base_command'; import { updateGeneratorKeySchema } from '../schemas'; import { COMMAND_UPDATE_KEY } from '../constants'; @@ -40,6 +41,15 @@ export class UpdateGeneratorKeyCommand extends BaseCommand { public async verify( context: CommandVerifyContext, ): Promise { + try { + validator.validate(updateGeneratorKeySchema, context.params); + } catch (err) { + return { + status: VerifyStatus.FAIL, + error: err as Error, + }; + } + const validatorExists = await this.stores .get(ValidatorStore) .has(context, context.transaction.senderAddress);