diff --git a/packages/horizon/contracts/interfaces/IRecurringCollector.sol b/packages/horizon/contracts/interfaces/IRecurringCollector.sol index a53439a7c..954b1be94 100644 --- a/packages/horizon/contracts/interfaces/IRecurringCollector.sol +++ b/packages/horizon/contracts/interfaces/IRecurringCollector.sol @@ -413,4 +413,13 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector { * @return The AgreementData struct containing the agreement's data. */ function getAgreement(bytes16 agreementId) external view returns (AgreementData memory); + + /** + * @notice Checks if an agreement is collectable. + * @dev "Collectable" means the agreement is in a valid state that allows collection attempts, + * not that there are necessarily funds available to collect. + * @param agreement The agreement data + * @return The boolean indicating if the agreement is collectable + */ + function isCollectable(AgreementData memory agreement) external view returns (bool); } diff --git a/packages/horizon/contracts/payments/collectors/RecurringCollector.sol b/packages/horizon/contracts/payments/collectors/RecurringCollector.sol index 99122a348..e1225f6fa 100644 --- a/packages/horizon/contracts/payments/collectors/RecurringCollector.sol +++ b/packages/horizon/contracts/payments/collectors/RecurringCollector.sol @@ -249,6 +249,11 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC return _getAgreement(agreementId); } + /// @inheritdoc IRecurringCollector + function isCollectable(AgreementData memory agreement) external pure returns (bool) { + return _isCollectable(agreement); + } + /** * @notice Decodes the collect data. * @param data The encoded collect parameters. @@ -270,7 +275,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC function _collect(CollectParams memory _params) private returns (uint256) { AgreementData storage agreement = _getAgreementStorage(_params.agreementId); require( - agreement.state == AgreementState.Accepted || agreement.state == AgreementState.CanceledByPayer, + _isCollectable(agreement), RecurringCollectorAgreementIncorrectState(_params.agreementId, agreement.state) ); @@ -537,4 +542,13 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC function _agreementCollectionStartAt(AgreementData memory _agreement) private pure returns (uint256) { return _agreement.lastCollectionAt > 0 ? _agreement.lastCollectionAt : _agreement.acceptedAt; } + + /** + * @notice Requires that the agreement is collectable. + * @param _agreement The agreement data + * @return The boolean indicating if the agreement is collectable + */ + function _isCollectable(AgreementData memory _agreement) private pure returns (bool) { + return _agreement.state == AgreementState.Accepted || _agreement.state == AgreementState.CanceledByPayer; + } } diff --git a/packages/horizon/package.json b/packages/horizon/package.json index 641fc61dd..4e2d86374 100644 --- a/packages/horizon/package.json +++ b/packages/horizon/package.json @@ -26,7 +26,9 @@ "build": "hardhat compile", "test": "forge test", "test:deployment": "SECURE_ACCOUNTS_DISABLE_PROVIDER=true hardhat test test/deployment/*.ts", - "test:integration": "./scripts/integration" + "test:integration": "./scripts/integration", + "test:coverage": "forge coverage --no-match-coverage \"test/*|contracts/mocks/*\"", + "test:coverage:lcov": "forge coverage --no-match-coverage \"test/*|contracts/mocks/*\" --report lcov" }, "devDependencies": { "@defi-wonderland/natspec-smells": "^1.1.6", diff --git a/packages/horizon/test/unit/data-service/utilities/ProvisionManager.t.sol b/packages/horizon/test/unit/data-service/utilities/ProvisionManager.t.sol new file mode 100644 index 000000000..3617e95a5 --- /dev/null +++ b/packages/horizon/test/unit/data-service/utilities/ProvisionManager.t.sol @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity 0.8.27; + +import { Test } from "forge-std/Test.sol"; + +import { ProvisionManager } from "../../../../contracts/data-service/utilities/ProvisionManager.sol"; +import { IHorizonStakingTypes } from "../../../../contracts/interfaces/internal/IHorizonStakingTypes.sol"; +import { PartialControllerMock } from "../../mocks/PartialControllerMock.t.sol"; +import { HorizonStakingMock } from "../../mocks/HorizonStakingMock.t.sol"; +import { ProvisionManagerImpl } from "./ProvisionManagerImpl.t.sol"; + +contract ProvisionManagerTest is Test { + ProvisionManagerImpl internal _provisionManager; + HorizonStakingMock internal _horizonStakingMock; + + function setUp() public { + _horizonStakingMock = new HorizonStakingMock(); + + PartialControllerMock.Entry[] memory entries = new PartialControllerMock.Entry[](1); + entries[0] = PartialControllerMock.Entry({ name: "Staking", addr: address(_horizonStakingMock) }); + _provisionManager = new ProvisionManagerImpl(address(new PartialControllerMock(entries))); + } + + /* solhint-disable graph/func-name-mixedcase */ + + function test_OnlyValidProvision(address serviceProvider) public { + vm.expectRevert( + abi.encodeWithSelector(ProvisionManager.ProvisionManagerProvisionNotFound.selector, serviceProvider) + ); + _provisionManager.onlyValidProvision_(serviceProvider); + + IHorizonStakingTypes.Provision memory provision; + provision.createdAt = 1; + + _horizonStakingMock.setProvision(serviceProvider, address(_provisionManager), provision); + + _provisionManager.onlyValidProvision_(serviceProvider); + } + + function test_OnlyAuthorizedForProvision(address serviceProvider, address sender) public { + vm.expectRevert( + abi.encodeWithSelector(ProvisionManager.ProvisionManagerNotAuthorized.selector, serviceProvider, sender) + ); + vm.prank(sender); + _provisionManager.onlyAuthorizedForProvision_(serviceProvider); + + _horizonStakingMock.setIsAuthorized(serviceProvider, address(_provisionManager), sender, true); + vm.prank(sender); + _provisionManager.onlyAuthorizedForProvision_(serviceProvider); + } + + /* solhint-enable graph/func-name-mixedcase */ +} diff --git a/packages/horizon/test/unit/data-service/utilities/ProvisionManagerImpl.t.sol b/packages/horizon/test/unit/data-service/utilities/ProvisionManagerImpl.t.sol new file mode 100644 index 000000000..4170d17da --- /dev/null +++ b/packages/horizon/test/unit/data-service/utilities/ProvisionManagerImpl.t.sol @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity 0.8.27; + +import { ProvisionManager } from "../../../../contracts/data-service/utilities/ProvisionManager.sol"; +import { GraphDirectory } from "../../../../contracts/utilities/GraphDirectory.sol"; + +contract ProvisionManagerImpl is GraphDirectory, ProvisionManager { + constructor(address controller) GraphDirectory(controller) {} + + function onlyValidProvision_(address serviceProvider) public view onlyValidProvision(serviceProvider) {} + + function onlyAuthorizedForProvision_( + address serviceProvider + ) public view onlyAuthorizedForProvision(serviceProvider) {} +} diff --git a/packages/horizon/test/unit/libraries/StakeClaims.t.sol b/packages/horizon/test/unit/libraries/StakeClaims.t.sol new file mode 100644 index 000000000..d98bdf78e --- /dev/null +++ b/packages/horizon/test/unit/libraries/StakeClaims.t.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity 0.8.27; + +import { Test } from "forge-std/Test.sol"; + +import { StakeClaims } from "../../../contracts/data-service/libraries/StakeClaims.sol"; + +contract StakeClaimsTest is Test { + /* solhint-disable graph/func-name-mixedcase */ + + function test_BuildStakeClaimId(address dataService, address serviceProvider, uint256 nonce) public pure { + bytes32 id = StakeClaims.buildStakeClaimId(dataService, serviceProvider, nonce); + bytes32 expectedId = keccak256(abi.encodePacked(dataService, serviceProvider, nonce)); + assertEq(id, expectedId, "StakeClaim ID does not match expected value"); + } + + /* solhint-enable graph/func-name-mixedcase */ +} diff --git a/packages/horizon/test/unit/mocks/HorizonStakingMock.t.sol b/packages/horizon/test/unit/mocks/HorizonStakingMock.t.sol new file mode 100644 index 000000000..647df06f7 --- /dev/null +++ b/packages/horizon/test/unit/mocks/HorizonStakingMock.t.sol @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import { IHorizonStakingTypes } from "../../../contracts/interfaces/internal/IHorizonStakingTypes.sol"; + +contract HorizonStakingMock { + mapping(address => mapping(address => IHorizonStakingTypes.Provision)) public provisions; + mapping(address => mapping(address => mapping(address => bool))) public authorizations; + + function setProvision( + address serviceProvider, + address verifier, + IHorizonStakingTypes.Provision memory provision + ) external { + provisions[serviceProvider][verifier] = provision; + } + + function getProvision( + address serviceProvider, + address verifier + ) external view returns (IHorizonStakingTypes.Provision memory) { + return provisions[serviceProvider][verifier]; + } + + function isAuthorized(address serviceProvider, address verifier, address operator) external view returns (bool) { + return authorizations[serviceProvider][verifier][operator]; + } + + function setIsAuthorized(address serviceProvider, address verifier, address operator, bool authorized) external { + authorizations[serviceProvider][verifier][operator] = authorized; + } +} diff --git a/packages/horizon/test/unit/mocks/InvalidControllerMock.t.sol b/packages/horizon/test/unit/mocks/InvalidControllerMock.t.sol new file mode 100644 index 000000000..f4d31da12 --- /dev/null +++ b/packages/horizon/test/unit/mocks/InvalidControllerMock.t.sol @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import { PartialControllerMock } from "./PartialControllerMock.t.sol"; + +contract InvalidControllerMock is PartialControllerMock { + constructor() PartialControllerMock(new PartialControllerMock.Entry[](0)) {} +} diff --git a/packages/horizon/test/unit/mocks/PartialControllerMock.t.sol b/packages/horizon/test/unit/mocks/PartialControllerMock.t.sol new file mode 100644 index 000000000..f315ff5ea --- /dev/null +++ b/packages/horizon/test/unit/mocks/PartialControllerMock.t.sol @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import { Test } from "forge-std/Test.sol"; + +import { ControllerMock } from "../../../contracts/mocks/ControllerMock.sol"; + +contract PartialControllerMock is ControllerMock, Test { + struct Entry { + string name; + address addr; + } + + address private _invalidContractAddress; + + Entry[] private _contracts; + + constructor(Entry[] memory contracts) ControllerMock(address(0)) { + for (uint256 i = 0; i < contracts.length; i++) { + _contracts.push(Entry({ name: contracts[i].name, addr: contracts[i].addr })); + } + _invalidContractAddress = makeAddr("invalidContractAddress"); + } + + function getContractProxy(bytes32 data) external view override returns (address) { + for (uint256 i = 0; i < _contracts.length; i++) { + if (keccak256(abi.encodePacked(_contracts[i].name)) == data) { + return _contracts[i].addr; + } + } + return _invalidContractAddress; + } +} diff --git a/packages/horizon/test/unit/payments/recurring-collector/RecurringCollectorAuthorizableTest.t.sol b/packages/horizon/test/unit/payments/recurring-collector/RecurringCollectorAuthorizableTest.t.sol index ff5e39848..91244fea1 100644 --- a/packages/horizon/test/unit/payments/recurring-collector/RecurringCollectorAuthorizableTest.t.sol +++ b/packages/horizon/test/unit/payments/recurring-collector/RecurringCollectorAuthorizableTest.t.sol @@ -5,16 +5,10 @@ import { IAuthorizable } from "../../../../contracts/interfaces/IAuthorizable.so import { RecurringCollector } from "../../../../contracts/payments/collectors/RecurringCollector.sol"; import { AuthorizableTest } from "../../../unit/utilities/Authorizable.t.sol"; -import { RecurringCollectorControllerMock } from "./RecurringCollectorControllerMock.t.sol"; +import { InvalidControllerMock } from "../../mocks/InvalidControllerMock.t.sol"; contract RecurringCollectorAuthorizableTest is AuthorizableTest { function newAuthorizable(uint256 thawPeriod) public override returns (IAuthorizable) { - return - new RecurringCollector( - "RecurringCollector", - "1", - address(new RecurringCollectorControllerMock(address(1))), - thawPeriod - ); + return new RecurringCollector("RecurringCollector", "1", address(new InvalidControllerMock()), thawPeriod); } } diff --git a/packages/horizon/test/unit/payments/recurring-collector/RecurringCollectorControllerMock.t.sol b/packages/horizon/test/unit/payments/recurring-collector/RecurringCollectorControllerMock.t.sol deleted file mode 100644 index 3425e8b01..000000000 --- a/packages/horizon/test/unit/payments/recurring-collector/RecurringCollectorControllerMock.t.sol +++ /dev/null @@ -1,25 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.27; - -import { Test } from "forge-std/Test.sol"; - -import { IPaymentsEscrow } from "../../../../contracts/interfaces/IPaymentsEscrow.sol"; -import { ControllerMock } from "../../../../contracts/mocks/ControllerMock.sol"; - -contract RecurringCollectorControllerMock is ControllerMock, Test { - address private _invalidContractAddress; - IPaymentsEscrow private _paymentsEscrow; - - constructor(address paymentsEscrow) ControllerMock(address(0)) { - _invalidContractAddress = makeAddr("invalidContractAddress"); - _paymentsEscrow = IPaymentsEscrow(paymentsEscrow); - } - - function getContractProxy(bytes32 data) external view override returns (address) { - return data == keccak256("PaymentsEscrow") ? address(_paymentsEscrow) : _invalidContractAddress; - } - - function getPaymentsEscrow() external view returns (address) { - return address(_paymentsEscrow); - } -} diff --git a/packages/horizon/test/unit/payments/recurring-collector/base.t.sol b/packages/horizon/test/unit/payments/recurring-collector/base.t.sol new file mode 100644 index 000000000..9512fbf87 --- /dev/null +++ b/packages/horizon/test/unit/payments/recurring-collector/base.t.sol @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import { IRecurringCollector } from "../../../../contracts/interfaces/IRecurringCollector.sol"; + +import { RecurringCollectorSharedTest } from "./shared.t.sol"; + +contract RecurringCollectorBaseTest is RecurringCollectorSharedTest { + /* + * TESTS + */ + + /* solhint-disable graph/func-name-mixedcase */ + + function test_RecoverRCASigner(FuzzyTestAccept memory fuzzyTestAccept) public view { + uint256 signerKey = boundKey(fuzzyTestAccept.unboundedSignerKey); + IRecurringCollector.SignedRCA memory signedRCA = _recurringCollectorHelper.generateSignedRCA( + fuzzyTestAccept.rca, + signerKey + ); + + assertEq( + _recurringCollector.recoverRCASigner(signedRCA), + vm.addr(signerKey), + "Recovered RCA signer does not match" + ); + } + + function test_RecoverRCAUSigner(FuzzyTestUpdate memory fuzzyTestUpdate) public view { + uint256 signerKey = boundKey(fuzzyTestUpdate.fuzzyTestAccept.unboundedSignerKey); + IRecurringCollector.SignedRCAU memory signedRCAU = _recurringCollectorHelper.generateSignedRCAU( + fuzzyTestUpdate.rcau, + signerKey + ); + + assertEq( + _recurringCollector.recoverRCAUSigner(signedRCAU), + vm.addr(signerKey), + "Recovered RCAU signer does not match" + ); + } + + /* solhint-enable graph/func-name-mixedcase */ +} diff --git a/packages/horizon/test/unit/payments/recurring-collector/shared.t.sol b/packages/horizon/test/unit/payments/recurring-collector/shared.t.sol index 397925600..8dd270b2f 100644 --- a/packages/horizon/test/unit/payments/recurring-collector/shared.t.sol +++ b/packages/horizon/test/unit/payments/recurring-collector/shared.t.sol @@ -9,7 +9,7 @@ import { IRecurringCollector } from "../../../../contracts/interfaces/IRecurring import { RecurringCollector } from "../../../../contracts/payments/collectors/RecurringCollector.sol"; import { Bounder } from "../../../unit/utils/Bounder.t.sol"; -import { RecurringCollectorControllerMock } from "./RecurringCollectorControllerMock.t.sol"; +import { PartialControllerMock } from "../../mocks/PartialControllerMock.t.sol"; import { PaymentsEscrowMock } from "./PaymentsEscrowMock.t.sol"; import { RecurringCollectorHelper } from "./RecurringCollectorHelper.t.sol"; @@ -35,10 +35,12 @@ contract RecurringCollectorSharedTest is Test, Bounder { function setUp() public { _paymentsEscrow = new PaymentsEscrowMock(); + PartialControllerMock.Entry[] memory entries = new PartialControllerMock.Entry[](1); + entries[0] = PartialControllerMock.Entry({ name: "PaymentsEscrow", addr: address(_paymentsEscrow) }); _recurringCollector = new RecurringCollector( "RecurringCollector", "1", - address(new RecurringCollectorControllerMock(address(_paymentsEscrow))), + address(new PartialControllerMock(entries)), 1 ); _recurringCollectorHelper = new RecurringCollectorHelper(_recurringCollector); diff --git a/packages/subgraph-service/contracts/libraries/IndexingAgreement.sol b/packages/subgraph-service/contracts/libraries/IndexingAgreement.sol index a3669fffc..d1bea35c8 100644 --- a/packages/subgraph-service/contracts/libraries/IndexingAgreement.sol +++ b/packages/subgraph-service/contracts/libraries/IndexingAgreement.sol @@ -254,6 +254,12 @@ library IndexingAgreement { */ error IndexingAgreementNotActive(bytes16 agreementId); + /** + * @notice Thrown when the agreement is not collectable + * @param agreementId The agreement ID + */ + error IndexingAgreementNotCollectable(bytes16 agreementId); + /** * @notice Thrown when trying to interact with an agreement not owned by the indexer * @param agreementId The agreement ID @@ -517,7 +523,7 @@ library IndexingAgreement { wrapper.agreement.allocationId, wrapper.collectorAgreement.serviceProvider ); - require(_isActive(wrapper), IndexingAgreementNotActive(params.agreementId)); + require(_isCollectable(wrapper), IndexingAgreementNotCollectable(params.agreementId)); require( wrapper.agreement.version == IndexingAgreementVersion.V1, @@ -692,17 +698,37 @@ library IndexingAgreement { /** * @notice Checks if the agreement is active * Requirements: + * - The indexing agreement is valid * - The underlying collector agreement has been accepted - * - The underlying collector agreement's data service is this contract - * - The indexing agreement has been accepted and has a valid allocation ID * @param wrapper The agreement wrapper containing the indexing agreement and collector agreement data * @return True if the agreement is active, false otherwise **/ function _isActive(AgreementWrapper memory wrapper) private view returns (bool) { - return - wrapper.collectorAgreement.dataService == address(this) && - wrapper.collectorAgreement.state == IRecurringCollector.AgreementState.Accepted && - wrapper.agreement.allocationId != address(0); + return _isValid(wrapper) && wrapper.collectorAgreement.state == IRecurringCollector.AgreementState.Accepted; + } + + /** + * @notice Checks if the agreement is collectable + * Requirements: + * - The indexing agreement is valid + * - The underlying collector agreement is collectable + * @param wrapper The agreement wrapper containing the indexing agreement and collector agreement data + * @return True if the agreement is collectable, false otherwise + **/ + function _isCollectable(AgreementWrapper memory wrapper) private view returns (bool) { + return _isValid(wrapper) && _directory().recurringCollector().isCollectable(wrapper.collectorAgreement); + } + + /** + * @notice Checks if the agreement is valid + * Requirements: + * - The underlying collector agreement's data service is this contract + * - The indexing agreement has been accepted and has a valid allocation ID + * @param wrapper The agreement wrapper containing the indexing agreement and collector agreement data + * @return True if the agreement is valid, false otherwise + **/ + function _isValid(AgreementWrapper memory wrapper) private view returns (bool) { + return wrapper.collectorAgreement.dataService == address(this) && wrapper.agreement.allocationId != address(0); } /** diff --git a/packages/subgraph-service/package.json b/packages/subgraph-service/package.json index 0b000778c..6138c0c61 100644 --- a/packages/subgraph-service/package.json +++ b/packages/subgraph-service/package.json @@ -26,7 +26,9 @@ "build": "hardhat compile", "test": "forge test", "test:deployment": "SECURE_ACCOUNTS_DISABLE_PROVIDER=true hardhat test test/deployment/*.ts", - "test:integration": "./scripts/integration" + "test:integration": "./scripts/integration", + "test:coverage": "forge coverage --no-match-coverage \"test/*|contracts/mocks/*\"", + "test:coverage:lcov": "forge coverage --no-match-coverage \"test/*|contracts/mocks/*\" --report lcov" }, "devDependencies": { "@defi-wonderland/natspec-smells": "^1.1.6", diff --git a/packages/subgraph-service/test/unit/libraries/IndexingAgreement.sol b/packages/subgraph-service/test/unit/libraries/IndexingAgreement.sol deleted file mode 100644 index 4afc6707e..000000000 --- a/packages/subgraph-service/test/unit/libraries/IndexingAgreement.sol +++ /dev/null @@ -1,18 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity 0.8.27; - -import { Test } from "forge-std/Test.sol"; -import { IndexingAgreement } from "../../../contracts/libraries/IndexingAgreement.sol"; - -contract IndexingAgreementTest is Test { - function test_StorageManagerLocation() public pure { - assertEq( - IndexingAgreement.INDEXING_AGREEMENT_STORAGE_MANAGER_LOCATION, - keccak256( - abi.encode( - uint256(keccak256("graphprotocol.subgraph-service.storage.StorageManager.IndexingAgreement")) - 1 - ) - ) & ~bytes32(uint256(0xff)) - ); - } -} diff --git a/packages/subgraph-service/test/unit/libraries/IndexingAgreement.t.sol b/packages/subgraph-service/test/unit/libraries/IndexingAgreement.t.sol new file mode 100644 index 000000000..a545c8571 --- /dev/null +++ b/packages/subgraph-service/test/unit/libraries/IndexingAgreement.t.sol @@ -0,0 +1,107 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity 0.8.27; + +import { Test } from "forge-std/Test.sol"; + +import { IRecurringCollector } from "@graphprotocol/horizon/contracts/interfaces/IRecurringCollector.sol"; +import { IndexingAgreement } from "../../../contracts/libraries/IndexingAgreement.sol"; +import { Directory } from "../../../contracts/utilities/Directory.sol"; + +contract IndexingAgreementTest is Test { + IndexingAgreement.StorageManager private _storageManager; + address private _mockCollector; + + function setUp() public { + _mockCollector = makeAddr("mockCollector"); + } + + function test_IndexingAgreement_Get(bytes16 agreementId) public { + vm.assume(agreementId != bytes16(0)); + + vm.mockCall( + address(this), + abi.encodeWithSelector(Directory.recurringCollector.selector), + abi.encode(IRecurringCollector(_mockCollector)) + ); + + IRecurringCollector.AgreementData memory collectorAgreement; + vm.mockCall( + _mockCollector, + abi.encodeWithSelector(IRecurringCollector.getAgreement.selector, agreementId), + abi.encode(collectorAgreement) + ); + + vm.expectRevert(abi.encodeWithSelector(IndexingAgreement.IndexingAgreementNotActive.selector, agreementId)); + IndexingAgreement.get(_storageManager, agreementId); + + collectorAgreement.dataService = address(this); + vm.mockCall( + _mockCollector, + abi.encodeWithSelector(IRecurringCollector.getAgreement.selector, agreementId), + abi.encode(collectorAgreement) + ); + + IndexingAgreement.AgreementWrapper memory wrapper = IndexingAgreement.get(_storageManager, agreementId); + assertEq(wrapper.collectorAgreement.dataService, address(this)); + } + + function test_IndexingAgreement_OnCloseAllocation(bytes16 agreementId, address allocationId, bool stale) public { + vm.assume(agreementId != bytes16(0)); + vm.assume(allocationId != address(0)); + + delete _storageManager; + vm.clearMockedCalls(); + + // No active agreement for allocation ID, returns early, no assertions needed + IndexingAgreement.onCloseAllocation(_storageManager, allocationId, stale); + + // Active agreement for allocation ID, but collector agreement is not set, returns early, no assertions needed + _storageManager.allocationToActiveAgreementId[allocationId] = agreementId; + + IRecurringCollector.AgreementData memory collectorAgreement; + + vm.mockCall( + address(this), + abi.encodeWithSelector(Directory.recurringCollector.selector), + abi.encode(IRecurringCollector(_mockCollector)) + ); + + vm.mockCall( + _mockCollector, + abi.encodeWithSelector(IRecurringCollector.getAgreement.selector, agreementId), + abi.encode(collectorAgreement) + ); + + IndexingAgreement.onCloseAllocation(_storageManager, allocationId, stale); + + // Active agreement for allocation ID, collector agreement is set, should cancel the agreement + collectorAgreement.dataService = address(this); + collectorAgreement.state = IRecurringCollector.AgreementState.Accepted; + + _storageManager.agreements[agreementId] = IndexingAgreement.State({ + allocationId: allocationId, + version: IndexingAgreement.IndexingAgreementVersion.V1 + }); + + vm.mockCall( + _mockCollector, + abi.encodeWithSelector(IRecurringCollector.getAgreement.selector, agreementId), + abi.encode(collectorAgreement) + ); + + vm.expectCall(_mockCollector, abi.encodeWithSelector(IRecurringCollector.cancel.selector, agreementId)); + + IndexingAgreement.onCloseAllocation(_storageManager, allocationId, stale); + } + + function test_IndexingAgreement_StorageManagerLocation() public pure { + assertEq( + IndexingAgreement.INDEXING_AGREEMENT_STORAGE_MANAGER_LOCATION, + keccak256( + abi.encode( + uint256(keccak256("graphprotocol.subgraph-service.storage.StorageManager.IndexingAgreement")) - 1 + ) + ) & ~bytes32(uint256(0xff)) + ); + } +} diff --git a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/accept.t.sol b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/accept.t.sol index 29f83126c..77b18308c 100644 --- a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/accept.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/accept.t.sol @@ -225,6 +225,25 @@ contract SubgraphServiceIndexingAgreementAcceptTest is SubgraphServiceIndexingAg function test_SubgraphService_AcceptIndexingAgreement_Revert_WhenAgreementAlreadyAllocated() public {} + function test_SubgraphService_AcceptIndexingAgreement_Revert_WhenInvalidTermsData(Seed memory seed) public { + Context storage ctx = _newCtx(seed); + IndexerState memory indexerState = _withIndexer(ctx); + IRecurringCollector.SignedRCA memory acceptable = _generateAcceptableSignedRCA(ctx, indexerState.addr); + bytes memory invalidTermsData = bytes("invalid terms data"); + acceptable.rca.metadata = abi.encode( + _newAcceptIndexingAgreementMetadataV1Terms(indexerState.subgraphDeploymentId, invalidTermsData) + ); + + bytes memory expectedErr = abi.encodeWithSelector( + IndexingAgreementDecoder.IndexingAgreementDecoderInvalidData.selector, + "decodeCollectIndexingFeeData", + invalidTermsData + ); + vm.expectRevert(expectedErr); + resetPrank(indexerState.addr); + subgraphService.acceptIndexingAgreement(indexerState.allocationId, acceptable); + } + function test_SubgraphService_AcceptIndexingAgreement(Seed memory seed) public { Context storage ctx = _newCtx(seed); IndexerState memory indexerState = _withIndexer(ctx); diff --git a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/base.t.sol b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/base.t.sol index 822cc21d7..2eda9dfc0 100644 --- a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/base.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/base.t.sol @@ -2,7 +2,9 @@ pragma solidity 0.8.27; import { TransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; +import { IRecurringCollector } from "@graphprotocol/horizon/contracts/interfaces/IRecurringCollector.sol"; +import { IndexingAgreement } from "../../../../contracts/libraries/IndexingAgreement.sol"; import { SubgraphServiceIndexingAgreementSharedTest } from "./shared.t.sol"; contract SubgraphServiceIndexingAgreementBaseTest is SubgraphServiceIndexingAgreementSharedTest { @@ -11,6 +13,25 @@ contract SubgraphServiceIndexingAgreementBaseTest is SubgraphServiceIndexingAgre */ /* solhint-disable graph/func-name-mixedcase */ + function test_SubgraphService_GetIndexingAgreement(Seed memory seed, address operator, bytes16 agreementId) public { + vm.assume(_isSafeSubgraphServiceCaller(operator)); + + resetPrank(address(operator)); + + // Get unkown indexing agreement + vm.expectRevert(abi.encodeWithSelector(IndexingAgreement.IndexingAgreementNotActive.selector, agreementId)); + subgraphService.getIndexingAgreement(agreementId); + + // Accept an indexing agreement + Context storage ctx = _newCtx(seed); + IndexerState memory indexerState = _withIndexer(ctx); + IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, indexerState); + IndexingAgreement.AgreementWrapper memory agreement = subgraphService.getIndexingAgreement( + accepted.rca.agreementId + ); + _assertEqualAgreement(accepted.rca, agreement); + } + function test_SubgraphService_Revert_WhenUnsafeAddress_WhenProxyAdmin(address indexer, bytes16 agreementId) public { address operator = _transparentUpgradeableProxyAdmin(); assertFalse(_isSafeSubgraphServiceCaller(operator)); diff --git a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/collect.t.sol b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/collect.t.sol index 85c203b6e..57a7a907f 100644 --- a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/collect.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/collect.t.sol @@ -11,6 +11,7 @@ import { ISubgraphService } from "../../../../contracts/interfaces/ISubgraphServ import { Allocation } from "../../../../contracts/libraries/Allocation.sol"; import { AllocationHandler } from "../../../../contracts/libraries/AllocationHandler.sol"; import { IndexingAgreement } from "../../../../contracts/libraries/IndexingAgreement.sol"; +import { IndexingAgreementDecoder } from "../../../../contracts/libraries/IndexingAgreementDecoder.sol"; import { SubgraphServiceIndexingAgreementSharedTest } from "./shared.t.sol"; @@ -175,6 +176,21 @@ contract SubgraphServiceIndexingAgreementCollectTest is SubgraphServiceIndexingA ); } + function test_SubgraphService_CollectIndexingFees_Revert_WhenInvalidData(Seed memory seed) public { + Context storage ctx = _newCtx(seed); + IndexerState memory indexerState = _withIndexer(ctx); + + bytes memory invalidData = bytes("invalid data"); + bytes memory expectedErr = abi.encodeWithSelector( + IndexingAgreementDecoder.IndexingAgreementDecoderInvalidData.selector, + "decodeCollectData", + invalidData + ); + vm.expectRevert(expectedErr); + resetPrank(indexerState.addr); + subgraphService.collect(indexerState.addr, IGraphPayments.PaymentTypes.IndexingFee, invalidData); + } + function test_SubgraphService_CollectIndexingFees_Revert_WhenInvalidAgreement( Seed memory seed, bytes16 agreementId, @@ -195,6 +211,27 @@ contract SubgraphServiceIndexingAgreementCollectTest is SubgraphServiceIndexingA ); } + function test_SubgraphService_CollectIndexingFees_Reverts_WhenInvalidNestedData(Seed memory seed) public { + Context storage ctx = _newCtx(seed); + IndexerState memory indexerState = _withIndexer(ctx); + IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, indexerState); + + resetPrank(indexerState.addr); + + bytes memory invalidNestedData = bytes("invalid nested data"); + bytes memory expectedErr = abi.encodeWithSelector( + IndexingAgreementDecoder.IndexingAgreementDecoderInvalidData.selector, + "decodeCollectIndexingFeeDataV1", + invalidNestedData + ); + vm.expectRevert(expectedErr); + subgraphService.collect( + indexerState.addr, + IGraphPayments.PaymentTypes.IndexingFee, + _encodeCollectData(accepted.rca.agreementId, invalidNestedData) + ); + } + function test_SubgraphService_CollectIndexingFees_Reverts_WhenStopService( Seed memory seed, uint256 entities, diff --git a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/integration.t.sol b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/integration.t.sol index 433ee0103..5c8758370 100644 --- a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/integration.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/integration.t.sol @@ -18,6 +18,13 @@ contract SubgraphServiceIndexingAgreementIntegrationTest is SubgraphServiceIndex uint256 indexerTokensLocked; } + struct ExpectedTokens { + uint256 expectedTotalTokensCollected; + uint256 expectedTokensLocked; + uint256 expectedProtocolTokensBurnt; + uint256 expectedIndexerTokensCollected; + } + /* * TESTS */ @@ -27,81 +34,164 @@ contract SubgraphServiceIndexingAgreementIntegrationTest is SubgraphServiceIndex Seed memory seed, uint256 fuzzyTokensCollected ) public { - uint256 expectedTotalTokensCollected = bound(fuzzyTokensCollected, 1000, 1_000_000); - uint256 expectedTokensLocked = stakeToFeesRatio * expectedTotalTokensCollected; - uint256 expectedProtocolTokensBurnt = expectedTotalTokensCollected.mulPPMRoundUp( - graphPayments.PROTOCOL_PAYMENT_CUT() - ); - uint256 expectedIndexerTokensCollected = expectedTotalTokensCollected - expectedProtocolTokensBurnt; - + // Setup + ExpectedTokens memory expectedTokens = _newExpectedTokens(fuzzyTokensCollected); Context storage ctx = _newCtx(seed); IndexerState memory indexerState = _withIndexer(ctx); - _addTokensToProvision(indexerState, expectedTokensLocked); + _addTokensToProvision(indexerState, expectedTokens.expectedTokensLocked); IRecurringCollector.RecurringCollectionAgreement memory rca = _recurringCollectorHelper.sensibleRCA( ctx.ctxInternal.seed.rca ); - uint256 agreementTokensPerSecond = 1; - rca.deadline = uint64(block.timestamp); // accept now - rca.endsAt = type(uint64).max; // no expiration - rca.maxInitialTokens = 0; // no initial payment - rca.maxOngoingTokensPerSecond = type(uint32).max; // unlimited tokens per second - rca.minSecondsPerCollection = 1; // 1 second between collections - rca.maxSecondsPerCollection = type(uint32).max; // no maximum time between collections - rca.serviceProvider = indexerState.addr; // service provider is the indexer - rca.dataService = address(subgraphService); // data service is the subgraph service - rca.metadata = _encodeAcceptIndexingAgreementMetadataV1( - indexerState.subgraphDeploymentId, - IndexingAgreement.IndexingAgreementTermsV1({ - tokensPerSecond: agreementTokensPerSecond, - tokensPerEntityPerSecond: 0 // no payment for entities - }) - ); + _sharedSetup(ctx, rca, indexerState, expectedTokens); - _setupPayerWithEscrow(rca.payer, ctx.payer.signerPrivateKey, indexerState.addr, expectedTotalTokensCollected); + TestState memory beforeCollect = _getState(rca.payer, indexerState.addr); + // Collect resetPrank(indexerState.addr); - // Set the payments destination to the indexer address - subgraphService.setPaymentsDestination(indexerState.addr); - // Accept the Indexing Agreement - subgraphService.acceptIndexingAgreement( - indexerState.allocationId, - _recurringCollectorHelper.generateSignedRCA(rca, ctx.payer.signerPrivateKey) + uint256 tokensCollected = subgraphService.collect( + indexerState.addr, + IGraphPayments.PaymentTypes.IndexingFee, + _encodeCollectDataV1( + rca.agreementId, + 1, + keccak256(abi.encodePacked("poi")), + epochManager.currentEpochBlock(), + bytes("") + ) ); - // Skip ahead to collection point - skip(expectedTotalTokensCollected / agreementTokensPerSecond); - // vm.assume(block.timestamp < type(uint64).max); + + TestState memory afterCollect = _getState(rca.payer, indexerState.addr); + _sharedAssert(beforeCollect, afterCollect, expectedTokens, tokensCollected); + } + + function test_SubgraphService_CollectIndexingFee_WhenCanceledByPayer_Integration( + Seed memory seed, + uint256 fuzzyTokensCollected + ) public { + // Setup + ExpectedTokens memory expectedTokens = _newExpectedTokens(fuzzyTokensCollected); + Context storage ctx = _newCtx(seed); + IndexerState memory indexerState = _withIndexer(ctx); + IRecurringCollector.RecurringCollectionAgreement memory rca = _recurringCollectorHelper.sensibleRCA( + ctx.ctxInternal.seed.rca + ); + _sharedSetup(ctx, rca, indexerState, expectedTokens); + + // Cancel the indexing agreement by the payer + resetPrank(ctx.payer.signer); + subgraphService.cancelIndexingAgreementByPayer(rca.agreementId); + TestState memory beforeCollect = _getState(rca.payer, indexerState.addr); - bytes16 agreementId = rca.agreementId; + + // Collect + resetPrank(indexerState.addr); uint256 tokensCollected = subgraphService.collect( indexerState.addr, IGraphPayments.PaymentTypes.IndexingFee, _encodeCollectDataV1( - agreementId, + rca.agreementId, 1, keccak256(abi.encodePacked("poi")), epochManager.currentEpochBlock(), bytes("") ) ); + TestState memory afterCollect = _getState(rca.payer, indexerState.addr); - uint256 indexerTokensCollected = afterCollect.indexerBalance - beforeCollect.indexerBalance; - uint256 protocolTokensBurnt = tokensCollected - indexerTokensCollected; + _sharedAssert(beforeCollect, afterCollect, expectedTokens, tokensCollected); + } + + /* solhint-enable graph/func-name-mixedcase */ + + function _sharedSetup( + Context storage _ctx, + IRecurringCollector.RecurringCollectionAgreement memory _rca, + IndexerState memory _indexerState, + ExpectedTokens memory _expectedTokens + ) internal { + _addTokensToProvision(_indexerState, _expectedTokens.expectedTokensLocked); + + IndexingAgreement.IndexingAgreementTermsV1 memory terms = IndexingAgreement.IndexingAgreementTermsV1({ + tokensPerSecond: 1, + tokensPerEntityPerSecond: 0 // no payment for entities + }); + _rca.deadline = uint64(block.timestamp); // accept now + _rca.endsAt = type(uint64).max; // no expiration + _rca.maxInitialTokens = 0; // no initial payment + _rca.maxOngoingTokensPerSecond = type(uint32).max; // unlimited tokens per second + _rca.minSecondsPerCollection = 1; // 1 second between collections + _rca.maxSecondsPerCollection = type(uint32).max; // no maximum time between collections + _rca.serviceProvider = _indexerState.addr; // service provider is the indexer + _rca.dataService = address(subgraphService); // data service is the subgraph service + _rca.metadata = _encodeAcceptIndexingAgreementMetadataV1(_indexerState.subgraphDeploymentId, terms); + + _setupPayerWithEscrow( + _rca.payer, + _ctx.payer.signerPrivateKey, + _indexerState.addr, + _expectedTokens.expectedTotalTokensCollected + ); + + resetPrank(_indexerState.addr); + // Set the payments destination to the indexer address + subgraphService.setPaymentsDestination(_indexerState.addr); + + // Accept the Indexing Agreement + subgraphService.acceptIndexingAgreement( + _indexerState.allocationId, + _recurringCollectorHelper.generateSignedRCA(_rca, _ctx.payer.signerPrivateKey) + ); + + // Skip ahead to collection point + skip(_expectedTokens.expectedTotalTokensCollected / terms.tokensPerSecond); + } + + function _newExpectedTokens(uint256 _fuzzyTokensCollected) internal view returns (ExpectedTokens memory) { + uint256 expectedTotalTokensCollected = bound(_fuzzyTokensCollected, 1000, 1_000_000); + uint256 expectedTokensLocked = stakeToFeesRatio * expectedTotalTokensCollected; + uint256 expectedProtocolTokensBurnt = expectedTotalTokensCollected.mulPPMRoundUp( + graphPayments.PROTOCOL_PAYMENT_CUT() + ); + uint256 expectedIndexerTokensCollected = expectedTotalTokensCollected - expectedProtocolTokensBurnt; + return + ExpectedTokens({ + expectedTotalTokensCollected: expectedTotalTokensCollected, + expectedTokensLocked: expectedTokensLocked, + expectedProtocolTokensBurnt: expectedProtocolTokensBurnt, + expectedIndexerTokensCollected: expectedIndexerTokensCollected + }); + } + + function _sharedAssert( + TestState memory _beforeCollect, + TestState memory _afterCollect, + ExpectedTokens memory _expectedTokens, + uint256 _tokensCollected + ) internal pure { + uint256 indexerTokensCollected = _afterCollect.indexerBalance - _beforeCollect.indexerBalance; + assertEq(_expectedTokens.expectedTotalTokensCollected, _tokensCollected, "Total tokens collected should match"); assertEq( - afterCollect.escrowBalance, - beforeCollect.escrowBalance - tokensCollected, - "Escrow balance should be reduced by the amount collected" + _expectedTokens.expectedProtocolTokensBurnt, + _tokensCollected - indexerTokensCollected, + "Protocol tokens burnt should match" ); - assertEq(tokensCollected, expectedTotalTokensCollected, "Total tokens collected should match"); - assertEq(expectedProtocolTokensBurnt, protocolTokensBurnt, "Protocol tokens burnt should match"); - assertEq(indexerTokensCollected, expectedIndexerTokensCollected, "Indexer tokens collected should match"); assertEq( - afterCollect.indexerTokensLocked, - beforeCollect.indexerTokensLocked + expectedTokensLocked, - "Locked tokens should match" + _expectedTokens.expectedIndexerTokensCollected, + indexerTokensCollected, + "Indexer tokens collected should match" + ); + assertEq( + _afterCollect.escrowBalance, + _beforeCollect.escrowBalance - _expectedTokens.expectedTotalTokensCollected, + "_Escrow balance should be reduced by the amount collected" ); - } - /* solhint-enable graph/func-name-mixedcase */ + assertEq( + _afterCollect.indexerTokensLocked, + _beforeCollect.indexerTokensLocked + _expectedTokens.expectedTokensLocked, + "_Locked tokens should match" + ); + } function _addTokensToProvision(IndexerState memory _indexerState, uint256 _tokensToAddToProvision) private { deal({ token: address(token), to: _indexerState.addr, give: _tokensToAddToProvision }); diff --git a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/shared.t.sol b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/shared.t.sol index 8574e60e7..2a5b2385a 100644 --- a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/shared.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/shared.t.sol @@ -310,14 +310,25 @@ contract SubgraphServiceIndexingAgreementSharedTest is SubgraphServiceTest, Boun function _newAcceptIndexingAgreementMetadataV1( bytes32 _subgraphDeploymentId + ) internal pure returns (IndexingAgreement.AcceptIndexingAgreementMetadata memory) { + return + _newAcceptIndexingAgreementMetadataV1Terms( + _subgraphDeploymentId, + abi.encode( + IndexingAgreement.IndexingAgreementTermsV1({ tokensPerSecond: 0, tokensPerEntityPerSecond: 0 }) + ) + ); + } + + function _newAcceptIndexingAgreementMetadataV1Terms( + bytes32 _subgraphDeploymentId, + bytes memory _terms ) internal pure returns (IndexingAgreement.AcceptIndexingAgreementMetadata memory) { return IndexingAgreement.AcceptIndexingAgreementMetadata({ subgraphDeploymentId: _subgraphDeploymentId, version: IndexingAgreement.IndexingAgreementVersion.V1, - terms: abi.encode( - IndexingAgreement.IndexingAgreementTermsV1({ tokensPerSecond: 0, tokensPerEntityPerSecond: 0 }) - ) + terms: _terms }); } @@ -343,18 +354,28 @@ contract SubgraphServiceIndexingAgreementSharedTest is SubgraphServiceTest, Boun bytes32 _poi, uint256 _poiBlock, bytes memory _metadata + ) internal pure returns (bytes memory) { + return _encodeCollectData(_agreementId, _encodeV1Data(_entities, _poi, _poiBlock, _metadata)); + } + + function _encodeCollectData(bytes16 _agreementId, bytes memory _nestedData) internal pure returns (bytes memory) { + return abi.encode(_agreementId, _nestedData); + } + + function _encodeV1Data( + uint256 _entities, + bytes32 _poi, + uint256 _poiBlock, + bytes memory _metadata ) internal pure returns (bytes memory) { return abi.encode( - _agreementId, - abi.encode( - IndexingAgreement.CollectIndexingFeeDataV1({ - entities: _entities, - poi: _poi, - poiBlockNumber: _poiBlock, - metadata: _metadata - }) - ) + IndexingAgreement.CollectIndexingFeeDataV1({ + entities: _entities, + poi: _poi, + poiBlockNumber: _poiBlock, + metadata: _metadata + }) ); } @@ -377,4 +398,18 @@ contract SubgraphServiceIndexingAgreementSharedTest is SubgraphServiceTest, Boun ) internal pure returns (bytes memory) { return abi.encode(_t); } + + function _assertEqualAgreement( + IRecurringCollector.RecurringCollectionAgreement memory _expected, + IndexingAgreement.AgreementWrapper memory _actual + ) internal pure { + assertEq(_expected.dataService, _actual.collectorAgreement.dataService); + assertEq(_expected.payer, _actual.collectorAgreement.payer); + assertEq(_expected.serviceProvider, _actual.collectorAgreement.serviceProvider); + assertEq(_expected.endsAt, _actual.collectorAgreement.endsAt); + assertEq(_expected.maxInitialTokens, _actual.collectorAgreement.maxInitialTokens); + assertEq(_expected.maxOngoingTokensPerSecond, _actual.collectorAgreement.maxOngoingTokensPerSecond); + assertEq(_expected.minSecondsPerCollection, _actual.collectorAgreement.minSecondsPerCollection); + assertEq(_expected.maxSecondsPerCollection, _actual.collectorAgreement.maxSecondsPerCollection); + } }