Skip to content

Commit 9a9f53b

Browse files
tmigonetmigone
committed
fix: use staking contract to check for allo collision (TRST-M06)
Signed-off-by: Tomás Migone <[email protected]>
1 parent 7efedcc commit 9a9f53b

File tree

6 files changed

+139
-24
lines changed

6 files changed

+139
-24
lines changed

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

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// SPDX-License-Identifier: GPL-3.0-or-later
22
pragma solidity 0.8.27;
33

4+
import { IHorizonStaking } from "@graphprotocol/horizon/contracts/interfaces/IHorizonStaking.sol";
5+
46
/**
57
* @title LegacyAllocation library
68
* @notice A library to handle legacy Allocations.
@@ -22,20 +24,14 @@ library LegacyAllocation {
2224
* @notice Thrown when attempting to migrate an allocation with an existing id
2325
* @param allocationId The allocation id
2426
*/
25-
error LegacyAllocationExists(address allocationId);
27+
error LegacyAllocationAlreadyExists(address allocationId);
2628

2729
/**
2830
* @notice Thrown when trying to get a non-existent allocation
2931
* @param allocationId The allocation id
3032
*/
3133
error LegacyAllocationDoesNotExist(address allocationId);
3234

33-
/**
34-
* @notice Thrown when trying to migrate an allocation that has already been migrated
35-
* @param allocationId The allocation id
36-
*/
37-
error LegacyAllocationAlreadyMigrated(address allocationId);
38-
3935
/**
4036
* @notice Migrate a legacy allocation
4137
* @dev Requirements:
@@ -52,7 +48,7 @@ library LegacyAllocation {
5248
address allocationId,
5349
bytes32 subgraphDeploymentId
5450
) internal {
55-
require(!self[allocationId].exists(), LegacyAllocationAlreadyMigrated(allocationId));
51+
require(!self[allocationId].exists(), LegacyAllocationAlreadyExists(allocationId));
5652

5753
State memory allocation = State({ indexer: indexer, subgraphDeploymentId: subgraphDeploymentId });
5854
self[allocationId] = allocation;
@@ -69,11 +65,19 @@ library LegacyAllocation {
6965

7066
/**
7167
* @notice Revert if a legacy allocation exists
68+
* @dev We first check the migrated mapping then the old staking contract.
69+
* @dev TODO: after the transition period when all the allocations are migrated we can
70+
* remove the call to the staking contract.
7271
* @param self The legacy allocation list mapping
7372
* @param allocationId The allocation id
7473
*/
75-
function revertIfExists(mapping(address => State) storage self, address allocationId) internal view {
76-
require(!self[allocationId].exists(), LegacyAllocationExists(allocationId));
74+
function revertIfExists(
75+
mapping(address => State) storage self,
76+
IHorizonStaking graphStaking,
77+
address allocationId
78+
) internal view {
79+
require(!self[allocationId].exists(), LegacyAllocationAlreadyExists(allocationId));
80+
require(!graphStaking.isAllocation(allocationId), LegacyAllocationAlreadyExists(allocationId));
7781
}
7882

7983
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
212212

213213
// Ensure allocation id is not reused
214214
// need to check both subgraph service (on allocations.create()) and legacy allocations
215-
legacyAllocations.revertIfExists(_allocationId);
215+
legacyAllocations.revertIfExists(_graphStaking(), _allocationId);
216216
Allocation.State memory allocation = allocations.create(
217217
_indexer,
218218
_allocationId,

‎packages/subgraph-service/test/shared/HorizonStakingShared.t.sol

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ import "forge-std/Test.sol";
55

66
import { IGraphPayments } from "@graphprotocol/horizon/contracts/interfaces/IGraphPayments.sol";
77
import { IHorizonStakingTypes } from "@graphprotocol/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol";
8+
import { IHorizonStakingExtension } from "@graphprotocol/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol";
89

910
import { SubgraphBaseTest } from "../SubgraphBaseTest.t.sol";
1011

1112
abstract contract HorizonStakingSharedTest is SubgraphBaseTest {
12-
1313
/*
1414
* HELPERS
1515
*/
@@ -45,11 +45,11 @@ abstract contract HorizonStakingSharedTest is SubgraphBaseTest {
4545
function _thawDeprovisionAndUnstake(address _indexer, address _verifier, uint256 _tokens) internal {
4646
// Initiate thaw request
4747
staking.thaw(_indexer, _verifier, _tokens);
48-
48+
4949
// Skip thawing period
5050
IHorizonStakingTypes.Provision memory provision = staking.getProvision(_indexer, _verifier);
5151
skip(provision.thawingPeriod + 1);
52-
52+
5353
// Deprovision and unstake
5454
staking.deprovision(_indexer, _verifier, 0);
5555
staking.unstake(_tokens);
@@ -64,6 +64,67 @@ abstract contract HorizonStakingSharedTest is SubgraphBaseTest {
6464
staking.setProvisionParameters(_indexer, _verifier, _maxVerifierCut, _thawingPeriod);
6565
}
6666

67+
function _setStorage_allocation_hardcoded(address indexer, address allocationId, uint256 tokens) internal {
68+
IHorizonStakingExtension.Allocation memory allocation = IHorizonStakingExtension.Allocation({
69+
indexer: indexer,
70+
subgraphDeploymentID: bytes32("0x12344321"),
71+
tokens: tokens,
72+
createdAtEpoch: 1234,
73+
closedAtEpoch: 1235,
74+
collectedFees: 1234,
75+
__DEPRECATED_effectiveAllocation: 1222234,
76+
accRewardsPerAllocatedToken: 1233334,
77+
distributedRebates: 1244434
78+
});
79+
80+
// __DEPRECATED_allocations
81+
uint256 allocationsSlot = 15;
82+
bytes32 allocationBaseSlot = keccak256(abi.encode(allocationId, allocationsSlot));
83+
vm.store(address(staking), allocationBaseSlot, bytes32(uint256(uint160(allocation.indexer))));
84+
vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 1), allocation.subgraphDeploymentID);
85+
vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 2), bytes32(tokens));
86+
vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 3), bytes32(allocation.createdAtEpoch));
87+
vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 4), bytes32(allocation.closedAtEpoch));
88+
vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 5), bytes32(allocation.collectedFees));
89+
vm.store(
90+
address(staking),
91+
bytes32(uint256(allocationBaseSlot) + 6),
92+
bytes32(allocation.__DEPRECATED_effectiveAllocation)
93+
);
94+
vm.store(
95+
address(staking),
96+
bytes32(uint256(allocationBaseSlot) + 7),
97+
bytes32(allocation.accRewardsPerAllocatedToken)
98+
);
99+
vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 8), bytes32(allocation.distributedRebates));
100+
101+
// _serviceProviders
102+
uint256 serviceProviderSlot = 14;
103+
bytes32 serviceProviderBaseSlot = keccak256(abi.encode(allocation.indexer, serviceProviderSlot));
104+
uint256 currentTokensStaked = uint256(vm.load(address(staking), serviceProviderBaseSlot));
105+
uint256 currentTokensProvisioned = uint256(
106+
vm.load(address(staking), bytes32(uint256(serviceProviderBaseSlot) + 1))
107+
);
108+
vm.store(
109+
address(staking),
110+
bytes32(uint256(serviceProviderBaseSlot) + 0),
111+
bytes32(currentTokensStaked + tokens)
112+
);
113+
vm.store(
114+
address(staking),
115+
bytes32(uint256(serviceProviderBaseSlot) + 1),
116+
bytes32(currentTokensProvisioned + tokens)
117+
);
118+
119+
// __DEPRECATED_subgraphAllocations
120+
uint256 subgraphsAllocationsSlot = 16;
121+
bytes32 subgraphAllocationsBaseSlot = keccak256(
122+
abi.encode(allocation.subgraphDeploymentID, subgraphsAllocationsSlot)
123+
);
124+
uint256 currentAllocatedTokens = uint256(vm.load(address(staking), subgraphAllocationsBaseSlot));
125+
vm.store(address(staking), subgraphAllocationsBaseSlot, bytes32(currentAllocatedTokens + tokens));
126+
}
127+
67128
/*
68129
* PRIVATE
69130
*/

‎packages/subgraph-service/test/subgraphService/SubgraphService.t.sol

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,17 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest {
379379
}
380380
}
381381

382+
function _migrateLegacyAllocation(address _indexer, address _allocationId, bytes32 _subgraphDeploymentID) internal {
383+
vm.expectEmit(address(subgraphService));
384+
emit AllocationManager.LegacyAllocationMigrated(_indexer, _allocationId, _subgraphDeploymentID);
385+
386+
subgraphService.migrateLegacyAllocation(_indexer, _allocationId, _subgraphDeploymentID);
387+
388+
(address afterIndexer, bytes32 afterSubgraphDeploymentId) = subgraphService.legacyAllocations(_allocationId);
389+
assertEq(afterIndexer, _indexer);
390+
assertEq(afterSubgraphDeploymentId, _subgraphDeploymentID);
391+
}
392+
382393
/*
383394
* HELPERS
384395
*/

‎packages/subgraph-service/test/subgraphService/allocation/start.t.sol

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ pragma solidity 0.8.27;
44
import "forge-std/Test.sol";
55

66
import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
7-
import { IDataService } from "@graphprotocol/horizon/contracts/data-service/interfaces/IDataService.sol";
87
import { ProvisionManager } from "@graphprotocol/horizon/contracts/data-service/utilities/ProvisionManager.sol";
98
import { ProvisionTracker } from "@graphprotocol/horizon/contracts/data-service/libraries/ProvisionTracker.sol";
109

@@ -137,35 +136,52 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest {
137136
subgraphService.startService(users.indexer, data);
138137
}
139138

140-
function test_SubgraphService_Allocation_Start_RevertWhen_ArealdyExists(uint256 tokens) public useIndexer {
139+
function test_SubgraphService_Allocation_Start_RevertWhen_AlreadyExists_SubgraphService(uint256 tokens) public useIndexer {
141140
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
142141

143142
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
144143
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
145144

146-
bytes32 slot = keccak256(abi.encode(allocationID, uint256(158)));
147-
vm.store(address(subgraphService), slot, bytes32(uint256(uint160(users.indexer))));
148-
vm.store(address(subgraphService), bytes32(uint256(slot) + 1), subgraphDeployment);
149-
150145
bytes memory data = _generateData(tokens);
146+
_startService(users.indexer, data);
147+
151148
vm.expectRevert(abi.encodeWithSelector(
152-
LegacyAllocation.LegacyAllocationExists.selector,
149+
Allocation.AllocationAlreadyExists.selector,
153150
allocationID
154151
));
155152
subgraphService.startService(users.indexer, data);
156153
}
157154

158-
function test_SubgraphService_Allocation_Start_RevertWhen_ReusingAllocationId(uint256 tokens) public useIndexer {
155+
function test_SubgraphService_Allocation_Start_RevertWhen_AlreadyExists_Migrated(uint256 tokens) public useIndexer {
159156
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
160157

161158
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
162159
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
163160

161+
resetPrank(users.governor);
162+
_migrateLegacyAllocation(users.indexer, allocationID, subgraphDeployment);
163+
164+
resetPrank(users.indexer);
164165
bytes memory data = _generateData(tokens);
165-
_startService(users.indexer, data);
166+
vm.expectRevert(abi.encodeWithSelector(
167+
LegacyAllocation.LegacyAllocationAlreadyExists.selector,
168+
allocationID
169+
));
170+
subgraphService.startService(users.indexer, data);
171+
}
172+
173+
function test_SubgraphService_Allocation_Start_RevertWhen_AlreadyExists_Staking(uint256 tokens) public useIndexer {
174+
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
166175

176+
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
177+
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
178+
179+
// create dummy allo in staking contract
180+
_setStorage_allocation_hardcoded(users.indexer, allocationID, tokens);
181+
182+
bytes memory data = _generateData(tokens);
167183
vm.expectRevert(abi.encodeWithSelector(
168-
Allocation.AllocationAlreadyExists.selector,
184+
LegacyAllocation.LegacyAllocationAlreadyExists.selector,
169185
allocationID
170186
));
171187
subgraphService.startService(users.indexer, data);

‎packages/subgraph-service/test/subgraphService/governance/legacy.t.sol

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// SPDX-License-Identifier: MIT
2+
pragma solidity 0.8.27;
3+
4+
import "forge-std/Test.sol";
5+
6+
import { OwnableUpgradeable } from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
7+
8+
import { SubgraphServiceTest } from "../SubgraphService.t.sol";
9+
10+
contract SubgraphServiceLegacyAllocation is SubgraphServiceTest {
11+
/*
12+
* TESTS
13+
*/
14+
15+
function test_MigrateAllocation() public useGovernor {
16+
_migrateLegacyAllocation(users.indexer, allocationID, subgraphDeployment);
17+
}
18+
19+
function test_MigrateAllocation_WhenNotGovernor() public useIndexer {
20+
vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, users.indexer));
21+
subgraphService.migrateLegacyAllocation(users.indexer, allocationID, subgraphDeployment);
22+
}
23+
}

0 commit comments

Comments
 (0)