[audit-01] fix: [TRST-H-1] IndexingAgreement.collect() on CanceledByPayer

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);
 }

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;
+    }
 }

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",

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 */
+}

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) {}
+}

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 */
+}

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;
+    }
+}

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)) {}
+}

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;
+    }
+}

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);
     }
 }

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 */
+}

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);