Skip to content

Horizon OZ2 audit - pending fixes #1131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: horizon
Choose a base branch
from
Draft

Horizon OZ2 audit - pending fixes #1131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
52988f1
chore: fix some copy in dispute manager comments
tmigone Mar 28, 2025
f2c3b7e
feat: require public poi when collecting rewards (CR-XX)
tmigone Mar 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions packages/subgraph-service/contracts/DisputeManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ contract DisputeManager is

/**
* @notice Make the subgraph service contract slash the indexer and reward the fisherman.
* Give the fisherman a reward equal to the fishermanRewardPercentage of slashed amount
* Give the fisherman a reward equal to the fishermanRewardCut of slashed amount
* @param _indexer Address of the indexer
* @param _tokensSlash Amount of tokens to slash from the indexer
* @param _tokensStakeSnapshot Snapshot of the indexer's stake at the time of the dispute creation
Expand Down Expand Up @@ -568,8 +568,8 @@ contract DisputeManager is
}

/**
* @notice Set the percent reward that the fisherman gets when slashing occurs.
* @dev Update the reward percentage to `_percentage`
* @notice Set the reward cut that the fisherman gets when slashing occurs.
* @dev Update the reward cut to `_fishermanRewardCut`
* @param _fishermanRewardCut The fisherman reward cut, in PPM
*/
function _setFishermanRewardCut(uint32 _fishermanRewardCut) private {
Expand All @@ -582,8 +582,8 @@ contract DisputeManager is
}

/**
* @notice Set the maximum percentage that can be used for slashing indexers.
* @param _maxSlashingCut Max percentage slashing for disputes, in PPM
* @notice Set the maximum cut that can be used for slashing indexers.
* @param _maxSlashingCut Max slashing cut, in PPM
*/
function _setMaxSlashingCut(uint32 _maxSlashingCut) private {
require(PPMMath.isValidPPM(_maxSlashingCut), DisputeManagerInvalidMaxSlashingCut(_maxSlashingCut));
Expand Down
11 changes: 9 additions & 2 deletions packages/subgraph-service/contracts/SubgraphService.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,13 @@ contract SubgraphService is
*
* @param indexer The address of the indexer
* @param paymentType The type of payment to collect as defined in {IGraphPayments}
* @param data Encoded data:
* - For query fees:
* - IGraphTallyCollector.SignedRAV `signedRav`: The signed RAV
* - For indexing rewards:
* - address `allocationId`: The id of the allocation
* - bytes32 `poi`: The POI being presented
* - bytes32 `publicPOI`: The public POI associated with the presented POI
*/
/// @inheritdoc IDataService
function collect(
Expand All @@ -275,12 +282,12 @@ contract SubgraphService is
);
paymentCollected = _collectQueryFees(signedRav);
} else if (paymentType == IGraphPayments.PaymentTypes.IndexingRewards) {
(address allocationId, bytes32 poi) = abi.decode(data, (address, bytes32));
(address allocationId, bytes32 poi, bytes32 publicPOI) = abi.decode(data, (address, bytes32, bytes32));
require(
_allocations.get(allocationId).indexer == indexer,
SubgraphServiceAllocationNotAuthorized(indexer, allocationId)
);
paymentCollected = _collectIndexingRewards(allocationId, poi, _delegationRatio);
paymentCollected = _collectIndexingRewards(allocationId, poi, publicPOI, _delegationRatio);
} else {
revert SubgraphServiceInvalidPaymentType(paymentType);
}
Expand Down
5 changes: 5 additions & 0 deletions packages/subgraph-service/contracts/utilities/AllocationManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
* @param tokensIndexerRewards The amount of tokens collected for the indexer
* @param tokensDelegationRewards The amount of tokens collected for delegators
* @param poi The POI presented
* @param publicPOI The public POI associated with the presented POI
* @param currentEpoch The current epoch
*/
event IndexingRewardsCollected(
Expand All @@ -71,6 +72,7 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
uint256 tokensIndexerRewards,
uint256 tokensDelegationRewards,
bytes32 poi,
bytes32 publicPOI,
uint256 currentEpoch
);

Expand Down Expand Up @@ -261,12 +263,14 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
*
* @param _allocationId The id of the allocation to collect rewards for
* @param _poi The POI being presented
* @param _publicPOI The public POI associated with the presented POI
* @param _delegationRatio The delegation ratio to consider when locking tokens
* @return The amount of tokens collected
*/
function _collectIndexingRewards(
address _allocationId,
bytes32 _poi,
bytes32 _publicPOI,
uint32 _delegationRatio
) internal returns (uint256) {
Allocation.State memory allocation = _allocations.get(_allocationId);
Expand Down Expand Up @@ -332,6 +336,7 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
tokensIndexerRewards,
tokensDelegationRewards,
_poi,
_publicPOI,
currentEpoch
);

Expand Down
4 changes: 3 additions & 1 deletion packages/subgraph-service/test/subgraphService/SubgraphService.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest {

struct IndexingRewardsData {
bytes32 poi;
bytes32 publicPOI;
uint256 tokensIndexerRewards;
uint256 tokensDelegationRewards;
}
Expand Down Expand Up @@ -314,7 +315,7 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest {
function _handleIndexingRewardsCollection(
bytes memory _data
) private returns (uint256 paymentCollected, address allocationId, IndexingRewardsData memory indexingRewardsData) {
(allocationId, indexingRewardsData.poi) = abi.decode(_data, (address, bytes32));
(allocationId, indexingRewardsData.poi, indexingRewardsData.publicPOI) = abi.decode(_data, (address, bytes32, bytes32));
Allocation.State memory allocation = subgraphService.getAllocation(allocationId);

// Calculate accumulated tokens, this depends on the rewards manager which we have mocked
Expand Down Expand Up @@ -348,6 +349,7 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest {
indexingRewardsData.tokensIndexerRewards,
indexingRewardsData.tokensDelegationRewards,
indexingRewardsData.poi,
indexingRewardsData.publicPOI,
epochManager.currentEpoch()
);

Expand Down
4 changes: 2 additions & 2 deletions packages/subgraph-service/test/subgraphService/allocation/forceClose.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ contract SubgraphServiceAllocationForceCloseTest is SubgraphServiceTest {
// Skip forward
skip(timeBetweenPOIs);

bytes memory data = abi.encode(allocationID, bytes32("POI1"));
bytes memory data = abi.encode(allocationID, bytes32("POI1"), bytes32("PUBLIC_POI1"));
_collect(users.indexer, IGraphPayments.PaymentTypes.IndexingRewards, data);
}

Expand All @@ -61,7 +61,7 @@ contract SubgraphServiceAllocationForceCloseTest is SubgraphServiceTest {

resetPrank(users.indexer);

bytes memory data = abi.encode(allocationID, bytes32("POI1"));
bytes memory data = abi.encode(allocationID, bytes32("POI1"), bytes32("PUBLIC_POI1"));
_collect(users.indexer, IGraphPayments.PaymentTypes.IndexingRewards, data);

resetPrank(permissionlessBob);
Expand Down
2 changes: 1 addition & 1 deletion packages/subgraph-service/test/subgraphService/allocation/resize.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ contract SubgraphServiceAllocationResizeTest is SubgraphServiceTest {
vm.roll(block.number + EPOCH_LENGTH);

IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards;
bytes memory data = abi.encode(allocationID, bytes32("POI1"));
bytes memory data = abi.encode(allocationID, bytes32("POI1"), bytes32("PUBLIC_POI1"));
_collect(users.indexer, paymentType, data);
_addToProvision(users.indexer, resizeTokens);
_resizeAllocation(users.indexer, allocationID, resizeTokens);
Expand Down
16 changes: 8 additions & 8 deletions packages/subgraph-service/test/subgraphService/collect/indexing/indexing.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {

function test_SubgraphService_Collect_Indexing(uint256 tokens) public useIndexer useAllocation(tokens) {
IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards;
bytes memory data = abi.encode(allocationID, bytes32("POI"));
bytes memory data = abi.encode(allocationID, bytes32("POI"), bytes32("PUBLIC_POI"));

// skip time to ensure allocation gets rewards
vm.roll(block.number + EPOCH_LENGTH);
Expand All @@ -40,7 +40,7 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {
vm.roll(block.number + EPOCH_LENGTH);

IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards;
bytes memory data = abi.encode(allocationID, bytes32("POI"));
bytes memory data = abi.encode(allocationID, bytes32("POI"), bytes32("PUBLIC_POI"));
_collect(users.indexer, paymentType, data);
}

Expand All @@ -65,7 +65,7 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {

resetPrank(users.indexer);
IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards;
bytes memory data = abi.encode(allocationID, bytes32("POI"));
bytes memory data = abi.encode(allocationID, bytes32("POI"), bytes32("PUBLIC_POI"));
_collect(users.indexer, paymentType, data);
}

Expand All @@ -76,7 +76,7 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {
vm.roll(block.number + EPOCH_LENGTH);

IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards;
bytes memory data = abi.encode(allocationID, bytes32("POI"));
bytes memory data = abi.encode(allocationID, bytes32("POI"), bytes32("PUBLIC_POI"));
_collect(users.indexer, paymentType, data);
}

Expand All @@ -92,7 +92,7 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {

resetPrank(users.indexer);

bytes memory data = abi.encode(allocationID, bytes32("POI"));
bytes memory data = abi.encode(allocationID, bytes32("POI"), bytes32("PUBLIC_POI"));
_collect(users.indexer, IGraphPayments.PaymentTypes.IndexingRewards, data);
}
}
Expand All @@ -118,7 +118,7 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {

resetPrank(users.indexer);

bytes memory data = abi.encode(allocationID, bytes32("POI"));
bytes memory data = abi.encode(allocationID, bytes32("POI"), bytes32("PUBLIC_POI"));
_collect(users.indexer, IGraphPayments.PaymentTypes.IndexingRewards, data);
}
}
Expand All @@ -145,7 +145,7 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {

// this collection should close the allocation
IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards;
bytes memory collectData = abi.encode(allocationID, bytes32("POI"));
bytes memory collectData = abi.encode(allocationID, bytes32("POI"), bytes32("PUBLIC_POI"));
_collect(users.indexer, paymentType, collectData);
}

Expand All @@ -156,7 +156,7 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {
// Setup new indexer
address newIndexer = makeAddr("newIndexer");
_createAndStartAllocation(newIndexer, tokens);
bytes memory data = abi.encode(allocationID, bytes32("POI"));
bytes memory data = abi.encode(allocationID, bytes32("POI"), bytes32("PUBLIC_POI"));

// skip time to ensure allocation gets rewards
vm.roll(block.number + EPOCH_LENGTH);
Expand Down
Loading