Update poa module schema validation and endpoint (#8967)

* Update poa endpoint to use schema * Use constants in PoA * Update getValidator * Update utils typing * Remove validation from poa command and add tests * Update constant naming * Update getValidators default return value weight * Update update_generator_key.spec.ts --------- Co-authored-by: Mitsuaki Uchimoto <mitsuaki-u@users.noreply.github.com>
LiskArchive · Sep 18, 2023 · 3a95f36 · 3a95f36
1 parent c659e39
commit 3a95f36
Show file tree

Hide file tree

Showing 11 changed files with 164 additions and 69 deletions.
diff --git a/framework/src/modules/poa/commands/register_authority.ts b/framework/src/modules/poa/commands/register_authority.ts
@@ -12,8 +12,6 @@
  * Removal or modification of this copyright notice is prohibited.
  */
 
-import { address } from '@liskhq/lisk-cryptography';
-import { validator } from '@liskhq/lisk-validator';
 import { BaseCommand } from '../../base_command';
 import { registerAuthoritySchema } from '../schemas';
 import {
@@ -50,26 +48,19 @@ export class RegisterAuthorityCommand extends BaseCommand {
 		context: CommandVerifyContext<RegisterAuthorityParams>,
 	): Promise<VerificationResult> {
 		const { name } = context.params;
-		try {
-			validator.validate(registerAuthoritySchema, context.params);
-		} catch (err) {
-			return {
-				status: VerifyStatus.FAIL,
-				error: err as Error,
-			};
-		}
 
 		if (!POA_VALIDATOR_NAME_REGEX.test(name)) {
 			throw new Error(`Name does not comply with format ${POA_VALIDATOR_NAME_REGEX.toString()}.`);
 		}
 
-		const nameExists = await this.stores.get(NameStore).has(context, Buffer.from(name));
+		const nameExists = await this.stores.get(NameStore).has(context, Buffer.from(name, 'utf-8'));
 		if (nameExists) {
 			throw new Error('Name already exists.');
 		}
 
-		const senderAddress = address.getAddressFromPublicKey(context.transaction.senderPublicKey);
-		const validatorExists = await this.stores.get(ValidatorStore).has(context, senderAddress);
+		const validatorExists = await this.stores
+			.get(ValidatorStore)
+			.has(context, context.transaction.senderAddress);
 		if (validatorExists) {
 			throw new Error('Validator already exists.');
 		}
@@ -82,20 +73,19 @@ export class RegisterAuthorityCommand extends BaseCommand {
 	public async execute(context: CommandExecuteContext<RegisterAuthorityParams>): Promise<void> {
 		const { params } = context;
 
-		const senderAddress = address.getAddressFromPublicKey(context.transaction.senderPublicKey);
 		this._feeMethod.payFee(context, this._authorityRegistrationFee);
 
-		await this.stores.get(ValidatorStore).set(context, senderAddress, {
+		await this.stores.get(ValidatorStore).set(context, context.transaction.senderAddress, {
 			name: params.name,
 		});
 
-		await this.stores.get(NameStore).set(context, Buffer.from(params.name), {
-			address: senderAddress,
+		await this.stores.get(NameStore).set(context, Buffer.from(params.name, 'utf-8'), {
+			address: context.transaction.senderAddress,
 		});
 
 		await this._validatorsMethod.registerValidatorKeys(
 			context,
-			senderAddress,
+			context.transaction.senderAddress,
 			params.blsKey,
 			params.generatorKey,
 			params.proofOfPossession,

diff --git 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<UpdateGeneratorKeyParams>,
 	): Promise<VerificationResult> {
-		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/constants.ts b/framework/src/modules/poa/constants.ts
@@ -33,7 +33,6 @@ export const COMMAND_REGISTER_AUTHORITY = 'registerAuthority';
 export const COMMAND_UPDATE_KEY = 'updateKey';
 export const COMMAND_UPDATE_AUTHORITY = 'updateAuthority';
 export const MAX_UINT64 = BigInt(2) ** BigInt(64) - BigInt(1);
-export const LENGTH_PROOF_OF_POSESSION = 96;
 export const defaultConfig = {
 	authorityRegistrationFee: AUTHORITY_REGISTRATION_FEE.toString(),
 };
@@ -43,3 +42,7 @@ export const defaultConfig = {
 export const KEY_SNAPSHOT_0 = utils.intToBuffer(0, 4);
 export const KEY_SNAPSHOT_1 = utils.intToBuffer(1, 4);
 export const KEY_SNAPSHOT_2 = utils.intToBuffer(2, 4);
+export const SUBSTORE_PREFIX_VALIDATOR_INDEX = 0;
+export const SUBSTORE_PREFIX_CHAIN_INDEX = 1;
+export const SUBSTORE_PREFIX_NAME_INDEX = 2;
+export const SUBSTORE_PREFIX_SNAPSHOT_INDEX = 3;
diff --git a/framework/src/modules/poa/endpoint.ts b/framework/src/modules/poa/endpoint.ts
@@ -12,14 +12,16 @@
  * Removal or modification of this copyright notice is prohibited.
  */
 
+import { validator } from '@liskhq/lisk-validator';
 import { address as cryptoAddress } from '@liskhq/lisk-cryptography';
 import { NotFoundError } from '@liskhq/lisk-db';
 import { BaseEndpoint } from '../base_endpoint';
 import { ValidatorStore } from './stores/validator';
 import { ModuleEndpointContext } from '../../types';
-import { KEY_SNAPSHOT_0 } from './constants';
+import { KEY_SNAPSHOT_0, NUM_BYTES_ADDRESS } from './constants';
 import { SnapshotStore } from './stores';
 import { Validator } from './types';
+import { getValidatorRequestSchema } from './schemas';
 
 export class PoAEndpoint extends BaseEndpoint {
 	private _authorityRegistrationFee!: bigint;
@@ -30,10 +32,10 @@ export class PoAEndpoint extends BaseEndpoint {
 
 	public async getValidator(context: ModuleEndpointContext): Promise<Validator> {
 		const validatorSubStore = this.stores.get(ValidatorStore);
-		const { address } = context.params;
-		if (typeof address !== 'string') {
-			throw new Error('Parameter address must be a string.');
-		}
+
+		validator.validate(getValidatorRequestSchema, context.params);
+		const address = context.params.address as string;
+
 		cryptoAddress.validateLisk32Address(address);
 
 		let validatorName: { name: string };
@@ -70,8 +72,8 @@ export class PoAEndpoint extends BaseEndpoint {
 		context: ModuleEndpointContext,
 	): Promise<{ validators: Validator[] }> {
 		const validatorStore = this.stores.get(ValidatorStore);
-		const startBuf = Buffer.alloc(20);
-		const endBuf = Buffer.alloc(20, 255);
+		const startBuf = Buffer.alloc(NUM_BYTES_ADDRESS);
+		const endBuf = Buffer.alloc(NUM_BYTES_ADDRESS, 255);
 		const validatorStoreData = await validatorStore.iterate(context, {
 			gte: startBuf,
 			lte: endBuf,
@@ -83,18 +85,17 @@ export class PoAEndpoint extends BaseEndpoint {
 		const validatorsData: Validator[] = [];
 		for (const data of validatorStoreData) {
 			const address = cryptoAddress.getLisk32AddressFromAddress(data.key);
-			// `name` comes from type `ValidatorName`
-			const { name } = await validatorStore.get(context, data.key);
+			const { value } = data;
 			const activeValidator = currentRoundSnapshot.validators.find(
 				v => cryptoAddress.getLisk32AddressFromAddress(v.address) === address,
 			);
 
-			const validator: Validator = {
-				name,
+			const v: Validator = {
+				name: value.name,
 				address,
-				weight: activeValidator ? activeValidator.weight.toString() : '',
+				weight: activeValidator ? activeValidator.weight.toString() : '0',
 			};
-			validatorsData.push(validator);
+			validatorsData.push(v);
 		}
 
 		// This is needed since response from this endpoint is returning data in unexpected sorting order on next execution

diff --git a/framework/src/modules/poa/module.ts b/framework/src/modules/poa/module.ts
@@ -29,6 +29,11 @@ import {
 	KEY_SNAPSHOT_2,
 	MAX_UINT64,
 	defaultConfig,
+	POA_VALIDATOR_NAME_REGEX,
+	SUBSTORE_PREFIX_VALIDATOR_INDEX,
+	SUBSTORE_PREFIX_CHAIN_INDEX,
+	SUBSTORE_PREFIX_NAME_INDEX,
+	SUBSTORE_PREFIX_SNAPSHOT_INDEX,
 } from './constants';
 import { shuffleValidatorList } from './utils';
 import { NextValidatorsSetter, MethodContext } from '../../state_machine/types';
@@ -78,14 +83,23 @@ export class PoAModule extends BaseModule {
 	public constructor() {
 		super();
 		this.events.register(AuthorityUpdateEvent, new AuthorityUpdateEvent(this.name));
-		this.stores.register(ValidatorStore, new ValidatorStore(this.name, 0));
-		this.stores.register(ChainPropertiesStore, new ChainPropertiesStore(this.name, 1));
-		this.stores.register(NameStore, new NameStore(this.name, 2));
-		this.stores.register(SnapshotStore, new SnapshotStore(this.name, 3));
+		this.stores.register(
+			ValidatorStore,
+			new ValidatorStore(this.name, SUBSTORE_PREFIX_VALIDATOR_INDEX),
+		);
+		this.stores.register(
+			ChainPropertiesStore,
+			new ChainPropertiesStore(this.name, SUBSTORE_PREFIX_CHAIN_INDEX),
+		);
+		this.stores.register(NameStore, new NameStore(this.name, SUBSTORE_PREFIX_NAME_INDEX));
+		this.stores.register(
+			SnapshotStore,
+			new SnapshotStore(this.name, SUBSTORE_PREFIX_SNAPSHOT_INDEX),
+		);
 	}
 
 	public get name() {
-		return 'poa';
+		return MODULE_NAME_POA;
 	}
 
 	public addDependencies(
@@ -213,7 +227,7 @@ export class PoAModule extends BaseModule {
 				throw new Error('`validators` must be ordered lexicographically by address.');
 			}
 
-			if (!/^[a-z0-9!@$&_.]+$/g.test(validators[i].name)) {
+			if (!POA_VALIDATOR_NAME_REGEX.test(validators[i].name)) {
 				throw new Error('`name` property is invalid. Must contain only characters a-z0-9!@$&_.');
 			}
 		}

diff --git a/framework/src/modules/poa/stores/snapshot.ts b/framework/src/modules/poa/stores/snapshot.ts
@@ -15,10 +15,6 @@ import { BaseStore } from '../../base_store';
 import { NUM_BYTES_ADDRESS } from '../constants';
 import { ActiveValidator } from '../types';
 
-export interface Validator {
-	address: Buffer;
-	weight: bigint;
-}
 export interface SnapshotObject {
 	validators: ActiveValidator[];
 	threshold: bigint;

diff --git a/framework/src/modules/poa/utils.ts b/framework/src/modules/poa/utils.ts
@@ -13,13 +13,12 @@
  */
 
 import { utils } from '@liskhq/lisk-cryptography';
-import { ValidatorWeightWithRoundHash } from './types';
-import { Validator } from './stores';
+import { ValidatorWeightWithRoundHash, ActiveValidator } from './types';
 
 // Same as pos/utils/shuffleValidatorList
 export const shuffleValidatorList = (
 	roundSeed: Buffer,
-	validators: Validator[],
+	validators: ActiveValidator[],
 ): ValidatorWeightWithRoundHash[] => {
 	const validatorsWithRoundHash: ValidatorWeightWithRoundHash[] = [];
 	for (const validator of validators) {

diff --git a/framework/test/unit/modules/poa/commands/register_authority.spec.ts b/framework/test/unit/modules/poa/commands/register_authority.spec.ts
@@ -12,6 +12,7 @@
  * Removal or modification of this copyright notice is prohibited.
  */
 
+import { validator } from '@liskhq/lisk-validator';
 import { address, utils } from '@liskhq/lisk-cryptography';
 import { TransactionAttrs } from '@liskhq/lisk-chain';
 import { codec } from '@liskhq/lisk-codec';
@@ -32,6 +33,7 @@ import {
 	LENGTH_GENERATOR_KEY,
 	MODULE_NAME_POA,
 	POA_VALIDATOR_NAME_REGEX,
+	MAX_LENGTH_NAME,
 } from '../../../../../src/modules/poa/constants';
 
 import { registerAuthoritySchema } from '../../../../../src/modules/poa/schemas';
@@ -101,6 +103,71 @@ describe('RegisterAuthority', () => {
 		nameStore = poaModule.stores.get(NameStore);
 	});
 
+	describe('verifySchema', () => {
+		it(`should throw error when name is longer than ${MAX_LENGTH_NAME}`, () => {
+			expect(() =>
+				validator.validate(registerAuthorityCommand.schema, {
+					...registerAuthorityTransactionParams,
+					name: 'aaaaaaaaaaaaaaaaaaaaaaa',
+				}),
+			).toThrow(`Property '.name' must NOT have more than 20 characters`);
+		});
+
+		it(`should throw error when bls key shorter than ${LENGTH_BLS_KEY}`, () => {
+			expect(() =>
+				validator.validate(registerAuthorityCommand.schema, {
+					...registerAuthorityTransactionParams,
+					blsKey: utils.getRandomBytes(LENGTH_BLS_KEY - 1),
+				}),
+			).toThrow(`Property '.blsKey' minLength not satisfied`);
+		});
+
+		it(`should throw error when bls key longer than ${LENGTH_BLS_KEY}`, () => {
+			expect(() =>
+				validator.validate(registerAuthorityCommand.schema, {
+					...registerAuthorityTransactionParams,
+					blsKey: utils.getRandomBytes(LENGTH_BLS_KEY + 1),
+				}),
+			).toThrow(`Property '.blsKey' maxLength exceeded`);
+		});
+
+		it(`should throw error when proof of possession shorter than ${LENGTH_PROOF_OF_POSSESSION}`, () => {
+			expect(() =>
+				validator.validate(registerAuthorityCommand.schema, {
+					...registerAuthorityTransactionParams,
+					proofOfPossession: utils.getRandomBytes(LENGTH_PROOF_OF_POSSESSION - 1),
+				}),
+			).toThrow(`Property '.proofOfPossession' minLength not satisfied`);
+		});
+
+		it(`should throw error when proof of possession longer than ${LENGTH_PROOF_OF_POSSESSION}`, () => {
+			expect(() =>
+				validator.validate(registerAuthorityCommand.schema, {
+					...registerAuthorityTransactionParams,
+					proofOfPossession: utils.getRandomBytes(LENGTH_PROOF_OF_POSSESSION + 1),
+				}),
+			).toThrow(`Property '.proofOfPossession' maxLength exceeded`);
+		});
+
+		it(`should throw error when generator key shorter than ${LENGTH_GENERATOR_KEY}`, () => {
+			expect(() =>
+				validator.validate(registerAuthorityCommand.schema, {
+					...registerAuthorityTransactionParams,
+					generatorKey: utils.getRandomBytes(LENGTH_GENERATOR_KEY - 1),
+				}),
+			).toThrow(`Property '.generatorKey' minLength not satisfied`);
+		});
+
+		it(`should throw error when generator key longer than ${LENGTH_GENERATOR_KEY}`, () => {
+			expect(() =>
+				validator.validate(registerAuthorityCommand.schema, {
+					...registerAuthorityTransactionParams,
+					generatorKey: utils.getRandomBytes(LENGTH_GENERATOR_KEY + 1),
+				}),
+			).toThrow(`Property '.generatorKey' maxLength exceeded`);
+		});
+	});
+
 	describe('verify', () => {
 		let context: CommandVerifyContext<RegisterAuthorityParams>;
 		beforeEach(() => {

diff --git a/framework/test/unit/modules/poa/commands/update_authority.spec.ts b/framework/test/unit/modules/poa/commands/update_authority.spec.ts
@@ -120,18 +120,7 @@ describe('UpdateAuthority', () => {
 		});
 	});
 
-	describe('verify', () => {
-		let context: CommandVerifyContext<UpdateAuthorityParams>;
-		beforeEach(() => {
-			context = testing
-				.createTransactionContext({
-					stateStore,
-					transaction: buildTransaction({}),
-					chainID,
-				})
-				.createCommandVerifyContext<UpdateAuthorityParams>(updateAuthoritySchema);
-		});
-
+	describe('verifySchema', () => {
 		it('should throw error when length of newValidators is less than 1', () => {
 			expect(() =>
 				validator.validate(updateAuthorityCommand.schema, {
@@ -141,7 +130,7 @@ describe('UpdateAuthority', () => {
 			).toThrow('must NOT have fewer than 1 items');
 		});
 
-		it('should throw error when length of newValidators is greater than MAX_NUM_VALIDATORS', async () => {
+		it('should throw error when length of newValidators is greater than MAX_NUM_VALIDATORS', () => {
 			expect(() =>
 				validator.validate(updateAuthorityCommand.schema, {
 					...updateAuthorityValidatorParams,
@@ -152,6 +141,19 @@ describe('UpdateAuthority', () => {
 				}),
 			).toThrow(`must NOT have more than ${MAX_NUM_VALIDATORS} items`);
 		});
+	});
+
+	describe('verify', () => {
+		let context: CommandVerifyContext<UpdateAuthorityParams>;
+		beforeEach(() => {
+			context = testing
+				.createTransactionContext({
+					stateStore,
+					transaction: buildTransaction({}),
+					chainID,
+				})
+				.createCommandVerifyContext<UpdateAuthorityParams>(updateAuthoritySchema);
+		});
 
 		it('should return error when newValidators are not lexicographically ordered', async () => {
 			context = testing