fix: remove unused errors, use LegacyAllocationAlreadyMigrated to prevent migrating a legacy allocation twice (OZ N-02) (#1029)

* fix: remove AllocationManagerZeroTokensAllocation. (OZ N-02)

* fix: remove DataServiceFeatureNotImplemented. (OZ N-02)

* fix: Use LegacyAllocationAlreadyMigrated. (OZ N-02)

Use LegacyAllocationAlreadyMigrated to prevent migrating a legacy allocation twice.

* fix: restore migrate function to require from if revert.

Co-authored-by: Tomás Migone <[email protected]>

* fix: missing closing braket.

---------

Co-authored-by: Tomás Migone <[email protected]>

Author: MoonBoi9001

packages/horizon/contracts/data-service/interfaces/IDataService.sol

@@ -60,11 +60,6 @@ interface IDataService {
      */
     event ServiceProviderSlashed(address indexed serviceProvider, uint256 tokens);
 
-    /**
-     * @notice Thrown to signal that a feature is not implemented by a data service.
-     */
-    error DataServiceFeatureNotImplemented();
-
     /**
      * @notice Registers a service provider with the data service. The service provider can now
      * start providing the service.

packages/subgraph-service/contracts/libraries/LegacyAllocation.sol

@@ -39,19 +39,20 @@ library LegacyAllocation {
     /**
      * @notice Migrate a legacy allocation
      * @dev Requirements:
-     * - The allocation must not exist
+     * - The allocation must not have been previously migrated
      * @param self The legacy allocation list mapping
      * @param indexer The indexer that owns the allocation
      * @param allocationId The allocation id
      * @param subgraphDeploymentId The subgraph deployment id the allocation is for
+     * @custom:error LegacyAllocationAlreadyMigrated if the allocation has already been migrated
      */
     function migrate(
         mapping(address => State) storage self,
         address indexer,
         address allocationId,
         bytes32 subgraphDeploymentId
     ) internal {
-        require(!self[allocationId].exists(), LegacyAllocationExists(allocationId));
+        require(!self[allocationId].exists(), LegacyAllocationAlreadyMigrated(allocationId));
 
         State memory allocation = State({ indexer: indexer, subgraphDeploymentId: subgraphDeploymentId });
         self[allocationId] = allocation;

packages/subgraph-service/contracts/utilities/AllocationManager.sol

@@ -139,13 +139,7 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
     error AllocationManagerInvalidZeroAllocationId();
 
     /**
-     * @notice Thrown when attempting to create an allocation with zero tokens
-     * @param allocationId The id of the allocation
-     */
-    error AllocationManagerZeroTokensAllocation(address allocationId);
-
-    /**
-     * @notice Thrown when attempting to collect indexing rewards on a closed allocation
+     * @notice Thrown when attempting to collect indexing rewards on a closed allocationl
      * @param allocationId The id of the allocation
      */
     error AllocationManagerAllocationClosed(address allocationId);
@@ -178,7 +172,7 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
     /**
      * @notice Imports a legacy allocation id into the subgraph service
      * This is a governor only action that is required to prevent indexers from re-using allocation ids from the
-     * legacy staking contract.
+     * legacy staking contract. It will revert with LegacyAllocationAlreadyMigrated if the allocation has already been migrated.
      * @param _indexer The address of the indexer
      * @param _allocationId The id of the allocation
      * @param _subgraphDeploymentId The id of the subgraph deployment