Skip to content

Commit

Permalink
chore: remove global operators and the verifier allowlist
Browse files Browse the repository at this point in the history
  • Loading branch information
@pcarranzav
pcarranzav committed May 8, 2024
1 parent 2053f65 commit 0b4e014
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 161 deletions.
51 changes: 19 additions & 32 deletions packages/horizon/contracts/HorizonStaking.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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];
}
}

Expand All @@ -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 {
Expand Down
51 changes: 0 additions & 51 deletions packages/horizon/contracts/HorizonStakingExtension.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions packages/horizon/contracts/HorizonStakingStorage.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
19 changes: 10 additions & 9 deletions packages/horizon/contracts/IHorizonStakingBase.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
35 changes: 0 additions & 35 deletions packages/horizon/contracts/IHorizonStakingExtension.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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
Expand All @@ -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);
}
18 changes: 0 additions & 18 deletions packages/horizon/contracts/StakingBackwardsCompatibility.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 3 additions & 9 deletions packages/horizon/test/HorizonStaking.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
9 changes: 5 additions & 4 deletions packages/horizon/test/HorizonStaking.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
})
})

0 comments on commit 0b4e014

Please sign in to comment.