feat: add NFT descriptor for a subgraph

Author: abarmat

contracts/base/ISubgraphNFTDescriptor.sol

@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+pragma solidity ^0.7.6;
+
+import "../discovery/IGNS.sol";
+
+/// @title Describes subgraph NFT tokens via URI
+interface ISubgraphNFTDescriptor {
+    /// @notice Produces the URI describing a particular token ID for a Subgraph
+    /// @dev Note this URI may be a data: URI with the JSON contents directly inlined
+    /// @param _gns GNS contract that holds the Subgraph data
+    /// @param _subgraphID The ID of the subgraph NFT for which to produce a description, which may not be valid
+    /// @return The URI of the ERC721-compliant metadata
+    function tokenURI(IGNS _gns, uint256 _subgraphID) external view returns (string memory);
+}

contracts/base/SubgraphNFT.sol

@@ -1,9 +1,38 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-pragma solidity ^0.7.4;
+pragma solidity ^0.7.6;
 
 import "@openzeppelin/contracts-upgradeable/token/ERC721/ERC721Upgradeable.sol";
 
+import "./ISubgraphNFTDescriptor.sol";
+
 abstract contract SubgraphNFT is ERC721Upgradeable {
-    function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {}
+    ISubgraphNFTDescriptor public tokenDescriptor;
+
+    // -- Events --
+
+    event TokenDescriptorUpdated(address tokenDescriptor);
+
+    // -- Functions --
+
+    /**
+     * @dev Initializes the contract by setting a `name`, `symbol` and `descriptor` to the token collection.
+     */
+    function __SubgraphNFT_init(address _tokenDescriptor) internal initializer {
+        __ERC721_init("Subgraph", "SG");
+        _setTokenDescriptor(address(_tokenDescriptor));
+    }
+
+    /**
+     * @dev Set the token descriptor contract used to create the ERC-721 metadata URI
+     * @param _tokenDescriptor Address of the contract that creates the NFT token URI
+     */
+    function _setTokenDescriptor(address _tokenDescriptor) internal {
+        require(
+            _tokenDescriptor != address(0) && AddressUpgradeable.isContract(_tokenDescriptor),
+            "NFT: Invalid token descriptor"
+        );
+        tokenDescriptor = ISubgraphNFTDescriptor(_tokenDescriptor);
+        emit TokenDescriptorUpdated(_tokenDescriptor);
+    }
 }

contracts/base/SubgraphNFTDescriptor.sol

@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+pragma solidity ^0.7.6;
+
+import "./ISubgraphNFTDescriptor.sol";
+
+/// @title Describes subgraph NFT tokens via URI
+contract SubgraphNFTDescriptor is ISubgraphNFTDescriptor {
+    /// @inheritdoc ISubgraphNFTDescriptor
+    function tokenURI(IGNS _gns, uint256 _subgraphID)
+        external
+        view
+        override
+        returns (string memory)
+    {
+        // TODO: fancy implementation
+        // uint256 signal = _gns.subgraphSignal(_subgraphID);
+        // uint256 tokens = _gns.subgraphTokens(_subgraphID);
+        // id
+        // owner
+        return "";
+    }
+}

contracts/discovery/GNS.sol

@@ -6,6 +6,7 @@ pragma abicoder v2;
 import "@openzeppelin/contracts/math/SafeMath.sol";
 
 import "../base/Multicall.sol";
+import "../base/SubgraphNFT.sol";
 import "../bancor/BancorFormula.sol";
 import "../upgrades/GraphUpgradeable.sol";
 import "../utils/TokenUtils.sol";
@@ -139,12 +140,16 @@ contract GNS is GNSV2Storage, GraphUpgradeable, IGNS, Multicall {
     /**
      * @dev Initialize this contract.
      */
-    function initialize(address _controller, address _bondingCurve) external onlyImpl {
+    function initialize(
+        address _controller,
+        address _bondingCurve,
+        address _tokenDescriptor
+    ) external onlyImpl {
         Managed._initialize(_controller);
 
+        // Dependencies
         bondingCurve = _bondingCurve;
-        // TODO: review token symbol
-        __ERC721_init("Subgraph", "SUB");
+        __SubgraphNFT_init(_tokenDescriptor);
 
         // Settings
         _setOwnerTaxPercentage(500000);
@@ -166,6 +171,14 @@ contract GNS is GNSV2Storage, GraphUpgradeable, IGNS, Multicall {
         _setOwnerTaxPercentage(_ownerTaxPercentage);
     }
 
+    /**
+     * @dev Set the token descriptor contract.
+     * @param _tokenDescriptor Address of the contract that creates the NFT token URI
+     */
+    function setTokenDescriptor(address _tokenDescriptor) external override onlyGovernor {
+        _setTokenDescriptor(_tokenDescriptor);
+    }
+
     /**
      * @dev Internal: Set the owner tax percentage. This is used to prevent a subgraph owner to drain all
      * the name curators tokens while upgrading or deprecating and is configurable in parts per hundred.
@@ -598,6 +611,38 @@ contract GNS is GNSV2Storage, GraphUpgradeable, IGNS, Multicall {
         return _getSubgraphData(_subgraphID).curatorNSignal[_curator];
     }
 
+    /**
+     * @dev Return the total signal on the subgraph.
+     * @param _subgraphID Subgraph ID
+     * @return Total signal on the subgraph
+     */
+    function subgraphSignal(uint256 _subgraphID) external view override returns (uint256) {
+        return _getSubgraphData(_subgraphID).nSignal;
+    }
+
+    /**
+     * @dev Return the total tokens on the subgraph at current value.
+     * @param _subgraphID Subgraph ID
+     * @return Total tokens on the subgraph
+     */
+    function subgraphTokens(uint256 _subgraphID) external view override returns (uint256) {
+        uint256 signal = _getSubgraphData(_subgraphID).nSignal;
+        if (signal > 0) {
+            (, uint256 tokens) = nSignalToTokens(_subgraphID, signal);
+            return tokens;
+        }
+        return 0;
+    }
+
+    /**
+     * @dev Return the URI describing a particular token ID for a Subgraph.
+     * @param _subgraphID Subgraph ID
+     * @return The URI of the ERC721-compliant metadata
+     */
+    function tokenURI(uint256 _subgraphID) public view override returns (string memory) {
+        return tokenDescriptor.tokenURI(this, _subgraphID);
+    }
+
     /**
      * @dev Create subgraphID for legacy subgraph and mint ownership NFT.
      * @param _graphAccount Account that created the subgraph

contracts/discovery/GNSStorage.sol

@@ -38,8 +38,6 @@ abstract contract GNSV1Storage is Managed {
 }
 
 abstract contract GNSV2Storage is GNSV1Storage, SubgraphNFT {
-    // TODO: review order of storage
-
     // Use it whenever a legacy (v1) subgraph NFT was claimed to maintain compatibility
     // Keep a reference from subgraphID => (graphAccount, subgraphNumber)
     mapping(uint256 => IGNS.LegacySubgraphKey) public legacySubgraphKeys;

contracts/discovery/IGNS.sol

@@ -26,6 +26,8 @@ interface IGNS {
 
     function setOwnerTaxPercentage(uint32 _ownerTaxPercentage) external;
 
+    function setTokenDescriptor(address _tokenDescriptor) external;
+
     // -- Publishing --
 
     function setDefaultName(
@@ -69,6 +71,10 @@ interface IGNS {
 
     // -- Getters --
 
+    function subgraphSignal(uint256 _subgraphID) external view returns (uint256);
+
+    function subgraphTokens(uint256 _subgraphID) external view returns (uint256);
+
     function tokensToNSignal(uint256 _subgraphID, uint256 _tokensIn)
         external
         view

test/gns.test.ts

@@ -10,6 +10,8 @@ import { getAccounts, randomHexBytes, Account, toGRT } from './lib/testHelpers'
 import { NetworkFixture } from './lib/fixtures'
 import { toBN, formatGRT } from './lib/testHelpers'
 
+const { AddressZero } = ethers.constants
+
 // Entities
 interface PublishSubgraph {
   subgraphDeploymentID: string
@@ -490,6 +492,47 @@ describe('GNS', () => {
     await fixture.tearDown()
   })
 
+  describe('Configuration', async function () {
+    describe('setOwnerTaxPercentage', function () {
+      const newValue = 10
+
+      it('should set `ownerTaxPercentage`', async function () {
+        // Can set if allowed
+        await gns.connect(governor.signer).setOwnerTaxPercentage(newValue)
+        expect(await gns.ownerTaxPercentage()).eq(newValue)
+      })
+
+      it('reject set `ownerTaxPercentage` if out of bounds', async function () {
+        const tx = gns.connect(governor.signer).setOwnerTaxPercentage(1000001)
+        await expect(tx).revertedWith('Owner tax must be MAX_PPM or less')
+      })
+
+      it('reject set `ownerTaxPercentage` if not allowed', async function () {
+        const tx = gns.connect(me.signer).setOwnerTaxPercentage(newValue)
+        await expect(tx).revertedWith('Caller must be Controller governor')
+      })
+    })
+
+    describe('setTokenDescriptor', function () {
+      it('should set `tokenDescriptor`', async function () {
+        const newTokenDescriptor = gns.address // I just use any contract address
+        const tx = gns.connect(governor.signer).setTokenDescriptor(newTokenDescriptor)
+        await expect(tx).emit(gns, 'TokenDescriptorUpdated').withArgs(newTokenDescriptor)
+        expect(await gns.tokenDescriptor()).eq(newTokenDescriptor)
+      })
+
+      it('revert set to empty address', async function () {
+        const tx = gns.connect(governor.signer).setTokenDescriptor(AddressZero)
+        await expect(tx).revertedWith('NFT: Invalid token descriptor')
+      })
+
+      it('revert set to non-contract', async function () {
+        const tx = gns.connect(governor.signer).setTokenDescriptor(randomHexBytes(20))
+        await expect(tx).revertedWith('NFT: Invalid token descriptor')
+      })
+    })
+  })
+
   describe('Publishing names and versions', function () {
     describe('setDefaultName', function () {
       it('setDefaultName emits the event', async function () {
@@ -865,26 +908,6 @@ describe('GNS', () => {
           await mintSignal(me, subgraph.id, tokensToDeposit)
         }
       })
-
-      describe('setOwnerTaxPercentage', function () {
-        const newValue = 10
-
-        it('should set `ownerTaxPercentage`', async function () {
-          // Can set if allowed
-          await gns.connect(governor.signer).setOwnerTaxPercentage(newValue)
-          expect(await gns.ownerTaxPercentage()).eq(newValue)
-        })
-
-        it('reject set `ownerTaxPercentage` if out of bounds', async function () {
-          const tx = gns.connect(governor.signer).setOwnerTaxPercentage(1000001)
-          await expect(tx).revertedWith('Owner tax must be MAX_PPM or less')
-        })
-
-        it('reject set `ownerTaxPercentage` if not allowed', async function () {
-          const tx = gns.connect(me.signer).setOwnerTaxPercentage(newValue)
-          await expect(tx).revertedWith('Caller must be Controller governor')
-        })
-      })
     })
   })
 
@@ -938,7 +961,7 @@ describe('GNS', () => {
 
     it('should revert if batching a call to initialize', async function () {
       // Call a forbidden function
-      const tx1 = await gns.populateTransaction.initialize(me.address, me.address)
+      const tx1 = await gns.populateTransaction.initialize(me.address, me.address, me.address)
 
       // Create a subgraph
       const tx2 = await gns.populateTransaction.publishNewSubgraph(

test/lib/deployment.ts

@@ -175,12 +175,13 @@ export async function deployGNS(
 ): Promise<GNS> {
   // Dependency
   const bondingCurve = (await deployContract('BancorFormula', deployer)) as unknown as BancorFormula
+  const subgraphDescriptor = await deployContract('SubgraphNFTDescriptor', deployer)
 
   // Deploy
   return network.deployContractWithProxy(
     proxyAdmin,
     'GNS',
-    [controller, bondingCurve.address],
+    [controller, bondingCurve.address, subgraphDescriptor.address],
     deployer,
   ) as unknown as GNS
 }