PoA Improvements #2 (#8991)

* Code improvements and remove shuffleValidatorList from PoA

* Revert changes that has already been resolved in #8967 at update_generator_key

---------

Co-authored-by: !shan <ishantiw.quasar@gmail.com>

framework/src/modules/poa/module.ts

@@ -35,7 +35,7 @@ import {
 	SUBSTORE_PREFIX_NAME_INDEX,
 	SUBSTORE_PREFIX_SNAPSHOT_INDEX,
 } from './constants';
-import { shuffleValidatorList } from './utils';
+import { shuffleValidatorList } from '../utils';
 import { NextValidatorsSetter, MethodContext } from '../../state_machine/types';
 import {
 	configSchema,
@@ -52,6 +52,7 @@ import {
 	RandomMethod,
 	ModuleConfigJSON,
 	ModuleConfig,
+	ActiveValidator,
 } from './types';
 import { RegisterAuthorityCommand } from './commands/register_authority';
 import { UpdateAuthorityCommand } from './commands/update_authority';
@@ -179,7 +180,10 @@ export class PoAModule extends BaseModule {
 				previousLengthValidators,
 			);
 
-			const nextValidators = shuffleValidatorList(randomSeed, snapshot1.validators);
+			const nextValidators = shuffleValidatorList<ActiveValidator>(
+				randomSeed,
+				snapshot1.validators,
+			);
 
 			await this._validatorsMethod.setValidatorsParams(
 				context as MethodContext,
@@ -220,23 +224,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 (!POA_VALIDATOR_NAME_REGEX.test(validators[i].name)) {
+		for (const poaValidator of validators) {
+			if (!POA_VALIDATOR_NAME_REGEX.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);
 
@@ -245,25 +244,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) {

framework/src/modules/poa/types.ts

@@ -27,8 +27,8 @@ export type ModuleConfigJSON = JSONObject<ModuleConfig>;
 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,

framework/src/modules/poa/utils.ts

@@ -1,42 +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, ActiveValidator } from './types';
-
-// Same as pos/utils/shuffleValidatorList
-export const shuffleValidatorList = (
-	roundSeed: Buffer,
-	validators: ActiveValidator[],
-): 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;
-};

framework/src/modules/pos/module.ts

@@ -70,12 +70,11 @@ import {
 	equalUnlocking,
 	isUsername,
 	selectStandbyValidators,
-	shuffleValidatorList,
 	sortUnlocking,
 	getModuleConfig,
 	getValidatorWeight,
-	ValidatorWeight,
 	isSharingCoefficientSorted,
+	ValidatorWeight,
 } from './utils';
 import { ValidatorStore } from './stores/validator';
 import { GenesisDataStore } from './stores/genesis';
@@ -93,6 +92,7 @@ import { CommissionChangeEvent } from './events/commission_change';
 import { ClaimRewardsCommand } from './commands/claim_rewards';
 import { getMainchainID } from '../interoperability/utils';
 import { RewardsAssignedEvent } from './events/rewards_assigned';
+import { shuffleValidatorList } from '../utils';
 
 export class PoSModule extends BaseModule {
 	public method = new PoSMethod(this.stores, this.events);
@@ -690,7 +690,7 @@ export class PoSModule extends BaseModule {
 		}
 
 		// Update the validators
-		const shuffledValidators = shuffleValidatorList(randomSeed1, validators);
+		const shuffledValidators = shuffleValidatorList<ValidatorWeight>(randomSeed1, validators);
 		let aggregateBFTWeight = BigInt(0);
 		const bftValidators: { address: Buffer; bftWeight: bigint }[] = [];
 		for (const v of shuffledValidators) {

framework/src/modules/pos/utils.ts

@@ -12,7 +12,7 @@
  * Removal or modification of this copyright notice is prohibited.
  */
 
-import { utils, ed } from '@liskhq/lisk-cryptography';
+import { ed } from '@liskhq/lisk-cryptography';
 import { math } from '@liskhq/lisk-utils';
 import {
 	ModuleConfig,
@@ -99,31 +99,6 @@ export const pickStandByValidator = (
 	return -1;
 };
 
-export const shuffleValidatorList = (
-	previousRoundSeed1: Buffer,
-	addresses: ValidatorWeight[],
-): ValidatorWeight[] => {
-	const validatorList = [...addresses].map(validator => ({
-		...validator,
-	})) as { address: Buffer; roundHash: Buffer; weight: bigint }[];
-
-	for (const validator of validatorList) {
-		const seedSource = Buffer.concat([previousRoundSeed1, validator.address]);
-		validator.roundHash = utils.hash(seedSource);
-	}
-
-	validatorList.sort((validator1, validator2) => {
-		const diff = validator1.roundHash.compare(validator2.roundHash);
-		if (diff !== 0) {
-			return diff;
-		}
-
-		return validator1.address.compare(validator2.address);
-	});
-
-	return validatorList;
-};
-
 export const selectStandbyValidators = (
 	validatorWeights: ValidatorWeight[],
 	randomSeed1: Buffer,

framework/src/modules/utils/index.ts

@@ -0,0 +1,15 @@
+/*
+ * 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.
+ */
+
+export { shuffleValidatorList } from './shuffleValidatorList';

framework/src/modules/utils/shuffleValidatorList.ts

@@ -0,0 +1,46 @@
+/*
+ * 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';
+
+export const shuffleValidatorList = <
+	T extends {
+		readonly address: Buffer;
+		weight: bigint;
+	},
+>(
+	roundSeed: Buffer,
+	addresses: T[],
+): (T & { roundHash: Buffer })[] => {
+	const validatorList = [...addresses].map(validator => ({
+		...validator,
+		roundHash: Buffer.from([]),
+	})) as (T & { roundHash: Buffer })[];
+
+	for (const validator of validatorList) {
+		const seedSource = Buffer.concat([roundSeed, validator.address]);
+		validator.roundHash = utils.hash(seedSource);
+	}
+
+	validatorList.sort((validator1, validator2) => {
+		const diff = validator1.roundHash.compare(validator2.roundHash);
+		if (diff !== 0) {
+			return diff;
+		}
+
+		return validator1.address.compare(validator2.address);
+	});
+
+	return validatorList;
+};

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.`);

framework/test/unit/modules/poa/module.spec.ts

@@ -35,6 +35,7 @@ import {
 	LENGTH_GENERATOR_KEY,
 } from '../../../../src/modules/poa/constants';
 import {
+	ActiveValidator,
 	FeeMethod,
 	ModuleConfigJSON,
 	RandomMethod,
@@ -55,7 +56,7 @@ import {
 	SnapshotObject,
 	ChainProperties,
 } from '../../../../src/modules/poa/stores';
-import { shuffleValidatorList } from '../../../../src/modules/poa/utils';
+import { shuffleValidatorList } from '../../../../src/modules/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<ActiveValidator>(randomSeed, validators);
 			await poaModule.afterTransactionsExecute(context);
 			expect(poaModule.stores.get(SnapshotStore).set).toHaveBeenCalledWith(
 				context,