Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(nft): db implementation (wasm target) #1877

Merged
merged 136 commits into from Jul 6, 2023
Merged

Conversation

laruh
Copy link
Member

@laruh laruh commented Jun 22, 2023

#900

  • IndexedDb support

  • db tests

  • handle the ipfs moralis links with bafy hash:
    check_moralis_ipfs_bafy function. it checks for ipfs moralis links whose hash starts with bafy. If it does, then we need to use a different gateway (e.g. ipfs.io) to handle those links. If token_uri, image or animation_url contain such link, then we change it to https://ipfs.io/ipfs/ (related to)
    image

  • get UriMeta from token_uri and from metadata:
    get_uri_meta function. it tries to get uri meta from token_uri, then tries to get uri meta from metadata field, then merges it into uri meta from token_uri. As a result it is more likely that image link will be not null. We use the resulting UriMeta for both NFT and history (related to)

  • protect_from_spam feature.
    If protect_from_spam param is true in get_nft_list, get_nft_metadata, get_nft_transfers reqs, then we will search urls in fields like collection_name, token_name, symbol. if they have it, then the text in these parameters will be replaced by URL redacted for user protection, and possible_spam in Nft or NftTransferHistory structures will be true

Dependencies added

regex = "1" in mm2src/coins/Cargo.toml

@laruh laruh added under review and removed blocked requires additional attention in progress Changes will be made from the author labels Jun 30, 2023
@onur-ozkan
Copy link
Member

if user already has this erc721 token in db, its impossible to get one more incidental erc721 token, its an error.

Can you clarify this problem? Is it fixed? What's the actual problem behind it, primary key conflicts?

@laruh
Copy link
Member Author

laruh commented Jul 3, 2023

if user already has this erc721 token in db, its impossible to get one more incidental erc721 token, its an error.

Can you clarify this problem? Is it fixed? What's the actual problem behind it, primary key conflicts?

Its not the problem, I just added the case when user sends erc721 token to themself in handle_receive_erc721.
You can check tx_hash: 0x0087e763a5b7fc61e906341f70e06e7881862cb71bc42d32e2d7485e56bcbc63 here.
For this I also needed to add a check if the sender's address doesnt match the user's address, when the user already has such erc721 token. In theory it is impossible to get two identical erc721 tokens. The exception is when you send 721 NFT to yourself, in that case it simply changes the block height.

shamardy
shamardy previously approved these changes Jul 3, 2023
@shamardy
Copy link
Collaborator

shamardy commented Jul 3, 2023

@ozkanonur @borngraced do you have any more comments/reviews regarding this PR? If not, I guess I can merge it.

borngraced
borngraced previously approved these changes Jul 3, 2023
Copy link
Member

@borngraced borngraced left a comment

Choose a reason for hiding this comment

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

LGTM 🔥

Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

Thanks for the previous fixes.

Some more notes and questions from my side

Comment on lines 669 to 676
let tx_meta = TxMeta {
token_address: nft_db.token_address,
token_id: nft_db.token_id,
token_uri: nft_db.token_uri,
collection_name: nft_db.collection_name,
image_url: nft_db.uri_meta.image_url,
token_name: nft_db.uri_meta.token_name,
};
Copy link
Member

Choose a reason for hiding this comment

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

I see this block is duplicated frequently, implementing From trait on this would be useful

Copy link
Member Author

Choose a reason for hiding this comment

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

done

mm2src/coins/nft/storage/wasm/wasm_storage.rs Outdated Show resolved Hide resolved
mm2src/coins/nft/nft_structs.rs Outdated Show resolved Hide resolved
mm2src/coins/nft.rs Outdated Show resolved Hide resolved
mm2src/coins/nft.rs Outdated Show resolved Hide resolved
@laruh laruh added in progress Changes will be made from the author and removed under review labels Jul 4, 2023
@laruh laruh dismissed stale reviews from borngraced and shamardy via 345cf5f July 5, 2023 13:35
@laruh laruh added under review and removed in progress Changes will be made from the author labels Jul 6, 2023
Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

LGTM

@shamardy shamardy merged commit 6ddb44d into dev Jul 6, 2023
30 of 34 checks passed
@shamardy shamardy deleted the nft-cache-support-wasm branch July 6, 2023 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants