diff --git a/packages/horizon/contracts/payments/collectors/RecurringCollector.sol b/packages/horizon/contracts/payments/collectors/RecurringCollector.sol index dd89dabad..9723fbe20 100644 --- a/packages/horizon/contracts/payments/collectors/RecurringCollector.sol +++ b/packages/horizon/contracts/payments/collectors/RecurringCollector.sol @@ -1375,6 +1375,12 @@ contract RecurringCollector is * @dev Allows RC to call data service functions (e.g. cancelByPayer) that check * rc.isAuthorized(payer, msg.sender). When msg.sender is RC itself, this returns true, * meaning RC is trusted to have verified authorization before delegating. + * @dev This override also governs the {Authorizable} `onlyAuthorized` guard, so + * thawSigner, cancelThawSigner and revokeAuthorizedSigner are callable by anyone for + * address(this). This is harmless because RC's own authorization record can never be + * authorized and the self-trust branch does not read it. The self-trust branch must not + * depend on stored authorization state (e.g. `revoked`); decouple the modifier from this + * predicate before changing that. * @param authorizer The authorizer address * @param signer The signer address to check authorization for * @return True if the signer is authorized diff --git a/packages/horizon/contracts/utilities/Authorizable.sol b/packages/horizon/contracts/utilities/Authorizable.sol index 24bdc32ac..a645ee5bd 100644 --- a/packages/horizon/contracts/utilities/Authorizable.sol +++ b/packages/horizon/contracts/utilities/Authorizable.sol @@ -15,6 +15,7 @@ import { MessageHashUtils } from "@openzeppelin/contracts/utils/cryptography/Mes * @dev Implements the {IAuthorizable} interface. * @notice A mechanism to authorize signers to sign messages on behalf of an authorizer. * Signers cannot be reused for different authorizers. + * Revocation is permanent: a revoked signer cannot be authorized again by any authorizer. * @dev Contract uses "authorizeSignerProof" as the domain for signer proofs. * Uses ERC-7201 namespaced storage for upgrade safety. * @custom:security-contact Please email security+contracts@thegraph.com if you find any diff --git a/packages/interfaces/contracts/data-service/IDataServiceAgreements.sol b/packages/interfaces/contracts/data-service/IDataServiceAgreements.sol index ea5b0dd54..00eb94a34 100644 --- a/packages/interfaces/contracts/data-service/IDataServiceAgreements.sol +++ b/packages/interfaces/contracts/data-service/IDataServiceAgreements.sol @@ -13,6 +13,9 @@ pragma solidity ^0.8.22; interface IDataServiceAgreements { /** * @notice Cancel an indexing agreement by payer / signer. + * @dev Called by the collector when a payer cancels an active agreement. Implementations must + * complete the cancellation by calling the collector's cancel with CancelAgreementBy.Payer; + * otherwise the agreement remains active in the collector. * @param agreementId The id of the indexing agreement */ function cancelIndexingAgreementByPayer(bytes16 agreementId) external; diff --git a/packages/interfaces/contracts/data-service/IDataServiceFees.sol b/packages/interfaces/contracts/data-service/IDataServiceFees.sol index e9bf60bf0..6b12f7dbd 100644 --- a/packages/interfaces/contracts/data-service/IDataServiceFees.sol +++ b/packages/interfaces/contracts/data-service/IDataServiceFees.sol @@ -31,6 +31,9 @@ interface IDataServiceFees is IDataService { * @dev This function is only meant to be called if the service provider has enough * stake claims that releasing them all at once would exceed the block gas limit. * @dev This function can be overriden and/or disabled. + * @dev Stake claims are released in creation order and traversal stops at the first claim that + * has not expired. If the dispute period is reduced, an older unexpired claim can temporarily + * block the release of newer claims that have already expired. * @dev Emits a {StakeClaimsReleased} event, and a {StakeClaimReleased} event for each claim released. * @param numClaimsToRelease Amount of stake claims to process. If 0, all stake claims are processed. */ diff --git a/packages/interfaces/contracts/horizon/IAgreementCollector.sol b/packages/interfaces/contracts/horizon/IAgreementCollector.sol index 41b39eba8..a25bf9d5b 100644 --- a/packages/interfaces/contracts/horizon/IAgreementCollector.sol +++ b/packages/interfaces/contracts/horizon/IAgreementCollector.sol @@ -140,6 +140,9 @@ interface IAgreementCollector is IPaymentsCollector { * payer writes against the wrong key and does not block the signature. * - SCOPE_PENDING and SCOPE_ACTIVE require payer authorization and no-op if nothing exists * on-chain. The payer (not the signer) must call. + * - SCOPE_ACTIVE does not change the agreement state directly: it forwards the request to the + * agreement's data service via {IDataServiceAgreements.cancelIndexingAgreementByPayer}, and + * the cancellation takes effect when the data service calls back into the collector. * Combining SCOPE_SIGNED with SCOPE_PENDING / SCOPE_ACTIVE in a single call is therefore only * useful when msg.sender is both the payer and the signer, which in this implementation only * happens when an EOA signs for itself as payer. diff --git a/packages/interfaces/contracts/subgraph-service/ISubgraphService.sol b/packages/interfaces/contracts/subgraph-service/ISubgraphService.sol index 9ed51115c..5bd235404 100644 --- a/packages/interfaces/contracts/subgraph-service/ISubgraphService.sol +++ b/packages/interfaces/contracts/subgraph-service/ISubgraphService.sol @@ -261,6 +261,8 @@ interface ISubgraphService is IDataServiceAgreements, IDataServiceFees { /** * @notice Sets the stake to fees ratio + * @dev The ratio is a plain multiplier, not a PPM value: collecting query or indexing fees + * locks `tokensCollected * stakeToFeesRatio` of the indexer's provision in a stake claim. * @param newStakeToFeesRatio The stake to fees ratio */ function setStakeToFeesRatio(uint256 newStakeToFeesRatio) external; diff --git a/packages/issuance/contracts/agreement/RecurringAgreementManager.sol b/packages/issuance/contracts/agreement/RecurringAgreementManager.sol index 750275800..9b96765a2 100644 --- a/packages/issuance/contracts/agreement/RecurringAgreementManager.sol +++ b/packages/issuance/contracts/agreement/RecurringAgreementManager.sol @@ -85,7 +85,10 @@ import { ReentrancyGuardTransient } from "@openzeppelin/contracts/utils/Reentran * {reconcileAgreement} is called. To fully halt collections, pause RecurringCollector too. * * Escalation ladder (targeted → full stop): - * 1. {emergencyRevokeRole} — disable a specific actor (operator, collector, guardian) + * 1. {emergencyRevokeRole} — disable a specific actor (operator, collector, guardian). + * Revoking COLLECTOR_ROLE only blocks discovery of new agreements; callbacks and + * reconciliation for already-tracked agreements continue. To contain a compromised + * collector, pause this contract (step 3) and {forceRemoveAgreement} its agreements. * 2. {emergencyClearEligibilityOracle} — fail-open if oracle blocks collections * 3. Pause this contract — blocks permissionless state changes, including collection * callbacks and reconciliation (see cross-contract note above) diff --git a/packages/issuance/contracts/allocate/IssuanceAllocator.sol b/packages/issuance/contracts/allocate/IssuanceAllocator.sol index dd45f611a..18acde1ad 100644 --- a/packages/issuance/contracts/allocate/IssuanceAllocator.sol +++ b/packages/issuance/contracts/allocate/IssuanceAllocator.sol @@ -85,10 +85,17 @@ import { ERC165Upgradeable } from "@openzeppelin/contracts-upgradeable/utils/int * * Total minted (self + allocator) for period: * ≤ max(selfMintingOffset, issuancePerBlock_final * blocks) + * + * When issuancePerBlock_final does not exceed the rates in force during the period: * ≤ Σ(issuancePerBlock_b) * * Therefore, total issuance never exceeds the sum of configured rates during the period. * + * Exception: a rate set over an undistributed window (e.g. while paused, via + * setIssuancePerBlock with minDistributedBlock) applies retroactively to the whole window, + * so a rate increase is bounded by issuancePerBlock_final * blocks rather than by the rates + * previously in force. This is intended; see {setIssuancePerBlock}. + * * 5. Offset Reconciliation: During pending distribution, selfMintingOffset is adjusted to account for * the period's issuance budget. When distribution catches up to current block, the offset is cleared. * Any remaining offset when cleared represents self-minting that occurred beyond what the final @@ -102,7 +109,8 @@ import { ERC165Upgradeable } from "@openzeppelin/contracts-upgradeable/utils/int * * This design ensures that even when issuancePerBlock or allocation rates change over time, and even * when self-minting targets mint independently, the total tokens minted never exceeds the sum of - * configured issuance rates during the period. + * configured issuance rates during the period, except for the intentional retroactive rate + * application described in the Issuance Upper Bound invariant. * * @dev There are a number of scenarios where the IssuanceAllocator could run into issues, including: * 1. The targetAddresses array could grow large enough that it exceeds the gas limit when calling distributeIssuance. diff --git a/packages/subgraph-service/contracts/SubgraphService.sol b/packages/subgraph-service/contracts/SubgraphService.sol index 1a7db3652..114af4dab 100644 --- a/packages/subgraph-service/contracts/SubgraphService.sol +++ b/packages/subgraph-service/contracts/SubgraphService.sol @@ -152,7 +152,6 @@ contract SubgraphService is * @dev Implements {IDataService-acceptProvisionPendingParameters} * * Requirements: - * - The indexer must be registered * - Must have previously staged provision parameters, using {IHorizonStaking-setProvisionParameters} * - The new provision parameters must be valid according to the subgraph service rules * diff --git a/packages/subgraph-service/contracts/libraries/Allocation.sol b/packages/subgraph-service/contracts/libraries/Allocation.sol index 404dc8cec..380350f26 100644 --- a/packages/subgraph-service/contracts/libraries/Allocation.sol +++ b/packages/subgraph-service/contracts/libraries/Allocation.sol @@ -133,7 +133,7 @@ library Allocation { /** * @notice Checks if an allocation is stale * @param self The allocation - * @param staleThreshold The time in blocks to consider an allocation stale + * @param staleThreshold The time in seconds to consider an allocation stale * @return True if the allocation is stale */ function isStale(IAllocation.State memory self, uint256 staleThreshold) internal view returns (bool) {