From 4b0bf404738e89eb9ef54f4059d3bfa3dffccafa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Wed, 27 Nov 2024 12:17:28 -0300 Subject: [PATCH] fix: remove collector allowance feature from payments escrow (TRST-CL1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .../contracts/interfaces/IPaymentsEscrow.sol | 101 ---------------- .../contracts/payments/PaymentsEscrow.sol | 75 +----------- .../ignition/configs/horizon.hardhat.json | 1 - .../ignition/modules/core/PaymentsEscrow.ts | 3 +- packages/horizon/test/GraphBase.t.sol | 2 +- .../horizon/test/escrow/GraphEscrow.t.sol | 6 - packages/horizon/test/escrow/collect.t.sol | 36 +----- packages/horizon/test/escrow/collector.t.sol | 108 ------------------ packages/horizon/test/escrow/paused.t.sol | 22 ---- .../tap-collector/collect/collect.t.sol | 11 -- .../PaymentsEscrowShared.t.sol | 16 --- packages/horizon/test/utils/Constants.sol | 1 - .../test/SubgraphBaseTest.t.sol | 2 - .../subgraphService/SubgraphService.t.sol | 2 +- .../subgraphService/collect/query/query.t.sol | 7 +- .../subgraph-service/test/utils/Constants.sol | 1 - 16 files changed, 10 insertions(+), 384 deletions(-) delete mode 100644 packages/horizon/test/escrow/collector.t.sol diff --git a/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol b/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol index c1eb7707a..4d7207481 100644 --- a/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol +++ b/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol @@ -25,50 +25,6 @@ interface IPaymentsEscrow { uint256 thawEndTimestamp; } - /// @notice Details for a payer-collector pair - /// @dev Collectors can be removed only after a thawing period - struct Collector { - // Amount of tokens the collector is allowed to collect - uint256 allowance; - // Timestamp at which the collector thawing period ends (zero if not thawing) - uint256 thawEndTimestamp; - } - - /** - * @notice Emitted when a payer authorizes a collector to collect funds - * @param payer The address of the payer - * @param collector The address of the collector - * @param addedAllowance The amount of tokens added to the collector's allowance - * @param newTotalAllowance The new total allowance after addition - */ - event AuthorizedCollector( - address indexed payer, - address indexed collector, - uint256 addedAllowance, - uint256 newTotalAllowance - ); - - /** - * @notice Emitted when a payer thaws a collector - * @param payer The address of the payer - * @param collector The address of the collector - */ - event ThawCollector(address indexed payer, address indexed collector); - - /** - * @notice Emitted when a payer cancels the thawing of a collector - * @param payer The address of the payer - * @param collector The address of the collector - */ - event CancelThawCollector(address indexed payer, address indexed collector); - - /** - * @notice Emitted when a payer revokes a collector authorization. - * @param payer The address of the payer - * @param collector The address of the collector - */ - event RevokeCollector(address indexed payer, address indexed collector); - /** * @notice Emitted when a payer deposits funds into the escrow for a payer-collector-receiver tuple * @param payer The address of the payer @@ -152,13 +108,6 @@ interface IPaymentsEscrow { */ error PaymentsEscrowThawingPeriodTooLong(uint256 thawingPeriod, uint256 maxWaitPeriod); - /** - * @notice Thrown when a collector has insufficient allowance to collect funds - * @param allowance The current allowance - * @param minAllowance The minimum required allowance - */ - error PaymentsEscrowInsufficientAllowance(uint256 allowance, uint256 minAllowance); - /** * @notice Thrown when the contract balance is not consistent with the collection amount * @param balanceBefore The balance before the collection @@ -172,54 +121,6 @@ interface IPaymentsEscrow { */ error PaymentsEscrowInvalidZeroTokens(); - /** - * @notice Authorize a collector to collect funds from the payer's escrow - * @dev This function can only be used to increase the allowance of a collector. - * To reduce it the authorization must be revoked and a new one must be created. - * - * Requirements: - * - `allowance` must be greater than zero - * - * Emits an {AuthorizedCollector} event - * - * @param collector The address of the collector - * @param allowance The amount of tokens to add to the collector's allowance - */ - function approveCollector(address collector, uint256 allowance) external; - - /** - * @notice Thaw a collector's collector authorization - * @dev The thawing period is defined by the `REVOKE_COLLECTOR_THAWING_PERIOD` constant - * - * Emits a {ThawCollector} event - * - * @param collector The address of the collector - */ - function thawCollector(address collector) external; - - /** - * @notice Cancel a collector's authorization thawing - * @dev Requirements: - * - `collector` must be thawing - * - * Emits a {CancelThawCollector} event - * - * @param collector The address of the collector - */ - function cancelThawCollector(address collector) external; - - /** - * @notice Revoke a collector's authorization. - * Removes the collector from the list of authorized collectors. - * @dev Requirements: - * - `collector` must have thawed - * - * Emits a {RevokeCollector} event - * - * @param collector The address of the collector - */ - function revokeCollector(address collector) external; - /** * @notice Deposits funds into the escrow for a payer-collector-receiver tuple, where * the payer is the transaction caller. @@ -277,8 +178,6 @@ interface IPaymentsEscrow { * @notice Collects funds from the payer-collector-receiver's escrow and sends them to {GraphPayments} for * distribution using the Graph Horizon Payments protocol. * The function will revert if there are not enough funds in the escrow. - * @dev Requirements: - * - `collector` needs to be authorized by the payer and have enough allowance * * Emits an {EscrowCollected} event * diff --git a/packages/horizon/contracts/payments/PaymentsEscrow.sol b/packages/horizon/contracts/payments/PaymentsEscrow.sol index 7cf7e9e38..5643d4a5b 100644 --- a/packages/horizon/contracts/payments/PaymentsEscrow.sol +++ b/packages/horizon/contracts/payments/PaymentsEscrow.sol @@ -23,10 +23,6 @@ import { GraphDirectory } from "../utilities/GraphDirectory.sol"; contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, IPaymentsEscrow { using TokenUtils for IGraphToken; - /// @notice Authorization details for payer-collector pairs - mapping(address payer => mapping(address collector => IPaymentsEscrow.Collector collectorDetails)) - public authorizedCollectors; - /// @notice Escrow account details for payer-collector-receiver tuples mapping(address payer => mapping(address collector => mapping(address receiver => IPaymentsEscrow.EscrowAccount escrowAccount))) public escrowAccounts; @@ -35,9 +31,6 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, /// @dev This is a precautionary measure to avoid inadvertedly locking funds for too long uint256 public constant MAX_WAIT_PERIOD = 90 days; - /// @notice Thawing period in seconds for authorized collectors - uint256 public immutable REVOKE_COLLECTOR_THAWING_PERIOD; - /// @notice Thawing period in seconds for escrow funds withdrawal uint256 public immutable WITHDRAW_ESCROW_THAWING_PERIOD; @@ -49,24 +42,14 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, /** * @notice Construct the PaymentsEscrow contract * @param controller The address of the controller - * @param revokeCollectorThawingPeriod Thawing period in seconds for authorized collectors * @param withdrawEscrowThawingPeriod Thawing period in seconds for escrow funds withdrawal */ - constructor( - address controller, - uint256 revokeCollectorThawingPeriod, - uint256 withdrawEscrowThawingPeriod - ) GraphDirectory(controller) { - require( - revokeCollectorThawingPeriod <= MAX_WAIT_PERIOD, - PaymentsEscrowThawingPeriodTooLong(revokeCollectorThawingPeriod, MAX_WAIT_PERIOD) - ); + constructor(address controller, uint256 withdrawEscrowThawingPeriod) GraphDirectory(controller) { require( withdrawEscrowThawingPeriod <= MAX_WAIT_PERIOD, PaymentsEscrowThawingPeriodTooLong(withdrawEscrowThawingPeriod, MAX_WAIT_PERIOD) ); - REVOKE_COLLECTOR_THAWING_PERIOD = revokeCollectorThawingPeriod; WITHDRAW_ESCROW_THAWING_PERIOD = withdrawEscrowThawingPeriod; } @@ -77,52 +60,6 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, __Multicall_init(); } - /** - * @notice See {IPaymentsEscrow-approveCollector} - */ - function approveCollector(address collector_, uint256 allowance) external override notPaused { - require(allowance != 0, PaymentsEscrowInvalidZeroTokens()); - Collector storage collector = authorizedCollectors[msg.sender][collector_]; - collector.allowance += allowance; - emit AuthorizedCollector(msg.sender, collector_, allowance, collector.allowance); - } - - /** - * @notice See {IPaymentsEscrow-thawCollector} - */ - function thawCollector(address collector) external override notPaused { - authorizedCollectors[msg.sender][collector].thawEndTimestamp = - block.timestamp + - REVOKE_COLLECTOR_THAWING_PERIOD; - emit ThawCollector(msg.sender, collector); - } - - /** - * @notice See {IPaymentsEscrow-cancelThawCollector} - */ - function cancelThawCollector(address collector) external override notPaused { - require(authorizedCollectors[msg.sender][collector].thawEndTimestamp != 0, PaymentsEscrowNotThawing()); - - authorizedCollectors[msg.sender][collector].thawEndTimestamp = 0; - emit CancelThawCollector(msg.sender, collector); - } - - /** - * @notice See {IPaymentsEscrow-revokeCollector} - */ - function revokeCollector(address collector_) external override notPaused { - Collector storage collector = authorizedCollectors[msg.sender][collector_]; - - require(collector.thawEndTimestamp != 0, PaymentsEscrowNotThawing()); - require( - collector.thawEndTimestamp < block.timestamp, - PaymentsEscrowStillThawing(block.timestamp, collector.thawEndTimestamp) - ); - - delete authorizedCollectors[msg.sender][collector_]; - emit RevokeCollector(msg.sender, collector_); - } - /** * @notice See {IPaymentsEscrow-deposit} */ @@ -194,19 +131,11 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, address dataService, uint256 tokensDataService ) external override notPaused { - // Check if collector is authorized and has enough funds - Collector storage collectorDetails = authorizedCollectors[payer][msg.sender]; - require( - collectorDetails.allowance >= tokens, - PaymentsEscrowInsufficientAllowance(collectorDetails.allowance, tokens) - ); - // Check if there are enough funds in the escrow account EscrowAccount storage account = escrowAccounts[payer][msg.sender][receiver]; require(account.balance >= tokens, PaymentsEscrowInsufficientBalance(account.balance, tokens)); - // Reduce amount from approved collector and account balance - collectorDetails.allowance -= tokens; + // Reduce amount from account balance account.balance -= tokens; uint256 balanceBefore = _graphToken().balanceOf(address(this)); diff --git a/packages/horizon/ignition/configs/horizon.hardhat.json b/packages/horizon/ignition/configs/horizon.hardhat.json index 894a2ed59..7c4214d37 100644 --- a/packages/horizon/ignition/configs/horizon.hardhat.json +++ b/packages/horizon/ignition/configs/horizon.hardhat.json @@ -36,7 +36,6 @@ "protocolPaymentCut": 10000 }, "PaymentsEscrow": { - "revokeCollectorThawingPeriod": 10000, "withdrawEscrowThawingPeriod": 10000 }, "TAPCollector": { diff --git a/packages/horizon/ignition/modules/core/PaymentsEscrow.ts b/packages/horizon/ignition/modules/core/PaymentsEscrow.ts index 1dbd07088..7c7948a7e 100644 --- a/packages/horizon/ignition/modules/core/PaymentsEscrow.ts +++ b/packages/horizon/ignition/modules/core/PaymentsEscrow.ts @@ -10,13 +10,12 @@ export default buildModule('PaymentsEscrow', (m) => { const { Controller, PeripheryRegistered } = m.useModule(GraphPeripheryModule) const { PaymentsEscrowProxyAdmin, PaymentsEscrowProxy, HorizonRegistered } = m.useModule(HorizonProxiesModule) - const revokeCollectorThawingPeriod = m.getParameter('revokeCollectorThawingPeriod') const withdrawEscrowThawingPeriod = m.getParameter('withdrawEscrowThawingPeriod') // Deploy PaymentsEscrow implementation const PaymentsEscrowImplementation = m.contract('PaymentsEscrow', PaymentsEscrowArtifact, - [Controller, revokeCollectorThawingPeriod, withdrawEscrowThawingPeriod], + [Controller, withdrawEscrowThawingPeriod], { after: [PeripheryRegistered, HorizonRegistered], }, diff --git a/packages/horizon/test/GraphBase.t.sol b/packages/horizon/test/GraphBase.t.sol index 7aa44d6f5..b2d43ba63 100644 --- a/packages/horizon/test/GraphBase.t.sol +++ b/packages/horizon/test/GraphBase.t.sol @@ -116,7 +116,7 @@ abstract contract GraphBaseTest is IHorizonStakingTypes, Utils, Constants { // PaymentsEscrow bytes memory escrowImplementationParameters = abi.encode( address(controller), - revokeCollectorThawingPeriod,withdrawEscrowThawingPeriod + withdrawEscrowThawingPeriod ); bytes memory escrowImplementationBytecode = abi.encodePacked( type(PaymentsEscrow).creationCode, diff --git a/packages/horizon/test/escrow/GraphEscrow.t.sol b/packages/horizon/test/escrow/GraphEscrow.t.sol index d3ffd21da..421bbfdd2 100644 --- a/packages/horizon/test/escrow/GraphEscrow.t.sol +++ b/packages/horizon/test/escrow/GraphEscrow.t.sol @@ -24,12 +24,6 @@ contract GraphEscrowTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest { _; } - modifier useCollector(uint256 tokens) { - vm.assume(tokens > 0); - escrow.approveCollector(users.verifier, tokens); - _; - } - modifier depositAndThawTokens(uint256 amount, uint256 thawAmount) { vm.assume(thawAmount > 0); vm.assume(amount > thawAmount); diff --git a/packages/horizon/test/escrow/collect.t.sol b/packages/horizon/test/escrow/collect.t.sol index 72b795ee9..254d8738d 100644 --- a/packages/horizon/test/escrow/collect.t.sol +++ b/packages/horizon/test/escrow/collect.t.sol @@ -106,47 +106,17 @@ contract GraphEscrowCollectTest is GraphEscrowTest { _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); resetPrank(users.gateway); - escrow.approveCollector(users.verifier, tokens); _depositTokens(users.verifier, users.indexer, tokens); resetPrank(users.verifier); _collect(IGraphPayments.PaymentTypes.QueryFee, users.gateway, users.indexer, tokens, subgraphDataServiceAddress, tokensDataService); } - function testCollect_RevertWhen_CollectorNotAuthorized(uint256 amount) public { - vm.assume(amount > 0); - vm.startPrank(users.verifier); - uint256 dataServiceCut = 30000; // 3% - bytes memory expectedError = abi.encodeWithSelector( - IPaymentsEscrow.PaymentsEscrowInsufficientAllowance.selector, - 0, - amount - ); - vm.expectRevert(expectedError); - escrow.collect(IGraphPayments.PaymentTypes.QueryFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, dataServiceCut); - vm.stopPrank(); - } - - function testCollect_RevertWhen_CollectorHasInsufficientAmount( - uint256 amount, - uint256 insufficientAmount - ) public useGateway useCollector(insufficientAmount) useDeposit(amount) { - vm.assume(insufficientAmount < amount); - - vm.startPrank(users.verifier); - bytes memory expectedError = abi.encodeWithSignature( - "PaymentsEscrowInsufficientAllowance(uint256,uint256)", - insufficientAmount, - amount - ); - vm.expectRevert(expectedError); - escrow.collect(IGraphPayments.PaymentTypes.QueryFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, 0); - } - function testCollect_RevertWhen_SenderHasInsufficientAmountInEscrow( uint256 amount, uint256 insufficientAmount - ) public useGateway useCollector(amount) useDeposit(insufficientAmount) { + ) public useGateway useDeposit(insufficientAmount) { + vm.assume(amount > 0); vm.assume(insufficientAmount < amount); vm.startPrank(users.verifier); @@ -162,7 +132,6 @@ contract GraphEscrowCollectTest is GraphEscrowTest { vm.assume(amount > 1 ether); resetPrank(users.gateway); - escrow.approveCollector(users.verifier, amount); _depositTokens(users.verifier, users.indexer, amount); resetPrank(users.verifier); @@ -181,7 +150,6 @@ contract GraphEscrowCollectTest is GraphEscrowTest { vm.assume(amount <= MAX_STAKING_TOKENS); resetPrank(users.gateway); - escrow.approveCollector(users.verifier, amount); _depositTokens(users.verifier, users.indexer, amount); resetPrank(users.verifier); diff --git a/packages/horizon/test/escrow/collector.t.sol b/packages/horizon/test/escrow/collector.t.sol deleted file mode 100644 index d6cb3bc0f..000000000 --- a/packages/horizon/test/escrow/collector.t.sol +++ /dev/null @@ -1,108 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.27; - -import "forge-std/Test.sol"; - -import { IPaymentsEscrow } from "../../contracts/interfaces/IPaymentsEscrow.sol"; - -import { GraphEscrowTest } from "./GraphEscrow.t.sol"; - -contract GraphEscrowCollectorTest is GraphEscrowTest { - - /* - * HELPERS - */ - - function _thawCollector() internal { - (uint256 beforeAllowance,) = escrow.authorizedCollectors(users.gateway, users.verifier); - vm.expectEmit(address(escrow)); - emit IPaymentsEscrow.ThawCollector(users.gateway, users.verifier); - escrow.thawCollector(users.verifier); - - (uint256 allowance, uint256 thawEndTimestamp) = escrow.authorizedCollectors(users.gateway, users.verifier); - assertEq(allowance, beforeAllowance); - assertEq(thawEndTimestamp, block.timestamp + revokeCollectorThawingPeriod); - } - - function _cancelThawCollector() internal { - (uint256 beforeAllowance, uint256 beforeThawEndTimestamp) = escrow.authorizedCollectors(users.gateway, users.verifier); - assertTrue(beforeThawEndTimestamp != 0, "Collector should be thawing"); - vm.expectEmit(address(escrow)); - emit IPaymentsEscrow.CancelThawCollector(users.gateway, users.verifier); - escrow.cancelThawCollector(users.verifier); - - (uint256 allowance, uint256 thawEndTimestamp) = escrow.authorizedCollectors(users.gateway, users.verifier); - assertEq(allowance, beforeAllowance); - assertEq(thawEndTimestamp, 0); - } - - function _revokeCollector() internal { - vm.expectEmit(address(escrow)); - emit IPaymentsEscrow.RevokeCollector(users.gateway, users.verifier); - escrow.revokeCollector(users.verifier); - - (uint256 allowance, uint256 thawEndTimestamp) = escrow.authorizedCollectors(users.gateway, users.verifier); - assertEq(allowance, 0); - assertEq(thawEndTimestamp, 0); - } - - /* - * TESTS - */ - - function testCollector_Approve( - uint256 tokens, - uint256 approveSteps - ) public useGateway { - approveSteps = bound(approveSteps, 1, 100); - vm.assume(tokens > approveSteps); - - uint256 approveTokens = tokens / approveSteps; - for (uint i = 0; i < approveSteps; i++) { - _approveCollector(users.verifier, approveTokens); - } - } - - function testCollector_RevertWhen_ApprovingForZeroAllowance( - uint256 amount - ) public useGateway useCollector(amount) { - bytes memory expectedError = abi.encodeWithSelector(IPaymentsEscrow.PaymentsEscrowInvalidZeroTokens.selector); - vm.expectRevert(expectedError); - escrow.approveCollector(users.verifier, 0); - } - - function testCollector_Thaw(uint256 amount) public useGateway useCollector(amount) { - _thawCollector(); - } - - function testCollector_CancelThaw(uint256 amount) public useGateway useCollector(amount) { - _thawCollector(); - _cancelThawCollector(); - } - - function testCollector_RevertWhen_CancelThawIsNotThawing(uint256 amount) public useGateway useCollector(amount) { - bytes memory expectedError = abi.encodeWithSignature("PaymentsEscrowNotThawing()"); - vm.expectRevert(expectedError); - escrow.cancelThawCollector(users.verifier); - vm.stopPrank(); - } - - function testCollector_Revoke(uint256 amount) public useGateway useCollector(amount) { - _thawCollector(); - skip(revokeCollectorThawingPeriod + 1); - _revokeCollector(); - } - - function testCollector_RevertWhen_RevokeIsNotThawing(uint256 amount) public useGateway useCollector(amount) { - bytes memory expectedError = abi.encodeWithSignature("PaymentsEscrowNotThawing()"); - vm.expectRevert(expectedError); - escrow.revokeCollector(users.verifier); - } - - function testCollector_RevertWhen_RevokeIsStillThawing(uint256 amount) public useGateway useCollector(amount) { - escrow.thawCollector(users.verifier); - bytes memory expectedError = abi.encodeWithSignature("PaymentsEscrowStillThawing(uint256,uint256)", block.timestamp, block.timestamp + revokeCollectorThawingPeriod); - vm.expectRevert(expectedError); - escrow.revokeCollector(users.verifier); - } -} \ No newline at end of file diff --git a/packages/horizon/test/escrow/paused.t.sol b/packages/horizon/test/escrow/paused.t.sol index a993da883..6019b5c15 100644 --- a/packages/horizon/test/escrow/paused.t.sol +++ b/packages/horizon/test/escrow/paused.t.sol @@ -63,26 +63,4 @@ contract GraphEscrowPausedTest is GraphEscrowTest { vm.expectRevert(abi.encodeWithSelector(IPaymentsEscrow.PaymentsEscrowIsPaused.selector)); escrow.collect(IGraphPayments.PaymentTypes.QueryFee, users.gateway, users.indexer, tokens, subgraphDataServiceAddress, tokensDataService); } - - // Collectors - - function testPaused_RevertWhen_ApproveCollector(uint256 tokens) public useGateway usePaused(true) { - vm.expectRevert(abi.encodeWithSelector(IPaymentsEscrow.PaymentsEscrowIsPaused.selector)); - escrow.approveCollector(users.verifier, tokens); - } - - function testPaused_RevertWhen_ThawCollector(uint256 tokens) public useGateway useCollector(tokens) usePaused(true) { - vm.expectRevert(abi.encodeWithSelector(IPaymentsEscrow.PaymentsEscrowIsPaused.selector)); - escrow.thawCollector(users.verifier); - } - - function testPaused_RevertWhen_CancelThawCollector(uint256 tokens) public useGateway useCollector(tokens) usePaused(true) { - vm.expectRevert(abi.encodeWithSelector(IPaymentsEscrow.PaymentsEscrowIsPaused.selector)); - escrow.cancelThawCollector(users.verifier); - } - - function testPaused_RevertWhen_RevokeCollector(uint256 tokens) public useGateway useCollector(tokens) usePaused(true) { - vm.expectRevert(abi.encodeWithSelector(IPaymentsEscrow.PaymentsEscrowIsPaused.selector)); - escrow.revokeCollector(users.verifier); - } } \ No newline at end of file diff --git a/packages/horizon/test/payments/tap-collector/collect/collect.t.sol b/packages/horizon/test/payments/tap-collector/collect/collect.t.sol index 555c9c50e..ecaa1ccec 100644 --- a/packages/horizon/test/payments/tap-collector/collect/collect.t.sol +++ b/packages/horizon/test/payments/tap-collector/collect/collect.t.sol @@ -51,7 +51,6 @@ contract TAPCollectorCollectTest is TAPCollectorTest { ) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner { tokens = bound(tokens, 1, type(uint128).max); - _approveCollector(address(tapCollector), tokens); _depositTokens(address(tapCollector), users.indexer, tokens); bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens)); @@ -67,7 +66,6 @@ contract TAPCollectorCollectTest is TAPCollectorTest { steps = uint8(bound(steps, 1, 100)); tokens = bound(tokens, steps, type(uint128).max); - _approveCollector(address(tapCollector), tokens); _depositTokens(address(tapCollector), users.indexer, tokens); resetPrank(users.verifier); @@ -88,7 +86,6 @@ contract TAPCollectorCollectTest is TAPCollectorTest { function testTAPCollector_Collect_RevertWhen_NoProvision(uint256 tokens) public useGateway useSigner { tokens = bound(tokens, 1, type(uint128).max); - _approveCollector(address(tapCollector), tokens); _depositTokens(address(tapCollector), users.indexer, tokens); bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens)); @@ -112,7 +109,6 @@ contract TAPCollectorCollectTest is TAPCollectorTest { tokens = bound(tokens, 1, type(uint128).max); resetPrank(users.gateway); - _approveCollector(address(tapCollector), tokens); _depositTokens(address(tapCollector), users.indexer, tokens); bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens)); @@ -132,7 +128,6 @@ contract TAPCollectorCollectTest is TAPCollectorTest { tokens = bound(tokens, 1, type(uint128).max); resetPrank(users.gateway); - _approveCollector(address(tapCollector), tokens); _depositTokens(address(tapCollector), users.indexer, tokens); // The sender authorizes another signer @@ -163,7 +158,6 @@ contract TAPCollectorCollectTest is TAPCollectorTest { tokens = bound(tokens, 1, type(uint128).max); resetPrank(users.gateway); - _approveCollector(address(tapCollector), tokens); _depositTokens(address(tapCollector), users.indexer, tokens); bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens)); @@ -183,7 +177,6 @@ contract TAPCollectorCollectTest is TAPCollectorTest { ) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner { tokens = bound(tokens, 1, type(uint128).max); - _approveCollector(address(tapCollector), tokens); _depositTokens(address(tapCollector), users.indexer, tokens); bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens)); @@ -200,7 +193,6 @@ contract TAPCollectorCollectTest is TAPCollectorTest { function testTAPCollector_Collect_RevertWhen_SignerNotAuthorized(uint256 tokens) public useGateway { tokens = bound(tokens, 1, type(uint128).max); - _approveCollector(address(tapCollector), tokens); _depositTokens(address(tapCollector), users.indexer, tokens); bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens)); @@ -215,7 +207,6 @@ contract TAPCollectorCollectTest is TAPCollectorTest { ) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner { tokens = bound(tokens, 1, type(uint128).max); - _approveCollector(address(tapCollector), tokens); _depositTokens(address(tapCollector), users.indexer, tokens); // Start thawing signer @@ -231,7 +222,6 @@ contract TAPCollectorCollectTest is TAPCollectorTest { function testTAPCollector_Collect_RevertIf_SignerWasRevoked(uint256 tokens) public useGateway useSigner { tokens = bound(tokens, 1, type(uint128).max); - _approveCollector(address(tapCollector), tokens); _depositTokens(address(tapCollector), users.indexer, tokens); // Start thawing signer @@ -251,7 +241,6 @@ contract TAPCollectorCollectTest is TAPCollectorTest { ) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner { tokens = bound(tokens, 1, type(uint128).max); - _approveCollector(address(tapCollector), tokens); _depositTokens(address(tapCollector), users.indexer, tokens); // Start thawing signer diff --git a/packages/horizon/test/shared/payments-escrow/PaymentsEscrowShared.t.sol b/packages/horizon/test/shared/payments-escrow/PaymentsEscrowShared.t.sol index 2bc435f7a..c3714dfd6 100644 --- a/packages/horizon/test/shared/payments-escrow/PaymentsEscrowShared.t.sol +++ b/packages/horizon/test/shared/payments-escrow/PaymentsEscrowShared.t.sol @@ -22,22 +22,6 @@ abstract contract PaymentsEscrowSharedTest is GraphBaseTest { * HELPERS */ - function _approveCollector(address _verifier, uint256 _tokens) internal { - (, address msgSender, ) = vm.readCallers(); - (uint256 beforeAllowance,) = escrow.authorizedCollectors(msgSender, _verifier); - vm.expectEmit(address(escrow)); - emit IPaymentsEscrow.AuthorizedCollector( - msgSender, // payer - _verifier, // collector - _tokens, // addedAllowance - beforeAllowance + _tokens // newTotalAllowance after the added allowance - ); - escrow.approveCollector(_verifier, _tokens); - (uint256 allowance, uint256 thawEndTimestamp) = escrow.authorizedCollectors(msgSender, _verifier); - assertEq(allowance - beforeAllowance, _tokens); - assertEq(thawEndTimestamp, 0); - } - function _depositTokens(address _collector, address _receiver, uint256 _tokens) internal { (, address msgSender, ) = vm.readCallers(); (uint256 escrowBalanceBefore,,) = escrow.escrowAccounts(msgSender, _collector, _receiver); diff --git a/packages/horizon/test/utils/Constants.sol b/packages/horizon/test/utils/Constants.sol index cd5cc2bfb..e9ad5c2e9 100644 --- a/packages/horizon/test/utils/Constants.sol +++ b/packages/horizon/test/utils/Constants.sol @@ -7,7 +7,6 @@ abstract contract Constants { uint256 internal constant MAX_STAKING_TOKENS = 10_000_000_000 ether; // GraphEscrow parameters uint256 internal constant withdrawEscrowThawingPeriod = 60; - uint256 internal constant revokeCollectorThawingPeriod = 60; // GraphPayments parameters uint256 internal constant protocolPaymentCut = 10000; // Staking constants diff --git a/packages/subgraph-service/test/SubgraphBaseTest.t.sol b/packages/subgraph-service/test/SubgraphBaseTest.t.sol index 099164473..663442e1f 100644 --- a/packages/subgraph-service/test/SubgraphBaseTest.t.sol +++ b/packages/subgraph-service/test/SubgraphBaseTest.t.sol @@ -113,7 +113,6 @@ abstract contract SubgraphBaseTest is Utils, Constants { vm.getCode("PaymentsEscrow.sol:PaymentsEscrow"), abi.encode( address(controller), - revokeCollectorThawingPeriod, withdrawEscrowThawingPeriod ) )); @@ -174,7 +173,6 @@ abstract contract SubgraphBaseTest is Utils, Constants { ); escrow = new PaymentsEscrow{salt: saltEscrow}( address(controller), - revokeCollectorThawingPeriod, withdrawEscrowThawingPeriod ); diff --git a/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol b/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol index b417f30bf..64842b1f2 100644 --- a/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol +++ b/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol @@ -232,7 +232,7 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest { ITAPCollector.SignedRAV memory signedRav = abi.decode(_data, (ITAPCollector.SignedRAV)); allocationId = abi.decode(signedRav.rav.metadata, (address)); allocation = subgraphService.getAllocation(allocationId); - (address payer, ) = tapCollector.authorizedSigners(_recoverRAVSigner(signedRav)); + (address payer, , ) = tapCollector.authorizedSigners(_recoverRAVSigner(signedRav)); // Total amount of tokens collected for indexer uint256 tokensCollected = tapCollector.tokensCollected(address(subgraphService), _indexer, payer); diff --git a/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol b/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol index 93972679f..65d7e23d7 100644 --- a/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol +++ b/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol @@ -54,8 +54,7 @@ contract SubgraphServiceRegisterTest is SubgraphServiceTest { }); } - function _approveCollector(uint256 tokens) private { - escrow.approveCollector(address(tapCollector), tokens); + function _deposit(uint256 tokens) private { token.approve(address(escrow), tokens); escrow.deposit(address(tapCollector), users.indexer, tokens); } @@ -91,7 +90,7 @@ contract SubgraphServiceRegisterTest is SubgraphServiceTest { tokensPayment = bound(tokensPayment, minimumProvisionTokens, maxTokensPayment); resetPrank(users.gateway); - _approveCollector(tokensPayment); + _deposit(tokensPayment); _authorizeSigner(); resetPrank(users.indexer); @@ -108,7 +107,7 @@ contract SubgraphServiceRegisterTest is SubgraphServiceTest { uint256 tokensPayment = tokensAllocated / stakeToFeesRatio / numPayments; resetPrank(users.gateway); - _approveCollector(tokensAllocated); + _deposit(tokensAllocated); _authorizeSigner(); resetPrank(users.indexer); diff --git a/packages/subgraph-service/test/utils/Constants.sol b/packages/subgraph-service/test/utils/Constants.sol index 76f864da1..025396ea8 100644 --- a/packages/subgraph-service/test/utils/Constants.sol +++ b/packages/subgraph-service/test/utils/Constants.sol @@ -21,7 +21,6 @@ abstract contract Constants { uint64 internal constant MAX_WAIT_PERIOD = 28 days; // GraphEscrow parameters uint256 internal constant withdrawEscrowThawingPeriod = 60; - uint256 internal constant revokeCollectorThawingPeriod = 60; // GraphPayments parameters uint256 internal constant protocolPaymentCut = 10000; // RewardsMananger parameters