From 89ac2f9d518a69f975fb2febf3ec66cadecb4e74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Wed, 11 Dec 2024 11:13:30 -0300 Subject: [PATCH 1/9] feat: extract cancel thaw escrow to its own function 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 | 17 ++++++-- .../contracts/payments/PaymentsEscrow.sol | 29 +++++++------ .../horizon/test/escrow/GraphEscrow.t.sol | 13 ++++++ packages/horizon/test/escrow/thaw.t.sol | 43 +++++++++++++------ 4 files changed, 75 insertions(+), 27 deletions(-) diff --git a/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol b/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol index 760a086a7..1c44c4e42 100644 --- a/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol +++ b/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol @@ -38,8 +38,10 @@ interface IPaymentsEscrow { * @notice Emitted when a payer cancels an escrow thawing * @param payer The address of the payer * @param receiver The address of the receiver + * @param tokensThawing The amount of tokens that were being thawed + * @param thawEndTimestamp The timestamp at which the thawing period was ending */ - event CancelThaw(address indexed payer, address indexed receiver); + event CancelThaw(address indexed payer, address indexed receiver, uint256 tokensThawing, uint256 thawEndTimestamp); /** * @notice Emitted when a payer thaws funds from the escrow for a payer-collector-receiver tuple @@ -145,13 +147,12 @@ interface IPaymentsEscrow { /** * @notice Thaw a specific amount of escrow from a payer-collector-receiver's escrow account. * The payer is the transaction caller. - * If `tokens` is zero and funds were already thawing it will cancel the thawing. * Note that repeated calls to this function will overwrite the previous thawing amount * and reset the thawing period. * @dev Requirements: * - `tokens` must be less than or equal to the available balance * - * Emits a {Thaw} event. If `tokens` is zero it will emit a {CancelThaw} event. + * Emits a {Thaw} event. * * @param collector The address of the collector * @param receiver The address of the receiver @@ -159,6 +160,16 @@ interface IPaymentsEscrow { */ function thaw(address collector, address receiver, uint256 tokens) external; + /** + * @notice Cancels the thawing of escrow from a payer-collector-receiver's escrow account. + * @param collector The address of the collector + * @param receiver The address of the receiver + * @dev Requirements: + * - The payer must be thawing funds + * Emits a {CancelThaw} event. + */ + function cancelThaw(address collector, address receiver) external; + /** * @notice Withdraws all thawed escrow from a payer-collector-receiver's escrow account. * The payer is the transaction caller. diff --git a/packages/horizon/contracts/payments/PaymentsEscrow.sol b/packages/horizon/contracts/payments/PaymentsEscrow.sol index 6f4252873..5e2408365 100644 --- a/packages/horizon/contracts/payments/PaymentsEscrow.sol +++ b/packages/horizon/contracts/payments/PaymentsEscrow.sol @@ -78,19 +78,9 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, * @notice See {IPaymentsEscrow-thaw} */ function thaw(address collector, address receiver, uint256 tokens) external override notPaused { - EscrowAccount storage account = escrowAccounts[msg.sender][collector][receiver]; - - // if amount thawing is zero and requested amount is zero this is an invalid request. - // otherwise if amount thawing is greater than zero and requested amount is zero this - // is a cancel thaw request. - if (tokens == 0) { - require(account.tokensThawing != 0, PaymentsEscrowNotThawing()); - account.tokensThawing = 0; - account.thawEndTimestamp = 0; - emit CancelThaw(msg.sender, receiver); - return; - } + require(tokens > 0, PaymentsEscrowInvalidZeroTokens()); + EscrowAccount storage account = escrowAccounts[msg.sender][collector][receiver]; require(account.balance >= tokens, PaymentsEscrowInsufficientBalance(account.balance, tokens)); account.tokensThawing = tokens; @@ -99,6 +89,21 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, emit Thaw(msg.sender, collector, receiver, tokens, account.thawEndTimestamp); } + /** + * @notice See {IPaymentsEscrow-cancelThaw} + */ + function cancelThaw(address collector, address receiver) external override notPaused { + EscrowAccount storage account = escrowAccounts[msg.sender][collector][receiver]; + require(account.tokensThawing != 0, PaymentsEscrowNotThawing()); + + uint256 tokensThawing = account.tokensThawing; + uint256 thawEndTimestamp = account.thawEndTimestamp; + account.tokensThawing = 0; + account.thawEndTimestamp = 0; + + emit CancelThaw(msg.sender, receiver, tokensThawing, thawEndTimestamp); + } + /** * @notice See {IPaymentsEscrow-withdraw} */ diff --git a/packages/horizon/test/escrow/GraphEscrow.t.sol b/packages/horizon/test/escrow/GraphEscrow.t.sol index 120713c6c..036772887 100644 --- a/packages/horizon/test/escrow/GraphEscrow.t.sol +++ b/packages/horizon/test/escrow/GraphEscrow.t.sol @@ -56,6 +56,19 @@ contract GraphEscrowTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest { assertEq(thawEndTimestamp, expectedThawEndTimestamp); } + function _cancelThawEscrow(address collector, address receiver) internal { + (, address msgSender, ) = vm.readCallers(); + (, uint256 amountThawingBefore, uint256 thawEndTimestampBefore) = escrow.escrowAccounts(msgSender, collector, receiver); + + vm.expectEmit(address(escrow)); + emit IPaymentsEscrow.CancelThaw(msgSender, receiver, amountThawingBefore, thawEndTimestampBefore); + escrow.cancelThaw(collector, receiver); + + (, uint256 amountThawing, uint256 thawEndTimestamp) = escrow.escrowAccounts(msgSender, collector, receiver); + assertEq(amountThawing, 0); + assertEq(thawEndTimestamp, 0); + } + struct CollectPaymentData { uint256 escrowBalance; uint256 paymentsBalance; diff --git a/packages/horizon/test/escrow/thaw.t.sol b/packages/horizon/test/escrow/thaw.t.sol index 017c3291f..e8e915b22 100644 --- a/packages/horizon/test/escrow/thaw.t.sol +++ b/packages/horizon/test/escrow/thaw.t.sol @@ -6,19 +6,32 @@ import "forge-std/Test.sol"; import { GraphEscrowTest } from "./GraphEscrow.t.sol"; contract GraphEscrowThawTest is GraphEscrowTest { - /* * TESTS */ function testThaw_Tokens(uint256 amount) public useGateway useDeposit(amount) { + amount = bound(amount, 1, type(uint256).max); _thawEscrow(users.verifier, users.indexer, amount); } - function testThaw_RevertWhen_InsufficientThawAmount( - uint256 amount - ) public useGateway useDeposit(amount) { - bytes memory expectedError = abi.encodeWithSignature("PaymentsEscrowNotThawing()"); + function testThaw_Tokens_SuccesiveCalls(uint256 amount) public useGateway { + amount = bound(amount, 2, type(uint256).max - 10); + _depositTokens(users.verifier, users.indexer, amount); + + uint256 firstAmountToThaw = (amount + 2 - 1) / 2; + uint256 secondAmountToThaw = (amount + 10 - 1) / 10; + _thawEscrow(users.verifier, users.indexer, firstAmountToThaw); + _thawEscrow(users.verifier, users.indexer, secondAmountToThaw); + + (, address msgSender, ) = vm.readCallers(); + (, uint256 amountThawing, uint256 thawEndTimestamp) = escrow.escrowAccounts(msgSender, users.verifier, users.indexer); + assertEq(amountThawing, secondAmountToThaw); + assertEq(thawEndTimestamp, block.timestamp + withdrawEscrowThawingPeriod); + } + + function testThaw_Tokens_RevertWhen_AmountIsZero() public useGateway { + bytes memory expectedError = abi.encodeWithSignature("PaymentsEscrowInvalidZeroTokens()"); vm.expectRevert(expectedError); escrow.thaw(users.verifier, users.indexer, 0); } @@ -28,17 +41,23 @@ contract GraphEscrowThawTest is GraphEscrowTest { uint256 overAmount ) public useGateway useDeposit(amount) { overAmount = bound(overAmount, amount + 1, type(uint256).max); - bytes memory expectedError = abi.encodeWithSignature("PaymentsEscrowInsufficientBalance(uint256,uint256)", amount, overAmount); + bytes memory expectedError = abi.encodeWithSignature( + "PaymentsEscrowInsufficientBalance(uint256,uint256)", + amount, + overAmount + ); vm.expectRevert(expectedError); escrow.thaw(users.verifier, users.indexer, overAmount); } function testThaw_CancelRequest(uint256 amount) public useGateway useDeposit(amount) { - escrow.thaw(users.verifier, users.indexer, amount); - escrow.thaw(users.verifier, users.indexer, 0); + _thawEscrow(users.verifier, users.indexer, amount); + _cancelThawEscrow(users.verifier, users.indexer); + } - (, uint256 amountThawing,uint256 thawEndTimestamp) = escrow.escrowAccounts(users.gateway, users.verifier, users.indexer); - assertEq(amountThawing, 0); - assertEq(thawEndTimestamp, 0); + function testThaw_CancelRequest_RevertWhen_NoThawing(uint256 amount) public useGateway useDeposit(amount) { + bytes memory expectedError = abi.encodeWithSignature("PaymentsEscrowNotThawing()"); + vm.expectRevert(expectedError); + escrow.cancelThaw(users.verifier, users.indexer); } -} \ No newline at end of file +} From 3e361a37a41a5853574c06824fbd9df241c7a60c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Wed, 11 Dec 2024 14:18:18 -0300 Subject: [PATCH 2/9] feat: add collector address to the RAV MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .../contracts/interfaces/ITAPCollector.sol | 13 +++++++-- .../payments/collectors/TAPCollector.sol | 14 +++++++--- .../tap-collector/collect/collect.t.sol | 27 +++++++++++++++++-- .../subgraphService/collect/query/query.t.sol | 3 ++- 4 files changed, 49 insertions(+), 8 deletions(-) diff --git a/packages/horizon/contracts/interfaces/ITAPCollector.sol b/packages/horizon/contracts/interfaces/ITAPCollector.sol index f9c482308..47c124b58 100644 --- a/packages/horizon/contracts/interfaces/ITAPCollector.sol +++ b/packages/horizon/contracts/interfaces/ITAPCollector.sol @@ -29,10 +29,12 @@ interface ITAPCollector is IPaymentsCollector { bytes32 collectionId; // The address of the payer the RAV was issued by address payer; - // The address of the data service the RAV was issued to - address dataService; + // The address of the collector where the RAV can be collected + address collector; // The address of the service provider the RAV was issued to address serviceProvider; + // The address of the data service the RAV was issued to + address dataService; // The RAV timestamp, indicating the latest TAP Receipt in the RAV uint64 timestampNs; // Total amount owed to the service provider since the beginning of the @@ -102,6 +104,13 @@ interface ITAPCollector is IPaymentsCollector { bytes signature ); + /** + * Thrown when attempting to collect a RAV that was not issued to this collector + * @param collector The address of this collector processing the RAV + * @param ravCollector The collector address noted in the RAV + */ + error TAPCollectorInvalidCollector(address collector, address ravCollector); + /** * Thrown when the signer is already authorized * @param authorizingPayer The address of the payer authorizing the signer diff --git a/packages/horizon/contracts/payments/collectors/TAPCollector.sol b/packages/horizon/contracts/payments/collectors/TAPCollector.sol index 426f84a15..63ec9f5db 100644 --- a/packages/horizon/contracts/payments/collectors/TAPCollector.sol +++ b/packages/horizon/contracts/payments/collectors/TAPCollector.sol @@ -29,7 +29,7 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector { /// @notice The EIP712 typehash for the ReceiptAggregateVoucher struct bytes32 private constant EIP712_RAV_TYPEHASH = keccak256( - "ReceiptAggregateVoucher(address payer,address dataService,address serviceProvider,uint64 timestampNs,uint128 valueAggregate,bytes metadata)" + "ReceiptAggregateVoucher(address payer,address collector,address serviceProvider,address dataService,uint64 timestampNs,uint128 valueAggregate,bytes metadata)" ); /// @notice Authorization details for payer-signer pairs @@ -166,8 +166,15 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector { bytes memory _data, uint256 _tokensToCollect ) private returns (uint256) { - // Ensure caller is the RAV data service (SignedRAV memory signedRAV, uint256 dataServiceCut) = abi.decode(_data, (SignedRAV, uint256)); + + // Ensure the RAV was issued to this collector + require( + signedRAV.rav.collector == address(this), + TAPCollectorInvalidCollector(address(this), signedRAV.rav.collector) + ); + + // Ensure caller is the RAV data service require( signedRAV.rav.dataService == msg.sender, TAPCollectorCallerNotDataService(msg.sender, signedRAV.rav.dataService) @@ -259,8 +266,9 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector { abi.encode( EIP712_RAV_TYPEHASH, _rav.payer, - _rav.dataService, + _rav.collector, _rav.serviceProvider, + _rav.dataService, _rav.timestampNs, _rav.valueAggregate, keccak256(_rav.metadata) 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 db1c994e4..646401ca8 100644 --- a/packages/horizon/test/payments/tap-collector/collect/collect.t.sol +++ b/packages/horizon/test/payments/tap-collector/collect/collect.t.sol @@ -42,15 +42,17 @@ contract TAPCollectorCollectTest is TAPCollectorTest { function _getRAV( address _allocationId, address _payer, - address _indexer, address _collector, + address _indexer, + address _dataService, uint128 _tokens ) private pure returns (ITAPCollector.ReceiptAggregateVoucher memory rav) { return ITAPCollector.ReceiptAggregateVoucher({ collectionId: bytes32(uint256(uint160(_allocationId))), payer: _payer, - dataService: _collector, + collector: _collector, + dataService: _dataService, serviceProvider: _indexer, timestampNs: 0, valueAggregate: _tokens, @@ -109,6 +111,27 @@ contract TAPCollectorCollectTest is TAPCollectorTest { } } + function testTAPCollector_Collect_RevertWhen_OtherCollector() public useGateway useSigner { + address otherCollector = makeAddr("otherCollector"); + bytes memory data = _getQueryFeeEncodedData( + signerPrivateKey, + users.gateway, + otherCollector, + users.indexer, + users.verifier, + uint128(0) + ); + + resetPrank(users.verifier); + bytes memory expectedError = abi.encodeWithSelector( + ITAPCollector.TAPCollectorInvalidCollector.selector, + address(tapCollector), + otherCollector + ); + vm.expectRevert(expectedError); + tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data); + } + function testTAPCollector_Collect_RevertWhen_NoProvision(uint256 tokens) public useGateway useSigner { tokens = bound(tokens, 1, type(uint128).max); 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 d6271cd46..1c629439f 100644 --- a/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol +++ b/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol @@ -49,8 +49,9 @@ contract SubgraphServiceRegisterTest is SubgraphServiceTest { ITAPCollector.ReceiptAggregateVoucher({ collectionId: collectionId, payer: users.gateway, - dataService: address(subgraphService), + collector: address(tapCollector), serviceProvider: indexer, + dataService: address(subgraphService), timestampNs: 0, valueAggregate: tokens, metadata: "" From f51de325a6ddd0fe573a9fd189568856c24a8bce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Fri, 13 Dec 2024 14:23:16 -0300 Subject: [PATCH 3/9] test: improve coverage for GraphPayments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .gitignore | 2 + .../contracts/interfaces/IGraphPayments.sol | 10 +- .../contracts/payments/GraphPayments.sol | 18 +- packages/horizon/test/GraphBase.t.sol | 77 +++++--- .../horizon/test/escrow/GraphEscrow.t.sol | 10 +- packages/horizon/test/escrow/collect.t.sol | 89 +++------ .../horizon/test/payments/GraphPayments.t.sol | 178 +++++++++++++----- 7 files changed, 226 insertions(+), 158 deletions(-) diff --git a/.gitignore b/.gitignore index 89efef0a5..5861633e5 100644 --- a/.gitignore +++ b/.gitignore @@ -34,8 +34,10 @@ bin/ .vscode # Coverage and other reports +coverage/ reports/ coverage.json +lcov.info # Local test files addresses-local.json diff --git a/packages/horizon/contracts/interfaces/IGraphPayments.sol b/packages/horizon/contracts/interfaces/IGraphPayments.sol index eaac08ae4..8019e8917 100644 --- a/packages/horizon/contracts/interfaces/IGraphPayments.sol +++ b/packages/horizon/contracts/interfaces/IGraphPayments.sol @@ -20,6 +20,7 @@ interface IGraphPayments { /** * @notice Emitted when a payment is collected + * @param paymentType The type of payment as defined in {IGraphPayments} * @param payer The address of the payer * @param receiver The address of the receiver * @param dataService The address of the data service @@ -30,6 +31,7 @@ interface IGraphPayments { * @param tokensReceiver Amount of tokens for the receiver */ event GraphPaymentCollected( + PaymentTypes paymentType, address indexed payer, address indexed receiver, address indexed dataService, @@ -40,14 +42,6 @@ interface IGraphPayments { uint256 tokensReceiver ); - /** - * @notice Thrown when the calculated amount of tokens to be paid out to all parties is - * not the same as the amount of tokens being collected - * @param tokens The amount of tokens being collected - * @param tokensCalculated The sum of all the tokens to be paid out - */ - error GraphPaymentsBadAccounting(uint256 tokens, uint256 tokensCalculated); - /** * @notice Thrown when the protocol payment cut is invalid * @param protocolPaymentCut The protocol payment cut diff --git a/packages/horizon/contracts/payments/GraphPayments.sol b/packages/horizon/contracts/payments/GraphPayments.sol index 0dd06ef72..57243c555 100644 --- a/packages/horizon/contracts/payments/GraphPayments.sol +++ b/packages/horizon/contracts/payments/GraphPayments.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.27; import { IGraphToken } from "@graphprotocol/contracts/contracts/token/IGraphToken.sol"; import { IGraphPayments } from "../interfaces/IGraphPayments.sol"; +import { IHorizonStakingTypes } from "../interfaces/internal/IHorizonStakingTypes.sol"; import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import { MulticallUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/MulticallUpgradeable.sol"; @@ -70,14 +71,14 @@ contract GraphPayments is Initializable, MulticallUpgradeable, GraphDirectory, I uint256 tokensDataService = tokensRemaining.mulPPMRoundUp(dataServiceCut); tokensRemaining = tokensRemaining - tokensDataService; - uint256 tokensDelegationPool = tokensRemaining.mulPPMRoundUp( - _graphStaking().getDelegationFeeCut(receiver, dataService, paymentType) - ); - tokensRemaining = tokensRemaining - tokensDelegationPool; - - // Ensure accounting is correct - uint256 tokensTotal = tokensProtocol + tokensDataService + tokensDelegationPool + tokensRemaining; - require(tokens == tokensTotal, GraphPaymentsBadAccounting(tokens, tokensTotal)); + uint256 tokensDelegationPool = 0; + IHorizonStakingTypes.DelegationPool memory pool = _graphStaking().getDelegationPool(receiver, dataService); + if (pool.shares > 0) { + tokensDelegationPool = tokensRemaining.mulPPMRoundUp( + _graphStaking().getDelegationFeeCut(receiver, dataService, paymentType) + ); + tokensRemaining = tokensRemaining - tokensDelegationPool; + } // Pay all parties _graphToken().burnTokens(tokensProtocol); @@ -92,6 +93,7 @@ contract GraphPayments is Initializable, MulticallUpgradeable, GraphDirectory, I _graphToken().pushTokens(receiver, tokensRemaining); emit GraphPaymentCollected( + paymentType, msg.sender, receiver, dataService, diff --git a/packages/horizon/test/GraphBase.t.sol b/packages/horizon/test/GraphBase.t.sol index b2eef0dd9..a01fb2958 100644 --- a/packages/horizon/test/GraphBase.t.sol +++ b/packages/horizon/test/GraphBase.t.sol @@ -41,7 +41,7 @@ abstract contract GraphBaseTest is IHorizonStakingTypes, Utils, Constants { RewardsManagerMock public rewardsManager; CurationMock public curation; TAPCollector tapCollector; - + HorizonStaking private stakingBase; HorizonStakingExtension private stakingExtension; @@ -103,22 +103,34 @@ abstract contract GraphBaseTest is IHorizonStakingTypes, Utils, Constants { GraphProxy stakingProxy = new GraphProxy(address(0), address(proxyAdmin)); // GraphPayments predict address - bytes memory paymentsParameters = abi.encode(address(controller), protocolPaymentCut); - bytes memory paymentsBytecode = abi.encodePacked( + bytes memory paymentsImplementationParameters = abi.encode(address(controller), protocolPaymentCut); + bytes memory paymentsImplementationBytecode = abi.encodePacked( type(GraphPayments).creationCode, - paymentsParameters + paymentsImplementationParameters ); - address predictedPaymentsAddress = _computeAddress( + address predictedPaymentsImplementationAddress = _computeAddress( "GraphPayments", - paymentsBytecode, + paymentsImplementationBytecode, users.deployer ); - - // PaymentsEscrow - bytes memory escrowImplementationParameters = abi.encode( - address(controller), - withdrawEscrowThawingPeriod + + bytes memory paymentsProxyParameters = abi.encode( + predictedPaymentsImplementationAddress, + users.governor, + abi.encodeCall(GraphPayments.initialize, ()) + ); + bytes memory paymentsProxyBytecode = abi.encodePacked( + type(TransparentUpgradeableProxy).creationCode, + paymentsProxyParameters + ); + address predictedPaymentsProxyAddress = _computeAddress( + "TransparentUpgradeableProxy", + paymentsProxyBytecode, + users.deployer ); + + // PaymentsEscrow + bytes memory escrowImplementationParameters = abi.encode(address(controller), withdrawEscrowThawingPeriod); bytes memory escrowImplementationBytecode = abi.encodePacked( type(PaymentsEscrow).creationCode, escrowImplementationParameters @@ -157,29 +169,32 @@ abstract contract GraphBaseTest is IHorizonStakingTypes, Utils, Constants { resetPrank(users.governor); controller.setContractProxy(keccak256("GraphToken"), address(token)); controller.setContractProxy(keccak256("PaymentsEscrow"), predictedEscrowProxyAddress); - controller.setContractProxy(keccak256("GraphPayments"), predictedPaymentsAddress); + controller.setContractProxy(keccak256("GraphPayments"), predictedPaymentsProxyAddress); controller.setContractProxy(keccak256("Staking"), address(stakingProxy)); controller.setContractProxy(keccak256("EpochManager"), address(epochManager)); controller.setContractProxy(keccak256("RewardsManager"), address(rewardsManager)); controller.setContractProxy(keccak256("Curation"), address(curation)); controller.setContractProxy(keccak256("GraphTokenGateway"), graphTokenGatewayAddress); controller.setContractProxy(keccak256("GraphProxyAdmin"), address(proxyAdmin)); - - resetPrank(users.deployer); - address paymentsAddress = _deployContract("GraphPayments", paymentsBytecode); - assertEq(paymentsAddress, predictedPaymentsAddress); - payments = GraphPayments(paymentsAddress); - - address escrowImplementationAddress = _deployContract("PaymentsEscrow", escrowImplementationBytecode); - address escrowProxyAddress = _deployContract("TransparentUpgradeableProxy", escrowProxyBytecode); - assertEq(escrowImplementationAddress, predictedEscrowImplementationAddress); - assertEq(escrowProxyAddress, predictedEscrowProxyAddress); - escrow = PaymentsEscrow(escrowProxyAddress); - stakingExtension = new HorizonStakingExtension( - address(controller), - subgraphDataServiceLegacyAddress - ); + resetPrank(users.deployer); + { + address paymentsImplementationAddress = _deployContract("GraphPayments", paymentsImplementationBytecode); + address paymentsProxyAddress = _deployContract("TransparentUpgradeableProxy", paymentsProxyBytecode); + assertEq(paymentsImplementationAddress, predictedPaymentsImplementationAddress); + assertEq(paymentsProxyAddress, predictedPaymentsProxyAddress); + payments = GraphPayments(paymentsProxyAddress); + } + + { + address escrowImplementationAddress = _deployContract("PaymentsEscrow", escrowImplementationBytecode); + address escrowProxyAddress = _deployContract("TransparentUpgradeableProxy", escrowProxyBytecode); + assertEq(escrowImplementationAddress, predictedEscrowImplementationAddress); + assertEq(escrowProxyAddress, predictedEscrowProxyAddress); + escrow = PaymentsEscrow(escrowProxyAddress); + } + + stakingExtension = new HorizonStakingExtension(address(controller), subgraphDataServiceLegacyAddress); stakingBase = new HorizonStaking( address(controller), address(stakingExtension), @@ -229,7 +244,11 @@ abstract contract GraphBaseTest is IHorizonStakingTypes, Utils, Constants { * PRIVATE */ - function _computeAddress(string memory contractName, bytes memory bytecode, address deployer) private pure returns (address) { + function _computeAddress( + string memory contractName, + bytes memory bytecode, + address deployer + ) private pure returns (address) { bytes32 salt = keccak256(abi.encodePacked(contractName, "Salt")); return Create2.computeAddress(salt, keccak256(bytecode), deployer); } @@ -238,4 +257,4 @@ abstract contract GraphBaseTest is IHorizonStakingTypes, Utils, Constants { bytes32 salt = keccak256(abi.encodePacked(contractName, "Salt")); return Create2.deploy(0, salt, bytecode); } -} \ No newline at end of file +} diff --git a/packages/horizon/test/escrow/GraphEscrow.t.sol b/packages/horizon/test/escrow/GraphEscrow.t.sol index 036772887..6bb56c97c 100644 --- a/packages/horizon/test/escrow/GraphEscrow.t.sol +++ b/packages/horizon/test/escrow/GraphEscrow.t.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.27; import "forge-std/Test.sol"; import { IPaymentsEscrow } from "../../contracts/interfaces/IPaymentsEscrow.sol"; import { IGraphPayments } from "../../contracts/interfaces/IGraphPayments.sol"; +import { IHorizonStakingTypes } from "../../contracts/interfaces/internal/IHorizonStakingTypes.sol"; import { HorizonStakingSharedTest } from "../shared/horizon-staking/HorizonStakingShared.t.sol"; import { PaymentsEscrowSharedTest } from "../shared/payments-escrow/PaymentsEscrowShared.t.sol"; @@ -112,9 +113,12 @@ contract GraphEscrowTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest { uint256 tokensDataService = (_tokens - _tokens.mulPPMRoundUp(payments.PROTOCOL_PAYMENT_CUT())).mulPPMRoundUp( _dataServiceCut ); - uint256 tokensDelegation = (_tokens - - _tokens.mulPPMRoundUp(payments.PROTOCOL_PAYMENT_CUT()) - - tokensDataService).mulPPMRoundUp(staking.getDelegationFeeCut(_receiver, _dataService, _paymentType)); + uint256 tokensDelegation = 0; + IHorizonStakingTypes.DelegationPool memory pool = staking.getDelegationPool(_receiver, _dataService); + if (pool.shares > 0) { + tokensDelegation = (_tokens - _tokens.mulPPMRoundUp(payments.PROTOCOL_PAYMENT_CUT()) - tokensDataService) + .mulPPMRoundUp(staking.getDelegationFeeCut(_receiver, _dataService, _paymentType)); + } uint256 receiverExpectedPayment = _tokens - _tokens.mulPPMRoundUp(payments.PROTOCOL_PAYMENT_CUT()) - tokensDataService - diff --git a/packages/horizon/test/escrow/collect.t.sol b/packages/horizon/test/escrow/collect.t.sol index 55f378b3a..3c18c93b6 100644 --- a/packages/horizon/test/escrow/collect.t.sol +++ b/packages/horizon/test/escrow/collect.t.sol @@ -15,6 +15,7 @@ contract GraphEscrowCollectTest is GraphEscrowTest { function testCollect_Tokens( uint256 tokens, + uint256 tokensToCollect, uint256 delegationTokens, uint256 dataServiceCut ) @@ -25,13 +26,43 @@ contract GraphEscrowCollectTest is GraphEscrowTest { { dataServiceCut = bound(dataServiceCut, 0, MAX_PPM); delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); + tokensToCollect = bound(tokensToCollect, 1, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); + resetPrank(users.gateway); + _depositTokens(users.verifier, users.indexer, tokensToCollect); + + // burn some tokens to prevent overflow + resetPrank(users.indexer); + token.burn(MAX_STAKING_TOKENS); + + resetPrank(users.verifier); + _collectEscrow( + IGraphPayments.PaymentTypes.QueryFee, + users.gateway, + users.indexer, + tokensToCollect, + subgraphDataServiceAddress, + dataServiceCut + ); + } + + function testCollect_Tokens_NoProvision( + uint256 tokens, + uint256 dataServiceCut + ) public useIndexer useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) { + dataServiceCut = bound(dataServiceCut, 0, MAX_PPM); + tokens = bound(tokens, 1, MAX_STAKING_TOKENS); + resetPrank(users.gateway); _depositTokens(users.verifier, users.indexer, tokens); + // burn some tokens to prevent overflow + resetPrank(users.indexer); + token.burn(MAX_STAKING_TOKENS); + resetPrank(users.verifier); _collectEscrow( IGraphPayments.PaymentTypes.QueryFee, @@ -67,62 +98,4 @@ contract GraphEscrowCollectTest is GraphEscrowTest { ); vm.stopPrank(); } - - function testCollect_RevertWhen_InvalidPool( - uint256 amount - ) - public - useIndexer - useProvision(amount, 0, 0) - useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) - { - vm.assume(amount > 1 ether); - - resetPrank(users.gateway); - _depositTokens(users.verifier, users.indexer, amount); - - resetPrank(users.verifier); - vm.expectRevert( - abi.encodeWithSelector( - IHorizonStakingMain.HorizonStakingInvalidDelegationPool.selector, - users.indexer, - subgraphDataServiceAddress - ) - ); - escrow.collect( - IGraphPayments.PaymentTypes.QueryFee, - users.gateway, - users.indexer, - amount, - subgraphDataServiceAddress, - 1 - ); - } - - function testCollect_RevertWhen_InvalidProvision( - uint256 amount - ) public useIndexer useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) { - vm.assume(amount > 1 ether); - vm.assume(amount <= MAX_STAKING_TOKENS); - - resetPrank(users.gateway); - _depositTokens(users.verifier, users.indexer, amount); - - resetPrank(users.verifier); - vm.expectRevert( - abi.encodeWithSelector( - IHorizonStakingMain.HorizonStakingInvalidProvision.selector, - users.indexer, - subgraphDataServiceAddress - ) - ); - escrow.collect( - IGraphPayments.PaymentTypes.QueryFee, - users.gateway, - users.indexer, - amount, - subgraphDataServiceAddress, - 1 - ); - } } diff --git a/packages/horizon/test/payments/GraphPayments.t.sol b/packages/horizon/test/payments/GraphPayments.t.sol index 494a7912a..3266d1e30 100644 --- a/packages/horizon/test/payments/GraphPayments.t.sol +++ b/packages/horizon/test/payments/GraphPayments.t.sol @@ -4,11 +4,21 @@ pragma solidity 0.8.27; import "forge-std/Test.sol"; import { IHorizonStakingMain } from "../../contracts/interfaces/internal/IHorizonStakingMain.sol"; +import { IHorizonStakingTypes } from "../../contracts/interfaces/internal/IHorizonStakingTypes.sol"; import { IGraphPayments } from "../../contracts/interfaces/IGraphPayments.sol"; import { GraphPayments } from "../../contracts/payments/GraphPayments.sol"; import { HorizonStakingSharedTest } from "../shared/horizon-staking/HorizonStakingShared.t.sol"; import { PPMMath } from "../../contracts/libraries/PPMMath.sol"; +import { IERC20Errors } from "@openzeppelin/contracts/interfaces/draft-IERC6093.sol"; + +contract GraphPaymentsExtended is GraphPayments { + constructor(address controller, uint256 protocolPaymentCut) GraphPayments(controller, protocolPaymentCut) {} + + function readController() external view returns (address) { + return address(_graphController()); + } +} contract GraphPaymentsTest is HorizonStakingSharedTest { using PPMMath for uint256; @@ -40,15 +50,22 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { // Calculate cuts uint256 tokensProtocol = _tokens.mulPPMRoundUp(payments.PROTOCOL_PAYMENT_CUT()); uint256 tokensDataService = (_tokens - tokensProtocol).mulPPMRoundUp(_dataServiceCut); - uint256 tokensDelegation = (_tokens - tokensProtocol - tokensDataService).mulPPMRoundUp( - staking.getDelegationFeeCut(_receiver, _dataService, _paymentType) - ); + uint256 tokensDelegation = 0; + { + IHorizonStakingTypes.DelegationPool memory pool = staking.getDelegationPool(_receiver, _dataService); + if (pool.shares > 0) { + tokensDelegation = (_tokens - tokensProtocol - tokensDataService).mulPPMRoundUp( + staking.getDelegationFeeCut(_receiver, _dataService, _paymentType) + ); + } + } uint256 receiverExpectedPayment = _tokens - tokensProtocol - tokensDataService - tokensDelegation; (, address msgSender, ) = vm.readCallers(); vm.expectEmit(address(payments)); emit IGraphPayments.GraphPaymentCollected( + _paymentType, msgSender, _receiver, _dataService, @@ -58,13 +75,7 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { tokensDelegation, receiverExpectedPayment ); - payments.collect( - _paymentType, - _receiver, - _tokens, - _dataService, - _dataServiceCut - ); + payments.collect(_paymentType, _receiver, _tokens, _dataService, _dataServiceCut); // After balances CollectPaymentData memory afterBalances = CollectPaymentData({ @@ -96,6 +107,13 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { * TESTS */ + function testConstructor() public { + uint256 protocolCut = 100_000; + GraphPaymentsExtended newPayments = new GraphPaymentsExtended(address(controller), protocolCut); + assertEq(address(newPayments.readController()), address(controller)); + assertEq(newPayments.PROTOCOL_PAYMENT_CUT(), protocolCut); + } + function testConstructor_RevertIf_InvalidProtocolPaymentCut(uint256 protocolPaymentCut) public { protocolPaymentCut = bound(protocolPaymentCut, MAX_PPM + 1, type(uint256).max); @@ -108,18 +126,34 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { new GraphPayments(address(controller), protocolPaymentCut); } + function testInitialize() public { + // Deploy new instance to test initialization + GraphPayments newPayments = new GraphPayments(address(controller), 100_000); + + // Should revert if not called by onlyInitializer + vm.expectRevert(); + newPayments.initialize(); + } + function testCollect( uint256 amount, + uint256 amountToCollect, uint256 dataServiceCut, - uint256 tokensDelegate - ) - public - useIndexer - useProvision(amount, 0, 0) - useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) - { + uint256 tokensDelegate, + uint256 delegationFeeCut + ) public useIndexer useProvision(amount, 0, 0) { + amountToCollect = bound(amountToCollect, 1, MAX_STAKING_TOKENS); dataServiceCut = bound(dataServiceCut, 0, MAX_PPM); - address escrowAddress = address(escrow); + tokensDelegate = bound(tokensDelegate, 1, MAX_STAKING_TOKENS); + delegationFeeCut = bound(delegationFeeCut, 0, MAX_PPM); // Covers zero, max, and everything in between + + // Set delegation fee cut + _setDelegationFeeCut( + users.indexer, + subgraphDataServiceAddress, + IGraphPayments.PaymentTypes.QueryFee, + delegationFeeCut + ); // Delegate tokens tokensDelegate = bound(tokensDelegate, MIN_DELEGATION, MAX_STAKING_TOKENS); @@ -127,6 +161,7 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { _delegate(users.indexer, subgraphDataServiceAddress, tokensDelegate, 0); // Add tokens in escrow + address escrowAddress = address(escrow); mint(escrowAddress, amount); vm.startPrank(escrowAddress); approve(address(payments), amount); @@ -142,6 +177,45 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { vm.stopPrank(); } + function testCollect_NoProvision( + uint256 amount, + uint256 dataServiceCut, + uint256 delegationFeeCut + ) public useIndexer { + amount = bound(amount, 1, MAX_STAKING_TOKENS); + dataServiceCut = bound(dataServiceCut, 0, MAX_PPM); + delegationFeeCut = bound(delegationFeeCut, 0, MAX_PPM); // Covers zero, max, and everything in between + + // Set delegation fee cut + _setDelegationFeeCut( + users.indexer, + subgraphDataServiceAddress, + IGraphPayments.PaymentTypes.QueryFee, + delegationFeeCut + ); + + // Add tokens in escrow + address escrowAddress = address(escrow); + mint(escrowAddress, amount); + vm.startPrank(escrowAddress); + approve(address(payments), amount); + + // burn some tokens to prevent overflow + resetPrank(users.indexer); + token.burn(MAX_STAKING_TOKENS); + + // Collect payments through GraphPayments + vm.startPrank(escrowAddress); + _collect( + IGraphPayments.PaymentTypes.QueryFee, + users.indexer, + amount, + subgraphDataServiceAddress, + dataServiceCut + ); + vm.stopPrank(); + } + function testCollect_RevertWhen_InvalidDataServiceCut( uint256 amount, uint256 dataServiceCut @@ -168,53 +242,53 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { ); } - function testCollect_RevertWhen_InvalidPool( - uint256 amount - ) - public - useIndexer - useProvision(amount, 0, 0) - useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) - { - vm.assume(amount > 1 ether); - address escrowAddress = address(escrow); + function testCollect_WithZeroAmount(uint256 amount) public useIndexer useProvision(amount, 0, 0) { + _collect(IGraphPayments.PaymentTypes.QueryFee, users.indexer, 0, subgraphDataServiceAddress, 0); + } - // Add tokens in escrow - mint(escrowAddress, amount); - vm.startPrank(escrowAddress); - approve(address(payments), amount); + function testCollect_RevertWhen_UnauthorizedCaller(uint256 amount) public useIndexer useProvision(amount, 0, 0) { + vm.assume(amount > 0 && amount <= MAX_STAKING_TOKENS); + + // Try to collect without being the escrow + resetPrank(users.indexer); - // Collect payments through GraphPayments vm.expectRevert( - abi.encodeWithSelector( - IHorizonStakingMain.HorizonStakingInvalidDelegationPool.selector, - users.indexer, - subgraphDataServiceAddress - ) + abi.encodeWithSelector(IERC20Errors.ERC20InsufficientAllowance.selector, address(payments), 0, amount) ); - payments.collect(IGraphPayments.PaymentTypes.QueryFee, users.indexer, amount, subgraphDataServiceAddress, 1); + + payments.collect(IGraphPayments.PaymentTypes.QueryFee, users.indexer, amount, subgraphDataServiceAddress, 0); } - function testCollect_RevertWhen_InvalidProvision( - uint256 amount - ) public useIndexer useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) { - vm.assume(amount > 1 ether); - vm.assume(amount <= MAX_STAKING_TOKENS); - address escrowAddress = address(escrow); + function testCollect_WithNoDelegation( + uint256 amount, + uint256 dataServiceCut, + uint256 delegationFeeCut + ) public useIndexer useProvision(amount, 0, 0) { + dataServiceCut = bound(dataServiceCut, 0, MAX_PPM); + delegationFeeCut = bound(delegationFeeCut, 0, MAX_PPM); + + // Set delegation fee cut + _setDelegationFeeCut( + users.indexer, + subgraphDataServiceAddress, + IGraphPayments.PaymentTypes.QueryFee, + delegationFeeCut + ); // Add tokens in escrow + address escrowAddress = address(escrow); mint(escrowAddress, amount); vm.startPrank(escrowAddress); approve(address(payments), amount); // Collect payments through GraphPayments - vm.expectRevert( - abi.encodeWithSelector( - IHorizonStakingMain.HorizonStakingInvalidProvision.selector, - users.indexer, - subgraphDataServiceAddress - ) + _collect( + IGraphPayments.PaymentTypes.QueryFee, + users.indexer, + amount, + subgraphDataServiceAddress, + dataServiceCut ); - payments.collect(IGraphPayments.PaymentTypes.QueryFee, users.indexer, amount, subgraphDataServiceAddress, 1); + vm.stopPrank(); } } From 2ee9e9fa3583838d5ef05af9924c2b82f4751211 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Fri, 13 Dec 2024 15:08:24 -0300 Subject: [PATCH 4/9] chore: some minor improvements to GraphPayments and tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .../contracts/interfaces/IGraphPayments.sol | 3 +- .../contracts/payments/GraphPayments.sol | 2 ++ .../horizon/test/payments/GraphPayments.t.sol | 29 +++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/horizon/contracts/interfaces/IGraphPayments.sol b/packages/horizon/contracts/interfaces/IGraphPayments.sol index 8019e8917..ebb27a71a 100644 --- a/packages/horizon/contracts/interfaces/IGraphPayments.sol +++ b/packages/horizon/contracts/interfaces/IGraphPayments.sol @@ -57,9 +57,10 @@ interface IGraphPayments { /** * @notice Collects funds from a payer. * It will pay cuts to all relevant parties and forward the rest to the receiver. + * Note that the collected amount can be zero. * @param paymentType The type of payment as defined in {IGraphPayments} * @param receiver The address of the receiver - * @param tokens The amount of tokens being collected + * @param tokens The amount of tokens being collected. * @param dataService The address of the data service * @param dataServiceCut The data service cut in PPM */ diff --git a/packages/horizon/contracts/payments/GraphPayments.sol b/packages/horizon/contracts/payments/GraphPayments.sol index 57243c555..a32a8a523 100644 --- a/packages/horizon/contracts/payments/GraphPayments.sol +++ b/packages/horizon/contracts/payments/GraphPayments.sol @@ -23,6 +23,8 @@ import { GraphDirectory } from "../utilities/GraphDirectory.sol"; contract GraphPayments is Initializable, MulticallUpgradeable, GraphDirectory, IGraphPayments { using TokenUtils for IGraphToken; using PPMMath for uint256; + + /// @notice Protocol payment cut in PPM uint256 public immutable PROTOCOL_PAYMENT_CUT; /** diff --git a/packages/horizon/test/payments/GraphPayments.t.sol b/packages/horizon/test/payments/GraphPayments.t.sol index 3266d1e30..9ab5fae1d 100644 --- a/packages/horizon/test/payments/GraphPayments.t.sol +++ b/packages/horizon/test/payments/GraphPayments.t.sol @@ -291,4 +291,33 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { ); vm.stopPrank(); } + + function testCollect_ViaMulticall(uint256 amount) public useIndexer { + amount = bound(amount, 1, MAX_STAKING_TOKENS / 2); // Divide by 2 as we'll make two calls + + address escrowAddress = address(escrow); + mint(escrowAddress, amount * 2); + vm.startPrank(escrowAddress); + approve(address(payments), amount * 2); + + bytes[] memory data = new bytes[](2); + data[0] = abi.encodeWithSelector( + payments.collect.selector, + IGraphPayments.PaymentTypes.QueryFee, + users.indexer, + amount, + subgraphDataServiceAddress, + 100_000 // 10% + ); + data[1] = abi.encodeWithSelector( + payments.collect.selector, + IGraphPayments.PaymentTypes.IndexingFee, + users.indexer, + amount, + subgraphDataServiceAddress, + 200_000 // 20% + ); + + payments.multicall(data); + } } From e1a174f4b714cda84243d371ddbf1c63e6f068de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Fri, 13 Dec 2024 17:19:04 -0300 Subject: [PATCH 5/9] fix: couple minor improvments to PaymentsEscrow and some new tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .../contracts/interfaces/IGraphPayments.sol | 4 +- .../contracts/interfaces/IPaymentsEscrow.sol | 18 ++++++++- .../contracts/payments/PaymentsEscrow.sol | 30 +++++++------- .../horizon/test/escrow/GraphEscrow.t.sol | 30 +++++++++++++- packages/horizon/test/escrow/collect.t.sol | 40 +++++++++++++++++++ packages/horizon/test/escrow/deposit.t.sol | 25 ++++++++++-- packages/horizon/test/escrow/thaw.t.sol | 22 ++++++++-- packages/horizon/test/escrow/withdraw.t.sol | 37 +++++++++++++++-- .../PaymentsEscrowShared.t.sol | 12 ++++++ 9 files changed, 188 insertions(+), 30 deletions(-) diff --git a/packages/horizon/contracts/interfaces/IGraphPayments.sol b/packages/horizon/contracts/interfaces/IGraphPayments.sol index ebb27a71a..f62828868 100644 --- a/packages/horizon/contracts/interfaces/IGraphPayments.sol +++ b/packages/horizon/contracts/interfaces/IGraphPayments.sol @@ -31,9 +31,9 @@ interface IGraphPayments { * @param tokensReceiver Amount of tokens for the receiver */ event GraphPaymentCollected( - PaymentTypes paymentType, + PaymentTypes indexed paymentType, address indexed payer, - address indexed receiver, + address receiver, address indexed dataService, uint256 tokens, uint256 tokensProtocol, diff --git a/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol b/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol index 1c44c4e42..da05d8c9f 100644 --- a/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol +++ b/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol @@ -37,11 +37,18 @@ interface IPaymentsEscrow { /** * @notice Emitted when a payer cancels an escrow thawing * @param payer The address of the payer + * @param collector The address of the collector * @param receiver The address of the receiver * @param tokensThawing The amount of tokens that were being thawed * @param thawEndTimestamp The timestamp at which the thawing period was ending */ - event CancelThaw(address indexed payer, address indexed receiver, uint256 tokensThawing, uint256 thawEndTimestamp); + event CancelThaw( + address indexed payer, + address indexed collector, + address indexed receiver, + uint256 tokensThawing, + uint256 thawEndTimestamp + ); /** * @notice Emitted when a payer thaws funds from the escrow for a payer-collector-receiver tuple @@ -70,12 +77,19 @@ interface IPaymentsEscrow { /** * @notice Emitted when a collector collects funds from the escrow for a payer-collector-receiver tuple + * @param paymentType The type of payment being collected as defined in the {IGraphPayments} interface * @param payer The address of the payer * @param collector The address of the collector * @param receiver The address of the receiver * @param tokens The amount of tokens collected */ - event EscrowCollected(address indexed payer, address indexed collector, address indexed receiver, uint256 tokens); + event EscrowCollected( + IGraphPayments.PaymentTypes indexed paymentType, + address indexed payer, + address indexed collector, + address receiver, + uint256 tokens + ); // -- Errors -- diff --git a/packages/horizon/contracts/payments/PaymentsEscrow.sol b/packages/horizon/contracts/payments/PaymentsEscrow.sol index 5e2408365..c1de3bce1 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 Escrow account details for payer-collector-receiver tuples - mapping(address payer => mapping(address collector => mapping(address receiver => IPaymentsEscrow.EscrowAccount escrowAccount))) - public escrowAccounts; - /// @notice The maximum thawing period (in seconds) for both escrow withdrawal and collector revocation /// @dev This is a precautionary measure to avoid inadvertedly locking funds for too long uint256 public constant MAX_WAIT_PERIOD = 90 days; @@ -34,6 +30,14 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, /// @notice Thawing period in seconds for escrow funds withdrawal uint256 public immutable WITHDRAW_ESCROW_THAWING_PERIOD; + /// @notice Escrow account details for payer-collector-receiver tuples + mapping(address payer => mapping(address collector => mapping(address receiver => IPaymentsEscrow.EscrowAccount escrowAccount))) + public escrowAccounts; + + /** + * @notice Modifier to prevent function execution when contract is paused + * @dev Reverts if the controller indicates the contract is paused + */ modifier notPaused() { require(!_graphController().paused(), PaymentsEscrowIsPaused()); _; @@ -101,7 +105,7 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, account.tokensThawing = 0; account.thawEndTimestamp = 0; - emit CancelThaw(msg.sender, receiver, tokensThawing, thawEndTimestamp); + emit CancelThaw(msg.sender, collector, receiver, tokensThawing, thawEndTimestamp); } /** @@ -143,18 +147,19 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, // Reduce amount from account balance account.balance -= tokens; - uint256 balanceBefore = _graphToken().balanceOf(address(this)); + uint256 escrowBalanceBefore = _graphToken().balanceOf(address(this)); _graphToken().approve(address(_graphPayments()), tokens); _graphPayments().collect(paymentType, receiver, tokens, dataService, dataServiceCut); - uint256 balanceAfter = _graphToken().balanceOf(address(this)); + // Verify that the escrow balance is consistent with the collected tokens + uint256 escrowBalanceAfter = _graphToken().balanceOf(address(this)); require( - balanceBefore == tokens + balanceAfter, - PaymentsEscrowInconsistentCollection(balanceBefore, balanceAfter, tokens) + escrowBalanceBefore == tokens + escrowBalanceAfter, + PaymentsEscrowInconsistentCollection(escrowBalanceBefore, escrowBalanceAfter, tokens) ); - emit EscrowCollected(payer, msg.sender, receiver, tokens); + emit EscrowCollected(paymentType, payer, msg.sender, receiver, tokens); } /** @@ -162,10 +167,7 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, */ function getBalance(address payer, address collector, address receiver) external view override returns (uint256) { EscrowAccount storage account = escrowAccounts[payer][collector][receiver]; - if (account.balance <= account.tokensThawing) { - return 0; - } - return account.balance - account.tokensThawing; + return account.balance > account.tokensThawing ? account.balance - account.tokensThawing : 0; } /** diff --git a/packages/horizon/test/escrow/GraphEscrow.t.sol b/packages/horizon/test/escrow/GraphEscrow.t.sol index 6bb56c97c..ee41a7909 100644 --- a/packages/horizon/test/escrow/GraphEscrow.t.sol +++ b/packages/horizon/test/escrow/GraphEscrow.t.sol @@ -30,7 +30,9 @@ contract GraphEscrowTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest { } modifier depositAndThawTokens(uint256 amount, uint256 thawAmount) { + vm.assume(amount > 0); vm.assume(thawAmount > 0); + vm.assume(amount <= MAX_STAKING_TOKENS); vm.assume(amount > thawAmount); _depositTokens(users.verifier, users.indexer, amount); escrow.thaw(users.verifier, users.indexer, thawAmount); @@ -62,7 +64,7 @@ contract GraphEscrowTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest { (, uint256 amountThawingBefore, uint256 thawEndTimestampBefore) = escrow.escrowAccounts(msgSender, collector, receiver); vm.expectEmit(address(escrow)); - emit IPaymentsEscrow.CancelThaw(msgSender, receiver, amountThawingBefore, thawEndTimestampBefore); + emit IPaymentsEscrow.CancelThaw(msgSender, collector, receiver, amountThawingBefore, thawEndTimestampBefore); escrow.cancelThaw(collector, receiver); (, uint256 amountThawing, uint256 thawEndTimestamp) = escrow.escrowAccounts(msgSender, collector, receiver); @@ -70,6 +72,30 @@ contract GraphEscrowTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest { assertEq(thawEndTimestamp, 0); } + function _withdrawEscrow(address collector, address receiver) internal { + (, address msgSender, ) = vm.readCallers(); + + (uint256 balanceBefore, uint256 amountThawingBefore, ) = escrow.escrowAccounts(msgSender, collector, receiver); + uint256 tokenBalanceBeforeSender = token.balanceOf(msgSender); + uint256 tokenBalanceBeforeEscrow = token.balanceOf(address(escrow)); + + uint256 amountToWithdraw = amountThawingBefore > balanceBefore ? balanceBefore : amountThawingBefore; + vm.expectEmit(address(escrow)); + emit IPaymentsEscrow.Withdraw(msgSender, collector, receiver, amountToWithdraw); + escrow.withdraw(collector, receiver); + + (uint256 balanceAfter, uint256 tokensThawingAfter, uint256 thawEndTimestampAfter) = escrow.escrowAccounts(msgSender, collector, receiver); + uint256 tokenBalanceAfterSender = token.balanceOf(msgSender); + uint256 tokenBalanceAfterEscrow = token.balanceOf(address(escrow)); + + assertEq(balanceAfter, balanceBefore - amountToWithdraw); + assertEq(tokensThawingAfter, 0); + assertEq(thawEndTimestampAfter, 0); + + assertEq(tokenBalanceAfterSender, tokenBalanceBeforeSender + amountToWithdraw); + assertEq(tokenBalanceAfterEscrow, tokenBalanceBeforeEscrow - amountToWithdraw); + } + struct CollectPaymentData { uint256 escrowBalance; uint256 paymentsBalance; @@ -105,7 +131,7 @@ contract GraphEscrowTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest { } vm.expectEmit(address(escrow)); - emit IPaymentsEscrow.EscrowCollected(_payer, _collector, _receiver, _tokens); + emit IPaymentsEscrow.EscrowCollected(_paymentType, _payer, _collector, _receiver, _tokens); escrow.collect(_paymentType, _payer, _receiver, _tokens, _dataService, _dataServiceCut); // Calculate cuts diff --git a/packages/horizon/test/escrow/collect.t.sol b/packages/horizon/test/escrow/collect.t.sol index 3c18c93b6..f4357d213 100644 --- a/packages/horizon/test/escrow/collect.t.sol +++ b/packages/horizon/test/escrow/collect.t.sol @@ -13,6 +13,7 @@ contract GraphEscrowCollectTest is GraphEscrowTest { * TESTS */ + // use users.verifier as collector function testCollect_Tokens( uint256 tokens, uint256 tokensToCollect, @@ -98,4 +99,43 @@ contract GraphEscrowCollectTest is GraphEscrowTest { ); vm.stopPrank(); } + + function testCollect_MultipleCollections( + uint256 depositAmount, + uint256 firstCollect, + uint256 secondCollect + ) public useIndexer { + // Tests multiple collect operations from the same escrow account + vm.assume(firstCollect < MAX_STAKING_TOKENS); + vm.assume(secondCollect < MAX_STAKING_TOKENS); + vm.assume(depositAmount > 0); + vm.assume(firstCollect > 0 && firstCollect < depositAmount); + vm.assume(secondCollect > 0 && secondCollect <= depositAmount - firstCollect); + + resetPrank(users.gateway); + _depositTokens(users.verifier, users.indexer, depositAmount); + + // burn some tokens to prevent overflow + resetPrank(users.indexer); + token.burn(MAX_STAKING_TOKENS); + + resetPrank(users.verifier); + _collectEscrow( + IGraphPayments.PaymentTypes.QueryFee, + users.gateway, + users.indexer, + firstCollect, + subgraphDataServiceAddress, + 0 + ); + + // _collectEscrow( + // IGraphPayments.PaymentTypes.QueryFee, + // users.gateway, + // users.indexer, + // secondCollect, + // subgraphDataServiceAddress, + // 0 + // ); + } } diff --git a/packages/horizon/test/escrow/deposit.t.sol b/packages/horizon/test/escrow/deposit.t.sol index 002d2b1a4..fba3b7c0d 100644 --- a/packages/horizon/test/escrow/deposit.t.sol +++ b/packages/horizon/test/escrow/deposit.t.sol @@ -6,13 +6,32 @@ import "forge-std/Test.sol"; import { GraphEscrowTest } from "./GraphEscrow.t.sol"; contract GraphEscrowDepositTest is GraphEscrowTest { - /* * TESTS */ function testDeposit_Tokens(uint256 amount) public useGateway useDeposit(amount) { - (uint256 indexerEscrowBalance,,) = escrow.escrowAccounts(users.gateway, users.verifier, users.indexer); + (uint256 indexerEscrowBalance, , ) = escrow.escrowAccounts(users.gateway, users.verifier, users.indexer); assertEq(indexerEscrowBalance, amount); } -} \ No newline at end of file + + function testDepositTo_Tokens(uint256 amount) public { + resetPrank(users.delegator); + token.approve(address(escrow), amount); + _depositToTokens(users.gateway, users.verifier, users.indexer, amount); + } + + // Tests multiple deposits accumulate correctly in the escrow account + function testDeposit_MultipleDeposits(uint256 amount1, uint256 amount2) public useGateway { + vm.assume(amount1 > 0); + vm.assume(amount2 > 0); + vm.assume(amount1 <= MAX_STAKING_TOKENS); + vm.assume(amount2 <= MAX_STAKING_TOKENS); + + _depositTokens(users.verifier, users.indexer, amount1); + _depositTokens(users.verifier, users.indexer, amount2); + + (uint256 balance,,) = escrow.escrowAccounts(users.gateway, users.verifier, users.indexer); + assertEq(balance, amount1 + amount2); + } +} diff --git a/packages/horizon/test/escrow/thaw.t.sol b/packages/horizon/test/escrow/thaw.t.sol index e8e915b22..f7c23371b 100644 --- a/packages/horizon/test/escrow/thaw.t.sol +++ b/packages/horizon/test/escrow/thaw.t.sol @@ -10,9 +10,21 @@ contract GraphEscrowThawTest is GraphEscrowTest { * TESTS */ - function testThaw_Tokens(uint256 amount) public useGateway useDeposit(amount) { - amount = bound(amount, 1, type(uint256).max); + function testThaw_PartialBalanceThaw( + uint256 amountDeposited, + uint256 amountThawed + ) public useGateway useDeposit(amountDeposited) { + vm.assume(amountThawed > 0); + vm.assume(amountThawed <= amountDeposited); + _thawEscrow(users.verifier, users.indexer, amountThawed); + } + + function testThaw_FullBalanceThaw(uint256 amount) public useGateway useDeposit(amount) { + vm.assume(amount > 0); _thawEscrow(users.verifier, users.indexer, amount); + + uint256 availableBalance = escrow.getBalance(users.gateway, users.verifier, users.indexer); + assertEq(availableBalance, 0); } function testThaw_Tokens_SuccesiveCalls(uint256 amount) public useGateway { @@ -25,7 +37,11 @@ contract GraphEscrowThawTest is GraphEscrowTest { _thawEscrow(users.verifier, users.indexer, secondAmountToThaw); (, address msgSender, ) = vm.readCallers(); - (, uint256 amountThawing, uint256 thawEndTimestamp) = escrow.escrowAccounts(msgSender, users.verifier, users.indexer); + (, uint256 amountThawing, uint256 thawEndTimestamp) = escrow.escrowAccounts( + msgSender, + users.verifier, + users.indexer + ); assertEq(amountThawing, secondAmountToThaw); assertEq(thawEndTimestamp, block.timestamp + withdrawEscrowThawingPeriod); } diff --git a/packages/horizon/test/escrow/withdraw.t.sol b/packages/horizon/test/escrow/withdraw.t.sol index a141d039b..db7001fd8 100644 --- a/packages/horizon/test/escrow/withdraw.t.sol +++ b/packages/horizon/test/escrow/withdraw.t.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.27; import "forge-std/Test.sol"; +import { IGraphPayments } from "../../contracts/interfaces/IGraphPayments.sol"; import { GraphEscrowTest } from "./GraphEscrow.t.sol"; contract GraphEscrowWithdrawTest is GraphEscrowTest { @@ -18,11 +19,8 @@ contract GraphEscrowWithdrawTest is GraphEscrowTest { // advance time skip(withdrawEscrowThawingPeriod + 1); - escrow.withdraw(users.verifier, users.indexer); + _withdrawEscrow(users.verifier, users.indexer); vm.stopPrank(); - - (uint256 indexerEscrowBalance,,) = escrow.escrowAccounts(users.gateway, users.verifier, users.indexer); - assertEq(indexerEscrowBalance, amount - thawAmount); } function testWithdraw_RevertWhen_NotThawing(uint256 amount) public useGateway useDeposit(amount) { @@ -39,4 +37,35 @@ contract GraphEscrowWithdrawTest is GraphEscrowTest { vm.expectRevert(expectedError); escrow.withdraw(users.verifier, users.indexer); } + + function testWithdraw_BalanceAfterCollect( + uint256 amountDeposited, + uint256 amountThawed, + uint256 amountCollected + ) public useGateway depositAndThawTokens(amountDeposited, amountThawed) { + vm.assume(amountCollected > 0); + vm.assume(amountCollected <= amountDeposited); + + // burn some tokens to prevent overflow + resetPrank(users.indexer); + token.burn(MAX_STAKING_TOKENS); + + // collect + resetPrank(users.verifier); + _collectEscrow( + IGraphPayments.PaymentTypes.QueryFee, + users.gateway, + users.indexer, + amountCollected, + subgraphDataServiceAddress, + 0 + ); + + // Advance time to simulate the thawing period + skip(withdrawEscrowThawingPeriod + 1); + + // withdraw the remaining thawed balance + resetPrank(users.gateway); + _withdrawEscrow(users.verifier, users.indexer); + } } \ No newline at end of file diff --git a/packages/horizon/test/shared/payments-escrow/PaymentsEscrowShared.t.sol b/packages/horizon/test/shared/payments-escrow/PaymentsEscrowShared.t.sol index c3714dfd6..28b0327ae 100644 --- a/packages/horizon/test/shared/payments-escrow/PaymentsEscrowShared.t.sol +++ b/packages/horizon/test/shared/payments-escrow/PaymentsEscrowShared.t.sol @@ -34,4 +34,16 @@ abstract contract PaymentsEscrowSharedTest is GraphBaseTest { (uint256 escrowBalanceAfter,,) = escrow.escrowAccounts(msgSender, _collector, _receiver); assertEq(escrowBalanceAfter - _tokens, escrowBalanceBefore); } + + function _depositToTokens(address _payer, address _collector, address _receiver, uint256 _tokens) internal { + (uint256 escrowBalanceBefore,,) = escrow.escrowAccounts(_payer, _collector, _receiver); + token.approve(address(escrow), _tokens); + + vm.expectEmit(address(escrow)); + emit IPaymentsEscrow.Deposit(_payer, _collector, _receiver, _tokens); + escrow.depositTo(_payer, _collector, _receiver, _tokens); + + (uint256 escrowBalanceAfter,,) = escrow.escrowAccounts(_payer, _collector, _receiver); + assertEq(escrowBalanceAfter - _tokens, escrowBalanceBefore); + } } From e2b20a422a4d8800efa54c0e1323d81b52d0de36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Thu, 6 Feb 2025 15:00:58 -0300 Subject: [PATCH 6/9] chore: revert adding collector address to RAV MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .../contracts/interfaces/ITAPCollector.sol | 9 -------- .../payments/collectors/TAPCollector.sol | 9 +------- .../tap-collector/collect/collect.t.sol | 23 ------------------- 3 files changed, 1 insertion(+), 40 deletions(-) diff --git a/packages/horizon/contracts/interfaces/ITAPCollector.sol b/packages/horizon/contracts/interfaces/ITAPCollector.sol index 47c124b58..fd17d15c7 100644 --- a/packages/horizon/contracts/interfaces/ITAPCollector.sol +++ b/packages/horizon/contracts/interfaces/ITAPCollector.sol @@ -29,8 +29,6 @@ interface ITAPCollector is IPaymentsCollector { bytes32 collectionId; // The address of the payer the RAV was issued by address payer; - // The address of the collector where the RAV can be collected - address collector; // The address of the service provider the RAV was issued to address serviceProvider; // The address of the data service the RAV was issued to @@ -104,13 +102,6 @@ interface ITAPCollector is IPaymentsCollector { bytes signature ); - /** - * Thrown when attempting to collect a RAV that was not issued to this collector - * @param collector The address of this collector processing the RAV - * @param ravCollector The collector address noted in the RAV - */ - error TAPCollectorInvalidCollector(address collector, address ravCollector); - /** * Thrown when the signer is already authorized * @param authorizingPayer The address of the payer authorizing the signer diff --git a/packages/horizon/contracts/payments/collectors/TAPCollector.sol b/packages/horizon/contracts/payments/collectors/TAPCollector.sol index 63ec9f5db..6a19dbe54 100644 --- a/packages/horizon/contracts/payments/collectors/TAPCollector.sol +++ b/packages/horizon/contracts/payments/collectors/TAPCollector.sol @@ -29,7 +29,7 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector { /// @notice The EIP712 typehash for the ReceiptAggregateVoucher struct bytes32 private constant EIP712_RAV_TYPEHASH = keccak256( - "ReceiptAggregateVoucher(address payer,address collector,address serviceProvider,address dataService,uint64 timestampNs,uint128 valueAggregate,bytes metadata)" + "ReceiptAggregateVoucher(address payer,address serviceProvider,address dataService,uint64 timestampNs,uint128 valueAggregate,bytes metadata)" ); /// @notice Authorization details for payer-signer pairs @@ -168,12 +168,6 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector { ) private returns (uint256) { (SignedRAV memory signedRAV, uint256 dataServiceCut) = abi.decode(_data, (SignedRAV, uint256)); - // Ensure the RAV was issued to this collector - require( - signedRAV.rav.collector == address(this), - TAPCollectorInvalidCollector(address(this), signedRAV.rav.collector) - ); - // Ensure caller is the RAV data service require( signedRAV.rav.dataService == msg.sender, @@ -266,7 +260,6 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector { abi.encode( EIP712_RAV_TYPEHASH, _rav.payer, - _rav.collector, _rav.serviceProvider, _rav.dataService, _rav.timestampNs, 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 646401ca8..366305b0f 100644 --- a/packages/horizon/test/payments/tap-collector/collect/collect.t.sol +++ b/packages/horizon/test/payments/tap-collector/collect/collect.t.sol @@ -42,7 +42,6 @@ contract TAPCollectorCollectTest is TAPCollectorTest { function _getRAV( address _allocationId, address _payer, - address _collector, address _indexer, address _dataService, uint128 _tokens @@ -51,7 +50,6 @@ contract TAPCollectorCollectTest is TAPCollectorTest { ITAPCollector.ReceiptAggregateVoucher({ collectionId: bytes32(uint256(uint160(_allocationId))), payer: _payer, - collector: _collector, dataService: _dataService, serviceProvider: _indexer, timestampNs: 0, @@ -111,27 +109,6 @@ contract TAPCollectorCollectTest is TAPCollectorTest { } } - function testTAPCollector_Collect_RevertWhen_OtherCollector() public useGateway useSigner { - address otherCollector = makeAddr("otherCollector"); - bytes memory data = _getQueryFeeEncodedData( - signerPrivateKey, - users.gateway, - otherCollector, - users.indexer, - users.verifier, - uint128(0) - ); - - resetPrank(users.verifier); - bytes memory expectedError = abi.encodeWithSelector( - ITAPCollector.TAPCollectorInvalidCollector.selector, - address(tapCollector), - otherCollector - ); - vm.expectRevert(expectedError); - tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data); - } - function testTAPCollector_Collect_RevertWhen_NoProvision(uint256 tokens) public useGateway useSigner { tokens = bound(tokens, 1, type(uint128).max); From 030619c7d016abee790dba7c6a5867fccc542b7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Thu, 6 Feb 2025 15:05:19 -0300 Subject: [PATCH 7/9] chore: revert adding collector address to RAV in SubgraphService MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .../test/subgraphService/collect/query/query.t.sol | 1 - 1 file changed, 1 deletion(-) 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 1c629439f..321f01a75 100644 --- a/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol +++ b/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol @@ -49,7 +49,6 @@ contract SubgraphServiceRegisterTest is SubgraphServiceTest { ITAPCollector.ReceiptAggregateVoucher({ collectionId: collectionId, payer: users.gateway, - collector: address(tapCollector), serviceProvider: indexer, dataService: address(subgraphService), timestampNs: 0, From 74096fc6ee05542a9d9551f43b3050683816ea59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Mon, 10 Feb 2025 16:25:31 -0300 Subject: [PATCH 8/9] feat: dont issue rewards to allos less than one epoch old MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .../contracts/libraries/Allocation.sol | 8 ++- .../contracts/utilities/AllocationManager.sol | 31 +++++++----- .../test/shared/SubgraphServiceShared.t.sol | 49 ++++++++++++------- .../subgraphService/SubgraphService.t.sol | 3 ++ .../subgraphService/allocation/resize.t.sol | 4 ++ .../collect/indexing/indexing.t.sol | 23 +++++++++ 6 files changed, 87 insertions(+), 31 deletions(-) diff --git a/packages/subgraph-service/contracts/libraries/Allocation.sol b/packages/subgraph-service/contracts/libraries/Allocation.sol index 320547f92..ad66b20ff 100644 --- a/packages/subgraph-service/contracts/libraries/Allocation.sol +++ b/packages/subgraph-service/contracts/libraries/Allocation.sol @@ -30,6 +30,8 @@ library Allocation { uint256 accRewardsPerAllocatedToken; // Accumulated rewards that are pending to be claimed due allocation resize uint256 accRewardsPending; + // Epoch when the allocation was created + uint256 createdAtEpoch; } /** @@ -68,7 +70,8 @@ library Allocation { address allocationId, bytes32 subgraphDeploymentId, uint256 tokens, - uint256 accRewardsPerAllocatedToken + uint256 accRewardsPerAllocatedToken, + uint256 createdAtEpoch ) internal returns (State memory) { require(!self[allocationId].exists(), AllocationAlreadyExists(allocationId)); @@ -80,7 +83,8 @@ library Allocation { closedAt: 0, lastPOIPresentedAt: 0, accRewardsPerAllocatedToken: accRewardsPerAllocatedToken, - accRewardsPending: 0 + accRewardsPending: 0, + createdAtEpoch: createdAtEpoch }); self[allocationId] = allocation; diff --git a/packages/subgraph-service/contracts/utilities/AllocationManager.sol b/packages/subgraph-service/contracts/utilities/AllocationManager.sol index 008a65eef..80264af93 100644 --- a/packages/subgraph-service/contracts/utilities/AllocationManager.sol +++ b/packages/subgraph-service/contracts/utilities/AllocationManager.sol @@ -40,12 +40,14 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca * @param allocationId The id of the allocation * @param subgraphDeploymentId The id of the subgraph deployment * @param tokens The amount of tokens allocated + * @param currentEpoch The current epoch */ event AllocationCreated( address indexed indexer, address indexed allocationId, bytes32 indexed subgraphDeploymentId, - uint256 tokens + uint256 tokens, + uint256 currentEpoch ); /** @@ -157,17 +159,14 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca // solhint-disable-next-line func-name-mixedcase function __AllocationManager_init(string memory _name, string memory _version) internal onlyInitializing { __EIP712_init(_name, _version); - __AllocationManager_init_unchained(_name, _version); + __AllocationManager_init_unchained(); } /** * @notice Initializes the contract */ // solhint-disable-next-line func-name-mixedcase - function __AllocationManager_init_unchained( - string memory _name, - string memory _version - ) internal onlyInitializing {} + function __AllocationManager_init_unchained() internal onlyInitializing {} /** * @notice Imports a legacy allocation id into the subgraph service @@ -213,12 +212,15 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca // Ensure allocation id is not reused // need to check both subgraph service (on allocations.create()) and legacy allocations _legacyAllocations.revertIfExists(_graphStaking(), _allocationId); + + uint256 currentEpoch = _graphEpochManager().currentEpoch(); Allocation.State memory allocation = _allocations.create( _indexer, _allocationId, _subgraphDeploymentId, _tokens, - _graphRewardsManager().onSubgraphAllocationUpdate(_subgraphDeploymentId) + _graphRewardsManager().onSubgraphAllocationUpdate(_subgraphDeploymentId), + currentEpoch ); // Check that the indexer has enough tokens available @@ -230,7 +232,7 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca _subgraphAllocatedTokens[allocation.subgraphDeploymentId] + allocation.tokens; - emit AllocationCreated(_indexer, _allocationId, _subgraphDeploymentId, allocation.tokens); + emit AllocationCreated(_indexer, _allocationId, _subgraphDeploymentId, allocation.tokens, currentEpoch); return allocation; } @@ -238,15 +240,20 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca * @notice Present a POI to collect indexing rewards for an allocation * This function will mint indexing rewards using the {RewardsManager} and distribute them to the indexer and delegators. * - * To qualify for indexing rewards: + * Conditions to qualify for indexing rewards: * - POI must be non-zero * - POI must not be stale, i.e: older than `maxPOIStaleness` * - allocation must not be altruistic (allocated tokens = 0) + * - allocation must be open for at least one epoch * * Note that indexers are required to periodically (at most every `maxPOIStaleness`) present POIs to collect rewards. * Rewards will not be issued to stale POIs, which means that indexers are advised to present a zero POI if they are * unable to present a valid one to prevent being locked out of future rewards. * + * Note on allocation duration restriction: this is required to ensure that non protocol chains have a valid block number for + * which to calculate POIs. EBO posts once per epoch typically at each epoch change, so we restrict rewards to allocations + * that have gone through at least one epoch change. + * * Emits a {IndexingRewardsCollected} event. * * @param _allocationId The id of the allocation to collect rewards for @@ -260,10 +267,12 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca Allocation.State memory allocation = _allocations.get(_allocationId); require(allocation.isOpen(), AllocationManagerAllocationClosed(_allocationId)); + uint256 currentEpoch = _graphEpochManager().currentEpoch(); + // Mint indexing rewards if all conditions are met uint256 tokensRewards = (!allocation.isStale(maxPOIStaleness) && !allocation.isAltruistic() && - _poi != bytes32(0)) + _poi != bytes32(0)) && currentEpoch > allocation.createdAtEpoch ? _graphRewardsManager().takeRewards(_allocationId) : 0; @@ -318,7 +327,7 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca tokensIndexerRewards, tokensDelegationRewards, _poi, - _graphEpochManager().currentEpoch() + currentEpoch ); // Check if the indexer is over-allocated and close the allocation if necessary diff --git a/packages/subgraph-service/test/shared/SubgraphServiceShared.t.sol b/packages/subgraph-service/test/shared/SubgraphServiceShared.t.sol index fa9d420fb..068e492e3 100644 --- a/packages/subgraph-service/test/shared/SubgraphServiceShared.t.sol +++ b/packages/subgraph-service/test/shared/SubgraphServiceShared.t.sol @@ -11,7 +11,6 @@ import { ISubgraphService } from "../../contracts/interfaces/ISubgraphService.so import { HorizonStakingSharedTest } from "./HorizonStakingShared.t.sol"; abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest { - /* * VARIABLES */ @@ -24,7 +23,7 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest { * MODIFIERS */ - modifier useIndexer { + modifier useIndexer() { vm.startPrank(users.indexer); _; vm.stopPrank(); @@ -35,7 +34,12 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest { vm.assume(tokens < 10_000_000_000 ether); _createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod); _register(users.indexer, abi.encode("url", "geoHash", address(0))); - bytes memory data = _createSubgraphAllocationData(users.indexer, subgraphDeployment, allocationIDPrivateKey, tokens); + bytes memory data = _createSubgraphAllocationData( + users.indexer, + subgraphDeployment, + allocationIDPrivateKey, + tokens + ); _startService(users.indexer, data); _; } @@ -43,7 +47,7 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest { modifier useDelegation(uint256 tokens) { vm.assume(tokens > MIN_DELEGATION); vm.assume(tokens < 10_000_000_000 ether); - (, address msgSender,) = vm.readCallers(); + (, address msgSender, ) = vm.readCallers(); resetPrank(users.delegator); token.approve(address(staking), tokens); _delegate(users.indexer, address(subgraphService), tokens, 0); @@ -72,10 +76,7 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest { ); vm.expectEmit(address(subgraphService)); - emit IDataService.ServiceProviderRegistered( - _indexer, - _data - ); + emit IDataService.ServiceProviderRegistered(_indexer, _data); // Register indexer subgraphService.register(_indexer, _data); @@ -91,14 +92,22 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest { } function _startService(address _indexer, bytes memory _data) internal { - (bytes32 subgraphDeploymentId, uint256 tokens, address allocationId,) = abi.decode( + (bytes32 subgraphDeploymentId, uint256 tokens, address allocationId, ) = abi.decode( _data, (bytes32, uint256, address, bytes) ); uint256 previousSubgraphAllocatedTokens = subgraphService.getSubgraphAllocatedTokens(subgraphDeploymentId); + uint256 currentEpoch = epochManager.currentEpoch(); vm.expectEmit(address(subgraphService)); emit IDataService.ServiceStarted(_indexer, _data); + emit AllocationManager.AllocationCreated( + _indexer, + allocationId, + subgraphDeploymentId, + tokens, + currentEpoch + ); // TODO: improve this uint256 accRewardsPerAllocatedToken = 0; @@ -116,9 +125,10 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest { assertEq(allocation.subgraphDeploymentId, subgraphDeploymentId); assertEq(allocation.createdAt, block.timestamp); assertEq(allocation.closedAt, 0); - assertEq(allocation.lastPOIPresentedAt, 0); + assertEq(allocation.lastPOIPresentedAt, 0); assertEq(allocation.accRewardsPerAllocatedToken, accRewardsPerAllocatedToken); assertEq(allocation.accRewardsPending, 0); + assertEq(allocation.createdAtEpoch, currentEpoch); // Check subgraph deployment allocated tokens uint256 subgraphAllocatedTokens = subgraphService.getSubgraphAllocatedTokens(subgraphDeploymentId); @@ -130,10 +140,17 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest { assertTrue(subgraphService.isActiveAllocation(allocationId)); Allocation.State memory allocation = subgraphService.getAllocation(allocationId); - uint256 previousSubgraphAllocatedTokens = subgraphService.getSubgraphAllocatedTokens(allocation.subgraphDeploymentId); - + uint256 previousSubgraphAllocatedTokens = subgraphService.getSubgraphAllocatedTokens( + allocation.subgraphDeploymentId + ); + vm.expectEmit(address(subgraphService)); - emit AllocationManager.AllocationClosed(_indexer, allocationId, allocation.subgraphDeploymentId, allocation.tokens); + emit AllocationManager.AllocationClosed( + _indexer, + allocationId, + allocation.subgraphDeploymentId, + allocation.tokens + ); emit IDataService.ServiceStopped(_indexer, _data); // stop allocation @@ -178,10 +195,6 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest { function _getIndexer(address _indexer) private view returns (ISubgraphService.Indexer memory) { (uint256 registeredAt, string memory url, string memory geoHash) = subgraphService.indexers(_indexer); - return ISubgraphService.Indexer({ - registeredAt: registeredAt, - url: url, - geoHash: geoHash - }); + return ISubgraphService.Indexer({ registeredAt: registeredAt, url: url, geoHash: geoHash }); } } diff --git a/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol b/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol index 73430c85c..11dcbfe03 100644 --- a/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol +++ b/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol @@ -302,6 +302,9 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest { // Calculate the payment collected by the indexer for this transaction paymentCollected = accRewardsPerTokens - allocation.accRewardsPerAllocatedToken; + uint256 currentEpoch = epochManager.currentEpoch(); + paymentCollected = currentEpoch > allocation.createdAtEpoch ? paymentCollected : 0; + uint256 delegatorCut = staking.getDelegationFeeCut( allocation.indexer, address(subgraphService), diff --git a/packages/subgraph-service/test/subgraphService/allocation/resize.t.sol b/packages/subgraph-service/test/subgraphService/allocation/resize.t.sol index 0df994929..ff8c504a9 100644 --- a/packages/subgraph-service/test/subgraphService/allocation/resize.t.sol +++ b/packages/subgraph-service/test/subgraphService/allocation/resize.t.sol @@ -29,6 +29,10 @@ contract SubgraphServiceAllocationResizeTest is SubgraphServiceTest { vm.assume(resizeTokens != tokens); mint(users.indexer, resizeTokens); + + // skip time to ensure allocation gets rewards + vm.roll(block.number + EPOCH_LENGTH); + IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards; bytes memory data = abi.encode(allocationID, bytes32("POI1")); _collect(users.indexer, paymentType, data); diff --git a/packages/subgraph-service/test/subgraphService/collect/indexing/indexing.t.sol b/packages/subgraph-service/test/subgraphService/collect/indexing/indexing.t.sol index 465f5c8cd..48eb36eb8 100644 --- a/packages/subgraph-service/test/subgraphService/collect/indexing/indexing.t.sol +++ b/packages/subgraph-service/test/subgraphService/collect/indexing/indexing.t.sol @@ -19,6 +19,10 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest { ) public useIndexer useAllocation(tokens) { IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards; bytes memory data = abi.encode(allocationID, bytes32("POI")); + + // skip time to ensure allocation gets rewards + vm.roll(block.number + EPOCH_LENGTH); + _collect(users.indexer, paymentType, data); } @@ -34,6 +38,11 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest { IGraphPayments.PaymentTypes.IndexingRewards, delegationFeeCut ); + + + // skip time to ensure allocation gets rewards + vm.roll(block.number + EPOCH_LENGTH); + IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards; bytes memory data = abi.encode(allocationID, bytes32("POI")); _collect(users.indexer, paymentType, data); @@ -54,6 +63,10 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest { // Undelegate resetPrank(users.delegator); staking.undelegate(users.indexer, address(subgraphService), delegationTokens); + + // skip time to ensure allocation gets rewards + vm.roll(block.number + EPOCH_LENGTH); + resetPrank(users.indexer); IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards; bytes memory data = abi.encode(allocationID, bytes32("POI")); @@ -63,6 +76,9 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest { function test_SubgraphService_Collect_Indexing_RewardsDestination( uint256 tokens ) public useIndexer useAllocation(tokens) useRewardsDestination { + // skip time to ensure allocation gets rewards + vm.roll(block.number + EPOCH_LENGTH); + IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards; bytes memory data = abi.encode(allocationID, bytes32("POI")); _collect(users.indexer, paymentType, data); @@ -121,6 +137,9 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest { // thaw some tokens to become over allocated staking.thaw(users.indexer, address(subgraphService), tokens / 2); + // skip time to ensure allocation gets rewards + vm.roll(block.number + EPOCH_LENGTH); + // this collection should close the allocation IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards; bytes memory collectData = abi.encode(allocationID, bytes32("POI")); @@ -135,6 +154,10 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest { address newIndexer = makeAddr("newIndexer"); _createAndStartAllocation(newIndexer, tokens); bytes memory data = abi.encode(allocationID, bytes32("POI")); + + // skip time to ensure allocation gets rewards + vm.roll(block.number + EPOCH_LENGTH); + // Attempt to collect from other indexer's allocation vm.expectRevert( abi.encodeWithSelector(ISubgraphService.SubgraphServiceAllocationNotAuthorized.selector, newIndexer, allocationID) From 40547142a5ee40fbbaf2cc936f2e7ffded102787 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Mon, 10 Feb 2025 16:49:54 -0300 Subject: [PATCH 9/9] chore: ensure contract size is below 24kB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .../contracts/rewards/IRewardsIssuer.sol | 7 ------ .../contracts/rewards/IRewardsManager.sol | 2 +- .../contracts/rewards/RewardsManager.sol | 25 +++++-------------- .../staking/HorizonStakingExtension.sol | 9 ------- .../contracts/SubgraphService.sol | 19 -------------- .../contracts/interfaces/ISubgraphService.sol | 7 ------ packages/subgraph-service/hardhat.config.ts | 9 +++++++ .../test/mocks/MockRewardsManager.sol | 2 +- .../test/shared/SubgraphServiceShared.t.sol | 12 +++------ .../subgraphService/SubgraphService.t.sol | 3 +-- 10 files changed, 22 insertions(+), 73 deletions(-) diff --git a/packages/contracts/contracts/rewards/IRewardsIssuer.sol b/packages/contracts/contracts/rewards/IRewardsIssuer.sol index fe6963fa7..9da18b92b 100644 --- a/packages/contracts/contracts/rewards/IRewardsIssuer.sol +++ b/packages/contracts/contracts/rewards/IRewardsIssuer.sol @@ -31,11 +31,4 @@ interface IRewardsIssuer { * @return Total tokens allocated to subgraph */ function getSubgraphAllocatedTokens(bytes32 _subgraphDeploymentId) external view returns (uint256); - - /** - * @notice Whether or not an allocation is active (i.e open) - * @param _allocationId Allocation Id - * @return Whether or not the allocation is active - */ - function isActiveAllocation(address _allocationId) external view returns (bool); } diff --git a/packages/contracts/contracts/rewards/IRewardsManager.sol b/packages/contracts/contracts/rewards/IRewardsManager.sol index a1f9a3ba6..b31064d1b 100644 --- a/packages/contracts/contracts/rewards/IRewardsManager.sol +++ b/packages/contracts/contracts/rewards/IRewardsManager.sol @@ -39,7 +39,7 @@ interface IRewardsManager { function getAccRewardsPerAllocatedToken(bytes32 _subgraphDeploymentID) external view returns (uint256, uint256); - function getRewards(address _allocationID) external view returns (uint256); + function getRewards(address _rewardsIssuer, address _allocationID) external view returns (uint256); function calcRewards(uint256 _tokens, uint256 _accRewardsPerAllocatedToken) external pure returns (uint256); diff --git a/packages/contracts/contracts/rewards/RewardsManager.sol b/packages/contracts/contracts/rewards/RewardsManager.sol index a02e4518b..72afc3e1a 100644 --- a/packages/contracts/contracts/rewards/RewardsManager.sol +++ b/packages/contracts/contracts/rewards/RewardsManager.sol @@ -322,24 +322,11 @@ contract RewardsManager is RewardsManagerV5Storage, GraphUpgradeable, IRewardsMa * @param _allocationID Allocation * @return Rewards amount for an allocation */ - function getRewards(address _allocationID) external view override returns (uint256) { - address rewardsIssuer = address(0); - - // Check both the legacy and new allocations - address[2] memory rewardsIssuers = [address(staking()), address(subgraphService)]; - for (uint256 i = 0; i < rewardsIssuers.length; i++) { - if (rewardsIssuers[i] != address(0)) { - if (IRewardsIssuer(rewardsIssuers[i]).isActiveAllocation(_allocationID)) { - rewardsIssuer = address(rewardsIssuers[i]); - break; - } - } - } - - // Bail if allo does not exist - if (rewardsIssuer == address(0)) { - return 0; - } + function getRewards(address _rewardsIssuer, address _allocationID) external view override returns (uint256) { + require( + _rewardsIssuer == address(staking()) || _rewardsIssuer == address(subgraphService), + "Not a rewards issuer" + ); ( , @@ -347,7 +334,7 @@ contract RewardsManager is RewardsManagerV5Storage, GraphUpgradeable, IRewardsMa uint256 tokens, uint256 alloAccRewardsPerAllocatedToken, uint256 accRewardsPending - ) = IRewardsIssuer(rewardsIssuer).getAllocationData(_allocationID); + ) = IRewardsIssuer(_rewardsIssuer).getAllocationData(_allocationID); (uint256 accRewardsPerAllocatedToken, ) = getAccRewardsPerAllocatedToken(subgraphDeploymentId); return diff --git a/packages/horizon/contracts/staking/HorizonStakingExtension.sol b/packages/horizon/contracts/staking/HorizonStakingExtension.sol index afb20855b..8074404f3 100644 --- a/packages/horizon/contracts/staking/HorizonStakingExtension.sol +++ b/packages/horizon/contracts/staking/HorizonStakingExtension.sol @@ -274,15 +274,6 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension return __DEPRECATED_subgraphAllocations[subgraphDeploymentID]; } - /** - * @notice Return true if allocation is active. - * @param allocationID Allocation identifier - * @return True if allocation is active - */ - function isActiveAllocation(address allocationID) external view override returns (bool) { - return _getAllocationState(allocationID) == AllocationState.Active; - } - /** * @notice Get the total amount of tokens staked by the indexer. * @param indexer Address of the indexer diff --git a/packages/subgraph-service/contracts/SubgraphService.sol b/packages/subgraph-service/contracts/SubgraphService.sol index db5aae0eb..f43669133 100644 --- a/packages/subgraph-service/contracts/SubgraphService.sol +++ b/packages/subgraph-service/contracts/SubgraphService.sol @@ -449,18 +449,6 @@ contract SubgraphService is return _subgraphAllocatedTokens[subgraphDeploymentId]; } - /** - * @notice Check if an allocation is open - * @dev Implements {IRewardsIssuer.isAllocationActive} - * @dev To be used by the {RewardsManager}. - * - * @param allocationId The allocation Id - * @return Wether or not the allocation is active - */ - function isActiveAllocation(address allocationId) external view override returns (bool) { - return _allocations[allocationId].isOpen(); - } - /** * @notice See {ISubgraphService.getLegacyAllocation} */ @@ -475,13 +463,6 @@ contract SubgraphService is return _encodeAllocationProof(indexer, allocationId); } - /** - * @notice See {ISubgraphService.isStaleAllocation} - */ - function isStaleAllocation(address allocationId) external view override returns (bool) { - return _allocations.get(allocationId).isStale(maxPOIStaleness); - } - /** * @notice See {ISubgraphService.isOverAllocated} */ diff --git a/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol b/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol index 7639ed4fb..da04b48ee 100644 --- a/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol +++ b/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol @@ -241,13 +241,6 @@ interface ISubgraphService is IDataServiceFees { */ function encodeAllocationProof(address indexer, address allocationId) external view returns (bytes32); - /** - * @notice Checks if an allocation is stale - * @param allocationId The id of the allocation - * @return True if the allocation is stale, false otherwise - */ - function isStaleAllocation(address allocationId) external view returns (bool); - /** * @notice Checks if an indexer is over-allocated * @param allocationId The id of the allocation diff --git a/packages/subgraph-service/hardhat.config.ts b/packages/subgraph-service/hardhat.config.ts index 1c80d89a0..8239cb494 100644 --- a/packages/subgraph-service/hardhat.config.ts +++ b/packages/subgraph-service/hardhat.config.ts @@ -17,6 +17,15 @@ if (process.env.BUILD_RUN !== 'true') { const config: HardhatUserConfig = { ...hardhatBaseConfig, + solidity: { + version: '0.8.27', + settings: { + optimizer: { + enabled: true, + runs: 50, + }, + }, + }, graph: { deployments: { ...hardhatBaseConfig.graph?.deployments, diff --git a/packages/subgraph-service/test/mocks/MockRewardsManager.sol b/packages/subgraph-service/test/mocks/MockRewardsManager.sol index 363fbf902..9e0047a89 100644 --- a/packages/subgraph-service/test/mocks/MockRewardsManager.sol +++ b/packages/subgraph-service/test/mocks/MockRewardsManager.sol @@ -61,7 +61,7 @@ contract MockRewardsManager is IRewardsManager { function getAccRewardsPerAllocatedToken(bytes32) external view returns (uint256, uint256) {} - function getRewards(address) external view returns (uint256) {} + function getRewards(address,address) external view returns (uint256) {} function calcRewards(uint256, uint256) external pure returns (uint256) {} diff --git a/packages/subgraph-service/test/shared/SubgraphServiceShared.t.sol b/packages/subgraph-service/test/shared/SubgraphServiceShared.t.sol index 068e492e3..8a51d3443 100644 --- a/packages/subgraph-service/test/shared/SubgraphServiceShared.t.sol +++ b/packages/subgraph-service/test/shared/SubgraphServiceShared.t.sol @@ -11,6 +11,8 @@ import { ISubgraphService } from "../../contracts/interfaces/ISubgraphService.so import { HorizonStakingSharedTest } from "./HorizonStakingShared.t.sol"; abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest { + using Allocation for Allocation.State; + /* * VARIABLES */ @@ -101,13 +103,7 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest { vm.expectEmit(address(subgraphService)); emit IDataService.ServiceStarted(_indexer, _data); - emit AllocationManager.AllocationCreated( - _indexer, - allocationId, - subgraphDeploymentId, - tokens, - currentEpoch - ); + emit AllocationManager.AllocationCreated(_indexer, allocationId, subgraphDeploymentId, tokens, currentEpoch); // TODO: improve this uint256 accRewardsPerAllocatedToken = 0; @@ -137,9 +133,9 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest { function _stopService(address _indexer, bytes memory _data) internal { address allocationId = abi.decode(_data, (address)); - assertTrue(subgraphService.isActiveAllocation(allocationId)); Allocation.State memory allocation = subgraphService.getAllocation(allocationId); + assertTrue(allocation.isOpen()); uint256 previousSubgraphAllocatedTokens = subgraphService.getSubgraphAllocatedTokens( allocation.subgraphDeploymentId ); diff --git a/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol b/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol index 11dcbfe03..1a37fed86 100644 --- a/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol +++ b/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol @@ -149,9 +149,8 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest { } function _closeStaleAllocation(address _allocationId) internal { - assertTrue(subgraphService.isActiveAllocation(_allocationId)); - Allocation.State memory allocation = subgraphService.getAllocation(_allocationId); + assertTrue(allocation.isOpen()); uint256 previousSubgraphAllocatedTokens = subgraphService.getSubgraphAllocatedTokens( allocation.subgraphDeploymentId );