diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index c0d18366e..210146bf4 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -341,9 +341,8 @@ interface IHorizonStakingMain { /** * @notice Thrown when attempting to create a provision with an invalid maximum verifier cut. * @param maxVerifierCut The maximum verifier cut - * @param maxMaxVerifierCut The maximum `maxVerifierCut` allowed */ - error HorizonStakingInvalidMaxVerifierCut(uint32 maxVerifierCut, uint32 maxMaxVerifierCut); + error HorizonStakingInvalidMaxVerifierCut(uint32 maxVerifierCut); /** * @notice Thrown when attempting to create a provision with an invalid thawing period. @@ -528,7 +527,7 @@ interface IHorizonStakingMain { * @dev Requirements: * - `tokens` cannot be zero. * - The `serviceProvider` must have enough idle stake to cover the tokens to provision. - * - `maxVerifierCut` must be less than or equal to `MAX_MAX_VERIFIER_CUT`. + * - `maxVerifierCut` must be a valid PPM. * - `thawingPeriod` must be less than or equal to `_maxThawingPeriod`. * * Emits a {ProvisionCreated} event. diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 325952ddd..a00cda8af 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -31,10 +31,6 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { using PPMMath for uint256; using LinkedList for LinkedList.List; - /// @dev Maximum value that can be set as the maxVerifierCut in a provision. - /// It is equivalent to 100% in parts-per-million - uint32 private constant MAX_MAX_VERIFIER_CUT = uint32(PPMMath.MAX_PPM); - /// @dev Fixed point precision uint256 private constant FIXED_POINT_PRECISION = 1e18; @@ -224,6 +220,11 @@ 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 storage prov = _provisions[serviceProvider][verifier]; require(prov.createdAt != 0, HorizonStakingInvalidProvision(serviceProvider, verifier)); @@ -626,10 +627,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint64 _thawingPeriod ) private { require(_tokens > 0, HorizonStakingInvalidZeroTokens()); - require( - _maxVerifierCut <= MAX_MAX_VERIFIER_CUT, - HorizonStakingInvalidMaxVerifierCut(_maxVerifierCut, MAX_MAX_VERIFIER_CUT) - ); + require(PPMMath.isValidPPM(_maxVerifierCut), HorizonStakingInvalidMaxVerifierCut(_maxVerifierCut)); require( _thawingPeriod <= _maxThawingPeriod, HorizonStakingInvalidThawingPeriod(_thawingPeriod, _maxThawingPeriod) diff --git a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol index c0026c5f6..f9796cf94 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol @@ -81,7 +81,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { // use assume instead of bound to avoid the bounding falling out of scope vm.assume(tokens > 0); vm.assume(tokens <= MAX_STAKING_TOKENS); - vm.assume(maxVerifierCut <= MAX_MAX_VERIFIER_CUT); + vm.assume(maxVerifierCut <= MAX_PPM); vm.assume(thawingPeriod <= MAX_THAWING_PERIOD); _createProvision(users.indexer, dataService, tokens, maxVerifierCut, thawingPeriod); diff --git a/packages/horizon/test/staking/provision/locked.t.sol b/packages/horizon/test/staking/provision/locked.t.sol index a8adfc774..a4d8eda92 100644 --- a/packages/horizon/test/staking/provision/locked.t.sol +++ b/packages/horizon/test/staking/provision/locked.t.sol @@ -24,7 +24,7 @@ contract HorizonStakingProvisionLockedTest is HorizonStakingTest { users.indexer, subgraphDataServiceAddress, amount, - MAX_MAX_VERIFIER_CUT, + MAX_PPM, MAX_THAWING_PERIOD ); @@ -52,7 +52,7 @@ contract HorizonStakingProvisionLockedTest is HorizonStakingTest { users.indexer, subgraphDataServiceAddress, amount, - MAX_MAX_VERIFIER_CUT, + MAX_PPM, MAX_THAWING_PERIOD ); } @@ -75,7 +75,7 @@ contract HorizonStakingProvisionLockedTest is HorizonStakingTest { users.indexer, subgraphDataServiceAddress, amount, - MAX_MAX_VERIFIER_CUT, + MAX_PPM, MAX_THAWING_PERIOD ); } diff --git a/packages/horizon/test/staking/provision/parameters.t.sol b/packages/horizon/test/staking/provision/parameters.t.sol index 8171f95b4..6eccca06e 100644 --- a/packages/horizon/test/staking/provision/parameters.t.sol +++ b/packages/horizon/test/staking/provision/parameters.t.sol @@ -7,6 +7,17 @@ import { HorizonStakingTest } from "../HorizonStaking.t.sol"; import { IHorizonStakingMain } from "../../../contracts/interfaces/internal/IHorizonStakingMain.sol"; contract HorizonStakingProvisionParametersTest is HorizonStakingTest { + + /* + * MODIFIERS + */ + + modifier useValidParameters(uint32 maxVerifierCut, uint64 thawingPeriod) { + vm.assume(maxVerifierCut <= MAX_PPM); + vm.assume(thawingPeriod <= MAX_THAWING_PERIOD); + _; + } + /* * TESTS */ @@ -15,14 +26,14 @@ contract HorizonStakingProvisionParametersTest is HorizonStakingTest { uint256 amount, uint32 maxVerifierCut, uint64 thawingPeriod - ) public useIndexer useProvision(amount, 0, 0) { + ) public useIndexer useProvision(amount, 0, 0) useValidParameters(maxVerifierCut, thawingPeriod) { _setProvisionParameters(users.indexer, subgraphDataServiceAddress, maxVerifierCut, thawingPeriod); } function test_ProvisionParametersSet_RevertWhen_ProvisionNotExists( uint32 maxVerifierCut, uint64 thawingPeriod - ) public useIndexer { + ) public useIndexer useValidParameters(maxVerifierCut, thawingPeriod) { vm.expectRevert( abi.encodeWithSignature( "HorizonStakingInvalidProvision(address,address)", @@ -71,4 +82,37 @@ contract HorizonStakingProvisionParametersTest is HorizonStakingTest { _acceptProvisionParameters(users.indexer); vm.stopPrank(); } + + function test_ProvisionParameters_RevertIf_InvalidMaxVerifierCut( + uint256 amount, + uint32 maxVerifierCut, + uint64 thawingPeriod + ) public useIndexer useProvision(amount, maxVerifierCut, thawingPeriod) { + maxVerifierCut = uint32(bound(maxVerifierCut, MAX_PPM + 1, type(uint32).max)); + vm.assume(thawingPeriod <= MAX_THAWING_PERIOD); + vm.expectRevert( + abi.encodeWithSelector( + IHorizonStakingMain.HorizonStakingInvalidMaxVerifierCut.selector, + maxVerifierCut + ) + ); + staking.setProvisionParameters(users.indexer, subgraphDataServiceAddress, maxVerifierCut, thawingPeriod); + } + + function test_ProvisionParameters_RevertIf_InvalidThawingPeriod( + uint256 amount, + uint32 maxVerifierCut, + uint64 thawingPeriod + ) public useIndexer useProvision(amount, maxVerifierCut, thawingPeriod) { + vm.assume(maxVerifierCut <= MAX_PPM); + thawingPeriod = uint64(bound(thawingPeriod, MAX_THAWING_PERIOD + 1, type(uint64).max)); + vm.expectRevert( + abi.encodeWithSelector( + IHorizonStakingMain.HorizonStakingInvalidThawingPeriod.selector, + thawingPeriod, + MAX_THAWING_PERIOD + ) + ); + staking.setProvisionParameters(users.indexer, subgraphDataServiceAddress, maxVerifierCut, thawingPeriod); + } } diff --git a/packages/horizon/test/staking/provision/provision.t.sol b/packages/horizon/test/staking/provision/provision.t.sol index 50dc12057..cabe2f25e 100644 --- a/packages/horizon/test/staking/provision/provision.t.sol +++ b/packages/horizon/test/staking/provision/provision.t.sol @@ -12,7 +12,7 @@ contract HorizonStakingProvisionTest is HorizonStakingTest { function testProvision_Create(uint256 tokens, uint32 maxVerifierCut, uint64 thawingPeriod) public useIndexer { tokens = bound(tokens, 1, MAX_STAKING_TOKENS); - maxVerifierCut = uint32(bound(maxVerifierCut, 0, MAX_MAX_VERIFIER_CUT)); + maxVerifierCut = uint32(bound(maxVerifierCut, 0, MAX_PPM)); thawingPeriod = uint32(bound(thawingPeriod, 0, MAX_THAWING_PERIOD)); _createProvision(users.indexer, subgraphDataServiceAddress, tokens, maxVerifierCut, thawingPeriod); @@ -28,11 +28,10 @@ contract HorizonStakingProvisionTest is HorizonStakingTest { uint256 amount, uint32 maxVerifierCut ) public useIndexer useStake(amount) { - vm.assume(maxVerifierCut > MAX_MAX_VERIFIER_CUT); + vm.assume(maxVerifierCut > MAX_PPM); bytes memory expectedError = abi.encodeWithSignature( - "HorizonStakingInvalidMaxVerifierCut(uint32,uint32)", - maxVerifierCut, - MAX_MAX_VERIFIER_CUT + "HorizonStakingInvalidMaxVerifierCut(uint32)", + maxVerifierCut ); vm.expectRevert(expectedError); staking.provision(users.indexer, subgraphDataServiceAddress, amount, maxVerifierCut, 0); diff --git a/packages/horizon/test/staking/serviceProvider/serviceProvider.t.sol b/packages/horizon/test/staking/serviceProvider/serviceProvider.t.sol index 63578791a..7db480869 100644 --- a/packages/horizon/test/staking/serviceProvider/serviceProvider.t.sol +++ b/packages/horizon/test/staking/serviceProvider/serviceProvider.t.sol @@ -84,7 +84,7 @@ contract HorizonStakingServiceProviderTest is HorizonStakingTest { uint256 amount, uint256 delegationAmount, uint32 delegationRatio - ) public useIndexer useProvision(amount, MAX_MAX_VERIFIER_CUT, MAX_THAWING_PERIOD) useDelegation(delegationAmount) { + ) public useIndexer useProvision(amount, MAX_PPM, MAX_THAWING_PERIOD) useDelegation(delegationAmount) { uint256 tokensAvailable = staking.getTokensAvailable(users.indexer, subgraphDataServiceAddress, delegationRatio); uint256 tokensDelegatedMax = amount * (uint256(delegationRatio)); @@ -95,7 +95,7 @@ contract HorizonStakingServiceProviderTest is HorizonStakingTest { function testServiceProvider_GetProviderTokensAvailable( uint256 amount, uint256 delegationAmount - ) public useIndexer useProvision(amount, MAX_MAX_VERIFIER_CUT, MAX_THAWING_PERIOD) useDelegation(delegationAmount) { + ) public useIndexer useProvision(amount, MAX_PPM, MAX_THAWING_PERIOD) useDelegation(delegationAmount) { uint256 providerTokensAvailable = staking.getProviderTokensAvailable(users.indexer, subgraphDataServiceAddress); // Should not include delegated tokens assertEq(providerTokensAvailable, amount); @@ -103,7 +103,7 @@ contract HorizonStakingServiceProviderTest is HorizonStakingTest { function testServiceProvider_HasStake( uint256 amount - ) public useIndexer useProvision(amount, MAX_MAX_VERIFIER_CUT, MAX_THAWING_PERIOD) { + ) public useIndexer useProvision(amount, MAX_PPM, MAX_THAWING_PERIOD) { assertTrue(staking.hasStake(users.indexer)); _thaw(users.indexer, subgraphDataServiceAddress, amount); @@ -116,7 +116,7 @@ contract HorizonStakingServiceProviderTest is HorizonStakingTest { function testServiceProvider_GetIndexerStakedTokens( uint256 amount - ) public useIndexer useProvision(amount, MAX_MAX_VERIFIER_CUT, MAX_THAWING_PERIOD) { + ) public useIndexer useProvision(amount, MAX_PPM, MAX_THAWING_PERIOD) { assertEq(staking.getIndexerStakedTokens(users.indexer), amount); _thaw(users.indexer, subgraphDataServiceAddress, amount); diff --git a/packages/horizon/test/staking/slash/slash.t.sol b/packages/horizon/test/staking/slash/slash.t.sol index aea8db986..66a645b7a 100644 --- a/packages/horizon/test/staking/slash/slash.t.sol +++ b/packages/horizon/test/staking/slash/slash.t.sol @@ -52,12 +52,11 @@ contract HorizonStakingSlashTest is HorizonStakingTest { uint256 slashTokens, uint256 verifierCutAmount, uint256 delegationTokens - ) public useIndexer useProvision(tokens, MAX_MAX_VERIFIER_CUT, 0) { + ) public useIndexer useProvision(tokens, MAX_PPM, 0) { vm.assume(slashTokens > tokens); delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); - verifierCutAmount = bound(verifierCutAmount, 0, MAX_MAX_VERIFIER_CUT); - uint256 maxVerifierTokens = (tokens * MAX_MAX_VERIFIER_CUT) / MAX_PPM; - vm.assume(verifierCutAmount <= maxVerifierTokens); + verifierCutAmount = bound(verifierCutAmount, 0, MAX_PPM); + vm.assume(verifierCutAmount <= tokens); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); @@ -71,12 +70,10 @@ contract HorizonStakingSlashTest is HorizonStakingTest { uint256 slashTokens, uint256 verifierCutAmount, uint256 delegationTokens - ) public useIndexer useProvision(tokens, MAX_MAX_VERIFIER_CUT, 0) useDelegationSlashing() { + ) public useIndexer useProvision(tokens, MAX_PPM, 0) useDelegationSlashing() { delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); slashTokens = bound(slashTokens, tokens + 1, tokens + 1 + delegationTokens); - verifierCutAmount = bound(verifierCutAmount, 0, MAX_MAX_VERIFIER_CUT); - uint256 maxVerifierTokens = (tokens * MAX_MAX_VERIFIER_CUT) / MAX_PPM; - vm.assume(verifierCutAmount <= maxVerifierTokens); + verifierCutAmount = bound(verifierCutAmount, 0, tokens); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); @@ -89,7 +86,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest { uint256 tokens, uint256 slashTokens, uint256 delegationTokens - ) public useIndexer useProvision(tokens, MAX_MAX_VERIFIER_CUT, 0) { + ) public useIndexer useProvision(tokens, MAX_PPM, 0) { delegationTokens = bound(delegationTokens, 0, MAX_STAKING_TOKENS); vm.assume(slashTokens > tokens + delegationTokens); diff --git a/packages/horizon/test/utils/Constants.sol b/packages/horizon/test/utils/Constants.sol index 270bb1e43..5b1449ea7 100644 --- a/packages/horizon/test/utils/Constants.sol +++ b/packages/horizon/test/utils/Constants.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.27; abstract contract Constants { - uint256 internal constant MAX_PPM = 1000000; // 100% in parts per million + uint32 internal constant MAX_PPM = 1000000; // 100% in parts per million uint256 internal constant delegationFeeCut = 100000; // 10% in parts per million uint256 internal constant MAX_STAKING_TOKENS = 10_000_000_000 ether; // GraphEscrow parameters @@ -12,7 +12,6 @@ abstract contract Constants { uint256 internal constant protocolPaymentCut = 10000; // Staking constants uint256 internal constant MAX_THAW_REQUESTS = 100; - uint32 internal constant MAX_MAX_VERIFIER_CUT = 1000000; // 100% in parts per million uint64 internal constant MAX_THAWING_PERIOD = 28 days; uint32 internal constant THAWING_PERIOD_IN_BLOCKS = 300; // Epoch manager diff --git a/packages/subgraph-service/test/subgraphService/provision/accept.t.sol b/packages/subgraph-service/test/subgraphService/provision/accept.t.sol index 44ce19b5f..812a27639 100644 --- a/packages/subgraph-service/test/subgraphService/provision/accept.t.sol +++ b/packages/subgraph-service/test/subgraphService/provision/accept.t.sol @@ -22,7 +22,7 @@ contract SubgraphServiceProvisionAcceptTest is SubgraphServiceTest { tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS); vm.assume(newVerifierCut >= fishermanRewardPercentage); vm.assume(newVerifierCut <= MAX_PPM); - vm.assume(newDisputePeriod >= disputePeriod); + newDisputePeriod = uint64(bound(newDisputePeriod, disputePeriod, MAX_WAIT_PERIOD)); // Setup indexer _createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod); @@ -58,7 +58,7 @@ contract SubgraphServiceProvisionAcceptTest is SubgraphServiceTest { uint32 newVerifierCut ) public useIndexer { tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS); - vm.assume(newVerifierCut > MAX_PPM); + vm.assume(newVerifierCut < maxSlashingPercentage); // Setup indexer _createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod); diff --git a/packages/subgraph-service/test/utils/Constants.sol b/packages/subgraph-service/test/utils/Constants.sol index eabf5e7b2..f1aac3d16 100644 --- a/packages/subgraph-service/test/utils/Constants.sol +++ b/packages/subgraph-service/test/utils/Constants.sol @@ -6,7 +6,7 @@ abstract contract Constants { uint256 internal constant MAX_PPM = 1_000_000; uint256 internal constant EPOCH_LENGTH = 1; // Dispute Manager - uint64 internal constant disputePeriod = 300; // 5 minutes + uint64 internal constant disputePeriod = 7 days; uint256 internal constant disputeDeposit = 100 ether; // 100 GRT uint32 internal constant fishermanRewardPercentage = 500000; // 50% uint32 internal constant maxSlashingPercentage = 500000; // 50%