From 292dc699a9e2d900ecdf3ee27ba2087941feca2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Fri, 21 Feb 2025 10:06:20 -0300 Subject: [PATCH 1/3] fix: only allow subgraph service as verifier during transition period MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .../interfaces/internal/IHorizonStakingMain.sol | 9 +++++++++ .../horizon/contracts/staking/HorizonStaking.sol | 5 +++++ packages/horizon/hardhat.config.ts | 2 +- .../horizon/test/staking/provision/parameters.t.sol | 1 - .../horizon/test/staking/provision/provision.t.sol | 13 +++++++++++++ packages/horizon/test/staking/stake/unstake.t.sol | 12 ++++++------ 6 files changed, 34 insertions(+), 8 deletions(-) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index 264005e8a..c5c9898d4 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -362,6 +362,13 @@ interface IHorizonStakingMain { */ error HorizonStakingNotAuthorized(address serviceProvider, address verifier, address caller); + /** + * @notice Thrown when attempting to create a provision for a data service other than the + * subgraph data service. This restriction only applies during the transition period. + * @param verifier The verifier address + */ + error HorizonStakingInvalidVerifier(address verifier); + /** * @notice Thrown when attempting to create a provision with an invalid maximum verifier cut. * @param maxVerifierCut The maximum verifier cut @@ -562,6 +569,8 @@ interface IHorizonStakingMain { * service, where the data service is the verifier. * This function can be called by the service provider or by an operator authorized by the provider * for this specific verifier. + * @dev During the transition period, only the subgraph data service can be used as a verifier. This + * prevents an escape hatch for legacy allocation stake. * @dev Requirements: * - `tokens` cannot be zero. * - The `serviceProvider` must have enough idle stake to cover the tokens to provision. diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 9c523d97e..95a4f5715 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -720,6 +720,11 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint64 _thawingPeriod ) private { require(_tokens > 0, HorizonStakingInvalidZeroTokens()); + // TODO: Remove this after the transition period - it prevents an early escape hatch for legacy allocations + require( + __DEPRECATED_thawingPeriod == 0 || _verifier == SUBGRAPH_DATA_SERVICE_ADDRESS, + HorizonStakingInvalidVerifier(_verifier) + ); require(PPMMath.isValidPPM(_maxVerifierCut), HorizonStakingInvalidMaxVerifierCut(_maxVerifierCut)); require( _thawingPeriod <= _maxThawingPeriod, diff --git a/packages/horizon/hardhat.config.ts b/packages/horizon/hardhat.config.ts index 9b726a2ec..494de23d5 100644 --- a/packages/horizon/hardhat.config.ts +++ b/packages/horizon/hardhat.config.ts @@ -22,7 +22,7 @@ const config: HardhatUserConfig = { settings: { optimizer: { enabled: true, - runs: 70, + runs: 20, }, }, }, diff --git a/packages/horizon/test/staking/provision/parameters.t.sol b/packages/horizon/test/staking/provision/parameters.t.sol index 589dd4fc3..6e0b4fabf 100644 --- a/packages/horizon/test/staking/provision/parameters.t.sol +++ b/packages/horizon/test/staking/provision/parameters.t.sol @@ -82,7 +82,6 @@ contract HorizonStakingProvisionParametersTest is HorizonStakingTest { vm.stopPrank(); } - function test_ProvisionParametersAccept_SameParameters( uint256 amount, uint32 maxVerifierCut, diff --git a/packages/horizon/test/staking/provision/provision.t.sol b/packages/horizon/test/staking/provision/provision.t.sol index 81ed628fd..663bbe9b9 100644 --- a/packages/horizon/test/staking/provision/provision.t.sol +++ b/packages/horizon/test/staking/provision/provision.t.sol @@ -108,4 +108,17 @@ contract HorizonStakingProvisionTest is HorizonStakingTest { vm.expectRevert(expectedError); staking.provision(users.indexer, subgraphDataServiceAddress, amount, maxVerifierCut, thawingPeriod); } + + function testProvision_RevertWhen_VerifierIsNotSubgraphDataServiceDuringTransitionPeriod( + uint256 amount + ) public useIndexer useStake(amount) { + // simulate the transition period + _setStorage_DeprecatedThawingPeriod(THAWING_PERIOD_IN_BLOCKS); + + // oddly we use subgraphDataServiceLegacyAddress as the subgraph service address + // so subgraphDataServiceAddress is not the subgraph service ¯\_(ツ)_/¯ + bytes memory expectedError = abi.encodeWithSignature("HorizonStakingInvalidVerifier(address)", subgraphDataServiceAddress); + vm.expectRevert(expectedError); + staking.provision(users.indexer, subgraphDataServiceAddress, amount, 0, 0); + } } diff --git a/packages/horizon/test/staking/stake/unstake.t.sol b/packages/horizon/test/staking/stake/unstake.t.sol index 13ea92e00..83c6a0a81 100644 --- a/packages/horizon/test/staking/stake/unstake.t.sol +++ b/packages/horizon/test/staking/stake/unstake.t.sol @@ -62,10 +62,10 @@ contract HorizonStakingUnstakeTest is HorizonStakingTest { _setStorage_ServiceProvider(users.indexer, tokensLocked, 0, tokensLocked, block.number, 0); // create provision, thaw and deprovision - _createProvision(users.indexer, subgraphDataServiceAddress, tokens, 0, MAX_THAWING_PERIOD); - _thaw(users.indexer, subgraphDataServiceAddress, tokens); + _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, MAX_THAWING_PERIOD); + _thaw(users.indexer, subgraphDataServiceLegacyAddress, tokens); skip(MAX_THAWING_PERIOD + 1); - _deprovision(users.indexer, subgraphDataServiceAddress, 0); + _deprovision(users.indexer, subgraphDataServiceLegacyAddress, 0); // unstake _unstake(tokensToUnstake); @@ -90,10 +90,10 @@ contract HorizonStakingUnstakeTest is HorizonStakingTest { _setStorage_ServiceProvider(users.indexer, tokensThawing, 0, tokensThawing, tokensThawingUntilBlock, 0); // create provision, thaw and deprovision - _createProvision(users.indexer, subgraphDataServiceAddress, tokens, 0, MAX_THAWING_PERIOD); - _thaw(users.indexer, subgraphDataServiceAddress, tokens); + _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, MAX_THAWING_PERIOD); + _thaw(users.indexer, subgraphDataServiceLegacyAddress, tokens); skip(MAX_THAWING_PERIOD + 1); - _deprovision(users.indexer, subgraphDataServiceAddress, 0); + _deprovision(users.indexer, subgraphDataServiceLegacyAddress, 0); // unstake _unstake(tokensToUnstake); From 662bf93005582b64e5410ad80f0d6bbd0196286e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Fri, 21 Feb 2025 10:09:11 -0300 Subject: [PATCH 2/3] chore: fix comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .../contracts/interfaces/internal/IHorizonStakingMain.sol | 2 +- packages/horizon/test/staking/provision/provision.t.sol | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index c5c9898d4..a934ac667 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -363,7 +363,7 @@ interface IHorizonStakingMain { error HorizonStakingNotAuthorized(address serviceProvider, address verifier, address caller); /** - * @notice Thrown when attempting to create a provision for a data service other than the + * @notice Thrown when attempting to create a provision with a verifier other than the * subgraph data service. This restriction only applies during the transition period. * @param verifier The verifier address */ diff --git a/packages/horizon/test/staking/provision/provision.t.sol b/packages/horizon/test/staking/provision/provision.t.sol index 663bbe9b9..ba580be25 100644 --- a/packages/horizon/test/staking/provision/provision.t.sol +++ b/packages/horizon/test/staking/provision/provision.t.sol @@ -117,7 +117,10 @@ contract HorizonStakingProvisionTest is HorizonStakingTest { // oddly we use subgraphDataServiceLegacyAddress as the subgraph service address // so subgraphDataServiceAddress is not the subgraph service ¯\_(ツ)_/¯ - bytes memory expectedError = abi.encodeWithSignature("HorizonStakingInvalidVerifier(address)", subgraphDataServiceAddress); + bytes memory expectedError = abi.encodeWithSignature( + "HorizonStakingInvalidVerifier(address)", + subgraphDataServiceAddress + ); vm.expectRevert(expectedError); staking.provision(users.indexer, subgraphDataServiceAddress, amount, 0, 0); } From 6460b849a284f93b07d4343bef3649cffc73772e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Fri, 21 Feb 2025 10:41:32 -0300 Subject: [PATCH 3/3] chore: invert require checks for gas efficiency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- packages/horizon/contracts/staking/HorizonStaking.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 95a4f5715..9002bce8c 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -722,7 +722,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { require(_tokens > 0, HorizonStakingInvalidZeroTokens()); // TODO: Remove this after the transition period - it prevents an early escape hatch for legacy allocations require( - __DEPRECATED_thawingPeriod == 0 || _verifier == SUBGRAPH_DATA_SERVICE_ADDRESS, + _verifier == SUBGRAPH_DATA_SERVICE_ADDRESS || __DEPRECATED_thawingPeriod == 0, HorizonStakingInvalidVerifier(_verifier) ); require(PPMMath.isValidPPM(_maxVerifierCut), HorizonStakingInvalidMaxVerifierCut(_maxVerifierCut));