Merge pull request #980 from graphprotocol/mde/remove-minimum-provisi…

…on-size

fix: remove MIN_PROVISION_SIZE from horizon staking

packages/horizon/contracts/staking/HorizonStaking.sol

@@ -34,9 +34,6 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
     /// It is equivalent to 100% in parts-per-million
     uint32 private constant MAX_MAX_VERIFIER_CUT = uint32(PPMMath.MAX_PPM);
 
-    /// @dev Minimum size of a provision
-    uint256 private constant MIN_PROVISION_SIZE = 1e18;
-
     /// @dev Fixed point precision
     uint256 private constant FIXED_POINT_PRECISION = 1e18;
 
@@ -599,7 +596,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
         uint32 _maxVerifierCut,
         uint64 _thawingPeriod
     ) private {
-        require(_tokens >= MIN_PROVISION_SIZE, HorizonStakingInsufficientTokens(_tokens, MIN_PROVISION_SIZE));
+        require(_tokens > 0, HorizonStakingInvalidZeroTokens());
         require(
             _maxVerifierCut <= MAX_MAX_VERIFIER_CUT,
             HorizonStakingInvalidMaxVerifierCut(_maxVerifierCut, MAX_MAX_VERIFIER_CUT)

packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol

@@ -19,14 +19,14 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
     }
 
     modifier assumeProvisionTokens(uint256 tokens) {
-        vm.assume(tokens > MIN_PROVISION_SIZE);
+        vm.assume(tokens > 0);
         vm.assume(tokens <= MAX_STAKING_TOKENS);
         _;
     }
 
     modifier useProvision(uint256 tokens, uint32 maxVerifierCut, uint64 thawingPeriod) {
         vm.assume(tokens <= MAX_STAKING_TOKENS);
-        vm.assume(tokens > MIN_PROVISION_SIZE);
+        vm.assume(tokens > 0);
         vm.assume(maxVerifierCut <= MAX_MAX_VERIFIER_CUT);
         vm.assume(thawingPeriod <= MAX_THAWING_PERIOD);
         _createProvision(subgraphDataServiceAddress, tokens, maxVerifierCut, thawingPeriod);

packages/horizon/test/staking/HorizonStaking.t.sol

@@ -21,14 +21,14 @@ contract HorizonStakingTest is HorizonStakingSharedTest, IHorizonStakingTypes {
     }
 
     modifier useStake(uint256 amount) {
-        vm.assume(amount > MIN_PROVISION_SIZE);
+        vm.assume(amount > 0);
         approve(address(staking), amount);
         staking.stake(amount);
         _;
     }
 
     modifier useStakeTo(address to, uint256 amount) {
-        vm.assume(amount > MIN_PROVISION_SIZE);
+        vm.assume(amount > 0);
         _stakeTo(to, amount);
         _;
     }

packages/horizon/test/staking/delegation/delegate.t.sol

@@ -48,7 +48,7 @@ contract HorizonStakingDelegateTest is HorizonStakingTest {
         uint256 amount,
         uint256 delegationAmount
     ) public useIndexer {
-        amount = bound(amount, MIN_PROVISION_SIZE, MAX_STAKING_TOKENS);
+        amount = bound(amount, 1 ether, MAX_STAKING_TOKENS);
         delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS);
         _createProvision(subgraphDataServiceLegacyAddress, amount, 0, 0);
 

packages/horizon/test/staking/delegation/undelegate.t.sol

@@ -44,7 +44,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest {
     ) public useIndexer useProvision(amount, 0, 0) useDelegation(delegationAmount) {
         resetPrank(users.delegator);
         Delegation memory delegation = _getDelegation(subgraphDataServiceAddress);
-        overDelegationShares = bound(overDelegationShares, delegation.shares + 1, MAX_STAKING_TOKENS);
+        overDelegationShares = bound(overDelegationShares, delegation.shares + 1, MAX_STAKING_TOKENS + 1);
 
         bytes memory expectedError = abi.encodeWithSignature(
             "HorizonStakingInsufficientShares(uint256,uint256)",
@@ -80,7 +80,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest {
         uint256 amount,
         uint256 delegationAmount
     ) public useIndexer {
-        amount = bound(amount, MIN_PROVISION_SIZE, MAX_STAKING_TOKENS);
+        amount = bound(amount, 1, MAX_STAKING_TOKENS);
         delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS);
         _createProvision(subgraphDataServiceLegacyAddress, amount, 0, 0);
 

packages/horizon/test/staking/provision/deprovision.t.sol

@@ -30,24 +30,16 @@ contract HorizonStakingDeprovisionTest is HorizonStakingTest {
         uint64 thawingPeriod,
         uint256 thawAmount
     ) public useIndexer useProvision(amount, maxVerifierCut, thawingPeriod) {
-        thawAmount = bound(thawAmount, MIN_DELEGATION, amount);
+        vm.assume(amount > 1);
+        thawAmount = bound(thawAmount, 2, amount);
         uint256 thawAmount1 = thawAmount / 2;
-        bytes32 thawRequestId = _createThawRequest(thawAmount1);
-        bytes32 thawRequestId2 = _createThawRequest(thawAmount - thawAmount1);
-        skip(thawingPeriod + 1);
-
-        console.log("thawAmount1: ", thawAmount1);
-        console.log("thawAmount2: ", thawAmount - thawAmount1);
+        _createThawRequest(thawAmount1);
+        _createThawRequest(thawAmount - thawAmount1);
 
-        ThawRequest memory thawRequest1 = staking.getThawRequest(thawRequestId);
-        ThawRequest memory thawRequest2 = staking.getThawRequest(thawRequestId2);
-        console.log("Thaw request 1 shares: ", thawRequest1.shares);
-        console.log("Thaw request 2 shares: ", thawRequest2.shares);
+        skip(thawingPeriod + 1);
 
-        console.log("Idle stake before deprovision: ", staking.getIdleStake(users.indexer));
         _deprovision(1);
         uint256 idleStake = staking.getIdleStake(users.indexer);
-        console.log("Idle stake after deprovision: ", idleStake);
         assertEq(idleStake, thawAmount1);
     }
 

packages/horizon/test/staking/provision/provision.t.sol

@@ -20,22 +20,16 @@ contract HorizonStakingProvisionTest is HorizonStakingTest {
         assertEq(provisionTokens, amount);
     }
 
-    function testProvision_RevertWhen_InsufficientTokens(uint256 amount) public useIndexer useStake(1000 ether) {
-        amount = bound(amount, 0, MIN_PROVISION_SIZE - 1);
-        bytes memory expectedError = abi.encodeWithSignature(
-            "HorizonStakingInsufficientTokens(uint256,uint256)",
-            amount,
-            MIN_PROVISION_SIZE
-        );
+    function testProvision_RevertWhen_ZeroTokens() public useIndexer useStake(1000 ether) {
+        bytes memory expectedError = abi.encodeWithSignature("HorizonStakingInvalidZeroTokens()");
         vm.expectRevert(expectedError);
-        staking.provision(users.indexer, subgraphDataServiceAddress, amount, 0, 0);
+        staking.provision(users.indexer, subgraphDataServiceAddress, 0, 0, 0);
     }
 
     function testProvision_RevertWhen_MaxVerifierCutTooHigh(
         uint256 amount,
         uint32 maxVerifierCut
     ) public useIndexer useStake(amount) {
-        vm.assume(amount > MIN_PROVISION_SIZE);
         vm.assume(maxVerifierCut > MAX_MAX_VERIFIER_CUT);
         bytes memory expectedError = abi.encodeWithSignature(
             "HorizonStakingInvalidMaxVerifierCut(uint32,uint32)",
@@ -50,7 +44,6 @@ contract HorizonStakingProvisionTest is HorizonStakingTest {
         uint256 amount,
         uint64 thawingPeriod
     ) public useIndexer useStake(amount) {
-        vm.assume(amount > MIN_PROVISION_SIZE);
         vm.assume(thawingPeriod > MAX_THAWING_PERIOD);
         bytes memory expectedError = abi.encodeWithSignature(
             "HorizonStakingInvalidThawingPeriod(uint64,uint64)",
@@ -65,7 +58,6 @@ contract HorizonStakingProvisionTest is HorizonStakingTest {
         uint256 amount,
         uint256 provisionTokens
     ) public useIndexer useStake(amount) {
-        vm.assume(amount > MIN_PROVISION_SIZE);
         vm.assume(provisionTokens > amount);
         bytes memory expectedError = abi.encodeWithSignature(
             "HorizonStakingInsufficientIdleStake(uint256,uint256)",

packages/horizon/test/staking/provision/reprovision.t.sol

@@ -36,15 +36,15 @@ contract HorizonStakingReprovisionTest is HorizonStakingTest {
     {
         skip(thawingPeriod + 1);
 
-        _createProvision(newDataService, MIN_PROVISION_SIZE, 0, thawingPeriod);
+        _createProvision(newDataService, 1 ether, 0, thawingPeriod);
 
         // nThawRequests == 0 reprovisions all thaw requests
         _reprovision(provisionAmount, 0);
         uint256 idleStake = staking.getIdleStake(users.indexer);
         assertEq(idleStake, 0 ether);
 
         uint256 provisionTokens = staking.getProviderTokensAvailable(users.indexer, newDataService);
-        assertEq(provisionTokens, provisionAmount + MIN_PROVISION_SIZE);
+        assertEq(provisionTokens, provisionAmount + 1 ether);
     }
 
     function testReprovision_OperatorMovingTokens(
@@ -64,13 +64,13 @@ contract HorizonStakingReprovisionTest is HorizonStakingTest {
 
         // Switch back to operator
         vm.startPrank(users.operator);
-        _createProvision(newDataService, MIN_PROVISION_SIZE, 0, thawingPeriod);
+        _createProvision(newDataService, 1 ether, 0, thawingPeriod);
         _reprovision(provisionAmount, 0);
         uint256 idleStake = staking.getIdleStake(users.indexer);
         assertEq(idleStake, 0 ether);
 
         uint256 provisionTokens = staking.getProviderTokensAvailable(users.indexer, newDataService);
-        assertEq(provisionTokens, provisionAmount + MIN_PROVISION_SIZE);
+        assertEq(provisionTokens, provisionAmount + 1 ether);
     }
 
     function testReprovision_RevertWhen_OperatorNotAuthorizedForNewDataService(
@@ -83,7 +83,7 @@ contract HorizonStakingReprovisionTest is HorizonStakingTest {
     {
         // Switch to indexer to create new provision
         vm.startPrank(users.indexer);
-        _createProvision(newDataService, MIN_PROVISION_SIZE, 0, 0);
+        _createProvision(newDataService, 1 ether, 0, 0);
 
         // Switch back to operator
         vm.startPrank(users.operator);
@@ -116,7 +116,7 @@ contract HorizonStakingReprovisionTest is HorizonStakingTest {
     {
         vm.assume(thawingPeriod > 0);
 
-        _createProvision(newDataService, MIN_PROVISION_SIZE, 0, thawingPeriod);
+        _createProvision(newDataService, 1 ether, 0, thawingPeriod);
 
         bytes memory expectedError = abi.encodeWithSignature(
             "HorizonStakingInsufficientIdleStake(uint256,uint256)",

packages/horizon/test/staking/slash/slash.t.sol

@@ -39,9 +39,9 @@ contract HorizonStakingSlashTest is HorizonStakingTest {
         uint256 verifierCutAmount
     ) public useIndexer useProvision(amount, maxVerifierCut, 0) {
         verifierCutAmount = bound(verifierCutAmount, 0, maxVerifierCut);
-
-        // TODO: when slashing for low amounts there's an arithmetic underflow
-        slashAmount = bound(slashAmount, MIN_PROVISION_SIZE, amount);
+        slashAmount = bound(slashAmount, 1, amount);
+        uint256 maxVerifierTokens = (slashAmount * maxVerifierCut) / MAX_PPM;
+        vm.assume(verifierCutAmount <= maxVerifierTokens);
 
         vm.startPrank(subgraphDataServiceAddress);
         _slash(slashAmount, verifierCutAmount);
@@ -53,6 +53,24 @@ contract HorizonStakingSlashTest is HorizonStakingTest {
         assertEq(verifierTokens, verifierCutAmount);
     }
 
+    function testSlash_RevertWhen_TooManyTokens(
+        uint256 amount,
+        uint32 maxVerifierCut,
+        uint256 verifierCutAmount
+    ) public useIndexer useProvision(amount, maxVerifierCut, 0) {
+        uint256 maxVerifierTokens = (amount * maxVerifierCut) / MAX_PPM;
+        verifierCutAmount = bound(verifierCutAmount, maxVerifierTokens + 1, MAX_STAKING_TOKENS);
+
+        vm.startPrank(subgraphDataServiceAddress);
+        bytes memory expectedError = abi.encodeWithSignature(
+            "HorizonStakingTooManyTokens(uint256,uint256)",
+            verifierCutAmount,
+            maxVerifierTokens
+        );
+        vm.expectRevert(expectedError);
+        _slash(amount, verifierCutAmount);
+    }
+
     function testSlash_DelegationDisabled_SlashingOverProvisionTokens(
         uint256 amount,
         uint256 slashAmount,
@@ -62,6 +80,8 @@ contract HorizonStakingSlashTest is HorizonStakingTest {
         delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS);
         slashAmount = bound(slashAmount, amount + 1, amount + delegationAmount);
         verifierCutAmount = bound(verifierCutAmount, 0, MAX_MAX_VERIFIER_CUT);
+        uint256 maxVerifierTokens = (amount * MAX_MAX_VERIFIER_CUT) / MAX_PPM;
+        vm.assume(verifierCutAmount <= maxVerifierTokens);
 
         resetPrank(users.delegator);
         _delegate(delegationAmount, subgraphDataServiceAddress);
@@ -89,6 +109,8 @@ contract HorizonStakingSlashTest is HorizonStakingTest {
         delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS);
         slashAmount = bound(slashAmount, amount + 1, amount + delegationAmount);
         verifierCutAmount = bound(verifierCutAmount, 0, MAX_MAX_VERIFIER_CUT);
+        uint256 maxVerifierTokens = (amount * MAX_MAX_VERIFIER_CUT) / MAX_PPM;
+        vm.assume(verifierCutAmount <= maxVerifierTokens);
 
         resetPrank(users.delegator);
         _delegate(delegationAmount, subgraphDataServiceAddress);
@@ -111,8 +133,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest {
         uint256 amount,
         uint256 slashAmount
     ) public useIndexer useStake(amount) {
-        // TODO: when slashing for low amounts there's an arithmetic underflow
-        slashAmount = bound(slashAmount, MIN_PROVISION_SIZE, amount);
+        slashAmount = bound(slashAmount, 1, amount);
         bytes memory expectedError = abi.encodeWithSignature(
             "HorizonStakingInsufficientTokens(uint256,uint256)",
             0 ether,

packages/horizon/test/staking/thaw/thaw.t.sol

@@ -35,6 +35,7 @@ contract HorizonStakingThawTest is HorizonStakingTest {
         uint256 thawAmount,
         uint256 thawAmount2
     ) public useIndexer useProvision(amount, 0, thawingPeriod) {
+        vm.assume(amount > 1);
         thawAmount = bound(thawAmount, 1, amount - 1);
         thawAmount2 = bound(thawAmount2, 1, amount - thawAmount);
         bytes32 thawRequestId = _createThawRequest(thawAmount);
@@ -96,6 +97,7 @@ contract HorizonStakingThawTest is HorizonStakingTest {
         uint64 thawingPeriod,
         uint256 thawAmount
     ) public useIndexer useProvision(amount, 0, thawingPeriod) {
+        vm.assume(amount >= MAX_THAW_REQUESTS + 1);
         thawAmount = bound(thawAmount, 1, amount / (MAX_THAW_REQUESTS + 1));
 
         for (uint256 i = 0; i < 100; i++) {

packages/horizon/test/utils/Constants.sol

@@ -12,8 +12,7 @@ abstract contract Constants {
     uint256 internal constant protocolPaymentCut = 10000;
     // Staking constants
     uint256 internal constant MAX_THAW_REQUESTS = 100;
-    uint256 internal constant MIN_PROVISION_SIZE = 1e18;
     uint32 internal constant MAX_MAX_VERIFIER_CUT = 1000000; // 100% in parts per million
     uint64 internal constant MAX_THAWING_PERIOD = 28 days;
-    uint256 internal constant MIN_DELEGATION = 1e18;
+    uint256 internal constant MIN_DELEGATION = 1 ether;
 }