Skip to content
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.

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

feat: dont issue rewards to allos less than one epoch old #1103

Merged
merged 3 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 0 additions & 7 deletions packages/contracts/contracts/rewards/IRewardsIssuer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,4 @@ interface IRewardsIssuer {
* @return Total tokens allocated to subgraph
*/
function getSubgraphAllocatedTokens(bytes32 _subgraphDeploymentId) external view returns (uint256);

/**
* @notice Whether or not an allocation is active (i.e open)
* @param _allocationId Allocation Id
* @return Whether or not the allocation is active
*/
function isActiveAllocation(address _allocationId) external view returns (bool);
}
2 changes: 1 addition & 1 deletion packages/contracts/contracts/rewards/IRewardsManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ interface IRewardsManager {

function getAccRewardsPerAllocatedToken(bytes32 _subgraphDeploymentID) external view returns (uint256, uint256);

function getRewards(address _allocationID) external view returns (uint256);
function getRewards(address _rewardsIssuer, address _allocationID) external view returns (uint256);

function calcRewards(uint256 _tokens, uint256 _accRewardsPerAllocatedToken) external pure returns (uint256);

Expand Down
25 changes: 6 additions & 19 deletions packages/contracts/contracts/rewards/RewardsManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -322,32 +322,19 @@ contract RewardsManager is RewardsManagerV5Storage, GraphUpgradeable, IRewardsMa
* @param _allocationID Allocation
* @return Rewards amount for an allocation
*/
function getRewards(address _allocationID) external view override returns (uint256) {
address rewardsIssuer = address(0);

// Check both the legacy and new allocations
address[2] memory rewardsIssuers = [address(staking()), address(subgraphService)];
for (uint256 i = 0; i < rewardsIssuers.length; i++) {
if (rewardsIssuers[i] != address(0)) {
if (IRewardsIssuer(rewardsIssuers[i]).isActiveAllocation(_allocationID)) {
rewardsIssuer = address(rewardsIssuers[i]);
break;
}
}
}

// Bail if allo does not exist
if (rewardsIssuer == address(0)) {
return 0;
}
function getRewards(address _rewardsIssuer, address _allocationID) external view override returns (uint256) {
require(
_rewardsIssuer == address(staking()) || _rewardsIssuer == address(subgraphService),
"Not a rewards issuer"
);

(
,
bytes32 subgraphDeploymentId,
uint256 tokens,
uint256 alloAccRewardsPerAllocatedToken,
uint256 accRewardsPending
) = IRewardsIssuer(rewardsIssuer).getAllocationData(_allocationID);
) = IRewardsIssuer(_rewardsIssuer).getAllocationData(_allocationID);

(uint256 accRewardsPerAllocatedToken, ) = getAccRewardsPerAllocatedToken(subgraphDeploymentId);
return
Expand Down
8 changes: 4 additions & 4 deletions packages/contracts/test/unit/rewards/rewards.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ describe('Rewards', () => {
await helpers.mine(ISSUANCE_RATE_PERIODS)

// Rewards
const contractRewards = await rewardsManager.getRewards(allocationID1)
const contractRewards = await rewardsManager.getRewards(staking.address, allocationID1)

// We trust using this function in the test because we tested it
// standalone in a previous test
Expand Down Expand Up @@ -504,12 +504,12 @@ describe('Rewards', () => {
await staking.connect(indexer1).closeAllocation(allocationID1, randomHexBytes())

// Rewards
const contractRewards = await rewardsManager.getRewards(allocationID1)
const contractRewards = await rewardsManager.getRewards(staking.address, allocationID1)
expect(contractRewards).eq(BigNumber.from(0))
})
it('rewards should be zero if the allocation does not exist', async function () {
// Rewards
const contractRewards = await rewardsManager.getRewards(allocationIDNull)
const contractRewards = await rewardsManager.getRewards(staking.address, allocationIDNull)
expect(contractRewards).eq(BigNumber.from(0))
})
})
Expand Down Expand Up @@ -1026,7 +1026,7 @@ describe('Rewards', () => {
await staking.connect(assetHolder).collect(tokensToCollect, allocationID1)

// check rewards diff
await rewardsManager.getRewards(allocationID1).then(formatGRT)
await rewardsManager.getRewards(staking.address, allocationID1).then(formatGRT)

await helpers.mine()
const accrual = await getRewardsAccrual(subgraphs)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,15 +274,6 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension
return __DEPRECATED_subgraphAllocations[subgraphDeploymentID];
}

/**
* @notice Return true if allocation is active.
* @param allocationID Allocation identifier
* @return True if allocation is active
*/
function isActiveAllocation(address allocationID) external view override returns (bool) {
return _getAllocationState(allocationID) == AllocationState.Active;
}

/**
* @notice Get the total amount of tokens staked by the indexer.
* @param indexer Address of the indexer
Expand Down
19 changes: 0 additions & 19 deletions packages/subgraph-service/contracts/SubgraphService.sol
Original file line number Diff line number Diff line change
Expand Up @@ -449,18 +449,6 @@ contract SubgraphService is
return _subgraphAllocatedTokens[subgraphDeploymentId];
}

/**
* @notice Check if an allocation is open
* @dev Implements {IRewardsIssuer.isAllocationActive}
* @dev To be used by the {RewardsManager}.
*
* @param allocationId The allocation Id
* @return Wether or not the allocation is active
*/
function isActiveAllocation(address allocationId) external view override returns (bool) {
return _allocations[allocationId].isOpen();
}

/**
* @notice See {ISubgraphService.getLegacyAllocation}
*/
Expand All @@ -475,13 +463,6 @@ contract SubgraphService is
return _encodeAllocationProof(indexer, allocationId);
}

/**
* @notice See {ISubgraphService.isStaleAllocation}
*/
function isStaleAllocation(address allocationId) external view override returns (bool) {
return _allocations.get(allocationId).isStale(maxPOIStaleness);
}

/**
* @notice See {ISubgraphService.isOverAllocated}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,6 @@ interface ISubgraphService is IDataServiceFees {
*/
function encodeAllocationProof(address indexer, address allocationId) external view returns (bytes32);

/**
* @notice Checks if an allocation is stale
* @param allocationId The id of the allocation
* @return True if the allocation is stale, false otherwise
*/
function isStaleAllocation(address allocationId) external view returns (bool);

/**
* @notice Checks if an indexer is over-allocated
* @param allocationId The id of the allocation
Expand Down
8 changes: 6 additions & 2 deletions packages/subgraph-service/contracts/libraries/Allocation.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ library Allocation {
uint256 accRewardsPerAllocatedToken;
// Accumulated rewards that are pending to be claimed due allocation resize
uint256 accRewardsPending;
// Epoch when the allocation was created
uint256 createdAtEpoch;
}

/**
Expand Down Expand Up @@ -68,7 +70,8 @@ library Allocation {
address allocationId,
bytes32 subgraphDeploymentId,
uint256 tokens,
uint256 accRewardsPerAllocatedToken
uint256 accRewardsPerAllocatedToken,
uint256 createdAtEpoch
) internal returns (State memory) {
require(!self[allocationId].exists(), AllocationAlreadyExists(allocationId));

Expand All @@ -80,7 +83,8 @@ library Allocation {
closedAt: 0,
lastPOIPresentedAt: 0,
accRewardsPerAllocatedToken: accRewardsPerAllocatedToken,
accRewardsPending: 0
accRewardsPending: 0,
createdAtEpoch: createdAtEpoch
});

self[allocationId] = allocation;
Expand Down
31 changes: 20 additions & 11 deletions packages/subgraph-service/contracts/utilities/AllocationManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,14 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
* @param allocationId The id of the allocation
* @param subgraphDeploymentId The id of the subgraph deployment
* @param tokens The amount of tokens allocated
* @param currentEpoch The current epoch
*/
event AllocationCreated(
address indexed indexer,
address indexed allocationId,
bytes32 indexed subgraphDeploymentId,
uint256 tokens
uint256 tokens,
uint256 currentEpoch
);

/**
Expand Down Expand Up @@ -157,17 +159,14 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
// solhint-disable-next-line func-name-mixedcase
function __AllocationManager_init(string memory _name, string memory _version) internal onlyInitializing {
__EIP712_init(_name, _version);
__AllocationManager_init_unchained(_name, _version);
__AllocationManager_init_unchained();
}

/**
* @notice Initializes the contract
*/
// solhint-disable-next-line func-name-mixedcase
function __AllocationManager_init_unchained(
string memory _name,
string memory _version
) internal onlyInitializing {}
function __AllocationManager_init_unchained() internal onlyInitializing {}

/**
* @notice Imports a legacy allocation id into the subgraph service
Expand Down Expand Up @@ -213,12 +212,15 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
// Ensure allocation id is not reused
// need to check both subgraph service (on allocations.create()) and legacy allocations
_legacyAllocations.revertIfExists(_graphStaking(), _allocationId);

uint256 currentEpoch = _graphEpochManager().currentEpoch();
Allocation.State memory allocation = _allocations.create(
_indexer,
_allocationId,
_subgraphDeploymentId,
_tokens,
_graphRewardsManager().onSubgraphAllocationUpdate(_subgraphDeploymentId)
_graphRewardsManager().onSubgraphAllocationUpdate(_subgraphDeploymentId),
currentEpoch
);

// Check that the indexer has enough tokens available
Expand All @@ -230,23 +232,28 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
_subgraphAllocatedTokens[allocation.subgraphDeploymentId] +
allocation.tokens;

emit AllocationCreated(_indexer, _allocationId, _subgraphDeploymentId, allocation.tokens);
emit AllocationCreated(_indexer, _allocationId, _subgraphDeploymentId, allocation.tokens, currentEpoch);
return allocation;
}

/**
* @notice Present a POI to collect indexing rewards for an allocation
* This function will mint indexing rewards using the {RewardsManager} and distribute them to the indexer and delegators.
*
* To qualify for indexing rewards:
* Conditions to qualify for indexing rewards:
* - POI must be non-zero
* - POI must not be stale, i.e: older than `maxPOIStaleness`
* - allocation must not be altruistic (allocated tokens = 0)
* - allocation must be open for at least one epoch
*
* Note that indexers are required to periodically (at most every `maxPOIStaleness`) present POIs to collect rewards.
* Rewards will not be issued to stale POIs, which means that indexers are advised to present a zero POI if they are
* unable to present a valid one to prevent being locked out of future rewards.
*
* Note on allocation duration restriction: this is required to ensure that non protocol chains have a valid block number for
* which to calculate POIs. EBO posts once per epoch typically at each epoch change, so we restrict rewards to allocations
* that have gone through at least one epoch change.
*
* Emits a {IndexingRewardsCollected} event.
*
* @param _allocationId The id of the allocation to collect rewards for
Expand All @@ -260,10 +267,12 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
Allocation.State memory allocation = _allocations.get(_allocationId);
require(allocation.isOpen(), AllocationManagerAllocationClosed(_allocationId));

uint256 currentEpoch = _graphEpochManager().currentEpoch();

// Mint indexing rewards if all conditions are met
uint256 tokensRewards = (!allocation.isStale(maxPOIStaleness) &&
!allocation.isAltruistic() &&
_poi != bytes32(0))
_poi != bytes32(0)) && currentEpoch > allocation.createdAtEpoch
? _graphRewardsManager().takeRewards(_allocationId)
: 0;

Expand Down Expand Up @@ -318,7 +327,7 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
tokensIndexerRewards,
tokensDelegationRewards,
_poi,
_graphEpochManager().currentEpoch()
currentEpoch
);

// Check if the indexer is over-allocated and close the allocation if necessary
Expand Down
9 changes: 9 additions & 0 deletions packages/subgraph-service/hardhat.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ if (process.env.BUILD_RUN !== 'true') {

const config: HardhatUserConfig = {
...hardhatBaseConfig,
solidity: {
version: '0.8.27',
settings: {
optimizer: {
enabled: true,
runs: 50,
},
},
},
graph: {
deployments: {
...hardhatBaseConfig.graph?.deployments,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ contract MockRewardsManager is IRewardsManager {

function getAccRewardsPerAllocatedToken(bytes32) external view returns (uint256, uint256) {}

function getRewards(address) external view returns (uint256) {}
function getRewards(address,address) external view returns (uint256) {}

function calcRewards(uint256, uint256) external pure returns (uint256) {}

Expand Down
Loading