diff --git a/packages/contracts-test/tests/unit/epochs.test.ts b/packages/contracts-test/tests/unit/epochs.test.ts deleted file mode 100644 index bbd433b7d..000000000 --- a/packages/contracts-test/tests/unit/epochs.test.ts +++ /dev/null @@ -1,171 +0,0 @@ -import { EpochManager } from '@graphprotocol/contracts' -import { deploy, DeployType, helpers, toBN } from '@graphprotocol/sdk' -import type { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' -import { expect } from 'chai' -import { BigNumber } from 'ethers' -import hre from 'hardhat' - -describe('EpochManager', () => { - const graph = hre.graph() - const defaults = graph.graphConfig.defaults - let me: SignerWithAddress - let governor: SignerWithAddress - - let epochManager: EpochManager - - const epochLength: BigNumber = toBN('3') - - before(async function () { - ;[me, governor] = await graph.getTestAccounts() - ;({ governor } = await graph.getNamedAccounts()) - }) - - beforeEach(async function () { - const { contract: controller } = await deploy( - DeployType.DeployAndSave, - governor, - { - name: 'Controller', - }, - graph.addressBook, - ) - await deploy( - DeployType.DeployAndSave, - governor, - { - name: 'GraphProxyAdmin', - }, - graph.addressBook, - ) - const epochManagerResult = await deploy( - DeployType.DeployWithProxy, - governor, - { - name: 'EpochManager', - args: [controller.address, defaults.epochs.lengthInBlocks], - }, - graph.addressBook, - { - name: 'GraphProxy', - }, - ) - epochManager = epochManagerResult.contract as EpochManager - }) - - describe('configuration', () => { - it('should set `epochLength', async function () { - // Set right in the constructor - expect(await epochManager.epochLength()).eq(defaults.epochs.lengthInBlocks) - - // Update and check new value - const newEpochLength = toBN('4') - const currentEpoch = await epochManager.currentEpoch() - const tx = epochManager.connect(governor).setEpochLength(newEpochLength) - await expect(tx).emit(epochManager, 'EpochLengthUpdate').withArgs(currentEpoch, newEpochLength) - expect(await epochManager.epochLength()).eq(newEpochLength) - }) - - it('reject set `epochLength` if zero', async function () { - // Update and check new value - const newEpochLength = toBN('0') - const tx = epochManager.connect(governor).setEpochLength(newEpochLength) - await expect(tx).revertedWith('Epoch length cannot be 0') - }) - }) - - describe('epoch lifecycle', function () { - // Use epochs every three blocks - // Blocks -> (1,2,3)(4,5,6)(7,8,9) - // Epochs -> 1 2 3 - beforeEach(async function () { - await epochManager.connect(governor).setEpochLength(epochLength) - }) - - describe('calculations', () => { - it('first epoch should be 1', async function () { - const currentEpoch = await epochManager.currentEpoch() - expect(currentEpoch).eq(1) - }) - - it('should return correct block number', async function () { - const currentBlock = await helpers.latestBlock() - expect(await epochManager.blockNum()).eq(currentBlock) - }) - - it('should return same starting block if we stay on the same epoch', async function () { - // Move right to the start of a new epoch - const blocksSinceEpochStart = await epochManager.currentEpochBlockSinceStart() - const blocksToNextEpoch = epochLength.sub(blocksSinceEpochStart) - await helpers.mineUpTo((await epochManager.blockNum()).add(blocksToNextEpoch)) - - const beforeCurrentEpochBlock = await epochManager.currentEpochBlock() - - // Advance block - will not jump to next epoch - await helpers.mine() - - const afterCurrentEpochBlock = await epochManager.currentEpochBlock() - expect(afterCurrentEpochBlock).equal(beforeCurrentEpochBlock) - }) - - it('should return next starting block if we move to the next epoch', async function () { - const beforeCurrentEpochBlock = await epochManager.currentEpochBlock() - - // Advance blocks to move to the next epoch - await helpers.mineUpTo(beforeCurrentEpochBlock.add(epochLength)) - - const afterCurrentEpochBlock = await epochManager.currentEpochBlock() - expect(afterCurrentEpochBlock).not.eq(beforeCurrentEpochBlock) - }) - - it('should return next epoch if advance > epochLength', async function () { - const nextEpoch = (await epochManager.currentEpoch()).add(toBN('1')) - - // Advance blocks and move to the next epoch - const currentEpochBlock = await epochManager.currentEpochBlock() - await helpers.mineUpTo(currentEpochBlock.add(epochLength)) - - const afterCurrentEpoch = await epochManager.currentEpoch() - expect(afterCurrentEpoch).eq(nextEpoch) - }) - }) - - describe('progression', () => { - beforeEach(async function () { - const currentEpochBlock = await epochManager.currentEpochBlock() - await helpers.mineUpTo(currentEpochBlock.add(epochLength)) - }) - - context('> epoch not run', function () { - it('should return that current epoch is not run', async function () { - expect(await epochManager.isCurrentEpochRun()).eq(false) - }) - - it('should run new epoch', async function () { - // Run epoch - const currentEpoch = await epochManager.currentEpoch() - const tx = epochManager.connect(me).runEpoch() - await expect(tx).emit(epochManager, 'EpochRun').withArgs(currentEpoch, me.address) - - // State - const lastRunEpoch = await epochManager.lastRunEpoch() - expect(lastRunEpoch).eq(currentEpoch) - }) - }) - - context('> epoch run', function () { - beforeEach(async function () { - await epochManager.runEpoch() - }) - - it('should return current epoch is already run', async function () { - expect(await epochManager.isCurrentEpochRun()).eq(true) - }) - - it('reject run new epoch', async function () { - const tx = epochManager.runEpoch() - await expect(tx).revertedWith('Current epoch already run') - }) - }) - }) - }) -}) diff --git a/packages/horizon/foundry.toml b/packages/horizon/foundry.toml index 1301b718b..63b1fc545 100644 --- a/packages/horizon/foundry.toml +++ b/packages/horizon/foundry.toml @@ -5,7 +5,7 @@ libs = ["node_modules"] auto_detect_remappings = false test = 'test' cache_path = 'cache_forge' -fs_permissions = [{ access = "read", path = "./" }] +fs_permissions = [{ access = "read", path = "./" }, { access = "read", path = "node_modules/@graphprotocol/contracts/artifacts" }] solc_version = '0.8.35' evm_version = 'cancun' # Suppress solc warnings emitted from third-party code we don't control diff --git a/packages/horizon/test/unit/epochs/EpochManager.t.sol b/packages/horizon/test/unit/epochs/EpochManager.t.sol new file mode 100644 index 000000000..6b21b8ef3 --- /dev/null +++ b/packages/horizon/test/unit/epochs/EpochManager.t.sol @@ -0,0 +1,188 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.27; + +import { IEpochManagerLike } from "./IEpochManagerLike.sol"; +import { ProtocolFixture } from "../shared/ProtocolFixture.t.sol"; + +contract EpochManagerTest is ProtocolFixture { + event EpochRun(uint256 indexed epoch, address caller); + event EpochLengthUpdate(uint256 indexed epoch, uint256 epochLength); + + uint256 internal constant DEFAULT_EPOCH_LENGTH = 60; + uint256 internal constant EPOCH_LENGTH = 3; + + IEpochManagerLike internal epochManager; + address internal me = makeAddr("me"); + + function setUp() public { + vm.warp(1_700_000_000); + _deployProtocolBase(); + address impl = _deployArtifact(string.concat(PKG_ARTIFACTS, "epochs/EpochManager.sol/EpochManager.json"), ""); + epochManager = IEpochManagerLike( + _deployBehindProxy( + impl, + abi.encodeCall(IEpochManagerLike.initialize, (address(controller), DEFAULT_EPOCH_LENGTH)) + ) + ); + } + + modifier withLifecycleLength() { + vm.prank(governor); + epochManager.setEpochLength(EPOCH_LENGTH); + _; + } + + function _rollToNextEpoch() internal { + vm.roll(epochManager.currentEpochBlock() + EPOCH_LENGTH); + } + + // -- configuration -- + + function test_Configuration_SetEpochLength() public { + assertEq(epochManager.epochLength(), DEFAULT_EPOCH_LENGTH); + + uint256 newEpochLength = 4; + uint256 currentEpoch = epochManager.currentEpoch(); + vm.expectEmit(address(epochManager)); + emit EpochLengthUpdate(currentEpoch, newEpochLength); + vm.prank(governor); + epochManager.setEpochLength(newEpochLength); + assertEq(epochManager.epochLength(), newEpochLength); + } + + function test_Configuration_RevertWhen_EpochLengthZero() public { + vm.expectRevert(bytes("Epoch length cannot be 0")); + vm.prank(governor); + epochManager.setEpochLength(0); + } + + function test_Configuration_RevertWhen_EpochLengthSame() public { + vm.expectRevert(bytes("Epoch length must be different to current")); + vm.prank(governor); + epochManager.setEpochLength(DEFAULT_EPOCH_LENGTH); + } + + // -- epoch lifecycle: calculations -- + + function test_Calculations_FirstEpochIsOne() public withLifecycleLength { + assertEq(epochManager.currentEpoch(), 1); + } + + function test_Calculations_BlockNum() public withLifecycleLength { + assertEq(epochManager.blockNum(), block.number); + vm.roll(block.number + 7); + assertEq(epochManager.blockNum(), block.number); + } + + function test_Calculations_SameStartingBlockOnSameEpoch() public withLifecycleLength { + // Move right to the start of a new epoch + uint256 blocksToNextEpoch = EPOCH_LENGTH - epochManager.currentEpochBlockSinceStart(); + vm.roll(epochManager.blockNum() + blocksToNextEpoch); + + uint256 beforeCurrentEpochBlock = epochManager.currentEpochBlock(); + + // Advance block - will not jump to next epoch + vm.roll(block.number + 1); + + assertEq(epochManager.currentEpochBlock(), beforeCurrentEpochBlock); + } + + function test_Calculations_NextStartingBlockOnNextEpoch() public withLifecycleLength { + uint256 beforeCurrentEpochBlock = epochManager.currentEpochBlock(); + + vm.roll(beforeCurrentEpochBlock + EPOCH_LENGTH); + + assertEq(epochManager.currentEpochBlock(), beforeCurrentEpochBlock + EPOCH_LENGTH); + } + + function test_Calculations_NextEpochWhenAdvancePastLength() public withLifecycleLength { + uint256 nextEpoch = epochManager.currentEpoch() + 1; + + _rollToNextEpoch(); + + assertEq(epochManager.currentEpoch(), nextEpoch); + } + + function test_EpochManager_SameBlockDoesNotAdvance() public withLifecycleLength { + uint256 epochBefore = epochManager.currentEpoch(); + uint256 epochBlockBefore = epochManager.currentEpochBlock(); + + assertEq(epochManager.currentEpoch(), epochBefore); + assertEq(epochManager.currentEpochBlock(), epochBlockBefore); + + vm.roll(epochBlockBefore + EPOCH_LENGTH - 1); + assertEq(epochManager.currentEpoch(), epochBefore); + assertEq(epochManager.currentEpochBlock(), epochBlockBefore); + + vm.roll(epochBlockBefore + EPOCH_LENGTH); + assertEq(epochManager.currentEpoch(), epochBefore + 1); + assertEq(epochManager.currentEpochBlock(), epochBlockBefore + EPOCH_LENGTH); + } + + function test_Calculations_EpochsSince() public withLifecycleLength { + uint256 startEpoch = epochManager.currentEpoch(); + uint256 startBlock = epochManager.currentEpochBlock(); + + vm.roll(startBlock + 2 * EPOCH_LENGTH + 1); + + assertEq(epochManager.currentEpoch(), startEpoch + 2); + assertEq(epochManager.epochsSince(startEpoch), 2); + assertEq(epochManager.epochsSince(startEpoch + 1), 1); + assertEq(epochManager.epochsSince(startEpoch + 2), 0); + assertEq(epochManager.epochsSince(startEpoch + 3), 0); + assertEq(epochManager.epochsSinceUpdate(), 2); + assertEq(epochManager.currentEpochBlockSinceStart(), 1); + } + + function test_Calculations_RevertWhen_BlockHashNotPast() public withLifecycleLength { + vm.roll(1000); + uint256 current = epochManager.blockNum(); + + vm.expectRevert(bytes("Can only retrieve past block hashes")); + epochManager.blockHash(current); + + vm.expectRevert(bytes("Can only retrieve past block hashes")); + epochManager.blockHash(current + 1); + } + + function test_Calculations_RevertWhen_BlockHashTooOld() public withLifecycleLength { + vm.roll(1000); + + vm.expectRevert(bytes("Can only retrieve hashes for last 256 blocks")); + epochManager.blockHash(1000 - 257); + } + + // -- epoch lifecycle: progression -- + + function test_Progression_NotRun() public withLifecycleLength { + _rollToNextEpoch(); + assertFalse(epochManager.isCurrentEpochRun()); + } + + function test_Progression_RunEpoch() public withLifecycleLength { + _rollToNextEpoch(); + + uint256 currentEpoch = epochManager.currentEpoch(); + vm.expectEmit(address(epochManager)); + emit EpochRun(currentEpoch, me); + vm.prank(me); + epochManager.runEpoch(); + + assertEq(epochManager.lastRunEpoch(), currentEpoch); + } + + function test_Progression_AlreadyRun() public withLifecycleLength { + _rollToNextEpoch(); + epochManager.runEpoch(); + + assertTrue(epochManager.isCurrentEpochRun()); + } + + function test_Progression_RevertWhen_AlreadyRun() public withLifecycleLength { + _rollToNextEpoch(); + epochManager.runEpoch(); + + vm.expectRevert(bytes("Current epoch already run")); + epochManager.runEpoch(); + } +} diff --git a/packages/horizon/test/unit/epochs/IEpochManagerLike.sol b/packages/horizon/test/unit/epochs/IEpochManagerLike.sol new file mode 100644 index 000000000..e170070f4 --- /dev/null +++ b/packages/horizon/test/unit/epochs/IEpochManagerLike.sol @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.27; + +import { IEpochManager } from "@graphprotocol/interfaces/contracts/contracts/epochs/IEpochManager.sol"; + +/// IEpochManager plus the initializer, storage getters and events the interface omits. +interface IEpochManagerLike is IEpochManager { + event EpochRun(uint256 indexed epoch, address caller); + event EpochLengthUpdate(uint256 indexed epoch, uint256 epochLength); + + function initialize(address controller, uint256 epochLength) external; + + function epochLength() external view returns (uint256); + + function lastRunEpoch() external view returns (uint256); + + function lastLengthUpdateEpoch() external view returns (uint256); + + function lastLengthUpdateBlock() external view returns (uint256); +} diff --git a/packages/horizon/test/unit/shared/ProtocolFixture.t.sol b/packages/horizon/test/unit/shared/ProtocolFixture.t.sol new file mode 100644 index 000000000..f38962c8e --- /dev/null +++ b/packages/horizon/test/unit/shared/ProtocolFixture.t.sol @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.27; + +import { Test } from "forge-std/Test.sol"; +import { IController } from "@graphprotocol/interfaces/contracts/contracts/governance/IController.sol"; +import { IGraphProxy } from "@graphprotocol/interfaces/contracts/contracts/upgrades/IGraphProxy.sol"; +import { IGraphProxyAdmin } from "@graphprotocol/interfaces/contracts/contracts/upgrades/IGraphProxyAdmin.sol"; + +abstract contract ProtocolFixture is Test { + string internal constant CORE_ARTIFACTS = "node_modules/@graphprotocol/contracts/artifacts/contracts/"; + string internal constant PKG_ARTIFACTS = "node_modules/@graphprotocol/contracts/artifacts/contracts/"; + + address internal governor; // makeAddr("governor"), set in _deployProtocolBase + IController internal controller; + IGraphProxyAdmin internal proxyAdmin; + + /// path is a full artifact path, e.g. string.concat(CORE_ARTIFACTS, "governance/Controller.sol/Controller.json"). + /// Deploys `.bytecode ++ args` with CREATE (honours an active vm.prank). + /// Reverts "artifact needs linking" if `.linkReferences` has any key (vm.parseJsonKeys), + /// and "artifact deploy failed" if CREATE returns 0. + function _deployArtifact(string memory path, bytes memory args) internal returns (address deployed) { + string memory json = vm.readFile(path); + require(vm.parseJsonKeys(json, ".linkReferences").length == 0, "artifact needs linking"); + bytes memory code = bytes.concat(vm.parseJsonBytes(json, ".bytecode"), args); + // solhint-disable-next-line no-inline-assembly + assembly { + deployed := create(0, add(code, 0x20), mload(code)) + } + require(deployed != address(0), "artifact deploy failed"); + } + + /// As governor: deploys Controller and GraphProxyAdmin from CORE_ARTIFACTS, then controller.setPaused(false). + /// Must be called with no active prank. + function _deployProtocolBase() internal { + governor = makeAddr("governor"); + vm.startPrank(governor); + controller = IController( + _deployArtifact(string.concat(CORE_ARTIFACTS, "governance/Controller.sol/Controller.json"), "") + ); + proxyAdmin = IGraphProxyAdmin( + _deployArtifact(string.concat(CORE_ARTIFACTS, "upgrades/GraphProxyAdmin.sol/GraphProxyAdmin.json"), "") + ); + controller.setPaused(false); + vm.stopPrank(); + } + + /// As governor: deploys GraphProxy(address(0), proxyAdmin) from CORE_ARTIFACTS, + /// proxyAdmin.upgrade(proxy, impl), proxyAdmin.acceptProxyAndCall(impl, proxy, initData). + /// Must be called with no active prank. + function _deployBehindProxy(address impl, bytes memory initData) internal returns (address proxy) { + vm.startPrank(governor); + proxy = _deployArtifact( + string.concat(CORE_ARTIFACTS, "upgrades/GraphProxy.sol/GraphProxy.json"), + abi.encode(address(0), address(proxyAdmin)) + ); + proxyAdmin.upgrade(IGraphProxy(proxy), impl); + proxyAdmin.acceptProxyAndCall(impl, IGraphProxy(proxy), initData); + vm.stopPrank(); + } + + /// As governor: controller.setContractProxy(keccak256(bytes(name)), addr). + function _registerContract(string memory name, address addr) internal { + vm.prank(governor); + controller.setContractProxy(keccak256(bytes(name)), addr); + } +} diff --git a/scripts/contracts-migration/test-mapping/t4.md b/scripts/contracts-migration/test-mapping/t4.md new file mode 100644 index 000000000..f9fa7eab1 --- /dev/null +++ b/scripts/contracts-migration/test-mapping/t4.md @@ -0,0 +1,23 @@ +# T4 test mapping: EpochManager + +New tests are in `packages/horizon`, deploying the EpochManager Hardhat artifact behind a GraphProxy via `ProtocolFixture`. The old fixture's `epochs.lengthInBlocks` default (60) is the initial epoch length; the lifecycle tests call `setEpochLength(3)` as the old `beforeEach` did. + +| Old file | Old test (describe › it) | New test (file::function) | Note | +| ------------------------------------------------- | -------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------- | ---- | +| packages/contracts-test/tests/unit/epochs.test.ts | EpochManager › configuration › should set `epochLength | test/unit/epochs/EpochManager.t.sol::test_Configuration_SetEpochLength | | +| packages/contracts-test/tests/unit/epochs.test.ts | EpochManager › configuration › reject set `epochLength` if zero | test/unit/epochs/EpochManager.t.sol::test_Configuration_RevertWhen_EpochLengthZero | | +| packages/contracts-test/tests/unit/epochs.test.ts | EpochManager › epoch lifecycle › calculations › first epoch should be 1 | test/unit/epochs/EpochManager.t.sol::test_Calculations_FirstEpochIsOne | | +| packages/contracts-test/tests/unit/epochs.test.ts | EpochManager › epoch lifecycle › calculations › should return correct block number | test/unit/epochs/EpochManager.t.sol::test_Calculations_BlockNum | | +| packages/contracts-test/tests/unit/epochs.test.ts | EpochManager › epoch lifecycle › calculations › should return same starting block if we stay on the same epoch | test/unit/epochs/EpochManager.t.sol::test_Calculations_SameStartingBlockOnSameEpoch | | +| packages/contracts-test/tests/unit/epochs.test.ts | EpochManager › epoch lifecycle › calculations › should return next starting block if we move to the next epoch | test/unit/epochs/EpochManager.t.sol::test_Calculations_NextStartingBlockOnNextEpoch | | +| packages/contracts-test/tests/unit/epochs.test.ts | EpochManager › epoch lifecycle › calculations › should return next epoch if advance > epochLength | test/unit/epochs/EpochManager.t.sol::test_Calculations_NextEpochWhenAdvancePastLength | | +| packages/contracts-test/tests/unit/epochs.test.ts | EpochManager › epoch lifecycle › progression › > epoch not run › should return that current epoch is not run | test/unit/epochs/EpochManager.t.sol::test_Progression_NotRun | | +| packages/contracts-test/tests/unit/epochs.test.ts | EpochManager › epoch lifecycle › progression › > epoch not run › should run new epoch | test/unit/epochs/EpochManager.t.sol::test_Progression_RunEpoch | | +| packages/contracts-test/tests/unit/epochs.test.ts | EpochManager › epoch lifecycle › progression › > epoch run › should return current epoch is already run | test/unit/epochs/EpochManager.t.sol::test_Progression_AlreadyRun | | +| packages/contracts-test/tests/unit/epochs.test.ts | EpochManager › epoch lifecycle › progression › > epoch run › reject run new epoch | test/unit/epochs/EpochManager.t.sol::test_Progression_RevertWhen_AlreadyRun | | + +## Coverage + +MISSING initialize: `EpochManager.initialize` is called through the proxy init data (`acceptProxyAndCall`) built with `abi.encodeCall` in `EpochManagerTest.setUp`, not by a `.initialize(` call; every test runs it, and `test_Configuration_SetEpochLength` asserts its result (`epochLength() == 60`). + +Additions with no old test: `test_Configuration_RevertWhen_EpochLengthSame` (`Epoch length must be different to current`), `test_Calculations_RevertWhen_BlockHashNotPast` and `test_Calculations_RevertWhen_BlockHashTooOld` (`blockHash` reverts), `test_Calculations_EpochsSince` (`epochsSince`, `epochsSinceUpdate`, `currentEpochBlockSinceStart` after rolls), and `test_EpochManager_SameBlockDoesNotAdvance` (same block, same epoch and epoch block). `runEpoch` and `isCurrentEpochRun` are covered by the ported progression tests.