diff --git a/packages/horizon/contracts/data-service/DataService.sol b/packages/horizon/contracts/data-service/DataService.sol index 8a06ad4ea..7923f1903 100644 --- a/packages/horizon/contracts/data-service/DataService.sol +++ b/packages/horizon/contracts/data-service/DataService.sol @@ -25,6 +25,8 @@ import { ProvisionManager } from "./utilities/ProvisionManager.sol"; * - If the data service implementation is NOT upgradeable, it must initialize the contract by calling * {__DataService_init} or {__DataService_init_unchained} in the constructor. Note that the `initializer` * will be required in the constructor. + * - Note that in both cases if using {__DataService_init_unchained} variant the corresponding parent + * initializers must be called in the implementation. */ abstract contract DataService is GraphDirectory, ProvisionManager, DataServiceV1Storage, IDataService { /** diff --git a/packages/horizon/contracts/data-service/DataServiceStorage.sol b/packages/horizon/contracts/data-service/DataServiceStorage.sol index a0271443c..3d8e84f82 100644 --- a/packages/horizon/contracts/data-service/DataServiceStorage.sol +++ b/packages/horizon/contracts/data-service/DataServiceStorage.sol @@ -3,5 +3,6 @@ pragma solidity 0.8.27; abstract contract DataServiceV1Storage { /// @dev Gap to allow adding variables in future upgrades + /// Note that this contract is not upgradeable but might be inherited by an upgradeable contract uint256[50] private __gap; } diff --git a/packages/horizon/contracts/data-service/extensions/DataServiceFees.sol b/packages/horizon/contracts/data-service/extensions/DataServiceFees.sol index 09102dd4a..6685dc5a7 100644 --- a/packages/horizon/contracts/data-service/extensions/DataServiceFees.sol +++ b/packages/horizon/contracts/data-service/extensions/DataServiceFees.sol @@ -14,6 +14,8 @@ import { DataServiceFeesV1Storage } from "./DataServiceFeesStorage.sol"; * @dev Implementation of the {IDataServiceFees} interface. * @notice Extension for the {IDataService} contract to handle payment collateralization * using a Horizon provision. See {IDataServiceFees} for more details. + * @dev This contract inherits from {DataService} which needs to be initialized, please see + * {DataService} for detailed instructions. */ abstract contract DataServiceFees is DataService, DataServiceFeesV1Storage, IDataServiceFees { using ProvisionTracker for mapping(address => uint256); @@ -127,9 +129,7 @@ abstract contract DataServiceFees is DataService, DataServiceFeesV1Storage, IDat * @param _claimId The ID of the stake claim */ function _getNextStakeClaim(bytes32 _claimId) private view returns (bytes32) { - StakeClaim memory claim = claims[_claimId]; - require(claim.createdAt != 0, DataServiceFeesClaimNotFound(_claimId)); - return claim.nextClaim; + return claims[_claimId].nextClaim; } /** diff --git a/packages/horizon/contracts/data-service/extensions/DataServiceFeesStorage.sol b/packages/horizon/contracts/data-service/extensions/DataServiceFeesStorage.sol index cb4f908dc..853b57209 100644 --- a/packages/horizon/contracts/data-service/extensions/DataServiceFeesStorage.sol +++ b/packages/horizon/contracts/data-service/extensions/DataServiceFeesStorage.sol @@ -18,5 +18,6 @@ abstract contract DataServiceFeesV1Storage { mapping(address serviceProvider => LinkedList.List list) public claimsLists; /// @dev Gap to allow adding variables in future upgrades + /// Note that this contract is not upgradeable but might be inherited by an upgradeable contract uint256[50] private __gap; } diff --git a/packages/horizon/contracts/data-service/extensions/DataServicePausable.sol b/packages/horizon/contracts/data-service/extensions/DataServicePausable.sol index 3c83fbb0e..475614454 100644 --- a/packages/horizon/contracts/data-service/extensions/DataServicePausable.sol +++ b/packages/horizon/contracts/data-service/extensions/DataServicePausable.sol @@ -14,6 +14,8 @@ import { DataService } from "../DataService.sol"; * pause guardians. * @dev Note that this extension does not provide an external function to set pause * guardians. This should be implemented in the derived contract. + * @dev This contract inherits from {DataService} which needs to be initialized, please see + * {DataService} for detailed instructions. */ abstract contract DataServicePausable is Pausable, DataService, IDataServicePausable { /// @notice List of pause guardians and their allowed status @@ -51,6 +53,10 @@ abstract contract DataServicePausable is Pausable, DataService, IDataServicePaus * @param _allowed The allowed status of the pause guardian */ function _setPauseGuardian(address _pauseGuardian, bool _allowed) internal { + require( + pauseGuardians[_pauseGuardian] == !_allowed, + DataServicePausablePauseGuardianNoChange(_pauseGuardian, _allowed) + ); pauseGuardians[_pauseGuardian] = _allowed; emit PauseGuardianSet(_pauseGuardian, _allowed); } diff --git a/packages/horizon/contracts/data-service/extensions/DataServicePausableUpgradeable.sol b/packages/horizon/contracts/data-service/extensions/DataServicePausableUpgradeable.sol index 82d7cc63b..2cecdedb6 100644 --- a/packages/horizon/contracts/data-service/extensions/DataServicePausableUpgradeable.sol +++ b/packages/horizon/contracts/data-service/extensions/DataServicePausableUpgradeable.sol @@ -10,11 +10,16 @@ import { DataService } from "../DataService.sol"; * @title DataServicePausableUpgradeable contract * @dev Implementation of the {IDataServicePausable} interface. * @dev Upgradeable version of the {DataServicePausable} contract. + * @dev This contract inherits from {DataService} which needs to be initialized, please see + * {DataService} for detailed instructions. */ abstract contract DataServicePausableUpgradeable is PausableUpgradeable, DataService, IDataServicePausable { /// @notice List of pause guardians and their allowed status mapping(address pauseGuardian => bool allowed) public pauseGuardians; + /// @dev Gap to allow adding variables in future upgrades + uint256[50] private __gap; + /** * @notice Checks if the caller is a pause guardian. */ @@ -61,7 +66,11 @@ abstract contract DataServicePausableUpgradeable is PausableUpgradeable, DataSer * @param _pauseGuardian The address of the pause guardian * @param _allowed The allowed status of the pause guardian */ - function _setPauseGuardian(address _pauseGuardian, bool _allowed) internal whenNotPaused { + function _setPauseGuardian(address _pauseGuardian, bool _allowed) internal { + require( + pauseGuardians[_pauseGuardian] == !_allowed, + DataServicePausablePauseGuardianNoChange(_pauseGuardian, _allowed) + ); pauseGuardians[_pauseGuardian] = _allowed; emit PauseGuardianSet(_pauseGuardian, _allowed); } diff --git a/packages/horizon/contracts/data-service/extensions/DataServiceRescuable.sol b/packages/horizon/contracts/data-service/extensions/DataServiceRescuable.sol index 0d2f0750d..0f57862d5 100644 --- a/packages/horizon/contracts/data-service/extensions/DataServiceRescuable.sol +++ b/packages/horizon/contracts/data-service/extensions/DataServiceRescuable.sol @@ -17,11 +17,17 @@ import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.s * that calls this contract's _rescueTokens. * @dev Note that this extension does not provide an external function to set * rescuers. This should be implemented in the derived contract. + * @dev This contract inherits from {DataService} which needs to be initialized, please see + * {DataService} for detailed instructions. */ abstract contract DataServiceRescuable is DataService, IDataServiceRescuable { /// @notice List of rescuers and their allowed status mapping(address rescuer => bool allowed) public rescuers; + /// @dev Gap to allow adding variables in future upgrades + /// Note that this contract is not upgradeable but might be inherited by an upgradeable contract + uint256[50] private __gap; + /** * @notice Checks if the caller is a rescuer. */ diff --git a/packages/horizon/contracts/data-service/interfaces/IDataServicePausable.sol b/packages/horizon/contracts/data-service/interfaces/IDataServicePausable.sol index bd27ca848..0579c6649 100644 --- a/packages/horizon/contracts/data-service/interfaces/IDataServicePausable.sol +++ b/packages/horizon/contracts/data-service/interfaces/IDataServicePausable.sol @@ -23,6 +23,13 @@ interface IDataServicePausable is IDataService { */ error DataServicePausableNotPauseGuardian(address account); + /** + * @notice Emitted when a pause guardian is set to the same allowed status + * @param account The address of the pause guardian + * @param allowed The allowed status of the pause guardian + */ + error DataServicePausablePauseGuardianNoChange(address account, bool allowed); + /** * @notice Pauses the data service. * @dev Note that only functions using the modifiers `whenNotPaused` diff --git a/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol b/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol index a3f96794c..68ac2813d 100644 --- a/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol +++ b/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol @@ -202,10 +202,16 @@ abstract contract ProvisionManager is Initializable, GraphDirectory, ProvisionMa /** * @notice Checks if the provision tokens of a service provider are within the valid range. + * Note that thawing tokens are not considered in this check. * @param _provision The provision to check. */ function _checkProvisionTokens(IHorizonStaking.Provision memory _provision) internal view virtual { - _checkValueInRange(_provision.tokens, minimumProvisionTokens, maximumProvisionTokens, "tokens"); + _checkValueInRange( + _provision.tokens - _provision.tokensThawing, + minimumProvisionTokens, + maximumProvisionTokens, + "tokens" + ); } /** diff --git a/packages/horizon/contracts/data-service/utilities/ProvisionManagerStorage.sol b/packages/horizon/contracts/data-service/utilities/ProvisionManagerStorage.sol index 0a6bed2be..0732afc9a 100644 --- a/packages/horizon/contracts/data-service/utilities/ProvisionManagerStorage.sol +++ b/packages/horizon/contracts/data-service/utilities/ProvisionManagerStorage.sol @@ -28,5 +28,6 @@ abstract contract ProvisionManagerV1Storage { uint32 public delegationRatio; /// @dev Gap to allow adding variables in future upgrades + /// Note that this contract is not upgradeable but might be inherited by an upgradeable contract uint256[50] private __gap; } diff --git a/packages/horizon/test/data-service/DataService.t.sol b/packages/horizon/test/data-service/DataService.t.sol index c535c6dea..d18e49ba9 100644 --- a/packages/horizon/test/data-service/DataService.t.sol +++ b/packages/horizon/test/data-service/DataService.t.sol @@ -65,6 +65,25 @@ contract DataServiceTest is HorizonStakingSharedTest { assertEq(max, dataServiceOverride.PROVISION_TOKENS_MAX()); } + function test_ProvisionTokens_WhenCheckingAValidProvision_WithThawing(uint256 tokens, uint256 tokensThaw) external useIndexer { + dataService.setProvisionTokensRange(dataService.PROVISION_TOKENS_MIN(), dataService.PROVISION_TOKENS_MAX()); + tokens = bound(tokens, dataService.PROVISION_TOKENS_MIN(), dataService.PROVISION_TOKENS_MAX()); + tokensThaw = bound(tokensThaw, tokens - dataService.PROVISION_TOKENS_MIN() + 1, tokens); + + _createProvision(users.indexer, address(dataService), tokens, 0, 0); + staking.thaw(users.indexer, address(dataService), tokensThaw); + vm.expectRevert( + abi.encodeWithSelector( + ProvisionManager.ProvisionManagerInvalidValue.selector, + "tokens", + tokens - tokensThaw, + dataService.PROVISION_TOKENS_MIN(), + dataService.PROVISION_TOKENS_MAX() + ) + ); + dataService.checkProvisionTokens(users.indexer); + } + function test_ProvisionTokens_WhenCheckingAValidProvision(uint256 tokens) external useIndexer { dataService.setProvisionTokensRange(dataService.PROVISION_TOKENS_MIN(), dataService.PROVISION_TOKENS_MAX()); tokens = bound(tokens, dataService.PROVISION_TOKENS_MIN(), dataService.PROVISION_TOKENS_MAX()); diff --git a/packages/horizon/test/data-service/extensions/DataServicePausable.t.sol b/packages/horizon/test/data-service/extensions/DataServicePausable.t.sol index 111838fe4..46cded2eb 100644 --- a/packages/horizon/test/data-service/extensions/DataServicePausable.t.sol +++ b/packages/horizon/test/data-service/extensions/DataServicePausable.t.sol @@ -70,6 +70,23 @@ contract DataServicePausableTest is HorizonStakingSharedTest { _assert_setPauseGuardian(address(this), false); } + function test_SetPauseGuardian_RevertWhen_AlreadyPauseGuardian() external { + _assert_setPauseGuardian(address(this), true); + vm.expectRevert( + abi.encodeWithSignature("DataServicePausablePauseGuardianNoChange(address,bool)", address(this), true) + ); + dataService.setPauseGuardian(address(this), true); + } + + function test_SetPauseGuardian_RevertWhen_AlreadyNotPauseGuardian() external { + _assert_setPauseGuardian(address(this), true); + _assert_setPauseGuardian(address(this), false); + vm.expectRevert( + abi.encodeWithSignature("DataServicePausablePauseGuardianNoChange(address,bool)", address(this), false) + ); + dataService.setPauseGuardian(address(this), false); + } + function test_PausedProtectedFn_RevertWhen_TheProtocolIsPaused() external { _assert_setPauseGuardian(address(this), true); _assert_pause();