Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Remove unnecessary calls to validator.validate in commands of NFT module #9012

Merged
merged 2 commits into from
Sep 20, 2023
Merged

Remove unnecessary calls to validator.validate in commands of NFT module #9012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc29841
Remove validation in verify hook
Incede Sep 18, 2023
4670a87
Merge branch 'release/6.1.0' into 8987-remove-validation
shuse2 Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion framework/src/modules/nft/cc_commands/cc_transfer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ export class CrossChainTransferCommand extends BaseCCCommand {
crossChainNFTTransferMessageParamsSchema,
ccm.params,
);
validator.validate(crossChainNFTTransferMessageParamsSchema, params);

if (ccm.status > MAX_RESERVED_ERROR_STATUS) {
throw new Error('Invalid CCM error code');
Expand Down
3 changes: 0 additions & 3 deletions framework/src/modules/nft/commands/transfer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
* Removal or modification of this copyright notice is prohibited.
*/

import { validator } from '@liskhq/lisk-validator';
import {
CommandExecuteContext,
CommandVerifyContext,
Expand Down Expand Up @@ -45,8 +44,6 @@ export class TransferCommand extends BaseCommand {
public async verify(context: CommandVerifyContext<Params>): Promise<VerificationResult> {
const { params } = context;

validator.validate<Params>(this.schema, params);

const nftStore = this.stores.get(NFTStore);

const nftExists = await nftStore.has(context, params.nftID);
Expand Down
3 changes: 0 additions & 3 deletions framework/src/modules/nft/commands/transfer_cross_chain.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
* Removal or modification of this copyright notice is prohibited.
*/

import { validator } from '@liskhq/lisk-validator';
import { crossChainTransferParamsSchema } from '../schemas';
import { NFTStore } from '../stores/nft';
import { NFTMethod } from '../method';
Expand Down Expand Up @@ -59,8 +58,6 @@ export class TransferCrossChainCommand extends BaseCommand {
public async verify(context: CommandVerifyContext<Params>): Promise<VerificationResult> {
const { params } = context;

validator.validate(this.schema, params);

const nftStore = this.stores.get(NFTStore);
const nftExists = await nftStore.has(context.getMethodContext(), params.nftID);

Expand Down
34 changes: 0 additions & 34 deletions framework/test/unit/modules/nft/cc_comands/cc_transfer.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,40 +183,6 @@ describe('CrossChain Transfer Command', () => {
await expect(command.verify(context)).resolves.toBeUndefined();
});

it('throw for if validation fails', async () => {
params = codec.encode(crossChainNFTTransferMessageParamsSchema, {
nftID: Buffer.alloc(LENGTH_NFT_ID + 1, 1),
senderAddress,
recipientAddress,
attributesArray,
data: '',
});
ccm = {
crossChainCommand: CROSS_CHAIN_COMMAND_NAME_TRANSFER,
module: module.name,
nonce: BigInt(1),
sendingChainID,
receivingChainID,
fee: BigInt(30000),
status: CCM_STATUS_OK,
params,
};
context = {
ccm,
transaction: defaultTransaction,
header: defaultHeader,
stateStore,
contextStore,
getMethodContext,
eventQueue,
getStore,
logger: fakeLogger,
chainID,
};

await expect(command.verify(context)).rejects.toThrow(`Property '.nftID' maxLength exceeded`);
});

it('throw for invalid ccm status', async () => {
ccm = {
crossChainCommand: CROSS_CHAIN_COMMAND_NAME_TRANSFER,
Expand Down
40 changes: 0 additions & 40 deletions framework/test/unit/modules/nft/commands/transfer.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,46 +99,6 @@ describe('Transfer command', () => {
});

describe('verify', () => {
it('should fail if nftID does not have valid length', async () => {
const nftMinLengthContext = createTransactionContextWithOverridingParams({
nftID: Buffer.alloc(LENGTH_NFT_ID - 1, 1),
});

const nftMaxLengthContext = createTransactionContextWithOverridingParams({
nftID: Buffer.alloc(LENGTH_NFT_ID + 1, 1),
});

await expect(
command.verify(nftMinLengthContext.createCommandVerifyContext(transferParamsSchema)),
).rejects.toThrow("'.nftID' minLength not satisfied");

await expect(
command.verify(nftMaxLengthContext.createCommandExecuteContext(transferParamsSchema)),
).rejects.toThrow("'.nftID' maxLength exceeded");
});

it('should fail if recipientAddress is not 20 bytes', async () => {
const recipientAddressIncorrectLengthContext = createTransactionContextWithOverridingParams({
recipientAddress: utils.getRandomBytes(22),
});

await expect(
command.verify(
recipientAddressIncorrectLengthContext.createCommandVerifyContext(transferParamsSchema),
),
).rejects.toThrow("'.recipientAddress' address length invalid");
});

it('should fail if data exceeds 64 characters', async () => {
const dataIncorrectLengthContext = createTransactionContextWithOverridingParams({
data: '1'.repeat(65),
});

await expect(
command.verify(dataIncorrectLengthContext.createCommandVerifyContext(transferParamsSchema)),
).rejects.toThrow("'.data' must NOT have more than 64 characters");
});

it('should fail if nftID does not exist', async () => {
const nftIDNotExistingContext = createTransactionContextWithOverridingParams({
nftID: Buffer.alloc(LENGTH_NFT_ID, 0),
Expand Down
86 changes: 0 additions & 86 deletions framework/test/unit/modules/nft/commands/transfer_cross_chain.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,92 +215,6 @@ describe('TransferCrossChainComand', () => {
).rejects.toThrow('Receiving chain cannot be the sending chain');
});

it('should fail if NFT does not have valid length', async () => {
const nftMinLengthContext = createTransactionContextWithOverridingParams({
nftID: utils.getRandomBytes(LENGTH_NFT_ID - 1),
});

const nftMaxLengthContext = createTransactionContextWithOverridingParams({
nftID: utils.getRandomBytes(LENGTH_NFT_ID + 1),
});

await expect(
command.verify(
nftMinLengthContext.createCommandVerifyContext(crossChainTransferParamsSchema),
),
).rejects.toThrow("'.nftID' minLength not satisfied");

await expect(
command.verify(
nftMaxLengthContext.createCommandExecuteContext(crossChainTransferParamsSchema),
),
).rejects.toThrow("'.nftID' maxLength exceeded");
});

it('should fail if receivingChainID does not have valid length', async () => {
const receivingChainIDMinLengthContext = createTransactionContextWithOverridingParams({
receivingChainID: utils.getRandomBytes(LENGTH_CHAIN_ID - 1),
});

const receivingChainIDMaxLengthContext = createTransactionContextWithOverridingParams({
receivingChainID: utils.getRandomBytes(LENGTH_CHAIN_ID + 1),
});

await expect(
command.verify(
receivingChainIDMinLengthContext.createCommandVerifyContext(
crossChainTransferParamsSchema,
),
),
).rejects.toThrow("'.receivingChainID' minLength not satisfied");

await expect(
command.verify(
receivingChainIDMaxLengthContext.createCommandVerifyContext(
crossChainTransferParamsSchema,
),
),
).rejects.toThrow("'.receivingChainID' maxLength exceeded");
});

it('should fail if recipientAddress does not have valid length', async () => {
const recipientAddressMinLengthContext = createTransactionContextWithOverridingParams({
recipientAddress: utils.getRandomBytes(LENGTH_ADDRESS - 1),
});

const recipientAddressMaxLenghtContext = createTransactionContextWithOverridingParams({
recipientAddress: utils.getRandomBytes(LENGTH_ADDRESS + 1),
});

await expect(
command.verify(
recipientAddressMinLengthContext.createCommandVerifyContext(
crossChainTransferParamsSchema,
),
),
).rejects.toThrow("'.recipientAddress' address length invalid");

await expect(
command.verify(
recipientAddressMaxLenghtContext.createCommandVerifyContext(
crossChainTransferParamsSchema,
),
),
).rejects.toThrow("'.recipientAddress' address length invalid");
});

it('should fail if data has more than 64 characters', async () => {
const dataMaxLengthContext = createTransactionContextWithOverridingParams({
data: '1'.repeat(65),
});

await expect(
command.verify(
dataMaxLengthContext.createCommandVerifyContext(crossChainTransferParamsSchema),
),
).rejects.toThrow("'.data' must NOT have more than 64 characters");
});

it('should fail if NFT does not exist', async () => {
const context = createTransactionContextWithOverridingParams({
nftID: utils.getRandomBytes(LENGTH_NFT_ID),
Expand Down