Indexing payments with escrow management and post-Horizon cleanup#1300
Closed
RembrandtK wants to merge 11 commits intomainfrom
Closed
Indexing payments with escrow management and post-Horizon cleanup#1300RembrandtK wants to merge 11 commits intomainfrom
RembrandtK wants to merge 11 commits intomainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR performs post-Horizon cleanup by removing legacy transition period code and consolidating version management for indexing payments with escrow handling.
Changes:
- Removed legacy allocation/delegation code and HorizonStakingExtension
- Updated Solidity version from 0.8.33 to ^0.8.27 across packages
- Replaced MathUtils library with OpenZeppelin's Math
- Added indexing fee dispute mechanism and agreement management
- Refactored test infrastructure to remove transition period tests
Reviewed changes
Copilot reviewed 212 out of 259 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/subgraph-service/contracts/DisputeManager.sol | Removed legacy dispute creation, added indexing fee dispute V1 |
| packages/subgraph-service/contracts/SubgraphServiceStorage.sol | Added indexingFeesCut storage variable |
| packages/horizon/contracts/staking/HorizonStakingBase.sol | Replaced MathUtils with OpenZeppelin Math, removed extension references |
| packages/horizon/contracts/utilities/GraphDirectory.sol | Removed deprecated Curation contract references |
| packages/issuance/test/unit/agreement-manager/*.t.sol | Added comprehensive test coverage for agreement management |
| packages/horizon/test/integration/during-transition-period/*.test.ts | Removed all transition period integration tests |
| packages/contracts/contracts/rewards/RewardsManager.sol | Removed staking contract as rewards issuer, SubgraphService only |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Indexing payments with escrow management and post-Horizon cleanup
🚨 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 #1300 +/- ##
==========================================
+ Coverage 88.55% 89.96% +1.40%
==========================================
Files 75 79 +4
Lines 4615 4953 +338
Branches 981 1052 +71
==========================================
+ Hits 4087 4456 +369
+ Misses 507 464 -43
- Partials 21 33 +12
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:
|
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.
Previously audited 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. Includes all TRST audit fixes: - TRST-H-1: IndexingAgreement.collect() on CanceledByPayer - TRST-H-2: Only agreement owner can collect indexing fee - TRST-H-3: collect() checks provision - TRST-M-1: correct TYPEHASH string for RCAU - TRST-M-2: shared collection window logic + improve _getCollectionInfo - TRST-M-3: Add nonce-based replay protection - TRST-L-3: Add deterministic agreement ID - TRST-L-5: Add slippage protection - TRST-L-6: Proper agreement version check - TRST-L-7: update() documentation - TRST-L-9: Cancel agreement if over-allocated - TRST-R-1: minor fixes - TRST-R-4: CEI violation - TRST-R-5: Terms validation - TRST-R-6: Configurable indexing fees cut Re-based from indexing-payments-baseline commits a7fb875..a9b500a
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. Includes OZ audit fixes: - OZ L-01: re-validate thawingPeriod when accepting provision parameters - OZ L-02: return correct result for getThawedTokens for delegations - OZ N-01: remove more deprecated code - OZ N-02: outdated documentation Adds force withdraw for legacy stake and delegations. Replaces MathUtils.min with OpenZeppelin Math.min throughout. Original commits on rem-horizon-cleanup-merge (f65877e..5b9463f) Plus housekeeping commits 8f5d93449 and ae9f59513
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.
Make thaw, cancelThaw, and withdraw idempotent so callers need not track escrow state before invoking these operations. Thaw accepts an evenIfTimerReset flag and a tokensThawing parameter for precise control. Refactor IPaymentsEscrow interface to match.
Add IContractApprover 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 IContractApprover.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.
Add RecurringAgreementManager (RAM) with full agreement lifecycle: register/remove indexers, create/revoke offers, reconcile escrow, and cancel agreements via IDataServiceAgreements. Includes configurable funding modes (JustInTime/Active/Full) with eligibility oracle, per-agreement collector with 2D escrow storage, contract approver collection callbacks, and comprehensive test suite.
RembrandtK
force-pushed
the
indexing-payments
branch
from
7f960bd to
1013fb0
Compare
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
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.
Content out of date, will get a force push soon.