diff --git a/framework/src/modules/nft/cc_commands/cc_transfer.ts b/framework/src/modules/nft/cc_commands/cc_transfer.ts index c8f9000446a..504ab7cbce4 100644 --- a/framework/src/modules/nft/cc_commands/cc_transfer.ts +++ b/framework/src/modules/nft/cc_commands/cc_transfer.ts @@ -105,24 +105,24 @@ export class CrossChainTransferCommand extends BaseCCCommand { if (nftChainID.equals(ownChainID)) { const storeData = await nftStore.get(getMethodContext(), nftID); + if (status === CCM_STATUS_CODE_OK) { - storeData.owner = recipientAddress; const storedAttributes = storeData.attributesArray; storeData.attributesArray = this._internalMethod.getNewAttributes( nftID, storedAttributes, receivedAttributes, ); - await nftStore.save(getMethodContext(), nftID, storeData); - await this._internalMethod.createUserEntry(getMethodContext(), recipientAddress, nftID); - await escrowStore.del(getMethodContext(), escrowStore.getKey(sendingChainID, nftID)); - } else { + } + + if (status !== CCM_STATUS_CODE_OK) { recipientAddress = senderAddress; - storeData.owner = recipientAddress; - await nftStore.save(getMethodContext(), nftID, storeData); - await this._internalMethod.createUserEntry(getMethodContext(), recipientAddress, nftID); - await escrowStore.del(getMethodContext(), escrowStore.getKey(sendingChainID, nftID)); } + + storeData.owner = recipientAddress; + await nftStore.save(getMethodContext(), nftID, storeData); + await this._internalMethod.createUserEntry(getMethodContext(), recipientAddress, nftID); + await escrowStore.del(getMethodContext(), escrowStore.getKey(sendingChainID, nftID)); } else { const isSupported = await this._method.isNFTSupported(getMethodContext(), nftID); if (!isSupported) { @@ -137,21 +137,22 @@ export class CrossChainTransferCommand extends BaseCCCommand { ); throw new Error('Non-supported NFT'); } + if (status === CCM_STATUS_CODE_OK) { this._feeMethod.payFee(getMethodContext(), BigInt(FEE_CREATE_NFT)); - await nftStore.save(getMethodContext(), nftID, { - owner: recipientAddress, - attributesArray: receivedAttributes as NFTAttributes[], - }); - await this._internalMethod.createUserEntry(getMethodContext(), recipientAddress, nftID); - } else { + } + + if (status !== CCM_STATUS_CODE_OK) { recipientAddress = senderAddress; - await nftStore.save(getMethodContext(), nftID, { - owner: recipientAddress, - attributesArray: receivedAttributes as NFTAttributes[], - }); - await this._internalMethod.createUserEntry(getMethodContext(), recipientAddress, nftID); } + + await this._internalMethod.createNFTEntry( + getMethodContext(), + recipientAddress, + nftID, + receivedAttributes as NFTAttributes[], + ); + await this._internalMethod.createUserEntry(getMethodContext(), recipientAddress, nftID); } this.events.get(CcmTransferEvent).log(context, { diff --git a/framework/src/modules/nft/internal_method.ts b/framework/src/modules/nft/internal_method.ts index bb27b9eac6a..3e785a5ae6c 100644 --- a/framework/src/modules/nft/internal_method.ts +++ b/framework/src/modules/nft/internal_method.ts @@ -16,7 +16,7 @@ import { codec } from '@liskhq/lisk-codec'; import { BaseMethod } from '../base_method'; import { NFTStore, NFTAttributes } from './stores/nft'; import { InteroperabilityMethod, ModuleConfig, NFTMethod } from './types'; -import { MethodContext } from '../../state_machine'; +import { GenesisBlockExecuteContext, MethodContext } from '../../state_machine'; import { TransferEvent } from './events/transfer'; import { UserStore } from './stores/user'; import { CROSS_CHAIN_COMMAND_NAME_TRANSFER, MODULE_NAME_NFT, NFT_NOT_LOCKED } from './constants'; @@ -39,7 +39,7 @@ export class InternalMethod extends BaseMethod { } public async createEscrowEntry( - methodContext: MethodContext, + methodContext: MethodContext | GenesisBlockExecuteContext, receivingChainID: Buffer, nftID: Buffer, ): Promise { @@ -49,7 +49,7 @@ export class InternalMethod extends BaseMethod { } public async createUserEntry( - methodContext: MethodContext, + methodContext: MethodContext | GenesisBlockExecuteContext, address: Buffer, nftID: Buffer, ): Promise { @@ -61,17 +61,13 @@ export class InternalMethod extends BaseMethod { } public async createNFTEntry( - methodContext: MethodContext, + methodContext: MethodContext | GenesisBlockExecuteContext, address: Buffer, nftID: Buffer, attributesArray: NFTAttributes[], ): Promise { - const moduleNames = []; - for (const item of attributesArray) { - moduleNames.push(item.module); - } - - if (new Set(moduleNames).size !== attributesArray.length) { + const hasDuplicates = this.hasDuplicateModuleNames(attributesArray); + if (hasDuplicates) { throw new Error('Invalid attributes array provided'); } @@ -82,6 +78,15 @@ export class InternalMethod extends BaseMethod { }); } + public hasDuplicateModuleNames(attributesArray: NFTAttributes[]): boolean { + const moduleNames = []; + for (const item of attributesArray) { + moduleNames.push(item.module); + } + + return new Set(moduleNames).size !== attributesArray.length; + } + public async transferInternal( methodContext: MethodContext, recipientAddress: Buffer, diff --git a/framework/src/modules/nft/method.ts b/framework/src/modules/nft/method.ts index 4ae5995f69d..90790475921 100644 --- a/framework/src/modules/nft/method.ts +++ b/framework/src/modules/nft/method.ts @@ -295,13 +295,7 @@ export class NFTMethod extends BaseMethod { collectionID: Buffer, attributesArray: NFTAttributes[], ): Promise { - const moduleNames = []; - for (const item of attributesArray) { - moduleNames.push(item.module); - } - if (new Set(moduleNames).size !== attributesArray.length) { - throw new Error('Invalid attributes array provided'); - } + this._internalMethod.hasDuplicateModuleNames(attributesArray); const index = await this.getNextAvailableIndex(methodContext, collectionID); const indexBytes = Buffer.alloc(LENGTH_INDEX); @@ -310,16 +304,9 @@ export class NFTMethod extends BaseMethod { const nftID = Buffer.concat([this._config.ownChainID, collectionID, indexBytes]); this._feeMethod.payFee(methodContext, BigInt(FEE_CREATE_NFT)); - const nftStore = this.stores.get(NFTStore); - await nftStore.save(methodContext, nftID, { - owner: address, - attributesArray, - }); + await this._internalMethod.createNFTEntry(methodContext, address, nftID, attributesArray); - const userStore = this.stores.get(UserStore); - await userStore.set(methodContext, userStore.getKey(address, nftID), { - lockingModule: NFT_NOT_LOCKED, - }); + await this._internalMethod.createUserEntry(methodContext, address, nftID); this.events.get(CreateEvent).log(methodContext, { address, diff --git a/framework/src/modules/nft/module.ts b/framework/src/modules/nft/module.ts index 92c876a4dfb..2a63430a894 100644 --- a/framework/src/modules/nft/module.ts +++ b/framework/src/modules/nft/module.ts @@ -67,7 +67,6 @@ import { LENGTH_ADDRESS, LENGTH_CHAIN_ID, MODULE_NAME_NFT, - NFT_NOT_LOCKED, } from './constants'; export class NFTModule extends BaseInteroperableModule { @@ -273,24 +272,15 @@ export class NFTModule extends BaseInteroperableModule { supportedChainsKeySet.add(supportedNFT.chainID); } - const nftStore = this.stores.get(NFTStore); - const escrowStore = this.stores.get(EscrowStore); - const userStore = this.stores.get(UserStore); - for (const nft of genesisStore.nftSubstore) { const { owner, nftID, attributesArray } = nft; - await nftStore.save(context, nftID, { - owner, - attributesArray, - }); + await this._internalMethod.createNFTEntry(context, owner, nftID, attributesArray); if (owner.length === LENGTH_CHAIN_ID) { - await escrowStore.set(context, escrowStore.getKey(owner, nftID), {}); + await this._internalMethod.createEscrowEntry(context, owner, nftID); } else { - await userStore.set(context, userStore.getKey(owner, nftID), { - lockingModule: NFT_NOT_LOCKED, - }); + await this._internalMethod.createUserEntry(context, owner, nftID); } } diff --git a/framework/test/unit/modules/nft/internal_method.spec.ts b/framework/test/unit/modules/nft/internal_method.spec.ts index b9a843e008e..798a00b0769 100644 --- a/framework/test/unit/modules/nft/internal_method.spec.ts +++ b/framework/test/unit/modules/nft/internal_method.spec.ts @@ -34,7 +34,7 @@ import { TransferEvent, TransferEventData } from '../../../../src/modules/nft/ev import { UserStore } from '../../../../src/modules/nft/stores/user'; import { EscrowStore } from '../../../../src/modules/nft/stores/escrow'; import { NFTMethod } from '../../../../src/modules/nft/method'; -import { InteroperabilityMethod } from '../../../../src/modules/nft/types'; +import { InteroperabilityMethod, NFTAttributes } from '../../../../src/modules/nft/types'; import { TransferCrossChainEvent, TransferCrossChainEventData, @@ -102,6 +102,38 @@ describe('InternalMethod', () => { }); }); + describe('hasDuplicateModuleNames', () => { + it('should return false when the attributes array is empty', () => { + const attributesArray: NFTAttributes[] = []; + + expect(internalMethod.hasDuplicateModuleNames(attributesArray)).toBeFalse(); + }); + + it('should return false when all module names are unique', () => { + const attributesArray: NFTAttributes[] = [ + { module: 'module1', attributes: Buffer.from('attributes1') }, + { module: 'module2', attributes: Buffer.from('attributes2') }, + { module: 'module3', attributes: Buffer.from('attributes3') }, + ]; + + const result = internalMethod.hasDuplicateModuleNames(attributesArray); + + expect(result).toBeFalse(); + }); + + it('should return true when there are duplicate module names', () => { + const attributesArray: NFTAttributes[] = [ + { module: 'module1', attributes: Buffer.from('attributes1') }, + { module: 'module1', attributes: Buffer.from('attributes2') }, + { module: 'module3', attributes: Buffer.from('attributes3') }, + ]; + + const result = internalMethod.hasDuplicateModuleNames(attributesArray); + + expect(result).toBeTrue(); + }); + }); + describe('createNFTEntry', () => { it('should throw for duplicate module names in attributes array', async () => { const attributesArray = [ @@ -132,7 +164,7 @@ describe('InternalMethod', () => { }, ]; - const sortedAttributesArray = unsortedAttributesArray.sort((a, b) => + const sortedAttributesArray = [...unsortedAttributesArray].sort((a, b) => a.module.localeCompare(b.module, 'en'), );