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

Refactor Subgraph NFT using composition #527

Merged
merged 4 commits into from
Jan 21, 2022
Merged

Conversation

abarmat
Copy link
Contributor

@abarmat abarmat commented Dec 3, 2021

Summary

In the current implementation the GNS was turned into a NFT registry by adding the related state variables and behaviours through an upgrade. The upgrade is not yet done in mainnet. While testing in Rinkeby the team identified that some platforms (OpenSea, Etherscan, Rarible, etc.) couldn't properly identify a contract that was previously deployed as a non-NFT that was upgraded to be an NFT registry. That means that they won't display the NFT metadata properly.

The original implementation was done in the following PR: #497

Solution

Use composition instead of inheritance to add the NFT functionality.

@abarmat abarmat changed the title feat: update nft descriptor Update NFT descriptor Dec 8, 2021
@abarmat abarmat force-pushed the ariel/gns-nft-descriptor branch from 5ac45c7 to 760a7ea Compare December 13, 2021 06:27
@abarmat abarmat marked this pull request as ready for review December 13, 2021 06:27
@abarmat abarmat changed the title Update NFT descriptor Refactor Subgraph NFT using composition Dec 13, 2021
@abarmat abarmat requested a review from jona December 13, 2021 15:37
@abarmat abarmat force-pushed the ariel/gns-nft-descriptor branch from 6bb2ed1 to 0a3888d Compare January 10, 2022 16:05
@codecov
Copy link

codecov bot commented Jan 10, 2022

Codecov Report

Merging #527 (cdac3a2) into dev (2cf753f) will increase coverage by 0.59%.
The diff coverage is 97.16%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #527      +/-   ##
==========================================
+ Coverage   90.60%   91.19%   +0.59%     
==========================================
  Files          35       37       +2     
  Lines        1703     1795      +92     
  Branches      282      293      +11     
==========================================
+ Hits         1543     1637      +94     
+ Misses        160      158       -2     
Flag Coverage Δ
unittests 91.19% <97.16%> (+0.59%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
contracts/discovery/GNS.sol 88.57% <88.88%> (+1.45%) ⬆️
contracts/libraries/HexStrings.sol 94.11% <94.11%> (ø)
contracts/curation/Curation.sol 100.00% <100.00%> (ø)
contracts/curation/GraphCurationToken.sol 100.00% <100.00%> (ø)
contracts/discovery/SubgraphNFT.sol 100.00% <100.00%> (ø)
contracts/discovery/SubgraphNFTDescriptor.sol 100.00% <100.00%> (ø)
contracts/libraries/Base58Encoder.sol 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9d3c48...cdac3a2. Read the comment docs.

@abarmat abarmat force-pushed the ariel/gns-nft-descriptor branch from 2ac1aa8 to cdac3a2 Compare January 21, 2022 14:55
@abarmat abarmat merged commit a756b43 into dev Jan 21, 2022
abarmat added a commit that referenced this pull request Jan 24, 2022
- Refactor Subgraph NFT transfewr using composition (#527)
@abarmat abarmat deleted the ariel/gns-nft-descriptor branch December 21, 2022 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant