diff --git a/framework/src/modules/nft/cc_commands/cc_transfer.ts b/framework/src/modules/nft/cc_commands/cc_transfer.ts index d4a28e89c6..ae4884b76f 100644 --- a/framework/src/modules/nft/cc_commands/cc_transfer.ts +++ b/framework/src/modules/nft/cc_commands/cc_transfer.ts @@ -112,24 +112,26 @@ 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 { 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)); } + + await this._internalMethod.createNFTEntry( + getMethodContext(), + recipientAddress, + nftID, + storeData.attributesArray, + ); + 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) { @@ -146,22 +148,21 @@ export class CrossChainTransferCommand extends BaseCCCommand { ); throw new Error('Non-supported NFT'); } + this._feeMethod.payFee(getMethodContext(), BigInt(FEE_CREATE_NFT)); - if (status === CCM_STATUS_CODE_OK) { - 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 3ed927ff1d..626ae05d62 100644 --- a/framework/src/modules/nft/internal_method.ts +++ b/framework/src/modules/nft/internal_method.ts @@ -90,12 +90,8 @@ export class InternalMethod extends BaseMethod { 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'); } @@ -106,6 +102,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 verifyTransfer( immutableMethodContext: ImmutableMethodContext, senderAddress: Buffer, diff --git a/framework/src/modules/nft/method.ts b/framework/src/modules/nft/method.ts index 6814453c4d..af78d9f046 100644 --- a/framework/src/modules/nft/method.ts +++ b/framework/src/modules/nft/method.ts @@ -255,14 +255,6 @@ 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'); - } - const index = await this.getNextAvailableIndex(methodContext, collectionID); const indexBytes = Buffer.alloc(LENGTH_INDEX); indexBytes.writeBigInt64BE(index); @@ -270,16 +262,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, @@ -798,7 +783,12 @@ export class NFTMethod extends BaseMethod { storedAttributes, receivedAttributes, ); - await nftStore.save(methodContext, nftID, nftData); + await this._internalMethod.createNFTEntry( + methodContext, + nftData.owner, + nftID, + nftData.attributesArray, + ); await this._internalMethod.createUserEntry(methodContext, nftData.owner, nftID); await escrowStore.del(methodContext, escrowStore.getKey(terminatedChainID, nftID)); @@ -835,7 +825,8 @@ export class NFTMethod extends BaseMethod { } else { nft.attributesArray.push({ module, attributes }); } - await nftStore.save(methodContext, nftID, nft); + + await this._internalMethod.createNFTEntry(methodContext, nft.owner, nftID, nft.attributesArray); this.events.get(SetAttributesEvent).log(methodContext, { nftID, diff --git a/framework/src/modules/nft/module.ts b/framework/src/modules/nft/module.ts index 9fc8653626..a6eb534171 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 { @@ -268,24 +267,20 @@ 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, { + await this._internalMethod.createNFTEntry( + context.getMethodContext(), owner, + nftID, attributesArray, - }); + ); if (owner.length === LENGTH_CHAIN_ID) { - await escrowStore.set(context, escrowStore.getKey(owner, nftID), {}); + await this._internalMethod.createEscrowEntry(context.getMethodContext(), owner, nftID); } else { - await userStore.set(context, userStore.getKey(owner, nftID), { - lockingModule: NFT_NOT_LOCKED, - }); + await this._internalMethod.createUserEntry(context.getMethodContext(), owner, nftID); } } diff --git a/framework/test/unit/modules/nft/cc_comands/cc_transfer.spec.ts b/framework/test/unit/modules/nft/cc_comands/cc_transfer.spec.ts index 9767f2b189..c422e16e89 100644 --- a/framework/test/unit/modules/nft/cc_comands/cc_transfer.spec.ts +++ b/framework/test/unit/modules/nft/cc_comands/cc_transfer.spec.ts @@ -18,6 +18,7 @@ import { NFTModule } from '../../../../../src/modules/nft/module'; import { InMemoryPrefixedStateDB } from '../../../../../src/testing'; import { ALL_SUPPORTED_NFTS_KEY, + CCM_STATUS_CODE_OK, CROSS_CHAIN_COMMAND_NAME_TRANSFER, FEE_CREATE_NFT, LENGTH_CHAIN_ID, @@ -708,5 +709,79 @@ describe('CrossChain Transfer Command', () => { sendingChainID: ccm.sendingChainID, }); }); + + it('should throw if duplicate module attributes are found when a foreign NFT is received - status === CCM_STATUS_CODE_OK', async () => { + params = codec.encode(crossChainNFTTransferMessageParamsSchema, { + nftID, + senderAddress, + recipientAddress, + attributesArray: [ + { module: 'module1', attributes: Buffer.alloc(5) }, + { module: 'module1', attributes: Buffer.alloc(5) }, + ], + data: '', + }); + ccm = { + crossChainCommand: CROSS_CHAIN_COMMAND_NAME_TRANSFER, + module: module.name, + nonce: BigInt(1), + sendingChainID, + receivingChainID, + fee: BigInt(30000), + status: CCM_STATUS_CODE_OK, + params, + }; + context = { + ccm, + transaction: defaultTransaction, + header: defaultHeader, + stateStore, + contextStore, + getMethodContext, + eventQueue: new EventQueue(0), + getStore, + logger: fakeLogger, + chainID, + }; + + await expect(command.execute(context)).rejects.toThrow('Invalid attributes array provided'); + }); + }); + + it('should throw if duplicate module attributes are found when a foreign NFT is bounced - status !== CCM_STATUS_CODE_OK', async () => { + params = codec.encode(crossChainNFTTransferMessageParamsSchema, { + nftID, + senderAddress, + recipientAddress, + attributesArray: [ + { module: 'module1', attributes: Buffer.alloc(5) }, + { module: 'module1', attributes: Buffer.alloc(5) }, + ], + data: '', + }); + ccm = { + crossChainCommand: CROSS_CHAIN_COMMAND_NAME_TRANSFER, + module: module.name, + nonce: BigInt(1), + sendingChainID, + receivingChainID, + fee: BigInt(30000), + status: 12345, + params, + }; + context = { + ccm, + transaction: defaultTransaction, + header: defaultHeader, + stateStore, + contextStore, + getMethodContext, + eventQueue: new EventQueue(0), + getStore, + logger: fakeLogger, + chainID, + }; + + await expect(command.execute(context)).rejects.toThrow('Invalid attributes array provided'); }); }); diff --git a/framework/test/unit/modules/nft/internal_method.spec.ts b/framework/test/unit/modules/nft/internal_method.spec.ts index 983daf71ec..89456d8d66 100644 --- a/framework/test/unit/modules/nft/internal_method.spec.ts +++ b/framework/test/unit/modules/nft/internal_method.spec.ts @@ -34,7 +34,11 @@ 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, TokenMethod } from '../../../../src/modules/nft/types'; +import { + InteroperabilityMethod, + NFTAttributes, + TokenMethod, +} from '../../../../src/modules/nft/types'; import { TransferCrossChainEvent, TransferCrossChainEventData, @@ -103,15 +107,47 @@ 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 = [ { - module: 'token', + module: 'module1', attributes: Buffer.alloc(8, 1), }, { - module: 'token', + module: 'module1', attributes: Buffer.alloc(8, 2), }, ]; @@ -124,16 +160,16 @@ describe('InternalMethod', () => { it('should create an entry in NFStore with attributes sorted by module if there is no duplicate module name', async () => { const unsortedAttributesArray = [ { - module: 'token', + module: 'module1', attributes: Buffer.alloc(8, 1), }, { - module: 'pos', + module: 'module2', attributes: Buffer.alloc(8, 1), }, ]; - const sortedAttributesArray = unsortedAttributesArray.sort((a, b) => + const sortedAttributesArray = [...unsortedAttributesArray].sort((a, b) => a.module.localeCompare(b.module, 'en'), );