diff --git a/packages/horizon/contracts/data-service/DataService.sol b/packages/horizon/contracts/data-service/DataService.sol index 8a06ad4ea..7923f1903 100644 --- a/packages/horizon/contracts/data-service/DataService.sol +++ b/packages/horizon/contracts/data-service/DataService.sol @@ -25,6 +25,8 @@ import { ProvisionManager } from "./utilities/ProvisionManager.sol"; * - If the data service implementation is NOT upgradeable, it must initialize the contract by calling * {__DataService_init} or {__DataService_init_unchained} in the constructor. Note that the `initializer` * will be required in the constructor. + * - Note that in both cases if using {__DataService_init_unchained} variant the corresponding parent + * initializers must be called in the implementation. */ abstract contract DataService is GraphDirectory, ProvisionManager, DataServiceV1Storage, IDataService { /** diff --git a/packages/horizon/contracts/data-service/DataServiceStorage.sol b/packages/horizon/contracts/data-service/DataServiceStorage.sol index a0271443c..3d8e84f82 100644 --- a/packages/horizon/contracts/data-service/DataServiceStorage.sol +++ b/packages/horizon/contracts/data-service/DataServiceStorage.sol @@ -3,5 +3,6 @@ pragma solidity 0.8.27; abstract contract DataServiceV1Storage { /// @dev Gap to allow adding variables in future upgrades + /// Note that this contract is not upgradeable but might be inherited by an upgradeable contract uint256[50] private __gap; } diff --git a/packages/horizon/contracts/data-service/extensions/DataServiceFees.sol b/packages/horizon/contracts/data-service/extensions/DataServiceFees.sol index 09102dd4a..6685dc5a7 100644 --- a/packages/horizon/contracts/data-service/extensions/DataServiceFees.sol +++ b/packages/horizon/contracts/data-service/extensions/DataServiceFees.sol @@ -14,6 +14,8 @@ import { DataServiceFeesV1Storage } from "./DataServiceFeesStorage.sol"; * @dev Implementation of the {IDataServiceFees} interface. * @notice Extension for the {IDataService} contract to handle payment collateralization * using a Horizon provision. See {IDataServiceFees} for more details. + * @dev This contract inherits from {DataService} which needs to be initialized, please see + * {DataService} for detailed instructions. */ abstract contract DataServiceFees is DataService, DataServiceFeesV1Storage, IDataServiceFees { using ProvisionTracker for mapping(address => uint256); @@ -127,9 +129,7 @@ abstract contract DataServiceFees is DataService, DataServiceFeesV1Storage, IDat * @param _claimId The ID of the stake claim */ function _getNextStakeClaim(bytes32 _claimId) private view returns (bytes32) { - StakeClaim memory claim = claims[_claimId]; - require(claim.createdAt != 0, DataServiceFeesClaimNotFound(_claimId)); - return claim.nextClaim; + return claims[_claimId].nextClaim; } /** diff --git a/packages/horizon/contracts/data-service/extensions/DataServiceFeesStorage.sol b/packages/horizon/contracts/data-service/extensions/DataServiceFeesStorage.sol index cb4f908dc..853b57209 100644 --- a/packages/horizon/contracts/data-service/extensions/DataServiceFeesStorage.sol +++ b/packages/horizon/contracts/data-service/extensions/DataServiceFeesStorage.sol @@ -18,5 +18,6 @@ abstract contract DataServiceFeesV1Storage { mapping(address serviceProvider => LinkedList.List list) public claimsLists; /// @dev Gap to allow adding variables in future upgrades + /// Note that this contract is not upgradeable but might be inherited by an upgradeable contract uint256[50] private __gap; } diff --git a/packages/horizon/contracts/data-service/extensions/DataServicePausable.sol b/packages/horizon/contracts/data-service/extensions/DataServicePausable.sol index 3c83fbb0e..475614454 100644 --- a/packages/horizon/contracts/data-service/extensions/DataServicePausable.sol +++ b/packages/horizon/contracts/data-service/extensions/DataServicePausable.sol @@ -14,6 +14,8 @@ import { DataService } from "../DataService.sol"; * pause guardians. * @dev Note that this extension does not provide an external function to set pause * guardians. This should be implemented in the derived contract. + * @dev This contract inherits from {DataService} which needs to be initialized, please see + * {DataService} for detailed instructions. */ abstract contract DataServicePausable is Pausable, DataService, IDataServicePausable { /// @notice List of pause guardians and their allowed status @@ -51,6 +53,10 @@ abstract contract DataServicePausable is Pausable, DataService, IDataServicePaus * @param _allowed The allowed status of the pause guardian */ function _setPauseGuardian(address _pauseGuardian, bool _allowed) internal { + require( + pauseGuardians[_pauseGuardian] == !_allowed, + DataServicePausablePauseGuardianNoChange(_pauseGuardian, _allowed) + ); pauseGuardians[_pauseGuardian] = _allowed; emit PauseGuardianSet(_pauseGuardian, _allowed); } diff --git a/packages/horizon/contracts/data-service/extensions/DataServicePausableUpgradeable.sol b/packages/horizon/contracts/data-service/extensions/DataServicePausableUpgradeable.sol index 82d7cc63b..2cecdedb6 100644 --- a/packages/horizon/contracts/data-service/extensions/DataServicePausableUpgradeable.sol +++ b/packages/horizon/contracts/data-service/extensions/DataServicePausableUpgradeable.sol @@ -10,11 +10,16 @@ import { DataService } from "../DataService.sol"; * @title DataServicePausableUpgradeable contract * @dev Implementation of the {IDataServicePausable} interface. * @dev Upgradeable version of the {DataServicePausable} contract. + * @dev This contract inherits from {DataService} which needs to be initialized, please see + * {DataService} for detailed instructions. */ abstract contract DataServicePausableUpgradeable is PausableUpgradeable, DataService, IDataServicePausable { /// @notice List of pause guardians and their allowed status mapping(address pauseGuardian => bool allowed) public pauseGuardians; + /// @dev Gap to allow adding variables in future upgrades + uint256[50] private __gap; + /** * @notice Checks if the caller is a pause guardian. */ @@ -61,7 +66,11 @@ abstract contract DataServicePausableUpgradeable is PausableUpgradeable, DataSer * @param _pauseGuardian The address of the pause guardian * @param _allowed The allowed status of the pause guardian */ - function _setPauseGuardian(address _pauseGuardian, bool _allowed) internal whenNotPaused { + function _setPauseGuardian(address _pauseGuardian, bool _allowed) internal { + require( + pauseGuardians[_pauseGuardian] == !_allowed, + DataServicePausablePauseGuardianNoChange(_pauseGuardian, _allowed) + ); pauseGuardians[_pauseGuardian] = _allowed; emit PauseGuardianSet(_pauseGuardian, _allowed); } diff --git a/packages/horizon/contracts/data-service/extensions/DataServiceRescuable.sol b/packages/horizon/contracts/data-service/extensions/DataServiceRescuable.sol index 0d2f0750d..0f57862d5 100644 --- a/packages/horizon/contracts/data-service/extensions/DataServiceRescuable.sol +++ b/packages/horizon/contracts/data-service/extensions/DataServiceRescuable.sol @@ -17,11 +17,17 @@ import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.s * that calls this contract's _rescueTokens. * @dev Note that this extension does not provide an external function to set * rescuers. This should be implemented in the derived contract. + * @dev This contract inherits from {DataService} which needs to be initialized, please see + * {DataService} for detailed instructions. */ abstract contract DataServiceRescuable is DataService, IDataServiceRescuable { /// @notice List of rescuers and their allowed status mapping(address rescuer => bool allowed) public rescuers; + /// @dev Gap to allow adding variables in future upgrades + /// Note that this contract is not upgradeable but might be inherited by an upgradeable contract + uint256[50] private __gap; + /** * @notice Checks if the caller is a rescuer. */ diff --git a/packages/horizon/contracts/data-service/interfaces/IDataServicePausable.sol b/packages/horizon/contracts/data-service/interfaces/IDataServicePausable.sol index bd27ca848..0579c6649 100644 --- a/packages/horizon/contracts/data-service/interfaces/IDataServicePausable.sol +++ b/packages/horizon/contracts/data-service/interfaces/IDataServicePausable.sol @@ -23,6 +23,13 @@ interface IDataServicePausable is IDataService { */ error DataServicePausableNotPauseGuardian(address account); + /** + * @notice Emitted when a pause guardian is set to the same allowed status + * @param account The address of the pause guardian + * @param allowed The allowed status of the pause guardian + */ + error DataServicePausablePauseGuardianNoChange(address account, bool allowed); + /** * @notice Pauses the data service. * @dev Note that only functions using the modifiers `whenNotPaused` diff --git a/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol b/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol index a3f96794c..68ac2813d 100644 --- a/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol +++ b/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol @@ -202,10 +202,16 @@ abstract contract ProvisionManager is Initializable, GraphDirectory, ProvisionMa /** * @notice Checks if the provision tokens of a service provider are within the valid range. + * Note that thawing tokens are not considered in this check. * @param _provision The provision to check. */ function _checkProvisionTokens(IHorizonStaking.Provision memory _provision) internal view virtual { - _checkValueInRange(_provision.tokens, minimumProvisionTokens, maximumProvisionTokens, "tokens"); + _checkValueInRange( + _provision.tokens - _provision.tokensThawing, + minimumProvisionTokens, + maximumProvisionTokens, + "tokens" + ); } /** diff --git a/packages/horizon/contracts/data-service/utilities/ProvisionManagerStorage.sol b/packages/horizon/contracts/data-service/utilities/ProvisionManagerStorage.sol index 0a6bed2be..0732afc9a 100644 --- a/packages/horizon/contracts/data-service/utilities/ProvisionManagerStorage.sol +++ b/packages/horizon/contracts/data-service/utilities/ProvisionManagerStorage.sol @@ -28,5 +28,6 @@ abstract contract ProvisionManagerV1Storage { uint32 public delegationRatio; /// @dev Gap to allow adding variables in future upgrades + /// Note that this contract is not upgradeable but might be inherited by an upgradeable contract uint256[50] private __gap; } 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/interfaces/internal/IHorizonStakingBase.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingBase.sol index e221dc2cf..0145dac16 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingBase.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingBase.sol @@ -24,6 +24,11 @@ interface IHorizonStakingBase { */ event StakeDeposited(address indexed serviceProvider, uint256 tokens); + /** + * @notice Thrown when using an invalid thaw request type. + */ + error HorizonStakingInvalidThawRequestType(); + /** * @notice Gets the details of a service provider. * @param serviceProvider The address of the service provider. @@ -134,21 +139,27 @@ interface IHorizonStakingBase { /** * @notice Gets a thaw request. + * @param thawRequestType The type of thaw request. * @param thawRequestId The id of the thaw request. * @return The thaw request details. */ - function getThawRequest(bytes32 thawRequestId) external view returns (IHorizonStakingTypes.ThawRequest memory); + function getThawRequest( + IHorizonStakingTypes.ThawRequestType thawRequestType, + bytes32 thawRequestId + ) external view returns (IHorizonStakingTypes.ThawRequest memory); /** * @notice Gets the metadata of a thaw request list. * Service provider and delegators each have their own thaw request list per provision. * Metadata includes the head and tail of the list, plus the total number of thaw requests. + * @param thawRequestType The type of thaw request. * @param serviceProvider The address of the service provider. * @param verifier The address of the verifier. * @param owner The owner of the thaw requests. Use either the service provider or delegator address. * @return The thaw requests list metadata. */ function getThawRequestList( + IHorizonStakingTypes.ThawRequestType thawRequestType, address serviceProvider, address verifier, address owner @@ -156,12 +167,18 @@ interface IHorizonStakingBase { /** * @notice Gets the amount of thawed tokens for a given provision. + * @param thawRequestType The type of thaw request. * @param serviceProvider The address of the service provider. * @param verifier The address of the verifier. * @param owner The owner of the thaw requests. Use either the service provider or delegator address. * @return The amount of thawed tokens. */ - function getThawedTokens(address serviceProvider, address verifier, address owner) external view returns (uint256); + function getThawedTokens( + IHorizonStakingTypes.ThawRequestType thawRequestType, + address serviceProvider, + address verifier, + address owner + ) external view returns (uint256); /** * @notice Gets the maximum allowed thawing period for a provision. diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol index a0b2dc1af..84318f536 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol @@ -77,6 +77,12 @@ interface IHorizonStakingExtension is IRewardsIssuer { uint256 delegationRewards ); + /** + * @dev Emitted when `indexer` was slashed for a total of `tokens` amount. + * Tracks `reward` amount of tokens given to `beneficiary`. + */ + event StakeSlashed(address indexed indexer, uint256 tokens, uint256 reward, address beneficiary); + /** * @notice Close an allocation and free the staked tokens. * To be eligible for rewards a proof of indexing must be presented. @@ -148,4 +154,14 @@ interface IHorizonStakingExtension is IRewardsIssuer { */ // solhint-disable-next-line func-name-mixedcase function __DEPRECATED_getThawingPeriod() external view returns (uint64); + + /** + * @notice Slash the indexer stake. Delegated tokens are not subject to slashing. + * @dev Can only be called by the slasher role. + * @param indexer Address of indexer to slash + * @param tokens Amount of tokens to slash from the indexer stake + * @param reward Amount of reward tokens to send to a beneficiary + * @param beneficiary Address of a beneficiary to receive a reward for the slashing + */ + function legacySlash(address indexer, uint256 tokens, uint256 reward, address beneficiary) external; } diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index b144b0ce6..50ca90fc3 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.27; import { IGraphPayments } from "../../interfaces/IGraphPayments.sol"; +import { IHorizonStakingTypes } from "./IHorizonStakingTypes.sol"; /** * @title Inferface for the {HorizonStaking} contract. @@ -205,6 +206,15 @@ interface IHorizonStakingMain { uint256 tokens ); + /** + * @notice Emitted when `delegator` withdrew delegated `tokens` from `indexer` using `withdrawDelegated`. + * @dev This event is for the legacy `withdrawDelegated` function. + * @param indexer The address of the indexer + * @param delegator The address of the delegator + * @param tokens The amount of tokens withdrawn + */ + event StakeDelegatedWithdrawn(address indexed indexer, address indexed delegator, uint256 tokens); + /** * @notice Emitted when tokens are added to a delegation pool's reserve. * @param serviceProvider The address of the service provider @@ -271,13 +281,15 @@ interface IHorizonStakingMain { * @param owner The address of the owner of the thaw requests * @param thawRequestsFulfilled The number of thaw requests fulfilled * @param tokens The total amount of tokens being released + * @param requestType The type of thaw request */ event ThawRequestsFulfilled( address indexed serviceProvider, address indexed verifier, address indexed owner, uint256 thawRequestsFulfilled, - uint256 tokens + uint256 tokens, + IHorizonStakingTypes.ThawRequestType requestType ); // -- Events: governance -- @@ -303,9 +315,8 @@ interface IHorizonStakingMain { /** * @notice Emitted when the delegation slashing global flag is set. - * @param enabled Whether delegation slashing is enabled or disabled. */ - event DelegationSlashingEnabled(bool enabled); + event DelegationSlashingEnabled(); // -- Errors: tokens @@ -415,6 +426,20 @@ interface IHorizonStakingMain { */ error HorizonStakingInvalidDelegationPool(address serviceProvider, address verifier); + /** + * @notice Thrown when the minimum token amount required for delegation is not met. + * @param tokens The actual token amount + * @param minTokens The minimum required token amount + */ + error HorizonStakingInsufficientDelegationTokens(uint256 tokens, uint256 minTokens); + + /** + * @notice Thrown when the minimum token amount required for undelegation with beneficiary is not met. + * @param tokens The actual token amount + * @param minTokens The minimum required token amount + */ + error HorizonStakingInsufficientUndelegationTokens(uint256 tokens, uint256 minTokens); + /** * @notice Thrown when attempting to undelegate with a beneficiary that is the zero address. */ @@ -515,6 +540,8 @@ interface IHorizonStakingMain { * - During the transition period it's locked for a period of time before it can be withdrawn * by calling {withdraw}. * - After the transition period it's immediately withdrawn. + * Note that after the transition period if there are tokens still locked they will have to be + * withdrawn by calling {withdraw}. * @dev Requirements: * - `_tokens` cannot be zero. * - `_serviceProvider` must have enough idle stake to cover the staking amount and any @@ -747,7 +774,7 @@ interface IHorizonStakingMain { * @param beneficiary The address where the tokens will be withdrawn after thawing * @return The ID of the thaw request */ - function undelegate( + function undelegateWithBeneficiary( address serviceProvider, address verifier, uint256 shares, @@ -772,6 +799,28 @@ interface IHorizonStakingMain { */ function withdrawDelegated(address serviceProvider, address verifier, uint256 nThawRequests) external; + /** + * @notice Withdraw undelegated with beneficiary tokens from a provision after thawing. + * @dev The parameter `nThawRequests` can be set to a non zero value to fulfill a specific number of thaw + * requests in the event that fulfilling all of them results in a gas limit error. + * @dev If the delegation pool was completely slashed before withdrawing, calling this function will fulfill + * the thaw requests with an amount equal to zero. + * + * Requirements: + * - Must have previously initiated a thaw request using {undelegateWithBeneficiary}. + * + * Emits {ThawRequestFulfilled}, {ThawRequestsFulfilled} and {DelegatedTokensWithdrawn} events. + * + * @param serviceProvider The service provider address + * @param verifier The verifier address + * @param nThawRequests The number of thaw requests to fulfill. Set to 0 to fulfill all thaw requests. + */ + function withdrawDelegatedWithBeneficiary( + address serviceProvider, + address verifier, + uint256 nThawRequests + ) external; + /** * @notice Re-delegate undelegated tokens from a provision after thawing to a `newServiceProvider` and `newVerifier`. * @dev The parameter `nThawRequests` can be set to a non zero value to fulfill a specific number of thaw @@ -838,13 +887,14 @@ interface IHorizonStakingMain { /** * @notice Withdraw undelegated tokens from the subgraph data service provision after thawing. * This function is for backwards compatibility with the legacy staking contract. - * It only allows withdrawing from the subgraph data service and DOES NOT have slippage protection in - * case the caller opts for re-delegating. + * It only allows withdrawing tokens undelegated before horizon upgrade. * @dev See {delegate}. * @param serviceProvider The service provider address - * @param newServiceProvider The address of a new service provider, if the delegator wants to re-delegate */ - function withdrawDelegated(address serviceProvider, address newServiceProvider) external; + function withdrawDelegated( + address serviceProvider, + address // newServiceProvider, deprecated + ) external returns (uint256); /** * @notice Slash a service provider. This can only be called by a verifier to which diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol index 0dfc6c774..e2376bf18 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol @@ -131,6 +131,17 @@ interface IHorizonStakingTypes { uint256 __DEPRECATED_tokensLockedUntil; } + /** + * @dev Enum to specify the type of thaw request. + * @param Provision Represents a thaw request for a provision. + * @param Delegation Represents a thaw request for a delegation. + */ + enum ThawRequestType { + Provision, + Delegation, + DelegationWithBeneficiary + } + /** * @notice Details of a stake thawing operation. * @dev ThawRequests are stored in linked lists by service provider/delegator, @@ -146,4 +157,42 @@ interface IHorizonStakingTypes { // Used to invalidate unfulfilled thaw requests uint256 thawingNonce; } + + /** + * @notice Parameters to fulfill thaw requests. + * @dev This struct is used to avoid stack too deep error in the `fulfillThawRequests` function. + * @param requestType The type of thaw request (Provision or Delegation) + * @param serviceProvider The address of the service provider + * @param verifier The address of the verifier + * @param owner The address of the owner of the thaw request + * @param tokensThawing The current amount of tokens already thawing + * @param sharesThawing The current amount of shares already thawing + * @param nThawRequests The number of thaw requests to fulfill. If set to 0, all thaw requests are fulfilled. + * @param thawingNonce The current valid thawing nonce. Any thaw request with a different nonce is invalid and should be ignored. + */ + struct FulfillThawRequestsParams { + ThawRequestType requestType; + address serviceProvider; + address verifier; + address owner; + uint256 tokensThawing; + uint256 sharesThawing; + uint256 nThawRequests; + uint256 thawingNonce; + } + + /** + * @notice Results of the traversal of thaw requests. + * @dev This struct is used to avoid stack too deep error in the `fulfillThawRequests` function. + * @param requestsFulfilled The number of thaw requests fulfilled + * @param tokensThawed The total amount of tokens thawed + * @param tokensThawing The total amount of tokens thawing + * @param sharesThawing The total amount of shares thawing + */ + struct TraverseThawRequestsResults { + uint256 requestsFulfilled; + uint256 tokensThawed; + uint256 tokensThawing; + uint256 sharesThawing; + } } 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/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 8df8f20f6..3414fe555 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.27; import { IGraphToken } from "@graphprotocol/contracts/contracts/token/IGraphToken.sol"; import { IHorizonStakingMain } from "../interfaces/internal/IHorizonStakingMain.sol"; +import { IHorizonStakingExtension } from "../interfaces/internal/IHorizonStakingExtension.sol"; import { IGraphPayments } from "../interfaces/IGraphPayments.sol"; import { TokenUtils } from "@graphprotocol/contracts/contracts/utils/TokenUtils.sol"; @@ -35,11 +36,17 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint256 private constant FIXED_POINT_PRECISION = 1e18; /// @dev Maximum number of simultaneous stake thaw requests (per provision) or undelegations (per delegation) - uint256 private constant MAX_THAW_REQUESTS = 100; + uint256 private constant MAX_THAW_REQUESTS = 1_000; /// @dev Address of the staking extension contract address private immutable STAKING_EXTENSION_ADDRESS; + /// @dev Minimum amount of delegation. + uint256 private constant MIN_DELEGATION = 1e18; + + /// @dev Minimum amount of undelegation with beneficiary. + uint256 private constant MIN_UNDELEGATION_WITH_BENEFICIARY = 10e18; + /** * @notice Checks that the caller is authorized to operate over a provision. * @param serviceProvider The address of the service provider. @@ -297,20 +304,20 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { address verifier, uint256 shares ) external override notPaused returns (bytes32) { - return _undelegate(serviceProvider, verifier, shares, msg.sender); + return _undelegate(ThawRequestType.Delegation, serviceProvider, verifier, shares, msg.sender); } /** * @notice See {IHorizonStakingMain-undelegate}. */ - function undelegate( + function undelegateWithBeneficiary( address serviceProvider, address verifier, uint256 shares, address beneficiary ) external override notPaused returns (bytes32) { require(beneficiary != address(0), HorizonStakingInvalidBeneficiaryZeroAddress()); - return _undelegate(serviceProvider, verifier, shares, beneficiary); + return _undelegate(ThawRequestType.DelegationWithBeneficiary, serviceProvider, verifier, shares, beneficiary); } /** @@ -321,7 +328,34 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { address verifier, uint256 nThawRequests ) external override notPaused { - _withdrawDelegated(serviceProvider, verifier, address(0), address(0), 0, nThawRequests); + _withdrawDelegated( + ThawRequestType.Delegation, + serviceProvider, + verifier, + address(0), + address(0), + 0, + nThawRequests + ); + } + + /** + * @notice See {IHorizonStakingMain-withdrawDelegatedWithBeneficiary}. + */ + function withdrawDelegatedWithBeneficiary( + address serviceProvider, + address verifier, + uint256 nThawRequests + ) external override notPaused { + _withdrawDelegated( + ThawRequestType.DelegationWithBeneficiary, + serviceProvider, + verifier, + address(0), + address(0), + 0, + nThawRequests + ); } /** @@ -338,6 +372,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { require(newServiceProvider != address(0), HorizonStakingInvalidServiceProviderZeroAddress()); require(newVerifier != address(0), HorizonStakingInvalidVerifierZeroAddress()); _withdrawDelegated( + ThawRequestType.Delegation, oldServiceProvider, oldVerifier, newServiceProvider, @@ -374,21 +409,43 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * @notice See {IHorizonStakingMain-undelegate}. */ function undelegate(address serviceProvider, uint256 shares) external override notPaused { - _undelegate(serviceProvider, SUBGRAPH_DATA_SERVICE_ADDRESS, shares, msg.sender); + _undelegate(ThawRequestType.Delegation, serviceProvider, SUBGRAPH_DATA_SERVICE_ADDRESS, shares, msg.sender); } /** * @notice See {IHorizonStakingMain-withdrawDelegated}. */ - function withdrawDelegated(address serviceProvider, address newServiceProvider) external override notPaused { - _withdrawDelegated( - serviceProvider, - SUBGRAPH_DATA_SERVICE_ADDRESS, - newServiceProvider, - SUBGRAPH_DATA_SERVICE_ADDRESS, - 0, - 0 - ); + function withdrawDelegated( + address serviceProvider, + address // newServiceProvider, deprecated + ) external override notPaused returns (uint256) { + // Get the delegation pool of the indexer + address delegator = msg.sender; + DelegationPoolInternal storage pool = _legacyDelegationPools[serviceProvider]; + DelegationInternal storage delegation = pool.delegators[delegator]; + + // Validation + uint256 tokensToWithdraw = 0; + uint256 currentEpoch = _graphEpochManager().currentEpoch(); + if ( + delegation.__DEPRECATED_tokensLockedUntil > 0 && currentEpoch >= delegation.__DEPRECATED_tokensLockedUntil + ) { + tokensToWithdraw = delegation.__DEPRECATED_tokensLocked; + } + require(tokensToWithdraw > 0, "!tokens"); + + // Reset lock + delegation.__DEPRECATED_tokensLocked = 0; + delegation.__DEPRECATED_tokensLockedUntil = 0; + + emit StakeDelegatedWithdrawn(serviceProvider, delegator, tokensToWithdraw); + + // -- Interactions -- + + // Return tokens to the delegator + _graphToken().pushTokens(delegator, tokensToWithdraw); + + return tokensToWithdraw; } /* @@ -404,6 +461,23 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint256 tokensVerifier, address verifierDestination ) external override notPaused { + // TODO remove after the transition period + // Check if sender is authorized to slash on the deprecated list + if (__DEPRECATED_slashers[msg.sender]) { + // Forward call to staking extension + (bool success, ) = STAKING_EXTENSION_ADDRESS.delegatecall( + abi.encodeWithSelector( + IHorizonStakingExtension.legacySlash.selector, + serviceProvider, + tokens, + tokensVerifier, + verifierDestination + ) + ); + require(success, "Delegatecall to legacySlash failed"); + return; + } + address verifier = msg.sender; Provision storage prov = _provisions[serviceProvider][verifier]; DelegationPoolInternal storage pool = _getDelegationPool(serviceProvider, verifier); @@ -432,8 +506,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { _graphToken().burnTokens(providerTokensSlashed - tokensVerifier); // Provision accounting - // TODO check for rounding issues - uint256 provisionFractionSlashed = (providerTokensSlashed * FIXED_POINT_PRECISION) / prov.tokens; + uint256 provisionFractionSlashed = (providerTokensSlashed * FIXED_POINT_PRECISION + prov.tokens - 1) / + prov.tokens; prov.tokensThawing = (prov.tokensThawing * (FIXED_POINT_PRECISION - provisionFractionSlashed)) / (FIXED_POINT_PRECISION); @@ -468,7 +542,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { _graphToken().burnTokens(tokensToSlash); // Delegation pool accounting - uint256 delegationFractionSlashed = (tokensToSlash * FIXED_POINT_PRECISION) / pool.tokens; + uint256 delegationFractionSlashed = (tokensToSlash * FIXED_POINT_PRECISION + pool.tokens - 1) / + pool.tokens; pool.tokens = pool.tokens - tokensToSlash; pool.tokensThawing = (pool.tokensThawing * (FIXED_POINT_PRECISION - delegationFractionSlashed)) / @@ -534,7 +609,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { */ function setDelegationSlashingEnabled() external override onlyGovernor { _delegationSlashingEnabled = true; - emit DelegationSlashingEnabled(_delegationSlashingEnabled); + emit DelegationSlashingEnabled(); } /** @@ -663,7 +738,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { } /** - * @notice See {IHorizonStakingMain-createProvision}. + * @notice See {IHorizonStakingMain-provision}. */ function _createProvision( address _serviceProvider, @@ -732,15 +807,17 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { // Calculate shares to issue // Thawing pool is reset/initialized when the pool is empty: prov.tokensThawing == 0 + // Round thawing shares up to ensure fairness and avoid undervaluing the shares due to rounding down. uint256 thawingShares = prov.tokensThawing == 0 ? _tokens - : ((prov.sharesThawing * _tokens) / prov.tokensThawing); + : ((prov.sharesThawing * _tokens + prov.tokensThawing - 1) / prov.tokensThawing); uint64 thawingUntil = uint64(block.timestamp + uint256(prov.thawingPeriod)); prov.sharesThawing = prov.sharesThawing + thawingShares; prov.tokensThawing = prov.tokensThawing + _tokens; bytes32 thawRequestId = _createThawRequest( + ThawRequestType.Provision, _serviceProvider, _verifier, _serviceProvider, @@ -765,15 +842,18 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint256 tokensThawed_ = 0; uint256 sharesThawing = prov.sharesThawing; uint256 tokensThawing = prov.tokensThawing; - (tokensThawed_, tokensThawing, sharesThawing) = _fulfillThawRequests( - _serviceProvider, - _verifier, - _serviceProvider, - tokensThawing, - sharesThawing, - _nThawRequests, - prov.thawingNonce - ); + + FulfillThawRequestsParams memory params = FulfillThawRequestsParams({ + requestType: ThawRequestType.Provision, + serviceProvider: _serviceProvider, + verifier: _verifier, + owner: _serviceProvider, + tokensThawing: tokensThawing, + sharesThawing: sharesThawing, + nThawRequests: _nThawRequests, + thawingNonce: prov.thawingNonce + }); + (tokensThawed_, tokensThawing, sharesThawing) = _fulfillThawRequests(params); prov.tokens = prov.tokens - tokensThawed_; prov.sharesThawing = sharesThawing; @@ -790,6 +870,9 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * have been done before calling this function. */ function _delegate(address _serviceProvider, address _verifier, uint256 _tokens, uint256 _minSharesOut) private { + // Enforces a minimum delegation amount to prevent share manipulation attacks. + // This stops attackers from inflating share value and blocking other delegators. + require(_tokens >= MIN_DELEGATION, HorizonStakingInsufficientDelegationTokens(_tokens, MIN_DELEGATION)); require( _provisions[_serviceProvider][_verifier].createdAt != 0, HorizonStakingInvalidProvision(_serviceProvider, _verifier) @@ -833,6 +916,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * that were not thawing will be preserved. */ function _undelegate( + ThawRequestType _requestType, address _serviceProvider, address _verifier, uint256 _shares, @@ -850,6 +934,18 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { // delegation pool shares -> delegation pool tokens -> thawing pool shares // Thawing pool is reset/initialized when the pool is empty: prov.tokensThawing == 0 uint256 tokens = (_shares * (pool.tokens - pool.tokensThawing)) / pool.shares; + + // Since anyone can undelegate for any beneficiary, we require a minimum amount to prevent + // malicious actors from flooding the thaw request list with tiny amounts and causing a + // denial of service attack by hitting the MAX_THAW_REQUESTS limit + if (_requestType == ThawRequestType.DelegationWithBeneficiary) { + require( + tokens >= MIN_UNDELEGATION_WITH_BENEFICIARY, + HorizonStakingInsufficientUndelegationTokens(tokens, MIN_UNDELEGATION_WITH_BENEFICIARY) + ); + } + + // Thawing shares are rounded down to protect the pool and avoid taking extra tokens from other participants. uint256 thawingShares = pool.tokensThawing == 0 ? tokens : ((tokens * pool.sharesThawing) / pool.tokensThawing); uint64 thawingUntil = uint64(block.timestamp + uint256(_provisions[_serviceProvider][_verifier].thawingPeriod)); @@ -858,8 +954,16 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { pool.shares = pool.shares - _shares; delegation.shares = delegation.shares - _shares; + if (delegation.shares != 0) { + uint256 remainingTokens = (delegation.shares * (pool.tokens - pool.tokensThawing)) / pool.shares; + require( + remainingTokens >= MIN_DELEGATION, + HorizonStakingInsufficientTokens(remainingTokens, MIN_DELEGATION) + ); + } bytes32 thawRequestId = _createThawRequest( + _requestType, _serviceProvider, _verifier, _beneficiary, @@ -876,6 +980,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * @notice See {IHorizonStakingMain-withdrawDelegated}. */ function _withdrawDelegated( + ThawRequestType _requestType, address _serviceProvider, address _verifier, address _newServiceProvider, @@ -894,15 +999,18 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint256 tokensThawed = 0; uint256 sharesThawing = pool.sharesThawing; uint256 tokensThawing = pool.tokensThawing; - (tokensThawed, tokensThawing, sharesThawing) = _fulfillThawRequests( - _serviceProvider, - _verifier, - msg.sender, - tokensThawing, - sharesThawing, - _nThawRequests, - pool.thawingNonce - ); + + FulfillThawRequestsParams memory params = FulfillThawRequestsParams({ + requestType: _requestType, + serviceProvider: _serviceProvider, + verifier: _verifier, + owner: msg.sender, + tokensThawing: tokensThawing, + sharesThawing: sharesThawing, + nThawRequests: _nThawRequests, + thawingNonce: pool.thawingNonce + }); + (tokensThawed, tokensThawing, sharesThawing) = _fulfillThawRequests(params); // The next subtraction should never revert becase: pool.tokens >= pool.tokensThawing and pool.tokensThawing >= tokensThawed // In the event the pool gets completely slashed tokensThawed will fulfil to 0. @@ -936,6 +1044,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * @return The ID of the thaw request */ function _createThawRequest( + ThawRequestType _requestType, address _serviceProvider, address _verifier, address _owner, @@ -943,18 +1052,23 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint64 _thawingUntil, uint256 _thawingNonce ) private returns (bytes32) { - LinkedList.List storage thawRequestList = _thawRequestLists[_serviceProvider][_verifier][_owner]; + require(_shares != 0, HorizonStakingInvalidZeroShares()); + LinkedList.List storage thawRequestList = _getThawRequestList( + _requestType, + _serviceProvider, + _verifier, + _owner + ); require(thawRequestList.count < MAX_THAW_REQUESTS, HorizonStakingTooManyThawRequests()); bytes32 thawRequestId = keccak256(abi.encodePacked(_serviceProvider, _verifier, _owner, thawRequestList.nonce)); - _thawRequests[thawRequestId] = ThawRequest({ - shares: _shares, - thawingUntil: _thawingUntil, - next: bytes32(0), - thawingNonce: _thawingNonce - }); + ThawRequest storage thawRequest = _getThawRequest(_requestType, thawRequestId); + thawRequest.shares = _shares; + thawRequest.thawingUntil = _thawingUntil; + thawRequest.next = bytes32(0); + thawRequest.thawingNonce = _thawingNonce; - if (thawRequestList.count != 0) _thawRequests[thawRequestList.tail].next = thawRequestId; + if (thawRequestList.count != 0) _getThawRequest(_requestType, thawRequestList.tail).next = thawRequestId; thawRequestList.addTail(thawRequestId); emit ThawRequestCreated(_serviceProvider, _verifier, _owner, _shares, _thawingUntil, thawRequestId); @@ -964,41 +1078,74 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { /** * @notice Traverses a thaw request list and fulfills expired thaw requests. * @dev Emits a {ThawRequestsFulfilled} event and a {ThawRequestFulfilled} event for each thaw request fulfilled. - * @param _serviceProvider The address of the service provider - * @param _verifier The address of the verifier - * @param _owner The address of the owner of the thaw request - * @param _tokensThawing The current amount of tokens already thawing - * @param _sharesThawing The current amount of shares already thawing - * @param _nThawRequests The number of thaw requests to fulfill. If set to 0, all thaw requests are fulfilled. - * @param _thawingNonce The current valid thawing nonce. Any thaw request with a different nonce is invalid and should be ignored. + * @param params The parameters for fulfilling thaw requests * @return The amount of thawed tokens * @return The amount of tokens still thawing * @return The amount of shares still thawing */ - function _fulfillThawRequests( - address _serviceProvider, - address _verifier, - address _owner, - uint256 _tokensThawing, - uint256 _sharesThawing, - uint256 _nThawRequests, - uint256 _thawingNonce - ) private returns (uint256, uint256, uint256) { - LinkedList.List storage thawRequestList = _thawRequestLists[_serviceProvider][_verifier][_owner]; + function _fulfillThawRequests(FulfillThawRequestsParams memory params) private returns (uint256, uint256, uint256) { + LinkedList.List storage thawRequestList = _getThawRequestList( + params.requestType, + params.serviceProvider, + params.verifier, + params.owner + ); require(thawRequestList.count > 0, HorizonStakingNothingThawing()); - uint256 tokensThawed = 0; + TraverseThawRequestsResults memory results = _traverseThawRequests(params, thawRequestList); + + emit ThawRequestsFulfilled( + params.serviceProvider, + params.verifier, + params.owner, + results.requestsFulfilled, + results.tokensThawed, + params.requestType + ); + + return (results.tokensThawed, results.tokensThawing, results.sharesThawing); + } + + /** + * @notice Traverses a thaw request list and fulfills expired thaw requests. + * @param params The parameters for fulfilling thaw requests + * @param thawRequestList The list of thaw requests to traverse + * @return The results of the traversal + */ + function _traverseThawRequests( + FulfillThawRequestsParams memory params, + LinkedList.List storage thawRequestList + ) private returns (TraverseThawRequestsResults memory) { + function(bytes32) view returns (bytes32) getNextItem = _getNextThawRequest(params.requestType); + function(bytes32) deleteItem = _getDeleteThawRequest(params.requestType); + + bytes memory acc = abi.encode( + params.requestType, + uint256(0), + params.tokensThawing, + params.sharesThawing, + params.thawingNonce + ); (uint256 thawRequestsFulfilled, bytes memory data) = thawRequestList.traverse( - _getNextThawRequest, + getNextItem, _fulfillThawRequest, - _deleteThawRequest, - abi.encode(tokensThawed, _tokensThawing, _sharesThawing, _thawingNonce), - _nThawRequests + deleteItem, + acc, + params.nThawRequests ); - (tokensThawed, _tokensThawing, _sharesThawing) = abi.decode(data, (uint256, uint256, uint256)); - emit ThawRequestsFulfilled(_serviceProvider, _verifier, _owner, thawRequestsFulfilled, tokensThawed); - return (tokensThawed, _tokensThawing, _sharesThawing); + (, uint256 tokensThawed, uint256 tokensThawing, uint256 sharesThawing) = abi.decode( + data, + (ThawRequestType, uint256, uint256, uint256) + ); + + return + TraverseThawRequestsResults({ + requestsFulfilled: thawRequestsFulfilled, + tokensThawed: tokensThawed, + tokensThawing: tokensThawing, + sharesThawing: sharesThawing + }); } /** @@ -1013,19 +1160,22 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * @return The updated accumulator data */ function _fulfillThawRequest(bytes32 _thawRequestId, bytes memory _acc) private returns (bool, bytes memory) { - ThawRequest storage thawRequest = _thawRequests[_thawRequestId]; + // decode + ( + ThawRequestType requestType, + uint256 tokensThawed, + uint256 tokensThawing, + uint256 sharesThawing, + uint256 thawingNonce + ) = abi.decode(_acc, (ThawRequestType, uint256, uint256, uint256, uint256)); + + ThawRequest storage thawRequest = _getThawRequest(requestType, _thawRequestId); // early exit if (thawRequest.thawingUntil > block.timestamp) { return (true, LinkedList.NULL_BYTES); } - // decode - (uint256 tokensThawed, uint256 tokensThawing, uint256 sharesThawing, uint256 thawingNonce) = abi.decode( - _acc, - (uint256, uint256, uint256, uint256) - ); - // process - only fulfill thaw requests for the current valid nonce uint256 tokens = 0; bool validThawRequest = thawRequest.thawingNonce == thawingNonce; @@ -1044,17 +1194,49 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { ); // encode - _acc = abi.encode(tokensThawed, tokensThawing, sharesThawing, thawingNonce); + _acc = abi.encode(requestType, tokensThawed, tokensThawing, sharesThawing, thawingNonce); return (false, _acc); } /** - * @notice Deletes a ThawRequest. - * @dev This function is used as a callback in the thaw requests linked list traversal. - * @param _thawRequestId The ID of the thaw request to delete + * @notice Determines the correct callback function for `deleteItem` based on the request type. + * @param _requestType The type of thaw request (Provision or Delegation). + * @return A function pointer to the appropriate `deleteItem` callback. + */ + function _getDeleteThawRequest(ThawRequestType _requestType) private pure returns (function(bytes32)) { + if (_requestType == ThawRequestType.Provision) { + return _deleteProvisionThawRequest; + } else if (_requestType == ThawRequestType.Delegation) { + return _deleteDelegationThawRequest; + } else if (_requestType == ThawRequestType.DelegationWithBeneficiary) { + return _deleteDelegationWithBeneficiaryThawRequest; + } else { + revert HorizonStakingInvalidThawRequestType(); + } + } + + /** + * @notice Deletes a thaw request for a provision. + * @param _thawRequestId The ID of the thaw request to delete. + */ + function _deleteProvisionThawRequest(bytes32 _thawRequestId) private { + delete _thawRequests[ThawRequestType.Provision][_thawRequestId]; + } + + /** + * @notice Deletes a thaw request for a delegation. + * @param _thawRequestId The ID of the thaw request to delete. + */ + function _deleteDelegationThawRequest(bytes32 _thawRequestId) private { + delete _thawRequests[ThawRequestType.Delegation][_thawRequestId]; + } + + /** + * @notice Deletes a thaw request for a delegation with a beneficiary. + * @param _thawRequestId The ID of the thaw request to delete. */ - function _deleteThawRequest(bytes32 _thawRequestId) private { - delete _thawRequests[_thawRequestId]; + function _deleteDelegationWithBeneficiaryThawRequest(bytes32 _thawRequestId) private { + delete _thawRequests[ThawRequestType.DelegationWithBeneficiary][_thawRequestId]; } /** diff --git a/packages/horizon/contracts/staking/HorizonStakingBase.sol b/packages/horizon/contracts/staking/HorizonStakingBase.sol index d29d3edec..5f2808255 100644 --- a/packages/horizon/contracts/staking/HorizonStakingBase.sol +++ b/packages/horizon/contracts/staking/HorizonStakingBase.sol @@ -173,48 +173,58 @@ abstract contract HorizonStakingBase is /** * @notice See {IHorizonStakingBase-getThawRequest}. */ - function getThawRequest(bytes32 thawRequestId) external view override returns (ThawRequest memory) { - return _thawRequests[thawRequestId]; + function getThawRequest( + ThawRequestType requestType, + bytes32 thawRequestId + ) external view override returns (ThawRequest memory) { + return _getThawRequest(requestType, thawRequestId); } /** * @notice See {IHorizonStakingBase-getThawRequestList}. */ function getThawRequestList( + ThawRequestType requestType, address serviceProvider, address verifier, address owner ) external view override returns (LinkedList.List memory) { - return _thawRequestLists[serviceProvider][verifier][owner]; + return _getThawRequestList(requestType, serviceProvider, verifier, owner); } /** * @notice See {IHorizonStakingBase-getThawedTokens}. */ function getThawedTokens( + ThawRequestType requestType, address serviceProvider, address verifier, address owner ) external view override returns (uint256) { - LinkedList.List storage thawRequestList = _thawRequestLists[serviceProvider][verifier][owner]; + LinkedList.List storage thawRequestList = _getThawRequestList(requestType, serviceProvider, verifier, owner); if (thawRequestList.count == 0) { return 0; } - uint256 tokens = 0; + uint256 thawedTokens = 0; Provision storage prov = _provisions[serviceProvider][verifier]; + uint256 tokensThawing = prov.tokensThawing; + uint256 sharesThawing = prov.sharesThawing; bytes32 thawRequestId = thawRequestList.head; while (thawRequestId != bytes32(0)) { - ThawRequest storage thawRequest = _thawRequests[thawRequestId]; + ThawRequest storage thawRequest = _getThawRequest(requestType, thawRequestId); if (thawRequest.thawingUntil <= block.timestamp) { - tokens += (thawRequest.shares * prov.tokensThawing) / prov.sharesThawing; + uint256 tokens = (thawRequest.shares * tokensThawing) / sharesThawing; + tokensThawing = tokensThawing - tokens; + sharesThawing = sharesThawing - thawRequest.shares; + thawedTokens = thawedTokens + tokens; } else { break; } thawRequestId = thawRequest.next; } - return tokens; + return thawedTokens; } /** @@ -258,11 +268,11 @@ abstract contract HorizonStakingBase is * TODO: update the calculation after the transition period. */ function _getIdleStake(address _serviceProvider) internal view returns (uint256) { - return - _serviceProviders[_serviceProvider].tokensStaked - - _serviceProviders[_serviceProvider].tokensProvisioned - - _serviceProviders[_serviceProvider].__DEPRECATED_tokensAllocated - + uint256 tokensUsed = _serviceProviders[_serviceProvider].tokensProvisioned + + _serviceProviders[_serviceProvider].__DEPRECATED_tokensAllocated + _serviceProviders[_serviceProvider].__DEPRECATED_tokensLocked; + uint256 tokensStaked = _serviceProviders[_serviceProvider].tokensStaked; + return tokensStaked > tokensUsed ? tokensStaked - tokensUsed : 0; } /** @@ -289,11 +299,83 @@ abstract contract HorizonStakingBase is } /** - * @notice Gets the next thaw request after `_thawRequestId`. - * @dev This function is used as a callback in the thaw requests linked list traversal. + * @notice Determines the correct callback function for `getNextItem` based on the request type. + * @param _requestType The type of thaw request (Provision or Delegation). + * @return A function pointer to the appropriate `getNextItem` callback. */ - function _getNextThawRequest(bytes32 _thawRequestId) internal view returns (bytes32) { - return _thawRequests[_thawRequestId].next; + function _getNextThawRequest( + ThawRequestType _requestType + ) internal pure returns (function(bytes32) view returns (bytes32)) { + if (_requestType == ThawRequestType.Provision) { + return _getNextProvisionThawRequest; + } else if (_requestType == ThawRequestType.Delegation) { + return _getNextDelegationThawRequest; + } else if (_requestType == ThawRequestType.DelegationWithBeneficiary) { + return _getNextDelegationWithBeneficiaryThawRequest; + } else { + revert HorizonStakingInvalidThawRequestType(); + } + } + + /** + * @notice Retrieves the next thaw request for a provision. + * @param _thawRequestId The ID of the current thaw request. + * @return The ID of the next thaw request in the list. + */ + function _getNextProvisionThawRequest(bytes32 _thawRequestId) internal view returns (bytes32) { + return _thawRequests[ThawRequestType.Provision][_thawRequestId].next; + } + + /** + * @notice Retrieves the next thaw request for a delegation. + * @param _thawRequestId The ID of the current thaw request. + * @return The ID of the next thaw request in the list. + */ + function _getNextDelegationThawRequest(bytes32 _thawRequestId) internal view returns (bytes32) { + return _thawRequests[ThawRequestType.Delegation][_thawRequestId].next; + } + + /** + * @notice Retrieves the next thaw request for a delegation with a beneficiary. + * @param _thawRequestId The ID of the current thaw request. + * @return The ID of the next thaw request in the list. + */ + function _getNextDelegationWithBeneficiaryThawRequest(bytes32 _thawRequestId) internal view returns (bytes32) { + return _thawRequests[ThawRequestType.DelegationWithBeneficiary][_thawRequestId].next; + } + + /** + * @notice Retrieves the thaw request list for the given request type. + * @dev Uses the `ThawRequestType` to determine which mapping to access. + * Reverts if the request type is unknown. + * @param _requestType The type of thaw request (Provision or Delegation). + * @param _serviceProvider The address of the service provider. + * @param _verifier The address of the verifier. + * @param _owner The address of the owner of the thaw request. + * @return The linked list of thaw requests for the specified request type. + */ + function _getThawRequestList( + ThawRequestType _requestType, + address _serviceProvider, + address _verifier, + address _owner + ) internal view returns (LinkedList.List storage) { + return _thawRequestLists[_requestType][_serviceProvider][_verifier][_owner]; + } + + /** + * @notice Retrieves a specific thaw request for the given request type. + * @dev Uses the `ThawRequestType` to determine which mapping to access. + * Reverts if the request type is unknown. + * @param _requestType The type of thaw request (Provision or Delegation). + * @param _thawRequestId The unique ID of the thaw request. + * @return The thaw request data for the specified request type and ID. + */ + function _getThawRequest( + ThawRequestType _requestType, + bytes32 _thawRequestId + ) internal view returns (IHorizonStakingTypes.ThawRequest storage) { + return _thawRequests[_requestType][_thawRequestId]; } /** diff --git a/packages/horizon/contracts/staking/HorizonStakingExtension.sol b/packages/horizon/contracts/staking/HorizonStakingExtension.sol index bc878a4f5..3b42ebf1a 100644 --- a/packages/horizon/contracts/staking/HorizonStakingExtension.sol +++ b/packages/horizon/contracts/staking/HorizonStakingExtension.sol @@ -19,8 +19,8 @@ import { HorizonStakingBase } from "./HorizonStakingBase.sol"; * to the Horizon Staking contract. It allows indexers to close allocations and collect pending query fees, but it * does not allow for the creation of new allocations. This should allow indexers to migrate to a subgraph data service * without losing rewards or having service interruptions. - * @dev TODO: Once the transition period passes this contract can be removed. It's expected the transition period to - * last for a full allocation cycle (28 epochs). + * @dev TODO: Once the transition period passes this contract can be removed (note that an upgrade to the RewardsManager + * will also be required). It's expected the transition period to last for a full allocation cycle (28 epochs). * @custom:security-contact Please email security+contracts@thegraph.com if you find any * bugs. We may have an active bug bounty program. */ @@ -36,6 +36,14 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension _; } + /** + * @dev Check if the caller is the slasher. + */ + modifier onlySlasher() { + require(__DEPRECATED_slashers[msg.sender] == true, "!slasher"); + _; + } + /** * @dev The staking contract is upgradeable however we still use the constructor to set * a few immutable variables. @@ -255,6 +263,54 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension return __DEPRECATED_thawingPeriod; } + function legacySlash( + address indexer, + uint256 tokens, + uint256 reward, + address beneficiary + ) external override onlySlasher notPaused { + ServiceProviderInternal storage indexerStake = _serviceProviders[indexer]; + + // Only able to slash a non-zero number of tokens + require(tokens > 0, "!tokens"); + + // Rewards comes from tokens slashed balance + require(tokens >= reward, "rewards>slash"); + + // Cannot slash stake of an indexer without any or enough stake + require(indexerStake.tokensStaked > 0, "!stake"); + require(tokens <= indexerStake.tokensStaked, "slash>stake"); + + // Validate beneficiary of slashed tokens + require(beneficiary != address(0), "!beneficiary"); + + // Slashing more tokens than freely available (over allocation condition) + // Unlock locked tokens to avoid the indexer to withdraw them + uint256 tokensUsed = indexerStake.__DEPRECATED_tokensAllocated + indexerStake.__DEPRECATED_tokensLocked; + uint256 tokensAvailable = tokensUsed > indexerStake.tokensStaked ? 0 : indexerStake.tokensStaked - tokensUsed; + if (tokens > tokensAvailable && indexerStake.__DEPRECATED_tokensLocked > 0) { + uint256 tokensOverAllocated = tokens - tokensAvailable; + uint256 tokensToUnlock = MathUtils.min(tokensOverAllocated, indexerStake.__DEPRECATED_tokensLocked); + indexerStake.__DEPRECATED_tokensLocked = indexerStake.__DEPRECATED_tokensLocked - tokensToUnlock; + if (indexerStake.__DEPRECATED_tokensLocked == 0) { + indexerStake.__DEPRECATED_tokensLockedUntil = 0; + } + } + + // Remove tokens to slash from the stake + indexerStake.tokensStaked = indexerStake.tokensStaked - tokens; + + // -- Interactions -- + + // Set apart the reward for the beneficiary and burn remaining slashed stake + _graphToken().burnTokens(tokens - reward); + + // Give the beneficiary a reward for slashing + _graphToken().pushTokens(beneficiary, reward); + + emit StakeSlashed(indexer, tokens, reward, beneficiary); + } + /** * @notice (Legacy) Return true if operator is allowed for the service provider on the subgraph data service. * @dev TODO: Delete after the transition period @@ -349,8 +405,7 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension // Anyone is allowed to close ONLY under two concurrent conditions // - After maxAllocationEpochs passed // - When the allocation is for non-zero amount of tokens - bool isIndexerOrOperator = msg.sender == alloc.indexer || - isOperator(alloc.indexer, SUBGRAPH_DATA_SERVICE_ADDRESS); + bool isIndexerOrOperator = msg.sender == alloc.indexer || isOperator(msg.sender, alloc.indexer); if (epochs <= __DEPRECATED_maxAllocationEpochs || alloc.tokens == 0) { require(isIndexerOrOperator, "!auth"); } diff --git a/packages/horizon/contracts/staking/HorizonStakingStorage.sol b/packages/horizon/contracts/staking/HorizonStakingStorage.sol index ce2755468..a470ac363 100644 --- a/packages/horizon/contracts/staking/HorizonStakingStorage.sol +++ b/packages/horizon/contracts/staking/HorizonStakingStorage.sol @@ -149,11 +149,12 @@ abstract contract HorizonStakingV1Storage { /// @dev Thaw requests /// Details for each thawing operation in the staking contract (for both service providers and delegators). - mapping(bytes32 thawRequestId => IHorizonStakingTypes.ThawRequest thawRequest) internal _thawRequests; + mapping(IHorizonStakingTypes.ThawRequestType thawRequestType => mapping(bytes32 thawRequestId => IHorizonStakingTypes.ThawRequest thawRequest)) + internal _thawRequests; /// @dev Thaw request lists /// Metadata defining linked lists of thaw requests for each service provider or delegator (owner) - mapping(address serviceProvider => mapping(address verifier => mapping(address owner => LinkedList.List list))) + mapping(IHorizonStakingTypes.ThawRequestType thawRequestType => mapping(address serviceProvider => mapping(address verifier => mapping(address owner => LinkedList.List list)))) internal _thawRequestLists; /// @dev Operator allow list diff --git a/packages/horizon/ignition/configs/horizon.hardhat.json b/packages/horizon/ignition/configs/horizon.hardhat.json index ef16c8136..4ac74b188 100644 --- a/packages/horizon/ignition/configs/horizon.hardhat.json +++ b/packages/horizon/ignition/configs/horizon.hardhat.json @@ -36,7 +36,6 @@ "protocolPaymentCut": 10000 }, "PaymentsEscrow": { - "revokeCollectorThawingPeriod": 10000, "withdrawEscrowThawingPeriod": 10000 }, "TAPCollector": { diff --git a/packages/horizon/ignition/modules/core/PaymentsEscrow.ts b/packages/horizon/ignition/modules/core/PaymentsEscrow.ts index 1dbd07088..7c7948a7e 100644 --- a/packages/horizon/ignition/modules/core/PaymentsEscrow.ts +++ b/packages/horizon/ignition/modules/core/PaymentsEscrow.ts @@ -10,13 +10,12 @@ export default buildModule('PaymentsEscrow', (m) => { const { Controller, PeripheryRegistered } = m.useModule(GraphPeripheryModule) const { PaymentsEscrowProxyAdmin, PaymentsEscrowProxy, HorizonRegistered } = m.useModule(HorizonProxiesModule) - const revokeCollectorThawingPeriod = m.getParameter('revokeCollectorThawingPeriod') const withdrawEscrowThawingPeriod = m.getParameter('withdrawEscrowThawingPeriod') // Deploy PaymentsEscrow implementation const PaymentsEscrowImplementation = m.contract('PaymentsEscrow', PaymentsEscrowArtifact, - [Controller, revokeCollectorThawingPeriod, withdrawEscrowThawingPeriod], + [Controller, withdrawEscrowThawingPeriod], { after: [PeripheryRegistered, HorizonRegistered], }, diff --git a/packages/horizon/test/GraphBase.t.sol b/packages/horizon/test/GraphBase.t.sol index 7aa44d6f5..b2eef0dd9 100644 --- a/packages/horizon/test/GraphBase.t.sol +++ b/packages/horizon/test/GraphBase.t.sol @@ -71,7 +71,8 @@ abstract contract GraphBaseTest is IHorizonStakingTypes, Utils, Constants { operator: createUser("operator"), gateway: createUser("gateway"), verifier: createUser("verifier"), - delegator: createUser("delegator") + delegator: createUser("delegator"), + legacySlasher: createUser("legacySlasher") }); // Deploy protocol contracts @@ -116,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/data-service/DataService.t.sol b/packages/horizon/test/data-service/DataService.t.sol index c535c6dea..d18e49ba9 100644 --- a/packages/horizon/test/data-service/DataService.t.sol +++ b/packages/horizon/test/data-service/DataService.t.sol @@ -65,6 +65,25 @@ contract DataServiceTest is HorizonStakingSharedTest { assertEq(max, dataServiceOverride.PROVISION_TOKENS_MAX()); } + function test_ProvisionTokens_WhenCheckingAValidProvision_WithThawing(uint256 tokens, uint256 tokensThaw) external useIndexer { + dataService.setProvisionTokensRange(dataService.PROVISION_TOKENS_MIN(), dataService.PROVISION_TOKENS_MAX()); + tokens = bound(tokens, dataService.PROVISION_TOKENS_MIN(), dataService.PROVISION_TOKENS_MAX()); + tokensThaw = bound(tokensThaw, tokens - dataService.PROVISION_TOKENS_MIN() + 1, tokens); + + _createProvision(users.indexer, address(dataService), tokens, 0, 0); + staking.thaw(users.indexer, address(dataService), tokensThaw); + vm.expectRevert( + abi.encodeWithSelector( + ProvisionManager.ProvisionManagerInvalidValue.selector, + "tokens", + tokens - tokensThaw, + dataService.PROVISION_TOKENS_MIN(), + dataService.PROVISION_TOKENS_MAX() + ) + ); + dataService.checkProvisionTokens(users.indexer); + } + function test_ProvisionTokens_WhenCheckingAValidProvision(uint256 tokens) external useIndexer { dataService.setProvisionTokensRange(dataService.PROVISION_TOKENS_MIN(), dataService.PROVISION_TOKENS_MAX()); tokens = bound(tokens, dataService.PROVISION_TOKENS_MIN(), dataService.PROVISION_TOKENS_MAX()); diff --git a/packages/horizon/test/data-service/extensions/DataServicePausable.t.sol b/packages/horizon/test/data-service/extensions/DataServicePausable.t.sol index 111838fe4..46cded2eb 100644 --- a/packages/horizon/test/data-service/extensions/DataServicePausable.t.sol +++ b/packages/horizon/test/data-service/extensions/DataServicePausable.t.sol @@ -70,6 +70,23 @@ contract DataServicePausableTest is HorizonStakingSharedTest { _assert_setPauseGuardian(address(this), false); } + function test_SetPauseGuardian_RevertWhen_AlreadyPauseGuardian() external { + _assert_setPauseGuardian(address(this), true); + vm.expectRevert( + abi.encodeWithSignature("DataServicePausablePauseGuardianNoChange(address,bool)", address(this), true) + ); + dataService.setPauseGuardian(address(this), true); + } + + function test_SetPauseGuardian_RevertWhen_AlreadyNotPauseGuardian() external { + _assert_setPauseGuardian(address(this), true); + _assert_setPauseGuardian(address(this), false); + vm.expectRevert( + abi.encodeWithSignature("DataServicePausablePauseGuardianNoChange(address,bool)", address(this), false) + ); + dataService.setPauseGuardian(address(this), false); + } + function test_PausedProtectedFn_RevertWhen_TheProtocolIsPaused() external { _assert_setPauseGuardian(address(this), true); _assert_pause(); 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 72b795ee9..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); - delegationTokens = bound(delegationTokens, 1, 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 19028bde1..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,16 +110,19 @@ 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 - tokensDelegate = bound(tokensDelegate, 1, MAX_STAKING_TOKENS); + tokensDelegate = bound(tokensDelegate, MIN_DELEGATION, MAX_STAKING_TOKENS); vm.startPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, tokensDelegate, 0); @@ -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/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol index 1fe4ceba3..3be6633fb 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol @@ -8,6 +8,7 @@ import { IGraphPayments } from "../../../contracts/interfaces/IGraphPayments.sol import { IHorizonStakingBase } from "../../../contracts/interfaces/internal/IHorizonStakingBase.sol"; import { IHorizonStakingMain } from "../../../contracts/interfaces/internal/IHorizonStakingMain.sol"; import { IHorizonStakingExtension } from "../../../contracts/interfaces/internal/IHorizonStakingExtension.sol"; +import { IHorizonStakingTypes } from "../../../contracts/interfaces/internal/IHorizonStakingTypes.sol"; import { LinkedList } from "../../../contracts/libraries/LinkedList.sol"; import { MathUtils } from "../../../contracts/libraries/MathUtils.sol"; @@ -400,6 +401,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { // before Provision memory beforeProvision = staking.getProvision(serviceProvider, verifier); LinkedList.List memory beforeThawRequestList = staking.getThawRequestList( + IHorizonStakingTypes.ThawRequestType.Provision, serviceProvider, verifier, serviceProvider @@ -428,13 +430,9 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { // after Provision memory afterProvision = staking.getProvision(serviceProvider, verifier); - ThawRequest memory afterThawRequest = staking.getThawRequest(thawRequestId); - LinkedList.List memory afterThawRequestList = staking.getThawRequestList( - serviceProvider, - verifier, - serviceProvider - ); - ThawRequest memory afterPreviousTailThawRequest = staking.getThawRequest(beforeThawRequestList.tail); + ThawRequest memory afterThawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Provision, thawRequestId); + LinkedList.List memory afterThawRequestList = _getThawRequestList(IHorizonStakingTypes.ThawRequestType.Provision, serviceProvider, verifier, serviceProvider); + ThawRequest memory afterPreviousTailThawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Provision, beforeThawRequestList.tail); // assert assertEq(afterProvision.tokens, beforeProvision.tokens); @@ -472,18 +470,21 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { Provision memory beforeProvision = staking.getProvision(serviceProvider, verifier); ServiceProviderInternal memory beforeServiceProvider = _getStorage_ServiceProviderInternal(serviceProvider); LinkedList.List memory beforeThawRequestList = staking.getThawRequestList( + IHorizonStakingTypes.ThawRequestType.Provision, serviceProvider, verifier, serviceProvider ); - CalcValues_ThawRequestData memory calcValues = calcThawRequestData( - serviceProvider, - verifier, - serviceProvider, - nThawRequests, - false - ); + Params_CalcThawRequestData memory params = Params_CalcThawRequestData({ + thawRequestType: IHorizonStakingTypes.ThawRequestType.Provision, + serviceProvider: serviceProvider, + verifier: verifier, + owner: serviceProvider, + iterations: nThawRequests, + delegation: false + }); + CalcValues_ThawRequestData memory calcValues = calcThawRequestData(params); // deprovision for (uint i = 0; i < calcValues.thawRequestsFulfilledList.length; i++) { @@ -503,7 +504,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { verifier, serviceProvider, calcValues.thawRequestsFulfilledList.length, - calcValues.tokensThawed + calcValues.tokensThawed, + IHorizonStakingTypes.ThawRequestType.Provision ); vm.expectEmit(address(staking)); emit IHorizonStakingMain.TokensDeprovisioned(serviceProvider, verifier, calcValues.tokensThawed); @@ -513,6 +515,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { Provision memory afterProvision = staking.getProvision(serviceProvider, verifier); ServiceProviderInternal memory afterServiceProvider = _getStorage_ServiceProviderInternal(serviceProvider); LinkedList.List memory afterThawRequestList = staking.getThawRequestList( + IHorizonStakingTypes.ThawRequestType.Provision, serviceProvider, verifier, serviceProvider @@ -540,7 +543,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { beforeServiceProvider.__DEPRECATED_tokensLockedUntil ); for (uint i = 0; i < calcValues.thawRequestsFulfilledListIds.length; i++) { - ThawRequest memory thawRequest = staking.getThawRequest(calcValues.thawRequestsFulfilledListIds[i]); + ThawRequest memory thawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Provision, calcValues.thawRequestsFulfilledListIds[i]); assertEq(thawRequest.shares, 0); assertEq(thawRequest.thawingUntil, 0); assertEq(thawRequest.next, bytes32(0)); @@ -583,17 +586,19 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { provision: staking.getProvision(serviceProvider, verifier), provisionNewVerifier: staking.getProvision(serviceProvider, newVerifier), serviceProvider: _getStorage_ServiceProviderInternal(serviceProvider), - thawRequestList: staking.getThawRequestList(serviceProvider, verifier, serviceProvider) + thawRequestList: staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Provision, serviceProvider, verifier, serviceProvider) }); // calc - CalcValues_ThawRequestData memory calcValues = calcThawRequestData( - serviceProvider, - verifier, - serviceProvider, - nThawRequests, - false - ); + Params_CalcThawRequestData memory params = Params_CalcThawRequestData({ + thawRequestType: IHorizonStakingTypes.ThawRequestType.Provision, + serviceProvider: serviceProvider, + verifier: verifier, + owner: serviceProvider, + iterations: nThawRequests, + delegation: false + }); + CalcValues_ThawRequestData memory calcValues = calcThawRequestData(params); // reprovision for (uint i = 0; i < calcValues.thawRequestsFulfilledList.length; i++) { @@ -613,7 +618,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { verifier, serviceProvider, calcValues.thawRequestsFulfilledList.length, - calcValues.tokensThawed + calcValues.tokensThawed, + IHorizonStakingTypes.ThawRequestType.Provision ); vm.expectEmit(address(staking)); emit IHorizonStakingMain.TokensDeprovisioned(serviceProvider, verifier, calcValues.tokensThawed); @@ -626,6 +632,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { Provision memory afterProvisionNewVerifier = staking.getProvision(serviceProvider, newVerifier); ServiceProviderInternal memory afterServiceProvider = _getStorage_ServiceProviderInternal(serviceProvider); LinkedList.List memory afterThawRequestList = staking.getThawRequestList( + IHorizonStakingTypes.ThawRequestType.Provision, serviceProvider, verifier, serviceProvider @@ -680,7 +687,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { // assert: thaw request list old verifier for (uint i = 0; i < calcValues.thawRequestsFulfilledListIds.length; i++) { - ThawRequest memory thawRequest = staking.getThawRequest(calcValues.thawRequestsFulfilledListIds[i]); + ThawRequest memory thawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Provision, calcValues.thawRequestsFulfilledListIds[i]); assertEq(thawRequest.shares, 0); assertEq(thawRequest.thawingUntil, 0); assertEq(thawRequest.next, bytes32(0)); @@ -882,8 +889,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { uint256 deltaShares = afterDelegation.shares - beforeDelegation.shares; // assertions - assertEq(beforePool.tokens + tokens, afterPool.tokens); - assertEq(beforePool.shares + calcShares, afterPool.shares); + assertEq(beforePool.tokens + tokens, afterPool.tokens, "afterPool.tokens FAIL"); + assertEq(beforePool.shares + calcShares, afterPool.shares, "afterPool.shares FAIL"); assertEq(beforePool.tokensThawing, afterPool.tokensThawing); assertEq(beforePool.sharesThawing, afterPool.sharesThawing); assertEq(beforePool.thawingNonce, afterPool.thawingNonce); @@ -898,16 +905,30 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { function _undelegate(address serviceProvider, address verifier, uint256 shares) internal { (, address caller, ) = vm.readCallers(); - __undelegate(serviceProvider, verifier, shares, false, caller); + __undelegate(IHorizonStakingTypes.ThawRequestType.Delegation, serviceProvider, verifier, shares, false, caller); } - function _undelegate(address serviceProvider, address verifier, uint256 shares, address beneficiary) internal { - __undelegate(serviceProvider, verifier, shares, false, beneficiary); + function _undelegateWithBeneficiary(address serviceProvider, address verifier, uint256 shares, address beneficiary) internal { + __undelegate( + IHorizonStakingTypes.ThawRequestType.DelegationWithBeneficiary, + serviceProvider, + verifier, + shares, + false, + beneficiary + ); } function _undelegate(address serviceProvider, uint256 shares) internal { (, address caller, ) = vm.readCallers(); - __undelegate(serviceProvider, subgraphDataServiceLegacyAddress, shares, true, caller); + __undelegate( + IHorizonStakingTypes.ThawRequestType.Delegation, + serviceProvider, + subgraphDataServiceLegacyAddress, + shares, + true, + caller + ); } struct BeforeValues_Undelegate { @@ -923,14 +944,21 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { bytes32 thawRequestId; } - function __undelegate(address serviceProvider, address verifier, uint256 shares, bool legacy, address beneficiary) private { + function __undelegate( + IHorizonStakingTypes.ThawRequestType thawRequestType, + address serviceProvider, + address verifier, + uint256 shares, + bool legacy, + address beneficiary + ) private { (, address delegator, ) = vm.readCallers(); // before BeforeValues_Undelegate memory beforeValues; beforeValues.pool = _getStorage_DelegationPoolInternal(serviceProvider, verifier, legacy); beforeValues.delegation = _getStorage_Delegation(serviceProvider, verifier, delegator, legacy); - beforeValues.thawRequestList = staking.getThawRequestList(serviceProvider, verifier, delegator); + beforeValues.thawRequestList = staking.getThawRequestList(thawRequestType, serviceProvider, verifier, delegator); beforeValues.delegatedTokens = staking.getDelegatedTokensAvailable(serviceProvider, verifier); // calc @@ -962,8 +990,12 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { emit IHorizonStakingMain.TokensUndelegated(serviceProvider, verifier, delegator, calcValues.tokens); if (legacy) { staking.undelegate(serviceProvider, shares); + } else if (thawRequestType == IHorizonStakingTypes.ThawRequestType.Delegation) { + staking.undelegate(serviceProvider, verifier, shares); + } else if (thawRequestType == IHorizonStakingTypes.ThawRequestType.DelegationWithBeneficiary) { + staking.undelegateWithBeneficiary(serviceProvider, verifier, shares, beneficiary); } else { - staking.undelegate(serviceProvider, verifier, shares, beneficiary); + revert("Invalid thaw request type"); } // after @@ -978,8 +1010,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { beneficiary, legacy ); - LinkedList.List memory afterThawRequestList = staking.getThawRequestList(serviceProvider, verifier, beneficiary); - ThawRequest memory afterThawRequest = staking.getThawRequest(calcValues.thawRequestId); + LinkedList.List memory afterThawRequestList = staking.getThawRequestList(thawRequestType, serviceProvider, verifier, beneficiary); + ThawRequest memory afterThawRequest = staking.getThawRequest(thawRequestType, calcValues.thawRequestId); uint256 afterDelegatedTokens = staking.getDelegatedTokensAvailable(serviceProvider, verifier); // assertions @@ -1008,15 +1040,35 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { address verifier, uint256 nThawRequests ) internal { - __withdrawDelegated( - serviceProvider, - verifier, - address(0), - address(0), - 0, - nThawRequests, - false - ); + Params_WithdrawDelegated memory params = Params_WithdrawDelegated({ + thawRequestType: IHorizonStakingTypes.ThawRequestType.Delegation, + serviceProvider: serviceProvider, + verifier: verifier, + newServiceProvider: address(0), + newVerifier: address(0), + minSharesForNewProvider: 0, + nThawRequests: nThawRequests, + legacy: verifier == subgraphDataServiceLegacyAddress + }); + __withdrawDelegated(params); + } + + function _withdrawDelegatedWithBeneficiary( + address serviceProvider, + address verifier, + uint256 nThawRequests + ) internal { + Params_WithdrawDelegated memory params = Params_WithdrawDelegated({ + thawRequestType: IHorizonStakingTypes.ThawRequestType.DelegationWithBeneficiary, + serviceProvider: serviceProvider, + verifier: verifier, + newServiceProvider: address(0), + newVerifier: address(0), + minSharesForNewProvider: 0, + nThawRequests: nThawRequests, + legacy: false + }); + __withdrawDelegated(params); } function _redelegate( @@ -1027,19 +1079,17 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { uint256 minSharesForNewProvider, uint256 nThawRequests ) internal { - __withdrawDelegated( - serviceProvider, - verifier, - newServiceProvider, - newVerifier, - minSharesForNewProvider, - nThawRequests, - false - ); - } - - function _withdrawDelegated(address serviceProvider, address newServiceProvider) internal { - __withdrawDelegated(serviceProvider, subgraphDataServiceLegacyAddress, newServiceProvider, subgraphDataServiceLegacyAddress, 0, 0, true); + Params_WithdrawDelegated memory params = Params_WithdrawDelegated({ + thawRequestType: IHorizonStakingTypes.ThawRequestType.Delegation, + serviceProvider: serviceProvider, + verifier: verifier, + newServiceProvider: newServiceProvider, + newVerifier: newVerifier, + minSharesForNewProvider: minSharesForNewProvider, + nThawRequests: nThawRequests, + legacy: false + }); + __withdrawDelegated(params); } struct BeforeValues_WithdrawDelegated { @@ -1059,35 +1109,40 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { uint256 stakingBalance; } - function __withdrawDelegated( - address _serviceProvider, - address _verifier, - address _newServiceProvider, - address _newVerifier, - uint256 _minSharesForNewProvider, - uint256 _nThawRequests, - bool legacy - ) private { + struct Params_WithdrawDelegated { + IHorizonStakingTypes.ThawRequestType thawRequestType; + address serviceProvider; + address verifier; + address newServiceProvider; + address newVerifier; + uint256 minSharesForNewProvider; + uint256 nThawRequests; + bool legacy; + } + + function __withdrawDelegated(Params_WithdrawDelegated memory params) private { (, address msgSender, ) = vm.readCallers(); - bool reDelegate = _newServiceProvider != address(0) && _newVerifier != address(0); + bool reDelegate = params.newServiceProvider != address(0) && params.newVerifier != address(0); // before BeforeValues_WithdrawDelegated memory beforeValues; - beforeValues.pool = _getStorage_DelegationPoolInternal(_serviceProvider, _verifier, legacy); - beforeValues.newPool = _getStorage_DelegationPoolInternal(_newServiceProvider, _newVerifier, legacy); - beforeValues.newDelegation = _getStorage_Delegation(_newServiceProvider, _newVerifier, msgSender, legacy); - beforeValues.thawRequestList = staking.getThawRequestList(_serviceProvider, _verifier, msgSender); + beforeValues.pool = _getStorage_DelegationPoolInternal(params.serviceProvider, params.verifier, params.legacy); + beforeValues.newPool = _getStorage_DelegationPoolInternal(params.newServiceProvider, params.newVerifier, params.legacy); + beforeValues.newDelegation = _getStorage_Delegation(params.newServiceProvider, params.newVerifier, msgSender, params.legacy); + beforeValues.thawRequestList = staking.getThawRequestList(params.thawRequestType, params.serviceProvider, params.verifier, msgSender); beforeValues.senderBalance = token.balanceOf(msgSender); beforeValues.stakingBalance = token.balanceOf(address(staking)); - CalcValues_ThawRequestData memory calcValues = calcThawRequestData( - _serviceProvider, - _verifier, - msgSender, - _nThawRequests, - true - ); + Params_CalcThawRequestData memory paramsCalc = Params_CalcThawRequestData({ + thawRequestType: params.thawRequestType, + serviceProvider: params.serviceProvider, + verifier: params.verifier, + owner: msgSender, + iterations: params.nThawRequests, + delegation: true + }); + CalcValues_ThawRequestData memory calcValues = calcThawRequestData(paramsCalc); // withdrawDelegated for (uint i = 0; i < calcValues.thawRequestsFulfilledList.length; i++) { @@ -1103,18 +1158,19 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { } vm.expectEmit(address(staking)); emit IHorizonStakingMain.ThawRequestsFulfilled( - _serviceProvider, - _verifier, + params.serviceProvider, + params.verifier, msgSender, calcValues.thawRequestsFulfilledList.length, - calcValues.tokensThawed + calcValues.tokensThawed, + params.thawRequestType ); if (calcValues.tokensThawed != 0) { vm.expectEmit(); if (reDelegate) { emit IHorizonStakingMain.TokensDelegated( - _newServiceProvider, - _newVerifier, + params.newServiceProvider, + params.newVerifier, msgSender, calcValues.tokensThawed ); @@ -1125,32 +1181,34 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { vm.expectEmit(); emit IHorizonStakingMain.DelegatedTokensWithdrawn( - _serviceProvider, - _verifier, + params.serviceProvider, + params.verifier, msgSender, calcValues.tokensThawed ); - if (legacy) { - staking.withdrawDelegated(_serviceProvider, _newServiceProvider); - } else if (reDelegate) { + if (reDelegate) { staking.redelegate( - _serviceProvider, - _verifier, - _newServiceProvider, - _newVerifier, - _minSharesForNewProvider, - _nThawRequests + params.serviceProvider, + params.verifier, + params.newServiceProvider, + params.newVerifier, + params.minSharesForNewProvider, + params.nThawRequests ); + } else if (params.thawRequestType == IHorizonStakingTypes.ThawRequestType.Delegation) { + staking.withdrawDelegated(params.serviceProvider, params.verifier, params.nThawRequests); + } else if (params.thawRequestType == IHorizonStakingTypes.ThawRequestType.DelegationWithBeneficiary) { + staking.withdrawDelegatedWithBeneficiary(params.serviceProvider, params.verifier, params.nThawRequests); } else { - staking.withdrawDelegated(_serviceProvider, _verifier, _nThawRequests); + revert("Invalid thaw request type"); } // after AfterValues_WithdrawDelegated memory afterValues; - afterValues.pool = _getStorage_DelegationPoolInternal(_serviceProvider, _verifier, legacy); - afterValues.newPool = _getStorage_DelegationPoolInternal(_newServiceProvider, _newVerifier, legacy); - afterValues.newDelegation = _getStorage_Delegation(_newServiceProvider, _newVerifier, msgSender, legacy); - afterValues.thawRequestList = staking.getThawRequestList(_serviceProvider, _verifier, msgSender); + afterValues.pool = _getStorage_DelegationPoolInternal(params.serviceProvider, params.verifier, params.legacy); + afterValues.newPool = _getStorage_DelegationPoolInternal(params.newServiceProvider, params.newVerifier, params.legacy); + afterValues.newDelegation = _getStorage_Delegation(params.newServiceProvider, params.newVerifier, msgSender, params.legacy); + afterValues.thawRequestList = staking.getThawRequestList(params.thawRequestType, params.serviceProvider, params.verifier, msgSender); afterValues.senderBalance = token.balanceOf(msgSender); afterValues.stakingBalance = token.balanceOf(address(staking)); @@ -1162,7 +1220,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { assertEq(afterValues.pool.thawingNonce, beforeValues.pool.thawingNonce); for (uint i = 0; i < calcValues.thawRequestsFulfilledListIds.length; i++) { - ThawRequest memory thawRequest = staking.getThawRequest(calcValues.thawRequestsFulfilledListIds[i]); + ThawRequest memory thawRequest = staking.getThawRequest(params.thawRequestType, calcValues.thawRequestsFulfilledListIds[i]); assertEq(thawRequest.shares, 0); assertEq(thawRequest.thawingUntil, 0); assertEq(thawRequest.next, bytes32(0)); @@ -1210,7 +1268,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { afterValues.newDelegation.__DEPRECATED_tokensLockedUntil, beforeValues.newDelegation.__DEPRECATED_tokensLockedUntil ); - assertGe(deltaShares, _minSharesForNewProvider); + assertGe(deltaShares, params.minSharesForNewProvider); assertEq(calcShares, deltaShares); assertEq(afterValues.senderBalance - beforeValues.senderBalance, 0); assertEq(beforeValues.stakingBalance - afterValues.stakingBalance, 0); @@ -1296,7 +1354,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { function _setDelegationSlashingEnabled() internal { // setDelegationSlashingEnabled vm.expectEmit(); - emit IHorizonStakingMain.DelegationSlashingEnabled(true); + emit IHorizonStakingMain.DelegationSlashingEnabled(); staking.setDelegationSlashingEnabled(); // after @@ -1417,7 +1475,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { uint256 tokensSlashed = calcValues.providerTokensSlashed + (isDelegationSlashingEnabled ? calcValues.delegationTokensSlashed : 0); uint256 provisionThawingTokens = (before.provision.tokensThawing * - (1e18 - ((calcValues.providerTokensSlashed * 1e18) / before.provision.tokens))) / (1e18); + (1e18 - ((calcValues.providerTokensSlashed * 1e18 + before.provision.tokens - 1) / before.provision.tokens))) / (1e18); // assert assertEq(afterProvision.tokens + calcValues.providerTokensSlashed, before.provision.tokens); @@ -1435,7 +1493,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { (before.provision.sharesThawing != 0 && afterProvision.sharesThawing == 0) ? before.provision.thawingNonce + 1 : before.provision.thawingNonce); if (isDelegationSlashingEnabled) { uint256 poolThawingTokens = (before.pool.tokensThawing * - (1e18 - ((calcValues.delegationTokensSlashed * 1e18) / before.pool.tokens))) / (1e18); + (1e18 - ((calcValues.delegationTokensSlashed * 1e18 + before.pool.tokens - 1) / before.pool.tokens))) / (1e18); assertEq(afterPool.tokens + calcValues.delegationTokensSlashed, before.pool.tokens); assertEq(afterPool.shares, before.pool.shares); assertEq(afterPool.tokensThawing, poolThawingTokens); @@ -2207,34 +2265,49 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { uint256[] thawRequestsFulfilledListTokens; } - function calcThawRequestData( - address serviceProvider, - address verifier, - address owner, - uint256 iterations, - bool delegation - ) private view returns (CalcValues_ThawRequestData memory) { - LinkedList.List memory thawRequestList = staking.getThawRequestList(serviceProvider, verifier, owner); + struct ThawingData { + uint256 tokensThawed; + uint256 tokensThawing; + uint256 sharesThawing; + uint256 thawRequestsFulfilled; + } + + struct Params_CalcThawRequestData { + IHorizonStakingTypes.ThawRequestType thawRequestType; + address serviceProvider; + address verifier; + address owner; + uint256 iterations; + bool delegation; + } + + function calcThawRequestData(Params_CalcThawRequestData memory params) private view returns (CalcValues_ThawRequestData memory) { + LinkedList.List memory thawRequestList = _getThawRequestList( + params.thawRequestType, + params.serviceProvider, + params.verifier, + params.owner + ); if (thawRequestList.count == 0) { return CalcValues_ThawRequestData(0, 0, 0, new ThawRequest[](0), new bytes32[](0), new uint256[](0)); } - Provision memory prov = staking.getProvision(serviceProvider, verifier); - DelegationPool memory pool = staking.getDelegationPool(serviceProvider, verifier); + Provision memory prov = staking.getProvision(params.serviceProvider, params.verifier); + DelegationPool memory pool = staking.getDelegationPool(params.serviceProvider, params.verifier); uint256 tokensThawed = 0; - uint256 tokensThawing = delegation ? pool.tokensThawing : prov.tokensThawing; - uint256 sharesThawing = delegation ? pool.sharesThawing : prov.sharesThawing; + uint256 tokensThawing = params.delegation ? pool.tokensThawing : prov.tokensThawing; + uint256 sharesThawing = params.delegation ? pool.sharesThawing : prov.sharesThawing; uint256 thawRequestsFulfilled = 0; bytes32 thawRequestId = thawRequestList.head; - while (thawRequestId != bytes32(0) && (iterations == 0 || thawRequestsFulfilled < iterations)) { - ThawRequest memory thawRequest = staking.getThawRequest(thawRequestId); - bool isThawRequestValid = thawRequest.thawingNonce == (delegation ? pool.thawingNonce : prov.thawingNonce); + while (thawRequestId != bytes32(0) && (params.iterations == 0 || thawRequestsFulfilled < params.iterations)) { + ThawRequest memory thawRequest = _getThawRequest(params.thawRequestType, thawRequestId); + bool isThawRequestValid = thawRequest.thawingNonce == (params.delegation ? pool.thawingNonce : prov.thawingNonce); if (thawRequest.thawingUntil <= block.timestamp) { thawRequestsFulfilled++; if (isThawRequestValid) { - uint256 tokens = delegation + uint256 tokens = params.delegation ? (thawRequest.shares * pool.tokensThawing) / pool.sharesThawing : (thawRequest.shares * prov.tokensThawing) / prov.sharesThawing; tokensThawed += tokens; @@ -2248,24 +2321,28 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { } // we need to do a second pass because solidity doesnt allow dynamic arrays on memory - ThawRequest[] memory thawRequestsFulfilledList = new ThawRequest[](thawRequestsFulfilled); - bytes32[] memory thawRequestsFulfilledListIds = new bytes32[](thawRequestsFulfilled); - uint256[] memory thawRequestsFulfilledListTokens = new uint256[](thawRequestsFulfilled); + CalcValues_ThawRequestData memory thawRequestData; + thawRequestData.tokensThawed = tokensThawed; + thawRequestData.tokensThawing = tokensThawing; + thawRequestData.sharesThawing = sharesThawing; + thawRequestData.thawRequestsFulfilledList = new ThawRequest[](thawRequestsFulfilled); + thawRequestData.thawRequestsFulfilledListIds = new bytes32[](thawRequestsFulfilled); + thawRequestData.thawRequestsFulfilledListTokens = new uint256[](thawRequestsFulfilled); uint256 i = 0; thawRequestId = thawRequestList.head; - while (thawRequestId != bytes32(0) && (iterations == 0 || i < iterations)) { - ThawRequest memory thawRequest = staking.getThawRequest(thawRequestId); - bool isThawRequestValid = thawRequest.thawingNonce == (delegation ? pool.thawingNonce : prov.thawingNonce); + while (thawRequestId != bytes32(0) && (params.iterations == 0 || i < params.iterations)) { + ThawRequest memory thawRequest = _getThawRequest(params.thawRequestType, thawRequestId); + bool isThawRequestValid = thawRequest.thawingNonce == (params.delegation ? pool.thawingNonce : prov.thawingNonce); if (thawRequest.thawingUntil <= block.timestamp) { if (isThawRequestValid) { - thawRequestsFulfilledListTokens[i] = delegation + thawRequestData.thawRequestsFulfilledListTokens[i] = params.delegation ? (thawRequest.shares * pool.tokensThawing) / pool.sharesThawing : (thawRequest.shares * prov.tokensThawing) / prov.sharesThawing; } - thawRequestsFulfilledListIds[i] = thawRequestId; - thawRequestsFulfilledList[i] = staking.getThawRequest(thawRequestId); - thawRequestId = thawRequestsFulfilledList[i].next; + thawRequestData.thawRequestsFulfilledListIds[i] = thawRequestId; + thawRequestData.thawRequestsFulfilledList[i] = _getThawRequest(params.thawRequestType, thawRequestId); + thawRequestId = thawRequestData.thawRequestsFulfilledList[i].next; i++; } else { break; @@ -2273,18 +2350,34 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { thawRequestId = thawRequest.next; } - assertEq(thawRequestsFulfilled, thawRequestsFulfilledList.length); - assertEq(thawRequestsFulfilled, thawRequestsFulfilledListIds.length); - assertEq(thawRequestsFulfilled, thawRequestsFulfilledListTokens.length); - - return - CalcValues_ThawRequestData( - tokensThawed, - tokensThawing, - sharesThawing, - thawRequestsFulfilledList, - thawRequestsFulfilledListIds, - thawRequestsFulfilledListTokens - ); + assertEq(thawRequestsFulfilled, thawRequestData.thawRequestsFulfilledList.length); + assertEq(thawRequestsFulfilled, thawRequestData.thawRequestsFulfilledListIds.length); + assertEq(thawRequestsFulfilled, thawRequestData.thawRequestsFulfilledListTokens.length); + + return thawRequestData; + } + + function _getThawRequestList( + IHorizonStakingTypes.ThawRequestType thawRequestType, + address serviceProvider, + address verifier, + address owner + ) private view returns (LinkedList.List memory) { + return staking.getThawRequestList( + thawRequestType, + serviceProvider, + verifier, + owner + ); + } + + function _getThawRequest( + IHorizonStakingTypes.ThawRequestType thawRequestType, + bytes32 thawRequestId + ) private view returns (ThawRequest memory) { + return staking.getThawRequest( + thawRequestType, + thawRequestId + ); } } 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/staking/HorizonStaking.t.sol b/packages/horizon/test/staking/HorizonStaking.t.sol index b1b45d118..d57b1c1b8 100644 --- a/packages/horizon/test/staking/HorizonStaking.t.sol +++ b/packages/horizon/test/staking/HorizonStaking.t.sol @@ -31,7 +31,7 @@ contract HorizonStakingTest is HorizonStakingSharedTest { modifier useDelegation(uint256 delegationAmount) { address msgSender; (, msgSender, ) = vm.readCallers(); - vm.assume(delegationAmount > 1); + vm.assume(delegationAmount >= MIN_DELEGATION); vm.assume(delegationAmount <= MAX_STAKING_TOKENS); vm.startPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationAmount, 0); @@ -56,4 +56,30 @@ contract HorizonStakingTest is HorizonStakingSharedTest { resetPrank(msgSender); _; } + + modifier useUndelegate(uint256 shares) { + resetPrank(users.delegator); + + DelegationPoolInternalTest memory pool = _getStorage_DelegationPoolInternal( + users.indexer, + subgraphDataServiceAddress, + false + ); + DelegationInternal memory delegation = _getStorage_Delegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + false + ); + + shares = bound(shares, 1, delegation.shares); + uint256 tokens = (shares * (pool.tokens - pool.tokensThawing)) / pool.shares; + if (shares < delegation.shares) { + uint256 remainingTokens = (shares * (pool.tokens - pool.tokensThawing - tokens)) / pool.shares; + vm.assume(remainingTokens >= MIN_DELEGATION); + } + + _undelegate(users.indexer, subgraphDataServiceAddress, shares); + _; + } } diff --git a/packages/horizon/test/staking/allocation/close.t.sol b/packages/horizon/test/staking/allocation/close.t.sol index ce3cab273..6257e30ec 100644 --- a/packages/horizon/test/staking/allocation/close.t.sol +++ b/packages/horizon/test/staking/allocation/close.t.sol @@ -12,6 +12,18 @@ contract HorizonStakingCloseAllocationTest is HorizonStakingTest { bytes32 internal constant _poi = keccak256("poi"); + /* + * MODIFIERS + */ + + modifier useLegacyOperator() { + resetPrank(users.indexer); + _setOperator(subgraphDataServiceLegacyAddress, users.operator, true); + vm.startPrank(users.operator); + _; + vm.stopPrank(); + } + /* * TESTS */ @@ -26,6 +38,16 @@ contract HorizonStakingCloseAllocationTest is HorizonStakingTest { _closeAllocation(_allocationId, _poi); } + function testCloseAllocation_Operator(uint256 tokens) public useLegacyOperator() useAllocation(1 ether) { + tokens = bound(tokens, 1, MAX_STAKING_TOKENS); + _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0); + + // Skip 15 epochs + vm.roll(15); + + _closeAllocation(_allocationId, _poi); + } + function testCloseAllocation_WithBeneficiaryAddress(uint256 tokens) public useIndexer useAllocation(1 ether) { tokens = bound(tokens, 1, MAX_STAKING_TOKENS); _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0); diff --git a/packages/horizon/test/staking/delegation/addToPool.t.sol b/packages/horizon/test/staking/delegation/addToPool.t.sol index ff5b957ca..2bd73f28f 100644 --- a/packages/horizon/test/staking/delegation/addToPool.t.sol +++ b/packages/horizon/test/staking/delegation/addToPool.t.sol @@ -10,6 +10,7 @@ contract HorizonStakingDelegationAddToPoolTest is HorizonStakingTest { modifier useValidDelegationAmount(uint256 tokens) { vm.assume(tokens <= MAX_STAKING_TOKENS); + vm.assume(tokens >= MIN_DELEGATION); _; } @@ -93,7 +94,7 @@ contract HorizonStakingDelegationAddToPoolTest is HorizonStakingTest { uint256 recoverAmount ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { recoverAmount = bound(recoverAmount, 1, MAX_STAKING_TOKENS); - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); // create delegation pool resetPrank(users.delegator); @@ -116,7 +117,7 @@ contract HorizonStakingDelegationAddToPoolTest is HorizonStakingTest { uint256 recoverAmount ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { recoverAmount = bound(recoverAmount, 1, MAX_STAKING_TOKENS); - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); // create delegation pool resetPrank(users.delegator); diff --git a/packages/horizon/test/staking/delegation/delegate.t.sol b/packages/horizon/test/staking/delegation/delegate.t.sol index ab58e4bde..043453e10 100644 --- a/packages/horizon/test/staking/delegation/delegate.t.sol +++ b/packages/horizon/test/staking/delegation/delegate.t.sol @@ -59,9 +59,25 @@ contract HorizonStakingDelegateTest is HorizonStakingTest { staking.delegate(users.indexer, subgraphDataServiceAddress, 0, 0); } + function testDelegate_RevertWhen_UnderMinDelegation( + uint256 amount, + uint256 delegationAmount + ) public useIndexer useProvision(amount, 0, 0) { + delegationAmount = bound(delegationAmount, 1, MIN_DELEGATION - 1); + vm.startPrank(users.delegator); + token.approve(address(staking), delegationAmount); + bytes memory expectedError = abi.encodeWithSelector( + IHorizonStakingMain.HorizonStakingInsufficientDelegationTokens.selector, + delegationAmount, + MIN_DELEGATION + ); + vm.expectRevert(expectedError); + staking.delegate(users.indexer, subgraphDataServiceAddress, delegationAmount, 0); + } + function testDelegate_LegacySubgraphService(uint256 amount, uint256 delegationAmount) public useIndexer { amount = bound(amount, 1 ether, MAX_STAKING_TOKENS); - delegationAmount = bound(delegationAmount, 1, MAX_STAKING_TOKENS); + delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS); _createProvision(users.indexer, subgraphDataServiceLegacyAddress, amount, 0, 0); resetPrank(users.delegator); @@ -72,7 +88,7 @@ contract HorizonStakingDelegateTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); @@ -96,7 +112,7 @@ contract HorizonStakingDelegateTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { - delegationTokens = bound(delegationTokens, 2, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION * 2, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); @@ -126,7 +142,7 @@ contract HorizonStakingDelegateTest is HorizonStakingTest { uint256 recoverAmount ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { recoverAmount = bound(recoverAmount, 1, MAX_STAKING_TOKENS); - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); // create delegation pool resetPrank(users.delegator); diff --git a/packages/horizon/test/staking/delegation/legacyWithdraw.t.sol b/packages/horizon/test/staking/delegation/legacyWithdraw.t.sol new file mode 100644 index 000000000..c90fa1200 --- /dev/null +++ b/packages/horizon/test/staking/delegation/legacyWithdraw.t.sol @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import "forge-std/Test.sol"; + +import { IHorizonStakingMain } from "../../../contracts/interfaces/internal/IHorizonStakingMain.sol"; +import { IHorizonStakingTypes } from "../../../contracts/interfaces/internal/IHorizonStakingTypes.sol"; +import { IHorizonStakingExtension } from "../../../contracts/interfaces/internal/IHorizonStakingExtension.sol"; +import { LinkedList } from "../../../contracts/libraries/LinkedList.sol"; + +import { HorizonStakingTest } from "../HorizonStaking.t.sol"; + +contract HorizonStakingLegacyWithdrawDelegationTest is HorizonStakingTest { + /* + * MODIFIERS + */ + + modifier useDelegator() { + resetPrank(users.delegator); + _; + } + + /* + * HELPERS + */ + + function _setLegacyDelegation( + address _indexer, + address _delegator, + uint256 _shares, + uint256 __DEPRECATED_tokensLocked, + uint256 __DEPRECATED_tokensLockedUntil + ) public { + // Calculate the base storage slot for the serviceProvider in the mapping + bytes32 baseSlot = keccak256(abi.encode(_indexer, uint256(20))); + + // Calculate the slot for the delegator's DelegationInternal struct + bytes32 delegatorSlot = keccak256(abi.encode(_delegator, bytes32(uint256(baseSlot) + 4))); + + // Use vm.store to set each field of the struct + vm.store(address(staking), bytes32(uint256(delegatorSlot)), bytes32(_shares)); + vm.store(address(staking), bytes32(uint256(delegatorSlot) + 1), bytes32(__DEPRECATED_tokensLocked)); + vm.store(address(staking), bytes32(uint256(delegatorSlot) + 2), bytes32(__DEPRECATED_tokensLockedUntil)); + } + + /* + * ACTIONS + */ + + function _legacyWithdrawDelegated(address _indexer) internal { + (, address delegator, ) = vm.readCallers(); + IHorizonStakingTypes.DelegationPool memory pool = staking.getDelegationPool(_indexer, subgraphDataServiceLegacyAddress); + uint256 beforeStakingBalance = token.balanceOf(address(staking)); + uint256 beforeDelegatorBalance = token.balanceOf(users.delegator); + + vm.expectEmit(address(staking)); + emit IHorizonStakingMain.StakeDelegatedWithdrawn(_indexer, delegator, pool.tokens); + staking.withdrawDelegated(users.indexer, address(0)); + + uint256 afterStakingBalance = token.balanceOf(address(staking)); + uint256 afterDelegatorBalance = token.balanceOf(users.delegator); + + assertEq(afterStakingBalance, beforeStakingBalance - pool.tokens); + assertEq(afterDelegatorBalance - pool.tokens, beforeDelegatorBalance); + + DelegationInternal memory delegation = _getStorage_Delegation( + _indexer, + subgraphDataServiceLegacyAddress, + delegator, + true + ); + assertEq(delegation.shares, 0); + assertEq(delegation.__DEPRECATED_tokensLocked, 0); + assertEq(delegation.__DEPRECATED_tokensLockedUntil, 0); + } + + /* + * TESTS + */ + + function testWithdraw_Legacy(uint256 tokensLocked) public useDelegator { + vm.assume(tokensLocked > 0); + + _setStorage_DelegationPool(users.indexer, tokensLocked, 0, 0); + _setLegacyDelegation(users.indexer, users.delegator, 0, tokensLocked, 1); + token.transfer(address(staking), tokensLocked); + + _legacyWithdrawDelegated(users.indexer); + } + + function testWithdraw_Legacy_RevertWhen_NoTokens() public useDelegator { + _setStorage_DelegationPool(users.indexer, 0, 0, 0); + _setLegacyDelegation(users.indexer, users.delegator, 0, 0, 0); + + vm.expectRevert("!tokens"); + staking.withdrawDelegated(users.indexer, address(0)); + } +} diff --git a/packages/horizon/test/staking/delegation/redelegate.t.sol b/packages/horizon/test/staking/delegation/redelegate.t.sol index 605e6601f..6e30348c4 100644 --- a/packages/horizon/test/staking/delegation/redelegate.t.sol +++ b/packages/horizon/test/staking/delegation/redelegate.t.sol @@ -9,19 +9,6 @@ import { HorizonStakingTest } from "../HorizonStaking.t.sol"; contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { - /* - * MODIFIERS - */ - - modifier useUndelegate(uint256 shares) { - resetPrank(users.delegator); - DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false); - shares = bound(shares, 1, delegation.shares); - - _undelegate(users.indexer, subgraphDataServiceAddress, shares); - _; - } - /* * HELPERS */ diff --git a/packages/horizon/test/staking/delegation/undelegate.t.sol b/packages/horizon/test/staking/delegation/undelegate.t.sol index 4cad2e0c3..e23fdadb8 100644 --- a/packages/horizon/test/staking/delegation/undelegate.t.sol +++ b/packages/horizon/test/staking/delegation/undelegate.t.sol @@ -32,7 +32,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { uint256 undelegateSteps ) public useIndexer useProvision(amount, 0, 0) { undelegateSteps = bound(undelegateSteps, 1, 10); - delegationAmount = bound(delegationAmount, 10 wei, MAX_STAKING_TOKENS); + delegationAmount = bound(delegationAmount, MIN_DELEGATION * undelegateSteps, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationAmount, 0); @@ -44,9 +44,17 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { ); uint256 undelegateAmount = delegation.shares / undelegateSteps; - for (uint i = 0; i < undelegateSteps; i++) { + for (uint i = 0; i < undelegateSteps - 1; i++) { _undelegate(users.indexer, subgraphDataServiceAddress, undelegateAmount); } + + delegation = _getStorage_Delegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + false + ); + _undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares); } function testUndelegate_WithBeneficiary( @@ -55,16 +63,40 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { address beneficiary ) public useIndexer useProvision(amount, 0, 0) useDelegation(delegationAmount) { vm.assume(beneficiary != address(0)); + vm.assume(delegationAmount >= MIN_UNDELEGATION_WITH_BENEFICIARY); resetPrank(users.delegator); DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false); - _undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); + _undelegateWithBeneficiary(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); + } + + function testUndelegate_RevertWhen_InsuficientTokens( + uint256 amount, + uint256 delegationAmount, + uint256 undelegateAmount + ) public useIndexer useProvision(amount, 0, 0) useDelegation(delegationAmount) { + undelegateAmount = bound(undelegateAmount, 1, delegationAmount); + resetPrank(users.delegator); + DelegationInternal memory delegation = _getStorage_Delegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + false + ); + undelegateAmount = bound(undelegateAmount, delegation.shares - MIN_DELEGATION + 1, delegation.shares - 1); + bytes memory expectedError = abi.encodeWithSelector( + IHorizonStakingMain.HorizonStakingInsufficientTokens.selector, + delegation.shares - undelegateAmount, + MIN_DELEGATION + ); + vm.expectRevert(expectedError); + staking.undelegate(users.indexer, subgraphDataServiceAddress, undelegateAmount); } function testUndelegate_RevertWhen_TooManyUndelegations() public useIndexer useProvision(1000 ether, 0, 0) - useDelegation(1000 ether) + useDelegation(10000 ether) { resetPrank(users.delegator); @@ -112,7 +144,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { function testUndelegate_LegacySubgraphService(uint256 amount, uint256 delegationAmount) public useIndexer { amount = bound(amount, 1, MAX_STAKING_TOKENS); - delegationAmount = bound(delegationAmount, 1, MAX_STAKING_TOKENS); + delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS); _createProvision(users.indexer, subgraphDataServiceLegacyAddress, amount, 0, 0); resetPrank(users.delegator); @@ -131,7 +163,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); @@ -162,7 +194,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing { - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); // delegate resetPrank(users.delegator); @@ -232,6 +264,6 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false); bytes memory expectedError = abi.encodeWithSelector(IHorizonStakingMain.HorizonStakingInvalidBeneficiaryZeroAddress.selector); vm.expectRevert(expectedError); - staking.undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares, address(0)); + staking.undelegateWithBeneficiary(users.indexer, subgraphDataServiceAddress, delegation.shares, address(0)); } } diff --git a/packages/horizon/test/staking/delegation/withdraw.t.sol b/packages/horizon/test/staking/delegation/withdraw.t.sol index c9aa04dbc..ab286c279 100644 --- a/packages/horizon/test/staking/delegation/withdraw.t.sol +++ b/packages/horizon/test/staking/delegation/withdraw.t.sol @@ -4,28 +4,12 @@ pragma solidity 0.8.27; import "forge-std/Test.sol"; import { IHorizonStakingMain } from "../../../contracts/interfaces/internal/IHorizonStakingMain.sol"; +import { IHorizonStakingTypes } from "../../../contracts/interfaces/internal/IHorizonStakingTypes.sol"; import { LinkedList } from "../../../contracts/libraries/LinkedList.sol"; import { HorizonStakingTest } from "../HorizonStaking.t.sol"; contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { - /* - * MODIFIERS - */ - - modifier useUndelegate(uint256 shares) { - resetPrank(users.delegator); - DelegationInternal memory delegation = _getStorage_Delegation( - users.indexer, - subgraphDataServiceAddress, - users.delegator, - false - ); - shares = bound(shares, 1, delegation.shares); - - _undelegate(users.indexer, subgraphDataServiceAddress, shares); - _; - } /* * TESTS @@ -42,11 +26,12 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { useUndelegate(withdrawShares) { LinkedList.List memory thawingRequests = staking.getThawRequestList( + IHorizonStakingTypes.ThawRequestType.Delegation, users.indexer, subgraphDataServiceAddress, users.delegator ); - ThawRequest memory thawRequest = staking.getThawRequest(thawingRequests.tail); + ThawRequest memory thawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Delegation, thawingRequests.tail); skip(thawRequest.thawingUntil + 1); @@ -79,7 +64,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { } function testWithdrawDelegation_LegacySubgraphService(uint256 delegationAmount) public useIndexer { - delegationAmount = bound(delegationAmount, 1, MAX_STAKING_TOKENS); + delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS); _createProvision(users.indexer, subgraphDataServiceLegacyAddress, 10_000_000 ether, 0, MAX_THAWING_PERIOD); resetPrank(users.delegator); @@ -93,22 +78,23 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { _undelegate(users.indexer, delegation.shares); LinkedList.List memory thawingRequests = staking.getThawRequestList( + IHorizonStakingTypes.ThawRequestType.Delegation, users.indexer, subgraphDataServiceLegacyAddress, users.delegator ); - ThawRequest memory thawRequest = staking.getThawRequest(thawingRequests.tail); + ThawRequest memory thawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Delegation, thawingRequests.tail); skip(thawRequest.thawingUntil + 1); - _withdrawDelegated(users.indexer, address(0)); + _withdrawDelegated(users.indexer, subgraphDataServiceLegacyAddress, 0); } function testWithdrawDelegation_RevertWhen_InvalidPool( uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, 0, MAX_THAWING_PERIOD) useDelegationSlashing() { - delegationTokens = bound(delegationTokens, 2, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION * 2, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); @@ -180,22 +166,24 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { useDelegation(delegationAmount) { vm.assume(beneficiary != address(0)); + vm.assume(beneficiary != address(staking)); + vm.assume(delegationAmount >= MIN_UNDELEGATION_WITH_BENEFICIARY); // Skip beneficiary if balance will overflow vm.assume(token.balanceOf(beneficiary) < type(uint256).max - delegationAmount); // Delegator undelegates to beneficiary resetPrank(users.delegator); DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false); - _undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); + _undelegateWithBeneficiary(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); // Thawing period ends - LinkedList.List memory thawingRequests = staking.getThawRequestList(users.indexer, subgraphDataServiceAddress, beneficiary); - ThawRequest memory thawRequest = staking.getThawRequest(thawingRequests.tail); + LinkedList.List memory thawingRequests = staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Delegation, users.indexer, subgraphDataServiceAddress, beneficiary); + ThawRequest memory thawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Delegation, thawingRequests.tail); skip(thawRequest.thawingUntil + 1); // Beneficiary withdraws delegated tokens resetPrank(beneficiary); - _withdrawDelegated(users.indexer, subgraphDataServiceAddress, 1); + _withdrawDelegatedWithBeneficiary(users.indexer, subgraphDataServiceAddress, 1); } function testWithdrawDelegation_RevertWhen_PreviousOwnerAttemptsToWithdraw( @@ -209,15 +197,16 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { { vm.assume(beneficiary != address(0)); vm.assume(beneficiary != users.delegator); + vm.assume(delegationAmount >= MIN_UNDELEGATION_WITH_BENEFICIARY); // Delegator undelegates to beneficiary resetPrank(users.delegator); DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false); - _undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); + _undelegateWithBeneficiary(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); // Thawing period ends - LinkedList.List memory thawingRequests = staking.getThawRequestList(users.indexer, subgraphDataServiceAddress, users.delegator); - ThawRequest memory thawRequest = staking.getThawRequest(thawingRequests.tail); + LinkedList.List memory thawingRequests = staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Delegation, users.indexer, subgraphDataServiceAddress, users.delegator); + ThawRequest memory thawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Delegation, thawingRequests.tail); skip(thawRequest.thawingUntil + 1); // Delegator attempts to withdraw delegated tokens, should revert since beneficiary is the thaw request owner diff --git a/packages/horizon/test/staking/provision/deprovision.t.sol b/packages/horizon/test/staking/provision/deprovision.t.sol index ff022e1aa..4fa97da6c 100644 --- a/packages/horizon/test/staking/provision/deprovision.t.sol +++ b/packages/horizon/test/staking/provision/deprovision.t.sol @@ -17,7 +17,7 @@ contract HorizonStakingDeprovisionTest is HorizonStakingTest { uint256 thawCount, uint256 deprovisionCount ) public useIndexer useProvision(amount, maxVerifierCut, thawingPeriod) { - thawCount = bound(thawCount, 1, MAX_THAW_REQUESTS); + thawCount = bound(thawCount, 1, 100); deprovisionCount = bound(deprovisionCount, 0, thawCount); vm.assume(amount >= thawCount); // ensure the provision has at least 1 token for each thaw step uint256 individualThawAmount = amount / thawCount; @@ -37,7 +37,7 @@ contract HorizonStakingDeprovisionTest is HorizonStakingTest { uint64 thawingPeriod, uint256 thawCount ) public useIndexer useProvision(amount, maxVerifierCut, thawingPeriod) { - thawCount = bound(thawCount, 2, MAX_THAW_REQUESTS); + thawCount = bound(thawCount, 2, 100); vm.assume(amount >= thawCount); // ensure the provision has at least 1 token for each thaw step uint256 individualThawAmount = amount / thawCount; diff --git a/packages/horizon/test/staking/provision/thaw.t.sol b/packages/horizon/test/staking/provision/thaw.t.sol index c3b4d6903..eb58e8e86 100644 --- a/packages/horizon/test/staking/provision/thaw.t.sol +++ b/packages/horizon/test/staking/provision/thaw.t.sol @@ -26,7 +26,7 @@ contract HorizonStakingThawTest is HorizonStakingTest { uint64 thawingPeriod, uint256 thawCount ) public useIndexer useProvision(amount, 0, thawingPeriod) { - thawCount = bound(thawCount, 1, MAX_THAW_REQUESTS); + thawCount = bound(thawCount, 1, 100); vm.assume(amount >= thawCount); // ensure the provision has at least 1 token for each thaw step uint256 individualThawAmount = amount / thawCount; @@ -72,13 +72,8 @@ contract HorizonStakingThawTest is HorizonStakingTest { staking.thaw(users.indexer, subgraphDataServiceAddress, thawAmount); } - function testThaw_RevertWhen_OverMaxThawRequests( - uint256 amount, - uint64 thawingPeriod, - uint256 thawAmount - ) public useIndexer useProvision(amount, 0, thawingPeriod) { - vm.assume(amount >= MAX_THAW_REQUESTS + 1); - thawAmount = bound(thawAmount, 1, amount / (MAX_THAW_REQUESTS + 1)); + function testThaw_RevertWhen_OverMaxThawRequests() public useIndexer useProvision(10000 ether, 0, 0) { + uint256 thawAmount = 1 ether; for (uint256 i = 0; i < MAX_THAW_REQUESTS; i++) { _thaw(users.indexer, subgraphDataServiceAddress, thawAmount); @@ -139,4 +134,28 @@ contract HorizonStakingThawTest is HorizonStakingTest { resetPrank(users.indexer); _thaw(users.indexer, subgraphDataServiceAddress, thawAmount); } + + function testThaw_GetThawedTokens( + uint256 amount, + uint64 thawingPeriod, + uint256 thawSteps + ) public useIndexer useProvision(amount, 0, thawingPeriod) { + thawSteps = bound(thawSteps, 1, 10); + + uint256 thawAmount = amount / thawSteps; + vm.assume(thawAmount > 0); + for (uint256 i = 0; i < thawSteps; i++) { + _thaw(users.indexer, subgraphDataServiceAddress, thawAmount); + } + + skip(thawingPeriod + 1); + + uint256 thawedTokens = staking.getThawedTokens( + ThawRequestType.Provision, + users.indexer, + subgraphDataServiceAddress, + users.indexer + ); + vm.assertEq(thawedTokens, thawAmount * thawSteps); + } } diff --git a/packages/horizon/test/staking/slash/legacySlash.t.sol b/packages/horizon/test/staking/slash/legacySlash.t.sol new file mode 100644 index 000000000..f5595fdac --- /dev/null +++ b/packages/horizon/test/staking/slash/legacySlash.t.sol @@ -0,0 +1,208 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import "forge-std/Test.sol"; + +import { IHorizonStakingExtension } from "../../../contracts/interfaces/internal/IHorizonStakingExtension.sol"; + +import { HorizonStakingTest } from "../HorizonStaking.t.sol"; + +contract HorizonStakingLegacySlashTest is HorizonStakingTest { + + /* + * MODIFIERS + */ + + modifier useLegacySlasher(address slasher) { + bytes32 storageKey = keccak256(abi.encode(slasher, 18)); + vm.store(address(staking), storageKey, bytes32(uint256(1))); + _; + } + + /* + * HELPERS + */ + + function _setIndexer( + address _indexer, + uint256 _tokensStaked, + uint256 _tokensAllocated, + uint256 _tokensLocked, + uint256 _tokensLockedUntil + ) public { + bytes32 baseSlot = keccak256(abi.encode(_indexer, 14)); + + vm.store(address(staking), bytes32(uint256(baseSlot)), bytes32(_tokensStaked)); + vm.store(address(staking), bytes32(uint256(baseSlot) + 1), bytes32(_tokensAllocated)); + vm.store(address(staking), bytes32(uint256(baseSlot) + 2), bytes32(_tokensLocked)); + vm.store(address(staking), bytes32(uint256(baseSlot) + 3), bytes32(_tokensLockedUntil)); + } + + /* + * ACTIONS + */ + + function _legacySlash(address _indexer, uint256 _tokens, uint256 _rewards, address _beneficiary) internal { + // before + uint256 beforeStakingBalance = token.balanceOf(address(staking)); + uint256 beforeRewardsDestinationBalance = token.balanceOf(_beneficiary); + + // slash + vm.expectEmit(address(staking)); + emit IHorizonStakingExtension.StakeSlashed(_indexer, _tokens, _rewards, _beneficiary); + staking.slash(_indexer, _tokens, _rewards, _beneficiary); + + // after + uint256 afterStakingBalance = token.balanceOf(address(staking)); + uint256 afterRewardsDestinationBalance = token.balanceOf(_beneficiary); + + assertEq(beforeStakingBalance - _tokens, afterStakingBalance); + assertEq(beforeRewardsDestinationBalance, afterRewardsDestinationBalance - _rewards); + } + + /* + * TESTS + */ + + function testSlash_Legacy( + uint256 tokens, + uint256 slashTokens, + uint256 reward + ) public useIndexer useLegacySlasher(users.legacySlasher) { + vm.assume(tokens > 1); + slashTokens = bound(slashTokens, 1, tokens); + reward = bound(reward, 0, slashTokens); + + _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0); + + resetPrank(users.legacySlasher); + _legacySlash(users.indexer, slashTokens, reward, makeAddr("fisherman")); + } + + function testSlash_Legacy_UsingLockedTokens( + uint256 tokens, + uint256 slashTokens, + uint256 reward + ) public useIndexer useLegacySlasher(users.legacySlasher) { + vm.assume(tokens > 1); + slashTokens = bound(slashTokens, 1, tokens); + reward = bound(reward, 0, slashTokens); + + _setIndexer(users.indexer, tokens, 0, tokens, block.timestamp + 1); + // Send tokens manually to staking + token.transfer(address(staking), tokens); + + resetPrank(users.legacySlasher); + _legacySlash(users.indexer, slashTokens, reward, makeAddr("fisherman")); + } + + function testSlash_Legacy_UsingAllocatedTokens( + uint256 tokens, + uint256 slashTokens, + uint256 reward + ) public useIndexer useLegacySlasher(users.legacySlasher) { + vm.assume(tokens > 1); + slashTokens = bound(slashTokens, 1, tokens); + reward = bound(reward, 0, slashTokens); + + _setIndexer(users.indexer, tokens, 0, tokens, 0); + // Send tokens manually to staking + token.transfer(address(staking), tokens); + + resetPrank(users.legacySlasher); + staking.legacySlash(users.indexer, slashTokens, reward, makeAddr("fisherman")); + } + + function testSlash_Legacy_RevertWhen_CallerNotSlasher( + uint256 tokens, + uint256 slashTokens, + uint256 reward + ) public useIndexer { + vm.assume(tokens > 0); + _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0); + + vm.expectRevert("!slasher"); + staking.legacySlash(users.indexer, slashTokens, reward, makeAddr("fisherman")); + } + + function testSlash_Legacy_RevertWhen_RewardsOverSlashTokens( + uint256 tokens, + uint256 slashTokens, + uint256 reward + ) public useIndexer useLegacySlasher(users.legacySlasher) { + vm.assume(tokens > 0); + vm.assume(slashTokens > 0); + vm.assume(reward > slashTokens); + + _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0); + + resetPrank(users.legacySlasher); + vm.expectRevert("rewards>slash"); + staking.legacySlash(users.indexer, slashTokens, reward, makeAddr("fisherman")); + } + + function testSlash_Legacy_RevertWhen_NoStake( + uint256 slashTokens, + uint256 reward + ) public useLegacySlasher(users.legacySlasher) { + vm.assume(slashTokens > 0); + reward = bound(reward, 0, slashTokens); + + resetPrank(users.legacySlasher); + vm.expectRevert("!stake"); + staking.legacySlash(users.indexer, slashTokens, reward, makeAddr("fisherman")); + } + + function testSlash_Legacy_RevertWhen_ZeroTokens( + uint256 tokens + ) public useIndexer useLegacySlasher(users.legacySlasher) { + vm.assume(tokens > 0); + + _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0); + + resetPrank(users.legacySlasher); + vm.expectRevert("!tokens"); + staking.legacySlash(users.indexer, 0, 0, makeAddr("fisherman")); + } + + function testSlash_Legacy_RevertWhen_NoBeneficiary( + uint256 tokens, + uint256 slashTokens, + uint256 reward + ) public useIndexer useLegacySlasher(users.legacySlasher) { + vm.assume(tokens > 0); + slashTokens = bound(slashTokens, 1, tokens); + reward = bound(reward, 0, slashTokens); + + _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0); + + resetPrank(users.legacySlasher); + vm.expectRevert("!beneficiary"); + staking.legacySlash(users.indexer, slashTokens, reward, address(0)); + } + + function test_LegacySlash_WhenTokensAllocatedGreaterThanStake() + public + useIndexer + useLegacySlasher(users.legacySlasher) + { + // Setup indexer with: + // - tokensStaked = 1000 GRT + // - tokensAllocated = 800 GRT + // - tokensLocked = 300 GRT + // This means tokensUsed (1100 GRT) > tokensStaked (1000 GRT) + _setIndexer( + users.indexer, + 1000 ether, // tokensStaked + 800 ether, // tokensAllocated + 300 ether, // tokensLocked + 0 // tokensLockedUntil + ); + + // Send tokens manually to staking + token.transfer(address(staking), 1100 ether); + + resetPrank(users.legacySlasher); + _legacySlash(users.indexer, 1000 ether, 500 ether, makeAddr("fisherman")); + } +} diff --git a/packages/horizon/test/staking/slash/slash.t.sol b/packages/horizon/test/staking/slash/slash.t.sol index 7c7933419..fc5676b0f 100644 --- a/packages/horizon/test/staking/slash/slash.t.sol +++ b/packages/horizon/test/staking/slash/slash.t.sol @@ -54,7 +54,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest { uint256 delegationTokens ) public useIndexer useProvision(tokens, MAX_PPM, 0) { vm.assume(slashTokens > tokens); - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); verifierCutAmount = bound(verifierCutAmount, 0, MAX_PPM); vm.assume(verifierCutAmount <= tokens); @@ -71,7 +71,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest { uint256 verifierCutAmount, uint256 delegationTokens ) public useIndexer useProvision(tokens, MAX_PPM, 0) useDelegationSlashing() { - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); slashTokens = bound(slashTokens, tokens + 1, tokens + 1 + delegationTokens); verifierCutAmount = bound(verifierCutAmount, 0, tokens); @@ -110,7 +110,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, MAX_PPM, 0) useDelegationSlashing { - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); @@ -140,4 +140,46 @@ contract HorizonStakingSlashTest is HorizonStakingTest { vm.startPrank(subgraphDataServiceAddress); _slash(users.indexer, subgraphDataServiceAddress, tokens + delegationTokens, 0); } + + function testSlash_RoundDown_TokensThawing_Provision() public useIndexer { + uint256 tokens = 1 ether + 1; + _useProvision(subgraphDataServiceAddress, tokens, MAX_PPM, MAX_THAWING_PERIOD); + + _thaw(users.indexer, subgraphDataServiceAddress, tokens); + + resetPrank(subgraphDataServiceAddress); + _slash(users.indexer, subgraphDataServiceAddress, 1, 0); + + resetPrank(users.indexer); + Provision memory provision = staking.getProvision(users.indexer, subgraphDataServiceAddress); + assertEq(provision.tokens, tokens - 1); + // Tokens thawing should be rounded down + assertEq(provision.tokensThawing, tokens - 2); + } + + function testSlash_RoundDown_TokensThawing_Delegation( + uint256 tokens + ) public useIndexer useProvision(tokens, MAX_PPM, 0) useDelegationSlashing { + resetPrank(users.delegator); + uint256 delegationTokens = 1 ether + 1; + _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); + + DelegationInternal memory delegation = _getStorage_Delegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + false + ); + _undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares); + + resetPrank(subgraphDataServiceAddress); + // Slash 1 token from delegation + _slash(users.indexer, subgraphDataServiceAddress, tokens + 1, 0); + + resetPrank(users.delegator); + DelegationPool memory pool = staking.getDelegationPool(users.indexer, subgraphDataServiceAddress); + assertEq(pool.tokens, delegationTokens - 1); + // Tokens thawing should be rounded down + assertEq(pool.tokensThawing, delegationTokens - 2); + } } diff --git a/packages/horizon/test/utils/Constants.sol b/packages/horizon/test/utils/Constants.sol index cd5cc2bfb..e74e3b0d1 100644 --- a/packages/horizon/test/utils/Constants.sol +++ b/packages/horizon/test/utils/Constants.sol @@ -7,13 +7,14 @@ 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 - uint256 internal constant MAX_THAW_REQUESTS = 100; + uint256 internal constant MAX_THAW_REQUESTS = 1_000; uint64 internal constant MAX_THAWING_PERIOD = 28 days; uint32 internal constant THAWING_PERIOD_IN_BLOCKS = 300; + uint256 internal constant MIN_DELEGATION = 1e18; + uint256 internal constant MIN_UNDELEGATION_WITH_BENEFICIARY = 10e18; // Epoch manager uint256 internal constant EPOCH_LENGTH = 1; // Rewards manager diff --git a/packages/horizon/test/utils/Users.sol b/packages/horizon/test/utils/Users.sol index b26329f91..ecd3927ab 100644 --- a/packages/horizon/test/utils/Users.sol +++ b/packages/horizon/test/utils/Users.sol @@ -9,4 +9,5 @@ struct Users { address gateway; address verifier; address delegator; + address legacySlasher; } \ No newline at end of file diff --git a/packages/subgraph-service/contracts/DisputeManager.sol b/packages/subgraph-service/contracts/DisputeManager.sol index 58c93a756..2854282f4 100644 --- a/packages/subgraph-service/contracts/DisputeManager.sol +++ b/packages/subgraph-service/contracts/DisputeManager.sol @@ -31,11 +31,9 @@ import { AttestationManager } from "./utilities/AttestationManager.sol"; * Indexers use the derived private key for an allocation to sign attestations. * * Indexing Disputes: - * Indexers present a Proof of Indexing (POI) when they close allocations to prove - * they were indexing a subgraph. The Staking contract emits that proof with the format - * keccak256(indexer.address, POI). - * Any fisherman can dispute the validity of a POI by submitting a dispute to this contract - * along with a deposit. + * Indexers periodically present a Proof of Indexing (POI) to prove they are indexing a subgraph. + * The Subgraph Service contract emits that proof which includes the POI. Any fisherman can dispute the + * validity of a POI by submitting a dispute to this contract along with a deposit. * * Arbitration: * Disputes can only be accepted, rejected or drawn by the arbitrator role that can be delegated @@ -59,6 +57,9 @@ contract DisputeManager is // Maximum value for fisherman reward cut in PPM uint32 public constant MAX_FISHERMAN_REWARD_CUT = 500000; + // Minimum value for dispute deposit + uint256 public constant MIN_DISPUTE_DEPOSIT = 1e18; // 1 GRT + // -- Modifiers -- /** @@ -140,7 +141,7 @@ contract DisputeManager is */ function createIndexingDispute(address allocationId, bytes32 poi) external override returns (bytes32) { // Get funds from fisherman - _pullFishermanDeposit(); + _graphToken().pullTokens(msg.sender, disputeDeposit); // Create a dispute return _createIndexingDisputeWithAllocation(msg.sender, disputeDeposit, allocationId, poi); @@ -158,7 +159,7 @@ contract DisputeManager is */ function createQueryDispute(bytes calldata attestationData) external override returns (bytes32) { // Get funds from fisherman - _pullFishermanDeposit(); + _graphToken().pullTokens(msg.sender, disputeDeposit); // Create a dispute return @@ -174,10 +175,14 @@ contract DisputeManager is * @notice Create query disputes for two conflicting attestations. * A conflicting attestation is a proof presented by two different indexers * where for the same request on a subgraph the response is different. - * For this type of dispute the fisherman is not required to present a deposit - * as one of the attestation is considered to be right. * Two linked disputes will be created and if the arbitrator resolve one, the other - * one will be automatically resolved. + * one will be automatically resolved. Note that: + * - it's not possible to reject a conflicting query dispute as by definition at least one + * of the attestations is incorrect. + * - if both attestations are proven to be incorrect, the arbitrator can slash the indexer twice. + * Requirements: + * - fisherman must have previously approved this contract to pull `disputeDeposit` amount + * of tokens from their balance. * @param attestationData1 First attestation data submitted * @param attestationData2 Second attestation data submitted * @return DisputeId1, DisputeId2 @@ -205,10 +210,23 @@ contract DisputeManager is ) ); + // Get funds from fisherman + _graphToken().pullTokens(msg.sender, disputeDeposit); + // Create the disputes // The deposit is zero for conflicting attestations - bytes32 dId1 = _createQueryDisputeWithAttestation(fisherman, 0, attestation1, attestationData1); - bytes32 dId2 = _createQueryDisputeWithAttestation(fisherman, 0, attestation2, attestationData2); + bytes32 dId1 = _createQueryDisputeWithAttestation( + fisherman, + disputeDeposit / 2, + attestation1, + attestationData1 + ); + bytes32 dId2 = _createQueryDisputeWithAttestation( + fisherman, + disputeDeposit / 2, + attestation2, + attestationData2 + ); // Store the linked disputes to be resolved disputes[dId1].relatedDisputeId = dId2; @@ -225,6 +243,8 @@ contract DisputeManager is * This function will revert if the indexer is not slashable, whether because it does not have * any stake available or the slashing percentage is configured to be zero. In those cases * a dispute must be resolved using drawDispute or rejectDispute. + * This function will also revert if the dispute is in conflict, to accept a conflicting dispute + * use acceptDisputeConflict. * @dev Accept a dispute with Id `disputeId` * @param disputeId Id of the dispute to be accepted * @param tokensSlash Amount of tokens to slash from the indexer @@ -233,49 +253,71 @@ contract DisputeManager is bytes32 disputeId, uint256 tokensSlash ) external override onlyArbitrator onlyPendingDispute(disputeId) { + require(!_isDisputeInConflict(disputes[disputeId]), DisputeManagerDisputeInConflict(disputeId)); Dispute storage dispute = disputes[disputeId]; + _acceptDispute(disputeId, dispute, tokensSlash); + } - // store the dispute status - dispute.status = IDisputeManager.DisputeStatus.Accepted; - - // Slash - uint256 tokensToReward = _slashIndexer(dispute.indexer, tokensSlash, dispute.stakeSnapshot); - - // Give the fisherman their reward and their deposit back - _graphToken().pushTokens(dispute.fisherman, tokensToReward + dispute.deposit); + /** + * @notice The arbitrator accepts a conflicting dispute as being valid. + * This function will revert if the indexer is not slashable, whether because it does not have + * any stake available or the slashing percentage is configured to be zero. In those cases + * a dispute must be resolved using drawDispute. + * @param disputeId Id of the dispute to be accepted + * @param tokensSlash Amount of tokens to slash from the indexer for the first dispute + * @param acceptDisputeInConflict Accept the conflicting dispute. Otherwise it will be drawn automatically + * @param tokensSlashRelated Amount of tokens to slash from the indexer for the related dispute in case + * acceptDisputeInConflict is true, otherwise it will be ignored + */ + function acceptDisputeConflict( + bytes32 disputeId, + uint256 tokensSlash, + bool acceptDisputeInConflict, + uint256 tokensSlashRelated + ) external override onlyArbitrator onlyPendingDispute(disputeId) { + require(_isDisputeInConflict(disputes[disputeId]), DisputeManagerDisputeNotInConflict(disputeId)); + Dispute storage dispute = disputes[disputeId]; + _acceptDispute(disputeId, dispute, tokensSlash); - if (_isDisputeInConflict(dispute)) { - rejectDispute(dispute.relatedDisputeId); + if (acceptDisputeInConflict) { + _acceptDispute(dispute.relatedDisputeId, disputes[dispute.relatedDisputeId], tokensSlashRelated); + } else { + _drawDispute(dispute.relatedDisputeId, disputes[dispute.relatedDisputeId]); } + } - emit DisputeAccepted(disputeId, dispute.indexer, dispute.fisherman, dispute.deposit + tokensToReward); + /** + * @notice The arbitrator rejects a dispute as being invalid. + * Note that conflicting query disputes cannot be rejected. + * @dev Reject a dispute with Id `disputeId` + * @param disputeId Id of the dispute to be rejected + */ + function rejectDispute(bytes32 disputeId) external override onlyArbitrator onlyPendingDispute(disputeId) { + Dispute storage dispute = disputes[disputeId]; + require(!_isDisputeInConflict(dispute), DisputeManagerDisputeInConflict(disputeId)); + _rejectDispute(disputeId, dispute); } /** * @notice The arbitrator draws dispute. + * Note that drawing a conflicting query dispute should not be possible however it is allowed + * to give arbitrators greater flexibility when resolving disputes. * @dev Ignore a dispute with Id `disputeId` * @param disputeId Id of the dispute to be disregarded */ function drawDispute(bytes32 disputeId) external override onlyArbitrator onlyPendingDispute(disputeId) { Dispute storage dispute = disputes[disputeId]; + _drawDispute(disputeId, dispute); - // Return deposit to the fisherman if any - if (dispute.deposit > 0) { - _graphToken().pushTokens(dispute.fisherman, dispute.deposit); + if (_isDisputeInConflict(dispute)) { + _drawDispute(dispute.relatedDisputeId, disputes[dispute.relatedDisputeId]); } - - // resolve related dispute if any - _drawDisputeInConflict(dispute); - - // store dispute status - dispute.status = IDisputeManager.DisputeStatus.Drawn; - - emit DisputeDrawn(disputeId, dispute.indexer, dispute.fisherman, dispute.deposit); } /** * @notice Once the dispute period ends, if the dispute status remains Pending, * the fisherman can cancel the dispute and get back their initial deposit. + * Note that cancelling a conflicting query dispute will also cancel the related dispute. * @dev Cancel a dispute with Id `disputeId` * @param disputeId Id of the dispute to be cancelled */ @@ -284,19 +326,11 @@ contract DisputeManager is // Check if dispute period has finished require(dispute.createdAt + disputePeriod < block.timestamp, DisputeManagerDisputePeriodNotFinished()); + _cancelDispute(disputeId, dispute); - // Return deposit to the fisherman if any - if (dispute.deposit > 0) { - _graphToken().pushTokens(dispute.fisherman, dispute.deposit); + if (_isDisputeInConflict(dispute)) { + _cancelDispute(dispute.relatedDisputeId, disputes[dispute.relatedDisputeId]); } - - // resolve related dispute if any - _cancelDisputeInConflict(dispute); - - // store dispute status - dispute.status = IDisputeManager.DisputeStatus.Cancelled; - - emit DisputeCancelled(disputeId, dispute.indexer, dispute.fisherman, dispute.deposit); } /** @@ -385,7 +419,10 @@ contract DisputeManager is * @param indexer The indexer address */ function getStakeSnapshot(address indexer) external view override returns (uint256) { - IHorizonStaking.Provision memory provision = _graphStaking().getProvision(indexer, address(subgraphService)); + IHorizonStaking.Provision memory provision = _graphStaking().getProvision( + indexer, + address(_getSubgraphService()) + ); return _getStakeSnapshot(indexer, provision.tokens); } @@ -401,29 +438,6 @@ contract DisputeManager is return Attestation.areConflicting(attestation1, attestation2); } - /** - * @notice The arbitrator rejects a dispute as being invalid. - * @dev Reject a dispute with Id `disputeId` - * @param disputeId Id of the dispute to be rejected - */ - function rejectDispute(bytes32 disputeId) public override onlyArbitrator onlyPendingDispute(disputeId) { - Dispute storage dispute = disputes[disputeId]; - - // store dispute status - dispute.status = IDisputeManager.DisputeStatus.Rejected; - - // For conflicting disputes, the related dispute must be accepted - require( - !_isDisputeInConflict(dispute), - DisputeManagerMustAcceptRelatedDispute(disputeId, dispute.relatedDisputeId) - ); - - // Burn the fisherman's deposit - _graphToken().burnTokens(dispute.deposit); - - emit DisputeRejected(disputeId, dispute.indexer, dispute.fisherman, dispute.deposit); - } - /** * @notice Returns the indexer that signed an attestation. * @param attestation Attestation @@ -433,7 +447,7 @@ contract DisputeManager is // Get attestation signer. Indexers signs with the allocationId address allocationId = _recoverSigner(attestation); - Allocation.State memory alloc = subgraphService.getAllocation(allocationId); + Allocation.State memory alloc = _getSubgraphService().getAllocation(allocationId); require(alloc.indexer != address(0), DisputeManagerIndexerNotFound(allocationId)); require( alloc.subgraphDeploymentId == attestation.subgraphDeploymentId, @@ -472,7 +486,10 @@ contract DisputeManager is address indexer = getAttestationIndexer(_attestation); // The indexer is disputable - IHorizonStaking.Provision memory provision = _graphStaking().getProvision(indexer, address(subgraphService)); + IHorizonStaking.Provision memory provision = _graphStaking().getProvision( + indexer, + address(_getSubgraphService()) + ); require(provision.tokens != 0, DisputeManagerZeroTokens()); // Create a disputeId @@ -535,12 +552,13 @@ contract DisputeManager is require(!isDisputeCreated(disputeId), DisputeManagerDisputeAlreadyCreated(disputeId)); // Allocation must exist - Allocation.State memory alloc = subgraphService.getAllocation(_allocationId); + ISubgraphService subgraphService_ = _getSubgraphService(); + Allocation.State memory alloc = subgraphService_.getAllocation(_allocationId); address indexer = alloc.indexer; require(indexer != address(0), DisputeManagerIndexerNotFound(_allocationId)); // The indexer must be disputable - IHorizonStaking.Provision memory provision = _graphStaking().getProvision(indexer, address(subgraphService)); + IHorizonStaking.Provision memory provision = _graphStaking().getProvision(indexer, address(subgraphService_)); require(provision.tokens != 0, DisputeManagerZeroTokens()); // Store dispute @@ -561,42 +579,33 @@ contract DisputeManager is return disputeId; } - /** - * @notice Draw the conflicting dispute if there is any for the one passed to this function. - * @param _dispute Dispute - * @return True if resolved - */ - function _drawDisputeInConflict(Dispute memory _dispute) private returns (bool) { - if (_isDisputeInConflict(_dispute)) { - bytes32 relatedDisputeId = _dispute.relatedDisputeId; - Dispute storage relatedDispute = disputes[relatedDisputeId]; - relatedDispute.status = IDisputeManager.DisputeStatus.Drawn; - return true; - } - return false; + function _acceptDispute(bytes32 _disputeId, Dispute storage _dispute, uint256 _tokensSlashed) private { + uint256 tokensToReward = _slashIndexer(_dispute.indexer, _tokensSlashed, _dispute.stakeSnapshot); + _dispute.status = IDisputeManager.DisputeStatus.Accepted; + _graphToken().pushTokens(_dispute.fisherman, tokensToReward + _dispute.deposit); + + emit DisputeAccepted(_disputeId, _dispute.indexer, _dispute.fisherman, _dispute.deposit + tokensToReward); } - /** - * @notice Cancel the conflicting dispute if there is any for the one passed to this function. - * @param _dispute Dispute - * @return True if cancelled - */ - function _cancelDisputeInConflict(Dispute memory _dispute) private returns (bool) { - if (_isDisputeInConflict(_dispute)) { - bytes32 relatedDisputeId = _dispute.relatedDisputeId; - Dispute storage relatedDispute = disputes[relatedDisputeId]; - relatedDispute.status = IDisputeManager.DisputeStatus.Cancelled; - return true; - } - return false; + function _rejectDispute(bytes32 _disputeId, Dispute storage _dispute) private { + _dispute.status = IDisputeManager.DisputeStatus.Rejected; + _graphToken().burnTokens(_dispute.deposit); + + emit DisputeRejected(_disputeId, _dispute.indexer, _dispute.fisherman, _dispute.deposit); } - /** - * @notice Pull `disputeDeposit` from fisherman account. - */ - function _pullFishermanDeposit() private { - // Transfer tokens to deposit from fisherman to this contract - _graphToken().pullTokens(msg.sender, disputeDeposit); + function _drawDispute(bytes32 _disputeId, Dispute storage _dispute) private { + _dispute.status = IDisputeManager.DisputeStatus.Drawn; + _graphToken().pushTokens(_dispute.fisherman, _dispute.deposit); + + emit DisputeDrawn(_disputeId, _dispute.indexer, _dispute.fisherman, _dispute.deposit); + } + + function _cancelDispute(bytes32 _disputeId, Dispute storage _dispute) private { + _dispute.status = IDisputeManager.DisputeStatus.Cancelled; + _graphToken().pushTokens(_dispute.fisherman, _dispute.deposit); + + emit DisputeCancelled(_disputeId, _dispute.indexer, _dispute.fisherman, _dispute.deposit); } /** @@ -611,8 +620,10 @@ contract DisputeManager is uint256 _tokensSlash, uint256 _tokensStakeSnapshot ) private returns (uint256) { + ISubgraphService subgraphService_ = _getSubgraphService(); + // Get slashable amount for indexer - IHorizonStaking.Provision memory provision = _graphStaking().getProvision(_indexer, address(subgraphService)); + IHorizonStaking.Provision memory provision = _graphStaking().getProvision(_indexer, address(subgraphService_)); // Ensure slash amount is within the cap uint256 maxTokensSlash = _tokensStakeSnapshot.mulPPM(maxSlashingCut); @@ -626,7 +637,7 @@ contract DisputeManager is uint256 maxRewardableTokens = Math.min(_tokensSlash, provision.tokens); uint256 tokensRewards = uint256(fishermanRewardCut).mulPPM(maxRewardableTokens); - subgraphService.slash(_indexer, abi.encode(_tokensSlash, tokensRewards)); + subgraphService_.slash(_indexer, abi.encode(_tokensSlash, tokensRewards)); return tokensRewards; } @@ -658,7 +669,7 @@ contract DisputeManager is * @param _disputeDeposit The dispute deposit in Graph Tokens */ function _setDisputeDeposit(uint256 _disputeDeposit) private { - require(_disputeDeposit != 0, DisputeManagerInvalidDisputeDeposit(_disputeDeposit)); + require(_disputeDeposit >= MIN_DISPUTE_DEPOSIT, DisputeManagerInvalidDisputeDeposit(_disputeDeposit)); disputeDeposit = _disputeDeposit; emit DisputeDepositSet(_disputeDeposit); } @@ -699,15 +710,23 @@ contract DisputeManager is emit SubgraphServiceSet(_subgraphService); } + /** + * @notice Get the address of the subgraph service + * @dev Will revert if the subgraph service is not set + * @return The subgraph service address + */ + function _getSubgraphService() private view returns (ISubgraphService) { + require(address(subgraphService) != address(0), DisputeManagerSubgraphServiceNotSet()); + return subgraphService; + } + /** * @notice Returns whether the dispute is for a conflicting attestation or not. * @param _dispute Dispute * @return True conflicting attestation dispute */ - function _isDisputeInConflict(Dispute memory _dispute) private view returns (bool) { - bytes32 relatedId = _dispute.relatedDisputeId; - // this is so the check returns false when rejecting the related dispute. - return relatedId != 0 && disputes[relatedId].status == IDisputeManager.DisputeStatus.Pending; + function _isDisputeInConflict(Dispute storage _dispute) private view returns (bool) { + return _dispute.relatedDisputeId != bytes32(0); } /** @@ -722,8 +741,9 @@ contract DisputeManager is * @return Total stake snapshot */ function _getStakeSnapshot(address _indexer, uint256 _indexerStake) private view returns (uint256) { - uint256 delegatorsStake = _graphStaking().getDelegationPool(_indexer, address(subgraphService)).tokens; - uint256 delegatorsStakeMax = _indexerStake * uint256(subgraphService.getDelegationRatio()); + ISubgraphService subgraphService_ = _getSubgraphService(); + uint256 delegatorsStake = _graphStaking().getDelegationPool(_indexer, address(subgraphService_)).tokens; + uint256 delegatorsStakeMax = _indexerStake * uint256(subgraphService_.getDelegationRatio()); uint256 stakeSnapshot = _indexerStake + MathUtils.min(delegatorsStake, delegatorsStakeMax); return stakeSnapshot; } diff --git a/packages/subgraph-service/contracts/SubgraphService.sol b/packages/subgraph-service/contracts/SubgraphService.sol index 5f45c0f96..a4c6596e8 100644 --- a/packages/subgraph-service/contracts/SubgraphService.sol +++ b/packages/subgraph-service/contracts/SubgraphService.sol @@ -313,11 +313,9 @@ contract SubgraphService is /** * @notice See {ISubgraphService.closeStaleAllocation} */ - function forceCloseAllocation(address allocationId) external override { + function closeStaleAllocation(address allocationId) external override { Allocation.State memory allocation = allocations.get(allocationId); - bool isStale = allocation.isStale(maxPOIStaleness); - bool isOverAllocated_ = _isOverAllocated(allocation.indexer, delegationRatio); - require(isStale || isOverAllocated_, SubgraphServiceCannotForceCloseAllocation(allocationId)); + require(allocation.isStale(maxPOIStaleness), SubgraphServiceCannotForceCloseAllocation(allocationId)); require(!allocation.isAltruistic(), SubgraphServiceAllocationIsAltruistic(allocationId)); _closeAllocation(allocationId); } @@ -425,6 +423,7 @@ contract SubgraphService is * @return subgraphDeploymentId Subgraph deployment id for the allocation * @return tokens Amount of allocated tokens * @return accRewardsPerAllocatedToken Rewards snapshot + * @return accRewardsPending Rewards pending to be minted due to allocation resize */ function getAllocationData( address allocationId @@ -555,13 +554,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/IDisputeManager.sol b/packages/subgraph-service/contracts/interfaces/IDisputeManager.sol index 95511ecc2..ee2e92ac2 100644 --- a/packages/subgraph-service/contracts/interfaces/IDisputeManager.sol +++ b/packages/subgraph-service/contracts/interfaces/IDisputeManager.sol @@ -169,6 +169,8 @@ interface IDisputeManager { error DisputeManagerDisputeNotPending(IDisputeManager.DisputeStatus status); error DisputeManagerDisputeAlreadyCreated(bytes32 disputeId); error DisputeManagerDisputePeriodNotFinished(); + error DisputeManagerDisputeInConflict(bytes32 disputeId); + error DisputeManagerDisputeNotInConflict(bytes32 disputeId); error DisputeManagerMustAcceptRelatedDispute(bytes32 disputeId, bytes32 relatedDisputeId); error DisputeManagerIndexerNotFound(address allocationId); error DisputeManagerNonMatchingSubgraphDeployment(bytes32 subgraphDeploymentId1, bytes32 subgraphDeploymentId2); @@ -180,6 +182,7 @@ interface IDisputeManager { bytes32 responseCID2, bytes32 subgraphDeploymentId2 ); + error DisputeManagerSubgraphServiceNotSet(); function setDisputePeriod(uint64 disputePeriod) external; @@ -204,6 +207,13 @@ interface IDisputeManager { function acceptDispute(bytes32 disputeId, uint256 tokensSlash) external; + function acceptDisputeConflict( + bytes32 disputeId, + uint256 tokensSlash, + bool acceptDisputeInConflict, + uint256 tokensSlashRelated + ) external; + function rejectDispute(bytes32 disputeId) external; function drawDispute(bytes32 disputeId) external; diff --git a/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol b/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol index 32ea9e8fb..7278a4a4f 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. @@ -127,22 +125,20 @@ interface ISubgraphService is IDataServiceFees { error SubgraphServiceInvalidZeroStakeToFeesRatio(); /** - * @notice Force close an allocation - * @dev This function can be permissionlessly called when the allocation is stale or - * if the indexer is over-allocated. This ensures that rewards for other allocations are - * not diluted by an inactive allocation, and that over-allocated indexers stop accumulating - * rewards with tokens they no longer have allocated. + * @notice Force close a stale allocation + * @dev This function can be permissionlessly called when the allocation is stale. This + * ensures that rewards for other allocations are not diluted by an inactive allocation. * * Requirements: * - Allocation must exist and be open - * - Allocation must be stale or indexer must be over-allocated + * - Allocation must be stale * - Allocation cannot be altruistic * * Emits a {AllocationClosed} event. * * @param allocationId The id of the allocation */ - function forceCloseAllocation(address allocationId) external; + function closeStaleAllocation(address allocationId) external; /** * @notice Change the amount of tokens in an allocation diff --git a/packages/subgraph-service/contracts/libraries/LegacyAllocation.sol b/packages/subgraph-service/contracts/libraries/LegacyAllocation.sol index c60783785..3f17c1cef 100644 --- a/packages/subgraph-service/contracts/libraries/LegacyAllocation.sol +++ b/packages/subgraph-service/contracts/libraries/LegacyAllocation.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.27; +import { IHorizonStaking } from "@graphprotocol/horizon/contracts/interfaces/IHorizonStaking.sol"; + /** * @title LegacyAllocation library * @notice A library to handle legacy Allocations. @@ -22,7 +24,7 @@ library LegacyAllocation { * @notice Thrown when attempting to migrate an allocation with an existing id * @param allocationId The allocation id */ - error LegacyAllocationExists(address allocationId); + error LegacyAllocationAlreadyExists(address allocationId); /** * @notice Thrown when trying to get a non-existent allocation @@ -30,12 +32,6 @@ library LegacyAllocation { */ error LegacyAllocationDoesNotExist(address allocationId); - /** - * @notice Thrown when trying to migrate an allocation that has already been migrated - * @param allocationId The allocation id - */ - error LegacyAllocationAlreadyMigrated(address allocationId); - /** * @notice Migrate a legacy allocation * @dev Requirements: @@ -52,7 +48,7 @@ library LegacyAllocation { address allocationId, bytes32 subgraphDeploymentId ) internal { - require(!self[allocationId].exists(), LegacyAllocationAlreadyMigrated(allocationId)); + require(!self[allocationId].exists(), LegacyAllocationAlreadyExists(allocationId)); State memory allocation = State({ indexer: indexer, subgraphDeploymentId: subgraphDeploymentId }); self[allocationId] = allocation; @@ -69,11 +65,19 @@ library LegacyAllocation { /** * @notice Revert if a legacy allocation exists + * @dev We first check the migrated mapping then the old staking contract. + * @dev TODO: after the transition period when all the allocations are migrated we can + * remove the call to the staking contract. * @param self The legacy allocation list mapping * @param allocationId The allocation id */ - function revertIfExists(mapping(address => State) storage self, address allocationId) internal view { - require(!self[allocationId].exists(), LegacyAllocationExists(allocationId)); + function revertIfExists( + mapping(address => State) storage self, + IHorizonStaking graphStaking, + address allocationId + ) internal view { + require(!self[allocationId].exists(), LegacyAllocationAlreadyExists(allocationId)); + require(!graphStaking.isAllocation(allocationId), LegacyAllocationAlreadyExists(allocationId)); } /** diff --git a/packages/subgraph-service/contracts/utilities/AllocationManager.sol b/packages/subgraph-service/contracts/utilities/AllocationManager.sol index 37e57828f..51ba74e7a 100644 --- a/packages/subgraph-service/contracts/utilities/AllocationManager.sol +++ b/packages/subgraph-service/contracts/utilities/AllocationManager.sol @@ -212,7 +212,7 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca // Ensure allocation id is not reused // need to check both subgraph service (on allocations.create()) and legacy allocations - legacyAllocations.revertIfExists(_allocationId); + legacyAllocations.revertIfExists(_graphStaking(), _allocationId); Allocation.State memory allocation = allocations.create( _indexer, _allocationId, 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/disputeManager/DisputeManager.t.sol b/packages/subgraph-service/test/disputeManager/DisputeManager.t.sol index 0d99aec0b..0349fe7da 100644 --- a/packages/subgraph-service/test/disputeManager/DisputeManager.t.sol +++ b/packages/subgraph-service/test/disputeManager/DisputeManager.t.sol @@ -3,8 +3,6 @@ pragma solidity 0.8.27; import "forge-std/Test.sol"; -import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; -import { TokenUtils } from "@graphprotocol/contracts/contracts/utils/TokenUtils.sol"; import { PPMMath } from "@graphprotocol/horizon/contracts/libraries/PPMMath.sol"; import { IDisputeManager } from "../../contracts/interfaces/IDisputeManager.sol"; import { Attestation } from "../../contracts/libraries/Attestation.sol"; @@ -26,7 +24,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { vm.stopPrank(); } - modifier useFisherman { + modifier useFisherman() { vm.startPrank(users.fisherman); _; vm.stopPrank(); @@ -64,6 +62,13 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { assertEq(disputeManager.disputeDeposit(), _disputeDeposit, "Dispute deposit should be set."); } + function _setSubgraphService(address _subgraphService) internal { + vm.expectEmit(address(disputeManager)); + emit IDisputeManager.SubgraphServiceSet(_subgraphService); + disputeManager.setSubgraphService(_subgraphService); + assertEq(address(disputeManager.subgraphService()), _subgraphService, "Subgraph service should be set."); + } + function _createIndexingDispute(address _allocationId, bytes32 _poi) internal returns (bytes32) { (, address fisherman, ) = vm.readCallers(); bytes32 expectedDisputeId = keccak256(abi.encodePacked(_allocationId, _poi)); @@ -88,7 +93,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { // Create the indexing dispute bytes32 _disputeId = disputeManager.createIndexingDispute(_allocationId, _poi); - + // Check that the dispute was created and that it has the correct ID assertTrue(disputeManager.isDisputeCreated(_disputeId), "Dispute should be created."); assertEq(expectedDisputeId, _disputeId, "Dispute ID should match"); @@ -99,20 +104,32 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { assertEq(dispute.fisherman, fisherman, "Fisherman should match"); assertEq(dispute.deposit, disputeDeposit, "Deposit should match"); assertEq(dispute.relatedDisputeId, bytes32(0), "Related dispute ID should be empty"); - assertEq(uint8(dispute.disputeType), uint8(IDisputeManager.DisputeType.IndexingDispute), "Dispute type should be indexing"); - assertEq(uint8(dispute.status), uint8(IDisputeManager.DisputeStatus.Pending), "Dispute status should be pending"); + assertEq( + uint8(dispute.disputeType), + uint8(IDisputeManager.DisputeType.IndexingDispute), + "Dispute type should be indexing" + ); + assertEq( + uint8(dispute.status), + uint8(IDisputeManager.DisputeStatus.Pending), + "Dispute status should be pending" + ); assertEq(dispute.createdAt, block.timestamp, "Created at should match"); assertEq(dispute.stakeSnapshot, stakeSnapshot, "Stake snapshot should match"); // Check that the fisherman was charged the dispute deposit uint256 afterFishermanBalance = token.balanceOf(fisherman); - assertEq(afterFishermanBalance, beforeFishermanBalance - disputeDeposit, "Fisherman should be charged the dispute deposit"); + assertEq( + afterFishermanBalance, + beforeFishermanBalance - disputeDeposit, + "Fisherman should be charged the dispute deposit" + ); return _disputeId; } function _createQueryDispute(bytes memory _attestationData) internal returns (bytes32) { - (, address fisherman,) = vm.readCallers(); + (, address fisherman, ) = vm.readCallers(); Attestation.State memory attestation = Attestation.parse(_attestationData); address indexer = disputeManager.getAttestationIndexer(attestation); bytes32 expectedDisputeId = keccak256( @@ -130,7 +147,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { // Approve the dispute deposit token.approve(address(disputeManager), disputeDeposit); - + vm.expectEmit(address(disputeManager)); emit IDisputeManager.QueryDisputeCreated( expectedDisputeId, @@ -141,9 +158,9 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { _attestationData, stakeSnapshot ); - + bytes32 _disputeID = disputeManager.createQueryDispute(_attestationData); - + // Check that the dispute was created and that it has the correct ID assertTrue(disputeManager.isDisputeCreated(_disputeID), "Dispute should be created."); assertEq(expectedDisputeId, _disputeID, "Dispute ID should match"); @@ -154,15 +171,27 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { assertEq(dispute.fisherman, fisherman, "Fisherman should match"); assertEq(dispute.deposit, disputeDeposit, "Deposit should match"); assertEq(dispute.relatedDisputeId, bytes32(0), "Related dispute ID should be empty"); - assertEq(uint8(dispute.disputeType), uint8(IDisputeManager.DisputeType.QueryDispute), "Dispute type should be query"); - assertEq(uint8(dispute.status), uint8(IDisputeManager.DisputeStatus.Pending), "Dispute status should be pending"); + assertEq( + uint8(dispute.disputeType), + uint8(IDisputeManager.DisputeType.QueryDispute), + "Dispute type should be query" + ); + assertEq( + uint8(dispute.status), + uint8(IDisputeManager.DisputeStatus.Pending), + "Dispute status should be pending" + ); assertEq(dispute.createdAt, block.timestamp, "Created at should match"); assertEq(dispute.stakeSnapshot, stakeSnapshot, "Stake snapshot should match"); // Check that the fisherman was charged the dispute deposit uint256 afterFishermanBalance = token.balanceOf(fisherman); - assertEq(afterFishermanBalance, beforeFishermanBalance - disputeDeposit, "Fisherman should be charged the dispute deposit"); - + assertEq( + afterFishermanBalance, + beforeFishermanBalance - disputeDeposit, + "Fisherman should be charged the dispute deposit" + ); + return _disputeID; } @@ -174,11 +203,12 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { uint256 stakeSnapshot1; uint256 stakeSnapshot2; } + function _createQueryDisputeConflict( bytes memory attestationData1, bytes memory attestationData2 ) internal returns (bytes32, bytes32) { - (, address fisherman,) = vm.readCallers(); + (, address fisherman, ) = vm.readCallers(); BeforeValues_CreateQueryDisputeConflict memory beforeValues; beforeValues.attestation1 = Attestation.parse(attestationData1); @@ -188,6 +218,11 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { beforeValues.stakeSnapshot1 = disputeManager.getStakeSnapshot(beforeValues.indexer1); beforeValues.stakeSnapshot2 = disputeManager.getStakeSnapshot(beforeValues.indexer2); + uint256 beforeFishermanBalance = token.balanceOf(fisherman); + + // Approve the dispute deposit + token.approve(address(disputeManager), disputeDeposit); + bytes32 expectedDisputeId1 = keccak256( abi.encodePacked( beforeValues.attestation1.requestCID, @@ -213,7 +248,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { expectedDisputeId1, beforeValues.indexer1, fisherman, - 0, + disputeDeposit / 2, beforeValues.attestation1.subgraphDeploymentId, attestationData1, beforeValues.stakeSnapshot1 @@ -223,13 +258,16 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { expectedDisputeId2, beforeValues.indexer2, fisherman, - 0, + disputeDeposit / 2, beforeValues.attestation2.subgraphDeploymentId, attestationData2, beforeValues.stakeSnapshot2 ); - (bytes32 _disputeId1, bytes32 _disputeId2) = disputeManager.createQueryDisputeConflict(attestationData1, attestationData2); + (bytes32 _disputeId1, bytes32 _disputeId2) = disputeManager.createQueryDisputeConflict( + attestationData1, + attestationData2 + ); // Check that the disputes were created and that they have the correct IDs assertTrue(disputeManager.isDisputeCreated(_disputeId1), "Dispute 1 should be created."); @@ -241,23 +279,47 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { IDisputeManager.Dispute memory dispute1 = _getDispute(_disputeId1); assertEq(dispute1.indexer, beforeValues.indexer1, "Indexer 1 should match"); assertEq(dispute1.fisherman, fisherman, "Fisherman 1 should match"); - assertEq(dispute1.deposit, 0, "Deposit 1 should match"); + assertEq(dispute1.deposit, disputeDeposit / 2, "Deposit 1 should match"); assertEq(dispute1.relatedDisputeId, _disputeId2, "Related dispute ID 1 should be the id of the other dispute"); - assertEq(uint8(dispute1.disputeType), uint8(IDisputeManager.DisputeType.QueryDispute), "Dispute type 1 should be query"); - assertEq(uint8(dispute1.status), uint8(IDisputeManager.DisputeStatus.Pending), "Dispute status 1 should be pending"); + assertEq( + uint8(dispute1.disputeType), + uint8(IDisputeManager.DisputeType.QueryDispute), + "Dispute type 1 should be query" + ); + assertEq( + uint8(dispute1.status), + uint8(IDisputeManager.DisputeStatus.Pending), + "Dispute status 1 should be pending" + ); assertEq(dispute1.createdAt, block.timestamp, "Created at 1 should match"); assertEq(dispute1.stakeSnapshot, beforeValues.stakeSnapshot1, "Stake snapshot 1 should match"); IDisputeManager.Dispute memory dispute2 = _getDispute(_disputeId2); assertEq(dispute2.indexer, beforeValues.indexer2, "Indexer 2 should match"); assertEq(dispute2.fisherman, fisherman, "Fisherman 2 should match"); - assertEq(dispute2.deposit, 0, "Deposit 2 should match"); + assertEq(dispute2.deposit, disputeDeposit / 2, "Deposit 2 should match"); assertEq(dispute2.relatedDisputeId, _disputeId1, "Related dispute ID 2 should be the id of the other dispute"); - assertEq(uint8(dispute2.disputeType), uint8(IDisputeManager.DisputeType.QueryDispute), "Dispute type 2 should be query"); - assertEq(uint8(dispute2.status), uint8(IDisputeManager.DisputeStatus.Pending), "Dispute status 2 should be pending"); + assertEq( + uint8(dispute2.disputeType), + uint8(IDisputeManager.DisputeType.QueryDispute), + "Dispute type 2 should be query" + ); + assertEq( + uint8(dispute2.status), + uint8(IDisputeManager.DisputeStatus.Pending), + "Dispute status 2 should be pending" + ); assertEq(dispute2.createdAt, block.timestamp, "Created at 2 should match"); assertEq(dispute2.stakeSnapshot, beforeValues.stakeSnapshot2, "Stake snapshot 2 should match"); + // Check that the fisherman was charged the dispute deposit + uint256 afterFishermanBalance = token.balanceOf(fisherman); + assertEq( + afterFishermanBalance, + beforeFishermanBalance - disputeDeposit, + "Fisherman should be charged the dispute deposit" + ); + return (_disputeId1, _disputeId2); } @@ -271,14 +333,25 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { uint256 fishermanReward = _tokensSlash.mulPPM(fishermanRewardPercentage); vm.expectEmit(address(disputeManager)); - emit IDisputeManager.DisputeAccepted(_disputeId, dispute.indexer, dispute.fisherman, dispute.deposit + fishermanReward); + emit IDisputeManager.DisputeAccepted( + _disputeId, + dispute.indexer, + dispute.fisherman, + dispute.deposit + fishermanReward + ); // Accept the dispute disputeManager.acceptDispute(_disputeId, _tokensSlash); // Check fisherman's got their reward and their deposit (if any) back - uint256 fishermanExpectedBalance = fishermanPreviousBalance + fishermanReward + disputeDeposit; - assertEq(token.balanceOf(fisherman), fishermanExpectedBalance, "Fisherman should get their reward and deposit back"); + uint256 fishermanExpectedBalance = fishermanPreviousBalance + + fishermanReward + + disputeDeposit; + assertEq( + token.balanceOf(fisherman), + fishermanExpectedBalance, + "Fisherman should get their reward and deposit back" + ); // Check indexer was slashed by the correct amount uint256 expectedIndexerTokensAvailable; @@ -287,21 +360,157 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { } else { expectedIndexerTokensAvailable = indexerTokensAvailable - _tokensSlash; } - assertEq(staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService)), expectedIndexerTokensAvailable, "Indexer should be slashed by the correct amount"); + assertEq( + staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService)), + expectedIndexerTokensAvailable, + "Indexer should be slashed by the correct amount" + ); // Check dispute status dispute = _getDispute(_disputeId); - assertEq(uint8(dispute.status), uint8(IDisputeManager.DisputeStatus.Accepted), "Dispute status should be accepted"); + assertEq( + uint8(dispute.status), + uint8(IDisputeManager.DisputeStatus.Accepted), + "Dispute status should be accepted" + ); + } - // If there's a related dispute, check that it was rejected - if (dispute.relatedDisputeId != bytes32(0)) { - IDisputeManager.Dispute memory relatedDispute = _getDispute(dispute.relatedDisputeId); - assertEq(uint8(relatedDispute.status), uint8(IDisputeManager.DisputeStatus.Rejected), "Related dispute status should be rejected"); + struct FishermanParams { + address fisherman; + uint256 previousBalance; + uint256 disputeDeposit; + uint256 relatedDisputeDeposit; + uint256 rewardPercentage; + uint256 rewardFirstDispute; + uint256 rewardRelatedDispute; + uint256 totalReward; + uint256 expectedBalance; + } + + function _acceptDisputeConflict(bytes32 _disputeId, uint256 _tokensSlash, bool _acceptRelatedDispute, uint256 _tokensRelatedSlash) internal { + IDisputeManager.Dispute memory dispute = _getDispute(_disputeId); + IDisputeManager.Dispute memory relatedDispute = _getDispute(dispute.relatedDisputeId); + uint256 indexerTokensAvailable = staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService)); + uint256 relatedIndexerTokensAvailable = staking.getProviderTokensAvailable(relatedDispute.indexer, address(subgraphService)); + + FishermanParams memory params; + params.fisherman = dispute.fisherman; + params.previousBalance = token.balanceOf(params.fisherman); + params.disputeDeposit = dispute.deposit; + params.relatedDisputeDeposit = relatedDispute.deposit; + params.rewardPercentage = disputeManager.fishermanRewardCut(); + params.rewardFirstDispute = _tokensSlash.mulPPM(params.rewardPercentage); + params.rewardRelatedDispute = (_acceptRelatedDispute) ? _tokensRelatedSlash.mulPPM(params.rewardPercentage) : 0; + params.totalReward = params.rewardFirstDispute + params.rewardRelatedDispute; + + vm.expectEmit(address(disputeManager)); + emit IDisputeManager.DisputeAccepted( + _disputeId, + dispute.indexer, + params.fisherman, + params.disputeDeposit + params.rewardFirstDispute + ); + + if (_acceptRelatedDispute) { + emit IDisputeManager.DisputeAccepted( + dispute.relatedDisputeId, + relatedDispute.indexer, + relatedDispute.fisherman, + relatedDispute.deposit + params.rewardRelatedDispute + ); + } else { + emit IDisputeManager.DisputeDrawn( + dispute.relatedDisputeId, + relatedDispute.indexer, + relatedDispute.fisherman, + relatedDispute.deposit + ); } + + // Accept the dispute + disputeManager.acceptDisputeConflict(_disputeId, _tokensSlash, _acceptRelatedDispute, _tokensRelatedSlash); + + // Check fisherman's got their reward and their deposit back + params.expectedBalance = params.previousBalance + + params.totalReward + + params.disputeDeposit + + params.relatedDisputeDeposit; + assertEq( + token.balanceOf(params.fisherman), + params.expectedBalance, + "Fisherman should get their reward and deposit back" + ); + + // If both disputes are for the same indexer, check that the indexer was slashed by the correct amount + if (dispute.indexer == relatedDispute.indexer) { + uint256 tokensToSlash = (_acceptRelatedDispute) ? _tokensSlash + _tokensRelatedSlash : _tokensSlash; + uint256 expectedIndexerTokensAvailable; + if (tokensToSlash > indexerTokensAvailable) { + expectedIndexerTokensAvailable = 0; + } else { + expectedIndexerTokensAvailable = indexerTokensAvailable - tokensToSlash; + } + assertEq( + staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService)), + expectedIndexerTokensAvailable, + "Indexer should be slashed by the correct amount" + ); + } else { + // Check indexer for first dispute was slashed by the correct amount + uint256 expectedIndexerTokensAvailable; + uint256 tokensToSlash = (_acceptRelatedDispute) ? _tokensSlash : _tokensSlash; + if (tokensToSlash > indexerTokensAvailable) { + expectedIndexerTokensAvailable = 0; + } else { + expectedIndexerTokensAvailable = indexerTokensAvailable - tokensToSlash; + } + assertEq( + staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService)), + expectedIndexerTokensAvailable, + "Indexer should be slashed by the correct amount" + ); + + // Check indexer for related dispute was slashed by the correct amount if it was accepted + if (_acceptRelatedDispute) { + uint256 expectedRelatedIndexerTokensAvailable; + if (_tokensRelatedSlash > relatedIndexerTokensAvailable) { + expectedRelatedIndexerTokensAvailable = 0; + } else { + expectedRelatedIndexerTokensAvailable = relatedIndexerTokensAvailable - _tokensRelatedSlash; + } + assertEq( + staking.getProviderTokensAvailable(relatedDispute.indexer, address(subgraphService)), + expectedRelatedIndexerTokensAvailable, + "Indexer should be slashed by the correct amount" + ); + } + } + + + // Check dispute status + dispute = _getDispute(_disputeId); + assertEq( + uint8(dispute.status), + uint8(IDisputeManager.DisputeStatus.Accepted), + "Dispute status should be accepted" + ); + + // If there's a related dispute, check it + relatedDispute = _getDispute(dispute.relatedDisputeId); + assertEq( + uint8(relatedDispute.status), + _acceptRelatedDispute + ? uint8(IDisputeManager.DisputeStatus.Accepted) + : uint8(IDisputeManager.DisputeStatus.Drawn), + "Related dispute status should be drawn" + ); } function _drawDispute(bytes32 _disputeId) internal { IDisputeManager.Dispute memory dispute = _getDispute(_disputeId); + bool isConflictingDispute = dispute.relatedDisputeId != bytes32(0); + IDisputeManager.Dispute memory relatedDispute; + if (isConflictingDispute) relatedDispute = _getDispute(dispute.relatedDisputeId); address fisherman = dispute.fisherman; uint256 fishermanPreviousBalance = token.balanceOf(fisherman); uint256 indexerTokensAvailable = staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService)); @@ -309,15 +518,29 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { vm.expectEmit(address(disputeManager)); emit IDisputeManager.DisputeDrawn(_disputeId, dispute.indexer, dispute.fisherman, dispute.deposit); + if (isConflictingDispute) { + emit IDisputeManager.DisputeDrawn( + dispute.relatedDisputeId, + relatedDispute.indexer, + relatedDispute.fisherman, + relatedDispute.deposit + ); + } // Draw the dispute disputeManager.drawDispute(_disputeId); // Check that the fisherman got their deposit back - uint256 fishermanExpectedBalance = fishermanPreviousBalance + dispute.deposit; + uint256 fishermanExpectedBalance = fishermanPreviousBalance + + dispute.deposit + + (isConflictingDispute ? relatedDispute.deposit : 0); assertEq(token.balanceOf(fisherman), fishermanExpectedBalance, "Fisherman should receive their deposit back."); // Check that indexer was not slashed - assertEq(staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService)), indexerTokensAvailable, "Indexer should not be slashed"); + assertEq( + staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService)), + indexerTokensAvailable, + "Indexer should not be slashed" + ); // Check dispute status dispute = _getDispute(_disputeId); @@ -325,8 +548,12 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { // If there's a related dispute, check that it was drawn too if (dispute.relatedDisputeId != bytes32(0)) { - IDisputeManager.Dispute memory relatedDispute = _getDispute(dispute.relatedDisputeId); - assertEq(uint8(relatedDispute.status), uint8(IDisputeManager.DisputeStatus.Drawn), "Related dispute status should be drawn"); + relatedDispute = _getDispute(dispute.relatedDisputeId); + assertEq( + uint8(relatedDispute.status), + uint8(IDisputeManager.DisputeStatus.Drawn), + "Related dispute status should be drawn" + ); } } @@ -346,17 +573,28 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { assertEq(token.balanceOf(users.fisherman), fishermanPreviousBalance, "Fisherman should lose the deposit."); // Check that indexer was not slashed - assertEq(staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService)), indexerTokensAvailable, "Indexer should not be slashed"); + assertEq( + staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService)), + indexerTokensAvailable, + "Indexer should not be slashed" + ); // Check dispute status dispute = _getDispute(_disputeId); - assertEq(uint8(dispute.status), uint8(IDisputeManager.DisputeStatus.Rejected), "Dispute status should be rejected"); + assertEq( + uint8(dispute.status), + uint8(IDisputeManager.DisputeStatus.Rejected), + "Dispute status should be rejected" + ); // Checl related id is empty assertEq(dispute.relatedDisputeId, bytes32(0), "Related dispute ID should be empty"); } function _cancelDispute(bytes32 _disputeId) internal { IDisputeManager.Dispute memory dispute = _getDispute(_disputeId); + bool isDisputeInConflict = dispute.relatedDisputeId != bytes32(0); + IDisputeManager.Dispute memory relatedDispute; + if (isDisputeInConflict) relatedDispute = _getDispute(dispute.relatedDisputeId); address fisherman = dispute.fisherman; uint256 fishermanPreviousBalance = token.balanceOf(fisherman); uint256 disputePeriod = disputeManager.disputePeriod(); @@ -368,24 +606,50 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { vm.expectEmit(address(disputeManager)); emit IDisputeManager.DisputeCancelled(_disputeId, dispute.indexer, dispute.fisherman, dispute.deposit); + if (isDisputeInConflict) { + emit IDisputeManager.DisputeCancelled( + dispute.relatedDisputeId, + relatedDispute.indexer, + relatedDispute.fisherman, + relatedDispute.deposit + ); + } + // Cancel the dispute disputeManager.cancelDispute(_disputeId); // Check that the fisherman got their deposit back - uint256 fishermanExpectedBalance = fishermanPreviousBalance + dispute.deposit; - assertEq(token.balanceOf(users.fisherman), fishermanExpectedBalance, "Fisherman should receive their deposit back."); + uint256 fishermanExpectedBalance = fishermanPreviousBalance + + dispute.deposit + + (isDisputeInConflict ? relatedDispute.deposit : 0); + assertEq( + token.balanceOf(users.fisherman), + fishermanExpectedBalance, + "Fisherman should receive their deposit back." + ); // Check that indexer was not slashed - assertEq(staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService)), indexerTokensAvailable, "Indexer should not be slashed"); + assertEq( + staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService)), + indexerTokensAvailable, + "Indexer should not be slashed" + ); // Check dispute status dispute = _getDispute(_disputeId); - assertEq(uint8(dispute.status), uint8(IDisputeManager.DisputeStatus.Cancelled), "Dispute status should be cancelled"); + assertEq( + uint8(dispute.status), + uint8(IDisputeManager.DisputeStatus.Cancelled), + "Dispute status should be cancelled" + ); - // If there's a related dispute, check that it was cancelled too - if (dispute.relatedDisputeId != bytes32(0)) { - IDisputeManager.Dispute memory relatedDispute = _getDispute(dispute.relatedDisputeId); - assertEq(uint8(relatedDispute.status), uint8(IDisputeManager.DisputeStatus.Cancelled), "Related dispute status should be cancelled"); + if (isDisputeInConflict) { + relatedDispute = _getDispute(dispute.relatedDisputeId); + assertEq( + uint8(relatedDispute.status), + uint8(IDisputeManager.DisputeStatus.Cancelled), + "Related dispute status should be cancelled" + ); } } @@ -398,11 +662,12 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { bytes32 responseCID, bytes32 subgraphDeploymentId ) internal pure returns (Attestation.Receipt memory receipt) { - return Attestation.Receipt({ - requestCID: requestCID, - responseCID: responseCID, - subgraphDeploymentId: subgraphDeploymentId - }); + return + Attestation.Receipt({ + requestCID: requestCID, + responseCID: responseCID, + subgraphDeploymentId: subgraphDeploymentId + }); } function _createConflictingAttestations( @@ -446,15 +711,20 @@ contract DisputeManagerTest is SubgraphServiceSharedTest { uint256 createdAt, uint256 stakeSnapshot ) = disputeManager.disputes(_disputeId); - return IDisputeManager.Dispute({ - indexer: indexer, - fisherman: fisherman, - deposit: deposit, - relatedDisputeId: relatedDisputeId, - disputeType: disputeType, - status: status, - createdAt: createdAt, - stakeSnapshot: stakeSnapshot - }); + return + IDisputeManager.Dispute({ + indexer: indexer, + fisherman: fisherman, + deposit: deposit, + relatedDisputeId: relatedDisputeId, + disputeType: disputeType, + status: status, + createdAt: createdAt, + stakeSnapshot: stakeSnapshot + }); + } + + function _setStorage_SubgraphService(address _subgraphService) internal { + vm.store(address(disputeManager), bytes32(uint256(51)), bytes32(uint256(uint160(_subgraphService)))); } } diff --git a/packages/subgraph-service/test/disputeManager/disputes/indexing/accept.t.sol b/packages/subgraph-service/test/disputeManager/disputes/indexing/accept.t.sol index 49bee9e26..f1d1dc24f 100644 --- a/packages/subgraph-service/test/disputeManager/disputes/indexing/accept.t.sol +++ b/packages/subgraph-service/test/disputeManager/disputes/indexing/accept.t.sol @@ -27,6 +27,36 @@ contract DisputeManagerIndexingAcceptDisputeTest is DisputeManagerTest { _acceptDispute(disputeID, tokensSlash); } + function test_Indexing_Accept_Dispute_RevertWhen_SubgraphServiceNotSet( + uint256 tokens, + uint256 tokensSlash + ) public useIndexer useAllocation(tokens) { + tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens)); + + resetPrank(users.fisherman); + bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1")); + + resetPrank(users.arbitrator); + // clear subgraph service address from storage + _setStorage_SubgraphService(address(0)); + + vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerSubgraphServiceNotSet.selector)); + disputeManager.acceptDispute(disputeID, tokensSlash); + } + + function test_Indexing_Accept_Dispute_OptParam( + uint256 tokens, + uint256 tokensSlash + ) public useIndexer useAllocation(tokens) { + tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens)); + + resetPrank(users.fisherman); + bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1")); + + resetPrank(users.arbitrator); + _acceptDispute(disputeID, tokensSlash); + } + function test_Indexing_Accept_RevertIf_CallerIsNotArbitrator( uint256 tokens, uint256 tokensSlash diff --git a/packages/subgraph-service/test/disputeManager/disputes/indexing/create.t.sol b/packages/subgraph-service/test/disputeManager/disputes/indexing/create.t.sol index 8d1b75c21..3b1df5011 100644 --- a/packages/subgraph-service/test/disputeManager/disputes/indexing/create.t.sol +++ b/packages/subgraph-service/test/disputeManager/disputes/indexing/create.t.sol @@ -7,16 +7,28 @@ import { IDisputeManager } from "../../../../contracts/interfaces/IDisputeManage import { DisputeManagerTest } from "../../DisputeManager.t.sol"; contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest { - /* * TESTS */ - function test_Indexing_Create_Dispute( + function test_Indexing_Create_Dispute(uint256 tokens) public useIndexer useAllocation(tokens) { + resetPrank(users.fisherman); + _createIndexingDispute(allocationID, bytes32("POI1")); + } + + function test_Indexing_Create_Dispute_RevertWhen_SubgraphServiceNotSet( uint256 tokens ) public useIndexer useAllocation(tokens) { resetPrank(users.fisherman); - _createIndexingDispute(allocationID, bytes32("POI1")); + + // clear subgraph service address from storage + _setStorage_SubgraphService(address(0)); + + // // Approve the dispute deposit + token.approve(address(disputeManager), disputeDeposit); + + vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerSubgraphServiceNotSet.selector)); + disputeManager.createIndexingDispute(allocationID, bytes32("POI2")); } function test_Indexing_Create_MultipleDisputes() public { @@ -33,7 +45,12 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest { _createProvision(indexer, tokens, maxSlashingPercentage, disputePeriod); _register(indexer, abi.encode("url", "geoHash", address(0))); uint256 allocationIDPrivateKey = uint256(keccak256(abi.encodePacked(i))); - bytes memory data = _createSubgraphAllocationData(indexer, subgraphDeployment, allocationIDPrivateKey, tokens); + bytes memory data = _createSubgraphAllocationData( + indexer, + subgraphDeployment, + allocationIDPrivateKey, + tokens + ); _startService(indexer, data); allocationIDPrivateKeys[i] = allocationIDPrivateKey; } @@ -48,7 +65,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest { uint256 tokens ) public useIndexer useAllocation(tokens) { resetPrank(users.fisherman); - bytes32 disputeID =_createIndexingDispute(allocationID, bytes32("POI1")); + bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1")); // Create another dispute with different fisherman address otherFisherman = makeAddr("otherFisherman"); @@ -78,9 +95,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest { vm.stopPrank(); } - function test_Indexing_Create_RevertIf_AllocationDoesNotExist( - uint256 tokens - ) public useFisherman { + function test_Indexing_Create_RevertIf_AllocationDoesNotExist(uint256 tokens) public useFisherman { tokens = bound(tokens, disputeDeposit, 10_000_000_000 ether); token.approve(address(disputeManager), tokens); bytes memory expectedError = abi.encodeWithSelector( @@ -92,9 +107,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest { vm.stopPrank(); } - function test_Indexing_Create_RevertIf_IndexerIsBelowStake( - uint256 tokens - ) public useIndexer useAllocation(tokens) { + function test_Indexing_Create_RevertIf_IndexerIsBelowStake(uint256 tokens) public useIndexer useAllocation(tokens) { // Close allocation bytes memory data = abi.encode(allocationID); _stopService(users.indexer, data); diff --git a/packages/subgraph-service/test/disputeManager/disputes/query/accept.t.sol b/packages/subgraph-service/test/disputeManager/disputes/query/accept.t.sol index 7670d381e..7262dadb9 100644 --- a/packages/subgraph-service/test/disputeManager/disputes/query/accept.t.sol +++ b/packages/subgraph-service/test/disputeManager/disputes/query/accept.t.sol @@ -34,6 +34,39 @@ contract DisputeManagerQueryAcceptDisputeTest is DisputeManagerTest { _acceptDispute(disputeID, tokensSlash); } + function test_Query_Accept_Dispute_RevertWhen_SubgraphServiceNotSet( + uint256 tokens, + uint256 tokensSlash + ) public useIndexer useAllocation(tokens) { + tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens)); + + resetPrank(users.fisherman); + Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId); + bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey); + bytes32 disputeID = _createQueryDispute(attestationData); + + resetPrank(users.arbitrator); + // clear subgraph service address from storage + _setStorage_SubgraphService(address(0)); + vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerSubgraphServiceNotSet.selector)); + disputeManager.acceptDispute(disputeID, tokensSlash); + } + + function test_Query_Accept_Dispute_OptParam( + uint256 tokens, + uint256 tokensSlash + ) public useIndexer useAllocation(tokens) { + tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens)); + + resetPrank(users.fisherman); + Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId); + bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey); + bytes32 disputeID = _createQueryDispute(attestationData); + + resetPrank(users.arbitrator); + _acceptDispute(disputeID, tokensSlash); + } + function test_Query_Accept_RevertIf_CallerIsNotArbitrator( uint256 tokens, uint256 tokensSlash @@ -72,4 +105,23 @@ contract DisputeManagerQueryAcceptDisputeTest is DisputeManagerTest { vm.expectRevert(expectedError); disputeManager.acceptDispute(disputeID, tokensSlash); } + + function test_Query_Accept_RevertWhen_UsingConflictAccept( + uint256 tokens, + uint256 tokensSlash + ) public useIndexer useAllocation(tokens) { + tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens)); + + resetPrank(users.fisherman); + Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId); + bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey); + bytes32 disputeID = _createQueryDispute(attestationData); + + resetPrank(users.arbitrator); + vm.expectRevert(abi.encodeWithSelector( + IDisputeManager.DisputeManagerDisputeNotInConflict.selector, + disputeID + )); + disputeManager.acceptDisputeConflict(disputeID, tokensSlash, true, 0); + } } diff --git a/packages/subgraph-service/test/disputeManager/disputes/query/create.t.sol b/packages/subgraph-service/test/disputeManager/disputes/query/create.t.sol index 4eba11744..3dd4f7bbf 100644 --- a/packages/subgraph-service/test/disputeManager/disputes/query/create.t.sol +++ b/packages/subgraph-service/test/disputeManager/disputes/query/create.t.sol @@ -17,13 +17,28 @@ contract DisputeManagerQueryCreateDisputeTest is DisputeManagerTest { * TESTS */ - function test_Query_Create_Dispute(uint256 tokens) public useIndexer useAllocation(tokens) { + function test_Query_Create_Dispute_Only(uint256 tokens) public useIndexer useAllocation(tokens) { resetPrank(users.fisherman); Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId); bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey); _createQueryDispute(attestationData); } + function test_Query_Create_Dispute_RevertWhen_SubgraphServiceNotSet(uint256 tokens) public useIndexer useAllocation(tokens) { + resetPrank(users.fisherman); + Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId); + bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey); + + // clear subgraph service address from storage + _setStorage_SubgraphService(address(0)); + + // // Approve the dispute deposit + token.approve(address(disputeManager), disputeDeposit); + + vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerSubgraphServiceNotSet.selector)); + disputeManager.createQueryDispute(attestationData); + } + function test_Query_Create_MultipleDisputes_DifferentFisherman( uint256 tokens ) public useIndexer useAllocation(tokens) { diff --git a/packages/subgraph-service/test/disputeManager/disputes/queryConflict/accept.t.sol b/packages/subgraph-service/test/disputeManager/disputes/queryConflict/accept.t.sol index 46a6d4bdd..f145277af 100644 --- a/packages/subgraph-service/test/disputeManager/disputes/queryConflict/accept.t.sol +++ b/packages/subgraph-service/test/disputeManager/disputes/queryConflict/accept.t.sol @@ -18,7 +18,7 @@ contract DisputeManagerQueryConflictAcceptDisputeTest is DisputeManagerTest { * TESTS */ - function test_Query_Conflict_Accept_Dispute( + function test_Query_Conflict_Accept_Dispute_Draw_Other( uint256 tokens, uint256 tokensSlash ) public useIndexer useAllocation(tokens) { @@ -33,11 +33,53 @@ contract DisputeManagerQueryConflictAcceptDisputeTest is DisputeManagerTest { allocationIDPrivateKey ); + uint256 fishermanBalanceBefore = token.balanceOf(users.fisherman); + resetPrank(users.fisherman); - (bytes32 disputeID1,) = _createQueryDisputeConflict(attestationData1, attestationData2); + (bytes32 disputeID1, ) = _createQueryDisputeConflict(attestationData1, attestationData2); resetPrank(users.arbitrator); - _acceptDispute(disputeID1, tokensSlash); + _acceptDisputeConflict(disputeID1, tokensSlash, false, 0); + + uint256 fishermanRewardPercentage = disputeManager.fishermanRewardCut(); + uint256 fishermanReward = tokensSlash.mulPPM(fishermanRewardPercentage); + uint256 fishermanBalanceAfter = token.balanceOf(users.fisherman); + + assertEq(fishermanBalanceAfter, fishermanBalanceBefore + fishermanReward); + } + + function test_Query_Conflict_Accept_Dispute_Accept_Other( + uint256 tokens, + uint256 tokensSlash, + uint256 tokensSlashRelatedDispute + ) public useIndexer useAllocation(tokens) { + tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens)); + tokensSlashRelatedDispute = bound(tokensSlashRelatedDispute, 1, uint256(maxSlashingPercentage).mulPPM(tokens)); + + (bytes memory attestationData1, bytes memory attestationData2) = _createConflictingAttestations( + requestCID, + subgraphDeployment, + responseCID1, + responseCID2, + allocationIDPrivateKey, + allocationIDPrivateKey + ); + + uint256 fishermanBalanceBefore = token.balanceOf(users.fisherman); + + resetPrank(users.fisherman); + (bytes32 disputeID1, ) = _createQueryDisputeConflict(attestationData1, attestationData2); + + resetPrank(users.arbitrator); + _acceptDisputeConflict(disputeID1, tokensSlash, true, tokensSlashRelatedDispute); + + uint256 fishermanRewardPercentage = disputeManager.fishermanRewardCut(); + uint256 fishermanRewardFirstDispute = tokensSlash.mulPPM(fishermanRewardPercentage); + uint256 fishermanRewardRelatedDispute = tokensSlashRelatedDispute.mulPPM(fishermanRewardPercentage); + uint256 fishermanReward = fishermanRewardFirstDispute + fishermanRewardRelatedDispute; + uint256 fishermanBalanceAfter = token.balanceOf(users.fisherman); + + assertEq(fishermanBalanceAfter, fishermanBalanceBefore + fishermanReward); } function test_Query_Conflict_Accept_RevertIf_CallerIsNotArbitrator( @@ -56,12 +98,12 @@ contract DisputeManagerQueryConflictAcceptDisputeTest is DisputeManagerTest { ); resetPrank(users.fisherman); - (bytes32 disputeID1,) = _createQueryDisputeConflict(attestationData1, attestationData2); + (bytes32 disputeID1, ) = _createQueryDisputeConflict(attestationData1, attestationData2); // attempt to accept dispute as fisherman resetPrank(users.fisherman); vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerNotArbitrator.selector)); - disputeManager.acceptDispute(disputeID1, tokensSlash); + disputeManager.acceptDisputeConflict(disputeID1, tokensSlash, false, 0); } function test_Query_Conflict_Accept_RevertWhen_SlashingOverMaxSlashPercentage( @@ -80,17 +122,85 @@ contract DisputeManagerQueryConflictAcceptDisputeTest is DisputeManagerTest { ); resetPrank(users.fisherman); - (bytes32 disputeID1,) = _createQueryDisputeConflict(attestationData1, attestationData2); + (bytes32 disputeID1, ) = _createQueryDisputeConflict(attestationData1, attestationData2); // max slashing percentage is 50% resetPrank(users.arbitrator); uint256 maxTokensToSlash = uint256(maxSlashingPercentage).mulPPM(tokens); bytes memory expectedError = abi.encodeWithSelector( - IDisputeManager.DisputeManagerInvalidTokensSlash.selector, + IDisputeManager.DisputeManagerInvalidTokensSlash.selector, tokensSlash, maxTokensToSlash ); vm.expectRevert(expectedError); + disputeManager.acceptDisputeConflict(disputeID1, tokensSlash, false, 0); + } + + function test_Query_Conflict_Accept_AcceptRelated_DifferentIndexer( + uint256 tokensFirstIndexer, + uint256 tokensSecondIndexer, + uint256 tokensSlash, + uint256 tokensSlashRelatedDispute + ) public useIndexer useAllocation(tokensFirstIndexer) { + tokensSecondIndexer = bound(tokensSecondIndexer, minimumProvisionTokens, 10_000_000_000 ether); + tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokensFirstIndexer)); + + // Setup different indexer for related dispute + address differentIndexer = makeAddr("DifferentIndexer"); + mint(differentIndexer, tokensSecondIndexer); + uint256 differentIndexerAllocationIDPrivateKey = uint256(keccak256(abi.encodePacked(differentIndexer))); + resetPrank(differentIndexer); + _createProvision(differentIndexer, tokensSecondIndexer, maxSlashingPercentage, disputePeriod); + _register(differentIndexer, abi.encode("url", "geoHash", address(0))); + bytes memory data = _createSubgraphAllocationData( + differentIndexer, + subgraphDeployment, + differentIndexerAllocationIDPrivateKey, + tokensSecondIndexer + ); + _startService(differentIndexer, data); + tokensSlashRelatedDispute = bound( + tokensSlashRelatedDispute, + 1, + uint256(maxSlashingPercentage).mulPPM(tokensSecondIndexer) + ); + + (bytes memory attestationData1, bytes memory attestationData2) = _createConflictingAttestations( + requestCID, + subgraphDeployment, + responseCID1, + responseCID2, + allocationIDPrivateKey, + differentIndexerAllocationIDPrivateKey + ); + + resetPrank(users.fisherman); + (bytes32 disputeID1, ) = _createQueryDisputeConflict(attestationData1, attestationData2); + + resetPrank(users.arbitrator); + _acceptDisputeConflict(disputeID1, tokensSlash, true, tokensSlashRelatedDispute); + } + + function test_Query_Conflict_Accept_RevertWhen_UsingSingleAccept( + uint256 tokens, + uint256 tokensSlash + ) public useIndexer useAllocation(tokens) { + tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens)); + + (bytes memory attestationData1, bytes memory attestationData2) = _createConflictingAttestations( + requestCID, + subgraphDeployment, + responseCID1, + responseCID2, + allocationIDPrivateKey, + allocationIDPrivateKey + ); + + resetPrank(users.fisherman); + (bytes32 disputeID1, ) = _createQueryDisputeConflict(attestationData1, attestationData2); + + resetPrank(users.arbitrator); + vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputeInConflict.selector, disputeID1)); disputeManager.acceptDispute(disputeID1, tokensSlash); } } diff --git a/packages/subgraph-service/test/disputeManager/disputes/queryConflict/reject.t.sol b/packages/subgraph-service/test/disputeManager/disputes/queryConflict/reject.t.sol index ff347f7d2..3b56a05d8 100644 --- a/packages/subgraph-service/test/disputeManager/disputes/queryConflict/reject.t.sol +++ b/packages/subgraph-service/test/disputeManager/disputes/queryConflict/reject.t.sol @@ -7,14 +7,11 @@ import { IDisputeManager } from "../../../../contracts/interfaces/IDisputeManage import { DisputeManagerTest } from "../../DisputeManager.t.sol"; contract DisputeManagerQueryConflictRejectDisputeTest is DisputeManagerTest { - /* * TESTS */ - function test_Query_Conflict_Reject_Revert( - uint256 tokens - ) public useIndexer useAllocation(tokens) { + function test_Query_Conflict_Reject_Revert(uint256 tokens) public useIndexer useAllocation(tokens) { bytes32 requestCID = keccak256(abi.encodePacked("Request CID")); bytes32 responseCID1 = keccak256(abi.encodePacked("Response CID 1")); bytes32 responseCID2 = keccak256(abi.encodePacked("Response CID 2")); @@ -29,14 +26,10 @@ contract DisputeManagerQueryConflictRejectDisputeTest is DisputeManagerTest { ); resetPrank(users.fisherman); - (bytes32 disputeID1, bytes32 disputeID2) = _createQueryDisputeConflict(attestationData1, attestationData2); + (bytes32 disputeID1, ) = _createQueryDisputeConflict(attestationData1, attestationData2); resetPrank(users.arbitrator); - vm.expectRevert(abi.encodeWithSelector( - IDisputeManager.DisputeManagerMustAcceptRelatedDispute.selector, - disputeID1, - disputeID2 - )); + vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputeInConflict.selector, disputeID1)); disputeManager.rejectDispute(disputeID1); } } diff --git a/packages/subgraph-service/test/disputeManager/governance/disputeDeposit.t.sol b/packages/subgraph-service/test/disputeManager/governance/disputeDeposit.t.sol index 1df13acb2..bfd97f731 100644 --- a/packages/subgraph-service/test/disputeManager/governance/disputeDeposit.t.sol +++ b/packages/subgraph-service/test/disputeManager/governance/disputeDeposit.t.sol @@ -14,12 +14,12 @@ contract DisputeManagerGovernanceDisputeDepositTest is DisputeManagerTest { */ function test_Governance_SetDisputeDeposit(uint256 disputeDeposit) public useGovernor { - vm.assume(disputeDeposit > 0); + vm.assume(disputeDeposit >= MIN_DISPUTE_DEPOSIT); _setDisputeDeposit(disputeDeposit); } - function test_Governance_RevertWhen_ZeroValue() public useGovernor { - uint256 disputeDeposit = 0; + function test_Governance_RevertWhen_DepositTooLow(uint256 disputeDeposit) public useGovernor { + vm.assume(disputeDeposit < MIN_DISPUTE_DEPOSIT); vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerInvalidDisputeDeposit.selector, disputeDeposit)); disputeManager.setDisputeDeposit(disputeDeposit); } diff --git a/packages/subgraph-service/test/disputeManager/governance/subgraphService.t.sol b/packages/subgraph-service/test/disputeManager/governance/subgraphService.t.sol new file mode 100644 index 000000000..8f39d48c3 --- /dev/null +++ b/packages/subgraph-service/test/disputeManager/governance/subgraphService.t.sol @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import "forge-std/Test.sol"; + +import { IDisputeManager } from "../../../contracts/interfaces/IDisputeManager.sol"; +import { DisputeManagerTest } from "../DisputeManager.t.sol"; + +contract DisputeManagerGovernanceSubgraphService is DisputeManagerTest { + + /* + * TESTS + */ + + function test_Governance_SetSubgraphService(address subgraphService) public useGovernor { + vm.assume(subgraphService != address(0)); + _setSubgraphService(subgraphService); + } + + function test_Governance_SetSubgraphService_RevertWhenZero() public useGovernor { + vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerInvalidZeroAddress.selector)); + disputeManager.setSubgraphService(address(0)); + } +} diff --git a/packages/subgraph-service/test/mocks/MockGRTToken.sol b/packages/subgraph-service/test/mocks/MockGRTToken.sol index 7d21fd00a..c54f4e24c 100644 --- a/packages/subgraph-service/test/mocks/MockGRTToken.sol +++ b/packages/subgraph-service/test/mocks/MockGRTToken.sol @@ -7,7 +7,9 @@ import "@graphprotocol/contracts/contracts/token/IGraphToken.sol"; contract MockGRTToken is ERC20, IGraphToken { constructor() ERC20("Graph Token", "GRT") {} - function burn(uint256 amount) external {} + function burn(uint256 amount) external { + _burn(msg.sender, amount); + } function burnFrom(address _from, uint256 amount) external { _burn(_from, amount); diff --git a/packages/subgraph-service/test/shared/HorizonStakingShared.t.sol b/packages/subgraph-service/test/shared/HorizonStakingShared.t.sol index e07516903..abe425793 100644 --- a/packages/subgraph-service/test/shared/HorizonStakingShared.t.sol +++ b/packages/subgraph-service/test/shared/HorizonStakingShared.t.sol @@ -5,11 +5,11 @@ import "forge-std/Test.sol"; import { IGraphPayments } from "@graphprotocol/horizon/contracts/interfaces/IGraphPayments.sol"; import { IHorizonStakingTypes } from "@graphprotocol/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol"; +import { IHorizonStakingExtension } from "@graphprotocol/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol"; import { SubgraphBaseTest } from "../SubgraphBaseTest.t.sol"; abstract contract HorizonStakingSharedTest is SubgraphBaseTest { - /* * HELPERS */ @@ -45,11 +45,11 @@ abstract contract HorizonStakingSharedTest is SubgraphBaseTest { function _thawDeprovisionAndUnstake(address _indexer, address _verifier, uint256 _tokens) internal { // Initiate thaw request staking.thaw(_indexer, _verifier, _tokens); - + // Skip thawing period IHorizonStakingTypes.Provision memory provision = staking.getProvision(_indexer, _verifier); skip(provision.thawingPeriod + 1); - + // Deprovision and unstake staking.deprovision(_indexer, _verifier, 0); staking.unstake(_tokens); @@ -64,6 +64,67 @@ abstract contract HorizonStakingSharedTest is SubgraphBaseTest { staking.setProvisionParameters(_indexer, _verifier, _maxVerifierCut, _thawingPeriod); } + function _setStorage_allocation_hardcoded(address indexer, address allocationId, uint256 tokens) internal { + IHorizonStakingExtension.Allocation memory allocation = IHorizonStakingExtension.Allocation({ + indexer: indexer, + subgraphDeploymentID: bytes32("0x12344321"), + tokens: tokens, + createdAtEpoch: 1234, + closedAtEpoch: 1235, + collectedFees: 1234, + __DEPRECATED_effectiveAllocation: 1222234, + accRewardsPerAllocatedToken: 1233334, + distributedRebates: 1244434 + }); + + // __DEPRECATED_allocations + uint256 allocationsSlot = 15; + bytes32 allocationBaseSlot = keccak256(abi.encode(allocationId, allocationsSlot)); + vm.store(address(staking), allocationBaseSlot, bytes32(uint256(uint160(allocation.indexer)))); + vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 1), allocation.subgraphDeploymentID); + vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 2), bytes32(tokens)); + vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 3), bytes32(allocation.createdAtEpoch)); + vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 4), bytes32(allocation.closedAtEpoch)); + vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 5), bytes32(allocation.collectedFees)); + vm.store( + address(staking), + bytes32(uint256(allocationBaseSlot) + 6), + bytes32(allocation.__DEPRECATED_effectiveAllocation) + ); + vm.store( + address(staking), + bytes32(uint256(allocationBaseSlot) + 7), + bytes32(allocation.accRewardsPerAllocatedToken) + ); + vm.store(address(staking), bytes32(uint256(allocationBaseSlot) + 8), bytes32(allocation.distributedRebates)); + + // _serviceProviders + uint256 serviceProviderSlot = 14; + bytes32 serviceProviderBaseSlot = keccak256(abi.encode(allocation.indexer, serviceProviderSlot)); + uint256 currentTokensStaked = uint256(vm.load(address(staking), serviceProviderBaseSlot)); + uint256 currentTokensProvisioned = uint256( + vm.load(address(staking), bytes32(uint256(serviceProviderBaseSlot) + 1)) + ); + vm.store( + address(staking), + bytes32(uint256(serviceProviderBaseSlot) + 0), + bytes32(currentTokensStaked + tokens) + ); + vm.store( + address(staking), + bytes32(uint256(serviceProviderBaseSlot) + 1), + bytes32(currentTokensProvisioned + tokens) + ); + + // __DEPRECATED_subgraphAllocations + uint256 subgraphsAllocationsSlot = 16; + bytes32 subgraphAllocationsBaseSlot = keccak256( + abi.encode(allocation.subgraphDeploymentID, subgraphsAllocationsSlot) + ); + uint256 currentAllocatedTokens = uint256(vm.load(address(staking), subgraphAllocationsBaseSlot)); + vm.store(address(staking), subgraphAllocationsBaseSlot, bytes32(currentAllocatedTokens + tokens)); + } + /* * PRIVATE */ diff --git a/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol b/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol index b417f30bf..5aafa3507 100644 --- a/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol +++ b/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol @@ -151,7 +151,7 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest { assertEq(afterSubgraphAllocatedTokens, _tokens); } - function _forceCloseAllocation(address _allocationId) internal { + function _closeStaleAllocation(address _allocationId) internal { assertTrue(subgraphService.isActiveAllocation(_allocationId)); Allocation.State memory allocation = subgraphService.getAllocation(_allocationId); @@ -168,7 +168,7 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest { ); // close stale allocation - subgraphService.forceCloseAllocation(_allocationId); + subgraphService.closeStaleAllocation(_allocationId); // update allocation allocation = subgraphService.getAllocation(_allocationId); @@ -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, @@ -379,6 +380,17 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest { } } + function _migrateLegacyAllocation(address _indexer, address _allocationId, bytes32 _subgraphDeploymentID) internal { + vm.expectEmit(address(subgraphService)); + emit AllocationManager.LegacyAllocationMigrated(_indexer, _allocationId, _subgraphDeploymentID); + + subgraphService.migrateLegacyAllocation(_indexer, _allocationId, _subgraphDeploymentID); + + (address afterIndexer, bytes32 afterSubgraphDeploymentId) = subgraphService.legacyAllocations(_allocationId); + assertEq(afterIndexer, _indexer); + assertEq(afterSubgraphDeploymentId, _subgraphDeploymentID); + } + /* * HELPERS */ diff --git a/packages/subgraph-service/test/subgraphService/allocation/forceClose.t.sol b/packages/subgraph-service/test/subgraphService/allocation/forceClose.t.sol index 8817355f6..5b35f8d4e 100644 --- a/packages/subgraph-service/test/subgraphService/allocation/forceClose.t.sol +++ b/packages/subgraph-service/test/subgraphService/allocation/forceClose.t.sol @@ -3,38 +3,30 @@ pragma solidity 0.8.27; import "forge-std/Test.sol"; -import { IDataService } from "@graphprotocol/horizon/contracts/data-service/interfaces/IDataService.sol"; import { IGraphPayments } from "@graphprotocol/horizon/contracts/interfaces/IGraphPayments.sol"; -import { ProvisionManager } from "@graphprotocol/horizon/contracts/data-service/utilities/ProvisionManager.sol"; -import { ProvisionTracker } from "@graphprotocol/horizon/contracts/data-service/libraries/ProvisionTracker.sol"; import { Allocation } from "../../../contracts/libraries/Allocation.sol"; -import { AllocationManager } from "../../../contracts/utilities/AllocationManager.sol"; import { ISubgraphService } from "../../../contracts/interfaces/ISubgraphService.sol"; -import { LegacyAllocation } from "../../../contracts/libraries/LegacyAllocation.sol"; import { SubgraphServiceTest } from "../SubgraphService.t.sol"; contract SubgraphServiceAllocationForceCloseTest is SubgraphServiceTest { - address private permissionlessBob = makeAddr("permissionlessBob"); /* * TESTS */ - function test_SubgraphService_Allocation_ForceClose_Stale( - uint256 tokens - ) public useIndexer useAllocation(tokens) { + function test_SubgraphService_Allocation_ForceClose_Stale(uint256 tokens) public useIndexer useAllocation(tokens) { // Skip forward skip(maxPOIStaleness + 1); resetPrank(permissionlessBob); - _forceCloseAllocation(allocationID); + _closeStaleAllocation(allocationID); } function test_SubgraphService_Allocation_ForceClose_Stale_AfterCollecting( uint256 tokens - ) public useIndexer useAllocation(tokens) { + ) public useIndexer useAllocation(tokens) { // Simulate POIs being submitted uint8 numberOfPOIs = 5; uint256 timeBetweenPOIs = 5 days; @@ -52,43 +44,10 @@ contract SubgraphServiceAllocationForceCloseTest is SubgraphServiceTest { // Close the stale allocation resetPrank(permissionlessBob); - _forceCloseAllocation(allocationID); - } - - function test_SubgraphService_Allocation_ForceClose_OverAllocated( - uint256 tokens - ) public useIndexer useAllocation(tokens) { - // thaw some tokens to become over allocated - staking.thaw(users.indexer, address(subgraphService), tokens / 2); - - resetPrank(permissionlessBob); - _forceCloseAllocation(allocationID); - } - - function test_SubgraphService_Allocation_ForceClose_OverAllocated_AfterCollecting( - uint256 tokens - ) public useIndexer useAllocation(tokens) { - // Simulate POIs being submitted - uint8 numberOfPOIs = 5; - uint256 timeBetweenPOIs = 5 days; - - for (uint8 i = 0; i < numberOfPOIs; i++) { - // Skip forward - skip(timeBetweenPOIs); - - bytes memory data = abi.encode(allocationID, bytes32("POI1")); - _collect(users.indexer, IGraphPayments.PaymentTypes.IndexingRewards, data); - } - - // thaw some tokens to become over allocated - staking.thaw(users.indexer, address(subgraphService), tokens / 2); - - // Close the over allocated allocation - resetPrank(permissionlessBob); - _forceCloseAllocation(allocationID); + _closeStaleAllocation(allocationID); } - function test_SubgraphService_Allocation_ForceClose_RevertIf_NotStaleOrOverAllocated( + function test_SubgraphService_Allocation_ForceClose_RevertIf_NotStale( uint256 tokens ) public useIndexer useAllocation(tokens) { // Simulate POIs being submitted @@ -98,12 +57,12 @@ contract SubgraphServiceAllocationForceCloseTest is SubgraphServiceTest { for (uint8 i = 0; i < numberOfPOIs; i++) { // Skip forward skip(timeBetweenPOIs); - + resetPrank(users.indexer); bytes memory data = abi.encode(allocationID, bytes32("POI1")); _collect(users.indexer, IGraphPayments.PaymentTypes.IndexingRewards, data); - + resetPrank(permissionlessBob); vm.expectRevert( abi.encodeWithSelector( @@ -111,13 +70,11 @@ contract SubgraphServiceAllocationForceCloseTest is SubgraphServiceTest { allocationID ) ); - subgraphService.forceCloseAllocation(allocationID); + subgraphService.closeStaleAllocation(allocationID); } } - function test_SubgraphService_Allocation_ForceClose_RevertIf_Altruistic( - uint256 tokens - ) public useIndexer { + function test_SubgraphService_Allocation_ForceClose_RevertIf_Altruistic(uint256 tokens) public useIndexer { tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS); _createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod); @@ -130,11 +87,8 @@ contract SubgraphServiceAllocationForceCloseTest is SubgraphServiceTest { resetPrank(permissionlessBob); vm.expectRevert( - abi.encodeWithSelector( - ISubgraphService.SubgraphServiceAllocationIsAltruistic.selector, - allocationID - ) + abi.encodeWithSelector(ISubgraphService.SubgraphServiceAllocationIsAltruistic.selector, allocationID) ); - subgraphService.forceCloseAllocation(allocationID); + subgraphService.closeStaleAllocation(allocationID); } } diff --git a/packages/subgraph-service/test/subgraphService/allocation/start.t.sol b/packages/subgraph-service/test/subgraphService/allocation/start.t.sol index f5d76a350..a7df1f1a8 100644 --- a/packages/subgraph-service/test/subgraphService/allocation/start.t.sol +++ b/packages/subgraph-service/test/subgraphService/allocation/start.t.sol @@ -4,7 +4,6 @@ pragma solidity 0.8.27; import "forge-std/Test.sol"; import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; -import { IDataService } from "@graphprotocol/horizon/contracts/data-service/interfaces/IDataService.sol"; import { ProvisionManager } from "@graphprotocol/horizon/contracts/data-service/utilities/ProvisionManager.sol"; import { ProvisionTracker } from "@graphprotocol/horizon/contracts/data-service/libraries/ProvisionTracker.sol"; @@ -137,35 +136,52 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest { subgraphService.startService(users.indexer, data); } - function test_SubgraphService_Allocation_Start_RevertWhen_ArealdyExists(uint256 tokens) public useIndexer { + function test_SubgraphService_Allocation_Start_RevertWhen_AlreadyExists_SubgraphService(uint256 tokens) public useIndexer { tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS); _createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod); _register(users.indexer, abi.encode("url", "geoHash", address(0))); - bytes32 slot = keccak256(abi.encode(allocationID, uint256(158))); - vm.store(address(subgraphService), slot, bytes32(uint256(uint160(users.indexer)))); - vm.store(address(subgraphService), bytes32(uint256(slot) + 1), subgraphDeployment); - bytes memory data = _generateData(tokens); + _startService(users.indexer, data); + vm.expectRevert(abi.encodeWithSelector( - LegacyAllocation.LegacyAllocationExists.selector, + Allocation.AllocationAlreadyExists.selector, allocationID )); subgraphService.startService(users.indexer, data); } - function test_SubgraphService_Allocation_Start_RevertWhen_ReusingAllocationId(uint256 tokens) public useIndexer { + function test_SubgraphService_Allocation_Start_RevertWhen_AlreadyExists_Migrated(uint256 tokens) public useIndexer { tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS); _createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod); _register(users.indexer, abi.encode("url", "geoHash", address(0))); + resetPrank(users.governor); + _migrateLegacyAllocation(users.indexer, allocationID, subgraphDeployment); + + resetPrank(users.indexer); bytes memory data = _generateData(tokens); - _startService(users.indexer, data); + vm.expectRevert(abi.encodeWithSelector( + LegacyAllocation.LegacyAllocationAlreadyExists.selector, + allocationID + )); + subgraphService.startService(users.indexer, data); + } + + function test_SubgraphService_Allocation_Start_RevertWhen_AlreadyExists_Staking(uint256 tokens) public useIndexer { + tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS); + _createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod); + _register(users.indexer, abi.encode("url", "geoHash", address(0))); + + // create dummy allo in staking contract + _setStorage_allocation_hardcoded(users.indexer, allocationID, tokens); + + bytes memory data = _generateData(tokens); vm.expectRevert(abi.encodeWithSelector( - Allocation.AllocationAlreadyExists.selector, + LegacyAllocation.LegacyAllocationAlreadyExists.selector, allocationID )); subgraphService.startService(users.indexer, data); 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/subgraphService/governance/legacy.t.sol b/packages/subgraph-service/test/subgraphService/governance/legacy.t.sol new file mode 100644 index 000000000..d1b5dd124 --- /dev/null +++ b/packages/subgraph-service/test/subgraphService/governance/legacy.t.sol @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import "forge-std/Test.sol"; + +import { OwnableUpgradeable } from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; + +import { SubgraphServiceTest } from "../SubgraphService.t.sol"; + +contract SubgraphServiceLegacyAllocation is SubgraphServiceTest { + /* + * TESTS + */ + + function test_MigrateAllocation() public useGovernor { + _migrateLegacyAllocation(users.indexer, allocationID, subgraphDeployment); + } + + function test_MigrateAllocation_WhenNotGovernor() public useIndexer { + vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, users.indexer)); + subgraphService.migrateLegacyAllocation(users.indexer, allocationID, subgraphDeployment); + } +} diff --git a/packages/subgraph-service/test/utils/Constants.sol b/packages/subgraph-service/test/utils/Constants.sol index 76f864da1..0c6419f3e 100644 --- a/packages/subgraph-service/test/utils/Constants.sol +++ b/packages/subgraph-service/test/utils/Constants.sol @@ -7,6 +7,7 @@ abstract contract Constants { uint256 internal constant EPOCH_LENGTH = 1; // Dispute Manager uint64 internal constant disputePeriod = 7 days; + uint256 internal constant MIN_DISPUTE_DEPOSIT = 1 ether; // 1 GRT uint256 internal constant disputeDeposit = 100 ether; // 100 GRT uint32 internal constant fishermanRewardPercentage = 500000; // 50% uint32 internal constant maxSlashingPercentage = 500000; // 50% @@ -21,7 +22,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