feat: remove asset holder allowlist (with latest main)

config/graph.arbitrum-goerli.yml

@@ -123,9 +123,6 @@ contracts:
       - fn: "setSlasher"
         slasher: "${{DisputeManager.address}}"
         allowed: true
-      - fn: "setAssetHolder"
-        assetHolder: "${{AllocationExchange.address}}"
-        allowed: true
       - fn: "syncAllContracts"
   RewardsManager:
     proxy: true

config/graph.arbitrum-localhost.yml

@@ -123,9 +123,6 @@ contracts:
       - fn: "setSlasher"
         slasher: "${{DisputeManager.address}}"
         allowed: true
-      - fn: "setAssetHolder"
-        assetHolder: "${{AllocationExchange.address}}"
-        allowed: true
       - fn: "syncAllContracts"
   RewardsManager:
     proxy: true

config/graph.arbitrum-one.yml

@@ -123,9 +123,6 @@ contracts:
       - fn: "setSlasher"
         slasher: "${{DisputeManager.address}}"
         allowed: true
-      - fn: "setAssetHolder"
-        assetHolder: "${{AllocationExchange.address}}"
-        allowed: true
       - fn: "syncAllContracts"
   RewardsManager:
     proxy: true

config/graph.goerli.yml

@@ -126,9 +126,6 @@ contracts:
       - fn: "setSlasher"
         slasher: "${{DisputeManager.address}}"
         allowed: true
-      - fn: "setAssetHolder"
-        assetHolder: "${{AllocationExchange.address}}"
-        allowed: true
       - fn: "syncAllContracts"
   RewardsManager:
     proxy: true

config/graph.localhost.yml

@@ -131,9 +131,6 @@ contracts:
       - fn: "setSlasher"
         slasher: "${{DisputeManager.address}}"
         allowed: true
-      - fn: "setAssetHolder"
-        assetHolder: "${{AllocationExchange.address}}"
-        allowed: true
       - fn: "syncAllContracts"
   RewardsManager:
     proxy: true

config/graph.mainnet.yml

@@ -126,9 +126,6 @@ contracts:
       - fn: "setSlasher"
         slasher: "${{DisputeManager.address}}"
         allowed: true
-      - fn: "setAssetHolder"
-        assetHolder: "${{AllocationExchange.address}}"
-        allowed: true
       - fn: "syncAllContracts"
   RewardsManager:
     proxy: true

contracts/staking/IStakingBase.sol

@@ -93,12 +93,6 @@ interface IStakingBase is IStakingData {
         uint32 cooldownBlocks
     );
 
-    /**
-     * @dev Emitted when `caller` set `assetHolder` address as `allowed` to send funds
-     * to staking contract.
-     */
-    event AssetHolderUpdate(address indexed caller, address indexed assetHolder, bool allowed);
-
     /**
      * @dev Emitted when `indexer` set `operator` access.
      */
@@ -219,14 +213,6 @@ interface IStakingBase is IStakingData {
         uint32 _lambdaDenominator
     ) external;
 
-    /**
-     * @notice Set an address as allowed asset holder.
-     * @dev This function can only be called by the governor.
-     * @param _assetHolder Address of allowed source for state channel funds
-     * @param _allowed True if asset holder is allowed
-     */
-    function setAssetHolder(address _assetHolder, bool _allowed) external;
-
     /**
      * @notice Authorize or unauthorize an address to be an operator for the caller.
      * @param _operator Address to authorize or unauthorize

contracts/staking/IStakingExtension.sol

@@ -237,15 +237,6 @@ interface IStakingExtension is IStakingData {
      */
     function rewardsDestination(address _indexer) external view returns (address);
 
-    /**
-     * @notice Getter for assetHolders[_maybeAssetHolder]:
-     * returns true if the address is an asset holder, i.e. an entity that can collect
-     * query fees into the Staking contract.
-     * @param _maybeAssetHolder The address that may or may not be an asset holder
-     * @return True if the address is an asset holder
-     */
-    function assetHolders(address _maybeAssetHolder) external view returns (bool);
-
     /**
      * @notice Getter for subgraphAllocations[_subgraphDeploymentId]:
      * returns the amount of tokens allocated to a subgraph deployment.

contracts/staking/Staking.sol

@@ -220,17 +220,6 @@ abstract contract Staking is StakingV4Storage, GraphUpgradeable, IStakingBase, M
         );
     }
 
-    /**
-     * @notice Set an address as allowed asset holder.
-     * @param _assetHolder Address of allowed source for state channel funds
-     * @param _allowed True if asset holder is allowed
-     */
-    function setAssetHolder(address _assetHolder, bool _allowed) external override onlyGovernor {
-        require(_assetHolder != address(0), "!assetHolder");
-        __assetHolders[_assetHolder] = _allowed;
-        emit AssetHolderUpdate(msg.sender, _assetHolder, _allowed);
-    }
-
     /**
      * @notice Authorize or unauthorize an address to be an operator for the caller.
      * @param _operator Address to authorize or unauthorize
@@ -354,7 +343,6 @@ abstract contract Staking is StakingV4Storage, GraphUpgradeable, IStakingBase, M
 
     /**
      * @dev Collect and rebate query fees from state channels to the indexer
-     * Funds received are only accepted from a valid sender.
      * To avoid reverting on the withdrawal from channel flow this function will accept calls with zero tokens.
      * We use an exponential rebate formula to calculate the amount of tokens to rebate to the indexer.
      * This implementation allows collecting multiple times on the same allocation, keeping track of the
@@ -366,13 +354,17 @@ abstract contract Staking is StakingV4Storage, GraphUpgradeable, IStakingBase, M
         // Allocation identifier validation
         require(_allocationID != address(0), "!alloc");
 
-        // The contract caller must be an authorized asset holder
-        require(__assetHolders[msg.sender] == true, "!assetHolder");
-
         // Allocation must exist
         AllocationState allocState = _getAllocationState(_allocationID);
         require(allocState != AllocationState.Null, "!collect");
 
+        // If the query fees are zero, we don't want to revert
+        // but we also don't need to do anything, so just return
+        if (_tokens == 0) {
+            return;
+        }
+
+        // Get allocation
         Allocation storage alloc = __allocations[_allocationID];
         bytes32 subgraphDeploymentID = alloc.subgraphDeploymentID;
 
@@ -382,9 +374,8 @@ abstract contract Staking is StakingV4Storage, GraphUpgradeable, IStakingBase, M
         uint256 queryRebates = 0; // Tokens to distribute to indexer
         uint256 delegationRewards = 0; // Tokens to distribute to delegators
 
-        // Process query fees only if non-zero amount
-        if (queryFees > 0) {
-            // -- Pull tokens from the authorized sender --
+        {
+            // -- Pull tokens from the sender --
             IGraphToken graphToken = graphToken();
             TokenUtils.pullTokens(graphToken, msg.sender, queryFees);
 
@@ -789,7 +780,7 @@ abstract contract Staking is StakingV4Storage, GraphUpgradeable, IStakingBase, M
 
         // Creates an allocation
         // Allocation identifiers are not reused
-        // The assetHolder address can send collected funds to the allocation
+        // Anyone can send collected funds to the allocation using collect()
         Allocation memory alloc = Allocation(
             _indexer,
             _subgraphDeploymentID,

contracts/staking/StakingExtension.sol

@@ -301,17 +301,6 @@
         return __rewardsDestination[_indexer];
     }
 
-    /**
-     * @notice Getter for assetHolders[_maybeAssetHolder]:
-     * returns true if the address is an asset holder, i.e. an entity that can collect
-     * query fees into the Staking contract.
-     * @param _maybeAssetHolder The address that may or may not be an asset holder
-     * @return True if the address is an asset holder
-     */
-    function assetHolders(address _maybeAssetHolder) external view override returns (bool) {
-        return __assetHolders[_maybeAssetHolder];
-    }
-
     /**
      * @notice Getter for operatorAuth[_indexer][_maybeOperator]:
      * returns true if the operator is authorized to operate on behalf of the indexer.
@@ -334,13 +323,13 @@
      * @param _subgraphDeploymentId The subgraph deployment for which to query the allocations
      * @return The amount of tokens allocated to the subgraph deployment
      */
     function subgraphAllocations(bytes32 _subgraphDeploymentId)
         external
         view
         override
         returns (uint256)
     {
         return __subgraphAllocations[_subgraphDeploymentId];
     }
 
     /**

contracts/staking/StakingStorage.sol

@@ -89,8 +89,8 @@ contract StakingV1Storage is Managed {
 
     // -- Asset Holders --
 
-    /// @dev Allowed AssetHolders that can collect query fees: assetHolder => is allowed
-    mapping(address => bool) internal __assetHolders;
+    /// @dev DEPRECATED: Allowed AssetHolders: assetHolder => is allowed
+    mapping(address => bool) private __DEPRECATED_assetHolders; // solhint-disable-line var-name-mixedcase
 }
 
 /**

e2e/deployment/config/staking.test.ts

@@ -26,11 +26,6 @@ describe('Staking configuration', () => {
     expect(isSlasher).eq(true)
   })
 
-  it('should allow AllocationExchange to collect query fees', async function () {
-    const allowed = await Staking.assetHolders(AllocationExchange.address)
-    expect(allowed).eq(true)
-  })
-
   it('minimumIndexerStake should match "minimumIndexerStake" in the config file', async function () {
     const value = await Staking.minimumIndexerStake()
     const expected = getItemValue(graphConfig, `contracts/${contractName}/init/minimumIndexerStake`)

test/disputes/poi.test.ts

@@ -105,9 +105,6 @@ describe('DisputeManager:POI', async () => {
     // Give some funds to the fisherman
     await grt.connect(governor.signer).mint(fisherman.address, fishermanTokens)
     await grt.connect(fisherman.signer).approve(disputeManager.address, fishermanTokens)
-
-    // Allow the asset holder
-    await staking.connect(governor.signer).setAssetHolder(assetHolder.address, true)
   })
 
   beforeEach(async function () {

test/disputes/query.test.ts

@@ -153,9 +153,6 @@ describe('DisputeManager:Query', async () => {
       await grt.connect(dst.signer).approve(disputeManager.address, fishermanTokens)
     }
 
-    // Allow the asset holder
-    await staking.connect(governor.signer).setAssetHolder(assetHolder.address, true)
-
     // Create an attestation
     const attestation = await buildAttestation(receipt, indexer1ChannelKey.privKey)
 

test/payments/allocationExchange.test.ts

@@ -77,7 +77,6 @@ describe('AllocationExchange', () => {
     await grt.connect(governor.signer).mint(allocationExchange.address, exchangeTokens)
 
     // Ensure the exchange is correctly setup
-    await staking.connect(governor.signer).setAssetHolder(allocationExchange.address, true)
     await allocationExchange.connect(governor.signer).setAuthority(authority.address, true)
     await allocationExchange.approveAll()
   })

test/rewards/rewards.test.ts

@@ -1013,7 +1013,6 @@ describe('Rewards', () => {
 
       // allow the asset holder
       const tokensToCollect = toGRT('10000')
-      await staking.connect(governor.signer).setAssetHolder(assetHolder.address, true)
 
       // signal in two subgraphs in the same block
       const subgraphs = [subgraphDeploymentID1, subgraphDeploymentID2]