diff --git a/packages/horizon/contracts/interfaces/IGraphPayments.sol b/packages/horizon/contracts/interfaces/IGraphPayments.sol index f446d6f52..eaac08ae4 100644 --- a/packages/horizon/contracts/interfaces/IGraphPayments.sol +++ b/packages/horizon/contracts/interfaces/IGraphPayments.sol @@ -23,27 +23,30 @@ interface IGraphPayments { * @param payer The address of the payer * @param receiver The address of the receiver * @param dataService The address of the data service - * @param tokensReceiver Amount of tokens for the receiver - * @param tokensDelegationPool Amount of tokens for delegators - * @param tokensDataService Amount of tokens for the data service + * @param tokens The total amount of tokens being collected * @param tokensProtocol Amount of tokens charged as protocol tax + * @param tokensDataService Amount of tokens for the data service + * @param tokensDelegationPool Amount of tokens for delegators + * @param tokensReceiver Amount of tokens for the receiver */ - event PaymentCollected( + event GraphPaymentCollected( address indexed payer, address indexed receiver, address indexed dataService, - uint256 tokensReceiver, - uint256 tokensDelegationPool, + uint256 tokens, + uint256 tokensProtocol, uint256 tokensDataService, - uint256 tokensProtocol + uint256 tokensDelegationPool, + uint256 tokensReceiver ); /** - * @notice Thrown when there are insufficient tokens to pay the required amount - * @param tokens The amount of tokens available - * @param minTokens The amount of tokens being collected + * @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 GraphPaymentsInsufficientTokens(uint256 tokens, uint256 minTokens); + error GraphPaymentsBadAccounting(uint256 tokens, uint256 tokensCalculated); /** * @notice Thrown when the protocol payment cut is invalid @@ -51,6 +54,12 @@ interface IGraphPayments { */ error GraphPaymentsInvalidProtocolPaymentCut(uint256 protocolPaymentCut); + /** + * @notice Thrown when trying to use a cut that is not expressed in PPM + * @param cut The cut + */ + error GraphPaymentsInvalidCut(uint256 cut); + /** * @notice Collects funds from a payer. * It will pay cuts to all relevant parties and forward the rest to the receiver. @@ -58,13 +67,13 @@ interface IGraphPayments { * @param receiver The address of the receiver * @param tokens The amount of tokens being collected * @param dataService The address of the data service - * @param tokensDataService The amount of tokens that should be sent to the data service + * @param dataServiceCut The data service cut in PPM */ function collect( PaymentTypes paymentType, address receiver, uint256 tokens, address dataService, - uint256 tokensDataService + uint256 dataServiceCut ) external; } diff --git a/packages/horizon/contracts/interfaces/IPaymentsCollector.sol b/packages/horizon/contracts/interfaces/IPaymentsCollector.sol index 85d09d59f..61854bb71 100644 --- a/packages/horizon/contracts/interfaces/IPaymentsCollector.sol +++ b/packages/horizon/contracts/interfaces/IPaymentsCollector.sol @@ -19,17 +19,15 @@ interface IPaymentsCollector { * @param paymentType The payment type collected as defined by {IGraphPayments} * @param payer The address of the payer * @param receiver The address of the receiver - * @param tokensReceiver The amount of tokens received by the receiver * @param dataService The address of the data service - * @param tokensDataService The amount of tokens received by the data service + * @param tokens The amount of tokens being collected */ event PaymentCollected( IGraphPayments.PaymentTypes indexed paymentType, address indexed payer, address receiver, - uint256 tokensReceiver, address indexed dataService, - uint256 tokensDataService + uint256 tokens ); /** diff --git a/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol b/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol index c1eb7707a..760a086a7 100644 --- a/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol +++ b/packages/horizon/contracts/interfaces/IPaymentsEscrow.sol @@ -25,50 +25,6 @@ interface IPaymentsEscrow { uint256 thawEndTimestamp; } - /// @notice Details for a payer-collector pair - /// @dev Collectors can be removed only after a thawing period - struct Collector { - // Amount of tokens the collector is allowed to collect - uint256 allowance; - // Timestamp at which the collector thawing period ends (zero if not thawing) - uint256 thawEndTimestamp; - } - - /** - * @notice Emitted when a payer authorizes a collector to collect funds - * @param payer The address of the payer - * @param collector The address of the collector - * @param addedAllowance The amount of tokens added to the collector's allowance - * @param newTotalAllowance The new total allowance after addition - */ - event AuthorizedCollector( - address indexed payer, - address indexed collector, - uint256 addedAllowance, - uint256 newTotalAllowance - ); - - /** - * @notice Emitted when a payer thaws a collector - * @param payer The address of the payer - * @param collector The address of the collector - */ - event ThawCollector(address indexed payer, address indexed collector); - - /** - * @notice Emitted when a payer cancels the thawing of a collector - * @param payer The address of the payer - * @param collector The address of the collector - */ - event CancelThawCollector(address indexed payer, address indexed collector); - - /** - * @notice Emitted when a payer revokes a collector authorization. - * @param payer The address of the payer - * @param collector The address of the collector - */ - event RevokeCollector(address indexed payer, address indexed collector); - /** * @notice Emitted when a payer deposits funds into the escrow for a payer-collector-receiver tuple * @param payer The address of the payer @@ -152,13 +108,6 @@ interface IPaymentsEscrow { */ error PaymentsEscrowThawingPeriodTooLong(uint256 thawingPeriod, uint256 maxWaitPeriod); - /** - * @notice Thrown when a collector has insufficient allowance to collect funds - * @param allowance The current allowance - * @param minAllowance The minimum required allowance - */ - error PaymentsEscrowInsufficientAllowance(uint256 allowance, uint256 minAllowance); - /** * @notice Thrown when the contract balance is not consistent with the collection amount * @param balanceBefore The balance before the collection @@ -172,54 +121,6 @@ interface IPaymentsEscrow { */ error PaymentsEscrowInvalidZeroTokens(); - /** - * @notice Authorize a collector to collect funds from the payer's escrow - * @dev This function can only be used to increase the allowance of a collector. - * To reduce it the authorization must be revoked and a new one must be created. - * - * Requirements: - * - `allowance` must be greater than zero - * - * Emits an {AuthorizedCollector} event - * - * @param collector The address of the collector - * @param allowance The amount of tokens to add to the collector's allowance - */ - function approveCollector(address collector, uint256 allowance) external; - - /** - * @notice Thaw a collector's collector authorization - * @dev The thawing period is defined by the `REVOKE_COLLECTOR_THAWING_PERIOD` constant - * - * Emits a {ThawCollector} event - * - * @param collector The address of the collector - */ - function thawCollector(address collector) external; - - /** - * @notice Cancel a collector's authorization thawing - * @dev Requirements: - * - `collector` must be thawing - * - * Emits a {CancelThawCollector} event - * - * @param collector The address of the collector - */ - function cancelThawCollector(address collector) external; - - /** - * @notice Revoke a collector's authorization. - * Removes the collector from the list of authorized collectors. - * @dev Requirements: - * - `collector` must have thawed - * - * Emits a {RevokeCollector} event - * - * @param collector The address of the collector - */ - function revokeCollector(address collector) external; - /** * @notice Deposits funds into the escrow for a payer-collector-receiver tuple, where * the payer is the transaction caller. @@ -277,8 +178,6 @@ interface IPaymentsEscrow { * @notice Collects funds from the payer-collector-receiver's escrow and sends them to {GraphPayments} for * distribution using the Graph Horizon Payments protocol. * The function will revert if there are not enough funds in the escrow. - * @dev Requirements: - * - `collector` needs to be authorized by the payer and have enough allowance * * Emits an {EscrowCollected} event * @@ -287,7 +186,7 @@ interface IPaymentsEscrow { * @param receiver The address of the receiver * @param tokens The amount of tokens to collect * @param dataService The address of the data service - * @param tokensDataService The amount of tokens that {GraphPayments} should send to the data service + * @param dataServiceCut The data service cut in PPM that {GraphPayments} should send */ function collect( IGraphPayments.PaymentTypes paymentType, @@ -295,11 +194,12 @@ interface IPaymentsEscrow { address receiver, uint256 tokens, address dataService, - uint256 tokensDataService + uint256 dataServiceCut ) external; /** * @notice Get the balance of a payer-collector-receiver tuple + * This function will return 0 if the current balance is less than the amount of funds being thawed. * @param payer The address of the payer * @param collector The address of the collector * @param receiver The address of the receiver diff --git a/packages/horizon/contracts/interfaces/ITAPCollector.sol b/packages/horizon/contracts/interfaces/ITAPCollector.sol index dd557de53..194edb11a 100644 --- a/packages/horizon/contracts/interfaces/ITAPCollector.sol +++ b/packages/horizon/contracts/interfaces/ITAPCollector.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.27; import { IPaymentsCollector } from "./IPaymentsCollector.sol"; +import { IGraphPayments } from "./IGraphPayments.sol"; /** * @title Interface for the {TAPCollector} contract @@ -18,10 +19,14 @@ interface ITAPCollector is IPaymentsCollector { address payer; // Timestamp at which thawing period ends (zero if not thawing) uint256 thawEndTimestamp; + // Whether the signer authorization was revoked + bool revoked; } /// @notice The Receipt Aggregate Voucher (RAV) struct struct ReceiptAggregateVoucher { + // 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 service provider the RAV was issued to @@ -119,6 +124,19 @@ interface ITAPCollector is IPaymentsCollector { */ error TAPCollectorSignerNotAuthorizedByPayer(address payer, address signer); + /** + * Thrown when the attempting to revoke a signer that was already revoked + * @param signer The address of the signer + */ + error TAPCollectorAuthorizationAlreadyRevoked(address payer, address signer); + + /** + * Thrown when attempting to thaw a signer that is already thawing + * @param signer The address of the signer + * @param thawEndTimestamp The timestamp at which the thawing period ends + */ + error TAPCollectorSignerAlreadyThawing(address signer, uint256 thawEndTimestamp); + /** * Thrown when the signer is not thawing * @param signer The address of the signer @@ -137,6 +155,19 @@ interface ITAPCollector is IPaymentsCollector { */ error TAPCollectorInvalidRAVSigner(); + /** + * Thrown when the RAV payer does not match the signers authorized payer + * @param authorizedPayer The address of the authorized payer + * @param ravPayer The address of the RAV payer + */ + error TAPCollectorInvalidRAVPayer(address authorizedPayer, address ravPayer); + + /** + * Thrown when the RAV is for a data service the service provider has no provision for + * @param dataService The address of the data service + */ + error TAPCollectorUnauthorizedDataService(address dataService); + /** * Thrown when the caller is not the data service the RAV was issued to * @param caller The address of the caller @@ -153,7 +184,15 @@ interface ITAPCollector is IPaymentsCollector { error TAPCollectorInconsistentRAVTokens(uint256 tokens, uint256 tokensCollected); /** - * @notice Authorize a signer to sign on behalf of the payer + * Thrown when the attempting to collect more tokens than what it's owed + * @param tokensToCollect The amount of tokens to collect + * @param maxTokensToCollect The maximum amount of tokens to collect + */ + error TAPCollectorInvalidTokensToCollectAmount(uint256 tokensToCollect, uint256 maxTokensToCollect); + + /** + * @notice Authorize a signer to sign on behalf of the payer. + * A signer can not be authorized for multiple payers even after revoking previous authorizations. * @dev Requirements: * - `signer` must not be already authorized * - `proofDeadline` must be greater than the current timestamp @@ -213,4 +252,21 @@ interface ITAPCollector is IPaymentsCollector { * @return The hash of the RAV. */ function encodeRAV(ReceiptAggregateVoucher calldata rav) external view returns (bytes32); + + /** + * @notice See {IPaymentsCollector.collect} + * This variant adds the ability to partially collect a RAV by specifying the amount of tokens to collect. + * + * Requirements: + * - The amount of tokens to collect must be less than or equal to the total amount of tokens in the RAV minus + * the tokens already collected. + * @param paymentType The payment type to collect + * @param data Additional data required for the payment collection + * @param tokensToCollect The amount of tokens to collect + */ + function collect( + IGraphPayments.PaymentTypes paymentType, + bytes calldata data, + uint256 tokensToCollect + ) external returns (uint256); } diff --git a/packages/horizon/contracts/payments/GraphPayments.sol b/packages/horizon/contracts/payments/GraphPayments.sol index 0ebe566a5..0dd06ef72 100644 --- a/packages/horizon/contracts/payments/GraphPayments.sol +++ b/packages/horizon/contracts/payments/GraphPayments.sol @@ -32,7 +32,7 @@ contract GraphPayments is Initializable, MulticallUpgradeable, GraphDirectory, I * @param protocolPaymentCut The protocol tax in PPM */ constructor(address controller, uint256 protocolPaymentCut) GraphDirectory(controller) { - require(PPMMath.isValidPPM(protocolPaymentCut), GraphPaymentsInvalidProtocolPaymentCut(protocolPaymentCut)); + require(PPMMath.isValidPPM(protocolPaymentCut), GraphPaymentsInvalidCut(protocolPaymentCut)); PROTOCOL_PAYMENT_CUT = protocolPaymentCut; _disableInitializers(); } @@ -52,41 +52,54 @@ contract GraphPayments is Initializable, MulticallUpgradeable, GraphDirectory, I address receiver, uint256 tokens, address dataService, - uint256 tokensDataService + uint256 dataServiceCut ) external { + require(PPMMath.isValidPPM(dataServiceCut), GraphPaymentsInvalidCut(dataServiceCut)); + + // Pull tokens from the sender _graphToken().pullTokens(msg.sender, tokens); - // Calculate cuts - uint256 tokensProtocol = tokens.mulPPM(PROTOCOL_PAYMENT_CUT); - uint256 delegationFeeCut = _graphStaking().getDelegationFeeCut(receiver, dataService, paymentType); - uint256 tokensDelegationPool = tokens.mulPPM(delegationFeeCut); - uint256 totalCut = tokensProtocol + tokensDataService + tokensDelegationPool; - require(tokens >= totalCut, GraphPaymentsInsufficientTokens(tokens, totalCut)); + // Calculate token amounts for each party + // Order matters: protocol -> data service -> delegators -> receiver + // Note the substractions should not underflow as we are only deducting a percentage of the remainder + uint256 tokensRemaining = tokens; + + uint256 tokensProtocol = tokensRemaining.mulPPMRoundUp(PROTOCOL_PAYMENT_CUT); + tokensRemaining = tokensRemaining - tokensProtocol; + + 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)); - // Pay protocol cut + // Pay all parties _graphToken().burnTokens(tokensProtocol); - // Pay data service cut _graphToken().pushTokens(dataService, tokensDataService); - // Pay delegators if (tokensDelegationPool > 0) { _graphToken().approve(address(_graphStaking()), tokensDelegationPool); _graphStaking().addToDelegationPool(receiver, dataService, tokensDelegationPool); } - // Pay receiver - uint256 tokensReceiverRemaining = tokens - totalCut; - _graphToken().pushTokens(receiver, tokensReceiverRemaining); + _graphToken().pushTokens(receiver, tokensRemaining); - emit PaymentCollected( + emit GraphPaymentCollected( msg.sender, receiver, dataService, - tokensReceiverRemaining, - tokensDelegationPool, + tokens, + tokensProtocol, tokensDataService, - tokensProtocol + tokensDelegationPool, + tokensRemaining ); } } diff --git a/packages/horizon/contracts/payments/PaymentsEscrow.sol b/packages/horizon/contracts/payments/PaymentsEscrow.sol index 7cf7e9e38..6f4252873 100644 --- a/packages/horizon/contracts/payments/PaymentsEscrow.sol +++ b/packages/horizon/contracts/payments/PaymentsEscrow.sol @@ -23,10 +23,6 @@ import { GraphDirectory } from "../utilities/GraphDirectory.sol"; contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, IPaymentsEscrow { using TokenUtils for IGraphToken; - /// @notice Authorization details for payer-collector pairs - mapping(address payer => mapping(address collector => IPaymentsEscrow.Collector collectorDetails)) - public authorizedCollectors; - /// @notice Escrow account details for payer-collector-receiver tuples mapping(address payer => mapping(address collector => mapping(address receiver => IPaymentsEscrow.EscrowAccount escrowAccount))) public escrowAccounts; @@ -35,9 +31,6 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, /// @dev This is a precautionary measure to avoid inadvertedly locking funds for too long uint256 public constant MAX_WAIT_PERIOD = 90 days; - /// @notice Thawing period in seconds for authorized collectors - uint256 public immutable REVOKE_COLLECTOR_THAWING_PERIOD; - /// @notice Thawing period in seconds for escrow funds withdrawal uint256 public immutable WITHDRAW_ESCROW_THAWING_PERIOD; @@ -49,24 +42,14 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, /** * @notice Construct the PaymentsEscrow contract * @param controller The address of the controller - * @param revokeCollectorThawingPeriod Thawing period in seconds for authorized collectors * @param withdrawEscrowThawingPeriod Thawing period in seconds for escrow funds withdrawal */ - constructor( - address controller, - uint256 revokeCollectorThawingPeriod, - uint256 withdrawEscrowThawingPeriod - ) GraphDirectory(controller) { - require( - revokeCollectorThawingPeriod <= MAX_WAIT_PERIOD, - PaymentsEscrowThawingPeriodTooLong(revokeCollectorThawingPeriod, MAX_WAIT_PERIOD) - ); + constructor(address controller, uint256 withdrawEscrowThawingPeriod) GraphDirectory(controller) { require( withdrawEscrowThawingPeriod <= MAX_WAIT_PERIOD, PaymentsEscrowThawingPeriodTooLong(withdrawEscrowThawingPeriod, MAX_WAIT_PERIOD) ); - REVOKE_COLLECTOR_THAWING_PERIOD = revokeCollectorThawingPeriod; WITHDRAW_ESCROW_THAWING_PERIOD = withdrawEscrowThawingPeriod; } @@ -77,52 +60,6 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, __Multicall_init(); } - /** - * @notice See {IPaymentsEscrow-approveCollector} - */ - function approveCollector(address collector_, uint256 allowance) external override notPaused { - require(allowance != 0, PaymentsEscrowInvalidZeroTokens()); - Collector storage collector = authorizedCollectors[msg.sender][collector_]; - collector.allowance += allowance; - emit AuthorizedCollector(msg.sender, collector_, allowance, collector.allowance); - } - - /** - * @notice See {IPaymentsEscrow-thawCollector} - */ - function thawCollector(address collector) external override notPaused { - authorizedCollectors[msg.sender][collector].thawEndTimestamp = - block.timestamp + - REVOKE_COLLECTOR_THAWING_PERIOD; - emit ThawCollector(msg.sender, collector); - } - - /** - * @notice See {IPaymentsEscrow-cancelThawCollector} - */ - function cancelThawCollector(address collector) external override notPaused { - require(authorizedCollectors[msg.sender][collector].thawEndTimestamp != 0, PaymentsEscrowNotThawing()); - - authorizedCollectors[msg.sender][collector].thawEndTimestamp = 0; - emit CancelThawCollector(msg.sender, collector); - } - - /** - * @notice See {IPaymentsEscrow-revokeCollector} - */ - function revokeCollector(address collector_) external override notPaused { - Collector storage collector = authorizedCollectors[msg.sender][collector_]; - - require(collector.thawEndTimestamp != 0, PaymentsEscrowNotThawing()); - require( - collector.thawEndTimestamp < block.timestamp, - PaymentsEscrowStillThawing(block.timestamp, collector.thawEndTimestamp) - ); - - delete authorizedCollectors[msg.sender][collector_]; - emit RevokeCollector(msg.sender, collector_); - } - /** * @notice See {IPaymentsEscrow-deposit} */ @@ -192,27 +129,19 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory, address receiver, uint256 tokens, address dataService, - uint256 tokensDataService + uint256 dataServiceCut ) external override notPaused { - // Check if collector is authorized and has enough funds - Collector storage collectorDetails = authorizedCollectors[payer][msg.sender]; - require( - collectorDetails.allowance >= tokens, - PaymentsEscrowInsufficientAllowance(collectorDetails.allowance, tokens) - ); - // Check if there are enough funds in the escrow account EscrowAccount storage account = escrowAccounts[payer][msg.sender][receiver]; require(account.balance >= tokens, PaymentsEscrowInsufficientBalance(account.balance, tokens)); - // Reduce amount from approved collector and account balance - collectorDetails.allowance -= tokens; + // Reduce amount from account balance account.balance -= tokens; uint256 balanceBefore = _graphToken().balanceOf(address(this)); _graphToken().approve(address(_graphPayments()), tokens); - _graphPayments().collect(paymentType, receiver, tokens, dataService, tokensDataService); + _graphPayments().collect(paymentType, receiver, tokens, dataService, dataServiceCut); uint256 balanceAfter = _graphToken().balanceOf(address(this)); require( @@ -228,6 +157,9 @@ 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; } diff --git a/packages/horizon/contracts/payments/collectors/TAPCollector.sol b/packages/horizon/contracts/payments/collectors/TAPCollector.sol index 57588a042..c8b42b87f 100644 --- a/packages/horizon/contracts/payments/collectors/TAPCollector.sol +++ b/packages/horizon/contracts/payments/collectors/TAPCollector.sol @@ -18,6 +18,8 @@ import { MessageHashUtils } from "@openzeppelin/contracts/utils/cryptography/Mes * @dev Note that the contract expects the RAV aggregate value to be monotonically increasing, each successive RAV for the same * (data service-payer-receiver) tuple should have a value greater than the previous one. The contract will keep track of the tokens * already collected and calculate the difference to collect. + * @dev The contract also implements a mechanism to authorize signers to sign RAVs on behalf of a payer. Signers cannot be reused + * for different payers. * @custom:security-contact Please email security+contracts@thegraph.com if you find any * bugs. We may have an active bug bounty program. */ @@ -27,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 dataService,address serviceProvider,uint64 timestampNs,uint128 valueAggregate,bytes metadata)" + "ReceiptAggregateVoucher(address payer,address dataService,address serviceProvider,uint64 timestampNs,uint128 valueAggregate,bytes metadata)" ); /// @notice Authorization details for payer-signer pairs @@ -79,6 +81,11 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector { PayerAuthorization storage authorization = authorizedSigners[signer]; require(authorization.payer == msg.sender, TAPCollectorSignerNotAuthorizedByPayer(msg.sender, signer)); + require(!authorization.revoked, TAPCollectorAuthorizationAlreadyRevoked(msg.sender, signer)); + require( + authorization.thawEndTimestamp == 0, + TAPCollectorSignerAlreadyThawing(signer, authorization.thawEndTimestamp) + ); authorization.thawEndTimestamp = block.timestamp + REVOKE_SIGNER_THAWING_PERIOD; emit SignerThawing(msg.sender, signer, authorization.thawEndTimestamp); @@ -110,7 +117,7 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector { TAPCollectorSignerStillThawing(block.timestamp, authorization.thawEndTimestamp) ); - delete authorizedSigners[signer]; + authorization.revoked = true; emit SignerRevoked(msg.sender, signer); } @@ -118,19 +125,19 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector { * @notice Initiate a payment collection through the payments protocol * See {IGraphPayments.collect}. * @dev Caller must be the data service the RAV was issued to. + * @dev Service provider must have an active provision with the data service to collect payments * @notice REVERT: This function may revert if ECDSA.recover fails, check ECDSA library for details. */ function collect(IGraphPayments.PaymentTypes paymentType, bytes memory data) external override returns (uint256) { - (SignedRAV memory signedRAV, uint256 dataServiceCut) = abi.decode(data, (SignedRAV, uint256)); - require( - signedRAV.rav.dataService == msg.sender, - TAPCollectorCallerNotDataService(msg.sender, signedRAV.rav.dataService) - ); - - address signer = _recoverRAVSigner(signedRAV); - require(authorizedSigners[signer].payer != address(0), TAPCollectorInvalidRAVSigner()); + return _collect(paymentType, data, 0); + } - return _collect(paymentType, authorizedSigners[signer].payer, signedRAV, dataServiceCut); + function collect( + IGraphPayments.PaymentTypes paymentType, + bytes memory data, + uint256 tokensToCollect + ) external override returns (uint256) { + return _collect(paymentType, data, tokensToCollect); } /** @@ -152,44 +159,75 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector { */ function _collect( IGraphPayments.PaymentTypes _paymentType, - address _payer, - SignedRAV memory _signedRAV, - uint256 _dataServiceCut + bytes memory _data, + uint256 _tokensToCollect ) private returns (uint256) { - address dataService = _signedRAV.rav.dataService; - address receiver = _signedRAV.rav.serviceProvider; + // Ensure caller is the RAV data service + (SignedRAV memory signedRAV, uint256 dataServiceCut) = abi.decode(_data, (SignedRAV, uint256)); + require( + signedRAV.rav.dataService == msg.sender, + TAPCollectorCallerNotDataService(msg.sender, signedRAV.rav.dataService) + ); - uint256 tokensRAV = _signedRAV.rav.valueAggregate; - uint256 tokensAlreadyCollected = tokensCollected[dataService][receiver][_payer]; + // Ensure RAV signer is authorized for a payer + address signer = _recoverRAVSigner(signedRAV); require( - tokensRAV > tokensAlreadyCollected, - TAPCollectorInconsistentRAVTokens(tokensRAV, tokensAlreadyCollected) + authorizedSigners[signer].payer != address(0) && !authorizedSigners[signer].revoked, + TAPCollectorInvalidRAVSigner() ); - uint256 tokensToCollect = tokensRAV - tokensAlreadyCollected; - uint256 tokensDataService = tokensToCollect.mulPPM(_dataServiceCut); + // Ensure RAV payer matches the authorized payer + address payer = authorizedSigners[signer].payer; + require(signedRAV.rav.payer == payer, TAPCollectorInvalidRAVPayer(payer, signedRAV.rav.payer)); - if (tokensToCollect > 0) { - tokensCollected[dataService][receiver][_payer] = tokensRAV; - _graphPaymentsEscrow().collect( - _paymentType, - _payer, - receiver, - tokensToCollect, - dataService, - tokensDataService + address dataService = signedRAV.rav.dataService; + address receiver = signedRAV.rav.serviceProvider; + + // Check the service provider has an active provision with the data service + // This prevents an attack where the payer can deny the service provider from collecting payments + // by using a signer as data service to syphon off the tokens in the escrow to an account they control + { + uint256 tokensAvailable = _graphStaking().getProviderTokensAvailable( + signedRAV.rav.serviceProvider, + signedRAV.rav.dataService ); + require(tokensAvailable > 0, TAPCollectorUnauthorizedDataService(signedRAV.rav.dataService)); + } + + uint256 tokensToCollect = 0; + { + uint256 tokensRAV = signedRAV.rav.valueAggregate; + uint256 tokensAlreadyCollected = tokensCollected[dataService][receiver][payer]; + require( + tokensRAV > tokensAlreadyCollected, + TAPCollectorInconsistentRAVTokens(tokensRAV, tokensAlreadyCollected) + ); + + if (_tokensToCollect == 0) { + tokensToCollect = tokensRAV - tokensAlreadyCollected; + } else { + require( + _tokensToCollect <= tokensRAV - tokensAlreadyCollected, + TAPCollectorInvalidTokensToCollectAmount(_tokensToCollect, tokensRAV - tokensAlreadyCollected) + ); + tokensToCollect = _tokensToCollect; + } + } + + if (tokensToCollect > 0) { + tokensCollected[dataService][receiver][payer] += tokensToCollect; + _graphPaymentsEscrow().collect(_paymentType, payer, receiver, tokensToCollect, dataService, dataServiceCut); } - emit PaymentCollected(_paymentType, _payer, receiver, tokensToCollect, dataService, tokensDataService); + emit PaymentCollected(_paymentType, payer, receiver, dataService, tokensToCollect); emit RAVCollected( - _payer, + payer, dataService, receiver, - _signedRAV.rav.timestampNs, - _signedRAV.rav.valueAggregate, - _signedRAV.rav.metadata, - _signedRAV.signature + signedRAV.rav.timestampNs, + signedRAV.rav.valueAggregate, + signedRAV.rav.metadata, + signedRAV.signature ); return tokensToCollect; } @@ -211,6 +249,7 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector { keccak256( abi.encode( EIP712_RAV_TYPEHASH, + _rav.payer, _rav.dataService, _rav.serviceProvider, _rav.timestampNs, diff --git a/packages/horizon/ignition/configs/horizon.hardhat.json b/packages/horizon/ignition/configs/horizon.hardhat.json index 894a2ed59..7c4214d37 100644 --- a/packages/horizon/ignition/configs/horizon.hardhat.json +++ b/packages/horizon/ignition/configs/horizon.hardhat.json @@ -36,7 +36,6 @@ "protocolPaymentCut": 10000 }, "PaymentsEscrow": { - "revokeCollectorThawingPeriod": 10000, "withdrawEscrowThawingPeriod": 10000 }, "TAPCollector": { diff --git a/packages/horizon/ignition/modules/core/PaymentsEscrow.ts b/packages/horizon/ignition/modules/core/PaymentsEscrow.ts index 1dbd07088..7c7948a7e 100644 --- a/packages/horizon/ignition/modules/core/PaymentsEscrow.ts +++ b/packages/horizon/ignition/modules/core/PaymentsEscrow.ts @@ -10,13 +10,12 @@ export default buildModule('PaymentsEscrow', (m) => { const { Controller, PeripheryRegistered } = m.useModule(GraphPeripheryModule) const { PaymentsEscrowProxyAdmin, PaymentsEscrowProxy, HorizonRegistered } = m.useModule(HorizonProxiesModule) - const revokeCollectorThawingPeriod = m.getParameter('revokeCollectorThawingPeriod') const withdrawEscrowThawingPeriod = m.getParameter('withdrawEscrowThawingPeriod') // Deploy PaymentsEscrow implementation const PaymentsEscrowImplementation = m.contract('PaymentsEscrow', PaymentsEscrowArtifact, - [Controller, revokeCollectorThawingPeriod, withdrawEscrowThawingPeriod], + [Controller, withdrawEscrowThawingPeriod], { after: [PeripheryRegistered, HorizonRegistered], }, diff --git a/packages/horizon/test/GraphBase.t.sol b/packages/horizon/test/GraphBase.t.sol index b58479fcf..b2eef0dd9 100644 --- a/packages/horizon/test/GraphBase.t.sol +++ b/packages/horizon/test/GraphBase.t.sol @@ -117,7 +117,7 @@ abstract contract GraphBaseTest is IHorizonStakingTypes, Utils, Constants { // PaymentsEscrow bytes memory escrowImplementationParameters = abi.encode( address(controller), - revokeCollectorThawingPeriod,withdrawEscrowThawingPeriod + withdrawEscrowThawingPeriod ); bytes memory escrowImplementationBytecode = abi.encodePacked( type(PaymentsEscrow).creationCode, diff --git a/packages/horizon/test/escrow/GraphEscrow.t.sol b/packages/horizon/test/escrow/GraphEscrow.t.sol index d3ffd21da..120713c6c 100644 --- a/packages/horizon/test/escrow/GraphEscrow.t.sol +++ b/packages/horizon/test/escrow/GraphEscrow.t.sol @@ -2,11 +2,15 @@ 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 { HorizonStakingSharedTest } from "../shared/horizon-staking/HorizonStakingShared.t.sol"; import { PaymentsEscrowSharedTest } from "../shared/payments-escrow/PaymentsEscrowShared.t.sol"; +import { PPMMath } from "../../contracts/libraries/PPMMath.sol"; contract GraphEscrowTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest { + using PPMMath for uint256; /* * MODIFIERS @@ -24,12 +28,6 @@ contract GraphEscrowTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest { _; } - modifier useCollector(uint256 tokens) { - vm.assume(tokens > 0); - escrow.approveCollector(users.verifier, tokens); - _; - } - modifier depositAndThawTokens(uint256 amount, uint256 thawAmount) { vm.assume(thawAmount > 0); vm.assume(amount > thawAmount); @@ -45,4 +43,101 @@ contract GraphEscrowTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest { function _approveEscrow(uint256 tokens) internal { token.approve(address(escrow), tokens); } -} \ No newline at end of file + + function _thawEscrow(address collector, address receiver, uint256 amount) internal { + (, address msgSender, ) = vm.readCallers(); + uint256 expectedThawEndTimestamp = block.timestamp + withdrawEscrowThawingPeriod; + vm.expectEmit(address(escrow)); + emit IPaymentsEscrow.Thaw(msgSender, collector, receiver, amount, expectedThawEndTimestamp); + escrow.thaw(collector, receiver, amount); + + (, uint256 amountThawing, uint256 thawEndTimestamp) = escrow.escrowAccounts(msgSender, collector, receiver); + assertEq(amountThawing, amount); + assertEq(thawEndTimestamp, expectedThawEndTimestamp); + } + + struct CollectPaymentData { + uint256 escrowBalance; + uint256 paymentsBalance; + uint256 receiverBalance; + uint256 delegationPoolBalance; + uint256 dataServiceBalance; + uint256 payerEscrowBalance; + } + + function _collectEscrow( + IGraphPayments.PaymentTypes _paymentType, + address _payer, + address _receiver, + uint256 _tokens, + address _dataService, + uint256 _dataServiceCut + ) internal { + (, address _collector, ) = vm.readCallers(); + + // Previous balances + CollectPaymentData memory previousBalances = CollectPaymentData({ + escrowBalance: token.balanceOf(address(escrow)), + paymentsBalance: token.balanceOf(address(payments)), + receiverBalance: token.balanceOf(_receiver), + delegationPoolBalance: staking.getDelegatedTokensAvailable(_receiver, _dataService), + dataServiceBalance: token.balanceOf(_dataService), + payerEscrowBalance: 0 + }); + + { + (uint256 payerEscrowBalance, , ) = escrow.escrowAccounts(_payer, _collector, _receiver); + previousBalances.payerEscrowBalance = payerEscrowBalance; + } + + vm.expectEmit(address(escrow)); + emit IPaymentsEscrow.EscrowCollected(_payer, _collector, _receiver, _tokens); + escrow.collect(_paymentType, _payer, _receiver, _tokens, _dataService, _dataServiceCut); + + // Calculate cuts + // this is nasty but stack is indeed too deep + 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 receiverExpectedPayment = _tokens - + _tokens.mulPPMRoundUp(payments.PROTOCOL_PAYMENT_CUT()) - + tokensDataService - + tokensDelegation; + + // After balances + CollectPaymentData memory afterBalances = CollectPaymentData({ + escrowBalance: token.balanceOf(address(escrow)), + paymentsBalance: token.balanceOf(address(payments)), + receiverBalance: token.balanceOf(_receiver), + delegationPoolBalance: staking.getDelegatedTokensAvailable(_receiver, _dataService), + dataServiceBalance: token.balanceOf(_dataService), + payerEscrowBalance: 0 + }); + { + (uint256 afterPayerEscrowBalance, , ) = escrow.escrowAccounts(_payer, _collector, _receiver); + afterBalances.payerEscrowBalance = afterPayerEscrowBalance; + } + + // Check receiver balance after payment + assertEq(afterBalances.receiverBalance - previousBalances.receiverBalance, receiverExpectedPayment); + assertEq(token.balanceOf(address(payments)), 0); + + // Check delegation pool balance after payment + assertEq(afterBalances.delegationPoolBalance - previousBalances.delegationPoolBalance, tokensDelegation); + + // Check that the escrow account has been updated + assertEq(previousBalances.escrowBalance, afterBalances.escrowBalance + _tokens); + + // Check that payments balance didn't change + assertEq(previousBalances.paymentsBalance, afterBalances.paymentsBalance); + + // Check data service balance after payment + assertEq(afterBalances.dataServiceBalance - previousBalances.dataServiceBalance, tokensDataService); + + // Check payers escrow balance after payment + assertEq(previousBalances.payerEscrowBalance - _tokens, afterBalances.payerEscrowBalance); + } +} diff --git a/packages/horizon/test/escrow/collect.t.sol b/packages/horizon/test/escrow/collect.t.sol index e47c04ec5..55f378b3a 100644 --- a/packages/horizon/test/escrow/collect.t.sol +++ b/packages/horizon/test/escrow/collect.t.sol @@ -5,89 +5,10 @@ import "forge-std/Test.sol"; import { IHorizonStakingMain } from "../../contracts/interfaces/internal/IHorizonStakingMain.sol"; import { IGraphPayments } from "../../contracts/interfaces/IGraphPayments.sol"; -import { IPaymentsEscrow } from "../../contracts/interfaces/IPaymentsEscrow.sol"; import { GraphEscrowTest } from "./GraphEscrow.t.sol"; contract GraphEscrowCollectTest is GraphEscrowTest { - - struct CollectPaymentData { - uint256 escrowBalance; - uint256 paymentsBalance; - uint256 receiverBalance; - uint256 delegationPoolBalance; - uint256 dataServiceBalance; - } - - function _collect( - IGraphPayments.PaymentTypes _paymentType, - address _payer, - address _receiver, - uint256 _tokens, - address _dataService, - uint256 _tokensDataService - ) private { - (, address _collector, ) = vm.readCallers(); - - // Previous balances - (uint256 previousPayerEscrowBalance,,) = escrow.escrowAccounts(_payer, _collector, _receiver); - CollectPaymentData memory previousBalances = CollectPaymentData({ - escrowBalance: token.balanceOf(address(escrow)), - paymentsBalance: token.balanceOf(address(payments)), - receiverBalance: token.balanceOf(_receiver), - delegationPoolBalance: staking.getDelegatedTokensAvailable( - _receiver, - _dataService - ), - dataServiceBalance: token.balanceOf(_dataService) - }); - - vm.expectEmit(address(escrow)); - emit IPaymentsEscrow.EscrowCollected(_payer, _collector, _receiver, _tokens); - escrow.collect(_paymentType, _payer, _receiver, _tokens, _dataService, _tokensDataService); - - // Calculate cuts - uint256 protocolPaymentCut = payments.PROTOCOL_PAYMENT_CUT(); - uint256 delegatorCut = staking.getDelegationFeeCut( - _receiver, - _dataService, - _paymentType - ); - - // After balances - (uint256 afterPayerEscrowBalance,,) = escrow.escrowAccounts(_payer, _collector, _receiver); - CollectPaymentData memory afterBalances = CollectPaymentData({ - escrowBalance: token.balanceOf(address(escrow)), - paymentsBalance: token.balanceOf(address(payments)), - receiverBalance: token.balanceOf(_receiver), - delegationPoolBalance: staking.getDelegatedTokensAvailable( - _receiver, - _dataService - ), - dataServiceBalance: token.balanceOf(_dataService) - }); - - // Check receiver balance after payment - uint256 receiverExpectedPayment = _tokens - _tokensDataService - _tokens * protocolPaymentCut / MAX_PPM - _tokens * delegatorCut / MAX_PPM; - assertEq(afterBalances.receiverBalance - previousBalances.receiverBalance, receiverExpectedPayment); - assertEq(token.balanceOf(address(payments)), 0); - - // Check delegation pool balance after payment - assertEq(afterBalances.delegationPoolBalance - previousBalances.delegationPoolBalance, _tokens * delegatorCut / MAX_PPM); - - // Check that the escrow account has been updated - assertEq(previousBalances.escrowBalance, afterBalances.escrowBalance + _tokens); - - // Check that payments balance didn't change - assertEq(previousBalances.paymentsBalance, afterBalances.paymentsBalance); - - // Check data service balance after payment - assertEq(afterBalances.dataServiceBalance - previousBalances.dataServiceBalance, _tokensDataService); - - // Check payers escrow balance after payment - assertEq(previousPayerEscrowBalance - _tokens, afterPayerEscrowBalance); - } - /* * TESTS */ @@ -95,83 +16,87 @@ contract GraphEscrowCollectTest is GraphEscrowTest { function testCollect_Tokens( uint256 tokens, uint256 delegationTokens, - uint256 tokensDataService - ) public useIndexer useProvision(tokens, 0, 0) useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) { - uint256 tokensProtocol = tokens * protocolPaymentCut / MAX_PPM; - uint256 tokensDelegatoion = tokens * delegationFeeCut / MAX_PPM; - vm.assume(tokensDataService < tokens - tokensProtocol - tokensDelegatoion); - + uint256 dataServiceCut + ) + public + useIndexer + useProvision(tokens, 0, 0) + useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) + { + dataServiceCut = bound(dataServiceCut, 0, MAX_PPM); delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); + resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); resetPrank(users.gateway); - escrow.approveCollector(users.verifier, tokens); _depositTokens(users.verifier, users.indexer, tokens); resetPrank(users.verifier); - _collect(IGraphPayments.PaymentTypes.QueryFee, users.gateway, users.indexer, tokens, subgraphDataServiceAddress, tokensDataService); - } - - function testCollect_RevertWhen_CollectorNotAuthorized(uint256 amount) public { - vm.assume(amount > 0); - vm.startPrank(users.verifier); - uint256 dataServiceCut = 30000; // 3% - bytes memory expectedError = abi.encodeWithSelector( - IPaymentsEscrow.PaymentsEscrowInsufficientAllowance.selector, - 0, - amount + _collectEscrow( + IGraphPayments.PaymentTypes.QueryFee, + users.gateway, + users.indexer, + tokens, + subgraphDataServiceAddress, + dataServiceCut ); - vm.expectRevert(expectedError); - escrow.collect(IGraphPayments.PaymentTypes.QueryFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, dataServiceCut); - vm.stopPrank(); } - function testCollect_RevertWhen_CollectorHasInsufficientAmount( + function testCollect_RevertWhen_SenderHasInsufficientAmountInEscrow( uint256 amount, uint256 insufficientAmount - ) public useGateway useCollector(insufficientAmount) useDeposit(amount) { + ) public useGateway useDeposit(insufficientAmount) { + vm.assume(amount > 0); vm.assume(insufficientAmount < amount); vm.startPrank(users.verifier); bytes memory expectedError = abi.encodeWithSignature( - "PaymentsEscrowInsufficientAllowance(uint256,uint256)", - insufficientAmount, + "PaymentsEscrowInsufficientBalance(uint256,uint256)", + insufficientAmount, amount ); vm.expectRevert(expectedError); - escrow.collect(IGraphPayments.PaymentTypes.QueryFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, 0); - } - - function testCollect_RevertWhen_SenderHasInsufficientAmountInEscrow( - uint256 amount, - uint256 insufficientAmount - ) public useGateway useCollector(amount) useDeposit(insufficientAmount) { - vm.assume(insufficientAmount < amount); - - vm.startPrank(users.verifier); - bytes memory expectedError = abi.encodeWithSignature("PaymentsEscrowInsufficientBalance(uint256,uint256)", insufficientAmount, amount); - vm.expectRevert(expectedError); - escrow.collect(IGraphPayments.PaymentTypes.QueryFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, 0); + escrow.collect( + IGraphPayments.PaymentTypes.QueryFee, + users.gateway, + users.indexer, + amount, + subgraphDataServiceAddress, + 0 + ); vm.stopPrank(); } function testCollect_RevertWhen_InvalidPool( uint256 amount - ) public useIndexer useProvision(amount, 0, 0) useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) { + ) + public + useIndexer + useProvision(amount, 0, 0) + useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) + { vm.assume(amount > 1 ether); resetPrank(users.gateway); - escrow.approveCollector(users.verifier, amount); _depositTokens(users.verifier, users.indexer, amount); resetPrank(users.verifier); - vm.expectRevert(abi.encodeWithSelector( - IHorizonStakingMain.HorizonStakingInvalidDelegationPool.selector, + vm.expectRevert( + abi.encodeWithSelector( + IHorizonStakingMain.HorizonStakingInvalidDelegationPool.selector, + users.indexer, + subgraphDataServiceAddress + ) + ); + escrow.collect( + IGraphPayments.PaymentTypes.QueryFee, + users.gateway, users.indexer, - subgraphDataServiceAddress - )); - escrow.collect(IGraphPayments.PaymentTypes.QueryFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, 1); + amount, + subgraphDataServiceAddress, + 1 + ); } function testCollect_RevertWhen_InvalidProvision( @@ -179,17 +104,25 @@ contract GraphEscrowCollectTest is GraphEscrowTest { ) public useIndexer useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) { vm.assume(amount > 1 ether); vm.assume(amount <= MAX_STAKING_TOKENS); - + resetPrank(users.gateway); - escrow.approveCollector(users.verifier, amount); _depositTokens(users.verifier, users.indexer, amount); resetPrank(users.verifier); - vm.expectRevert(abi.encodeWithSelector( - IHorizonStakingMain.HorizonStakingInvalidProvision.selector, + vm.expectRevert( + abi.encodeWithSelector( + IHorizonStakingMain.HorizonStakingInvalidProvision.selector, + users.indexer, + subgraphDataServiceAddress + ) + ); + escrow.collect( + IGraphPayments.PaymentTypes.QueryFee, + users.gateway, users.indexer, - subgraphDataServiceAddress - )); - escrow.collect(IGraphPayments.PaymentTypes.QueryFee, users.gateway, users.indexer, amount, subgraphDataServiceAddress, 1); + amount, + subgraphDataServiceAddress, + 1 + ); } -} \ No newline at end of file +} diff --git a/packages/horizon/test/escrow/collector.t.sol b/packages/horizon/test/escrow/collector.t.sol deleted file mode 100644 index d6cb3bc0f..000000000 --- a/packages/horizon/test/escrow/collector.t.sol +++ /dev/null @@ -1,108 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.27; - -import "forge-std/Test.sol"; - -import { IPaymentsEscrow } from "../../contracts/interfaces/IPaymentsEscrow.sol"; - -import { GraphEscrowTest } from "./GraphEscrow.t.sol"; - -contract GraphEscrowCollectorTest is GraphEscrowTest { - - /* - * HELPERS - */ - - function _thawCollector() internal { - (uint256 beforeAllowance,) = escrow.authorizedCollectors(users.gateway, users.verifier); - vm.expectEmit(address(escrow)); - emit IPaymentsEscrow.ThawCollector(users.gateway, users.verifier); - escrow.thawCollector(users.verifier); - - (uint256 allowance, uint256 thawEndTimestamp) = escrow.authorizedCollectors(users.gateway, users.verifier); - assertEq(allowance, beforeAllowance); - assertEq(thawEndTimestamp, block.timestamp + revokeCollectorThawingPeriod); - } - - function _cancelThawCollector() internal { - (uint256 beforeAllowance, uint256 beforeThawEndTimestamp) = escrow.authorizedCollectors(users.gateway, users.verifier); - assertTrue(beforeThawEndTimestamp != 0, "Collector should be thawing"); - vm.expectEmit(address(escrow)); - emit IPaymentsEscrow.CancelThawCollector(users.gateway, users.verifier); - escrow.cancelThawCollector(users.verifier); - - (uint256 allowance, uint256 thawEndTimestamp) = escrow.authorizedCollectors(users.gateway, users.verifier); - assertEq(allowance, beforeAllowance); - assertEq(thawEndTimestamp, 0); - } - - function _revokeCollector() internal { - vm.expectEmit(address(escrow)); - emit IPaymentsEscrow.RevokeCollector(users.gateway, users.verifier); - escrow.revokeCollector(users.verifier); - - (uint256 allowance, uint256 thawEndTimestamp) = escrow.authorizedCollectors(users.gateway, users.verifier); - assertEq(allowance, 0); - assertEq(thawEndTimestamp, 0); - } - - /* - * TESTS - */ - - function testCollector_Approve( - uint256 tokens, - uint256 approveSteps - ) public useGateway { - approveSteps = bound(approveSteps, 1, 100); - vm.assume(tokens > approveSteps); - - uint256 approveTokens = tokens / approveSteps; - for (uint i = 0; i < approveSteps; i++) { - _approveCollector(users.verifier, approveTokens); - } - } - - function testCollector_RevertWhen_ApprovingForZeroAllowance( - uint256 amount - ) public useGateway useCollector(amount) { - bytes memory expectedError = abi.encodeWithSelector(IPaymentsEscrow.PaymentsEscrowInvalidZeroTokens.selector); - vm.expectRevert(expectedError); - escrow.approveCollector(users.verifier, 0); - } - - function testCollector_Thaw(uint256 amount) public useGateway useCollector(amount) { - _thawCollector(); - } - - function testCollector_CancelThaw(uint256 amount) public useGateway useCollector(amount) { - _thawCollector(); - _cancelThawCollector(); - } - - function testCollector_RevertWhen_CancelThawIsNotThawing(uint256 amount) public useGateway useCollector(amount) { - bytes memory expectedError = abi.encodeWithSignature("PaymentsEscrowNotThawing()"); - vm.expectRevert(expectedError); - escrow.cancelThawCollector(users.verifier); - vm.stopPrank(); - } - - function testCollector_Revoke(uint256 amount) public useGateway useCollector(amount) { - _thawCollector(); - skip(revokeCollectorThawingPeriod + 1); - _revokeCollector(); - } - - function testCollector_RevertWhen_RevokeIsNotThawing(uint256 amount) public useGateway useCollector(amount) { - bytes memory expectedError = abi.encodeWithSignature("PaymentsEscrowNotThawing()"); - vm.expectRevert(expectedError); - escrow.revokeCollector(users.verifier); - } - - function testCollector_RevertWhen_RevokeIsStillThawing(uint256 amount) public useGateway useCollector(amount) { - escrow.thawCollector(users.verifier); - bytes memory expectedError = abi.encodeWithSignature("PaymentsEscrowStillThawing(uint256,uint256)", block.timestamp, block.timestamp + revokeCollectorThawingPeriod); - vm.expectRevert(expectedError); - escrow.revokeCollector(users.verifier); - } -} \ No newline at end of file diff --git a/packages/horizon/test/escrow/getters.t.sol b/packages/horizon/test/escrow/getters.t.sol new file mode 100644 index 000000000..6434e1b30 --- /dev/null +++ b/packages/horizon/test/escrow/getters.t.sol @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import "forge-std/Test.sol"; +import { IGraphPayments } from "../../contracts/interfaces/IGraphPayments.sol"; + +import { GraphEscrowTest } from "./GraphEscrow.t.sol"; + +contract GraphEscrowGettersTest is GraphEscrowTest { + /* + * TESTS + */ + + function testGetBalance(uint256 amount) public useGateway useDeposit(amount) { + uint256 balance = escrow.getBalance(users.gateway, users.verifier, users.indexer); + assertEq(balance, amount); + } + + function testGetBalance_WhenThawing( + uint256 amountDeposit, + uint256 amountThawing + ) public useGateway useDeposit(amountDeposit) { + vm.assume(amountThawing > 0); + vm.assume(amountDeposit >= amountThawing); + + // thaw some funds + _thawEscrow(users.verifier, users.indexer, amountThawing); + + uint256 balance = escrow.getBalance(users.gateway, users.verifier, users.indexer); + assertEq(balance, amountDeposit - amountThawing); + } + + function testGetBalance_WhenCollectedOverThawing( + uint256 amountDeposit, + uint256 amountThawing, + uint256 amountCollected + ) public useGateway useDeposit(amountDeposit) { + vm.assume(amountThawing > 0); + vm.assume(amountDeposit > 0); + vm.assume(amountDeposit >= amountThawing); + vm.assume(amountDeposit >= amountCollected); + vm.assume(amountDeposit - amountCollected < amountThawing); + + // thaw some funds + _thawEscrow(users.verifier, users.indexer, amountThawing); + + // users start with max uint256 balance so we burn to avoid overflow + // TODO: we should modify all tests to consider users have a max balance thats less than max uint256 + resetPrank(users.indexer); + token.burn(amountCollected); + + // collect some funds to get the balance of the account below the amount thawing + resetPrank(users.verifier); + _collectEscrow( + IGraphPayments.PaymentTypes.QueryFee, + users.gateway, + users.indexer, + amountCollected, + subgraphDataServiceAddress, + 0 + ); + + // balance should always be 0 since thawing funds > available funds + uint256 balance = escrow.getBalance(users.gateway, users.verifier, users.indexer); + assertEq(balance, 0); + } +} diff --git a/packages/horizon/test/escrow/paused.t.sol b/packages/horizon/test/escrow/paused.t.sol index a993da883..6019b5c15 100644 --- a/packages/horizon/test/escrow/paused.t.sol +++ b/packages/horizon/test/escrow/paused.t.sol @@ -63,26 +63,4 @@ contract GraphEscrowPausedTest is GraphEscrowTest { vm.expectRevert(abi.encodeWithSelector(IPaymentsEscrow.PaymentsEscrowIsPaused.selector)); escrow.collect(IGraphPayments.PaymentTypes.QueryFee, users.gateway, users.indexer, tokens, subgraphDataServiceAddress, tokensDataService); } - - // Collectors - - function testPaused_RevertWhen_ApproveCollector(uint256 tokens) public useGateway usePaused(true) { - vm.expectRevert(abi.encodeWithSelector(IPaymentsEscrow.PaymentsEscrowIsPaused.selector)); - escrow.approveCollector(users.verifier, tokens); - } - - function testPaused_RevertWhen_ThawCollector(uint256 tokens) public useGateway useCollector(tokens) usePaused(true) { - vm.expectRevert(abi.encodeWithSelector(IPaymentsEscrow.PaymentsEscrowIsPaused.selector)); - escrow.thawCollector(users.verifier); - } - - function testPaused_RevertWhen_CancelThawCollector(uint256 tokens) public useGateway useCollector(tokens) usePaused(true) { - vm.expectRevert(abi.encodeWithSelector(IPaymentsEscrow.PaymentsEscrowIsPaused.selector)); - escrow.cancelThawCollector(users.verifier); - } - - function testPaused_RevertWhen_RevokeCollector(uint256 tokens) public useGateway useCollector(tokens) usePaused(true) { - vm.expectRevert(abi.encodeWithSelector(IPaymentsEscrow.PaymentsEscrowIsPaused.selector)); - escrow.revokeCollector(users.verifier); - } } \ No newline at end of file diff --git a/packages/horizon/test/escrow/thaw.t.sol b/packages/horizon/test/escrow/thaw.t.sol index 8e2674e00..017c3291f 100644 --- a/packages/horizon/test/escrow/thaw.t.sol +++ b/packages/horizon/test/escrow/thaw.t.sol @@ -3,8 +3,6 @@ pragma solidity 0.8.27; import "forge-std/Test.sol"; -import { IPaymentsEscrow } from "../../contracts/interfaces/IPaymentsEscrow.sol"; - import { GraphEscrowTest } from "./GraphEscrow.t.sol"; contract GraphEscrowThawTest is GraphEscrowTest { @@ -14,14 +12,7 @@ contract GraphEscrowThawTest is GraphEscrowTest { */ function testThaw_Tokens(uint256 amount) public useGateway useDeposit(amount) { - uint256 expectedThawEndTimestamp = block.timestamp + withdrawEscrowThawingPeriod; - vm.expectEmit(address(escrow)); - emit IPaymentsEscrow.Thaw(users.gateway, users.verifier, users.indexer, amount, expectedThawEndTimestamp); - escrow.thaw(users.verifier, users.indexer, amount); - - (, uint256 amountThawing,uint256 thawEndTimestamp) = escrow.escrowAccounts(users.gateway, users.verifier, users.indexer); - assertEq(amountThawing, amount); - assertEq(thawEndTimestamp, expectedThawEndTimestamp); + _thawEscrow(users.verifier, users.indexer, amount); } function testThaw_RevertWhen_InsufficientThawAmount( diff --git a/packages/horizon/test/payments/GraphPayments.t.sol b/packages/horizon/test/payments/GraphPayments.t.sol index 47740b4cd..494a7912a 100644 --- a/packages/horizon/test/payments/GraphPayments.t.sol +++ b/packages/horizon/test/payments/GraphPayments.t.sol @@ -8,8 +8,10 @@ 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"; contract GraphPaymentsTest is HorizonStakingSharedTest { + using PPMMath for uint256; struct CollectPaymentData { uint256 escrowBalance; @@ -24,54 +26,52 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { address _receiver, uint256 _tokens, address _dataService, - uint256 _tokensDataService + uint256 _dataServiceCut ) private { // Previous balances CollectPaymentData memory previousBalances = CollectPaymentData({ escrowBalance: token.balanceOf(address(escrow)), paymentsBalance: token.balanceOf(address(payments)), receiverBalance: token.balanceOf(_receiver), - delegationPoolBalance: staking.getDelegatedTokensAvailable( - _receiver, - _dataService - ), + delegationPoolBalance: staking.getDelegatedTokensAvailable(_receiver, _dataService), dataServiceBalance: token.balanceOf(_dataService) }); // Calculate cuts - uint256 protocolPaymentCut = payments.PROTOCOL_PAYMENT_CUT(); - uint256 delegatorCut = staking.getDelegationFeeCut( - _receiver, - _dataService, - _paymentType + 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 tokensProtocol = _tokens * protocolPaymentCut / MAX_PPM; - uint256 tokensDelegation = _tokens * delegatorCut / MAX_PPM; - uint256 receiverExpectedPayment = _tokens - _tokensDataService - tokensProtocol - tokensDelegation; + uint256 receiverExpectedPayment = _tokens - tokensProtocol - tokensDataService - tokensDelegation; - (,address msgSender, ) = vm.readCallers(); + (, address msgSender, ) = vm.readCallers(); vm.expectEmit(address(payments)); - emit IGraphPayments.PaymentCollected( + emit IGraphPayments.GraphPaymentCollected( msgSender, _receiver, _dataService, - receiverExpectedPayment, + _tokens, + tokensProtocol, + tokensDataService, tokensDelegation, - _tokensDataService, - tokensProtocol + receiverExpectedPayment + ); + payments.collect( + _paymentType, + _receiver, + _tokens, + _dataService, + _dataServiceCut ); - payments.collect(_paymentType, _receiver, _tokens, _dataService, _tokensDataService); // After balances CollectPaymentData memory afterBalances = CollectPaymentData({ escrowBalance: token.balanceOf(address(escrow)), paymentsBalance: token.balanceOf(address(payments)), receiverBalance: token.balanceOf(_receiver), - delegationPoolBalance: staking.getDelegatedTokensAvailable( - _receiver, - _dataService - ), + delegationPoolBalance: staking.getDelegatedTokensAvailable(_receiver, _dataService), dataServiceBalance: token.balanceOf(_dataService) }); @@ -89,7 +89,7 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { assertEq(previousBalances.paymentsBalance, afterBalances.paymentsBalance); // Check data service balance after payment - assertEq(afterBalances.dataServiceBalance - previousBalances.dataServiceBalance, _tokensDataService); + assertEq(afterBalances.dataServiceBalance - previousBalances.dataServiceBalance, tokensDataService); } /* @@ -97,11 +97,11 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { */ function testConstructor_RevertIf_InvalidProtocolPaymentCut(uint256 protocolPaymentCut) public { - protocolPaymentCut = bound(protocolPaymentCut, MAX_PPM + 1, MAX_PPM + 100); + protocolPaymentCut = bound(protocolPaymentCut, MAX_PPM + 1, type(uint256).max); resetPrank(users.deployer); bytes memory expectedError = abi.encodeWithSelector( - IGraphPayments.GraphPaymentsInvalidProtocolPaymentCut.selector, + IGraphPayments.GraphPaymentsInvalidCut.selector, protocolPaymentCut ); vm.expectRevert(expectedError); @@ -110,12 +110,15 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { function testCollect( uint256 amount, - uint256 tokensDataService, + uint256 dataServiceCut, uint256 tokensDelegate - ) public useIndexer useProvision(amount, 0, 0) useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) { - uint256 tokensProtocol = amount * protocolPaymentCut / MAX_PPM; - uint256 tokensDelegation = amount * delegationFeeCut / MAX_PPM; - vm.assume(tokensDataService < amount - tokensProtocol - tokensDelegation); + ) + public + useIndexer + useProvision(amount, 0, 0) + useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) + { + dataServiceCut = bound(dataServiceCut, 0, MAX_PPM); address escrowAddress = address(escrow); // Delegate tokens @@ -129,35 +132,50 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { approve(address(payments), amount); // Collect payments through GraphPayments - _collect(IGraphPayments.PaymentTypes.QueryFee, users.indexer, amount, subgraphDataServiceAddress, tokensDataService); + _collect( + IGraphPayments.PaymentTypes.QueryFee, + users.indexer, + amount, + subgraphDataServiceAddress, + dataServiceCut + ); vm.stopPrank(); } - function testCollect_RevertWhen_InsufficientAmount( + function testCollect_RevertWhen_InvalidDataServiceCut( uint256 amount, - uint256 tokensDataService - ) public useIndexer useProvision(amount, 0, 0) useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) { - tokensDataService = bound(tokensDataService, amount + 1, MAX_STAKING_TOKENS + 1); - - address escrowAddress = address(escrow); - mint(escrowAddress, amount); - vm.startPrank(escrowAddress); - approve(address(payments), amount); + uint256 dataServiceCut + ) + public + useIndexer + useProvision(amount, 0, 0) + useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) + { + dataServiceCut = bound(dataServiceCut, MAX_PPM + 1, type(uint256).max); - bytes memory expectedError; - { - uint256 tokensProtocol = amount * protocolPaymentCut / MAX_PPM; - uint256 tokensDelegatoion = amount * delegationFeeCut / MAX_PPM; - uint256 requiredAmount = tokensDataService + tokensProtocol + tokensDelegatoion; - expectedError = abi.encodeWithSignature("GraphPaymentsInsufficientTokens(uint256,uint256)", amount, requiredAmount); - } + resetPrank(users.deployer); + bytes memory expectedError = abi.encodeWithSelector( + IGraphPayments.GraphPaymentsInvalidCut.selector, + dataServiceCut + ); vm.expectRevert(expectedError); - payments.collect(IGraphPayments.PaymentTypes.QueryFee, users.indexer, amount, subgraphDataServiceAddress, tokensDataService); + payments.collect( + IGraphPayments.PaymentTypes.QueryFee, + users.indexer, + amount, + subgraphDataServiceAddress, + dataServiceCut + ); } function testCollect_RevertWhen_InvalidPool( uint256 amount - ) public useIndexer useProvision(amount, 0, 0) useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) { + ) + public + useIndexer + useProvision(amount, 0, 0) + useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut) + { vm.assume(amount > 1 ether); address escrowAddress = address(escrow); @@ -167,11 +185,13 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { approve(address(payments), amount); // Collect payments through GraphPayments - vm.expectRevert(abi.encodeWithSelector( - IHorizonStakingMain.HorizonStakingInvalidDelegationPool.selector, - users.indexer, - subgraphDataServiceAddress - )); + vm.expectRevert( + abi.encodeWithSelector( + IHorizonStakingMain.HorizonStakingInvalidDelegationPool.selector, + users.indexer, + subgraphDataServiceAddress + ) + ); payments.collect(IGraphPayments.PaymentTypes.QueryFee, users.indexer, amount, subgraphDataServiceAddress, 1); } @@ -181,18 +201,20 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { vm.assume(amount > 1 ether); vm.assume(amount <= MAX_STAKING_TOKENS); address escrowAddress = address(escrow); - + // Add tokens in 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 - )); + vm.expectRevert( + abi.encodeWithSelector( + IHorizonStakingMain.HorizonStakingInvalidProvision.selector, + users.indexer, + subgraphDataServiceAddress + ) + ); payments.collect(IGraphPayments.PaymentTypes.QueryFee, users.indexer, amount, subgraphDataServiceAddress, 1); } } diff --git a/packages/horizon/test/payments/tap-collector/TAPCollector.t.sol b/packages/horizon/test/payments/tap-collector/TAPCollector.t.sol index 25b4c901c..362cff8af 100644 --- a/packages/horizon/test/payments/tap-collector/TAPCollector.t.sol +++ b/packages/horizon/test/payments/tap-collector/TAPCollector.t.sol @@ -60,15 +60,16 @@ contract TAPCollectorTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest function _authorizeSigner(address _signer, uint256 _proofDeadline, bytes memory _proof) internal { (, address msgSender, ) = vm.readCallers(); - + vm.expectEmit(address(tapCollector)); emit ITAPCollector.SignerAuthorized(msgSender, _signer); - + tapCollector.authorizeSigner(_signer, _proofDeadline, _proof); - - (address _payer, uint256 thawEndTimestamp) = tapCollector.authorizedSigners(_signer); + + (address _payer, uint256 thawEndTimestamp, bool revoked) = tapCollector.authorizedSigners(_signer); assertEq(_payer, msgSender); assertEq(thawEndTimestamp, 0); + assertEq(revoked, false); } function _thawSigner(address _signer) internal { @@ -80,9 +81,10 @@ contract TAPCollectorTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest tapCollector.thawSigner(_signer); - (address _payer, uint256 thawEndTimestamp) = tapCollector.authorizedSigners(_signer); + (address _payer, uint256 thawEndTimestamp, bool revoked) = tapCollector.authorizedSigners(_signer); assertEq(_payer, msgSender); assertEq(thawEndTimestamp, expectedThawEndTimestamp); + assertEq(revoked, false); } function _cancelThawSigner(address _signer) internal { @@ -93,42 +95,69 @@ contract TAPCollectorTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest tapCollector.cancelThawSigner(_signer); - (address _payer, uint256 thawEndTimestamp) = tapCollector.authorizedSigners(_signer); + (address _payer, uint256 thawEndTimestamp, bool revoked) = tapCollector.authorizedSigners(_signer); assertEq(_payer, msgSender); assertEq(thawEndTimestamp, 0); + assertEq(revoked, false); } function _revokeAuthorizedSigner(address _signer) internal { (, address msgSender, ) = vm.readCallers(); + (address beforePayer, uint256 beforeThawEndTimestamp, ) = tapCollector.authorizedSigners(_signer); + vm.expectEmit(address(tapCollector)); emit ITAPCollector.SignerRevoked(msgSender, _signer); tapCollector.revokeAuthorizedSigner(_signer); - (address _payer, uint256 thawEndTimestamp) = tapCollector.authorizedSigners(_signer); - assertEq(_payer, address(0)); - assertEq(thawEndTimestamp, 0); + (address afterPayer, uint256 afterThawEndTimestamp, bool afterRevoked) = tapCollector.authorizedSigners( + _signer + ); + + assertEq(beforePayer, afterPayer); + assertEq(beforeThawEndTimestamp, afterThawEndTimestamp); + assertEq(afterRevoked, true); } function _collect(IGraphPayments.PaymentTypes _paymentType, bytes memory _data) internal { - (ITAPCollector.SignedRAV memory signedRAV, uint256 dataServiceCut) = abi.decode(_data, (ITAPCollector.SignedRAV, uint256)); + __collect(_paymentType, _data, 0); + } + + function _collect(IGraphPayments.PaymentTypes _paymentType, bytes memory _data, uint256 _tokensToCollect) internal { + __collect(_paymentType, _data, _tokensToCollect); + } + + function __collect( + IGraphPayments.PaymentTypes _paymentType, + bytes memory _data, + uint256 _tokensToCollect + ) internal { + (ITAPCollector.SignedRAV memory signedRAV, ) = abi.decode( + _data, + (ITAPCollector.SignedRAV, uint256) + ); bytes32 messageHash = tapCollector.encodeRAV(signedRAV.rav); address _signer = ECDSA.recover(messageHash, signedRAV.signature); - (address _payer, ) = tapCollector.authorizedSigners(_signer); - uint256 tokensAlreadyCollected = tapCollector.tokensCollected(signedRAV.rav.dataService, signedRAV.rav.serviceProvider, _payer); - uint256 tokensToCollect = signedRAV.rav.valueAggregate - tokensAlreadyCollected; - uint256 tokensDataService = tokensToCollect.mulPPM(dataServiceCut); - + (address _payer, , ) = tapCollector.authorizedSigners(_signer); + uint256 tokensAlreadyCollected = tapCollector.tokensCollected( + signedRAV.rav.dataService, + signedRAV.rav.serviceProvider, + _payer + ); + uint256 tokensToCollect = _tokensToCollect == 0 + ? signedRAV.rav.valueAggregate - tokensAlreadyCollected + : _tokensToCollect; + vm.expectEmit(address(tapCollector)); emit IPaymentsCollector.PaymentCollected( - _paymentType, + _paymentType, _payer, signedRAV.rav.serviceProvider, - tokensToCollect, signedRAV.rav.dataService, - tokensDataService + tokensToCollect ); + vm.expectEmit(address(tapCollector)); emit ITAPCollector.RAVCollected( _payer, signedRAV.rav.dataService, @@ -138,11 +167,19 @@ contract TAPCollectorTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest signedRAV.rav.metadata, signedRAV.signature ); - - uint256 tokensCollected = tapCollector.collect(_paymentType, _data); - assertEq(tokensCollected, tokensToCollect); + uint256 tokensCollected = _tokensToCollect == 0 + ? tapCollector.collect(_paymentType, _data) + : tapCollector.collect(_paymentType, _data, _tokensToCollect); - uint256 tokensCollectedAfter = tapCollector.tokensCollected(signedRAV.rav.dataService, signedRAV.rav.serviceProvider, _payer); - assertEq(tokensCollectedAfter, signedRAV.rav.valueAggregate); + uint256 tokensCollectedAfter = tapCollector.tokensCollected( + signedRAV.rav.dataService, + signedRAV.rav.serviceProvider, + _payer + ); + assertEq(tokensCollected, tokensToCollect); + assertEq( + tokensCollectedAfter, + _tokensToCollect == 0 ? signedRAV.rav.valueAggregate : tokensAlreadyCollected + _tokensToCollect + ); } } 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 06b0e027a..a4e1eafa7 100644 --- a/packages/horizon/test/payments/tap-collector/collect/collect.t.sol +++ b/packages/horizon/test/payments/tap-collector/collect/collect.t.sol @@ -9,18 +9,18 @@ import { IGraphPayments } from "../../../../contracts/interfaces/IGraphPayments. import { TAPCollectorTest } from "../TAPCollector.t.sol"; contract TAPCollectorCollectTest is TAPCollectorTest { - /* - * HELPERS - */ + * HELPERS + */ function _getQueryFeeEncodedData( uint256 _signerPrivateKey, + address _payer, address _indexer, address _collector, uint128 _tokens ) private view returns (bytes memory) { - ITAPCollector.ReceiptAggregateVoucher memory rav = _getRAV(_indexer, _collector, _tokens); + ITAPCollector.ReceiptAggregateVoucher memory rav = _getRAV(_payer, _indexer, _collector, _tokens); bytes32 messageHash = tapCollector.encodeRAV(rav); (uint8 v, bytes32 r, bytes32 s) = vm.sign(_signerPrivateKey, messageHash); bytes memory signature = abi.encodePacked(r, s, v); @@ -29,12 +29,14 @@ contract TAPCollectorCollectTest is TAPCollectorTest { } function _getRAV( + address _payer, address _indexer, address _collector, uint128 _tokens ) private pure returns (ITAPCollector.ReceiptAggregateVoucher memory rav) { return ITAPCollector.ReceiptAggregateVoucher({ + payer: _payer, dataService: _collector, serviceProvider: _indexer, timestampNs: 0, @@ -47,43 +49,149 @@ contract TAPCollectorCollectTest is TAPCollectorTest { * TESTS */ - function testTAPCollector_Collect(uint256 tokens) public useGateway useSigner { + function testTAPCollector_Collect( + uint256 tokens + ) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner { tokens = bound(tokens, 1, type(uint128).max); - - _approveCollector(address(tapCollector), tokens); + _depositTokens(address(tapCollector), users.indexer, tokens); - - bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens)); + + bytes memory data = _getQueryFeeEncodedData( + signerPrivateKey, + users.gateway, + users.indexer, + users.verifier, + uint128(tokens) + ); resetPrank(users.verifier); _collect(IGraphPayments.PaymentTypes.QueryFee, data); } - function testTAPCollector_Collect_Multiple(uint256 tokens, uint8 steps) public useGateway useSigner { + function testTAPCollector_Collect_Multiple( + uint256 tokens, + uint8 steps + ) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner { steps = uint8(bound(steps, 1, 100)); tokens = bound(tokens, steps, type(uint128).max); - - _approveCollector(address(tapCollector), tokens); + _depositTokens(address(tapCollector), users.indexer, tokens); resetPrank(users.verifier); uint256 payed = 0; uint256 tokensPerStep = tokens / steps; for (uint256 i = 0; i < steps; i++) { - bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(payed + tokensPerStep)); + bytes memory data = _getQueryFeeEncodedData( + signerPrivateKey, + users.gateway, + users.indexer, + users.verifier, + uint128(payed + tokensPerStep) + ); _collect(IGraphPayments.PaymentTypes.QueryFee, data); payed += tokensPerStep; } } + function testTAPCollector_Collect_RevertWhen_NoProvision(uint256 tokens) public useGateway useSigner { + tokens = bound(tokens, 1, type(uint128).max); + + _depositTokens(address(tapCollector), users.indexer, tokens); + + bytes memory data = _getQueryFeeEncodedData( + signerPrivateKey, + users.gateway, + users.indexer, + users.verifier, + uint128(tokens) + ); + + resetPrank(users.verifier); + bytes memory expectedError = abi.encodeWithSelector( + ITAPCollector.TAPCollectorUnauthorizedDataService.selector, + users.verifier + ); + vm.expectRevert(expectedError); + tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data); + } + + function testTAPCollector_Collect_RevertWhen_ProvisionEmpty( + uint256 tokens + ) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner { + // thaw tokens from the provision + resetPrank(users.indexer); + staking.thaw(users.indexer, users.verifier, 100); + + tokens = bound(tokens, 1, type(uint128).max); + + resetPrank(users.gateway); + _depositTokens(address(tapCollector), users.indexer, tokens); + + bytes memory data = _getQueryFeeEncodedData( + signerPrivateKey, + users.gateway, + users.indexer, + users.verifier, + uint128(tokens) + ); + + resetPrank(users.verifier); + bytes memory expectedError = abi.encodeWithSelector( + ITAPCollector.TAPCollectorUnauthorizedDataService.selector, + users.verifier + ); + vm.expectRevert(expectedError); + tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data); + } + + function testTAPCollector_Collect_PreventSignerAttack( + uint256 tokens + ) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner { + tokens = bound(tokens, 1, type(uint128).max); + + resetPrank(users.gateway); + _depositTokens(address(tapCollector), users.indexer, tokens); + + // The sender authorizes another signer + (address anotherSigner, uint256 anotherSignerPrivateKey) = makeAddrAndKey("anotherSigner"); + { + uint256 proofDeadline = block.timestamp + 1; + bytes memory anotherSignerProof = _getSignerProof(proofDeadline, anotherSignerPrivateKey); + _authorizeSigner(anotherSigner, proofDeadline, anotherSignerProof); + } + + // And crafts a RAV using the new signer as the data service + bytes memory data = _getQueryFeeEncodedData( + anotherSignerPrivateKey, + users.gateway, + users.indexer, + anotherSigner, + uint128(tokens) + ); + + // the call should revert because the service provider has no provision with the "data service" + resetPrank(anotherSigner); + bytes memory expectedError = abi.encodeWithSelector( + ITAPCollector.TAPCollectorUnauthorizedDataService.selector, + anotherSigner + ); + vm.expectRevert(expectedError); + tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data); + } + function testTAPCollector_Collect_RevertWhen_CallerNotDataService(uint256 tokens) public useGateway useSigner { tokens = bound(tokens, 1, type(uint128).max); - + resetPrank(users.gateway); - _approveCollector(address(tapCollector), tokens); _depositTokens(address(tapCollector), users.indexer, tokens); - bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens)); + bytes memory data = _getQueryFeeEncodedData( + signerPrivateKey, + users.gateway, + users.indexer, + users.verifier, + uint128(tokens) + ); resetPrank(users.indexer); bytes memory expectedError = abi.encodeWithSelector( @@ -95,49 +203,93 @@ contract TAPCollectorCollectTest is TAPCollectorTest { tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data); } - function testTAPCollector_Collect_RevertWhen_InconsistentRAVTokens(uint256 tokens) public useGateway useSigner { + function testTAPCollector_Collect_RevertWhen_PayerMismatch( + uint256 tokens + ) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner { tokens = bound(tokens, 1, type(uint128).max); - - _approveCollector(address(tapCollector), tokens); + + resetPrank(users.gateway); _depositTokens(address(tapCollector), users.indexer, tokens); - bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens)); + + (address anotherPayer, ) = makeAddrAndKey("anotherPayer"); + bytes memory data = _getQueryFeeEncodedData( + signerPrivateKey, + anotherPayer, + users.indexer, + users.verifier, + uint128(tokens) + ); + + resetPrank(users.verifier); + bytes memory expectedError = abi.encodeWithSelector( + ITAPCollector.TAPCollectorInvalidRAVPayer.selector, + users.gateway, + anotherPayer + ); + vm.expectRevert(expectedError); + tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data); + } + + function testTAPCollector_Collect_RevertWhen_InconsistentRAVTokens( + uint256 tokens + ) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner { + tokens = bound(tokens, 1, type(uint128).max); + + _depositTokens(address(tapCollector), users.indexer, tokens); + bytes memory data = _getQueryFeeEncodedData( + signerPrivateKey, + users.gateway, + users.indexer, + users.verifier, + uint128(tokens) + ); resetPrank(users.verifier); _collect(IGraphPayments.PaymentTypes.QueryFee, data); // Attempt to collect again - vm.expectRevert(abi.encodeWithSelector( - ITAPCollector.TAPCollectorInconsistentRAVTokens.selector, - tokens, - tokens - )); + vm.expectRevert( + abi.encodeWithSelector(ITAPCollector.TAPCollectorInconsistentRAVTokens.selector, tokens, tokens) + ); tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data); } function testTAPCollector_Collect_RevertWhen_SignerNotAuthorized(uint256 tokens) public useGateway { tokens = bound(tokens, 1, type(uint128).max); - - _approveCollector(address(tapCollector), tokens); + _depositTokens(address(tapCollector), users.indexer, tokens); - - bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens)); + + bytes memory data = _getQueryFeeEncodedData( + signerPrivateKey, + users.gateway, + users.indexer, + users.verifier, + uint128(tokens) + ); resetPrank(users.verifier); vm.expectRevert(abi.encodeWithSelector(ITAPCollector.TAPCollectorInvalidRAVSigner.selector)); tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data); } - function testTAPCollector_Collect_ThawingSigner(uint256 tokens) public useGateway useSigner { + function testTAPCollector_Collect_ThawingSigner( + uint256 tokens + ) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner { tokens = bound(tokens, 1, type(uint128).max); - - _approveCollector(address(tapCollector), tokens); + _depositTokens(address(tapCollector), users.indexer, tokens); // Start thawing signer _thawSigner(signer); skip(revokeSignerThawingPeriod + 1); - - bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens)); + + bytes memory data = _getQueryFeeEncodedData( + signerPrivateKey, + users.gateway, + users.indexer, + users.verifier, + uint128(tokens) + ); resetPrank(users.verifier); _collect(IGraphPayments.PaymentTypes.QueryFee, data); @@ -145,36 +297,99 @@ contract TAPCollectorCollectTest is TAPCollectorTest { function testTAPCollector_Collect_RevertIf_SignerWasRevoked(uint256 tokens) public useGateway useSigner { tokens = bound(tokens, 1, type(uint128).max); - - _approveCollector(address(tapCollector), tokens); + _depositTokens(address(tapCollector), users.indexer, tokens); // Start thawing signer _thawSigner(signer); skip(revokeSignerThawingPeriod + 1); _revokeAuthorizedSigner(signer); - - bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens)); + + bytes memory data = _getQueryFeeEncodedData( + signerPrivateKey, + users.gateway, + users.indexer, + users.verifier, + uint128(tokens) + ); resetPrank(users.verifier); vm.expectRevert(abi.encodeWithSelector(ITAPCollector.TAPCollectorInvalidRAVSigner.selector)); tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data); } - function testTAPCollector_Collect_ThawingSignerCanceled(uint256 tokens) public useGateway useSigner { + function testTAPCollector_Collect_ThawingSignerCanceled( + uint256 tokens + ) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner { tokens = bound(tokens, 1, type(uint128).max); - - _approveCollector(address(tapCollector), tokens); + _depositTokens(address(tapCollector), users.indexer, tokens); // Start thawing signer _thawSigner(signer); skip(revokeSignerThawingPeriod + 1); _cancelThawSigner(signer); - - bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens)); + + bytes memory data = _getQueryFeeEncodedData( + signerPrivateKey, + users.gateway, + users.indexer, + users.verifier, + uint128(tokens) + ); resetPrank(users.verifier); _collect(IGraphPayments.PaymentTypes.QueryFee, data); } + + function testTAPCollector_CollectPartial( + uint256 tokens, + uint256 tokensToCollect + ) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner { + tokens = bound(tokens, 1, type(uint128).max); + tokensToCollect = bound(tokensToCollect, 1, tokens); + + _depositTokens(address(tapCollector), users.indexer, tokens); + + bytes memory data = _getQueryFeeEncodedData( + signerPrivateKey, + users.gateway, + users.indexer, + users.verifier, + uint128(tokens) + ); + + resetPrank(users.verifier); + _collect(IGraphPayments.PaymentTypes.QueryFee, data, tokensToCollect); + } + + function testTAPCollector_CollectPartial_RevertWhen_AmountTooHigh( + uint256 tokens, + uint256 tokensToCollect + ) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner { + tokens = bound(tokens, 1, type(uint128).max - 1); + + _depositTokens(address(tapCollector), users.indexer, tokens); + + bytes memory data = _getQueryFeeEncodedData( + signerPrivateKey, + users.gateway, + users.indexer, + users.verifier, + uint128(tokens) + ); + + resetPrank(users.verifier); + uint256 tokensAlreadyCollected = tapCollector.tokensCollected(users.verifier, users.indexer, users.gateway); + tokensToCollect = bound(tokensToCollect, tokens - tokensAlreadyCollected + 1, type(uint128).max); + + vm.expectRevert( + abi.encodeWithSelector( + ITAPCollector.TAPCollectorInvalidTokensToCollectAmount.selector, + tokensToCollect, + tokens - tokensAlreadyCollected + ) + ); + tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data, tokensToCollect); + } } diff --git a/packages/horizon/test/payments/tap-collector/signer/authorizeSigner.t.sol b/packages/horizon/test/payments/tap-collector/signer/authorizeSigner.t.sol index b337c48c7..30ffd4610 100644 --- a/packages/horizon/test/payments/tap-collector/signer/authorizeSigner.t.sol +++ b/packages/horizon/test/payments/tap-collector/signer/authorizeSigner.t.sol @@ -49,6 +49,27 @@ contract TAPCollectorAuthorizeSignerTest is TAPCollectorTest { tapCollector.authorizeSigner(signer, proofDeadline, signerProof); } + function testTAPCollector_AuthorizeSigner_RevertWhen_AlreadyAuthroizedAfterRevoking() public useGateway { + // Authorize signer + uint256 proofDeadline = block.timestamp + 1; + bytes memory signerProof = _getSignerProof(proofDeadline, signerPrivateKey); + _authorizeSigner(signer, proofDeadline, signerProof); + + // Revoke signer + _thawSigner(signer); + skip(revokeSignerThawingPeriod + 1); + _revokeAuthorizedSigner(signer); + + // Attempt to authorize signer again + bytes memory expectedError = abi.encodeWithSelector( + ITAPCollector.TAPCollectorSignerAlreadyAuthorized.selector, + users.gateway, + signer + ); + vm.expectRevert(expectedError); + tapCollector.authorizeSigner(signer, proofDeadline, signerProof); + } + function testTAPCollector_AuthorizeSigner_RevertWhen_ProofExpired() public useGateway { // Sign proof with payer uint256 proofDeadline = block.timestamp - 1; diff --git a/packages/horizon/test/payments/tap-collector/signer/thawSigner.t.sol b/packages/horizon/test/payments/tap-collector/signer/thawSigner.t.sol index fb47c37fb..def79d831 100644 --- a/packages/horizon/test/payments/tap-collector/signer/thawSigner.t.sol +++ b/packages/horizon/test/payments/tap-collector/signer/thawSigner.t.sol @@ -26,4 +26,31 @@ contract TAPCollectorThawSignerTest is TAPCollectorTest { vm.expectRevert(expectedError); tapCollector.thawSigner(signer); } + + function testTAPCollector_ThawSigner_RevertWhen_AlreadyRevoked() public useGateway useSigner { + _thawSigner(signer); + skip(revokeSignerThawingPeriod + 1); + _revokeAuthorizedSigner(signer); + + bytes memory expectedError = abi.encodeWithSelector( + ITAPCollector.TAPCollectorAuthorizationAlreadyRevoked.selector, + users.gateway, + signer + ); + vm.expectRevert(expectedError); + tapCollector.thawSigner(signer); + } + + function testTAPCollector_ThawSigner_RevertWhen_AlreadyThawing() public useGateway useSigner { + _thawSigner(signer); + + (,uint256 thawEndTimestamp,) = tapCollector.authorizedSigners(signer); + bytes memory expectedError = abi.encodeWithSelector( + ITAPCollector.TAPCollectorSignerAlreadyThawing.selector, + signer, + thawEndTimestamp + ); + vm.expectRevert(expectedError); + tapCollector.thawSigner(signer); + } } diff --git a/packages/horizon/test/shared/payments-escrow/PaymentsEscrowShared.t.sol b/packages/horizon/test/shared/payments-escrow/PaymentsEscrowShared.t.sol index 2bc435f7a..c3714dfd6 100644 --- a/packages/horizon/test/shared/payments-escrow/PaymentsEscrowShared.t.sol +++ b/packages/horizon/test/shared/payments-escrow/PaymentsEscrowShared.t.sol @@ -22,22 +22,6 @@ abstract contract PaymentsEscrowSharedTest is GraphBaseTest { * HELPERS */ - function _approveCollector(address _verifier, uint256 _tokens) internal { - (, address msgSender, ) = vm.readCallers(); - (uint256 beforeAllowance,) = escrow.authorizedCollectors(msgSender, _verifier); - vm.expectEmit(address(escrow)); - emit IPaymentsEscrow.AuthorizedCollector( - msgSender, // payer - _verifier, // collector - _tokens, // addedAllowance - beforeAllowance + _tokens // newTotalAllowance after the added allowance - ); - escrow.approveCollector(_verifier, _tokens); - (uint256 allowance, uint256 thawEndTimestamp) = escrow.authorizedCollectors(msgSender, _verifier); - assertEq(allowance - beforeAllowance, _tokens); - assertEq(thawEndTimestamp, 0); - } - function _depositTokens(address _collector, address _receiver, uint256 _tokens) internal { (, address msgSender, ) = vm.readCallers(); (uint256 escrowBalanceBefore,,) = escrow.escrowAccounts(msgSender, _collector, _receiver); diff --git a/packages/horizon/test/utils/Constants.sol b/packages/horizon/test/utils/Constants.sol index d96c39202..e74e3b0d1 100644 --- a/packages/horizon/test/utils/Constants.sol +++ b/packages/horizon/test/utils/Constants.sol @@ -7,7 +7,6 @@ abstract contract Constants { uint256 internal constant MAX_STAKING_TOKENS = 10_000_000_000 ether; // GraphEscrow parameters uint256 internal constant withdrawEscrowThawingPeriod = 60; - uint256 internal constant revokeCollectorThawingPeriod = 60; // GraphPayments parameters uint256 internal constant protocolPaymentCut = 10000; // Staking constants diff --git a/packages/subgraph-service/contracts/SubgraphService.sol b/packages/subgraph-service/contracts/SubgraphService.sol index a07463a25..b1403124b 100644 --- a/packages/subgraph-service/contracts/SubgraphService.sol +++ b/packages/subgraph-service/contracts/SubgraphService.sol @@ -556,13 +556,10 @@ contract SubgraphService is IGraphPayments.PaymentTypes.QueryFee, abi.encode(_signedRav, curationCut) ); - uint256 tokensCurators = tokensCollected.mulPPM(curationCut); uint256 balanceAfter = _graphToken().balanceOf(address(this)); - require( - balanceBefore + tokensCurators == balanceAfter, - SubgraphServiceInconsistentCollection(balanceBefore, balanceAfter, tokensCurators) - ); + require(balanceAfter >= balanceBefore, SubgraphServiceInconsistentCollection(balanceBefore, balanceAfter)); + uint256 tokensCurators = balanceAfter - balanceBefore; if (tokensCollected > 0) { // lock stake as economic security for fees diff --git a/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol b/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol index 32ea9e8fb..44b9b3e6d 100644 --- a/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol +++ b/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol @@ -80,13 +80,11 @@ interface ISubgraphService is IDataServiceFees { error SubgraphServiceInvalidPaymentType(IGraphPayments.PaymentTypes paymentType); /** - * @notice Thrown when the contract GRT balance is inconsistent with the payment amount collected - * from Graph Payments + * @notice Thrown when the contract GRT balance is inconsistent after collecting from Graph Payments * @param balanceBefore The contract GRT balance before the collection * @param balanceAfter The contract GRT balance after the collection - * @param tokensDataService The amount of tokens sent to the subgraph service */ - error SubgraphServiceInconsistentCollection(uint256 balanceBefore, uint256 balanceAfter, uint256 tokensDataService); + error SubgraphServiceInconsistentCollection(uint256 balanceBefore, uint256 balanceAfter); /** * @notice @notice Thrown when the service provider in the RAV does not match the expected indexer. diff --git a/packages/subgraph-service/test/SubgraphBaseTest.t.sol b/packages/subgraph-service/test/SubgraphBaseTest.t.sol index 099164473..663442e1f 100644 --- a/packages/subgraph-service/test/SubgraphBaseTest.t.sol +++ b/packages/subgraph-service/test/SubgraphBaseTest.t.sol @@ -113,7 +113,6 @@ abstract contract SubgraphBaseTest is Utils, Constants { vm.getCode("PaymentsEscrow.sol:PaymentsEscrow"), abi.encode( address(controller), - revokeCollectorThawingPeriod, withdrawEscrowThawingPeriod ) )); @@ -174,7 +173,6 @@ abstract contract SubgraphBaseTest is Utils, Constants { ); escrow = new PaymentsEscrow{salt: saltEscrow}( address(controller), - revokeCollectorThawingPeriod, withdrawEscrowThawingPeriod ); diff --git a/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol b/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol index b417f30bf..619ad4bab 100644 --- a/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol +++ b/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol @@ -232,7 +232,7 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest { ITAPCollector.SignedRAV memory signedRav = abi.decode(_data, (ITAPCollector.SignedRAV)); allocationId = abi.decode(signedRav.rav.metadata, (address)); allocation = subgraphService.getAllocation(allocationId); - (address payer, ) = tapCollector.authorizedSigners(_recoverRAVSigner(signedRav)); + (address payer, , ) = tapCollector.authorizedSigners(_recoverRAVSigner(signedRav)); // Total amount of tokens collected for indexer uint256 tokensCollected = tapCollector.tokensCollected(address(subgraphService), _indexer, payer); @@ -242,7 +242,8 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest { // Calculate curation cut uint256 curationFeesCut = subgraphService.curationFeesCut(); queryFeeData.curationCut = curation.isCurated(allocation.subgraphDeploymentId) ? curationFeesCut : 0; - uint256 tokensCurators = paymentCollected.mulPPM(queryFeeData.curationCut); + uint256 tokensProtocol = paymentCollected.mulPPMRoundUp(queryFeeData.protocolPaymentCut); + uint256 tokensCurators = (paymentCollected - tokensProtocol).mulPPMRoundUp(queryFeeData.curationCut); vm.expectEmit(address(subgraphService)); emit ISubgraphService.QueryFeesCollected(_indexer, paymentCollected, tokensCurators); @@ -302,8 +303,8 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest { if (_paymentType == IGraphPayments.PaymentTypes.QueryFee) { // Check indexer got paid the correct amount { - uint256 tokensProtocol = paymentCollected.mulPPM(protocolPaymentCut); - uint256 curationTokens = paymentCollected.mulPPM(queryFeeData.curationCut); + uint256 tokensProtocol = paymentCollected.mulPPMRoundUp(protocolPaymentCut); + uint256 curationTokens = (paymentCollected - tokensProtocol).mulPPMRoundUp(queryFeeData.curationCut); uint256 expectedIndexerTokensPayment = paymentCollected - tokensProtocol - curationTokens; assertEq( collectPaymentDataAfter.indexerBalance, diff --git a/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol b/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol index 93972679f..d42c36f65 100644 --- a/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol +++ b/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol @@ -46,6 +46,7 @@ contract SubgraphServiceRegisterTest is SubgraphServiceTest { ) private view returns (ITAPCollector.ReceiptAggregateVoucher memory rav) { return ITAPCollector.ReceiptAggregateVoucher({ + payer: users.gateway, dataService: address(subgraphService), serviceProvider: indexer, timestampNs: 0, @@ -54,8 +55,7 @@ contract SubgraphServiceRegisterTest is SubgraphServiceTest { }); } - function _approveCollector(uint256 tokens) private { - escrow.approveCollector(address(tapCollector), tokens); + function _deposit(uint256 tokens) private { token.approve(address(escrow), tokens); escrow.deposit(address(tapCollector), users.indexer, tokens); } @@ -91,7 +91,7 @@ contract SubgraphServiceRegisterTest is SubgraphServiceTest { tokensPayment = bound(tokensPayment, minimumProvisionTokens, maxTokensPayment); resetPrank(users.gateway); - _approveCollector(tokensPayment); + _deposit(tokensPayment); _authorizeSigner(); resetPrank(users.indexer); @@ -108,7 +108,7 @@ contract SubgraphServiceRegisterTest is SubgraphServiceTest { uint256 tokensPayment = tokensAllocated / stakeToFeesRatio / numPayments; resetPrank(users.gateway); - _approveCollector(tokensAllocated); + _deposit(tokensAllocated); _authorizeSigner(); resetPrank(users.indexer); diff --git a/packages/subgraph-service/test/utils/Constants.sol b/packages/subgraph-service/test/utils/Constants.sol index 76f864da1..025396ea8 100644 --- a/packages/subgraph-service/test/utils/Constants.sol +++ b/packages/subgraph-service/test/utils/Constants.sol @@ -21,7 +21,6 @@ abstract contract Constants { uint64 internal constant MAX_WAIT_PERIOD = 28 days; // GraphEscrow parameters uint256 internal constant withdrawEscrowThawingPeriod = 60; - uint256 internal constant revokeCollectorThawingPeriod = 60; // GraphPayments parameters uint256 internal constant protocolPaymentCut = 10000; // RewardsMananger parameters