From 168bc53494c54fe81b85269d5baa4cf3da94f4ac Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Mon, 26 Oct 2026 17:43:56 -0300 Subject: [PATCH] test(issuance): port RewardsManager accounting and snapshot inversion tests to Foundry --- .../rewards-signal-allocation-update.test.ts | 564 ------------------ .../rewards-snapshot-inversion.test.ts | 444 -------------- .../test/unit/rewards-manager/reclaim.t.sol | 5 - .../test/unit/rewards-manager/shared.t.sol | 15 + .../signalAllocationUpdate.t.sol | 303 ++++++++++ .../rewards-manager/snapshotInversion.t.sol | 302 ++++++++++ .../contracts-migration/test-mapping/t5e.md | 31 + 7 files changed, 651 insertions(+), 1013 deletions(-) delete mode 100644 packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts delete mode 100644 packages/contracts-test/tests/unit/rewards/rewards-snapshot-inversion.test.ts create mode 100644 packages/issuance/test/unit/rewards-manager/signalAllocationUpdate.t.sol create mode 100644 packages/issuance/test/unit/rewards-manager/snapshotInversion.t.sol create mode 100644 scripts/contracts-migration/test-mapping/t5e.md diff --git a/packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts b/packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts deleted file mode 100644 index 930c2b21c..000000000 --- a/packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts +++ /dev/null @@ -1,564 +0,0 @@ -import { Curation } from '@graphprotocol/contracts' -import { GraphToken } from '@graphprotocol/contracts' -import { IStaking } from '@graphprotocol/contracts' -import { RewardsManager } from '@graphprotocol/contracts' -import { deriveChannelKey, GraphNetworkContracts, helpers, randomHexBytes, toBN, toGRT } from '@graphprotocol/sdk' -import type { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' -import { expect } from 'chai' -import { constants } from 'ethers' -import hre from 'hardhat' - -import { NetworkFixture } from '../lib/fixtures' - -const { HashZero } = constants - -/** - * Invariant: signal/allocation update accounting. - * - * When `onSubgraphSignalUpdate()` runs before `onSubgraphAllocationUpdate()` in the SAME BLOCK, - * the per-signal delta is zero. Rewards already tracked in `accRewardsForSubgraph` must still be - * distributed to allocations via the snapshot delta - * (`accRewardsForSubgraph - accRewardsForSubgraphSnapshot`), rather than relying on the per-signal - * delta alone. Distribution must never depend on the ordering of these two calls within a block. - * - * IMPORTANT: These tests use evm_setAutomine to batch transactions into one block so the - * per-signal delta is zero, exercising the snapshot-delta path. - */ -describe('Rewards: Signal and Allocation Update Accounting', () => { - const graph = hre.graph() - let governor: SignerWithAddress - let curator: SignerWithAddress - let indexer: SignerWithAddress - - let fixture: NetworkFixture - let contracts: GraphNetworkContracts - let grt: GraphToken - let curation: Curation - let staking: IStaking - let rewardsManager: RewardsManager - - const channelKey = deriveChannelKey() - const subgraphDeploymentID = randomHexBytes() - const allocationID = channelKey.address - const metadata = HashZero - - const ISSUANCE_PER_BLOCK = toBN('200000000000000000000') // 200 GRT every block - const tokensToSignal = toGRT('1000') - const tokensToStake = toGRT('100000') - const tokensToAllocate = toGRT('10000') - - before(async function () { - ;[curator, indexer] = await graph.getTestAccounts() - ;({ governor } = await graph.getNamedAccounts()) - - fixture = new NetworkFixture(graph.provider) - contracts = await fixture.load(governor) - grt = contracts.GraphToken as GraphToken - curation = contracts.Curation as Curation - staking = contracts.Staking as IStaking - rewardsManager = contracts.RewardsManager as RewardsManager - - // Set the staking contract as the subgraph service so it can call takeRewards - await rewardsManager.connect(governor).setSubgraphService(staking.address) - }) - - beforeEach(async function () { - await fixture.setUp() - }) - - afterEach(async function () { - await fixture.tearDown() - }) - - async function setupSubgraphWithAllocation() { - // Setup: curator signals on subgraph - await grt.connect(governor).mint(curator.address, tokensToSignal) - await grt.connect(curator).approve(curation.address, tokensToSignal) - await curation.connect(curator).mint(subgraphDeploymentID, tokensToSignal, 0) - - // Setup: indexer stakes and allocates - await grt.connect(governor).mint(indexer.address, tokensToStake) - await grt.connect(indexer).approve(staking.address, tokensToStake) - await staking.connect(indexer).stake(tokensToStake) - await staking - .connect(indexer) - .allocateFrom( - indexer.address, - subgraphDeploymentID, - tokensToAllocate, - allocationID, - metadata, - await channelKey.generateProof(indexer.address), - ) - } - - describe('onSubgraphSignalUpdate followed by onSubgraphAllocationUpdate', function () { - it('should properly distribute rewards when signal update precedes allocation update (same block)', async function () { - await setupSubgraphWithAllocation() - - // Advance blocks to accumulate rewards - await helpers.mine(100) - - // Get expected rewards before any updates - const expectedRewardsForSubgraph = await rewardsManager.getAccRewardsForSubgraph(subgraphDeploymentID) - expect(expectedRewardsForSubgraph).to.be.gt(0, 'Should have accumulated rewards') - - // Get initial state - const subgraphBefore = await rewardsManager.subgraphs(subgraphDeploymentID) - const accRewardsPerAllocatedTokenBefore = subgraphBefore.accRewardsPerAllocatedToken - - // Disable automine to batch transactions into the same block - await hre.network.provider.send('evm_setAutomine', [false]) - - try { - // First: call onSubgraphSignalUpdate (this zeros the per-signal delta) - // This simulates what happens when a curator mints/burns signal - const signalTx = await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID) - - // Second: call onSubgraphAllocationUpdate (in same block, per-signal delta is 0) - // This simulates what happens when an allocation is opened/closed - const allocTx = await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - - // Mine both transactions in the same block - await hre.network.provider.send('evm_mine') - - // Wait for both transactions to be mined - await signalTx.wait() - await allocTx.wait() - } finally { - // Re-enable automine - await hre.network.provider.send('evm_setAutomine', [true]) - } - - // Verify rewards were tracked at subgraph level - const subgraphAfterSignal = await rewardsManager.subgraphs(subgraphDeploymentID) - expect(subgraphAfterSignal.accRewardsForSubgraph).to.be.gt( - 0, - 'accRewardsForSubgraph should be updated after signal update', - ) - - // Get final state - const subgraphAfterAllocation = await rewardsManager.subgraphs(subgraphDeploymentID) - - // accRewardsPerAllocatedToken must advance via the snapshot delta even when the per-signal - // delta is zero, so accumulated rewards reach allocations regardless of update ordering. - expect(subgraphAfterAllocation.accRewardsPerAllocatedToken).to.be.gt( - accRewardsPerAllocatedTokenBefore, - 'accRewardsPerAllocatedToken should increase when a signal update precedes an allocation update in the same block', - ) - - // Verify snapshot consistency - expect(subgraphAfterAllocation.accRewardsForSubgraphSnapshot).to.equal( - subgraphAfterAllocation.accRewardsForSubgraph, - 'Snapshots should be in sync after updates', - ) - }) - - it('should not brick rewards when signal update zeros the per-signal delta (same block)', async function () { - await setupSubgraphWithAllocation() - - // Advance blocks - await helpers.mine(100) - - // Get the view function result (what rewards SHOULD be) before any updates - // Note: We call this to ensure the function works, but we verify via stored state below - await rewardsManager.getAccRewardsPerAllocatedToken(subgraphDeploymentID) - - // Disable automine to batch transactions into the same block - await hre.network.provider.send('evm_setAutomine', [false]) - - try { - // Call signal update first (zeros per-signal delta and accumulates rewards in accRewardsForSubgraph) - const signalTx = await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID) - - // Call allocation update (per-signal delta is now 0, but rewards are in accRewardsForSubgraph) - const allocTx = await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - - // Mine both transactions in the same block - await hre.network.provider.send('evm_mine') - - // Wait for both transactions to be mined - await signalTx.wait() - await allocTx.wait() - } finally { - // Re-enable automine - await hre.network.provider.send('evm_setAutomine', [true]) - } - - // Get the rewards accumulated in accRewardsForSubgraph - const afterSignal = await rewardsManager.subgraphs(subgraphDeploymentID) - const rewardsAccumulated = afterSignal.accRewardsForSubgraph - - // These rewards should eventually be distributed to allocations - expect(rewardsAccumulated).to.be.gt(0, 'Rewards should be accumulated at subgraph level') - - // Get stored state - const subgraph = await rewardsManager.subgraphs(subgraphDeploymentID) - - // Even when the per-signal delta is zero, accRewardsPerAllocatedToken must reflect the - // rewards accumulated in accRewardsForSubgraph via the snapshot delta, so they are - // distributed to allocations rather than stranded. - expect(subgraph.accRewardsPerAllocatedToken).to.be.gt( - 0, - 'accRewardsPerAllocatedToken should be non-zero so accumulated rewards are distributed, not stranded', - ) - - // Verify view function and stored state are consistent - const [viewAccRewardsPerAllocatedToken] = - await rewardsManager.getAccRewardsPerAllocatedToken(subgraphDeploymentID) - - // The view should equal the stored value (since snapshots are synced) - expect(viewAccRewardsPerAllocatedToken).to.equal( - subgraph.accRewardsPerAllocatedToken, - 'View function should match stored state after updates', - ) - }) - - it('should handle multiple signal updates without losing rewards (same block allocation)', async function () { - await setupSubgraphWithAllocation() - - // Advance blocks - await helpers.mine(50) - - // First signal update - await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID) - const afterFirstSignal = await rewardsManager.subgraphs(subgraphDeploymentID) - - // Advance more blocks - await helpers.mine(50) - - // Disable automine to batch signal + allocation into same block - await hre.network.provider.send('evm_setAutomine', [false]) - - try { - // Second signal update (without allocation update in between) - const signalTx = await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID) - - // Allocation update in the same block (per-signal delta is 0) - const allocTx = await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - - // Mine both in the same block - await hre.network.provider.send('evm_mine') - - await signalTx.wait() - await allocTx.wait() - } finally { - await hre.network.provider.send('evm_setAutomine', [true]) - } - - const afterSecondSignal = await rewardsManager.subgraphs(subgraphDeploymentID) - - // Rewards should have accumulated - expect(afterSecondSignal.accRewardsForSubgraph).to.be.gt( - afterFirstSignal.accRewardsForSubgraph, - 'Rewards should accumulate across signal updates', - ) - - const afterAllocation = await rewardsManager.subgraphs(subgraphDeploymentID) - - // All accumulated rewards should be distributed - expect(afterAllocation.accRewardsPerAllocatedToken).to.be.gt( - 0, - 'Rewards from multiple signal updates should be distributed', - ) - - // Snapshots should be in sync - expect(afterAllocation.accRewardsForSubgraphSnapshot).to.equal( - afterAllocation.accRewardsForSubgraph, - 'Snapshots should be in sync', - ) - }) - }) - - describe('snapshot consistency in reclaim paths', function () { - it('should update accRewardsForSubgraphSnapshot when rewards are reclaimed due to denial', async function () { - await setupSubgraphWithAllocation() - - // Deny the subgraph - await rewardsManager.connect(governor).setSubgraphAvailabilityOracle(governor.address) - await rewardsManager.connect(governor).setDenied(subgraphDeploymentID, true) - - // Advance blocks to accumulate rewards - await helpers.mine(100) - - // Get state before - const subgraphBefore = await rewardsManager.subgraphs(subgraphDeploymentID) - - // Call allocation update - should reclaim (not distribute) rewards - await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - - // Get state after - const subgraphAfter = await rewardsManager.subgraphs(subgraphDeploymentID) - - // accRewardsPerAllocatedToken should NOT increase (rewards reclaimed, not distributed) - expect(subgraphAfter.accRewardsPerAllocatedToken).to.equal( - subgraphBefore.accRewardsPerAllocatedToken, - 'accRewardsPerAllocatedToken should not increase when denied', - ) - - // accRewardsForSubgraphSnapshot must advance in the reclaim path so the same rewards - // cannot be reclaimed again on a later update. - expect(subgraphAfter.accRewardsForSubgraphSnapshot).to.be.gte( - subgraphBefore.accRewardsForSubgraphSnapshot, - 'accRewardsForSubgraphSnapshot should be updated in reclaim path', - ) - }) - - it('should not double-reclaim rewards after snapshot update', async function () { - await setupSubgraphWithAllocation() - - // Deny the subgraph - await rewardsManager.connect(governor).setSubgraphAvailabilityOracle(governor.address) - await rewardsManager.connect(governor).setDenied(subgraphDeploymentID, true) - - // Advance blocks - await helpers.mine(100) - - // First allocation update - reclaims rewards - await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - const afterFirstReclaim = await rewardsManager.subgraphs(subgraphDeploymentID) - - // Second allocation update - each tx advances a block, so there's 1 more block of rewards - // The key invariant is that rewards are properly accounted for, not double-reclaimed - await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - const afterSecondReclaim = await rewardsManager.subgraphs(subgraphDeploymentID) - - // The snapshot should have advanced by at most 1 block's worth of rewards - // (Each transaction creates a new block in Hardhat) - const maxOneBlockReward = ISSUANCE_PER_BLOCK.mul(tokensToSignal).div(await grt.balanceOf(curation.address)) - - const snapshotDiff = afterSecondReclaim.accRewardsForSubgraphSnapshot.sub( - afterFirstReclaim.accRewardsForSubgraphSnapshot, - ) - - // The difference should be at most one block's worth of rewards - expect(snapshotDiff).to.be.lte( - maxOneBlockReward.mul(2), // Allow for rounding and timing - 'Should only process one block worth of new rewards', - ) - - // Verify accRewardsPerAllocatedToken didn't increase (rewards still reclaimed, not distributed) - expect(afterSecondReclaim.accRewardsPerAllocatedToken).to.equal( - afterFirstReclaim.accRewardsPerAllocatedToken, - 'accRewardsPerAllocatedToken should not change during reclaim', - ) - }) - }) - - describe('onSubgraphSignalUpdate on denied subgraph', function () { - it('should reclaim rewards when onSubgraphSignalUpdate is called on denied subgraph', async function () { - await setupSubgraphWithAllocation() - - // Configure reclaim address for SUBGRAPH_DENIED - const SUBGRAPH_DENIED = hre.ethers.utils.id('SUBGRAPH_DENIED') - await rewardsManager.connect(governor).setReclaimAddress(SUBGRAPH_DENIED, governor.address) - - // Verify reclaim address was set - const reclaimAddr = await rewardsManager.getReclaimAddress(SUBGRAPH_DENIED) - expect(reclaimAddr).to.equal(governor.address, 'Reclaim address should be set') - - // Deny the subgraph - await rewardsManager.connect(governor).setSubgraphAvailabilityOracle(governor.address) - await rewardsManager.connect(governor).setDenied(subgraphDeploymentID, true) - - // Record state after denial (setDenied calls onSubgraphAllocationUpdate internally) - const afterDenial = await rewardsManager.subgraphs(subgraphDeploymentID) - - // Advance blocks - rewards should accumulate - await helpers.mine(100) - - // Call onSubgraphSignalUpdate (simulates curator action) - // For a denied subgraph, rewards are reclaimed immediately rather than accumulated. - const tx = await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID) - const receipt = await tx.wait() - const afterSignalUpdate = await rewardsManager.subgraphs(subgraphDeploymentID) - - // For a denied subgraph, accRewardsForSubgraph must NOT change - // (rewards are reclaimed directly, not stored) - expect(afterSignalUpdate.accRewardsForSubgraph).to.equal( - afterDenial.accRewardsForSubgraph, - 'accRewardsForSubgraph should not change for denied subgraphs (rewards reclaimed)', - ) - - // Verify reclaim event was emitted - const reclaimEvent = receipt.events?.find((e) => e.event === 'RewardsReclaimed') - expect(reclaimEvent).to.not.be.undefined - // Event args: (reason, rewards, indexer, allocationId, subgraphDeploymentId) - const rewards = reclaimEvent!.args![1] // rewards is second arg - expect(rewards).to.be.gt(0, 'Should have reclaimed rewards') - }) - - it('should accumulate rewards for claimable subgraphs in onSubgraphSignalUpdate', async function () { - await setupSubgraphWithAllocation() - - // Record initial state (subgraph is claimable by default) - const initialState = await rewardsManager.subgraphs(subgraphDeploymentID) - - // Advance blocks - rewards should accumulate - await helpers.mine(100) - - // Call onSubgraphSignalUpdate - await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID) - const afterSignalUpdate = await rewardsManager.subgraphs(subgraphDeploymentID) - - // For claimable subgraphs: accRewardsForSubgraph SHOULD increase - expect(afterSignalUpdate.accRewardsForSubgraph).to.be.gt( - initialState.accRewardsForSubgraph, - 'accRewardsForSubgraph should increase for claimable subgraphs', - ) - }) - - it('view function getAccRewardsForSubgraph should not jump during denial', async function () { - await setupSubgraphWithAllocation() - - // Accumulate some rewards while claimable - await helpers.mine(50) - - // Deny the subgraph (setDenied distributes pre-denial rewards via onSubgraphAllocationUpdate) - await rewardsManager.connect(governor).setSubgraphAvailabilityOracle(governor.address) - await rewardsManager.connect(governor).setDenied(subgraphDeploymentID, true) - - // Record view value immediately after denial - const rewardsAtDenial = await rewardsManager.getAccRewardsForSubgraph(subgraphDeploymentID) - expect(rewardsAtDenial).to.be.gt(0, 'Should have accumulated pre-denial rewards') - - // Advance blocks during denial - await helpers.mine(100) - - // View function should return SAME value (no jump up during denial) - const rewardsDuringDenial = await rewardsManager.getAccRewardsForSubgraph(subgraphDeploymentID) - expect(rewardsDuringDenial).to.equal(rewardsAtDenial, 'View should not increase during denial') - - // A signal update on a denied subgraph reclaims the accumulated rewards, so the view stays - // stable and does not jump on the next allocation update. - // Configure reclaim address so rewards are reclaimed - const SUBGRAPH_DENIED = hre.ethers.utils.id('SUBGRAPH_DENIED') - await rewardsManager.connect(governor).setReclaimAddress(SUBGRAPH_DENIED, governor.address) - await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID) - - // View function should STILL return same value (rewards reclaimed, not accumulated) - const rewardsAfterSignalUpdate = await rewardsManager.getAccRewardsForSubgraph(subgraphDeploymentID) - expect(rewardsAfterSignalUpdate).to.equal(rewardsAtDenial, 'View should not jump after signal update') - - // Mine more blocks - await helpers.mine(50) - - // Call allocation update - await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - - // View should STILL be stable (rewards reclaimed, not accumulated) - const rewardsAfterAllocationUpdate = await rewardsManager.getAccRewardsForSubgraph(subgraphDeploymentID) - expect(rewardsAfterAllocationUpdate).to.equal(rewardsAtDenial, 'View should not jump after allocation update') - }) - }) - - describe('onSubgraphSignalUpdate with no allocations', function () { - it('should reclaim as NO_ALLOCATED_TOKENS when signal exists but no allocations', async function () { - // Setup: only signal, no allocation - await grt.connect(governor).mint(curator.address, tokensToSignal) - await grt.connect(curator).approve(curation.address, tokensToSignal) - await curation.connect(curator).mint(subgraphDeploymentID, tokensToSignal, 0) - - // Configure reclaim address for NO_ALLOCATED_TOKENS - const NO_ALLOCATED_TOKENS = hre.ethers.utils.id('NO_ALLOCATED_TOKENS') - await rewardsManager.connect(governor).setReclaimAddress(NO_ALLOCATED_TOKENS, governor.address) - - // Record initial state - const initialState = await rewardsManager.subgraphs(subgraphDeploymentID) - - // Advance blocks - rewards should accumulate - await helpers.mine(100) - - // Call onSubgraphSignalUpdate - should reclaim as NO_ALLOCATED_TOKENS - const tx = await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID) - const receipt = await tx.wait() - const afterSignalUpdate = await rewardsManager.subgraphs(subgraphDeploymentID) - - // accRewardsForSubgraph should NOT change (rewards reclaimed, not accumulated) - expect(afterSignalUpdate.accRewardsForSubgraph).to.equal( - initialState.accRewardsForSubgraph, - 'accRewardsForSubgraph should not change when no allocations (rewards reclaimed)', - ) - - // Verify reclaim event was emitted with NO_ALLOCATED_TOKENS reason - const reclaimEvent = receipt.events?.find((e) => e.event === 'RewardsReclaimed') - expect(reclaimEvent).to.not.be.undefined - expect(reclaimEvent!.args![0]).to.equal(NO_ALLOCATED_TOKENS, 'Should reclaim with NO_ALLOCATED_TOKENS reason') - expect(reclaimEvent!.args![1]).to.be.gt(0, 'Should have reclaimed rewards') - }) - - it('view function should not show phantom rewards when no allocations', async function () { - // Setup: only signal, no allocation - await grt.connect(governor).mint(curator.address, tokensToSignal) - await grt.connect(curator).approve(curation.address, tokensToSignal) - await curation.connect(curator).mint(subgraphDeploymentID, tokensToSignal, 0) - - // Record view immediately after signal - const viewAfterSignal = await rewardsManager.getAccRewardsForSubgraph(subgraphDeploymentID) - - // Advance blocks - await helpers.mine(100) - - // Configure reclaim and call signal update - const NO_ALLOCATED_TOKENS = hre.ethers.utils.id('NO_ALLOCATED_TOKENS') - await rewardsManager.connect(governor).setReclaimAddress(NO_ALLOCATED_TOKENS, governor.address) - await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID) - - // View should remain stable (rewards reclaimed) - const viewAfterReclaim = await rewardsManager.getAccRewardsForSubgraph(subgraphDeploymentID) - expect(viewAfterReclaim).to.equal(viewAfterSignal, 'View should not grow when no allocations') - }) - }) - - describe('invariant: no rewards lost or double-counted', function () { - it('should maintain accounting invariant across mixed updates (with same-block scenarios)', async function () { - await setupSubgraphWithAllocation() - - // Sequence exercising the same-block signal/allocation ordering - await helpers.mine(25) - - // First: signal update followed by allocation update in SAME BLOCK - await hre.network.provider.send('evm_setAutomine', [false]) - try { - const signalTx1 = await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID) - const allocTx1 = await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - await hre.network.provider.send('evm_mine') - await signalTx1.wait() - await allocTx1.wait() - } finally { - await hre.network.provider.send('evm_setAutomine', [true]) - } - - await helpers.mine(25) - - // Second: double signal update followed by allocation update in SAME BLOCK - await hre.network.provider.send('evm_setAutomine', [false]) - try { - const signalTx2 = await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID) - const signalTx3 = await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID) - const allocTx2 = await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - await hre.network.provider.send('evm_mine') - await signalTx2.wait() - await signalTx3.wait() - await allocTx2.wait() - } finally { - await hre.network.provider.send('evm_setAutomine', [true]) - } - - // Final state check - const finalSubgraph = await rewardsManager.subgraphs(subgraphDeploymentID) - - // Key invariant: snapshots should be in sync - expect(finalSubgraph.accRewardsForSubgraphSnapshot).to.equal( - finalSubgraph.accRewardsForSubgraph, - 'INVARIANT VIOLATED: accRewardsForSubgraphSnapshot != accRewardsForSubgraph', - ) - - // Rewards should have been distributed - expect(finalSubgraph.accRewardsPerAllocatedToken).to.be.gt( - 0, - 'Rewards should have been distributed to allocations', - ) - }) - }) -}) diff --git a/packages/contracts-test/tests/unit/rewards/rewards-snapshot-inversion.test.ts b/packages/contracts-test/tests/unit/rewards/rewards-snapshot-inversion.test.ts deleted file mode 100644 index da09d182e..000000000 --- a/packages/contracts-test/tests/unit/rewards/rewards-snapshot-inversion.test.ts +++ /dev/null @@ -1,444 +0,0 @@ -import { Curation } from '@graphprotocol/contracts' -import { GraphToken } from '@graphprotocol/contracts' -import { IStaking } from '@graphprotocol/contracts' -import { RewardsManager } from '@graphprotocol/contracts' -import { deriveChannelKey, GraphNetworkContracts, helpers, randomHexBytes, toGRT } from '@graphprotocol/sdk' -import type { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' -import { expect } from 'chai' -import { BigNumber, constants, utils } from 'ethers' -import hre from 'hardhat' - -import { NetworkFixture } from '../lib/fixtures' - -const { HashZero } = constants - -/** - * Tests for snapshot inversion on upgrade. - * - * Terminology: - * A = accRewardsForSubgraph (stored accumulator, set at signal updates) - * S = accRewardsForSubgraphSnapshot (stored snapshot, set at allocation updates) - * P = rewardsSinceSignalSnapshot (pending rewards since last signal snapshot) - * - * For affected subgraphs, on-chain storage can hold an inverted snapshot state - * where A < S: the snapshot S leads (it was written from a view value of - * storage + pending) while the accumulator A lags at its stored value. - * - * Invariant the reward math must uphold: with an inverted state (A < S), the - * computation of A + P - S must never underflow. It does so by adding pending - * first and subtracting the snapshot last — `A.add(P).sub(S)` — so the - * intermediate `A + P` stays non-negative. Since P covers T1→now and the gap - * S - A covers T1→T2, and now >= T2, we have S - A <= P, so S <= A + P always - * holds; no clamping is needed. Subtracting the gap S - A discards rewards - * already distributed, preventing double-counting. - * - * These tests use `hardhat_setStorageAt` to construct the inverted storage state - * directly so the invariant can be exercised for affected subgraphs. - */ -describe('Rewards: Snapshot Inversion', () => { - const graph = hre.graph() - let governor: SignerWithAddress - let curator: SignerWithAddress - let indexer: SignerWithAddress - - let fixture: NetworkFixture - let contracts: GraphNetworkContracts - let grt: GraphToken - let curation: Curation - let staking: IStaking - let rewardsManager: RewardsManager - - const channelKey = deriveChannelKey() - const subgraphDeploymentID = randomHexBytes() - const allocationID = channelKey.address - const metadata = HashZero - - const tokensToSignal = toGRT('1000') - const tokensToStake = toGRT('100000') - const tokensToAllocate = toGRT('10000') - - // Storage slot for the `subgraphs` mapping in RewardsManagerV1Storage. - // Computed by counting all inherited storage variables: - // Managed: controller(0), _addressCache(1), __gap[10](2-11) = 12 slots - // V1Storage: __DEPRECATED_issuanceRate(12), accRewardsPerSignal(13), - // accRewardsPerSignalLastBlockUpdated(14), subgraphAvailabilityOracle(15), - // subgraphs(16) - const SUBGRAPHS_MAPPING_SLOT = 16 - - /** - * Compute the storage slot for a field within a Subgraph struct in the subgraphs mapping. - * - * For `mapping(bytes32 => Subgraph)` at slot S, key K: - * base = keccak256(abi.encode(K, S)) - * field 0 (accRewardsForSubgraph) = base + 0 - * field 1 (accRewardsForSubgraphSnapshot) = base + 1 - * field 2 (accRewardsPerSignalSnapshot) = base + 2 - * field 3 (accRewardsPerAllocatedToken) = base + 3 - */ - function subgraphStorageSlot(subgraphId: string, fieldOffset: number): string { - const baseSlot = utils.keccak256( - utils.defaultAbiCoder.encode(['bytes32', 'uint256'], [subgraphId, SUBGRAPHS_MAPPING_SLOT]), - ) - return utils.hexZeroPad(BigNumber.from(baseSlot).add(fieldOffset).toHexString(), 32) - } - - /** - * Set a uint256 value at a specific storage slot of the RewardsManager proxy. - */ - async function setStorage(slot: string, value: BigNumber): Promise { - await hre.network.provider.send('hardhat_setStorageAt', [ - rewardsManager.address, - slot, - utils.hexZeroPad(value.toHexString(), 32), - ]) - } - - /** - * Create the inverted snapshot state that exists on-chain for affected subgraphs. - * - * Sets: accRewardsForSubgraphSnapshot = accRewardsForSubgraph + gap - * This is the state left by the old `onSubgraphAllocationUpdate` which wrote - * the snapshot from a view function (storage + pending), while leaving - * accRewardsForSubgraph at its stored value. - */ - async function createInvertedState(subgraphId: string, gap: BigNumber): Promise { - const subgraph = await rewardsManager.subgraphs(subgraphId) - const currentAccRewards = subgraph.accRewardsForSubgraph - const invertedSnapshot = currentAccRewards.add(gap) - - // Write accRewardsForSubgraphSnapshot = currentAccRewards + gap (field offset 1) - const snapshotSlot = subgraphStorageSlot(subgraphId, 1) - await setStorage(snapshotSlot, invertedSnapshot) - - // Verify the inversion was written correctly - const after = await rewardsManager.subgraphs(subgraphId) - expect(after.accRewardsForSubgraphSnapshot).to.equal(invertedSnapshot) - expect(after.accRewardsForSubgraph).to.be.lt(after.accRewardsForSubgraphSnapshot) - } - - before(async function () { - ;[curator, indexer] = await graph.getTestAccounts() - ;({ governor } = await graph.getNamedAccounts()) - - fixture = new NetworkFixture(graph.provider) - contracts = await fixture.load(governor) - grt = contracts.GraphToken as GraphToken - curation = contracts.Curation as Curation - staking = contracts.Staking as IStaking - rewardsManager = contracts.RewardsManager as RewardsManager - - // Set the staking contract as the subgraph service so RewardsManager - // can see allocations via _getSubgraphAllocatedTokens() - await rewardsManager.connect(governor).setSubgraphService(staking.address) - }) - - beforeEach(async function () { - await fixture.setUp() - }) - - afterEach(async function () { - await fixture.tearDown() - }) - - async function setupSubgraphWithAllocation() { - // Set issuance rate (200 GRT/block) — the fixture defaults to 0 - await rewardsManager.connect(governor).setIssuancePerBlock(toGRT('200')) - - // Curator signals on subgraph - await grt.connect(governor).mint(curator.address, tokensToSignal) - await grt.connect(curator).approve(curation.address, tokensToSignal) - await curation.connect(curator).mint(subgraphDeploymentID, tokensToSignal, 0) - - // Indexer stakes and allocates - await grt.connect(governor).mint(indexer.address, tokensToStake) - await grt.connect(indexer).approve(staking.address, tokensToStake) - await staking.connect(indexer).stake(tokensToStake) - await staking - .connect(indexer) - .allocateFrom( - indexer.address, - subgraphDeploymentID, - tokensToAllocate, - allocationID, - metadata, - await channelKey.generateProof(indexer.address), - ) - - // Accumulate some rewards - await helpers.mine(50) - - // Sync subgraph state so we have non-zero accRewardsForSubgraph - await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID) - } - - describe('storage slot verification', function () { - it('should correctly compute and write to subgraph storage slots', async function () { - await setupSubgraphWithAllocation() - - // Read current state - const before = await rewardsManager.subgraphs(subgraphDeploymentID) - expect(before.accRewardsForSubgraph).to.not.equal(0, 'precondition: should have accumulated rewards') - - // Write a known value to accRewardsForSubgraphSnapshot (field 1) - const testValue = BigNumber.from('12345678901234567890') - const snapshotSlot = subgraphStorageSlot(subgraphDeploymentID, 1) - await setStorage(snapshotSlot, testValue) - - // Read back and verify - const after = await rewardsManager.subgraphs(subgraphDeploymentID) - expect(after.accRewardsForSubgraphSnapshot).to.equal(testValue) - // Other fields should be unchanged - expect(after.accRewardsForSubgraph).to.equal(before.accRewardsForSubgraph) - expect(after.accRewardsPerSignalSnapshot).to.equal(before.accRewardsPerSignalSnapshot) - expect(after.accRewardsPerAllocatedToken).to.equal(before.accRewardsPerAllocatedToken) - }) - }) - - describe('inverted state: accumulated < snapshot', function () { - it('should not revert on onSubgraphSignalUpdate with inverted state', async function () { - await setupSubgraphWithAllocation() - - // Create the pre-upgrade inverted state (snapshot > accumulated by ~7000 GRT) - const gap = toGRT('7000') - await createInvertedState(subgraphDeploymentID, gap) - - // Advance enough blocks so P > gap. At ~200 GRT/block, 50 blocks ≈ 10,000 GRT > 7,000. - await helpers.mine(50) - - // With an inverted state (A < S), the update must not revert: pending P is - // added before the snapshot S is subtracted (A.add(P).sub(S)), so the - // intermediate A + P stays non-negative and A + P >= S always holds. - await expect(rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID)).to.not.be.reverted - }) - - it('should not revert on onSubgraphAllocationUpdate with inverted state', async function () { - await setupSubgraphWithAllocation() - - const gap = toGRT('7000') - await createInvertedState(subgraphDeploymentID, gap) - - await helpers.mine(50) - - await expect(rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID)).to.not.be.reverted - }) - - it('should sync snapshots after first successful call', async function () { - await setupSubgraphWithAllocation() - - const gap = toGRT('7000') - await createInvertedState(subgraphDeploymentID, gap) - - await helpers.mine(50) - - // First call with inverted state - await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID) - - // After processing the inverted state, snapshots should be synced - const after = await rewardsManager.subgraphs(subgraphDeploymentID) - expect(after.accRewardsForSubgraphSnapshot).to.equal( - after.accRewardsForSubgraph, - 'snapshot should equal accumulated after processing inverted state', - ) - - // Subsequent calls should work normally - await helpers.mine(10) - await expect(rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID)).to.not.be.reverted - - const afterSecond = await rewardsManager.subgraphs(subgraphDeploymentID) - expect(afterSecond.accRewardsForSubgraphSnapshot).to.equal(afterSecond.accRewardsForSubgraph) - }) - }) - - describe('accounting correctness with inverted state', function () { - it('should correctly compute undistributed rewards: (A+P).sub(S)', async function () { - await setupSubgraphWithAllocation() - - // Record state before inversion - const before = await rewardsManager.subgraphs(subgraphDeploymentID) - const perAllocBefore = before.accRewardsPerAllocatedToken - - // Create inversion with a small gap (smaller than rewards that will accrue) - const gap = toGRT('500') - await createInvertedState(subgraphDeploymentID, gap) - - // Advance enough blocks that S < A + P (i.e., new rewards exceed the gap) - // With 200 GRT/block and only one subgraph signalled, each block adds ~200 GRT of P - // 10 blocks ≈ 2000 GRT of P, gap = 500 GRT - // So (A + P) - S = A + 2000 - (A + 500) = 1500 GRT undistributed - await helpers.mine(10) - - // Call allocation update to distribute rewards - await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - - const after = await rewardsManager.subgraphs(subgraphDeploymentID) - - // accRewardsPerAllocatedToken should increase (rewards were distributed) - expect(perAllocBefore).to.be.lt(after.accRewardsPerAllocatedToken, 'should distribute rewards: 0 < (A + P) - S') - - // The distributed amount should be less than total new rewards (P) - // because the gap represents already-distributed rewards - // Undistributed = (A + P) - S = P - gap (since S = A + gap) - // If P ≈ 2000 GRT and gap = 500 GRT, undistributed ≈ 1500 GRT - // Subtracting the gap is what keeps this from being P ≈ 2000 GRT (double-counting) - - // Verify snapshots are synced - expect(after.accRewardsForSubgraphSnapshot).to.equal(after.accRewardsForSubgraph) - }) - - it('should not double-count: distributed rewards account for the gap', async function () { - await setupSubgraphWithAllocation() - - // Get a reference: how many rewards are distributed in normal operation - const stateBefore = await rewardsManager.subgraphs(subgraphDeploymentID) - - // Create a scenario where gap = 500 GRT - const gap = toGRT('500') - await createInvertedState(subgraphDeploymentID, gap) - - await helpers.mine(20) - - // Process the inverted state - await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - const afterInverted = await rewardsManager.subgraphs(subgraphDeploymentID) - const perAllocAfterInverted = afterInverted.accRewardsPerAllocatedToken - - // Now do a SECOND allocation update with normal state (snapshots are synced) - await helpers.mine(20) - await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - const afterNormal = await rewardsManager.subgraphs(subgraphDeploymentID) - - // The second update should distribute ~20 blocks worth of rewards - // The first update distributed less (because gap was subtracted) - // This proves no double-counting: the gap was properly deducted - const firstDelta = perAllocAfterInverted.sub(stateBefore.accRewardsPerAllocatedToken) - const secondDelta = afterNormal.accRewardsPerAllocatedToken.sub(perAllocAfterInverted) - - // First delta < second delta because the gap was subtracted - // (both periods have ~20 blocks, but first period deducts the 500 GRT gap) - expect(firstDelta).to.be.lt(secondDelta, 'first update should distribute less due to gap deduction') - }) - - it('should distribute exactly P - gap rewards (gap deducted from pending)', async function () { - await setupSubgraphWithAllocation() - - // Sync state so we have a clean baseline - await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - const baseline = await rewardsManager.subgraphs(subgraphDeploymentID) - const perAllocBaseline = baseline.accRewardsPerAllocatedToken - - // Create inversion with a known gap - const gap = toGRT('500') - await createInvertedState(subgraphDeploymentID, gap) - - // Mine blocks, then do a normal (non-inverted) reference run in a parallel universe - // We can't do that, but we CAN check that the gap is properly deducted by - // comparing inverted vs non-inverted runs over the same block count. - - // First: process the inverted state - await helpers.mine(20) - await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - const afterInverted = await rewardsManager.subgraphs(subgraphDeploymentID) - const invertedDelta = afterInverted.accRewardsPerAllocatedToken.sub(perAllocBaseline) - - // Second: run the same block count with synced state (no gap) - await helpers.mine(20) - await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - const afterNormal = await rewardsManager.subgraphs(subgraphDeploymentID) - const normalDelta = afterNormal.accRewardsPerAllocatedToken.sub(afterInverted.accRewardsPerAllocatedToken) - - // The inverted run should distribute LESS because the gap was subtracted. - // Both periods have ~20 blocks of rewards, but the inverted period deducts 500 GRT. - expect(invertedDelta).to.be.lt(normalDelta, 'inverted period should distribute less due to gap deduction') - expect(invertedDelta).to.not.equal(0, 'should still distribute some rewards (gap < P)') - }) - }) - - describe('normal operation (no inversion)', function () { - it('should produce identical results when A == S (synced snapshot steady state)', async function () { - await setupSubgraphWithAllocation() - - // Ensure snapshots are synced (normal state) - await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - const synced = await rewardsManager.subgraphs(subgraphDeploymentID) - expect(synced.accRewardsForSubgraphSnapshot).to.equal(synced.accRewardsForSubgraph) - - const perAllocBefore = synced.accRewardsPerAllocatedToken - - // Advance and update - this is the normal steady-state path - await helpers.mine(20) - await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - - const after = await rewardsManager.subgraphs(subgraphDeploymentID) - - // Rewards should be distributed normally - expect(perAllocBefore).to.be.lt(after.accRewardsPerAllocatedToken) - expect(after.accRewardsForSubgraphSnapshot).to.equal(after.accRewardsForSubgraph) - }) - - it('should handle zero rewards gracefully (same block, no new rewards)', async function () { - await setupSubgraphWithAllocation() - - // Sync state - await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - - // Call again immediately (same block via automine off) - await hre.network.provider.send('evm_setAutomine', [false]) - try { - const tx = await rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID) - await hre.network.provider.send('evm_mine') - await tx.wait() - } finally { - await hre.network.provider.send('evm_setAutomine', [true]) - } - - const after = await rewardsManager.subgraphs(subgraphDeploymentID) - - // Per-alloc-token should be unchanged (zero rewards in same block) - // Note: the transaction itself mines a block, so there may be minimal reward - expect(after.accRewardsForSubgraphSnapshot).to.equal(after.accRewardsForSubgraph) - }) - }) - - describe('realistic pre-upgrade scenario', function () { - it('should handle the exact Arbitrum Sepolia state pattern', async function () { - await setupSubgraphWithAllocation() - - // Simulate: - // 1. Old onSubgraphSignalUpdate wrote accRewardsForSubgraph = X (signal-level view value) - // 2. Old onSubgraphAllocationUpdate wrote accRewardsForSubgraphSnapshot = X + delta - // (via getAccRewardsForSubgraph view which returns storage + pending) - // 3. Proxy upgrade preserves this state - // 4. New code calls _updateSubgraphRewards: A.sub(S) underflows - - // Read current A value - const state = await rewardsManager.subgraphs(subgraphDeploymentID) - const A = state.accRewardsForSubgraph - - // Set S = A + 7235 GRT (matching the ~7235 GRT gap observed on Arbitrum Sepolia) - const observedGap = toGRT('7235') - const accSlot = subgraphStorageSlot(subgraphDeploymentID, 1) - await setStorage(accSlot, A.add(observedGap)) - - // Verify the inversion - const inverted = await rewardsManager.subgraphs(subgraphDeploymentID) - expect(inverted.accRewardsForSubgraph).to.be.lt(inverted.accRewardsForSubgraphSnapshot) - - // Advance blocks (some time passes after upgrade before first interaction) - await helpers.mine(50) - - // First interaction after "upgrade": should NOT revert - await expect(rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID)).to.not.be.reverted - - // State should be healed - const healed = await rewardsManager.subgraphs(subgraphDeploymentID) - expect(healed.accRewardsForSubgraphSnapshot).to.equal(healed.accRewardsForSubgraph) - - // All subsequent operations should work - await helpers.mine(10) - await expect(rewardsManager.connect(governor).onSubgraphAllocationUpdate(subgraphDeploymentID)).to.not.be.reverted - - await helpers.mine(10) - await expect(rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID)).to.not.be.reverted - }) - }) -}) diff --git a/packages/issuance/test/unit/rewards-manager/reclaim.t.sol b/packages/issuance/test/unit/rewards-manager/reclaim.t.sol index 1107a487a..32ed0ce78 100644 --- a/packages/issuance/test/unit/rewards-manager/reclaim.t.sol +++ b/packages/issuance/test/unit/rewards-manager/reclaim.t.sol @@ -43,11 +43,6 @@ contract RewardsManagerReclaimTest is RewardsManagerSharedTest { return _expectedAllocationRewards(EPOCH_LENGTH, ALLOCATION, ALLOCATION); } - /// Rewards of the subgraph (holding all the signal) over `blocks`, as reclaimed at the subgraph level. - function _subgraphRewards(uint256 blocks) internal pure returns (uint256) { - return _expectedSubgraphRewards(blocks, SIGNAL, SIGNAL); - } - function _setReclaimAddress(bytes32 reason, address target) internal { vm.prank(governor); rm.setReclaimAddress(reason, target); diff --git a/packages/issuance/test/unit/rewards-manager/shared.t.sol b/packages/issuance/test/unit/rewards-manager/shared.t.sol index 9f7057a97..e34e077f1 100644 --- a/packages/issuance/test/unit/rewards-manager/shared.t.sol +++ b/packages/issuance/test/unit/rewards-manager/shared.t.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.27; import { Vm } from "forge-std/Vm.sol"; +import { IRewardsManager } from "@graphprotocol/interfaces/contracts/contracts/rewards/IRewardsManager.sol"; import { IProviderEligibility } from "@graphprotocol/interfaces/contracts/issuance/eligibility/IProviderEligibility.sol"; import { ProtocolFixture } from "../shared/ProtocolFixture.t.sol"; @@ -111,6 +112,11 @@ contract RewardsManagerSharedTest is ProtocolFixture { return (_expectedRewardsPerSignal(blocks, totalSignal) * subgraphSignal) / 1e18; } + /// Rewards of a subgraph holding all the signal (`SIGNAL`) over `blocks`. + function _subgraphRewards(uint256 blocks) internal pure returns (uint256) { + return _expectedSubgraphRewards(blocks, SIGNAL, SIGNAL); + } + function _expectedRewardsPerAllocatedToken( uint256 subgraphRewards, uint256 allocatedTokens @@ -128,6 +134,15 @@ contract RewardsManagerSharedTest is ProtocolFixture { return (_expectedRewardsPerAllocatedToken(subgraphRewards, allocatedTokens) * tokens) / 1e18; } + function _subgraph(bytes32 sd) internal view returns (IRewardsManager.Subgraph memory subgraph) { + ( + subgraph.accRewardsForSubgraph, + subgraph.accRewardsForSubgraphSnapshot, + subgraph.accRewardsPerSignalSnapshot, + subgraph.accRewardsPerAllocatedToken + ) = rm.subgraphs(sd); + } + function _assertNotEmitted(Vm.Log[] memory logs, bytes32 topic) internal view { for (uint256 i = 0; i < logs.length; ++i) { if (logs[i].emitter == address(rm)) assertTrue(logs[i].topics[0] != topic, "unexpected event"); diff --git a/packages/issuance/test/unit/rewards-manager/signalAllocationUpdate.t.sol b/packages/issuance/test/unit/rewards-manager/signalAllocationUpdate.t.sol new file mode 100644 index 000000000..b5d295086 --- /dev/null +++ b/packages/issuance/test/unit/rewards-manager/signalAllocationUpdate.t.sol @@ -0,0 +1,303 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.27; + +import { IRewardsManager } from "@graphprotocol/interfaces/contracts/contracts/rewards/IRewardsManager.sol"; +import { RewardsCondition } from "@graphprotocol/interfaces/contracts/contracts/rewards/RewardsCondition.sol"; + +import { RewardsManagerSharedTest } from "./shared.t.sol"; + +/// Signal/allocation update accounting: when `onSubgraphSignalUpdate` runs before `onSubgraphAllocationUpdate` +/// in the same block, the per-signal delta of the second call is zero, and rewards already tracked in +/// `accRewardsForSubgraph` must still reach allocations. Hardhat's batched (automine off) blocks are +/// consecutive calls with no `vm.roll` in between. +contract RewardsManagerSignalAllocationUpdateTest is RewardsManagerSharedTest { + event RewardsReclaimed( + bytes32 indexed reason, + uint256 amount, + address indexed indexer, + address indexed allocationID, + bytes32 subgraphDeploymentID + ); + + bytes32 internal constant SUBGRAPH_DEPLOYMENT_ID = keccak256("subgraphDeploymentID"); + + address internal allocationID = makeAddr("allocationID"); + address internal reclaimWallet = makeAddr("reclaimWallet"); + + // -- Helpers -- + + function _setupSubgraphWithAllocation() internal { + _signalThenAllocate(SUBGRAPH_DEPLOYMENT_ID, allocationID); + } + + function _deny() internal { + vm.startPrank(governor); + rm.setSubgraphAvailabilityOracle(governor); + rm.setDenied(SUBGRAPH_DEPLOYMENT_ID, true); + vm.stopPrank(); + } + + function _setReclaimAddress(bytes32 reason, address target) internal { + vm.prank(governor); + rm.setReclaimAddress(reason, target); + } + + function _perAllocatedToken(uint256 blocks) internal pure returns (uint256) { + return _expectedRewardsPerAllocatedToken(_subgraphRewards(blocks), ALLOCATION); + } + + // -- onSubgraphSignalUpdate followed by onSubgraphAllocationUpdate -- + + function test_OnSubgraphSignalUpdateFollowedByOnSubgraphAllocationUpdate_DistributesWhenSignalUpdatePrecedesSameBlock() + public + { + _setupSubgraphWithAllocation(); + vm.roll(block.number + 100); + + uint256 expectedRewardsForSubgraph = rm.getAccRewardsForSubgraph(SUBGRAPH_DEPLOYMENT_ID); + assertGt(expectedRewardsForSubgraph, 0); + assertEq(expectedRewardsForSubgraph, _subgraphRewards(100)); + uint256 accRewardsPerAllocatedTokenBefore = _subgraph(SUBGRAPH_DEPLOYMENT_ID).accRewardsPerAllocatedToken; + + // Same block: no vm.roll between the two hooks + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + + IRewardsManager.Subgraph memory sg = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + assertGt(sg.accRewardsForSubgraph, 0); + assertEq(sg.accRewardsForSubgraph, expectedRewardsForSubgraph); + assertGt(sg.accRewardsPerAllocatedToken, accRewardsPerAllocatedTokenBefore); + assertEq(sg.accRewardsPerAllocatedToken, accRewardsPerAllocatedTokenBefore + _perAllocatedToken(100)); + assertEq(sg.accRewardsForSubgraphSnapshot, sg.accRewardsForSubgraph); + } + + function test_OnSubgraphSignalUpdateFollowedByOnSubgraphAllocationUpdate_DoesNotBrickRewardsWhenPerSignalDeltaIsZero() + public + { + _setupSubgraphWithAllocation(); + vm.roll(block.number + 100); + + rm.getAccRewardsPerAllocatedToken(SUBGRAPH_DEPLOYMENT_ID); + + // Same block: no vm.roll between the two hooks + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + + IRewardsManager.Subgraph memory sg = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + assertGt(sg.accRewardsForSubgraph, 0); + assertEq(sg.accRewardsForSubgraph, _subgraphRewards(100)); + assertGt(sg.accRewardsPerAllocatedToken, 0); + assertEq(sg.accRewardsPerAllocatedToken, _perAllocatedToken(100)); + + (uint256 viewAccRewardsPerAllocatedToken, ) = rm.getAccRewardsPerAllocatedToken(SUBGRAPH_DEPLOYMENT_ID); + assertEq(viewAccRewardsPerAllocatedToken, sg.accRewardsPerAllocatedToken); + } + + function test_OnSubgraphSignalUpdateFollowedByOnSubgraphAllocationUpdate_MultipleSignalUpdatesDoNotLoseRewards() + public + { + _setupSubgraphWithAllocation(); + vm.roll(block.number + 50); + + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + IRewardsManager.Subgraph memory afterFirstSignal = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + assertEq(afterFirstSignal.accRewardsForSubgraph, _subgraphRewards(50)); + + vm.roll(block.number + 50); + + // Same block: no vm.roll between the two hooks + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + + IRewardsManager.Subgraph memory sg = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + assertGt(sg.accRewardsForSubgraph, afterFirstSignal.accRewardsForSubgraph); + assertEq(sg.accRewardsForSubgraph, 2 * _subgraphRewards(50)); + assertGt(sg.accRewardsPerAllocatedToken, 0); + assertEq(sg.accRewardsPerAllocatedToken, 2 * _perAllocatedToken(50)); + assertEq(sg.accRewardsForSubgraphSnapshot, sg.accRewardsForSubgraph); + } + + // -- snapshot consistency in reclaim paths -- + + function test_SnapshotConsistencyInReclaimPaths_UpdatesSnapshotWhenReclaimedDueToDenial() public { + _setupSubgraphWithAllocation(); + // Deny in the next block, as the separate Hardhat transactions: that block is distributed first + vm.roll(block.number + 1); + _deny(); + + vm.roll(block.number + 100); + + IRewardsManager.Subgraph memory before = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + IRewardsManager.Subgraph memory sg = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + + assertEq(sg.accRewardsPerAllocatedToken, before.accRewardsPerAllocatedToken); + assertGe(sg.accRewardsForSubgraphSnapshot, before.accRewardsForSubgraphSnapshot); + // The accumulator stays frozen at the pre-denial block, the snapshot stays synced with it, and the + // per-signal snapshot moves to now, so the reclaimed period cannot be processed again + assertEq(sg.accRewardsForSubgraph, _subgraphRewards(1)); + assertEq(sg.accRewardsForSubgraphSnapshot, sg.accRewardsForSubgraph); + assertEq(sg.accRewardsPerSignalSnapshot, rm.accRewardsPerSignal()); + assertGt(sg.accRewardsPerSignalSnapshot, before.accRewardsPerSignalSnapshot); + } + + function test_SnapshotConsistencyInReclaimPaths_NoDoubleReclaimAfterSnapshotUpdate() public { + _setupSubgraphWithAllocation(); + _setReclaimAddress(RewardsCondition.SUBGRAPH_DENIED, reclaimWallet); + _deny(); + + vm.roll(block.number + 100); + + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + IRewardsManager.Subgraph memory afterFirstReclaim = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + assertEq(token.balanceOf(reclaimWallet), _subgraphRewards(100)); + + // The next Hardhat transaction: one more block + vm.roll(block.number + 1); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + IRewardsManager.Subgraph memory afterSecondReclaim = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + + uint256 maxOneBlockReward = (ISSUANCE_PER_BLOCK * SIGNAL) / token.balanceOf(address(curation)); + uint256 snapshotDiff = afterSecondReclaim.accRewardsForSubgraphSnapshot - + afterFirstReclaim.accRewardsForSubgraphSnapshot; + assertLe(snapshotDiff, 2 * maxOneBlockReward); + assertEq(snapshotDiff, 0); + // Only the new block is reclaimed the second time + assertEq(token.balanceOf(reclaimWallet), _subgraphRewards(100) + _subgraphRewards(1)); + assertEq(afterSecondReclaim.accRewardsPerAllocatedToken, afterFirstReclaim.accRewardsPerAllocatedToken); + } + + // -- onSubgraphSignalUpdate on denied subgraph -- + + function test_OnSubgraphSignalUpdateOnDeniedSubgraph_ReclaimsRewards() public { + _setupSubgraphWithAllocation(); + _setReclaimAddress(RewardsCondition.SUBGRAPH_DENIED, reclaimWallet); + assertEq(rm.getReclaimAddress(RewardsCondition.SUBGRAPH_DENIED), reclaimWallet); + + // Deny in the next block, as the separate Hardhat transactions: that block is distributed first + vm.roll(block.number + 1); + _deny(); + IRewardsManager.Subgraph memory afterDenial = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + assertEq(afterDenial.accRewardsForSubgraph, _subgraphRewards(1)); + + vm.roll(block.number + 100); + + uint256 expectedRewards = _subgraphRewards(100); + assertGt(expectedRewards, 0); + vm.expectEmit(address(rm)); + emit RewardsReclaimed( + RewardsCondition.SUBGRAPH_DENIED, + expectedRewards, + address(0), + address(0), + SUBGRAPH_DEPLOYMENT_ID + ); + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + + assertEq(_subgraph(SUBGRAPH_DEPLOYMENT_ID).accRewardsForSubgraph, afterDenial.accRewardsForSubgraph); + assertEq(token.balanceOf(reclaimWallet), expectedRewards); + } + + function test_OnSubgraphSignalUpdateOnDeniedSubgraph_AccumulatesRewardsForClaimableSubgraph() public { + _setupSubgraphWithAllocation(); + IRewardsManager.Subgraph memory initialState = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + + vm.roll(block.number + 100); + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + + uint256 accRewardsForSubgraph = _subgraph(SUBGRAPH_DEPLOYMENT_ID).accRewardsForSubgraph; + assertGt(accRewardsForSubgraph, initialState.accRewardsForSubgraph); + assertEq(accRewardsForSubgraph, initialState.accRewardsForSubgraph + _subgraphRewards(100)); + } + + function test_OnSubgraphSignalUpdateOnDeniedSubgraph_ViewDoesNotJumpDuringDenial() public { + _setupSubgraphWithAllocation(); + vm.roll(block.number + 50); + + // setDenied distributes the pre-denial rewards through onSubgraphAllocationUpdate + _deny(); + + uint256 rewardsAtDenial = rm.getAccRewardsForSubgraph(SUBGRAPH_DEPLOYMENT_ID); + assertGt(rewardsAtDenial, 0); + assertEq(rewardsAtDenial, _subgraphRewards(50)); + + vm.roll(block.number + 100); + assertEq(rm.getAccRewardsForSubgraph(SUBGRAPH_DEPLOYMENT_ID), rewardsAtDenial); + + _setReclaimAddress(RewardsCondition.SUBGRAPH_DENIED, reclaimWallet); + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + assertEq(rm.getAccRewardsForSubgraph(SUBGRAPH_DEPLOYMENT_ID), rewardsAtDenial); + + vm.roll(block.number + 50); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + assertEq(rm.getAccRewardsForSubgraph(SUBGRAPH_DEPLOYMENT_ID), rewardsAtDenial); + } + + // -- onSubgraphSignalUpdate with no allocations -- + + function test_OnSubgraphSignalUpdateWithNoAllocations_ReclaimsAsNoAllocatedTokens() public { + _signal(SUBGRAPH_DEPLOYMENT_ID, SIGNAL); + _setReclaimAddress(RewardsCondition.NO_ALLOCATED_TOKENS, reclaimWallet); + IRewardsManager.Subgraph memory initialState = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + + vm.roll(block.number + 100); + + uint256 expectedRewards = _subgraphRewards(100); + assertGt(expectedRewards, 0); + vm.expectEmit(address(rm)); + emit RewardsReclaimed( + RewardsCondition.NO_ALLOCATED_TOKENS, + expectedRewards, + address(0), + address(0), + SUBGRAPH_DEPLOYMENT_ID + ); + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + + assertEq(_subgraph(SUBGRAPH_DEPLOYMENT_ID).accRewardsForSubgraph, initialState.accRewardsForSubgraph); + assertEq(token.balanceOf(reclaimWallet), expectedRewards); + } + + function test_OnSubgraphSignalUpdateWithNoAllocations_ViewShowsNoPhantomRewards() public { + _signal(SUBGRAPH_DEPLOYMENT_ID, SIGNAL); + uint256 viewAfterSignal = rm.getAccRewardsForSubgraph(SUBGRAPH_DEPLOYMENT_ID); + + vm.roll(block.number + 100); + + _setReclaimAddress(RewardsCondition.NO_ALLOCATED_TOKENS, reclaimWallet); + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + + assertEq(rm.getAccRewardsForSubgraph(SUBGRAPH_DEPLOYMENT_ID), viewAfterSignal); + assertEq(viewAfterSignal, 0); + // The 100 blocks went to the reclaim address instead + assertEq(token.balanceOf(reclaimWallet), _subgraphRewards(100)); + } + + // -- invariant: no rewards lost or double-counted -- + + function test_InvariantNoRewardsLostOrDoubleCounted_MixedUpdatesWithSameBlockScenarios() public { + _setupSubgraphWithAllocation(); + vm.roll(block.number + 25); + + // Same block: signal update, then allocation update + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + + vm.roll(block.number + 25); + + // Same block: two signal updates, then allocation update + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + + IRewardsManager.Subgraph memory sg = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + assertEq(sg.accRewardsForSubgraphSnapshot, sg.accRewardsForSubgraph); + assertGt(sg.accRewardsPerAllocatedToken, 0); + // Each 25-block period is counted exactly once, whatever the call ordering + assertEq(sg.accRewardsForSubgraph, 2 * _subgraphRewards(25)); + assertEq(sg.accRewardsPerAllocatedToken, 2 * _perAllocatedToken(25)); + uint256 allocationRewards = rm.getRewards(address(issuer), allocationID); + assertEq(allocationRewards, (sg.accRewardsPerAllocatedToken * ALLOCATION) / 1e18); + assertLe(allocationRewards, sg.accRewardsForSubgraph); + } +} diff --git a/packages/issuance/test/unit/rewards-manager/snapshotInversion.t.sol b/packages/issuance/test/unit/rewards-manager/snapshotInversion.t.sol new file mode 100644 index 000000000..18fb75ece --- /dev/null +++ b/packages/issuance/test/unit/rewards-manager/snapshotInversion.t.sol @@ -0,0 +1,302 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.27; + +import { IRewardsManager } from "@graphprotocol/interfaces/contracts/contracts/rewards/IRewardsManager.sol"; + +import { RewardsManagerSharedTest } from "./shared.t.sol"; + +/// Snapshot inversion on upgrade. With A = accRewardsForSubgraph, S = accRewardsForSubgraphSnapshot and +/// P = rewards since the signal snapshot, pre-upgrade storage can hold A < S. The update computes +/// (A + P) - S, adding P before subtracting S, so it never underflows and the gap S - A is not paid twice. +/// The inverted state is written with `vm.store`. +contract RewardsManagerSnapshotInversionTest is RewardsManagerSharedTest { + bytes32 internal constant SUBGRAPH_DEPLOYMENT_ID = keccak256("subgraphDeploymentID"); + + /// `subgraphs` in RewardsManagerV1Storage: Managed's controller (0), _addressCache (1) and __gap[10] (2-11), + /// then __DEPRECATED_issuanceRate (12), accRewardsPerSignal (13), accRewardsPerSignalLastBlockUpdated (14), + /// subgraphAvailabilityOracle (15) + uint256 internal constant SUBGRAPHS_MAPPING_SLOT = 16; + uint256 internal constant ACC_REWARDS_PER_SIGNAL_SLOT = 13; + uint256 internal constant ACC_REWARDS_PER_SIGNAL_LAST_BLOCK_UPDATED_SLOT = 14; + + /// Fields of `IRewardsManager.Subgraph`, as offsets from the struct's base slot + uint256 internal constant ACC_REWARDS_FOR_SUBGRAPH = 0; + uint256 internal constant ACC_REWARDS_FOR_SUBGRAPH_SNAPSHOT = 1; + uint256 internal constant ACC_REWARDS_PER_SIGNAL_SNAPSHOT = 2; + uint256 internal constant ACC_REWARDS_PER_ALLOCATED_TOKEN = 3; + + /// Blocks mined by the old setup between the allocation and its signal update + uint256 internal constant SETUP_BLOCKS = 50; + + address internal allocationID = makeAddr("allocationID"); + + // -- Helpers -- + + function _subgraphStorageSlot(bytes32 sd, uint256 fieldOffset) internal pure returns (bytes32) { + return bytes32(uint256(keccak256(abi.encode(sd, SUBGRAPHS_MAPPING_SLOT))) + fieldOffset); + } + + /// Signal, allocate, accrue `SETUP_BLOCKS`, then sync the subgraph with a signal update, so A > 0. + function _setupSubgraphWithAllocation() internal { + _signalThenAllocate(SUBGRAPH_DEPLOYMENT_ID, allocationID); + vm.roll(block.number + SETUP_BLOCKS); + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + } + + /// The state left by the old `onSubgraphAllocationUpdate`, which wrote S from a view (storage + pending) + /// while A stayed at its stored value: S = A + gap. + function _createInvertedState(uint256 gap) internal { + uint256 invertedSnapshot = _subgraph(SUBGRAPH_DEPLOYMENT_ID).accRewardsForSubgraph + gap; + vm.store( + address(rm), + _subgraphStorageSlot(SUBGRAPH_DEPLOYMENT_ID, ACC_REWARDS_FOR_SUBGRAPH_SNAPSHOT), + bytes32(invertedSnapshot) + ); + + IRewardsManager.Subgraph memory sg = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + assertEq(sg.accRewardsForSubgraphSnapshot, invertedSnapshot); + assertLt(sg.accRewardsForSubgraph, sg.accRewardsForSubgraphSnapshot); + } + + function _perAllocatedToken(uint256 rewards) internal pure returns (uint256) { + return _expectedRewardsPerAllocatedToken(rewards, ALLOCATION); + } + + // -- storage slot verification -- + + function test_SnapshotInversion_StorageSlotsMatchLayout() public { + _setupSubgraphWithAllocation(); + + IRewardsManager.Subgraph memory before = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + assertTrue(before.accRewardsForSubgraph != 0); + assertTrue(before.accRewardsForSubgraphSnapshot != 0); + assertTrue(before.accRewardsPerSignalSnapshot != 0); + assertTrue(before.accRewardsPerAllocatedToken != 0); + + // The inherited prefix: the plain slots before the mapping + assertEq(uint256(vm.load(address(rm), bytes32(ACC_REWARDS_PER_SIGNAL_SLOT))), rm.accRewardsPerSignal()); + assertEq( + uint256(vm.load(address(rm), bytes32(ACC_REWARDS_PER_SIGNAL_LAST_BLOCK_UPDATED_SLOT))), + rm.accRewardsPerSignalLastBlockUpdated() + ); + + // Each struct field reads back through the getter + bytes32 sd = SUBGRAPH_DEPLOYMENT_ID; + assertEq( + uint256(vm.load(address(rm), _subgraphStorageSlot(sd, ACC_REWARDS_FOR_SUBGRAPH))), + before.accRewardsForSubgraph + ); + assertEq( + uint256(vm.load(address(rm), _subgraphStorageSlot(sd, ACC_REWARDS_FOR_SUBGRAPH_SNAPSHOT))), + before.accRewardsForSubgraphSnapshot + ); + assertEq( + uint256(vm.load(address(rm), _subgraphStorageSlot(sd, ACC_REWARDS_PER_SIGNAL_SNAPSHOT))), + before.accRewardsPerSignalSnapshot + ); + assertEq( + uint256(vm.load(address(rm), _subgraphStorageSlot(sd, ACC_REWARDS_PER_ALLOCATED_TOKEN))), + before.accRewardsPerAllocatedToken + ); + + // A write to the snapshot field changes only that field + uint256 testValue = 12345678901234567890; + vm.store(address(rm), _subgraphStorageSlot(sd, ACC_REWARDS_FOR_SUBGRAPH_SNAPSHOT), bytes32(testValue)); + + IRewardsManager.Subgraph memory sg = _subgraph(sd); + assertEq(sg.accRewardsForSubgraphSnapshot, testValue); + assertEq(sg.accRewardsForSubgraph, before.accRewardsForSubgraph); + assertEq(sg.accRewardsPerSignalSnapshot, before.accRewardsPerSignalSnapshot); + assertEq(sg.accRewardsPerAllocatedToken, before.accRewardsPerAllocatedToken); + } + + // -- inverted state: accumulated < snapshot -- + + function test_InvertedStateAccumulatedBelowSnapshot_SignalUpdateDoesNotRevert() public { + _setupSubgraphWithAllocation(); + _createInvertedState(7000 ether); + + // P (50 blocks at 200 GRT) exceeds the gap + vm.roll(block.number + 50); + + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + } + + function test_InvertedStateAccumulatedBelowSnapshot_AllocationUpdateDoesNotRevert() public { + _setupSubgraphWithAllocation(); + _createInvertedState(7000 ether); + + vm.roll(block.number + 50); + + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + } + + function test_InvertedStateAccumulatedBelowSnapshot_SyncsSnapshotsAfterFirstCall() public { + _setupSubgraphWithAllocation(); + _createInvertedState(7000 ether); + + vm.roll(block.number + 50); + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + + IRewardsManager.Subgraph memory sg = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + assertEq(sg.accRewardsForSubgraphSnapshot, sg.accRewardsForSubgraph); + + vm.roll(block.number + 10); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + + sg = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + assertEq(sg.accRewardsForSubgraphSnapshot, sg.accRewardsForSubgraph); + } + + // -- accounting correctness with inverted state -- + + function test_AccountingCorrectnessWithInvertedState_ComputesUndistributedAsAPlusPMinusS() public { + _setupSubgraphWithAllocation(); + IRewardsManager.Subgraph memory before = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + + uint256 gap = 500 ether; + _createInvertedState(gap); + + // P = 10 blocks, about 2000 GRT, more than the gap + vm.roll(block.number + 10); + uint256 pending = _subgraphRewards(10); + assertGt(pending, gap); + + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + IRewardsManager.Subgraph memory sg = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + + assertLt(before.accRewardsPerAllocatedToken, sg.accRewardsPerAllocatedToken); + // (A + P) - S = (A + P) - (A + gap) = P - gap + assertEq( + sg.accRewardsPerAllocatedToken, + before.accRewardsPerAllocatedToken + _perAllocatedToken(pending - gap) + ); + assertEq(sg.accRewardsForSubgraph, before.accRewardsForSubgraph + pending); + assertEq(sg.accRewardsForSubgraphSnapshot, sg.accRewardsForSubgraph); + } + + function test_AccountingCorrectnessWithInvertedState_DoesNotDoubleCountGap() public { + _setupSubgraphWithAllocation(); + IRewardsManager.Subgraph memory stateBefore = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + + uint256 gap = 500 ether; + _createInvertedState(gap); + + vm.roll(block.number + 20); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + uint256 perAllocAfterInverted = _subgraph(SUBGRAPH_DEPLOYMENT_ID).accRewardsPerAllocatedToken; + + vm.roll(block.number + 20); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + uint256 perAllocAfterNormal = _subgraph(SUBGRAPH_DEPLOYMENT_ID).accRewardsPerAllocatedToken; + + uint256 firstDelta = perAllocAfterInverted - stateBefore.accRewardsPerAllocatedToken; + uint256 secondDelta = perAllocAfterNormal - perAllocAfterInverted; + + assertLt(firstDelta, secondDelta); + assertEq(firstDelta, _perAllocatedToken(_subgraphRewards(20) - gap)); + assertEq(secondDelta, _perAllocatedToken(_subgraphRewards(20))); + } + + function test_AccountingCorrectnessWithInvertedState_DistributesExactlyPendingMinusGap() public { + _setupSubgraphWithAllocation(); + + // Sync in the next block, as the next Hardhat transaction + vm.roll(block.number + 1); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + uint256 perAllocBaseline = _subgraph(SUBGRAPH_DEPLOYMENT_ID).accRewardsPerAllocatedToken; + + uint256 gap = 500 ether; + _createInvertedState(gap); + + vm.roll(block.number + 20); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + uint256 perAllocAfterInverted = _subgraph(SUBGRAPH_DEPLOYMENT_ID).accRewardsPerAllocatedToken; + uint256 invertedDelta = perAllocAfterInverted - perAllocBaseline; + + vm.roll(block.number + 20); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + uint256 normalDelta = _subgraph(SUBGRAPH_DEPLOYMENT_ID).accRewardsPerAllocatedToken - perAllocAfterInverted; + + assertLt(invertedDelta, normalDelta); + assertTrue(invertedDelta != 0); + assertEq(invertedDelta, _perAllocatedToken(_subgraphRewards(20) - gap)); + assertEq(normalDelta, _perAllocatedToken(_subgraphRewards(20))); + } + + // -- normal operation (no inversion) -- + + function test_NormalOperationNoInversion_SyncedSnapshotSteadyState() public { + _setupSubgraphWithAllocation(); + + // Sync in the next block, as the next Hardhat transaction + vm.roll(block.number + 1); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + IRewardsManager.Subgraph memory synced = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + assertEq(synced.accRewardsForSubgraphSnapshot, synced.accRewardsForSubgraph); + + vm.roll(block.number + 20); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + IRewardsManager.Subgraph memory sg = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + + assertLt(synced.accRewardsPerAllocatedToken, sg.accRewardsPerAllocatedToken); + assertEq( + sg.accRewardsPerAllocatedToken, + synced.accRewardsPerAllocatedToken + _perAllocatedToken(_subgraphRewards(20)) + ); + assertEq(sg.accRewardsForSubgraphSnapshot, sg.accRewardsForSubgraph); + } + + function test_NormalOperationNoInversion_ZeroRewardsInSameBlock() public { + _setupSubgraphWithAllocation(); + + // Sync in the next block, as the next Hardhat transaction + vm.roll(block.number + 1); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + IRewardsManager.Subgraph memory synced = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + + // Same block: no vm.roll, so no new rewards + uint256 accRewardsPerAllocatedToken = rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + + IRewardsManager.Subgraph memory sg = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + assertEq(sg.accRewardsForSubgraphSnapshot, sg.accRewardsForSubgraph); + assertEq(accRewardsPerAllocatedToken, synced.accRewardsPerAllocatedToken); + assertEq(sg.accRewardsPerAllocatedToken, synced.accRewardsPerAllocatedToken); + assertEq(sg.accRewardsForSubgraph, synced.accRewardsForSubgraph); + } + + // -- realistic pre-upgrade scenario -- + + function test_RealisticPreUpgradeScenario_HandlesArbitrumSepoliaStatePattern() public { + _setupSubgraphWithAllocation(); + IRewardsManager.Subgraph memory state = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + + // S = A + 7235 GRT, the gap observed on Arbitrum Sepolia + uint256 observedGap = 7235 ether; + vm.store( + address(rm), + _subgraphStorageSlot(SUBGRAPH_DEPLOYMENT_ID, ACC_REWARDS_FOR_SUBGRAPH_SNAPSHOT), + bytes32(state.accRewardsForSubgraph + observedGap) + ); + + IRewardsManager.Subgraph memory inverted = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + assertLt(inverted.accRewardsForSubgraph, inverted.accRewardsForSubgraphSnapshot); + + vm.roll(block.number + 50); + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + + IRewardsManager.Subgraph memory healed = _subgraph(SUBGRAPH_DEPLOYMENT_ID); + assertEq(healed.accRewardsForSubgraphSnapshot, healed.accRewardsForSubgraph); + assertEq(healed.accRewardsForSubgraph, state.accRewardsForSubgraph + _subgraphRewards(50)); + assertEq( + healed.accRewardsPerAllocatedToken, + state.accRewardsPerAllocatedToken + _perAllocatedToken(_subgraphRewards(50) - observedGap) + ); + + vm.roll(block.number + 10); + rm.onSubgraphAllocationUpdate(SUBGRAPH_DEPLOYMENT_ID); + + vm.roll(block.number + 10); + rm.onSubgraphSignalUpdate(SUBGRAPH_DEPLOYMENT_ID); + } +} diff --git a/scripts/contracts-migration/test-mapping/t5e.md b/scripts/contracts-migration/test-mapping/t5e.md new file mode 100644 index 000000000..b0689b542 --- /dev/null +++ b/scripts/contracts-migration/test-mapping/t5e.md @@ -0,0 +1,31 @@ +# T5e test mapping: RewardsManager signal/allocation update accounting and snapshot inversion + +New tests are in `packages/issuance`, on T5a's `RewardsManagerSharedTest` (the RewardsManager Hardhat artifact behind a GraphProxy). `setupSubgraphWithAllocation` becomes the shared `_signalThenAllocate` (`SIGNAL` / `ALLOCATION` instead of the old 1000 / 10,000 GRT; signal, `vm.roll(+1)`, allocate), and `issuer` plays the legacy Staking contract. The hooks are called directly (the old tests called them as governor; they have no access check). `mine(n)` becomes `vm.roll(n)`. Hardhat batched blocks (`evm_setAutomine(false)` … `evm_mine`) become consecutive calls with no `vm.roll` in between. Where a following Hardhat transaction mattered (a sync, or `setDenied` after the allocation), the port rolls one block. Expected values come from the rolls through the shared `_subgraphRewards` and `_expectedRewardsPerAllocatedToken`, and are asserted exactly next to the old `gt` / `lt` checks. `hardhat_setStorageAt` becomes `vm.store` at `keccak256(abi.encode(sd, uint256(16))) + field`, and `test_SnapshotInversion_StorageSlotsMatchLayout` checks that layout. No ported case uses a zero-token allocation, so the `RMIssuerMock` hook caveat does not apply. + +| Old file | Old test (describe › it) | New test (file::function) | Note | +| ----------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts | Rewards: Signal and Allocation Update Accounting › onSubgraphSignalUpdate followed by onSubgraphAllocationUpdate › should properly distribute rewards when signal update precedes allocation update (same block) | test/unit/rewards-manager/signalAllocationUpdate.t.sol::test_OnSubgraphSignalUpdateFollowedByOnSubgraphAllocationUpdate_DistributesWhenSignalUpdatePrecedesSameBlock | Batched block → the two hooks with no `vm.roll` between. `gt` checks kept; the view before, the stored `accRewardsForSubgraph` and `accRewardsPerAllocatedToken` are also asserted exactly from `vm.roll(100)` | +| packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts | Rewards: Signal and Allocation Update Accounting › onSubgraphSignalUpdate followed by onSubgraphAllocationUpdate › should not brick rewards when signal update zeros the per-signal delta (same block) | test/unit/rewards-manager/signalAllocationUpdate.t.sol::test_OnSubgraphSignalUpdateFollowedByOnSubgraphAllocationUpdate_DoesNotBrickRewardsWhenPerSignalDeltaIsZero | Batched block → no `vm.roll` between the hooks. Exact accumulator and per-token values added next to the `gt` checks; view == stored kept | +| packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts | Rewards: Signal and Allocation Update Accounting › onSubgraphSignalUpdate followed by onSubgraphAllocationUpdate › should handle multiple signal updates without losing rewards (same block allocation) | test/unit/rewards-manager/signalAllocationUpdate.t.sol::test_OnSubgraphSignalUpdateFollowedByOnSubgraphAllocationUpdate_MultipleSignalUpdatesDoNotLoseRewards | `mine(50)` twice → `vm.roll(50)` twice; second signal + allocation update in one block. Exact `2 × R(50)` accumulator and per-token value added | +| packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts | Rewards: Signal and Allocation Update Accounting › snapshot consistency in reclaim paths › should update accRewardsForSubgraphSnapshot when rewards are reclaimed due to denial | test/unit/rewards-manager/signalAllocationUpdate.t.sol::test_SnapshotConsistencyInReclaimPaths_UpdatesSnapshotWhenReclaimedDueToDenial | Old `gte` on the snapshot is vacuous (the denied path keeps S = A, so it held with equality). Kept, plus the real values: A frozen at the one pre-denial block (`vm.roll(+1)` before `setDenied`, as the separate Hardhat transactions), S == A, per-signal snapshot moved to `accRewardsPerSignal` | +| packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts | Rewards: Signal and Allocation Update Accounting › snapshot consistency in reclaim paths › should not double-reclaim rewards after snapshot update | test/unit/rewards-manager/signalAllocationUpdate.t.sol::test_SnapshotConsistencyInReclaimPaths_NoDoubleReclaimAfterSnapshotUpdate | Old `lte(2 × one block)` bound on the snapshot diff kept; the real diff is 0 and is asserted. The old test had no reclaim address, so nothing showed what was reclaimed: the port sets a SUBGRAPH_DENIED address and asserts R(100), then only R(1) for the next block (`vm.roll(+1)`, the next Hardhat transaction) | +| packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts | Rewards: Signal and Allocation Update Accounting › onSubgraphSignalUpdate on denied subgraph › should reclaim rewards when onSubgraphSignalUpdate is called on denied subgraph | test/unit/rewards-manager/signalAllocationUpdate.t.sol::test_OnSubgraphSignalUpdateOnDeniedSubgraph_ReclaimsRewards | Reclaim address is a `makeAddr` wallet (old: governor). `vm.roll(+1)` before `setDenied`, so A > 0. Event `gt(0)` → `vm.expectEmit` with the exact R(100) and all args; wallet balance asserted | +| packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts | Rewards: Signal and Allocation Update Accounting › onSubgraphSignalUpdate on denied subgraph › should accumulate rewards for claimable subgraphs in onSubgraphSignalUpdate | test/unit/rewards-manager/signalAllocationUpdate.t.sol::test_OnSubgraphSignalUpdateOnDeniedSubgraph_AccumulatesRewardsForClaimableSubgraph | `gt` kept; exact increase R(100) added | +| packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts | Rewards: Signal and Allocation Update Accounting › onSubgraphSignalUpdate on denied subgraph › view function getAccRewardsForSubgraph should not jump during denial | test/unit/rewards-manager/signalAllocationUpdate.t.sol::test_OnSubgraphSignalUpdateOnDeniedSubgraph_ViewDoesNotJumpDuringDenial | `mine(50/100/50)` → `vm.roll`. View at denial asserted exactly as R(50) | +| packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts | Rewards: Signal and Allocation Update Accounting › onSubgraphSignalUpdate with no allocations › should reclaim as NO_ALLOCATED_TOKENS when signal exists but no allocations | test/unit/rewards-manager/signalAllocationUpdate.t.sol::test_OnSubgraphSignalUpdateWithNoAllocations_ReclaimsAsNoAllocatedTokens | Reason and `gt(0)` → `vm.expectEmit` with the exact R(100); wallet balance asserted. No allocation, so the RMIssuerMock zero-token caveat does not apply | +| packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts | Rewards: Signal and Allocation Update Accounting › onSubgraphSignalUpdate with no allocations › view function should not show phantom rewards when no allocations | test/unit/rewards-manager/signalAllocationUpdate.t.sol::test_OnSubgraphSignalUpdateWithNoAllocations_ViewShowsNoPhantomRewards | Also asserts the view is 0 and the 100 blocks reached the reclaim wallet | +| packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts | Rewards: Signal and Allocation Update Accounting › invariant: no rewards lost or double-counted › should maintain accounting invariant across mixed updates (with same-block scenarios) | test/unit/rewards-manager/signalAllocationUpdate.t.sol::test_InvariantNoRewardsLostOrDoubleCounted_MixedUpdatesWithSameBlockScenarios | Two batched blocks → consecutive hooks with no `vm.roll`. Adds the exact `2 × R(25)` accumulator and per-token value, and `getRewards` ≤ A | +| packages/contracts-test/tests/unit/rewards/rewards-snapshot-inversion.test.ts | Rewards: Snapshot Inversion › storage slot verification › should correctly compute and write to subgraph storage slots | test/unit/rewards-manager/snapshotInversion.t.sol::test_SnapshotInversion_StorageSlotsMatchLayout | Name per plan. `hardhat_setStorageAt` → `vm.store`. Before the write, `vm.load` of slots 13, 14 and all four struct fields at `keccak256(abi.encode(sd, 16)) + 0..3` must equal the getters (all non-zero) | +| packages/contracts-test/tests/unit/rewards/rewards-snapshot-inversion.test.ts | Rewards: Snapshot Inversion › inverted state: accumulated < snapshot › should not revert on onSubgraphSignalUpdate with inverted state | test/unit/rewards-manager/snapshotInversion.t.sol::test_InvertedStateAccumulatedBelowSnapshot_SignalUpdateDoesNotRevert | `not.reverted` → plain call. `_createInvertedState` re-checks the written S and A < S | +| packages/contracts-test/tests/unit/rewards/rewards-snapshot-inversion.test.ts | Rewards: Snapshot Inversion › inverted state: accumulated < snapshot › should not revert on onSubgraphAllocationUpdate with inverted state | test/unit/rewards-manager/snapshotInversion.t.sol::test_InvertedStateAccumulatedBelowSnapshot_AllocationUpdateDoesNotRevert | `not.reverted` → plain call | +| packages/contracts-test/tests/unit/rewards/rewards-snapshot-inversion.test.ts | Rewards: Snapshot Inversion › inverted state: accumulated < snapshot › should sync snapshots after first successful call | test/unit/rewards-manager/snapshotInversion.t.sol::test_InvertedStateAccumulatedBelowSnapshot_SyncsSnapshotsAfterFirstCall | | +| packages/contracts-test/tests/unit/rewards/rewards-snapshot-inversion.test.ts | Rewards: Snapshot Inversion › accounting correctness with inverted state › should correctly compute undistributed rewards: (A+P).sub(S) | test/unit/rewards-manager/snapshotInversion.t.sol::test_AccountingCorrectnessWithInvertedState_ComputesUndistributedAsAPlusPMinusS | The old test only checked `lt` (its "≈ 1500 GRT" was a comment). The port asserts the per-token increase is exactly (R(10) − gap) / ALLOCATION and A grows by exactly R(10) | +| packages/contracts-test/tests/unit/rewards/rewards-snapshot-inversion.test.ts | Rewards: Snapshot Inversion › accounting correctness with inverted state › should not double-count: distributed rewards account for the gap | test/unit/rewards-manager/snapshotInversion.t.sol::test_AccountingCorrectnessWithInvertedState_DoesNotDoubleCountGap | `lt` kept; both deltas asserted exactly: R(20) − gap, then R(20) | +| packages/contracts-test/tests/unit/rewards/rewards-snapshot-inversion.test.ts | Rewards: Snapshot Inversion › accounting correctness with inverted state › should distribute exactly P - gap rewards (gap deducted from pending) | test/unit/rewards-manager/snapshotInversion.t.sol::test_AccountingCorrectnessWithInvertedState_DistributesExactlyPendingMinusGap | The baseline sync rolls one block, as the next Hardhat transaction (with no roll it is a no-op). Despite the title, the old test only checked `lt` and `!= 0`; the port asserts exactly P − gap | +| packages/contracts-test/tests/unit/rewards/rewards-snapshot-inversion.test.ts | Rewards: Snapshot Inversion › normal operation (no inversion) › should produce identical results when A == S (synced snapshot steady state) | test/unit/rewards-manager/snapshotInversion.t.sol::test_NormalOperationNoInversion_SyncedSnapshotSteadyState | Baseline sync rolls one block, as the next Hardhat transaction. Exact R(20) per-token increase added | +| packages/contracts-test/tests/unit/rewards/rewards-snapshot-inversion.test.ts | Rewards: Snapshot Inversion › normal operation (no inversion) › should handle zero rewards gracefully (same block, no new rewards) | test/unit/rewards-manager/snapshotInversion.t.sol::test_NormalOperationNoInversion_ZeroRewardsInSameBlock | In Hardhat the "batched" single transaction still landed in a new block (the old comment notes it). The port makes it really same-block (no `vm.roll`) and asserts what the old test only described: per-token value, A and the return value unchanged | +| packages/contracts-test/tests/unit/rewards/rewards-snapshot-inversion.test.ts | Rewards: Snapshot Inversion › realistic pre-upgrade scenario › should handle the exact Arbitrum Sepolia state pattern | test/unit/rewards-manager/snapshotInversion.t.sol::test_RealisticPreUpgradeScenario_HandlesArbitrumSepoliaStatePattern | `vm.store` of S = A + 7235 GRT. Healed A and per-token value asserted exactly; later `not.reverted` → plain calls | + +## Coverage + +`contracts/rewards/RewardsManager.sol`: every external/public name in `functionsHit` is called by an issuance test. No `MISSING` names. This PR adds direct calls to `onSubgraphSignalUpdate`, `onSubgraphAllocationUpdate`, `getAccRewardsForSubgraph`, `getAccRewardsPerAllocatedToken` and `getRewards`, and reaches `_updateSubgraphRewards` with an inverted snapshot (A < S).