Merge pull request #1215 from graphprotocol/tmigone/register-fix

Author: tmigone

packages/interfaces/contracts/subgraph-service/ISubgraphService.sol

@@ -20,12 +20,10 @@ import { ILegacyAllocation } from "./internal/ILegacyAllocation.sol";
 interface ISubgraphService is IDataServiceFees {
     /**
      * @notice Indexer details
-     * @param registeredAt The timestamp when the indexer registered
      * @param url The URL where the indexer can be reached at for queries
      * @param geoHash The indexer's geo location, expressed as a geo hash
      */
     struct Indexer {
-        uint256 registeredAt;
         string url;
         string geoHash;
     }
@@ -83,11 +81,6 @@ interface ISubgraphService is IDataServiceFees {
      */
     error SubgraphServiceEmptyGeohash();
 
-    /**
-     * @notice Thrown when an indexer tries to register but they are already registered
-     */
-    error SubgraphServiceIndexerAlreadyRegistered();
-
     /**
      * @notice Thrown when an indexer tries to perform an operation but they are not registered
      * @param indexer The address of the indexer that is not registered

packages/interfaces/contracts/toolshed/ISubgraphServiceToolshed.sol

@@ -27,13 +27,10 @@ interface ISubgraphServiceToolshed is
      * @dev Note that this storage getter actually returns a ISubgraphService.Indexer struct, but ethers v6 is not
      *      good at dealing with dynamic types on return values.
      * @param indexer The address of the indexer
-     * @return registeredAt The timestamp when the indexer registered
      * @return url The URL where the indexer can be reached at for queries
      * @return geoHash The indexer's geo location, expressed as a geo hash
      */
-    function indexers(
-        address indexer
-    ) external view returns (uint256 registeredAt, string memory url, string memory geoHash);
+    function indexers(address indexer) external view returns (string memory url, string memory geoHash);
 
     /**
      * @notice Gets the allocation provision tracker

packages/subgraph-service/contracts/SubgraphService.sol

@@ -13,9 +13,7 @@ import { ILegacyAllocation } from "@graphprotocol/interfaces/contracts/subgraph-
 import { OwnableUpgradeable } from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
 import { MulticallUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/MulticallUpgradeable.sol";
 import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
-import {
-    DataServicePausableUpgradeable
-} from "@graphprotocol/horizon/contracts/data-service/extensions/DataServicePausableUpgradeable.sol";
+import { DataServicePausableUpgradeable } from "@graphprotocol/horizon/contracts/data-service/extensions/DataServicePausableUpgradeable.sol";
 import { DataService } from "@graphprotocol/horizon/contracts/data-service/DataService.sol";
 import { DataServiceFees } from "@graphprotocol/horizon/contracts/data-service/extensions/DataServiceFees.sol";
 import { Directory } from "./utilities/Directory.sol";
@@ -54,7 +52,7 @@ contract SubgraphService is
      * @param indexer The address of the indexer
      */
     modifier onlyRegisteredIndexer(address indexer) {
-        require(indexers[indexer].registeredAt != 0, SubgraphServiceIndexerNotRegistered(indexer));
+        require(bytes(indexers[indexer].url).length > 0, SubgraphServiceIndexerNotRegistered(indexer));
         _;
     }
 
@@ -98,7 +96,6 @@ contract SubgraphService is
      * @dev Implements {IDataService.register}
      *
      * Requirements:
-     * - The indexer must not be already registered
      * - The URL must not be empty
      * - The provision must be valid according to the subgraph service rules
      *
@@ -123,13 +120,10 @@ contract SubgraphService is
 
         require(bytes(url).length > 0, SubgraphServiceEmptyUrl());
         require(bytes(geohash).length > 0, SubgraphServiceEmptyGeohash());
-        require(indexers[indexer].registeredAt == 0, SubgraphServiceIndexerAlreadyRegistered());
 
         // Register the indexer
-        indexers[indexer] = Indexer({ registeredAt: block.timestamp, url: url, geoHash: geohash });
-        if (paymentsDestination_ != address(0)) {
-            _setPaymentsDestination(indexer, paymentsDestination_);
-        }
+        indexers[indexer] = Indexer({ url: url, geoHash: geohash });
+        _setPaymentsDestination(indexer, paymentsDestination_);
 
         emit ServiceProviderRegistered(indexer, data);
     }

packages/subgraph-service/scripts/ops/protocol-activity.ts

@@ -110,7 +110,7 @@ async function main() {
   console.log('📝 Subgraph Service - registering...')
   for (const signer of signers) {
     const indexer = await SubgraphService.indexers(signer.address)
-    const isRegistered = indexer.registeredAt !== 0n
+    const isRegistered = indexer.url.length > 0
     if (!isRegistered) {
       const paymentsDestination = Math.random() < 0.5 ? signer.address : ethers.ZeroAddress
       const data = abi.encode(['string', 'string', 'address'], ['http://indexer.xyz', '69y7mznpp', paymentsDestination])

packages/subgraph-service/tasks/test/seed.ts

@@ -93,7 +93,7 @@ task('test:seed', 'Seed the test environment, must be run after deployment').set
 
     const indexerData = await subgraphService.indexers(indexerSigner.address)
 
-    console.log(`Indexer registered at: ${indexerData.registeredAt}`)
+    console.log(`Indexer registered at: ${indexerData.url} - ${indexerData.geoHash}`)
   }
 
   console.log('\n--- STEP 3: Start allocations ---')

packages/subgraph-service/test/unit/shared/SubgraphServiceShared.t.sol

@@ -87,7 +87,6 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest {
 
         // Check registered indexer data
         ISubgraphService.Indexer memory indexer = _getIndexer(_indexer);
-        assertEq(indexer.registeredAt, block.timestamp);
         assertEq(indexer.url, url);
         assertEq(indexer.geoHash, geohash);
 
@@ -197,7 +196,7 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest {
      */
 
     function _getIndexer(address _indexer) private view returns (ISubgraphService.Indexer memory) {
-        (uint256 registeredAt, string memory url, string memory geoHash) = subgraphService.indexers(_indexer);
-        return ISubgraphService.Indexer({ registeredAt: registeredAt, url: url, geoHash: geoHash });
+        (string memory url, string memory geoHash) = subgraphService.indexers(_indexer);
+        return ISubgraphService.Indexer({ url: url, geoHash: geoHash });
     }
 }

packages/subgraph-service/test/unit/subgraphService/provider/register.t.sol

@@ -19,12 +19,18 @@ contract SubgraphServiceProviderRegisterTest is SubgraphServiceTest {
         _register(users.indexer, data);
     }
 
-    function test_SubgraphService_Provider_Register_RevertIf_AlreadyRegistered(
+    function test_SubgraphService_Provider_Register_MultipleTimes(
         uint256 tokens
     ) public useIndexer useAllocation(tokens) {
-        vm.expectRevert(abi.encodeWithSelector(ISubgraphService.SubgraphServiceIndexerAlreadyRegistered.selector));
         bytes memory data = abi.encode("url", "geoHash", users.rewardsDestination);
-        subgraphService.register(users.indexer, data);
+        _register(users.indexer, data);
+
+        bytes memory data2 = abi.encode("url2", "geoHash2", users.rewardsDestination);
+        _register(users.indexer, data2);
+
+        (string memory url, string memory geoHash) = subgraphService.indexers(users.indexer);
+        assertEq(url, "url2");
+        assertEq(geoHash, "geoHash2");
     }
 
     function test_SubgraphService_Provider_Register_RevertWhen_InvalidProvision() public useIndexer {