diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index b73bcfbcd..574111b32 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -496,6 +496,15 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { return _isAuthorized(serviceProvider, verifier, operator); } + /* + * LEGACY + */ + + /// @inheritdoc IHorizonStakingMain + function isAllocation(address allocationID) external view override returns (bool) { + return _getLegacyAllocationState(allocationID) != LegacyAllocationState.Null; + } + /* * PRIVATE FUNCTIONS */ @@ -1170,6 +1179,25 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { } } + /** + * @notice Return the current state of a legacy allocation + * @param _allocationID Allocation identifier + * @return LegacyAllocationState enum with the state of the allocation + */ + function _getLegacyAllocationState(address _allocationID) private view returns (LegacyAllocationState) { + LegacyAllocation storage alloc = __DEPRECATED_allocations[_allocationID]; + + if (alloc.indexer == address(0)) { + return LegacyAllocationState.Null; + } + + if (alloc.createdAtEpoch != 0 && alloc.closedAtEpoch == 0) { + return LegacyAllocationState.Active; + } + + return LegacyAllocationState.Closed; + } + /** * @notice Determines the correct callback function for `deleteItem` based on the request type. * @param _requestType The type of thaw request (Provision or Delegation). diff --git a/packages/horizon/contracts/staking/HorizonStakingStorage.sol b/packages/horizon/contracts/staking/HorizonStakingStorage.sol index aabe4d778..7f4818725 100644 --- a/packages/horizon/contracts/staking/HorizonStakingStorage.sol +++ b/packages/horizon/contracts/staking/HorizonStakingStorage.sol @@ -63,8 +63,9 @@ abstract contract HorizonStakingV1Storage { mapping(address serviceProvider => IHorizonStakingTypes.ServiceProviderInternal details) internal _serviceProviders; /// @dev Allocation details. - /// Deprecated, now applied on the subgraph data service - mapping(address allocationId => bytes32 __DEPRECATED_allocation) internal __DEPRECATED_allocations; + /// Deprecated, now applied on the subgraph data service. + /// Kept for storage compatibility and to check for allocation id collisions. + mapping(address allocationId => IHorizonStakingTypes.LegacyAllocation allocation) internal __DEPRECATED_allocations; /// @dev Subgraph allocations, tracks the tokens allocated to a subgraph deployment /// Deprecated, now applied on the SubgraphService diff --git a/packages/horizon/test/unit/payments/graph-tally-collector/GraphTallyCollector.t.sol b/packages/horizon/test/unit/payments/graph-tally-collector/GraphTallyCollector.t.sol index b8e569574..ccd242255 100644 --- a/packages/horizon/test/unit/payments/graph-tally-collector/GraphTallyCollector.t.sol +++ b/packages/horizon/test/unit/payments/graph-tally-collector/GraphTallyCollector.t.sol @@ -42,7 +42,7 @@ contract GraphTallyTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest { * HELPERS */ - function _getSignerProof(uint256 _proofDeadline, uint256 _signer) internal view returns (bytes memory) { + function _getSignerProof(uint256 _proofDeadline, uint256 _signer) internal returns (bytes memory) { (, address msgSender, ) = vm.readCallers(); bytes32 messageHash = keccak256( abi.encodePacked( diff --git a/packages/horizon/test/unit/staking/legacy/isAllocation.t.sol b/packages/horizon/test/unit/staking/legacy/isAllocation.t.sol new file mode 100644 index 000000000..1f19ccc00 --- /dev/null +++ b/packages/horizon/test/unit/staking/legacy/isAllocation.t.sol @@ -0,0 +1,107 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import { IHorizonStakingTypes } from "@graphprotocol/interfaces/contracts/horizon/internal/IHorizonStakingTypes.sol"; +import { HorizonStakingSharedTest } from "../../shared/horizon-staking/HorizonStakingShared.t.sol"; + +contract HorizonStakingIsAllocationTest is HorizonStakingSharedTest { + /* + * TESTS + */ + + function test_IsAllocation_ReturnsFalse_WhenAllocationDoesNotExist() public { + address nonExistentAllocationId = makeAddr("nonExistentAllocation"); + assertFalse(staking.isAllocation(nonExistentAllocationId)); + } + + function test_IsAllocation_ReturnsTrue_WhenActiveAllocationExists() public { + address allocationId = makeAddr("activeAllocation"); + + // Set up an active legacy allocation in storage + _setLegacyAllocationInStaking( + allocationId, + users.indexer, + bytes32("subgraphDeploymentId"), + 1000 ether, // tokens + 1, // createdAtEpoch + 0 // closedAtEpoch (0 = still active) + ); + + assertTrue(staking.isAllocation(allocationId)); + } + + function test_IsAllocation_ReturnsTrue_WhenClosedAllocationExists() public { + address allocationId = makeAddr("closedAllocation"); + + // Set up a closed legacy allocation in storage + _setLegacyAllocationInStaking( + allocationId, + users.indexer, + bytes32("subgraphDeploymentId"), + 1000 ether, // tokens + 1, // createdAtEpoch + 10 // closedAtEpoch (non-zero = closed) + ); + + assertTrue(staking.isAllocation(allocationId)); + } + + function test_IsAllocation_ReturnsFalse_WhenIndexerIsZeroAddress() public { + address allocationId = makeAddr("zeroIndexerAllocation"); + + // Set up an allocation with zero indexer (should be considered Null) + _setLegacyAllocationInStaking( + allocationId, + address(0), // indexer is zero + bytes32("subgraphDeploymentId"), + 1000 ether, + 1, + 0 + ); + + assertFalse(staking.isAllocation(allocationId)); + } + + /* + * HELPERS + */ + + /** + * @notice Sets a legacy allocation directly in HorizonStaking storage + * @dev The __DEPRECATED_allocations mapping is at storage slot 10 in HorizonStakingStorage + * The LegacyAllocation struct has the following layout: + * - slot 0: indexer (address) + * - slot 1: subgraphDeploymentID (bytes32) + * - slot 2: tokens (uint256) + * - slot 3: createdAtEpoch (uint256) + * - slot 4: closedAtEpoch (uint256) + * - slot 5: collectedFees (uint256) + * - slot 6: __DEPRECATED_effectiveAllocation (uint256) + * - slot 7: accRewardsPerAllocatedToken (uint256) + * - slot 8: distributedRebates (uint256) + */ + function _setLegacyAllocationInStaking( + address _allocationId, + address _indexer, + bytes32 _subgraphDeploymentId, + uint256 _tokens, + uint256 _createdAtEpoch, + uint256 _closedAtEpoch + ) internal { + // Storage slot for __DEPRECATED_allocations mapping in HorizonStaking + // Use `forge inspect HorizonStaking storage-layout` to verify + uint256 allocationsSlot = 15; + bytes32 allocationBaseSlot = keccak256(abi.encode(_allocationId, allocationsSlot)); + + // Set indexer (slot 0) + vm.store(address(staking), allocationBaseSlot, bytes32(uint256(uint160(_indexer)))); + // Set subgraphDeploymentID (slot 1) + vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 1), _subgraphDeploymentId); + // Set tokens (slot 2) + vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 2), bytes32(_tokens)); + // Set createdAtEpoch (slot 3) + vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 3), bytes32(_createdAtEpoch)); + // Set closedAtEpoch (slot 4) + vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 4), bytes32(_closedAtEpoch)); + } +} diff --git a/packages/interfaces/contracts/horizon/internal/IHorizonStakingMain.sol b/packages/interfaces/contracts/horizon/internal/IHorizonStakingMain.sol index ddc595409..1c87fee1e 100644 --- a/packages/interfaces/contracts/horizon/internal/IHorizonStakingMain.sol +++ b/packages/interfaces/contracts/horizon/internal/IHorizonStakingMain.sol @@ -978,4 +978,13 @@ interface IHorizonStakingMain { * @return The amount of tokens withdrawn */ function forceWithdrawDelegated(address serviceProvider, address delegator) external returns (uint256); + + /** + * @notice Return if allocationID is used. + * @dev This function is used to check for allocation id collisions with legacy allocations + * that were created before the Horizon upgrade. + * @param allocationID Address used as signer by the indexer for an allocation + * @return True if allocationID already used + */ + function isAllocation(address allocationID) external view returns (bool); } diff --git a/packages/interfaces/contracts/horizon/internal/IHorizonStakingTypes.sol b/packages/interfaces/contracts/horizon/internal/IHorizonStakingTypes.sol index e8fff211b..22cdb5b4b 100644 --- a/packages/interfaces/contracts/horizon/internal/IHorizonStakingTypes.sol +++ b/packages/interfaces/contracts/horizon/internal/IHorizonStakingTypes.sol @@ -200,4 +200,42 @@ interface IHorizonStakingTypes { uint256 tokensThawing; uint256 sharesThawing; } + + /** + * @notice Legacy allocation representation + * @dev Kept for storage compatibility and to check for allocation id collisions. + * @param indexer The indexer address + * @param subgraphDeploymentID The subgraph deployment ID + * @param tokens The amount of tokens allocated to the subgraph deployment + * @param createdAtEpoch The epoch when the allocation was created + * @param closedAtEpoch The epoch when the allocation was closed + * @param collectedFees The amount of collected fees for the allocation + * @param __DEPRECATED_effectiveAllocation Deprecated field + * @param accRewardsPerAllocatedToken Snapshot used for reward calculation + * @param distributedRebates The amount of collected rebates that have been rebated + */ + struct LegacyAllocation { + address indexer; + bytes32 subgraphDeploymentID; + uint256 tokens; + uint256 createdAtEpoch; + uint256 closedAtEpoch; + uint256 collectedFees; + uint256 __DEPRECATED_effectiveAllocation; + uint256 accRewardsPerAllocatedToken; + uint256 distributedRebates; + } + + /** + * @dev Possible states a legacy allocation can be. + * States: + * - Null = indexer == address(0) + * - Active = not Null && tokens > 0 + * - Closed = Active && closedAtEpoch != 0 + */ + enum LegacyAllocationState { + Null, + Active, + Closed + } } diff --git a/packages/subgraph-service/contracts/libraries/AllocationHandler.sol b/packages/subgraph-service/contracts/libraries/AllocationHandler.sol index 7d78ca6d3..d45db0ae6 100644 --- a/packages/subgraph-service/contracts/libraries/AllocationHandler.sol +++ b/packages/subgraph-service/contracts/libraries/AllocationHandler.sol @@ -252,7 +252,7 @@ library AllocationHandler { // Ensure allocation id is not reused // need to check both subgraph service (on allocations.create()) and legacy allocations - _legacyAllocations.revertIfExists(params._allocationId); + _legacyAllocations.revertIfExists(params.graphStaking, params._allocationId); IAllocation.State memory allocation = _allocations.create( params._indexer, diff --git a/packages/subgraph-service/contracts/libraries/LegacyAllocation.sol b/packages/subgraph-service/contracts/libraries/LegacyAllocation.sol index 151b15f08..746d124ee 100644 --- a/packages/subgraph-service/contracts/libraries/LegacyAllocation.sol +++ b/packages/subgraph-service/contracts/libraries/LegacyAllocation.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.33; +import { IHorizonStaking } from "@graphprotocol/interfaces/contracts/horizon/IHorizonStaking.sol"; import { ILegacyAllocation } from "@graphprotocol/interfaces/contracts/subgraph-service/internal/ILegacyAllocation.sol"; /** @@ -15,15 +16,21 @@ library LegacyAllocation { /** * @notice Revert if a legacy allocation exists - * @dev We check the migrated allocations mapping. + * @dev We check both the migrated allocations mapping and the legacy staking contract. * @param self The legacy allocation list mapping + * @param graphStaking The Horizon Staking contract * @param allocationId The allocation id */ function revertIfExists( mapping(address => ILegacyAllocation.State) storage self, + IHorizonStaking graphStaking, address allocationId ) internal view { require(!self[allocationId].exists(), ILegacyAllocation.LegacyAllocationAlreadyExists(allocationId)); + require( + !graphStaking.isAllocation(allocationId), + ILegacyAllocation.LegacyAllocationAlreadyExists(allocationId) + ); } /** diff --git a/packages/subgraph-service/test/unit/subgraphService/SubgraphService.t.sol b/packages/subgraph-service/test/unit/subgraphService/SubgraphService.t.sol index 74b0718bb..5002900f1 100644 --- a/packages/subgraph-service/test/unit/subgraphService/SubgraphService.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/SubgraphService.t.sol @@ -492,6 +492,45 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest { assertEq(afterLegacyAllocation.subgraphDeploymentId, _subgraphDeploymentId); } + /** + * @notice Sets a legacy allocation directly in HorizonStaking storage + * @dev The __DEPRECATED_allocations mapping is at storage slot 15 in HorizonStaking + * Use `forge inspect HorizonStaking storage-layout` to verify + * The LegacyAllocation struct has the following layout: + * - slot 0: indexer (address) + * - slot 1: subgraphDeploymentID (bytes32) + * - slot 2: tokens (uint256) + * - slot 3: createdAtEpoch (uint256) + * - slot 4: closedAtEpoch (uint256) + * - slot 5: collectedFees (uint256) + * - slot 6: __DEPRECATED_effectiveAllocation (uint256) + * - slot 7: accRewardsPerAllocatedToken (uint256) + * - slot 8: distributedRebates (uint256) + */ + function _setLegacyAllocationInStaking( + address _allocationId, + address _indexer, + bytes32 _subgraphDeploymentId + ) internal { + // Storage slot for __DEPRECATED_allocations mapping in HorizonStaking + uint256 allocationsSlot = 15; + bytes32 allocationBaseSlot = keccak256(abi.encode(_allocationId, allocationsSlot)); + + // Set indexer (slot 0) + vm.store(address(staking), allocationBaseSlot, bytes32(uint256(uint160(_indexer)))); + // Set subgraphDeploymentID (slot 1) + vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 1), _subgraphDeploymentId); + // Set tokens (slot 2) - non-zero to indicate active allocation + vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 2), bytes32(uint256(1000 ether))); + // Set createdAtEpoch (slot 3) - non-zero + vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 3), bytes32(uint256(1))); + // Set closedAtEpoch (slot 4) - non-zero to indicate closed + vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 4), bytes32(uint256(10))); + + // Verify the allocation is now visible via isAllocation + assertTrue(staking.isAllocation(_allocationId)); + } + /* * HELPERS */ diff --git a/packages/subgraph-service/test/unit/subgraphService/allocation/start.t.sol b/packages/subgraph-service/test/unit/subgraphService/allocation/start.t.sol index 5617f4d7b..68c3c6674 100644 --- a/packages/subgraph-service/test/unit/subgraphService/allocation/start.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/allocation/start.t.sol @@ -165,8 +165,9 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest { _createProvision(users.indexer, tokens, FISHERMAN_REWARD_PERCENTAGE, DISPUTE_PERIOD); _register(users.indexer, abi.encode("url", "geoHash", address(0))); - // simulate legacy allocation migration - _migrateLegacyAllocation(users.indexer, allocationId, subgraphDeployment); + // Set a legacy allocation directly in HorizonStaking storage + // This simulates an allocation that was created before Horizon and exists in the staking contract + _setLegacyAllocationInStaking(allocationId, users.indexer, subgraphDeployment); bytes memory data = _generateData(tokens); vm.expectRevert(abi.encodeWithSelector(ILegacyAllocation.LegacyAllocationAlreadyExists.selector, allocationId)); diff --git a/packages/subgraph-service/test/unit/subgraphService/collect/query/query.t.sol b/packages/subgraph-service/test/unit/subgraphService/collect/query/query.t.sol index 4915ac17f..76fae1307 100644 --- a/packages/subgraph-service/test/unit/subgraphService/collect/query/query.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/collect/query/query.t.sol @@ -21,7 +21,7 @@ contract SubgraphServiceRegisterTest is SubgraphServiceTest { * HELPERS */ - function _getSignerProof(uint256 _proofDeadline, uint256 _signer) private view returns (bytes memory) { + function _getSignerProof(uint256 _proofDeadline, uint256 _signer) private returns (bytes memory) { (, address msgSender, ) = vm.readCallers(); bytes32 messageHash = keccak256( abi.encodePacked(