Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Update rewards reclaiming
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## issuance-baseline #1279 +/- ##
=====================================================
+ Coverage 83.44% 86.95% +3.50%
=====================================================
Files 42 46 +4
Lines 2120 2491 +371
Branches 638 745 +107
=====================================================
+ Hits 1769 2166 +397
+ Misses 351 325 -26
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Tests: - Remove .skip from rewards test suites in contracts-test - Re-enable forge test and lint in subgraph-service package.json Workspace: - Remove issuance, deployment exclusions from pnpm-workspace.yaml - Restore toolshed issuance dependency to workspace:^ - Delete issuance types placeholders (no longer needed) - Update pnpm-lock.yaml Revert "fix(build): add placeholder issuance types for baseline build" This reverts commit 4288133.
Hardhat 3.x requires Node.js 22+ due to use of Iterator.prototype.flatMap
Move public members from toolshed into IIssuanceAllocator. Toolshed interfaces should only aggregate for type generation. See docs/InterfaceConsolidationPattern.md
Move public members from toolshed into IRewardsManager. Toolshed interfaces should only aggregate for type generation. See docs/InterfaceConsolidationPattern.md
Move public members from toolshed into IDisputeManager. Toolshed interfaces should only aggregate for type generation. See docs/InterfaceConsolidationPattern.md
Move public members from toolshed into IAllocationManager. Toolshed interfaces should only aggregate for type generation. See docs/InterfaceConsolidationPattern.md
Move public members from toolshed into IDataServicePausable. Toolshed interfaces should only aggregate for type generation. See docs/InterfaceConsolidationPattern.md
Move public storage getters from ISubgraphServiceToolshed into ISubgraphService. Toolshed interfaces should only aggregate for type generation, not define unique function signatures. See docs/InterfaceConsolidationPattern.md
Creates a local OZ5-compatible IGraphToken interface to resolve dependency conflicts between issuance package (OZ5) and interfaces package (OZ3). The minimal interface includes only IERC20 + mint(), which is all the issuance contracts require.
Correct acceptProxy and acceptProxyAndCall signatures. Interface mistakenly had single-param signatures from GraphUpgradeable instead of two-param signatures from GraphProxyAdmin.
Rename clarifies semantic meaning: these are conditions under which rewards behavior changes (staleness, denial, etc.), not reasons explaining why something happened. The term "condition" better reflects that these are states checked during reward collection that determine how rewards are handled.
Add NO_SIGNAL, BELOW_MINIMUM_SIGNAL, NO_ALLOCATION conditions. Enables granular reclaim routing for edge cases where rewards cannot be distributed to indexers.
Fallback address when reason-specific reclaim address not set. Adds setDefaultReclaimAddress (governor) and getDefaultReclaimAddress.
Switch from AccessControlUpgradeable to AccessControlEnumerableUpgradeable to enable on-chain role enumeration via getRoleMemberCount() and getRoleMember(). This enables deployment verification scripts to enumerate all role holders without relying on event indexing.
Renames getRewardsIssuancePerBlock() to getAllocatedIssuancePerBlock() and adds getRawIssuancePerBlock() to expose the raw storage value. This separation enables debugging IssuanceAllocator configuration and verifying allocator adjustments are applied correctly. See docs/RewardsManagerIssuanceSplit.md
Unused bytes parameter removed from event and _reclaimRewards.
Prevents indexers from claiming rewards earned while a subgraph was denied. Pre-denial rewards remain claimable after undeny. Key changes: - _setDenied() calls onSubgraphAllocationUpdate() BEFORE state change - onSubgraphAllocationUpdate() reclaims rewards when denied - accRewardsPerAllocatedToken freezes during denied period See docs/DeniedSubgraphRewardsAnalysis.md
… handling Major refactor of _presentPoi() for clarity and denial support: - Extract _distributeIndexingRewards() to separate distribution concerns - Single condition determination block instead of scattered if-else - POIPresented event now emits condition for off-chain visibility - Remove unnecessary ALTRUISTIC_ALLOCATION reclaim - Add SUBGRAPH_DENIED condition handling See docs/AllocationManagerRewardCollectionRefactor.md
Consolidate reward claimability checks into single helper. Returns new rewards, signal amount, and denial condition (SUBGRAPH_DENIED, BELOW_MINIMUM_SIGNAL, or NONE).
Simplify denial logic with single reclaim attempt. Evaluates both denial conditions upfront, emits appropriate events, then uses first applicable reclaim address. Prefers SUBGRAPH_DENIED over INDEXER_INELIGIBLE when both apply.
Add fallback to defaultReclaimAddress. Priority: reason-specific address → default address → drop. Early return on zero rewards.
Reclaim rewards when no signal exists. Uses _getNewRewardsPerSignal to detect zero-signal periods and routes issuance to NO_SIGNAL reclaim address instead of dropping.
Extract helper to sum allocated tokens across all issuers. Enables consistent token counting for reward calculations when multiple reward issuers exist (Staking + SubgraphService).
- Adjust expected rewards to account for pre-allocation reclaim - Use evm_setAutomine to batch signal mints in same block - Move b1 snapshot after allocation creation - Check only subgraph1 rewards (subgraph2 has no allocation) - rewards-calculations: update expected subgraph rewards from 1400 to 1000 - rewards-distribution: batch signals in same block, check sg1 only - rewards-interface: update IRewardsManager interface ID
RembrandtK
force-pushed
the
issuance-audit
branch
from
37a5a25 to
f830204
Compare
Add tests to verify timestamp is always updated when transitioning between zero and non-zero issuance rates, preventing over-issuance of rewards for periods when issuance was disabled.
Avoid redundant SLOADs by using local variables for accRewardsForSubgraph and the already-cached accRewardsPerAllocatedToken.
…gnalUpdate Avoid redundant SLOADs by using local variables instead of re-reading storage after writes.
| * @dev Gets the effective issuance per block, taking into account the IssuanceAllocator if set | ||
| */ | ||
| function getRewardsIssuancePerBlock() public view override returns (uint256) { | ||
| function getAllocatedIssuancePerBlock() public view override returns (uint256) { |
There was a problem hiding this comment.
Do we have off-chain components using this function? This would be a breaking change for them
There was a problem hiding this comment.
No, this is not deployed yet.
Previously, rewards were silently dropped when signal/allocation hooks were called on non-claimable subgraphs, causing permanent loss. Both hooks now consistently reclaim rewards for SUBGRAPH_DENIED, BELOW_MINIMUM_SIGNAL, and NO_ALLOCATION conditions. Hook Changes (onSubgraphSignalUpdate & onSubgraphAllocationUpdate): - Add reward reclaim logic for all non-claimable conditions - When not claimable: reclaim immediately, accRewardsForSubgraph NOT updated - When claimable: add to accRewardsForSubgraph, increase accRewardsPerAllocatedToken - Centralize condition checking in _getSubgraphRewardsState helper - Implement fallback chain: when multiple conditions apply, prefer ones with configured reclaim addresses (DENIED → BELOW_MINIMUM → NO_ALLOCATION) - Use updateAccRewardsPerSignal for same-block caching and prompt NO_SIGNAL reclaim - Add early return when both snapshots already current (gas optimization) - Skip storage writes when accumulators unchanged (gas optimization) View Function Changes: - getAccRewardsForSubgraph: exclude new rewards for all reclaim conditions - getAccRewardsPerAllocatedToken: only include rewards when claimable - getRewards: reflect deterministic exclusions (denied, below minimum, no allocations) - Note: indexer eligibility is NOT checked by view functions - it can change independently and is only validated at claim time Implementation Details: - _getSubgraphRewardsState: centralized helper returns condition + allocated tokens - Preserves accRewardsPerAllocatedToken value when subgraphAllocatedTokens=0 (returns stored value instead of 0, maintains accumulator for when allocations return) - Track undistributed rewards (accRewardsForSubgraph - snapshot) for proper reclaim Testing: - Add rewards-signal-allocation-update.test.ts: comprehensive tests for same-block signal/allocation updates and edge cases where per-signal delta=0 - Update existing tests for new accumulator behavior and reclaim conditions Documentation: - Add RewardConditions.md: complete reference table of all conditions, handling layers, and reward outcomes - Remove DeniedSubgraphRewardsAnalysis.md (content consolidated) - Update RewardAccountingSafety.md accumulator behavior description
Use semantic constant consistent with rest of code.
Call onSubgraphAllocationUpdate when _presentPoi exits early for ALLOCATION_TOO_YOUNG or SUBGRAPH_DENIED conditions. Keeps reward tracking and reclaim accumulation current even when rewards aren't collected, ensuring accumulation or reclaims in predictable period. Also cache currentEpoch to avoid duplicate external call.
2 participants
Uh oh!
There was an error while loading. Please reload this page.