From 05de8aa8826e747fe3df504d93a5f7079ec25954 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Tue, 4 Jun 2024 19:05:36 -0300 Subject: [PATCH] fix: remove MIN_PROVISION_SIZE from horizon staking --- .../contracts/staking/HorizonStaking.sol | 5 +-- .../HorizonStakingShared.t.sol | 4 +-- .../horizon/test/staking/HorizonStaking.t.sol | 4 +-- .../test/staking/allocation/allocation.t.sol | 0 .../test/staking/delegation/delegate.t.sol | 2 +- .../test/staking/delegation/undelegate.t.sol | 4 +-- .../test/staking/provision/deprovision.t.sol | 18 +++-------- .../test/staking/provision/provision.t.sol | 14 ++------- .../test/staking/provision/reprovision.t.sol | 12 +++---- .../horizon/test/staking/slash/slash.t.sol | 31 ++++++++++++++++--- packages/horizon/test/staking/thaw/thaw.t.sol | 2 ++ packages/horizon/test/utils/Constants.sol | 3 +- 12 files changed, 51 insertions(+), 48 deletions(-) create mode 100644 packages/horizon/test/staking/allocation/allocation.t.sol diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index c8d7f437f..4caa10a96 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/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) diff --git a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol index 85cbbcbaf..1f1ea3310 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/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); diff --git a/packages/horizon/test/staking/HorizonStaking.t.sol b/packages/horizon/test/staking/HorizonStaking.t.sol index e6ecc555c..bb3703b34 100644 --- a/packages/horizon/test/staking/HorizonStaking.t.sol +++ b/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); _; } diff --git a/packages/horizon/test/staking/allocation/allocation.t.sol b/packages/horizon/test/staking/allocation/allocation.t.sol new file mode 100644 index 000000000..e69de29bb diff --git a/packages/horizon/test/staking/delegation/delegate.t.sol b/packages/horizon/test/staking/delegation/delegate.t.sol index 708c3bf5c..c254756fc 100644 --- a/packages/horizon/test/staking/delegation/delegate.t.sol +++ b/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); diff --git a/packages/horizon/test/staking/delegation/undelegate.t.sol b/packages/horizon/test/staking/delegation/undelegate.t.sol index 9b74f729d..5dcc85183 100644 --- a/packages/horizon/test/staking/delegation/undelegate.t.sol +++ b/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); diff --git a/packages/horizon/test/staking/provision/deprovision.t.sol b/packages/horizon/test/staking/provision/deprovision.t.sol index 9f3438739..fbb9d0fc4 100644 --- a/packages/horizon/test/staking/provision/deprovision.t.sol +++ b/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); } diff --git a/packages/horizon/test/staking/provision/provision.t.sol b/packages/horizon/test/staking/provision/provision.t.sol index 823a87d54..fc941ec7f 100644 --- a/packages/horizon/test/staking/provision/provision.t.sol +++ b/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)", diff --git a/packages/horizon/test/staking/provision/reprovision.t.sol b/packages/horizon/test/staking/provision/reprovision.t.sol index 3d1d803dc..5d7ed8068 100644 --- a/packages/horizon/test/staking/provision/reprovision.t.sol +++ b/packages/horizon/test/staking/provision/reprovision.t.sol @@ -36,7 +36,7 @@ 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); @@ -44,7 +44,7 @@ contract HorizonStakingReprovisionTest is HorizonStakingTest { 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)", diff --git a/packages/horizon/test/staking/slash/slash.t.sol b/packages/horizon/test/staking/slash/slash.t.sol index 50f4d1e88..758ebc1cf 100644 --- a/packages/horizon/test/staking/slash/slash.t.sol +++ b/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, diff --git a/packages/horizon/test/staking/thaw/thaw.t.sol b/packages/horizon/test/staking/thaw/thaw.t.sol index 1f747f7f0..2ed99682f 100644 --- a/packages/horizon/test/staking/thaw/thaw.t.sol +++ b/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++) { diff --git a/packages/horizon/test/utils/Constants.sol b/packages/horizon/test/utils/Constants.sol index 46445b039..f3e7f1157 100644 --- a/packages/horizon/test/utils/Constants.sol +++ b/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; } \ No newline at end of file