Rewards Eligibility Oracle testing#1289
Draft
RembrandtK wants to merge 15 commits intomainfrom
Draft
Conversation
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1289 +/- ##
=======================================
Coverage 88.55% 88.55%
=======================================
Files 75 75
Lines 4615 4615
Branches 981 981
=======================================
Hits 4087 4087
Misses 507 507
Partials 21 21
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:
|
…underflow The old code's two hooks wrote to different fields with inverted semantics: - onSubgraphSignalUpdate set accRewardsForSubgraph (A) from storage - onSubgraphAllocationUpdate set accRewardsForSubgraphSnapshot (S) from a view (storage + pending), so S leads and A lags after allocation updates After the proxy upgrade, _updateSubgraphRewards computed A.sub(S).add(P) which underflows on the intermediate A - S when A < S. Rearranging to A.add(P).sub(S) adds pending rewards first, avoiding the intermediate underflow. S <= A + P always holds because P covers T1→now (a superset of the T1→T2 gap S - A). Observed on Arbitrum Sepolia: A < S by ~7,235 GRT for subgraphs whose last pre-upgrade interaction was onSubgraphAllocationUpdate. All reward operations (signal, allocation, claim) reverted permanently.
RewardsManager and SubgraphService implementation addresses updated with new deployment metadata.
Rewards Eligibility Oracle testing
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
Hardhat tasks for managing testnet GRT: - grt:status: show governor, minter status, total supply - grt:balance: query GRT balance for any address - grt:transfer: send GRT from deployer's balance (no minter role needed) - grt:mint: mint new GRT (requires minter role)
…to config reference
- Bump address-book to 1.2.0 (REO and rewards reclaiming addresses) - Add address-book to publish workflow package choices - Add auto-tagging step to publish workflow - Add publishing guide for address-book release process
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces documentation and tooling updates to support Rewards Eligibility Oracle (REO) / issuance-upgrade testing on Arbitrum Sepolia, along with deployment workflow improvements (local network support, status/verification tasks, sync behavior adjustments) and a RewardsManager underflow fix validated by a new unit test.
Changes:
- Add comprehensive REO + rewards-condition + subgraph-denial test plans and supporting Notion tracking docs.
- Improve deployment tooling (new REO/GRT tasks, localNetwork support, status/verify ergonomics, sync/bytecode handling, deployment tagging script).
- Fix a RewardsManager snapshot inversion underflow and add a focused regression test.
Reviewed changes
Copilot reviewed 46 out of 46 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/toolshed/src/hardhat/hardhat.base.config.ts | Add keystore/env fallback logic for Etherscan/Arbiscan API key resolution. |
| packages/subgraph-service/addresses.json | Update Arbitrum Sepolia SubgraphService implementation + deployment metadata. |
| packages/issuance/docs/testing/reo/support/NotionTracker.csv | Add Notion-importable CSV tracker for REO/rewards testing progress. |
| packages/issuance/docs/testing/reo/support/NotionSetup.md | Document how to import/configure the Notion tracker database. |
| packages/issuance/docs/testing/reo/support/IssuanceAllocatorTestPlan.md | Add IssuanceAllocator-focused test plan (separate from REO). |
| packages/issuance/docs/testing/reo/SubgraphDenialTestPlan.md | Add detailed subgraph denial behavior test plan for the issuance upgrade. |
| packages/issuance/docs/testing/reo/RewardsConditionsTestPlan.md | Add detailed reclaim/POI/signal condition testing plan. |
| packages/issuance/docs/testing/reo/README.md | Add top-level navigation/structure for issuance upgrade testing docs. |
| packages/issuance/docs/testing/reo/IndexerTestGuide.md | Add condensed per-indexer eligibility testing guide (incl. mock REO path). |
| packages/issuance/docs/testing/reo/BaselineTestPlan.md | Add upgrade-agnostic baseline indexer operational test plan. |
| packages/issuance/contracts/eligibility/mocks/MockRewardsEligibilityOracle.sol | Add a simple mock REO contract for testnet/integration testing. |
| packages/horizon/addresses.json | Update RewardsManager implementation + deployment metadata for Sepolia. |
| packages/deployment/tasks/verify-contract.ts | Change missing API key handling to print guidance and return early. |
| packages/deployment/tasks/reo-tasks.ts | Add Hardhat tasks to enable/disable/status REO validation with governance fallback. |
| packages/deployment/tasks/grt-tasks.ts | Add Hardhat tasks for GRT status/balance/transfer/mint utilities. |
| packages/deployment/tasks/deployment-status.ts | Improve status task resilience by using direct HTTP RPC (env/default URLs) and clearer warnings. |
| packages/deployment/scripts/tag-deployment.sh | Add script to create annotated git tags based on address book diffs. |
| packages/deployment/scripts/debug-deploy-state.ts | Add diagnostic script for investigating “unchanged” deploy output vs bytecode. |
| packages/deployment/scripts/check-rocketh-bytecode.ts | Add diagnostic comparing rocketh stored bytecode vs local artifact. |
| packages/deployment/scripts/check-bytecode.ts | Add diagnostic comparing local/address-book/on-chain bytecode hashes. |
| packages/deployment/rocketh/config.ts | Add Graph local network chain (1337) and localNetwork environment mapping. |
| packages/deployment/lib/sync-utils.ts | Refactor bytecode-change detection and adjust proxy/impl sync + rocketh record handling. |
| packages/deployment/lib/controller-utils.ts | Add canSignAsGovernor() helper (detects if provider can sign as governor). |
| packages/deployment/lib/address-book-utils.ts | Add localNetwork mode detection and local-network address-book path resolution. |
| packages/deployment/lib/abis.ts | Update IREWARDS_MANAGER_INTERFACE_ID constant to new stable value. |
| packages/deployment/hardhat.config.ts | Wire in new tasks; add localNetwork chain descriptor/config; set HARDHAT_NETWORK for v3 localNetwork. |
| packages/deployment/docs/deploy/RewardsEligibilityOracleDeployment.md | Fix documentation link path to GovernanceWorkflow. |
| packages/deployment/docs/deploy/IssuanceAllocatorDeployment.md | Fix relative links and update referenced deployment script paths. |
| packages/deployment/docs/SyncBytecodeDetectionFix.md | Add explanation of sync bytecode detection issues and intended fixes. |
| packages/deployment/docs/LocalForkTesting.md | Document localNetwork (rem-local-network) usage and differences vs fork mode. |
| packages/deployment/docs/DeploymentSetup.md | Document localNetwork and add WIP deployment tagging workflow section. |
| packages/deployment/deploy/rewards/reclaim/04_configure.ts | Use governor signing detection to execute directly or emit governance TX batch. |
| packages/deployment/deploy/rewards/eligibility/06_integrate.ts | Allow direct execution when governor is signable; otherwise generate governance TX. |
| packages/deployment/deploy/rewards/eligibility/04_configure.ts | Use governor signing detection instead of deployer role check for direct execution. |
| packages/deployment/deploy/allocate/pilot/04_configure.ts | Switch to governance TX builder pattern with optional direct execution. |
| packages/deployment/deploy/allocate/allocator/07_activate.ts | Enable optional direct execution of activation batch when governor is signable. |
| packages/deployment/README.md | Fix/update documentation links and add LocalForkTesting reference. |
| packages/contracts/hardhat.config.ts | Add env var fallback for ARBISCAN_API_KEY in etherscan config. |
| packages/contracts/contracts/rewards/RewardsManager.sol | Fix undistributed rewards computation ordering to avoid underflow in upgrade edge case. |
| packages/contracts-test/tests/unit/rewards/rewards-snapshot-inversion.test.ts | Add regression tests that simulate snapshot inversion via direct storage writes. |
| packages/address-book/package.json | Bump @graphprotocol/address-book version to 1.2.0. |
| packages/address-book/docs/PublishingGuide.md | Add publishing/release guide for address-book package and network-monitor rollout. |
| packages/address-book/CHANGELOG.md | Add 1.2.0 changelog entry describing address-book updates. |
| docs/RewardsBehaviourChanges.md | Add functional summary of reward-behavior changes in the issuance upgrade. |
| .github/workflows/publish.yml | Allow publishing address-book and add release tag creation step. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix typo "recwards" -> "rewards" in address-book CHANGELOG - Throw error instead of silent return on missing Arbiscan API key - Update SyncBytecodeDetectionFix docs to match actual implementation
The src/issuance/ directory and symlink were not created when issuance support was added to the publish scripts, causing prepublishOnly to fail.
- Add TestnetDetails.md and MainnetDetails.md with Explorer, Gateway, network subgraph IDs, RPC, and contract addresses for each network - Correct network subgraph IDs: Graph Network Arbitrum Sepolia (3xQHhMu...) for testnet, Graph Network Arbitrum (DZz4kDT...) for mainnet - Document that testnet RewardsManager uses MockREO; explain msg.sender mechanic and cast commands for checking/setting eligibility - Standardise API key env var to GRAPH_API_KEY throughout - Move GraphQL lowercase/Unicode warning to README as general guidance - Replace inline env tables in BaselineTestPlan and README with links to the new detail files
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.
Working branch for Rewards Eligibility Oracle testing and associated deployment changes.
This branch will not be merged as is, being superseded by: #1326
For the convenience of not breaking incoming links, this branch is liable to be radically forced pushed to with updated rebased versions.