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

Commit

Permalink
NFT recover method now checks if NFT exists (#9028)
Browse files Browse the repository at this point in the history 
* NFT recover method now checks if NFT exists

* Rename storeKey and storeValue variables into nftID and nft

---------

Co-authored-by: Incede <33103370+Incede@users.noreply.github.com>
  • Loading branch information
@bobanm @Incede
bobanm and Incede authored Oct 9, 2023
1 parent 5b2792f commit 2eabd20
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 56 deletions.
41 changes: 29 additions & 12 deletions framework/src/modules/nft/method.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -700,23 +700,22 @@ export class NFTMethod extends BaseMethod {
methodContext: MethodContext,
terminatedChainID: Buffer,
substorePrefix: Buffer,
storeKey: Buffer,
storeValue: Buffer,
nftID: Buffer,
nft: Buffer,
): Promise<void> {
const nftStore = this.stores.get(NFTStore);
const nftID = storeKey;
let isValidInput = true;
let decodedValue: NFTStoreData;
try {
decodedValue = codec.decode<NFTStoreData>(nftStoreSchema, storeValue);
decodedValue = codec.decode<NFTStoreData>(nftStoreSchema, nft);
validator.validate(nftStoreSchema, decodedValue);
} catch (error) {
isValidInput = false;
}

if (
!substorePrefix.equals(nftStore.subStorePrefix) ||
storeKey.length !== LENGTH_NFT_ID ||
nftID.length !== LENGTH_NFT_ID ||
!isValidInput
) {
this.events.get(RecoverEvent).error(
Expand Down Expand Up @@ -744,8 +743,26 @@ export class NFTMethod extends BaseMethod {
throw new Error('Recovery called by a foreign chain');
}

const nft = await nftStore.get(methodContext, nftID);
if (!nft.owner.equals(terminatedChainID)) {
let nftData;
try {
nftData = await this.getNFT(methodContext, nftID);
} catch (error) {
if (error instanceof NotFoundError) {
this.events.get(RecoverEvent).error(
methodContext,
{
terminatedChainID,
nftID,
},
NftEventResult.RESULT_NFT_DOES_NOT_EXIST,
);

throw new Error('NFT substore entry does not exist');
}
throw error;
}

if (!nftData.owner.equals(terminatedChainID)) {
this.events.get(RecoverEvent).error(
methodContext,
{
Expand All @@ -772,17 +789,17 @@ export class NFTMethod extends BaseMethod {
}

const escrowStore = this.stores.get(EscrowStore);
nft.owner = storeValueOwner;
const storedAttributes = nft.attributesArray;
nftData.owner = storeValueOwner;
const storedAttributes = nftData.attributesArray;
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const receivedAttributes = decodedValue!.attributesArray;
nft.attributesArray = this._internalMethod.getNewAttributes(
nftData.attributesArray = this._internalMethod.getNewAttributes(
nftID,
storedAttributes,
receivedAttributes,
);
await nftStore.save(methodContext, nftID, nft);
await this._internalMethod.createUserEntry(methodContext, nft.owner, nftID);
await nftStore.save(methodContext, nftID, nftData);
await this._internalMethod.createUserEntry(methodContext, nftData.owner, nftID);
await escrowStore.del(methodContext, escrowStore.getKey(terminatedChainID, nftID));

this.events.get(RecoverEvent).log(methodContext, {
Expand Down
111 changes: 67 additions & 44 deletions framework/test/unit/modules/nft/method.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1583,17 +1583,17 @@ describe('NFTMethod', () => {
});

describe('recover', () => {
const terminatedChainID = utils.getRandomBytes(LENGTH_CHAIN_ID);
const terminatedChainID = Buffer.alloc(LENGTH_CHAIN_ID, 8);
const substorePrefix = Buffer.from('0000', 'hex');
const storeKey = utils.getRandomBytes(LENGTH_NFT_ID);
const storeValue = codec.encode(nftStoreSchema, {
const newNftID = Buffer.alloc(LENGTH_NFT_ID, 1);
const nft = codec.encode(nftStoreSchema, {
owner: utils.getRandomBytes(LENGTH_CHAIN_ID),
attributesArray: [],
});

it('should throw and emit error recover event if substore prefix is not valid', async () => {
await expect(
method.recover(methodContext, terminatedChainID, Buffer.alloc(2, 2), storeKey, storeValue),
method.recover(methodContext, terminatedChainID, Buffer.alloc(2, 2), nftID, nft),
).rejects.toThrow('Invalid inputs');

checkEventResult<RecoverEventData>(
Expand All @@ -1603,17 +1603,17 @@ describe('NFTMethod', () => {
0,
{
terminatedChainID,
nftID: storeKey,
nftID,
},
NftEventResult.RESULT_RECOVER_FAIL_INVALID_INPUTS,
);
});

it('should throw and emit error recover event if store key length is not valid', async () => {
const newStoreKey = utils.getRandomBytes(LENGTH_NFT_ID + 1);
it('should throw and emit error recover event if NFT ID length is not valid', async () => {
const invalidNftID = utils.getRandomBytes(LENGTH_NFT_ID + 1);

await expect(
method.recover(methodContext, terminatedChainID, substorePrefix, newStoreKey, storeValue),
method.recover(methodContext, terminatedChainID, substorePrefix, invalidNftID, nft),
).rejects.toThrow('Invalid inputs');
checkEventResult<RecoverEventData>(
methodContext.eventQueue,
Expand All @@ -1622,19 +1622,19 @@ describe('NFTMethod', () => {
0,
{
terminatedChainID,
nftID: newStoreKey,
nftID: invalidNftID,
},
NftEventResult.RESULT_RECOVER_FAIL_INVALID_INPUTS,
);
});

it('should throw and emit error recover event if store value is not valid', async () => {
it('should throw and emit error recover event if NFT is not valid', async () => {
await expect(
method.recover(
methodContext,
terminatedChainID,
substorePrefix,
storeKey,
nftID,
Buffer.from('asfas'),
),
).rejects.toThrow('Invalid inputs');
Expand All @@ -1646,7 +1646,7 @@ describe('NFTMethod', () => {
0,
{
terminatedChainID,
nftID: storeKey,
nftID,
},
NftEventResult.RESULT_RECOVER_FAIL_INVALID_INPUTS,
);
Expand All @@ -1662,7 +1662,7 @@ describe('NFTMethod', () => {
});

await expect(
method.recover(methodContext, terminatedChainID, substorePrefix, storeKey, newStoreValue),
method.recover(methodContext, terminatedChainID, substorePrefix, nftID, newStoreValue),
).rejects.toThrow('Invalid inputs');
checkEventResult<RecoverEventData>(
methodContext.eventQueue,
Expand All @@ -1671,15 +1671,25 @@ describe('NFTMethod', () => {
0,
{
terminatedChainID,
nftID: storeKey,
nftID,
},
NftEventResult.RESULT_RECOVER_FAIL_INVALID_INPUTS,
);
});

it('should throw and emit error recover event if nft chain id is not same as own chain id', async () => {
// ensure that random NFT is on a different chain than ownChainID
const randomNftID = Buffer.concat([
Buffer.alloc(LENGTH_CHAIN_ID, 9),
utils.getRandomBytes(LENGTH_NFT_ID - LENGTH_CHAIN_ID),
]);
await nftStore.save(methodContext, randomNftID, {
owner: utils.getRandomBytes(LENGTH_ADDRESS),
attributesArray: [],
});

await expect(
method.recover(methodContext, terminatedChainID, substorePrefix, storeKey, storeValue),
method.recover(methodContext, terminatedChainID, substorePrefix, randomNftID, nft),
).rejects.toThrow('Recovery called by a foreign chain');

checkEventResult<RecoverEventData>(
Expand All @@ -1689,21 +1699,42 @@ describe('NFTMethod', () => {
0,
{
terminatedChainID,
nftID: storeKey,
nftID: randomNftID,
},
NftEventResult.RESULT_INITIATED_BY_NONNATIVE_CHAIN,
);
});

it('should throw and emit error recover event if nft does not exist', async () => {
const unknownNftID = Buffer.concat([
config.ownChainID,
utils.getRandomBytes(LENGTH_NFT_ID - LENGTH_CHAIN_ID),
]);

await expect(
method.recover(methodContext, terminatedChainID, substorePrefix, unknownNftID, nft),
).rejects.toThrow('NFT substore entry does not exist');
checkEventResult<RecoverEventData>(
methodContext.eventQueue,
1,
RecoverEvent,
0,
{
terminatedChainID,
nftID: unknownNftID,
},
NftEventResult.RESULT_NFT_DOES_NOT_EXIST,
);
});

it('should throw and emit error recover event if nft is not escrowed to terminated chain', async () => {
const newStoreKey = Buffer.alloc(LENGTH_NFT_ID, 1);
await nftStore.save(methodContext, newStoreKey, {
await nftStore.save(methodContext, newNftID, {
owner: utils.getRandomBytes(LENGTH_CHAIN_ID),
attributesArray: [],
});

await expect(
method.recover(methodContext, terminatedChainID, substorePrefix, newStoreKey, storeValue),
method.recover(methodContext, terminatedChainID, substorePrefix, newNftID, nft),
).rejects.toThrow('NFT was not escrowed to terminated chain');

checkEventResult<RecoverEventData>(
Expand All @@ -1713,21 +1744,20 @@ describe('NFTMethod', () => {
0,
{
terminatedChainID,
nftID: newStoreKey,
nftID: newNftID,
},
NftEventResult.RESULT_NFT_NOT_ESCROWED,
);
});

it('should throw and emit error recover event if store value owner length is invalid', async () => {
const newStoreKey = Buffer.alloc(LENGTH_NFT_ID, 1);
await nftStore.save(methodContext, newStoreKey, {
it('should throw and emit error recover event if NFT owner length is invalid', async () => {
await nftStore.save(methodContext, newNftID, {
owner: terminatedChainID,
attributesArray: [],
});

await expect(
method.recover(methodContext, terminatedChainID, substorePrefix, newStoreKey, storeValue),
method.recover(methodContext, terminatedChainID, substorePrefix, newNftID, nft),
).rejects.toThrow('Invalid account information');

checkEventResult<RecoverEventData>(
Expand All @@ -1737,33 +1767,26 @@ describe('NFTMethod', () => {
0,
{
terminatedChainID,
nftID: newStoreKey,
nftID: newNftID,
},
NftEventResult.RESULT_INVALID_ACCOUNT,
);
});

it('should set appropriate values to stores and resolve with emitting success recover event if params are valid', async () => {
const newStoreKey = Buffer.alloc(LENGTH_NFT_ID, 1);
const storeValueOwner = utils.getRandomBytes(LENGTH_ADDRESS);
const newStoreValue = codec.encode(nftStoreSchema, {
owner: storeValueOwner,
const nftOwner = utils.getRandomBytes(LENGTH_ADDRESS);
const newNft = codec.encode(nftStoreSchema, {
owner: nftOwner,
attributesArray: [],
});
await nftStore.save(methodContext, newStoreKey, {
await nftStore.save(methodContext, newNftID, {
owner: terminatedChainID,
attributesArray: [],
});
jest.spyOn(internalMethod, 'createUserEntry');

await expect(
method.recover(
methodContext,
terminatedChainID,
substorePrefix,
newStoreKey,
newStoreValue,
),
method.recover(methodContext, terminatedChainID, substorePrefix, newNftID, newNft),
).resolves.toBeUndefined();

checkEventResult<RecoverEventData>(
Expand All @@ -1773,22 +1796,22 @@ describe('NFTMethod', () => {
0,
{
terminatedChainID,
nftID: newStoreKey,
nftID: newNftID,
},
NftEventResult.RESULT_SUCCESSFUL,
);
const nftStoreData = await nftStore.get(methodContext, newStoreKey);
const retrievedNft = await nftStore.get(methodContext, newNftID);
const escrowStore = module.stores.get(EscrowStore);
const escrowAccountExists = await escrowStore.has(
methodContext,
escrowStore.getKey(terminatedChainID, newStoreKey),
escrowStore.getKey(terminatedChainID, newNftID),
);
expect(nftStoreData.owner).toStrictEqual(storeValueOwner);
expect(nftStoreData.attributesArray).toEqual([]);
expect(retrievedNft.owner).toStrictEqual(nftOwner);
expect(retrievedNft.attributesArray).toEqual([]);
expect(internalMethod['createUserEntry']).toHaveBeenCalledWith(
methodContext,
storeValueOwner,
newStoreKey,
nftOwner,
newNftID,
);
expect(escrowAccountExists).toBe(false);
});
Expand Down

0 comments on commit 2eabd20

Please sign in to comment.