fix: Use LegacyAllocationAlreadyMigrated. (OZ N-02)

Use LegacyAllocationAlreadyMigrated to prevent migrating a legacy allocation twice.
graphprotocol · Sep 17, 2024 · 9848954 · 9848954
1 parent 95ffd24
commit 9848954
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 5 deletions.
diff --git a/packages/subgraph-service/contracts/libraries/LegacyAllocation.sol b/packages/subgraph-service/contracts/libraries/LegacyAllocation.sol
@@ -39,19 +39,22 @@ 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));
+        if (self[allocationId].exists()) {
+            revert LegacyAllocationAlreadyMigrated(allocationId);
+        }
 
         State memory allocation = State({ indexer: indexer, subgraphDeploymentId: subgraphDeploymentId });
         self[allocationId] = allocation;

diff --git a/packages/subgraph-service/contracts/utilities/AllocationManager.sol b/packages/subgraph-service/contracts/utilities/AllocationManager.sol
@@ -169,14 +169,18 @@ 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 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
+     * @custom:error LegacyAllocationMigrated if the allocation has already been migrated
      */
     function _migrateLegacyAllocation(address _indexer, address _allocationId, bytes32 _subgraphDeploymentId) internal {
-        legacyAllocations.migrate(_indexer, _allocationId, _subgraphDeploymentId);
-        emit LegacyAllocationMigrated(_indexer, _allocationId, _subgraphDeploymentId);
+        try legacyAllocations.migrate(_indexer, _allocationId, _subgraphDeploymentId) {
+            emit LegacyAllocationMigrated(_indexer, _allocationId, _subgraphDeploymentId);
+        } catch Error(string memory reason) {
+            revert(reason);
+        }
     }
 
     /**