From d80a02523cb64e5b1799897cd3cf156ac069195f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Mon, 19 Aug 2024 15:10:24 -0300 Subject: [PATCH 1/2] fix: ensure maxVerifierCut is a valid PPM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .../data-service/utilities/ProvisionManager.sol | 4 +++- .../utilities/ProvisionManagerStorage.sol | 4 ++-- packages/horizon/test/data-service/DataService.t.sol | 11 ++++++++++- .../test/data-service/DataServiceUpgradeable.t.sol | 4 +++- .../extensions/DataServicePausableUpgradeable.t.sol | 4 +++- .../subgraph-service/contracts/SubgraphService.sol | 4 ++-- 6 files changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol b/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol index de3580aea..c89e9e473 100644 --- a/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol +++ b/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.26; import { IHorizonStaking } from "../../interfaces/IHorizonStaking.sol"; import { UintRange } from "../../libraries/UintRange.sol"; +import { PPMMath } from "../../libraries/PPMMath.sol"; import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import { GraphDirectory } from "../../utilities/GraphDirectory.sol"; @@ -150,7 +151,7 @@ abstract contract ProvisionManager is Initializable, GraphDirectory, ProvisionMa // solhint-disable-next-line func-name-mixedcase function __ProvisionManager_init_unchained() internal onlyInitializing { _setProvisionTokensRange(type(uint256).min, type(uint256).max); - _setVerifierCutRange(type(uint32).min, type(uint32).max); + _setVerifierCutRange(type(uint32).min, uint32(PPMMath.MAX_PPM)); _setThawingPeriodRange(type(uint64).min, type(uint64).max); } @@ -197,6 +198,7 @@ abstract contract ProvisionManager is Initializable, GraphDirectory, ProvisionMa */ function _setVerifierCutRange(uint32 _min, uint32 _max) internal { require(_min <= _max, ProvisionManagerInvalidRange(_min, _max)); + require(_max <= PPMMath.MAX_PPM, ProvisionManagerInvalidRange(_min, _max)); minimumVerifierCut = _min; maximumVerifierCut = _max; emit VerifierCutRangeSet(_min, _max); diff --git a/packages/horizon/contracts/data-service/utilities/ProvisionManagerStorage.sol b/packages/horizon/contracts/data-service/utilities/ProvisionManagerStorage.sol index 70f10d6e3..8649055db 100644 --- a/packages/horizon/contracts/data-service/utilities/ProvisionManagerStorage.sol +++ b/packages/horizon/contracts/data-service/utilities/ProvisionManagerStorage.sol @@ -17,10 +17,10 @@ abstract contract ProvisionManagerV1Storage { /// @notice The maximum thawing period allowed to register a provision in the data service uint64 public maximumThawingPeriod; - /// @notice The minimum verifier cut required to register a provision in the data service + /// @notice The minimum verifier cut required to register a provision in the data service (in PPM) uint32 public minimumVerifierCut; - /// @notice The maximum verifier cut allowed to register a provision in the data service + /// @notice The maximum verifier cut allowed to register a provision in the data service (in PPM) uint32 public maximumVerifierCut; /// @notice How much delegation the service provider can effectively use diff --git a/packages/horizon/test/data-service/DataService.t.sol b/packages/horizon/test/data-service/DataService.t.sol index ea5fd51a7..98f7aadb3 100644 --- a/packages/horizon/test/data-service/DataService.t.sol +++ b/packages/horizon/test/data-service/DataService.t.sol @@ -22,7 +22,7 @@ contract DataServiceTest is HorizonStakingSharedTest { function test_Constructor_WhenTheContractIsDeployedWithAValidController() external view { _assert_delegationRatio(type(uint32).min); _assert_provisionTokens_range(type(uint256).min, type(uint256).max); - _assert_verifierCut_range(type(uint32).min, type(uint32).max); + _assert_verifierCut_range(type(uint32).min, uint32(PPMMath.MAX_PPM)); _assert_thawingPeriod_range(type(uint64).min, type(uint64).max); } @@ -106,6 +106,7 @@ contract DataServiceTest is HorizonStakingSharedTest { function test_VerifierCut_WhenSettingAValidRange(uint32 min, uint32 max) external { vm.assume(min <= max); + vm.assume(max <= uint32(PPMMath.MAX_PPM)); _assert_set_verifierCut_range(min, max); } @@ -116,6 +117,14 @@ contract DataServiceTest is HorizonStakingSharedTest { dataService.setVerifierCutRange(min, max); } + function test_VerifierCut_RevertWhen_SettingAnInvalidMax(uint32 min, uint32 max) external { + vm.assume(max > uint32(PPMMath.MAX_PPM)); + vm.assume(min <= max); + + vm.expectRevert(abi.encodeWithSelector(ProvisionManager.ProvisionManagerInvalidRange.selector, min, max)); + dataService.setVerifierCutRange(min, max); + } + function test_VerifierCut_WhenGettingTheRange() external { dataService.setVerifierCutRange(dataService.VERIFIER_CUT_MIN(), dataService.VERIFIER_CUT_MAX()); _assert_verifierCut_range(dataService.VERIFIER_CUT_MIN(), dataService.VERIFIER_CUT_MAX()); diff --git a/packages/horizon/test/data-service/DataServiceUpgradeable.t.sol b/packages/horizon/test/data-service/DataServiceUpgradeable.t.sol index 7ff6c8246..c8721260f 100644 --- a/packages/horizon/test/data-service/DataServiceUpgradeable.t.sol +++ b/packages/horizon/test/data-service/DataServiceUpgradeable.t.sol @@ -5,6 +5,8 @@ import { GraphBaseTest } from "../GraphBase.t.sol"; import { DataServiceBaseUpgradeable } from "./implementations/DataServiceBaseUpgradeable.sol"; import { UnsafeUpgrades } from "openzeppelin-foundry-upgrades/Upgrades.sol"; +import { PPMMath } from "./../../contracts/libraries/PPMMath.sol"; + contract DataServiceUpgradeableTest is GraphBaseTest { function test_WhenTheContractIsDeployed() external { (DataServiceBaseUpgradeable dataService, DataServiceBaseUpgradeable implementation) = _deployDataService(); @@ -20,7 +22,7 @@ contract DataServiceUpgradeableTest is GraphBaseTest { (uint32 minVerifierCut, uint32 maxVerifierCut) = dataService.getVerifierCutRange(); assertEq(minVerifierCut, type(uint32).min); - assertEq(maxVerifierCut, type(uint32).max); + assertEq(maxVerifierCut, uint32(PPMMath.MAX_PPM)); (uint64 minThawingPeriod, uint64 maxThawingPeriod) = dataService.getThawingPeriodRange(); assertEq(minThawingPeriod, type(uint64).min); diff --git a/packages/horizon/test/data-service/extensions/DataServicePausableUpgradeable.t.sol b/packages/horizon/test/data-service/extensions/DataServicePausableUpgradeable.t.sol index eef727465..9f4a8822b 100644 --- a/packages/horizon/test/data-service/extensions/DataServicePausableUpgradeable.t.sol +++ b/packages/horizon/test/data-service/extensions/DataServicePausableUpgradeable.t.sol @@ -5,6 +5,8 @@ import { GraphBaseTest } from "../../GraphBase.t.sol"; import { DataServiceImpPausableUpgradeable } from "../implementations/DataServiceImpPausableUpgradeable.sol"; import { UnsafeUpgrades } from "openzeppelin-foundry-upgrades/Upgrades.sol"; +import { PPMMath } from "./../../../contracts/libraries/PPMMath.sol"; + contract DataServicePausableUpgradeableTest is GraphBaseTest { function test_WhenTheContractIsDeployed() external { ( @@ -23,7 +25,7 @@ contract DataServicePausableUpgradeableTest is GraphBaseTest { (uint32 minVerifierCut, uint32 maxVerifierCut) = dataService.getVerifierCutRange(); assertEq(minVerifierCut, type(uint32).min); - assertEq(maxVerifierCut, type(uint32).max); + assertEq(maxVerifierCut, uint32(PPMMath.MAX_PPM)); (uint64 minThawingPeriod, uint64 maxThawingPeriod) = dataService.getThawingPeriodRange(); assertEq(minThawingPeriod, type(uint64).min); diff --git a/packages/subgraph-service/contracts/SubgraphService.sol b/packages/subgraph-service/contracts/SubgraphService.sol index a3b9d1541..27fdefd2f 100644 --- a/packages/subgraph-service/contracts/SubgraphService.sol +++ b/packages/subgraph-service/contracts/SubgraphService.sol @@ -464,11 +464,11 @@ contract SubgraphService is /** * @notice Getter for the accepted verifier cut range for provisions * @return min The minimum verifier cut which is defined by {DisputeManager-getVerifierCut} - * @return max The maximum is unbounded + * @return max The maximum is 100% in PPM */ function _getVerifierCutRange() internal view override returns (uint32 min, uint32 max) { uint32 verifierCut = _disputeManager().getVerifierCut(); - return (verifierCut, type(uint32).max); + return (verifierCut, uint32(PPMMath.MAX_PPM)); } /** From d7814eaeb98fdf25551a7957cf2e0a35fea954c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Mon, 19 Aug 2024 15:24:20 -0300 Subject: [PATCH 2/2] fix: use isvalidPPM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .../contracts/data-service/utilities/ProvisionManager.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol b/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol index c89e9e473..ad18bee93 100644 --- a/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol +++ b/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol @@ -198,7 +198,7 @@ abstract contract ProvisionManager is Initializable, GraphDirectory, ProvisionMa */ function _setVerifierCutRange(uint32 _min, uint32 _max) internal { require(_min <= _max, ProvisionManagerInvalidRange(_min, _max)); - require(_max <= PPMMath.MAX_PPM, ProvisionManagerInvalidRange(_min, _max)); + require(PPMMath.isValidPPM(_max), ProvisionManagerInvalidRange(_min, _max)); minimumVerifierCut = _min; maximumVerifierCut = _max; emit VerifierCutRangeSet(_min, _max);