diff --git a/packages/horizon/contracts/HorizonStaking.sol b/packages/horizon/contracts/HorizonStaking.sol index a2f0d0a38..75be4377f 100644 --- a/packages/horizon/contracts/HorizonStaking.sol +++ b/packages/horizon/contracts/HorizonStaking.sol @@ -13,9 +13,8 @@ import { HorizonStakingV1Storage } from "./HorizonStakingStorage.sol"; contract HorizonStaking is HorizonStakingV1Storage, IHorizonStakingBase, GraphUpgradeable { /// Maximum value that can be set as the maxVerifierCut in a provision. - /// It is equivalent to 50% in parts-per-million, to protect delegators from - /// service providers using a malicious verifier. - uint32 private constant MAX_MAX_VERIFIER_CUT = 500000; // 50% + /// It is equivalent to 100% in parts-per-million + uint32 private constant MAX_MAX_VERIFIER_CUT = 1000000; // 50% /// Minimum size of a provision uint256 private constant MIN_PROVISION_SIZE = 1e18; @@ -37,7 +36,6 @@ contract HorizonStaking is HorizonStakingV1Storage, IHorizonStakingBase, GraphUp error HorizonStakingInvalidZeroTokens(); error HorizonStakingInvalidProvision(address serviceProvider, address verifier); error HorizonStakingNotAuthorized(address caller, address serviceProvider, address verifier); - error HorizonStakingNotGlobalAuthorized(address caller, address serviceProvider); error HorizonStakingInsufficientCapacity(); constructor( @@ -137,9 +135,6 @@ contract HorizonStaking is HorizonStakingV1Storage, IHorizonStakingBase, GraphUp if (getIdleStake(_serviceProvider) < _tokens) { revert HorizonStakingInsufficientCapacity(); } - if (!verifierAllowlist[_serviceProvider][_verifier]) { - revert HorizonStakingVerifierNotAllowed(_verifier); - } _createProvision(_serviceProvider, _tokens, _verifier, _maxVerifierCut, _thawingPeriod); } @@ -244,13 +239,12 @@ contract HorizonStaking is HorizonStakingV1Storage, IHorizonStakingBase, GraphUp } // moves idle stake back to the owner's account - stake is removed from the protocol - // global operators are allowed to call this but stake is always sent to the service provider's address - function unstake(address _serviceProvider, uint256 _tokens) external override notPaused { - require(isGlobalAuthorized(msg.sender, _serviceProvider), "!auth"); + function unstake(uint256 _tokens) external override notPaused { + address serviceProvider = msg.sender; require(_tokens > 0, "!tokens"); - require(getIdleStake(_serviceProvider) >= _tokens, "insufficient idle stake"); + require(getIdleStake(serviceProvider) >= _tokens, "insufficient idle stake"); - ServiceProviderInternal storage sp = serviceProviders[_serviceProvider]; + ServiceProviderInternal storage sp = serviceProviders[serviceProvider]; uint256 stakedTokens = sp.tokensStaked; // Check that the indexer's stake minus // TODO this is only needed until legacy allocations are closed, @@ -264,12 +258,12 @@ contract HorizonStaking is HorizonStakingV1Storage, IHorizonStakingBase, GraphUp uint256 lockingPeriod = __DEPRECATED_thawingPeriod; if (lockingPeriod == 0) { sp.tokensStaked = stakedTokens - _tokens; - TokenUtils.pushTokens(_graphToken(), _serviceProvider, _tokens); - emit StakeWithdrawn(_serviceProvider, _tokens); + TokenUtils.pushTokens(_graphToken(), serviceProvider, _tokens); + emit StakeWithdrawn(serviceProvider, _tokens); } else { // Before locking more tokens, withdraw any unlocked ones if possible if (sp.__DEPRECATED_tokensLockedUntil != 0 && block.number >= sp.__DEPRECATED_tokensLockedUntil) { - _withdraw(_serviceProvider); + _withdraw(serviceProvider); } // TODO remove after the transition period // Take into account period averaging for multiple unstake requests @@ -285,7 +279,7 @@ contract HorizonStaking is HorizonStakingV1Storage, IHorizonStakingBase, GraphUp // Update balances sp.__DEPRECATED_tokensLocked = sp.__DEPRECATED_tokensLocked + _tokens; sp.__DEPRECATED_tokensLockedUntil = block.number + lockingPeriod; - emit StakeLocked(_serviceProvider, sp.__DEPRECATED_tokensLocked, sp.__DEPRECATED_tokensLockedUntil); + emit StakeLocked(serviceProvider, sp.__DEPRECATED_tokensLocked, sp.__DEPRECATED_tokensLockedUntil); } } @@ -357,15 +351,18 @@ contract HorizonStaking is HorizonStakingV1Storage, IHorizonStakingBase, GraphUp * @param _serviceProvider The service provider on behalf of whom they're claiming to act * @param _verifier The verifier / data service on which they're claiming to act */ - function isAuthorized(address _operator, address _serviceProvider, address _verifier) private view returns (bool) { + function isAuthorized( + address _operator, + address _serviceProvider, + address _verifier + ) public view override returns (bool) { if (_operator == _serviceProvider) { return true; } if (_verifier == SUBGRAPH_DATA_SERVICE_ADDRESS) { - return legacyOperatorAuth[_serviceProvider][_operator] || globalOperatorAuth[_serviceProvider][_operator]; + return legacyOperatorAuth[_serviceProvider][_operator]; } else { - return - operatorAuth[_serviceProvider][_verifier][_operator] || globalOperatorAuth[_serviceProvider][_operator]; + return operatorAuth[_serviceProvider][_verifier][_operator]; } } @@ -384,23 +381,13 @@ contract HorizonStaking is HorizonStakingV1Storage, IHorizonStakingBase, GraphUp return provisions[_serviceProvider][_verifier].tokens - provisions[_serviceProvider][_verifier].tokensThawing; } - /** - * @notice Check if an operator is authorized for the caller on all their allowlisted verifiers and global stake. - * @param _operator The address to check for auth - * @param _serviceProvider The service provider on behalf of whom they're claiming to act - */ - function isGlobalAuthorized(address _operator, address _serviceProvider) public view override returns (bool) { - return _operator == _serviceProvider || globalOperatorAuth[_serviceProvider][_operator]; - } - /** * @notice Withdraw indexer tokens once the thawing period has passed. * @dev This is only needed during the transition period while we still have * a global lock. After that, unstake() will also withdraw. */ - function withdrawLocked(address _serviceProvider) external override notPaused { - require(isGlobalAuthorized(msg.sender, _serviceProvider), "!auth"); - _withdraw(_serviceProvider); + function withdrawLocked() external override notPaused { + _withdraw(msg.sender); } function delegate(address _serviceProvider, address _verifier, uint256 _tokens) public override notPartialPaused { diff --git a/packages/horizon/contracts/HorizonStakingExtension.sol b/packages/horizon/contracts/HorizonStakingExtension.sol index 2571a9928..d9b61939e 100644 --- a/packages/horizon/contracts/HorizonStakingExtension.sol +++ b/packages/horizon/contracts/HorizonStakingExtension.sol @@ -25,10 +25,6 @@ contract HorizonStakingExtension is StakingBackwardsCompatibility, IHorizonStaki _; } - error HorizonStakingInvalidVerifier(address verifier); - error HorizonStakingVerifierAlreadyAllowed(address verifier); - error HorizonStakingVerifierNotAllowed(address verifier); - constructor( address _controller, address _subgraphDataServiceAddress, @@ -82,53 +78,6 @@ contract HorizonStakingExtension is StakingBackwardsCompatibility, IHorizonStaki return sp; } - /** - * @notice Allow verifier for stake provisions. - * After calling this, and a timelock period, the service provider will - * be allowed to provision stake that is slashable by the verifier. - * @param _verifier The address of the contract that can slash the provision - */ - function allowVerifier(address _verifier) external override { - if (_verifier == address(0)) { - revert HorizonStakingInvalidVerifier(_verifier); - } - if (verifierAllowlist[msg.sender][_verifier]) { - revert HorizonStakingVerifierAlreadyAllowed(_verifier); - } - verifierAllowlist[msg.sender][_verifier] = true; - emit VerifierAllowed(msg.sender, _verifier); - } - - /** - * @notice Deny a verifier for stake provisions. - * After calling this, the service provider will immediately - * be unable to provision any stake to the verifier. - * Any existing provisions will be unaffected. - * @param _verifier The address of the contract that can slash the provision - */ - function denyVerifier(address _verifier) external { - if (!verifierAllowlist[msg.sender][_verifier]) { - revert HorizonStakingVerifierNotAllowed(_verifier); - } - verifierAllowlist[msg.sender][_verifier] = false; - emit VerifierDenied(msg.sender, _verifier); - } - - /** - * @notice Authorize or unauthorize an address to be an operator for the caller on all data services. - * @param _operator Address to authorize or unauthorize - * @param _allowed Whether the operator is authorized or not - */ - function setGlobalOperator(address _operator, bool _allowed) external override { - require(_operator != msg.sender, "operator == sender"); - globalOperatorAuth[msg.sender][_operator] = _allowed; - emit GlobalOperatorSet(msg.sender, _operator, _allowed); - } - - function isAllowedVerifier(address _serviceProvider, address _verifier) external view override returns (bool) { - return _verifier == SUBGRAPH_DATA_SERVICE_ADDRESS || verifierAllowlist[_serviceProvider][_verifier]; - } - /** * @notice Authorize or unauthorize an address to be an operator for the caller on a data service. * @param _operator Address to authorize or unauthorize diff --git a/packages/horizon/contracts/HorizonStakingStorage.sol b/packages/horizon/contracts/HorizonStakingStorage.sol index cec775a94..161566e9c 100644 --- a/packages/horizon/contracts/HorizonStakingStorage.sol +++ b/packages/horizon/contracts/HorizonStakingStorage.sol @@ -136,9 +136,6 @@ abstract contract HorizonStakingV1Storage is Managed, IHorizonStakingTypes { mapping(bytes32 => ThawRequest) internal thawRequests; - // indexer => operator => authorized - mapping(address => mapping(address => bool)) internal globalOperatorAuth; - // indexer => verifier => operator => authorized mapping(address => mapping(address => mapping(address => bool))) internal operatorAuth; diff --git a/packages/horizon/contracts/IHorizonStakingBase.sol b/packages/horizon/contracts/IHorizonStakingBase.sol index b014bd93b..afbd8a6ea 100644 --- a/packages/horizon/contracts/IHorizonStakingBase.sol +++ b/packages/horizon/contracts/IHorizonStakingBase.sol @@ -129,7 +129,7 @@ interface IHorizonStakingBase is IHorizonStakingTypes { ) external; // moves thawed stake back to the owner's account - stake is removed from the protocol - function unstake(address _serviceProvider, uint256 _tokens) external; + function unstake(uint256 _tokens) external; // delegate tokens to a provider on a data service function delegate(address _serviceProvider, address _verifier, uint256 _tokens) external; @@ -151,19 +151,20 @@ interface IHorizonStakingBase is IHorizonStakingTypes { // `getStake(serviceProvider) - ServiceProvider.tokensProvisioned` function getIdleStake(address _serviceProvider) external view returns (uint256 tokens); - /** - * @notice Check if an operator is authorized for the caller on all their allowlisted verifiers and global stake. - * @param _operator The address to check for auth - * @param _serviceProvider The service provider on behalf of whom they're claiming to act - */ - function isGlobalAuthorized(address _operator, address _serviceProvider) external view returns (bool); - /** * @notice Withdraw indexer tokens once the thawing period has passed. * @dev This is only needed during the transition period while we still have * a global lock. After that, unstake() will also withdraw. */ - function withdrawLocked(address _serviceProvider) external; + function withdrawLocked() external; function setDelegationSlashingEnabled(bool _enabled) external; + + /** + * @notice Check if an operator is authorized for the caller on a specific verifier / data service. + * @param _operator The address to check for auth + * @param _serviceProvider The service provider on behalf of whom they're claiming to act + * @param _verifier The verifier / data service on which they're claiming to act + */ + function isAuthorized(address _operator, address _serviceProvider, address _verifier) external view returns (bool); } diff --git a/packages/horizon/contracts/IHorizonStakingExtension.sol b/packages/horizon/contracts/IHorizonStakingExtension.sol index 013147abc..6e8bfe161 100644 --- a/packages/horizon/contracts/IHorizonStakingExtension.sol +++ b/packages/horizon/contracts/IHorizonStakingExtension.sol @@ -6,21 +6,6 @@ pragma abicoder v2; import { IHorizonStakingTypes } from "./IHorizonStakingTypes.sol"; interface IHorizonStakingExtension { - /** - * @dev Emitted when serviceProvider allows a verifier - */ - event VerifierAllowed(address indexed serviceProvider, address indexed verifier); - - /** - * @dev Emitted when serviceProvider denies a verifier - */ - event VerifierDenied(address indexed serviceProvider, address indexed verifier); - - /** - * @dev Emitted when a global operator (for all data services) is allowed or denied by a service provider - */ - event GlobalOperatorSet(address indexed serviceProvider, address indexed operator, bool allowed); - /** * @dev Emitted when an operator is allowed or denied by a service provider for a particular data service */ @@ -36,24 +21,6 @@ interface IHorizonStakingExtension { address serviceProvider ) external view returns (IHorizonStakingTypes.ServiceProvider memory); - function allowVerifier(address _verifier) external; - - /** - * @notice Deny a verifier for stake provisions. - * After calling this, the service provider will immediately - * be unable to provision any stake to the verifier. - * Any existing provisions will be unaffected. - * @param _verifier The address of the contract that can slash the provision - */ - function denyVerifier(address _verifier) external; - - /** - * @notice Authorize or unauthorize an address to be an operator for the caller on all data services. - * @param _operator Address to authorize or unauthorize - * @param _allowed Whether the operator is authorized or not - */ - function setGlobalOperator(address _operator, bool _allowed) external; - /** * @notice Authorize or unauthorize an address to be an operator for the caller on a data service. * @param _operator Address to authorize or unauthorize @@ -62,7 +29,5 @@ interface IHorizonStakingExtension { */ function setOperator(address _operator, address _verifier, bool _allowed) external; - function isAllowedVerifier(address _serviceProvider, address _verifier) external view returns (bool); - function getMaxThawingPeriod() external view returns (uint64); } diff --git a/packages/horizon/contracts/StakingBackwardsCompatibility.sol b/packages/horizon/contracts/StakingBackwardsCompatibility.sol index dcc4b3c46..63784155f 100644 --- a/packages/horizon/contracts/StakingBackwardsCompatibility.sol +++ b/packages/horizon/contracts/StakingBackwardsCompatibility.sol @@ -49,24 +49,6 @@ abstract contract StakingBackwardsCompatibility is EXPONENTIAL_REBATES_ADDRESS = _exponentialRebatesAddress; } - /** - * @notice Check if an operator is authorized for the caller on a specific verifier / data service. - * @param _operator The address to check for auth - * @param _serviceProvider The service provider on behalf of whom they're claiming to act - * @param _verifier The verifier / data service on which they're claiming to act - */ - function isAuthorized(address _operator, address _serviceProvider, address _verifier) internal view returns (bool) { - if (_operator == _serviceProvider) { - return true; - } - if (_verifier == SUBGRAPH_DATA_SERVICE_ADDRESS) { - return legacyOperatorAuth[_serviceProvider][_operator] || globalOperatorAuth[_serviceProvider][_operator]; - } else { - return - operatorAuth[_serviceProvider][_verifier][_operator] || globalOperatorAuth[_serviceProvider][_operator]; - } - } - /** * @notice Set the address of the counterpart (L1 or L2) staking contract. * @dev This function can only be called by the governor. diff --git a/packages/horizon/test/HorizonStaking.t.sol b/packages/horizon/test/HorizonStaking.t.sol index 68005bb5f..144b0c7a5 100644 --- a/packages/horizon/test/HorizonStaking.t.sol +++ b/packages/horizon/test/HorizonStaking.t.sol @@ -24,18 +24,12 @@ contract HorizonStakingTest is Test { staking = IHorizonStaking(address(new HorizonStaking(address(controller), address(ext), address(0x1)))); } - function test_AllowVerifier() public { - address verifier = address(0x1337); - address serviceProvider = address(this); - HorizonStakingExtension(payable(address(staking))).allowVerifier(verifier); - assertTrue(staking.isAllowedVerifier(serviceProvider, verifier)); - } - function test_SetGlobalOperator() public { address operator = address(0x1337); + address dataService = address(0x1338); address serviceProvider = address(this); - staking.setGlobalOperator(operator, true); - assertTrue(staking.isGlobalAuthorized(operator, serviceProvider)); + staking.setOperator(operator, dataService, true); + assertTrue(staking.isAuthorized(operator, serviceProvider, dataService)); } } diff --git a/packages/horizon/test/HorizonStaking.ts b/packages/horizon/test/HorizonStaking.ts index ead17a92c..6f6e1ad88 100644 --- a/packages/horizon/test/HorizonStaking.ts +++ b/packages/horizon/test/HorizonStaking.ts @@ -26,12 +26,13 @@ describe('HorizonStaking', function () { return { horizonStaking, owner } } - describe('Verifier allowlist', function () { - it('adds a verifier to the allowlist', async function () { + describe('setOperator', function () { + it('adds an operator', async function () { const { horizonStaking, owner } = await loadFixture(deployFixture) const verifier = ethers.Wallet.createRandom().address - await horizonStaking.connect(owner).allowVerifier(verifier) - expect(await horizonStaking.isAllowedVerifier(owner, verifier)).to.be.true + const operator = ethers.Wallet.createRandom().address + await horizonStaking.connect(owner).setOperator(operator, verifier, true) + expect(await horizonStaking.isAuthorized(operator, owner, verifier)).to.be.true }) }) })