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

transferCrossChainInternal Method for NFT Module #8520

Merged

Conversation

has5aan
Copy link
Contributor

@has5aan has5aan commented May 31, 2023

What was the problem?

This PR resolves #8493 #8491

How was it solved?

Implements InternalMethod.transferCrossChainInternal

How was it tested?

Implemented unit tests.

@has5aan has5aan requested review from Incede and mjerkov May 31, 2023 14:22
@has5aan has5aan self-assigned this May 31, 2023
@has5aan has5aan changed the title transferCrossChainInternal NFT Module transferCrossChainInternal Method for NFT Module May 31, 2023
@codecov
Copy link

codecov bot commented May 31, 2023

Codecov Report

Merging #8520 (d3e98a4) into feature/6917-implement-nft-module (e9e7f2e) will increase coverage by 0.08%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@                          Coverage Diff                          @@
##           feature/6917-implement-nft-module    #8520      +/-   ##
=====================================================================
+ Coverage                              83.23%   83.32%   +0.08%     
=====================================================================
  Files                                    621      621              
  Lines                                  22544    22604      +60     
  Branches                                3258     3268      +10     
=====================================================================
+ Hits                                   18765    18834      +69     
+ Misses                                  3779     3770       -9     
Impacted Files Coverage Δ
framework/src/modules/nft/constants.ts 100.00% <100.00%> (ø)
framework/src/modules/nft/events/destroy.ts 100.00% <100.00%> (+28.57%) ⬆️
framework/src/modules/nft/internal_method.ts 100.00% <100.00%> (+18.51%) ⬆️
framework/src/modules/nft/method.ts 92.00% <100.00%> (+9.39%) ⬆️
framework/src/modules/nft/schemas.ts 100.00% <100.00%> (ø)
framework/src/modules/nft/stores/escrow.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@has5aan has5aan force-pushed the 8493_nft_transferCrossChainInternal branch from e0f76ce to 720548b Compare June 1, 2023 12:47
Base automatically changed from 8393_nft_transfer_command to feature/6917-implement-nft-module June 1, 2023 14:46
@has5aan has5aan force-pushed the 8493_nft_transferCrossChainInternal branch from 720548b to e78c475 Compare June 1, 2023 17:12
Copy link
Contributor

@mjerkov mjerkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, I left just some minor comments or suggestions, but nothing too important I guess.

framework/src/modules/nft/method.ts Outdated Show resolved Hide resolved
framework/src/modules/nft/method.ts Outdated Show resolved Hide resolved
framework/src/modules/nft/method.ts Outdated Show resolved Hide resolved
framework/src/modules/nft/stores/escrow.ts Show resolved Hide resolved
framework/src/modules/nft/schemas.ts Outdated Show resolved Hide resolved
framework/test/unit/modules/nft/method.spec.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@mjerkov mjerkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

framework/src/modules/nft/method.ts Outdated Show resolved Hide resolved
framework/test/unit/modules/nft/method.spec.ts Outdated Show resolved Hide resolved
framework/test/unit/modules/nft/method.spec.ts Outdated Show resolved Hide resolved
@has5aan has5aan merged commit 0ae50b1 into feature/6917-implement-nft-module Jun 7, 2023
@has5aan has5aan deleted the 8493_nft_transferCrossChainInternal branch June 7, 2023 10:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants