Repository: LiskHQ/lisk-core (ts)

Issue:
Commands are responsible for validating their parameters
### Expected behavior
RegisterKeysCommand should validate the params in the verify function 

### Actual behavior
The params is validated in [execute](https://github.com/LiskHQ/lisk-core/blob/30167c3c340f7758e5788f0b7a98859dcc12dcf0/src/application/modules/legacy/commands/register_keys.ts#L67-L68)

### Steps to reproduce
N/A

### Which version(s) does this affect? (Environment, OS, etc...)
4.0.0-beta.0

Pull request:
Commands are responsible for validating their parameters
### What was the problem?

This PR resolves #778 

### How was it solved?
- [x] Add params validation to verify function
- [x] Add unit test case

### How was it tested?
Local by running unit tests

Patch:
diff --git a/src/application/modules/legacy/commands/register_keys.ts b/src/application/modules/legacy/commands/register_keys.ts
index 05450f5..dd9be2c 100644
--- a/src/application/modules/legacy/commands/register_keys.ts
+++ b/src/application/modules/legacy/commands/register_keys.ts
@@ -47,6 +47,17 @@ export class RegisterKeysCommand extends BaseCommand {
 	}
 
 	public async verify(ctx: CommandVerifyContext): Promise<VerificationResult> {
+		const params = (ctx.params as unknown) as registerKeysData;
+
+		try {
+			validator.validate(registerKeysParamsSchema, params);
+		} catch (err) {
+			return {
+				status: VerifyStatus.FAIL,
+				error: err as Error,
+			};
+		}
+
 		const validatorAddress = getAddressFromPublicKey(ctx.transaction.senderPublicKey);
 		const validatorKeys = await this._validatorsMethod.getValidatorKeys(
 			ctx.getMethodContext(),
@@ -65,8 +76,6 @@ export class RegisterKeysCommand extends BaseCommand {
 
 	public async execute(ctx: CommandExecuteContext): Promise<void> {
 		const params = (ctx.params as unknown) as registerKeysData;
-		validator.validate(registerKeysParamsSchema, params);
-
 		const validatorAddress = getAddressFromPublicKey(ctx.transaction.senderPublicKey);
 
 		await this._validatorsMethod.setValidatorGeneratorKey(
diff --git a/src/application/modules/legacy/endpoint.ts b/src/application/modules/legacy/endpoint.ts
index 11b25b5..2f670e7 100644
--- a/src/application/modules/legacy/endpoint.ts
+++ b/src/application/modules/legacy/endpoint.ts
@@ -32,12 +32,12 @@ export class LegacyEndpoint extends BaseEndpoint {
 	public async getLegacyAccount(
 		ctx: ModuleEndpointContext,
 	): Promise<JSONObject<LegacyStoreData> | undefined> {
-		validator.validate(legacyAccountRequestSchema, ctx.params);
+		try {
+			validator.validate(legacyAccountRequestSchema, ctx.params);
 
-		const publicKey = Buffer.from(ctx.params.publicKey as string, 'hex');
-		const legacyStore = this.stores.get(LegacyAccountStore);
+			const publicKey = Buffer.from(ctx.params.publicKey as string, 'hex');
+			const legacyStore = this.stores.get(LegacyAccountStore);
 
-		try {
 			const legacyAddressBuffer = getLegacyAddress(publicKey);
 			const legacyAddress = legacyAddressBuffer.toString('hex');
 			const hasLegacyAddress = await legacyStore.has(ctx, legacyAddressBuffer);


Test patch:
diff --git a/test/application/modules/legacy/commands/register_keys.spec.ts b/test/application/modules/legacy/commands/register_keys.spec.ts
index d67ed85..76158b3 100644
--- a/test/application/modules/legacy/commands/register_keys.spec.ts
+++ b/test/application/modules/legacy/commands/register_keys.spec.ts
@@ -159,6 +159,53 @@ describe('Register keys command', () => {
 				VerifyStatus.FAIL,
 			);
 		});
+
+		it('should throw error if transaction params does not follow registerKeysParamsSchema', async () => {
+			// Mock dependencies
+			const setValidatorBLSKey = jest.fn();
+			const getValidatorKeys = jest.fn().mockReturnValue({ generatorKey: Buffer.alloc(32) });
+			const unbanValidator = jest.fn();
+			registerKeysCommand.addDependencies(
+				{ setValidatorBLSKey, getValidatorKeys } as any,
+				{ unbanValidator } as any,
+			);
+
+			// Create context
+			const invalidParams = {
+				blsKey: getRandomBytes(48),
+				proofOfPossession: getRandomBytes(64).toString('hex'),
+			};
+			const invalidRegisterKeysParamsSchema = {
+				$id: '/legacy/command/invalidRegisterKeysParams',
+				type: 'object',
+				required: ['blsKey', 'proofOfPossession'],
+				properties: {
+					blsKey: {
+						dataType: 'bytes',
+						fieldNumber: 1,
+					},
+					proofOfPossession: {
+						dataType: 'string',
+						fieldNumber: 2,
+					},
+				},
+			};
+			const invalidRegisterKeysTransaction = getRegisterKeysTransaction(
+				invalidParams,
+				invalidRegisterKeysParamsSchema,
+			);
+			const context = testing
+				.createTransactionContext({
+					chainID,
+					transaction: invalidRegisterKeysTransaction,
+				})
+				.createCommandVerifyContext(invalidRegisterKeysParamsSchema);
+
+			await expect(registerKeysCommand.verify(context)).resolves.toHaveProperty(
+				'status',
+				VerifyStatus.FAIL,
+			);
+		});
 	});
 
 	describe('execute', () => {
@@ -277,51 +324,6 @@ describe('Register keys command', () => {
 			expect(unbanValidator).toHaveBeenCalledTimes(0);
 		});
 
-		it('should throw error if transaction params does not follow registerBLSKeyParamsSchema', async () => {
-			// Mock dependencies
-			const setValidatorBLSKey = jest.fn();
-			const getValidatorKeys = jest.fn().mockReturnValue({ generatorKey: Buffer.alloc(32) });
-			const unbanValidator = jest.fn();
-			registerKeysCommand.addDependencies(
-				{ setValidatorBLSKey, getValidatorKeys } as any,
-				{ unbanValidator } as any,
-			);
-
-			// Create context
-			const invalidParams = {
-				blsKey: getRandomBytes(48),
-				proofOfPossession: getRandomBytes(64).toString('hex'),
-			};
-			const invalidRegisterKeysParamsSchema = {
-				$id: '/legacy/command/invalidRegisterKeysParams',
-				type: 'object',
-				required: ['blsKey', 'proofOfPossession'],
-				properties: {
-					blsKey: {
-						dataType: 'bytes',
-						fieldNumber: 1,
-					},
-					proofOfPossession: {
-						dataType: 'string',
-						fieldNumber: 2,
-					},
-				},
-			};
-			const invalidRegisterKeysTransaction = getRegisterKeysTransaction(
-				invalidParams,
-				invalidRegisterKeysParamsSchema,
-			);
-			const context = testing
-				.createTransactionContext({
-					chainID,
-					transaction: invalidRegisterKeysTransaction,
-				})
-				.createCommandExecuteContext(invalidRegisterKeysParamsSchema);
-
-			await expect(registerKeysCommand.execute(context)).rejects.toThrow();
-			expect(setValidatorBLSKey).toHaveBeenCalledTimes(0);
-		});
-
 		it('should throw error when transaction params has no BLS key', async () => {
 			// Mock dependencies
 			const getValidatorKeys = jest
diff --git a/test/application/modules/legacy/endpoint.spec.ts b/test/application/modules/legacy/endpoint.spec.ts
index a7b3a42..6344e87 100644
--- a/test/application/modules/legacy/endpoint.spec.ts
+++ b/test/application/modules/legacy/endpoint.spec.ts
@@ -133,5 +133,16 @@ describe('LegacyEndpoint', () => {
 			expect(legacyAccount).toHaveProperty('legacyAddress', expectedLegacyAccount.legacyAddress);
 			expect(legacyAccount).toHaveProperty('balance', expectedLegacyAccount.balance);
 		});
+
+		it('should throw error when called with invalid params', async () => {
+			const context = testing.createTransientModuleEndpointContext({
+				stateStore,
+				params: {
+					address: getAddressFromPublicKey(nonExistingPublicKey),
+				},
+			});
+
+			await expect(legacyEndpoint.getLegacyAccount(context)).rejects.toThrow();
+		});
 	});
 });