Open
Conversation
Fixes TRST-M-1 audit finding: Wrong TYPEHASH string is used for agreement updates, limiting functionality. * Fixed EIP712_RCAU_TYPEHASH to use correct uint64 types for deadline and endsAt fields (was incorrectly using uint256) * This prevents signature verification failures for RecurringCollectionAgreementUpdate
Fixes TRST-M-2 audit finding: Collection for an elapsed or canceled agreement could be wrong due to temporal calculation inconsistencies between IndexingAgreement and RecurringCollector layers. * Replace isCollectable() with getCollectionInfo() that returns both collectability and duration * Make RecurringCollector the single source of truth for temporal logic * Update IndexingAgreement to call getCollectionInfo() once and pass duration to _tokensToCollect()
Fixes signature replay attack vulnerability where old signed RecurringCollectionAgreementUpdate messages could be replayed to revert agreements to previous terms. ## Changes - Add `nonce` field to RecurringCollectionAgreementUpdate struct (uint32) - Add `updateNonce` field to AgreementData struct to track current nonce - Add nonce validation in RecurringCollector.update() to ensure sequential updates - Update EIP712_RCAU_TYPEHASH to include nonce field - Add comprehensive tests for nonce validation and replay attack prevention - Add RecurringCollectorInvalidUpdateNonce error for invalid nonce attempts ## Implementation Details - Nonces start at 0 when agreement is accepted - Each update must use current nonce + 1 - Nonce is incremented after successful update - Uses uint32 for gas optimization (supports 4B+ updates per agreement) - Single source of truth: nonce stored in AgreementData struct
Implements slippage protection mechanism to prevent silent token loss during rate-limited collections in RecurringCollector agreements. The implementation uses type(uint256).max convention to disable slippage checks, providing users full control over acceptable token loss during rate limiting. Resolves audit finding TRST-L-5: "RecurringCollector silently reduces collected tokens without user consent"
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
…Z L-01) Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
…tions (OZ L-02) Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Normalize all Solidity pragmas to caret form (^0.8.27) for forward compatibility, and bump the compiler version from 0.8.33 to 0.8.34 for the issuance and subgraph-service packages. Archive the now- obsolete CompilerUpgrade0833.md doc.
…-payments-management-audit
…-payments-management-audit
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Replace the hard revert (RecurringCollectorCollectionTooLate) with a Math.min cap in _getCollectionInfo. Collections past maxSecondsPerCollection now succeed with tokens capped at maxSecondsPerCollection worth of service, rather than failing entirely. Changes: - _getCollectionInfo caps elapsed seconds at maxSecondsPerCollection - Remove RecurringCollectorCollectionTooLate error from interface - Replace test_Collect_Revert_WhenCollectingTooLate with test_Collect_OK_WhenCollectingPastMaxSeconds - Update maxSecondsPerCollection NatSpec to reflect cap semantics - Fix zero-token test to use correct _sensibleAuthorizeAndAccept API
… zero-POI special case Move _requireValidCollect() call outside the tokens != 0 guard so temporal constraints (min/maxSecondsPerCollection) are always enforced, even for zero-token collections. This prevents advancing lastCollectionAt without passing temporal validation. Remove the zero-POI special case in IndexingAgreement that bypassed token calculation when entities == 0 && poi == bytes32(0). The temporal validation now handles this consistently.
Add adjustThaw for timer-aware escrow thaw management: caps at balance, preserves timer on decrease, and optionally skips increases that would reset the timer. Expose escrowAccounts mapping in IPaymentsEscrow interface.
Generalise the provider eligibility interface so it is not rewards-specific: - IRewardsEligibility → IProviderEligibility (same isEligible selector) - New IProviderEligibilityManagement with shared setter/getter/event - RewardsManager implements IProviderEligibilityManagement; remove bespoke setter/getter/event from IRewardsManager - Update RewardsEligibilityOracle, mocks, tests, deployment scripts and docs accordingly
Add IAgreementOwner interface enabling contracts to approve RCA accept/update by implementing an on-chain callback. When the signature parameter is empty, accept() and update() fall back to calling IAgreementOwner.approveAgreement() on the payer contract instead of verifying an ECDSA signature. Also adds getMaxNextClaim() view and removes the SignedRCA/SignedRCAU wrapper structs in favor of separate (struct, bytes) parameters.
Extract IDataServiceAgreements from ISubgraphService with cancelIndexingAgreementByPayer and bitmask-dispatched enforce pattern. Add ProvisionManager support for data-service callback verification.
…eanup Add EnumerableSet-based indexer tracking to RewardsEligibilityOracle with a helper contract for paginated queries. Introduce retention-based cleanup that removes indexers only after a configurable idle period. Split REO interface into focused sub-interfaces for administration, status, events, maintenance, and helper operations. Also refactor issuance constructors from address to IGraphToken type and normalise pragma to ^0.8.27 across issuance test files.
Indexing payments management audit
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1301 +/- ##
==========================================
+ Coverage 88.55% 90.24% +1.68%
==========================================
Files 75 81 +6
Lines 4615 5113 +498
Branches 981 1079 +98
==========================================
+ Hits 4087 4614 +527
+ Misses 507 465 -42
- Partials 21 34 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…greement updates Add RecurringAgreementManager with configurable escrow funding modes, enumerable agreement tracking, lifecycle management, and escrow reconciliation. Extends IAgreementOwner with beforeCollection/ afterCollection callbacks. Includes revokeAgreementUpdate and pending update escrow cleanup on cancel.
RembrandtK
force-pushed
the
indexing-payments-management-audit
branch
from
843fdd2 to
a23ad68
Compare
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Filter for files in audit scope:
bf6d4cb Indexing payments (merge)
Previously audited (2025-06-Indexing-Payments.pdf) implementation of indexing agreements for the Graph Protocol's recurring payment system. Adds RecurringCollector, IndexingAgreement library, and integrates indexing fee collection into SubgraphService and DisputeManager.
28edcd7 Post-Horizon migration removal of transition code (merge)
Remove HorizonStakingExtension, ExponentialRebates, LibFixedMath, MathUtils, and all transition-period guard logic. Simplify HorizonStaking to direct implementation without delegatecall split. Clean up RewardsManager multi-issuer model to single subgraphService. Remove legacy allocation/dispute code from SubgraphService.
f4451f1 feat: add back legacy allocation id collision check
fa99514 fix: cap maxSecondsPerCollection instead of reverting
c6836a7 fix: enforce temporal validation on zero-token collections and remove zero-POI special case
8efaec9 feat: add adjustThaw to PaymentsEscrow
3f1578c refactor: rename IRewardsEligibility to IProviderEligibility
(For the purpose of this audit note that the interfaces being changed are not in production.)
d20bc84 feat: contract approver model for RecurringCollector accept/update
ec72360 feat: IDataServiceAgreements interface and SubgraphService integration
89def3d feat: enumerable indexer tracking for REO and issuance constructor cleanup
(Do not need storage compatibility for delpoying an upgrade.)
843fdd2 feat: RecurringAgreementManager with lifecycle, escrow funding, and agreement updates