From 98489542679fbf1f0ec7a22ea9ee25488b1c4866 Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Tue, 17 Sep 2024 15:44:53 +0100 Subject: [PATCH] fix: Use LegacyAllocationAlreadyMigrated. (OZ N-02) Use LegacyAllocationAlreadyMigrated to prevent migrating a legacy allocation twice. --- .../contracts/libraries/LegacyAllocation.sol | 7 +++++-- .../contracts/utilities/AllocationManager.sol | 10 +++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/subgraph-service/contracts/libraries/LegacyAllocation.sol b/packages/subgraph-service/contracts/libraries/LegacyAllocation.sol index 83bef8037..6ce99f22c 100644 --- a/packages/subgraph-service/contracts/libraries/LegacyAllocation.sol +++ b/packages/subgraph-service/contracts/libraries/LegacyAllocation.sol @@ -39,11 +39,12 @@ 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, @@ -51,7 +52,9 @@ library LegacyAllocation { 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 index d7c749f69..144785687 100644 --- 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); + } } /**