From f657198bfefd8f2b00d04679375e7b0bcf00a7cc Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Tue, 21 May 2024 18:47:05 -0300 Subject: [PATCH 1/7] chore: fix unit tests and refactor --- .../contracts/data-service/GraphDirectory.sol | 1 + .../contracts/payments/GraphPayments.sol | 6 +- packages/horizon/test/GraphBase.t.sol | 36 +++++----- .../horizon/test/escrow/GraphEscrow.t.sol | 30 +++++--- packages/horizon/test/escrow/collect.t.sol | 69 ++++++++----------- packages/horizon/test/escrow/collector.t.sol | 27 ++------ packages/horizon/test/escrow/deposit.t.sol | 2 +- packages/horizon/test/escrow/thaw.t.sol | 6 +- packages/horizon/test/escrow/withdraw.t.sol | 2 +- .../horizon/test/payments/GraphPayments.t.sol | 20 +++--- .../horizon-staking/HorizonStaking.t.sol | 26 ++++++- packages/horizon/test/utils/Constants.sol | 4 +- 12 files changed, 120 insertions(+), 109 deletions(-) diff --git a/packages/horizon/contracts/data-service/GraphDirectory.sol b/packages/horizon/contracts/data-service/GraphDirectory.sol index f05d9db66..805136db1 100644 --- a/packages/horizon/contracts/data-service/GraphDirectory.sol +++ b/packages/horizon/contracts/data-service/GraphDirectory.sol @@ -16,6 +16,7 @@ import { IGraphProxyAdmin } from "@graphprotocol/contracts/contracts/upgrades/IG import { ICuration } from "@graphprotocol/contracts/contracts/curation/ICuration.sol"; +import "forge-std/console.sol"; /** * @title GraphDirectory contract * @notice This contract is meant to be inherited by other contracts that diff --git a/packages/horizon/contracts/payments/GraphPayments.sol b/packages/horizon/contracts/payments/GraphPayments.sol index 2ba8f8dc8..8fddc6a07 100644 --- a/packages/horizon/contracts/payments/GraphPayments.sol +++ b/packages/horizon/contracts/payments/GraphPayments.sol @@ -69,7 +69,11 @@ contract GraphPayments is Multicall, GraphDirectory, GraphPaymentsStorageV1Stora } // Pay the rest to the receiver - uint256 tokensReceiverRemaining = tokens - tokensProtocol - tokensDataService - tokensDelegationPool; + uint256 totalCut = tokensProtocol + tokensDataService + tokensDelegationPool; + if (totalCut > tokens) { + revert GraphPaymentsCollectorInsufficientAmount(tokens, totalCut); + } + uint256 tokensReceiverRemaining = tokens - totalCut; _graphToken().pushTokens(receiver, tokensReceiverRemaining); emit GraphPaymentsCollected( diff --git a/packages/horizon/test/GraphBase.t.sol b/packages/horizon/test/GraphBase.t.sol index 36721ed2c..51a5b4d5a 100644 --- a/packages/horizon/test/GraphBase.t.sol +++ b/packages/horizon/test/GraphBase.t.sol @@ -67,10 +67,13 @@ abstract contract GraphBaseTest is Test, Constants { } function deployProtocolContracts() private { - vm.prank(users.governor); + vm.startPrank(users.governor); controller = new Controller(); + controller.setContractProxy(keccak256("GraphToken"), address(token)); + vm.stopPrank(); + - // GraphPayments preddict address + // GraphPayments predict address bytes32 saltPayments = keccak256("GraphPaymentsSalt"); bytes32 paymentsHash = keccak256(bytes.concat( vm.getCode("GraphPayments.sol:GraphPayments"), @@ -82,7 +85,7 @@ abstract contract GraphBaseTest is Test, Constants { users.deployer ); - // GraphEscrow preddict address + // GraphEscrow predict address bytes32 saltEscrow = keccak256("GraphEscrowSalt"); bytes32 escrowHash = keccak256(bytes.concat( vm.getCode("GraphEscrow.sol:GraphEscrow"), @@ -98,7 +101,7 @@ abstract contract GraphBaseTest is Test, Constants { users.deployer ); - // HorizonStakingExtension preddict address + // HorizonStakingExtension predict address bytes32 saltHorizonStakingExtension = keccak256("HorizonStakingExtensionSalt"); bytes32 horizonStakingExtensionHash = keccak256(bytes.concat( vm.getCode("HorizonStakingExtension.sol:HorizonStakingExtension"), @@ -110,7 +113,7 @@ abstract contract GraphBaseTest is Test, Constants { users.deployer ); - // HorizonStaking preddict address + // HorizonStaking predict address bytes32 saltHorizonStaking = keccak256("saltHorizonStaking"); bytes32 horizonStakingHash = keccak256(bytes.concat( vm.getCode("HorizonStaking.sol:HorizonStaking"), @@ -120,18 +123,17 @@ abstract contract GraphBaseTest is Test, Constants { subgraphDataServiceAddress ) )); - // address predictedAddressHorizonStaking = vm.computeCreate2Address( - // saltHorizonStaking, - // horizonStakingHash, - // users.deployer - // ); + address predictedAddressHorizonStaking = vm.computeCreate2Address( + saltHorizonStaking, + horizonStakingHash, + users.deployer + ); // Setup controller vm.startPrank(users.governor); - controller.setContractProxy(keccak256("GraphToken"), address(token)); controller.setContractProxy(keccak256("GraphEscrow"), predictedAddressEscrow); controller.setContractProxy(keccak256("GraphPayments"), predictedPaymentsAddress); - // controller.setContractProxy(keccak256("Staking"), predictedAddressHorizonStaking); + controller.setContractProxy(keccak256("Staking"), address(predictedAddressHorizonStaking)); vm.stopPrank(); vm.startPrank(users.deployer); @@ -144,16 +146,16 @@ abstract contract GraphBaseTest is Test, Constants { revokeCollectorThawingPeriod, withdrawEscrowThawingPeriod ); + stakingExtension = new HorizonStakingExtension{salt: saltHorizonStakingExtension}( + address(controller), + subgraphDataServiceAddress + ); stakingBase = new HorizonStaking{salt: saltHorizonStaking}( address(controller), - predictedAddressHorizonStakingExtension, + address(stakingExtension), subgraphDataServiceAddress ); staking = IHorizonStaking(address(stakingBase)); - // stakingExtension = new HorizonStakingExtension{salt: saltHorizonStakingExtension}( - // address(controller), - // subgraphDataServiceAddress - // ); vm.stopPrank(); } diff --git a/packages/horizon/test/escrow/GraphEscrow.t.sol b/packages/horizon/test/escrow/GraphEscrow.t.sol index f420c1964..fffa82830 100644 --- a/packages/horizon/test/escrow/GraphEscrow.t.sol +++ b/packages/horizon/test/escrow/GraphEscrow.t.sol @@ -13,15 +13,23 @@ contract GraphEscrowTest is HorizonStakingSharedTest { vm.stopPrank(); } - modifier approveEscrow(uint256 amount) { - _approveEscrow(amount); + modifier approveEscrow(uint256 tokens) { + changePrank(users.gateway); + _approveEscrow(tokens); _; } - modifier depositTokens(uint256 amount) { - vm.assume(amount > 0); - vm.assume(amount <= 10000 ether); - _depositTokens(amount); + modifier useDeposit(uint256 tokens) { + changePrank(users.gateway); + vm.assume(tokens > 0); + vm.assume(tokens <= 10_000_000_000 ether); + _depositTokens(tokens); + _; + } + + modifier useCollector(uint256 tokens) { + changePrank(users.gateway); + escrow.approveCollector(users.verifier, tokens); _; } @@ -29,12 +37,12 @@ contract GraphEscrowTest is HorizonStakingSharedTest { HorizonStakingSharedTest.setUp(); } - function _depositTokens(uint256 amount) internal { - token.approve(address(escrow), amount); - escrow.deposit(users.indexer, amount); + function _depositTokens(uint256 tokens) internal { + token.approve(address(escrow), tokens); + escrow.deposit(users.indexer, tokens); } - function _approveEscrow(uint256 amount) internal { - token.approve(address(escrow), amount); + function _approveEscrow(uint256 tokens) internal { + token.approve(address(escrow), tokens); } } \ No newline at end of file diff --git a/packages/horizon/test/escrow/collect.t.sol b/packages/horizon/test/escrow/collect.t.sol index 1f2b733f6..db7f549e8 100644 --- a/packages/horizon/test/escrow/collect.t.sol +++ b/packages/horizon/test/escrow/collect.t.sol @@ -8,68 +8,57 @@ import { IGraphPayments } from "../../contracts/interfaces/IGraphPayments.sol"; contract GraphEscrowCollectTest is GraphEscrowTest { - function testCollect() public { - uint256 amount = 1000 ether; - createProvision(amount); - setDelegationFeeCut(0, 100000); - + function testCollect_Tokens(uint256 amount, uint256 tokensDataService) public useProvision(amount, 0, 0) useDelegationFeeCut(0, delegationFeeCut) { + uint256 tokensProtocol = amount * protocolPaymentCut / MAX_PPM; + uint256 tokensDelegatoion = amount * delegationFeeCut / MAX_PPM; + vm.assume(tokensDataService < amount - tokensProtocol - tokensDelegatoion); + vm.startPrank(users.gateway); - escrow.approveCollector(users.verifier, 1000 ether); - token.approve(address(escrow), 1000 ether); - escrow.deposit(users.indexer, 1000 ether); + escrow.approveCollector(users.verifier, amount); + _depositTokens(amount); vm.stopPrank(); uint256 indexerPreviousBalance = token.balanceOf(users.indexer); vm.prank(users.verifier); - escrow.collect(IGraphPayments.PaymentTypes.IndexingFee, users.gateway, users.indexer, 100 ether, subgraphDataServiceAddress, 3 ether); + escrow.collect(IGraphPayments.PaymentTypes.IndexingFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, tokensDataService); uint256 indexerBalance = token.balanceOf(users.indexer); - assertEq(indexerBalance - indexerPreviousBalance, 86 ether); + uint256 indexerExpectedPayment = amount - tokensDataService - tokensProtocol - tokensDelegatoion; + assertEq(indexerBalance - indexerPreviousBalance, indexerExpectedPayment); + assertTrue(true); } - function testCollect_RevertWhen_CollectorNotAuthorized() public { - address indexer = address(0xA3); - uint256 amount = 1000 ether; - + function testCollect_RevertWhen_CollectorNotAuthorized(uint256 amount) public { vm.startPrank(users.verifier); uint256 dataServiceCut = 30000; // 3% bytes memory expectedError = abi.encodeWithSignature("GraphEscrowCollectorNotAuthorized(address,address)", users.gateway, users.verifier); vm.expectRevert(expectedError); - escrow.collect(IGraphPayments.PaymentTypes.IndexingFee, users.gateway, indexer, amount, subgraphDataServiceAddress, dataServiceCut); + escrow.collect(IGraphPayments.PaymentTypes.IndexingFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, dataServiceCut); vm.stopPrank(); } - function testCollect_RevertWhen_CollectorHasInsufficientAmount() public { - vm.prank(users.gateway); - escrow.approveCollector(users.verifier, 100 ether); - - address indexer = address(0xA3); - uint256 amount = 1000 ether; - - vm.startPrank(users.gateway); - token.approve(address(escrow), amount); - escrow.deposit(indexer, amount); - vm.stopPrank(); + function testCollect_RevertWhen_CollectorHasInsufficientAmount( + uint256 amount, + uint256 insufficientAmount + ) public useGateway useCollector(insufficientAmount) useDeposit(amount) { + vm.assume(insufficientAmount < amount); - vm.startPrank(users.verifier); - uint256 dataServiceCut = 30 ether; - bytes memory expectedError = abi.encodeWithSignature("GraphEscrowCollectorInsufficientAmount(uint256,uint256)", 100 ether, 1000 ether); + changePrank(users.verifier); + bytes memory expectedError = abi.encodeWithSignature("GraphEscrowCollectorInsufficientAmount(uint256,uint256)", insufficientAmount, amount); vm.expectRevert(expectedError); - escrow.collect(IGraphPayments.PaymentTypes.IndexingFee, users.gateway, indexer, 1000 ether, subgraphDataServiceAddress, dataServiceCut); - vm.stopPrank(); + escrow.collect(IGraphPayments.PaymentTypes.IndexingFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, 0); } - function testCollect_RevertWhen_SenderHasInsufficientAmountInEscrow() public { - vm.startPrank(users.gateway); - escrow.approveCollector(users.verifier, 1000 ether); - token.approve(address(escrow), 1000 ether); - escrow.deposit(users.indexer, 100 ether); - vm.stopPrank(); + function testCollect_RevertWhen_SenderHasInsufficientAmountInEscrow( + uint256 amount, + uint256 insufficientAmount + ) public useGateway useCollector(amount) useDeposit(insufficientAmount) { + vm.assume(insufficientAmount < amount); - vm.prank(users.verifier); - bytes memory expectedError = abi.encodeWithSignature("GraphEscrowInsufficientAmount(uint256,uint256)", 100 ether, 200 ether); + changePrank(users.verifier); + bytes memory expectedError = abi.encodeWithSignature("GraphEscrowInsufficientAmount(uint256,uint256)", insufficientAmount, amount); vm.expectRevert(expectedError); - escrow.collect(IGraphPayments.PaymentTypes.IndexingFee, users.gateway, users.indexer, 200 ether, subgraphDataServiceAddress, 3 ether); + escrow.collect(IGraphPayments.PaymentTypes.IndexingFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, 0); vm.stopPrank(); } } \ No newline at end of file diff --git a/packages/horizon/test/escrow/collector.t.sol b/packages/horizon/test/escrow/collector.t.sol index f4ef2b11c..be1524056 100644 --- a/packages/horizon/test/escrow/collector.t.sol +++ b/packages/horizon/test/escrow/collector.t.sol @@ -6,15 +6,10 @@ import "forge-std/Test.sol"; import { GraphEscrowTest } from "./GraphEscrow.t.sol"; contract GraphEscrowCollectorTest is GraphEscrowTest { - function setUp() public virtual override { - GraphEscrowTest.setUp(); - vm.prank(users.gateway); - escrow.approveCollector(users.verifier, 1000 ether); - } // Collector approve tests - function testCollector_Approve() public view { + function testCollector_Approve(uint256 amount) public useGateway useCollector(amount) { (bool authorized,, uint256 thawEndTimestamp) = escrow.authorizedCollectors(users.gateway, users.verifier); assertEq(authorized, true); assertEq(thawEndTimestamp, 0); @@ -22,8 +17,7 @@ contract GraphEscrowCollectorTest is GraphEscrowTest { // Collector thaw tests - function testCollector_Thaw() public { - vm.prank(users.gateway); + function testCollector_Thaw(uint256 amount) public useGateway useCollector(amount) { escrow.thawCollector(users.verifier); (bool authorized,, uint256 thawEndTimestamp) = escrow.authorizedCollectors(users.gateway, users.verifier); @@ -33,15 +27,13 @@ contract GraphEscrowCollectorTest is GraphEscrowTest { // Collector cancel thaw tests - function testCollector_CancelThaw() public { - vm.prank(users.gateway); + function testCollector_CancelThaw(uint256 amount) public useGateway useCollector(amount) { escrow.thawCollector(users.verifier); (bool authorized,, uint256 thawEndTimestamp) = escrow.authorizedCollectors(users.gateway, users.verifier); assertEq(authorized, true); assertEq(thawEndTimestamp, block.timestamp + revokeCollectorThawingPeriod); - vm.prank(users.gateway); escrow.cancelThawCollector(users.verifier); (authorized,, thawEndTimestamp) = escrow.authorizedCollectors(users.gateway, users.verifier); @@ -49,7 +41,7 @@ contract GraphEscrowCollectorTest is GraphEscrowTest { assertEq(thawEndTimestamp, 0); } - function testCollector_RevertWhen_CancelThawIsNotThawing() public { + function testCollector_RevertWhen_CancelThawIsNotThawing(uint256 amount) public useGateway useCollector(amount) { bytes memory expectedError = abi.encodeWithSignature("GraphEscrowNotThawing()"); vm.expectRevert(expectedError); escrow.cancelThawCollector(users.verifier); @@ -58,30 +50,25 @@ contract GraphEscrowCollectorTest is GraphEscrowTest { // Collector revoke tests - function testCollector_Revoke() public { - vm.startPrank(users.gateway); + function testCollector_Revoke(uint256 amount) public useGateway useCollector(amount) { escrow.thawCollector(users.verifier); skip(revokeCollectorThawingPeriod + 1); escrow.revokeCollector(users.verifier); - vm.stopPrank(); (bool authorized,,) = escrow.authorizedCollectors(users.gateway, users.verifier); assertEq(authorized, false); } - function testCollector_RevertWhen_RevokeIsNotThawing() public { + function testCollector_RevertWhen_RevokeIsNotThawing(uint256 amount) public useGateway useCollector(amount) { bytes memory expectedError = abi.encodeWithSignature("GraphEscrowNotThawing()"); vm.expectRevert(expectedError); - vm.prank(users.gateway); escrow.revokeCollector(users.verifier); } - function testCollector_RevertWhen_RevokeIsStillThawing() public { - vm.startPrank(users.gateway); + function testCollector_RevertWhen_RevokeIsStillThawing(uint256 amount) public useGateway useCollector(amount) { escrow.thawCollector(users.verifier); bytes memory expectedError = abi.encodeWithSignature("GraphEscrowStillThawing(uint256,uint256)", block.timestamp, block.timestamp + revokeCollectorThawingPeriod); vm.expectRevert(expectedError); escrow.revokeCollector(users.verifier); - vm.stopPrank(); } } \ No newline at end of file diff --git a/packages/horizon/test/escrow/deposit.t.sol b/packages/horizon/test/escrow/deposit.t.sol index 48574880e..8a83043e2 100644 --- a/packages/horizon/test/escrow/deposit.t.sol +++ b/packages/horizon/test/escrow/deposit.t.sol @@ -7,7 +7,7 @@ import { GraphEscrowTest } from "./GraphEscrow.t.sol"; contract GraphEscrowDepositTest is GraphEscrowTest { - function testDeposit_Tokens(uint256 amount) public useGateway depositTokens(amount) { + function testDeposit_Tokens(uint256 amount) public useGateway useDeposit(amount) { (uint256 indexerEscrowBalance,,) = escrow.escrowAccounts(users.gateway, users.indexer); assertEq(indexerEscrowBalance, amount); } diff --git a/packages/horizon/test/escrow/thaw.t.sol b/packages/horizon/test/escrow/thaw.t.sol index 51400feac..a9d95460b 100644 --- a/packages/horizon/test/escrow/thaw.t.sol +++ b/packages/horizon/test/escrow/thaw.t.sol @@ -7,7 +7,7 @@ import { GraphEscrowTest } from "./GraphEscrow.t.sol"; contract GraphEscrowThawTest is GraphEscrowTest { - function testThaw_Tokens(uint256 amount) public useGateway depositTokens(amount) { + function testThaw_Tokens(uint256 amount) public useGateway useDeposit(amount) { escrow.thaw(users.indexer, amount); (, uint256 amountThawing,uint256 thawEndTimestamp) = escrow.escrowAccounts(users.gateway, users.indexer); @@ -17,7 +17,7 @@ contract GraphEscrowThawTest is GraphEscrowTest { function testThaw_RevertWhen_InsufficientThawAmount( uint256 amount - ) public useGateway depositTokens(amount) { + ) public useGateway useDeposit(amount) { bytes memory expectedError = abi.encodeWithSignature("GraphEscrowInsufficientThawAmount()"); vm.expectRevert(expectedError); escrow.thaw(users.indexer, 0); @@ -25,7 +25,7 @@ contract GraphEscrowThawTest is GraphEscrowTest { function testThaw_RevertWhen_InsufficientAmount( uint256 amount - ) public useGateway depositTokens(amount) { + ) public useGateway useDeposit(amount) { uint256 overAmount = amount + 1; bytes memory expectedError = abi.encodeWithSignature("GraphEscrowInsufficientAmount(uint256,uint256)", amount, overAmount); vm.expectRevert(expectedError); diff --git a/packages/horizon/test/escrow/withdraw.t.sol b/packages/horizon/test/escrow/withdraw.t.sol index 075a16a74..be47fea83 100644 --- a/packages/horizon/test/escrow/withdraw.t.sol +++ b/packages/horizon/test/escrow/withdraw.t.sol @@ -29,7 +29,7 @@ contract GraphEscrowWithdrawTest is GraphEscrowTest { assertEq(indexerEscrowBalance, amount - thawAmount); } - function testWithdraw_RevertWhen_NotThawing(uint256 amount) public useGateway depositTokens(amount) { + function testWithdraw_RevertWhen_NotThawing(uint256 amount) public useGateway useDeposit(amount) { bytes memory expectedError = abi.encodeWithSignature("GraphEscrowNotThawing()"); vm.expectRevert(expectedError); escrow.withdraw(users.indexer); diff --git a/packages/horizon/test/payments/GraphPayments.t.sol b/packages/horizon/test/payments/GraphPayments.t.sol index 4bf00ff8d..74f255d9c 100644 --- a/packages/horizon/test/payments/GraphPayments.t.sol +++ b/packages/horizon/test/payments/GraphPayments.t.sol @@ -9,12 +9,10 @@ import { HorizonStakingSharedTest } from "../shared/horizon-staking/HorizonStaki contract GraphPaymentsTest is HorizonStakingSharedTest { - function testCollect() public { - // Setup Staking - uint256 amount = 1000 ether; - createProvision(amount); - setDelegationFeeCut(0, 100000); - + function testCollect(uint256 amount, uint256 tokensDataService) public useProvision(amount, 0, 0) useDelegationFeeCut(0, 100000) { + uint256 tokensProtocol = amount * protocolPaymentCut / MAX_PPM; + uint256 tokensDelegatoion = amount * delegationFeeCut / MAX_PPM; + vm.assume(tokensDataService < amount - tokensProtocol - tokensDelegatoion); address escrowAddress = address(escrow); // Add tokens in escrow @@ -23,18 +21,18 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { approve(address(payments), amount); // Collect payments through GraphPayments - uint256 dataServiceCut = 30 ether; // 3% uint256 indexerPreviousBalance = token.balanceOf(users.indexer); - payments.collect(IGraphPayments.PaymentTypes.IndexingFee, users.indexer, amount, subgraphDataServiceAddress, dataServiceCut); + payments.collect(IGraphPayments.PaymentTypes.QueryFee, users.indexer, amount, subgraphDataServiceAddress, tokensDataService); vm.stopPrank(); uint256 indexerBalance = token.balanceOf(users.indexer); - assertEq(indexerBalance - indexerPreviousBalance, 860 ether); + uint256 expectedPayment = amount - tokensDataService - tokensProtocol - tokensDelegatoion; + assertEq(indexerBalance - indexerPreviousBalance, expectedPayment); uint256 dataServiceBalance = token.balanceOf(subgraphDataServiceAddress); - assertEq(dataServiceBalance, 30 ether); + assertEq(dataServiceBalance, tokensDataService); uint256 delegatorBalance = staking.getDelegatedTokensAvailable(users.indexer, subgraphDataServiceAddress); - assertEq(delegatorBalance, 100 ether); + assertEq(delegatorBalance, tokensDelegatoion); } } diff --git a/packages/horizon/test/shared/horizon-staking/HorizonStaking.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStaking.t.sol index c5aec8aa2..c65032e9a 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStaking.t.sol +++ b/packages/horizon/test/shared/horizon-staking/HorizonStaking.t.sol @@ -7,6 +7,18 @@ import { GraphBaseTest } from "../../GraphBase.t.sol"; abstract contract HorizonStakingSharedTest is GraphBaseTest { + modifier useProvision(uint256 tokens, uint32 maxVerifierCut, uint64 thawingPeriod) { + vm.assume(tokens <= 10_000_000_000 ether); + vm.assume(tokens > 1e18); + _createProvision(tokens, maxVerifierCut, thawingPeriod); + _; + } + + modifier useDelegationFeeCut(uint256 paymentType, uint256 cut) { + _setDelegationFeeCut(paymentType, cut); + _; + } + /* Set Up */ function setUp() public virtual override { @@ -15,14 +27,22 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { /* Helpers */ - function createProvision(uint256 tokens) internal { + function _createProvision(uint256 tokens, uint32 maxVerifierCut, uint64 thawingPeriod) internal { vm.startPrank(users.indexer); token.approve(address(staking), tokens); staking.stakeTo(users.indexer, tokens); - staking.provision(users.indexer, subgraphDataServiceAddress, tokens, 0, 0); + staking.provision( + users.indexer, + subgraphDataServiceAddress, + tokens, + maxVerifierCut, + thawingPeriod + ); + vm.stopPrank(); } - function setDelegationFeeCut(uint256 paymentType, uint256 cut) internal { + function _setDelegationFeeCut(uint256 paymentType, uint256 cut) internal { + vm.prank(users.indexer); staking.setDelegationFeeCut(users.indexer, subgraphDataServiceAddress, paymentType, cut); } } diff --git a/packages/horizon/test/utils/Constants.sol b/packages/horizon/test/utils/Constants.sol index 8e03271ec..ff7f77dfb 100644 --- a/packages/horizon/test/utils/Constants.sol +++ b/packages/horizon/test/utils/Constants.sol @@ -2,9 +2,11 @@ pragma solidity ^0.8.24; abstract contract Constants { + uint256 internal constant MAX_PPM = 1000000; // 100% in parts per million + uint256 internal constant delegationFeeCut = 100000; // 10% in parts per million // GraphEscrow parameters uint256 internal constant withdrawEscrowThawingPeriod = 60; - // GraphPayments parameters uint256 internal constant revokeCollectorThawingPeriod = 60; + // GraphPayments parameters uint256 internal constant protocolPaymentCut = 10000; } \ No newline at end of file From 31ae8e451d1890dd93e2a3ef1ceb4acb530d128f Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Tue, 21 May 2024 18:54:18 -0300 Subject: [PATCH 2/7] fix: approving collector for smaller allowance --- packages/horizon/contracts/payments/PaymentsEscrow.sol | 9 +++++++-- packages/horizon/test/escrow/collector.t.sol | 10 ++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/horizon/contracts/payments/PaymentsEscrow.sol b/packages/horizon/contracts/payments/PaymentsEscrow.sol index f8ac5ca37..345b4309d 100644 --- a/packages/horizon/contracts/payments/PaymentsEscrow.sol +++ b/packages/horizon/contracts/payments/PaymentsEscrow.sol @@ -84,8 +84,13 @@ contract PaymentsEscrow is Multicall, GraphDirectory, IPaymentsEscrow { // approve a data service to collect funds function approveCollector(address dataService, uint256 amount) external { - authorizedCollectors[msg.sender][dataService].authorized = true; - authorizedCollectors[msg.sender][dataService].amount = amount; + IGraphEscrow.Collector storage collector = authorizedCollectors[msg.sender][dataService]; + if (collector.amount > amount) { + revert GraphEscrowCollectorInsufficientAmount(collector.amount, amount); + } + + collector.authorized = true; + collector.amount = amount; emit AuthorizedCollector(msg.sender, dataService); } diff --git a/packages/horizon/test/escrow/collector.t.sol b/packages/horizon/test/escrow/collector.t.sol index be1524056..c87545b1a 100644 --- a/packages/horizon/test/escrow/collector.t.sol +++ b/packages/horizon/test/escrow/collector.t.sol @@ -15,6 +15,16 @@ contract GraphEscrowCollectorTest is GraphEscrowTest { assertEq(thawEndTimestamp, 0); } + function testCollector_RevertWhen_ApprovingForSmallerAllowance( + uint256 amount, + uint256 smallerAmount + ) public useGateway useCollector(amount) { + vm.assume(smallerAmount < amount); + bytes memory expectedError = abi.encodeWithSignature("GraphEscrowCollectorInsufficientAmount(uint256,uint256)", amount, smallerAmount); + vm.expectRevert(expectedError); + escrow.approveCollector(users.verifier, smallerAmount); + } + // Collector thaw tests function testCollector_Thaw(uint256 amount) public useGateway useCollector(amount) { From 0c438c842ba2b3b30569fb2c9c32e4f5d9453901 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Wed, 22 May 2024 11:59:33 -0300 Subject: [PATCH 3/7] fix: type error --- packages/horizon/contracts/payments/PaymentsEscrow.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/horizon/contracts/payments/PaymentsEscrow.sol b/packages/horizon/contracts/payments/PaymentsEscrow.sol index 345b4309d..f1c684887 100644 --- a/packages/horizon/contracts/payments/PaymentsEscrow.sol +++ b/packages/horizon/contracts/payments/PaymentsEscrow.sol @@ -84,7 +84,7 @@ contract PaymentsEscrow is Multicall, GraphDirectory, IPaymentsEscrow { // approve a data service to collect funds function approveCollector(address dataService, uint256 amount) external { - IGraphEscrow.Collector storage collector = authorizedCollectors[msg.sender][dataService]; + Collector storage collector = authorizedCollectors[msg.sender][dataService]; if (collector.amount > amount) { revert GraphEscrowCollectorInsufficientAmount(collector.amount, amount); } From 5247ebf93ecf04fc276e091f6c0f7175cce0d04f Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Wed, 22 May 2024 18:26:54 -0300 Subject: [PATCH 4/7] fix: rebase and fixes --- .../contracts/upgrades/GraphProxy.sol | 5 +- .../contracts/upgrades/GraphProxyAdmin.sol | 2 +- .../contracts/upgrades/GraphProxyStorage.sol | 2 +- .../contracts/payments/PaymentsEscrow.sol | 4 +- packages/horizon/test/GraphBase.t.sol | 57 ++++++++----------- packages/horizon/test/HorizonStaking.t.sol | 33 ----------- packages/horizon/test/HorizonStaking.ts | 38 ------------- 7 files changed, 28 insertions(+), 113 deletions(-) delete mode 100644 packages/horizon/test/HorizonStaking.t.sol delete mode 100644 packages/horizon/test/HorizonStaking.ts diff --git a/packages/contracts/contracts/upgrades/GraphProxy.sol b/packages/contracts/contracts/upgrades/GraphProxy.sol index 7d227b065..c5c01ca8f 100644 --- a/packages/contracts/contracts/upgrades/GraphProxy.sol +++ b/packages/contracts/contracts/upgrades/GraphProxy.sol @@ -1,8 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.7.6; - -import { Address } from "@openzeppelin/contracts/utils/Address.sol"; +pragma solidity >=0.6.12 <0.9.0; import { GraphProxyStorage } from "./GraphProxyStorage.sol"; @@ -160,7 +158,6 @@ contract GraphProxy is GraphProxyStorage, IGraphProxy { */ function _acceptUpgrade() internal { address _pendingImplementation = _getPendingImplementation(); - require(Address.isContract(_pendingImplementation), "Impl must be a contract"); require(_pendingImplementation != address(0), "Impl cannot be zero address"); require(msg.sender == _pendingImplementation, "Only pending implementation"); diff --git a/packages/contracts/contracts/upgrades/GraphProxyAdmin.sol b/packages/contracts/contracts/upgrades/GraphProxyAdmin.sol index 7d809d5ec..8fb735901 100644 --- a/packages/contracts/contracts/upgrades/GraphProxyAdmin.sol +++ b/packages/contracts/contracts/upgrades/GraphProxyAdmin.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.7.6; +pragma solidity >=0.6.12 <0.9.0; import { Governed } from "../governance/Governed.sol"; diff --git a/packages/contracts/contracts/upgrades/GraphProxyStorage.sol b/packages/contracts/contracts/upgrades/GraphProxyStorage.sol index 05b922647..42e2a0545 100644 --- a/packages/contracts/contracts/upgrades/GraphProxyStorage.sol +++ b/packages/contracts/contracts/upgrades/GraphProxyStorage.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.7.6; +pragma solidity >=0.6.12 <0.9.0; /** * @title Graph Proxy Storage diff --git a/packages/horizon/contracts/payments/PaymentsEscrow.sol b/packages/horizon/contracts/payments/PaymentsEscrow.sol index f1c684887..9404190be 100644 --- a/packages/horizon/contracts/payments/PaymentsEscrow.sol +++ b/packages/horizon/contracts/payments/PaymentsEscrow.sol @@ -78,8 +78,8 @@ contract PaymentsEscrow is Multicall, GraphDirectory, IPaymentsEscrow { revert GraphEscrowThawingPeriodTooLong(withdrawEscrowThawingPeriod, MAX_THAWING_PERIOD); } - revokeCollectorThawingPeriod = revokeCollectorThawingPeriod; - withdrawEscrowThawingPeriod = withdrawEscrowThawingPeriod; + REVOKE_COLLECTOR_THAWING_PERIOD = revokeCollectorThawingPeriod; + WITHDRAW_ESCROW_THAWING_PERIOD = withdrawEscrowThawingPeriod; } // approve a data service to collect funds diff --git a/packages/horizon/test/GraphBase.t.sol b/packages/horizon/test/GraphBase.t.sol index 51a5b4d5a..f348ca3f0 100644 --- a/packages/horizon/test/GraphBase.t.sol +++ b/packages/horizon/test/GraphBase.t.sol @@ -3,6 +3,8 @@ pragma solidity ^0.8.24; import "forge-std/Test.sol"; +import { GraphProxyAdmin } from "@graphprotocol/contracts/contracts/upgrades/GraphProxyAdmin.sol"; +import { GraphProxy } from "@graphprotocol/contracts/contracts/upgrades/GraphProxy.sol"; import { Controller } from "@graphprotocol/contracts/contracts/governance/Controller.sol"; import { PaymentsEscrow } from "contracts/payments/PaymentsEscrow.sol"; @@ -18,6 +20,7 @@ abstract contract GraphBaseTest is Test, Constants { /* Contracts */ + GraphProxyAdmin public proxyAdmin; Controller public controller; MockGRTToken public token; GraphPayments public payments; @@ -41,6 +44,7 @@ abstract contract GraphBaseTest is Test, Constants { function setUp() public virtual { // Deploy ERC20 token + vm.prank(users.deployer); token = new MockGRTToken(); // Setup Users @@ -68,10 +72,13 @@ abstract contract GraphBaseTest is Test, Constants { function deployProtocolContracts() private { vm.startPrank(users.governor); + proxyAdmin = new GraphProxyAdmin(); controller = new Controller(); - controller.setContractProxy(keccak256("GraphToken"), address(token)); vm.stopPrank(); + // Staking Proxy + vm.prank(users.deployer); + GraphProxy stakingProxy = new GraphProxy(address(0), address(proxyAdmin)); // GraphPayments predict address bytes32 saltPayments = keccak256("GraphPaymentsSalt"); @@ -101,39 +108,16 @@ abstract contract GraphBaseTest is Test, Constants { users.deployer ); - // HorizonStakingExtension predict address - bytes32 saltHorizonStakingExtension = keccak256("HorizonStakingExtensionSalt"); - bytes32 horizonStakingExtensionHash = keccak256(bytes.concat( - vm.getCode("HorizonStakingExtension.sol:HorizonStakingExtension"), - abi.encode(address(controller), subgraphDataServiceAddress) - )); - address predictedAddressHorizonStakingExtension = vm.computeCreate2Address( - saltHorizonStakingExtension, - horizonStakingExtensionHash, - users.deployer - ); - - // HorizonStaking predict address - bytes32 saltHorizonStaking = keccak256("saltHorizonStaking"); - bytes32 horizonStakingHash = keccak256(bytes.concat( - vm.getCode("HorizonStaking.sol:HorizonStaking"), - abi.encode( - address(controller), - predictedAddressHorizonStakingExtension, - subgraphDataServiceAddress - ) - )); - address predictedAddressHorizonStaking = vm.computeCreate2Address( - saltHorizonStaking, - horizonStakingHash, - users.deployer - ); - // Setup controller vm.startPrank(users.governor); - controller.setContractProxy(keccak256("GraphEscrow"), predictedAddressEscrow); + controller.setContractProxy(keccak256("GraphToken"), address(token)); + controller.setContractProxy(keccak256("PaymentsEscrow"), predictedAddressEscrow); controller.setContractProxy(keccak256("GraphPayments"), predictedPaymentsAddress); - controller.setContractProxy(keccak256("Staking"), address(predictedAddressHorizonStaking)); + controller.setContractProxy(keccak256("Staking"), address(stakingProxy)); + controller.setContractProxy(keccak256("EpochManager"), makeAddr("EpochManager")); + controller.setContractProxy(keccak256("RewardsManager"), makeAddr("RewardsManager")); + controller.setContractProxy(keccak256("Curation"), makeAddr("Curation")); + controller.setContractProxy(keccak256("GraphTokenGateway"), makeAddr("GraphTokenGateway")); vm.stopPrank(); vm.startPrank(users.deployer); @@ -146,16 +130,21 @@ abstract contract GraphBaseTest is Test, Constants { revokeCollectorThawingPeriod, withdrawEscrowThawingPeriod ); - stakingExtension = new HorizonStakingExtension{salt: saltHorizonStakingExtension}( + stakingExtension = new HorizonStakingExtension( address(controller), subgraphDataServiceAddress ); - stakingBase = new HorizonStaking{salt: saltHorizonStaking}( + stakingBase = new HorizonStaking( address(controller), address(stakingExtension), subgraphDataServiceAddress ); - staking = IHorizonStaking(address(stakingBase)); + vm.stopPrank(); + + vm.startPrank(users.governor); + proxyAdmin.upgrade(stakingProxy, address(stakingBase)); + proxyAdmin.acceptProxy(stakingBase, stakingProxy); + staking = IHorizonStaking(address(stakingProxy)); vm.stopPrank(); } diff --git a/packages/horizon/test/HorizonStaking.t.sol b/packages/horizon/test/HorizonStaking.t.sol deleted file mode 100644 index 55088f0fe..000000000 --- a/packages/horizon/test/HorizonStaking.t.sol +++ /dev/null @@ -1,33 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0 -pragma solidity 0.8.24; - -import "forge-std/Test.sol"; -import "forge-std/console.sol"; -import { HorizonStaking } from "../contracts/staking/HorizonStaking.sol"; -import { ControllerMock } from "../contracts/mocks/ControllerMock.sol"; -import { HorizonStakingExtension } from "../contracts/staking/HorizonStakingExtension.sol"; -import { ExponentialRebates } from "../contracts/staking/libraries/ExponentialRebates.sol"; -import { IHorizonStaking } from "../contracts/interfaces/IHorizonStaking.sol"; - -contract HorizonStakingTest is Test { - HorizonStakingExtension ext; - IHorizonStaking staking; - ControllerMock controller; - - function setUp() public { - console.log("Deploying Controller mock"); - controller = new ControllerMock(address(0x1)); - console.log("Deploying HorizonStaking"); - ext = new HorizonStakingExtension(address(controller), address(0x1)); - staking = IHorizonStaking(address(new HorizonStaking(address(controller), address(ext), address(0x1)))); - } - - function test_SetGlobalOperator() public { - address operator = address(0x1337); - address dataService = address(0x1338); - address serviceProvider = address(this); - - staking.setOperator(operator, dataService, true); - assertTrue(staking.isAuthorized(operator, serviceProvider, dataService)); - } -} diff --git a/packages/horizon/test/HorizonStaking.ts b/packages/horizon/test/HorizonStaking.ts deleted file mode 100644 index 28eb281f7..000000000 --- a/packages/horizon/test/HorizonStaking.ts +++ /dev/null @@ -1,38 +0,0 @@ -import hardhat from 'hardhat' - -import { expect } from 'chai' -import { loadFixture } from '@nomicfoundation/hardhat-toolbox/network-helpers' -import { ZeroAddress } from 'ethers' -import { IHorizonStaking } from '../typechain-types' - -const ethers = hardhat.ethers - -describe('HorizonStaking', function () { - async function deployFixture() { - const [owner] = await ethers.getSigners() - const ControllerMock = await ethers.getContractFactory('ControllerMock') - const controller = await ControllerMock.deploy(owner.address) - await controller.waitForDeployment() - const ExponentialRebates = await ethers.getContractFactory('ExponentialRebates') - const exponentialRebates = await ExponentialRebates.deploy() - await exponentialRebates.waitForDeployment() - const HorizonStakingExtension = await ethers.getContractFactory('HorizonStakingExtension', { libraries: { ExponentialRebates: exponentialRebates.target } }) - const horizonStakingExtension = await HorizonStakingExtension.deploy(controller.target, ZeroAddress) - await horizonStakingExtension.waitForDeployment() - const HorizonStaking = await ethers.getContractFactory('HorizonStaking') - const horizonStakingContract = await HorizonStaking.deploy(controller.target, horizonStakingExtension.target, ZeroAddress) - await horizonStakingContract.waitForDeployment() - const horizonStaking = (await ethers.getContractAt('IHorizonStaking', horizonStakingContract.target)) as unknown as IHorizonStaking - return { horizonStaking, owner } - } - - describe('setOperator', function () { - it('adds an operator', async function () { - const { horizonStaking, owner } = await loadFixture(deployFixture) - const verifier = ethers.Wallet.createRandom().address - const operator = ethers.Wallet.createRandom().address - await horizonStaking.connect(owner).setOperator(operator, verifier, true) - expect(await horizonStaking.isAuthorized(operator, owner, verifier)).to.be.true - }) - }) -}) From 8947c0780f8c11d800986a4e45095c1745db597e Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Wed, 22 May 2024 18:39:34 -0300 Subject: [PATCH 5/7] fix: more rebase fixes --- packages/horizon/test/GraphBase.t.sol | 2 ++ packages/horizon/test/escrow/collect.t.sol | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/horizon/test/GraphBase.t.sol b/packages/horizon/test/GraphBase.t.sol index f348ca3f0..3055c9245 100644 --- a/packages/horizon/test/GraphBase.t.sol +++ b/packages/horizon/test/GraphBase.t.sol @@ -118,6 +118,8 @@ abstract contract GraphBaseTest is Test, Constants { controller.setContractProxy(keccak256("RewardsManager"), makeAddr("RewardsManager")); controller.setContractProxy(keccak256("Curation"), makeAddr("Curation")); controller.setContractProxy(keccak256("GraphTokenGateway"), makeAddr("GraphTokenGateway")); + controller.setContractProxy(keccak256("BridgeEscrow"), makeAddr("BridgeEscrow")); + controller.setContractProxy(keccak256("GraphProxyAdmin"), makeAddr("GraphProxyAdmin")); vm.stopPrank(); vm.startPrank(users.deployer); diff --git a/packages/horizon/test/escrow/collect.t.sol b/packages/horizon/test/escrow/collect.t.sol index db7f549e8..4c66cfc0c 100644 --- a/packages/horizon/test/escrow/collect.t.sol +++ b/packages/horizon/test/escrow/collect.t.sol @@ -20,7 +20,7 @@ contract GraphEscrowCollectTest is GraphEscrowTest { uint256 indexerPreviousBalance = token.balanceOf(users.indexer); vm.prank(users.verifier); - escrow.collect(IGraphPayments.PaymentTypes.IndexingFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, tokensDataService); + escrow.collect(IGraphPayments.PaymentTypes.QueryFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, tokensDataService); uint256 indexerBalance = token.balanceOf(users.indexer); uint256 indexerExpectedPayment = amount - tokensDataService - tokensProtocol - tokensDelegatoion; @@ -33,7 +33,7 @@ contract GraphEscrowCollectTest is GraphEscrowTest { uint256 dataServiceCut = 30000; // 3% bytes memory expectedError = abi.encodeWithSignature("GraphEscrowCollectorNotAuthorized(address,address)", users.gateway, users.verifier); vm.expectRevert(expectedError); - escrow.collect(IGraphPayments.PaymentTypes.IndexingFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, dataServiceCut); + escrow.collect(IGraphPayments.PaymentTypes.QueryFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, dataServiceCut); vm.stopPrank(); } @@ -46,7 +46,7 @@ contract GraphEscrowCollectTest is GraphEscrowTest { changePrank(users.verifier); bytes memory expectedError = abi.encodeWithSignature("GraphEscrowCollectorInsufficientAmount(uint256,uint256)", insufficientAmount, amount); vm.expectRevert(expectedError); - escrow.collect(IGraphPayments.PaymentTypes.IndexingFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, 0); + escrow.collect(IGraphPayments.PaymentTypes.QueryFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, 0); } function testCollect_RevertWhen_SenderHasInsufficientAmountInEscrow( @@ -58,7 +58,7 @@ contract GraphEscrowCollectTest is GraphEscrowTest { changePrank(users.verifier); bytes memory expectedError = abi.encodeWithSignature("GraphEscrowInsufficientAmount(uint256,uint256)", insufficientAmount, amount); vm.expectRevert(expectedError); - escrow.collect(IGraphPayments.PaymentTypes.IndexingFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, 0); + escrow.collect(IGraphPayments.PaymentTypes.QueryFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, 0); vm.stopPrank(); } } \ No newline at end of file From 3b39aa5133abfa4416c8cc6cfaaa8ee258f31f55 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Thu, 23 May 2024 11:31:22 -0300 Subject: [PATCH 6/7] fix: renamed error, move code to trigger revert --- .../contracts/payments/GraphPayments.sol | 22 ++++++++------- .../horizon/test/payments/GraphPayments.t.sol | 28 ++++++++++++++++++- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/packages/horizon/contracts/payments/GraphPayments.sol b/packages/horizon/contracts/payments/GraphPayments.sol index 8fddc6a07..c5818ca0c 100644 --- a/packages/horizon/contracts/payments/GraphPayments.sol +++ b/packages/horizon/contracts/payments/GraphPayments.sol @@ -30,7 +30,7 @@ contract GraphPayments is Multicall, GraphDirectory, GraphPaymentsStorageV1Stora error GraphPaymentsNotThawing(); error GraphPaymentsStillThawing(uint256 currentTimestamp, uint256 thawEndTimestamp); error GraphPaymentsCollectorNotAuthorized(address sender, address dataService); - error GraphPaymentsCollectorInsufficientAmount(uint256 available, uint256 required); + error GraphPaymentsInsufficientAmount(uint256 available, uint256 required); // -- Events -- @@ -54,25 +54,27 @@ contract GraphPayments is Multicall, GraphDirectory, GraphPaymentsStorageV1Stora ) external { _graphToken().pullTokens(msg.sender, tokens); - // Pay protocol cut + // Calculate cuts uint256 tokensProtocol = (tokens * PROTOCOL_PAYMENT_CUT) / MAX_PPM; + uint256 delegationFeeCut = _graphStaking().getDelegationFeeCut(receiver, dataService, uint8(paymentType)); + uint256 tokensDelegationPool = (tokens * delegationFeeCut) / MAX_PPM; + uint256 totalCut = tokensProtocol + tokensDataService + tokensDelegationPool; + if (totalCut > tokens) { + revert GraphPaymentsInsufficientAmount(tokens, totalCut); + } + + // Pay protocol cut _graphToken().burnTokens(tokensProtocol); // Pay data service cut _graphToken().pushTokens(dataService, tokensDataService); - // Get delegation cut - uint256 delegationFeeCut = _graphStaking().getDelegationFeeCut(receiver, dataService, uint8(paymentType)); - uint256 tokensDelegationPool = (tokens * delegationFeeCut) / MAX_PPM; + // Pay delegators if (tokensDelegationPool > 0) { _graphStaking().addToDelegationPool(receiver, dataService, tokensDelegationPool); } - // Pay the rest to the receiver - uint256 totalCut = tokensProtocol + tokensDataService + tokensDelegationPool; - if (totalCut > tokens) { - revert GraphPaymentsCollectorInsufficientAmount(tokens, totalCut); - } + // Pay receiver uint256 tokensReceiverRemaining = tokens - totalCut; _graphToken().pushTokens(receiver, tokensReceiverRemaining); diff --git a/packages/horizon/test/payments/GraphPayments.t.sol b/packages/horizon/test/payments/GraphPayments.t.sol index 74f255d9c..519f6e105 100644 --- a/packages/horizon/test/payments/GraphPayments.t.sol +++ b/packages/horizon/test/payments/GraphPayments.t.sol @@ -9,7 +9,10 @@ import { HorizonStakingSharedTest } from "../shared/horizon-staking/HorizonStaki contract GraphPaymentsTest is HorizonStakingSharedTest { - function testCollect(uint256 amount, uint256 tokensDataService) public useProvision(amount, 0, 0) useDelegationFeeCut(0, 100000) { + function testCollect( + uint256 amount, + uint256 tokensDataService + ) public useProvision(amount, 0, 0) useDelegationFeeCut(0, delegationFeeCut) { uint256 tokensProtocol = amount * protocolPaymentCut / MAX_PPM; uint256 tokensDelegatoion = amount * delegationFeeCut / MAX_PPM; vm.assume(tokensDataService < amount - tokensProtocol - tokensDelegatoion); @@ -35,4 +38,27 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { uint256 delegatorBalance = staking.getDelegatedTokensAvailable(users.indexer, subgraphDataServiceAddress); assertEq(delegatorBalance, tokensDelegatoion); } + + function testCollect_RevertWhen_InsufficientAmount( + uint256 amount, + uint256 tokensDataService + ) public useProvision(amount, 0, 0) useDelegationFeeCut(0, delegationFeeCut) { + vm.assume(tokensDataService <= 10_000_000_000 ether); + vm.assume(tokensDataService > amount); + + address escrowAddress = address(escrow); + mint(escrowAddress, amount); + vm.startPrank(escrowAddress); + approve(address(payments), amount); + + bytes memory expectedError; + { + uint256 tokensProtocol = amount * protocolPaymentCut / MAX_PPM; + uint256 tokensDelegatoion = amount * delegationFeeCut / MAX_PPM; + uint256 requiredAmount = tokensDataService + tokensProtocol + tokensDelegatoion; + expectedError = abi.encodeWithSignature("GraphPaymentsInsufficientAmount(uint256,uint256)", amount, requiredAmount); + } + vm.expectRevert(expectedError); + payments.collect(IGraphPayments.PaymentTypes.QueryFee, users.indexer, amount, subgraphDataServiceAddress, tokensDataService); + } } From 3a66efa829d081494b2ecb69d9a61f727d4cc8c0 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Thu, 23 May 2024 11:34:33 -0300 Subject: [PATCH 7/7] fix: removed unwanted import --- packages/horizon/contracts/data-service/GraphDirectory.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/horizon/contracts/data-service/GraphDirectory.sol b/packages/horizon/contracts/data-service/GraphDirectory.sol index 805136db1..f05d9db66 100644 --- a/packages/horizon/contracts/data-service/GraphDirectory.sol +++ b/packages/horizon/contracts/data-service/GraphDirectory.sol @@ -16,7 +16,6 @@ import { IGraphProxyAdmin } from "@graphprotocol/contracts/contracts/upgrades/IG import { ICuration } from "@graphprotocol/contracts/contracts/curation/ICuration.sol"; -import "forge-std/console.sol"; /** * @title GraphDirectory contract * @notice This contract is meant to be inherited by other contracts that