From 85a038f0a82185fc4c576b36406f301eed6c4bed Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 20 Sep 2024 14:30:01 -0300 Subject: [PATCH 1/2] chore(Horizon): reprovision only tokens thawed --- .../internal/IHorizonStakingMain.sol | 3 +- .../contracts/staking/HorizonStaking.sol | 22 +++++---- .../HorizonStakingShared.t.sol | 9 ++-- .../test/staking/provision/reprovision.t.sol | 48 ++----------------- 4 files changed, 22 insertions(+), 60 deletions(-) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index eafce457a..6e0cc024f 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -614,13 +614,12 @@ interface IHorizonStakingMain { * @param serviceProvider The service provider address * @param oldVerifier The verifier address for which the tokens are currently provisioned * @param newVerifier The verifier address for which the tokens will be provisioned - * @param tokens The amount of tokens to move + * @param nThawRequests The number of thaw requests to fulfill. Set to 0 to fulfill all thaw requests. */ function reprovision( address serviceProvider, address oldVerifier, address newVerifier, - uint256 tokens, uint256 nThawRequests ) external; diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index c8709e639..a48589c64 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -207,7 +207,6 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { address serviceProvider, address oldVerifier, address newVerifier, - uint256 tokens, uint256 nThawRequests ) external @@ -216,8 +215,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { onlyAuthorized(serviceProvider, oldVerifier) onlyAuthorized(serviceProvider, newVerifier) { - _deprovision(serviceProvider, oldVerifier, nThawRequests); - _addToProvision(serviceProvider, newVerifier, tokens); + uint256 tokensThawed = _deprovision(serviceProvider, oldVerifier, nThawRequests); + _addToProvision(serviceProvider, newVerifier, tokensThawed); } /** @@ -693,13 +692,17 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { /** * @notice See {IHorizonStakingMain-deprovision}. */ - function _deprovision(address _serviceProvider, address _verifier, uint256 _nThawRequests) private { + function _deprovision( + address _serviceProvider, + address _verifier, + uint256 _nThawRequests + ) private returns (uint256 tokensThawed) { Provision storage prov = _provisions[_serviceProvider][_verifier]; - uint256 tokensThawed = 0; + uint256 _tokensThawed = 0; uint256 sharesThawing = prov.sharesThawing; uint256 tokensThawing = prov.tokensThawing; - (tokensThawed, tokensThawing, sharesThawing) = _fulfillThawRequests( + (_tokensThawed, tokensThawing, sharesThawing) = _fulfillThawRequests( _serviceProvider, _verifier, _serviceProvider, @@ -708,12 +711,13 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { _nThawRequests ); - prov.tokens = prov.tokens - tokensThawed; + prov.tokens = prov.tokens - _tokensThawed; prov.sharesThawing = sharesThawing; prov.tokensThawing = tokensThawing; - _serviceProviders[_serviceProvider].tokensProvisioned -= tokensThawed; + _serviceProviders[_serviceProvider].tokensProvisioned -= _tokensThawed; - emit TokensDeprovisioned(_serviceProvider, _verifier, tokensThawed); + emit TokensDeprovisioned(_serviceProvider, _verifier, _tokensThawed); + return _tokensThawed; } /** diff --git a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol index d9f300979..e171e455e 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol @@ -559,7 +559,6 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { address serviceProvider, address verifier, address newVerifier, - uint256 tokens, uint256 nThawRequests ) internal { // before @@ -595,8 +594,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { vm.expectEmit(address(staking)); emit IHorizonStakingMain.TokensDeprovisioned(serviceProvider, verifier, calcValues.tokensThawed); vm.expectEmit(); - emit IHorizonStakingMain.ProvisionIncreased(serviceProvider, newVerifier, tokens); - staking.reprovision(serviceProvider, verifier, newVerifier, tokens, nThawRequests); + emit IHorizonStakingMain.ProvisionIncreased(serviceProvider, newVerifier, calcValues.tokensThawed); + staking.reprovision(serviceProvider, verifier, newVerifier, nThawRequests); // after Provision memory afterProvision = staking.getProvision(serviceProvider, verifier); @@ -619,7 +618,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { assertEq(afterProvision.thawingPeriodPending, beforeValues.provision.thawingPeriodPending); // assert: provision new verifier - assertEq(afterProvisionNewVerifier.tokens, beforeValues.provisionNewVerifier.tokens + tokens); + assertEq(afterProvisionNewVerifier.tokens, beforeValues.provisionNewVerifier.tokens + calcValues.tokensThawed); assertEq(afterProvisionNewVerifier.tokensThawing, beforeValues.provisionNewVerifier.tokensThawing); assertEq(afterProvisionNewVerifier.sharesThawing, beforeValues.provisionNewVerifier.sharesThawing); assertEq(afterProvisionNewVerifier.maxVerifierCut, beforeValues.provisionNewVerifier.maxVerifierCut); @@ -632,7 +631,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { assertEq(afterServiceProvider.tokensStaked, beforeValues.serviceProvider.tokensStaked); assertEq( afterServiceProvider.tokensProvisioned, - beforeValues.serviceProvider.tokensProvisioned + tokens - calcValues.tokensThawed + beforeValues.serviceProvider.tokensProvisioned + calcValues.tokensThawed - calcValues.tokensThawed ); assertEq(afterServiceProvider.__DEPRECATED_tokensAllocated, beforeValues.serviceProvider.__DEPRECATED_tokensAllocated); assertEq(afterServiceProvider.__DEPRECATED_tokensLocked, beforeValues.serviceProvider.__DEPRECATED_tokensLocked); diff --git a/packages/horizon/test/staking/provision/reprovision.t.sol b/packages/horizon/test/staking/provision/reprovision.t.sol index 6fd170e4b..722ecfe59 100644 --- a/packages/horizon/test/staking/provision/reprovision.t.sol +++ b/packages/horizon/test/staking/provision/reprovision.t.sol @@ -25,29 +25,7 @@ contract HorizonStakingReprovisionTest is HorizonStakingTest { _createProvision(users.indexer, newDataService, 1 ether, 0, thawingPeriod); - _reprovision(users.indexer, subgraphDataServiceAddress, newDataService, provisionAmount, 0); - } - - function testReprovision_TokensOverThawingTokens() public useIndexer { - uint64 thawingPeriod = 1 days; - - // create provision A, thaw 10 ether, skip time so they are fully thawed - _createProvision(users.indexer, subgraphDataServiceAddress, 100 ether, 0, thawingPeriod); - _thaw(users.indexer, subgraphDataServiceAddress, 10 ether); - skip(thawingPeriod + 1); - - // create provision B - _createProvision(users.indexer, newDataService, 1 ether, 0, thawingPeriod); - - // reprovision 100 ether from A to B - // this should revert because there are only 10 ether that thawed and the service provider - // doesn't have additional idle stake to cover the difference - vm.expectRevert(); - staking.reprovision(users.indexer, subgraphDataServiceAddress, newDataService, 100 ether, 0); - - // now add some idle stake and try again, it should not revert - _stake(100 ether); - _reprovision(users.indexer, subgraphDataServiceAddress, newDataService, 100 ether, 0); + _reprovision(users.indexer, subgraphDataServiceAddress, newDataService, 0); } function testReprovision_OperatorMovingTokens( @@ -64,7 +42,7 @@ contract HorizonStakingReprovisionTest is HorizonStakingTest { // Switch back to operator vm.startPrank(users.operator); _createProvision(users.indexer, newDataService, 1 ether, 0, thawingPeriod); - _reprovision(users.indexer, subgraphDataServiceAddress, newDataService, provisionAmount, 0); + _reprovision(users.indexer, subgraphDataServiceAddress, newDataService, 0); } function testReprovision_RevertWhen_OperatorNotAuthorizedForNewDataService( @@ -85,30 +63,12 @@ contract HorizonStakingReprovisionTest is HorizonStakingTest { newDataService ); vm.expectRevert(expectedError); - staking.reprovision(users.indexer, subgraphDataServiceAddress, newDataService, provisionAmount, 0); + staking.reprovision(users.indexer, subgraphDataServiceAddress, newDataService, 0); } function testReprovision_RevertWhen_NoThawingTokens(uint256 amount) public useIndexer useProvision(amount, 0, 0) { bytes memory expectedError = abi.encodeWithSignature("HorizonStakingNothingThawing()"); vm.expectRevert(expectedError); - staking.reprovision(users.indexer, subgraphDataServiceAddress, newDataService, amount, 0); - } - - function testReprovision_RevertWhen_StillThawing( - uint64 thawingPeriod, - uint256 provisionAmount - ) public useIndexer useProvision(provisionAmount, 0, thawingPeriod) { - vm.assume(thawingPeriod > 0); - _thaw(users.indexer, subgraphDataServiceAddress, provisionAmount); - - _createProvision(users.indexer, newDataService, 1 ether, 0, thawingPeriod); - - bytes memory expectedError = abi.encodeWithSignature( - "HorizonStakingInsufficientIdleStake(uint256,uint256)", - provisionAmount, - 0 - ); - vm.expectRevert(expectedError); - staking.reprovision(users.indexer, subgraphDataServiceAddress, newDataService, provisionAmount, 0); + staking.reprovision(users.indexer, subgraphDataServiceAddress, newDataService, 0); } } From 9b4e6264891cdf9a50651581f53107ae124a6fcf Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Tue, 1 Oct 2024 14:59:13 -0300 Subject: [PATCH 2/2] fix: rename variable --- .../horizon/contracts/staking/HorizonStaking.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index a48589c64..918f947c7 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -699,10 +699,10 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { ) private returns (uint256 tokensThawed) { Provision storage prov = _provisions[_serviceProvider][_verifier]; - uint256 _tokensThawed = 0; + uint256 tokensThawed_ = 0; uint256 sharesThawing = prov.sharesThawing; uint256 tokensThawing = prov.tokensThawing; - (_tokensThawed, tokensThawing, sharesThawing) = _fulfillThawRequests( + (tokensThawed_, tokensThawing, sharesThawing) = _fulfillThawRequests( _serviceProvider, _verifier, _serviceProvider, @@ -711,13 +711,13 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { _nThawRequests ); - prov.tokens = prov.tokens - _tokensThawed; + prov.tokens = prov.tokens - tokensThawed_; prov.sharesThawing = sharesThawing; prov.tokensThawing = tokensThawing; - _serviceProviders[_serviceProvider].tokensProvisioned -= _tokensThawed; + _serviceProviders[_serviceProvider].tokensProvisioned -= tokensThawed_; - emit TokensDeprovisioned(_serviceProvider, _verifier, _tokensThawed); - return _tokensThawed; + emit TokensDeprovisioned(_serviceProvider, _verifier, tokensThawed_); + return tokensThawed_; } /**