diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index 1e4f95a08..f1fb05b35 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -700,10 +700,15 @@ interface IHorizonStakingMain { /** * @notice Stages a provision parameter update. Note that the change is not effective until the verifier calls - * {acceptProvisionParameters}. + * {acceptProvisionParameters}. Calling this function is a no-op if the new parameters are the same as the current + * ones. * @dev This two step update process prevents the service provider from changing the parameters * without the verifier's consent. * + * Requirements: + * - `thawingPeriod` must be less than or equal to `_maxThawingPeriod`. Note that if `_maxThawingPeriod` changes the + * function will not revert if called with the same thawing period as the current one. + * * Emits a {ProvisionParametersStaged} event if at least one of the parameters changed. * * @param serviceProvider The service provider address diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 549360dce..1785348cc 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -219,19 +219,26 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint32 newMaxVerifierCut, uint64 newThawingPeriod ) external override notPaused onlyAuthorized(serviceProvider, verifier) { - require(PPMMath.isValidPPM(newMaxVerifierCut), HorizonStakingInvalidMaxVerifierCut(newMaxVerifierCut)); - require( - newThawingPeriod <= _maxThawingPeriod, - HorizonStakingInvalidThawingPeriod(newThawingPeriod, _maxThawingPeriod) - ); - // Provision must exist Provision storage prov = _provisions[serviceProvider][verifier]; require(prov.createdAt != 0, HorizonStakingInvalidProvision(serviceProvider, verifier)); - if ((prov.maxVerifierCutPending != newMaxVerifierCut) || (prov.thawingPeriodPending != newThawingPeriod)) { - prov.maxVerifierCutPending = newMaxVerifierCut; - prov.thawingPeriodPending = newThawingPeriod; + bool verifierCutChanged = prov.maxVerifierCutPending != newMaxVerifierCut; + bool thawingPeriodChanged = prov.thawingPeriodPending != newThawingPeriod; + + if (verifierCutChanged || thawingPeriodChanged) { + if (verifierCutChanged) { + require(PPMMath.isValidPPM(newMaxVerifierCut), HorizonStakingInvalidMaxVerifierCut(newMaxVerifierCut)); + prov.maxVerifierCutPending = newMaxVerifierCut; + } + if (thawingPeriodChanged) { + require( + newThawingPeriod <= _maxThawingPeriod, + HorizonStakingInvalidThawingPeriod(newThawingPeriod, _maxThawingPeriod) + ); + prov.thawingPeriodPending = newThawingPeriod; + } + prov.lastParametersStagedAt = block.timestamp; emit ProvisionParametersStaged(serviceProvider, verifier, newMaxVerifierCut, newThawingPeriod); } diff --git a/packages/horizon/test/staking/provision/parameters.t.sol b/packages/horizon/test/staking/provision/parameters.t.sol index 6e0b4fabf..ff08edf79 100644 --- a/packages/horizon/test/staking/provision/parameters.t.sol +++ b/packages/horizon/test/staking/provision/parameters.t.sol @@ -70,6 +70,41 @@ contract HorizonStakingProvisionParametersTest is HorizonStakingTest { staking.setProvisionParameters(users.indexer, subgraphDataServiceAddress, maxVerifierCut, thawingPeriod); } + function test_ProvisionParametersSet_MaxMaxThawingPeriodChanged( + uint256 amount, + uint32 maxVerifierCut, + uint64 thawingPeriod + ) public useIndexer { + vm.assume(amount > 0); + vm.assume(amount <= MAX_STAKING_TOKENS); + vm.assume(maxVerifierCut <= MAX_PPM); + + // create provision with initial parameters + uint32 initialMaxVerifierCut = 1000; + uint64 initialThawingPeriod = 14 days; // Max thawing period is 28 days + _createProvision(users.indexer, subgraphDataServiceAddress, amount, initialMaxVerifierCut, initialThawingPeriod); + + // change the max thawing period allowed so that the initial thawing period is not valid anymore + uint64 newMaxThawingPeriod = 7 days; + resetPrank(users.governor); + _setMaxThawingPeriod(newMaxThawingPeriod); + + // set the verifier cut to a new value - keep the thawing period the same, it should be allowed + resetPrank(users.indexer); + _setProvisionParameters(users.indexer, subgraphDataServiceAddress, maxVerifierCut, initialThawingPeriod); + + // now try to change the thawing period to a new value that is invalid + vm.assume(thawingPeriod > initialThawingPeriod); + vm.expectRevert( + abi.encodeWithSelector( + IHorizonStakingMain.HorizonStakingInvalidThawingPeriod.selector, + thawingPeriod, + newMaxThawingPeriod + ) + ); + staking.setProvisionParameters(users.indexer, subgraphDataServiceAddress, maxVerifierCut, thawingPeriod); + } + function test_ProvisionParametersAccept( uint256 amount, uint32 maxVerifierCut,