[Preview] Rewards Eligibility Oracle (REO) #1235

RembrandtK · 2025-10-07T07:26:40Z

Currently comparing to build-lint-upgrade, when that merged to main (via #1233) can create as new PR against main for easier audit and merge.

socket-security · 2025-10-07T07:28:01Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	@nomicfoundation/hardhat-toolbox@5.0.0
	@openzeppelin/hardhat-upgrades@3.9.1

View full report

Copilot

Pull Request Overview

This PR introduces a new Rewards Eligibility Oracle (REO) system that allows authorized oracles to manage indexer eligibility for receiving rewards on The Graph network. The system implements a "deny by default" approach where indexers must be explicitly marked as eligible by oracles to receive rewards.

Key changes:

New RewardsEligibilityOracle contract with time-based eligibility management
Integration with existing RewardsManager to check eligibility before distributing rewards
Comprehensive test suite with consolidated patterns and shared fixtures

Reviewed Changes

Copilot reviewed 34 out of 36 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
packages/issuance/contracts/eligibility/RewardsEligibilityOracle.sol	Core oracle contract implementing eligibility management with role-based access control
packages/contracts/contracts/rewards/RewardsManager.sol	Updated to integrate with eligibility oracle and check indexer eligibility before rewards
packages/interfaces/contracts/issuance/eligibility/IRewardsEligibilityOracle.sol	Interface definition for rewards eligibility oracle
packages/issuance/test/tests/RewardsEligibilityOracle.test.ts	Comprehensive test suite for the oracle contract
packages/contracts/test/tests/unit/rewards/rewards.test.ts	Enabled previously skipped eligibility oracle tests

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

packages/contracts/contracts/rewards/RewardsManagerStorage.sol

packages/issuance/test/tests/helpers/sharedFixtures.ts

packages/issuance/test/tests/helpers/fixtures.ts

packages/issuance/test/scripts/generateInterfaceIds.js

openzeppelin-code · 2025-10-07T07:43:58Z

[Preview] Rewards Eligibility Oracle (REO)

Generated at commit: 61e32c91e3adcc903f0f11b6c72253b9dceb80d9

🚨 Report Summary

	Severity Level	Results
Contracts	Critical High Medium Low Note Total	2 4 0 15 38 59
Dependencies	Critical High Medium Low Note Total	0 0 0 0 0 0

For more details view the full report in OpenZeppelin Code Inspector

codecov · 2025-10-07T07:49:28Z

Codecov Report

❌ Patch coverage is 96.55172% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.69%. Comparing base (804da6c) to head (61e32c9).
⚠️ Report is 1 commits behind head on build-lint-upgrade.

Files with missing lines	Patch %	Lines
...s/contracts/tests/MockRewardsEligibilityOracle.sol	66.66%	3 Missing ⚠️

Additional details and impacted files

@@                  Coverage Diff                   @@
##           build-lint-upgrade    #1235      +/-   ##
======================================================
+ Coverage               83.15%   83.69%   +0.53%     
======================================================
  Files                      48       51       +3     
  Lines                    2096     2183      +87     
  Branches                  620      643      +23     
======================================================
+ Hits                     1743     1827      +84     
- Misses                    353      356       +3

Flag	Coverage Δ
unittests	`83.69% <96.55%> (+0.53%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

• Consolidated duplicate test fixture files into single fixtures.ts • Removed circular dependency between fixtures.ts and sharedFixtures.ts • Fixed incorrect function calls (grantOperatorRole → grantRole, setValidityPeriod → setEligibilityPeriod) • Updated default eligibility period from 7 days to 14 days (matches contract default) • Updated all test imports to use consolidated fixtures • Fixed RewardsManagerV6Storage class documentation comment

Copilot

Pull Request Overview

Copilot reviewed 33 out of 35 changed files in this pull request and generated 7 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

packages/issuance/test/utils/testPatterns.ts

packages/issuance/test/tests/consolidated/AccessControl.test.ts

packages/issuance/test/tests/RewardsEligibilityOracle.test.ts

packages/issuance/test/scripts/generateInterfaceIds.js

packages/issuance/test/tests/helpers/fixtures.ts

packages/issuance/test/tests/RewardsEligibilityOracle.test.ts

packages/issuance/hardhat.config.cjs

• Convert require() to ES6 imports for consistency (chai, ethers) • Add descriptive comments to empty catch blocks in generateInterfaceIds.js • Add mocha configuration to hardhat.config.cjs • Remove commented-out paths configuration • Add test:coverage script to package.json • Improve error handling in fixtures.ts (console.warn → console.error + throw)

Copilot

Pull Request Overview

Copilot reviewed 34 out of 35 changed files in this pull request and generated 6 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-07T09:33:41Z

packages/issuance/test/tests/helpers/fixtures.ts

+    )
+    throw error
+  }
+}


The function resetContractState catches all errors and logs them to console, but then still throws the error. This pattern makes it difficult to understand which specific reset operation failed. Consider catching and handling specific expected errors (like role management errors) separately from unexpected errors, or provide more specific error messages.

Copilot · 2025-10-07T09:33:42Z

packages/issuance/test/tests/RewardsEligibilityOracle.test.ts

+    } catch {
+      // Role management errors during reset are non-fatal and may occur if roles are already revoked or not present.
+      // These errors are expected and can be safely ignored.
+    }


Empty catch blocks make debugging difficult. Consider catching specific expected error types (like AccessControl errors) and only silently ignore those, while allowing unexpected errors to propagate. This would help identify genuine issues during test development.

Copilot · 2025-10-07T09:33:42Z

packages/issuance/test/tests/RewardsEligibilityOracle.test.ts

+    } catch {
+      // Ignore reset errors


The comment 'Ignore reset errors' is too vague and the empty catch block hides potentially important debugging information. Consider being more specific about which errors are expected and safe to ignore, or at least log the error type for debugging purposes.

Suggested change

} catch {

// Ignore reset errors

} catch (err) {

// Errors during reset to default values (e.g., roles not present, already set values) are non-fatal and can be safely ignored.

// Logging for debugging purposes:

console.warn('Non-fatal error during RewardsEligibilityOracle state reset:', err);

Copilot · 2025-10-07T09:33:42Z

packages/contracts/contracts/rewards/RewardsManager.sol

+                require(
+                    IERC165(newRewardsEligibilityOracle).supportsInterface(type(IRewardsEligibilityOracle).interfaceId),
+                    "Contract does not support IRewardsEligibilityOracle interface"
+                );


The error message could be more descriptive by including the address that failed the interface check. Consider changing to something like 'Contract at {address} does not support IRewardsEligibilityOracle interface' to aid in debugging.

Copilot · 2025-10-07T09:33:42Z

packages/issuance/test/scripts/generateInterfaceIds.js

+              return
+            }
+          } catch {
+            // Not JSON, continue - this is expected for non-JSON output lines


While the comment explains why the error is ignored, catching all parsing errors without any logging could hide issues where the expected JSON output format changes. Consider logging non-JSON lines in verbose mode or adding a counter to ensure at least one valid JSON response was found.

Suggested change

// Not JSON, continue - this is expected for non-JSON output lines

// Not JSON, continue - this is expected for non-JSON output lines

if (!SILENT) {

console.warn(`[generateInterfaceIds] Non-JSON output line: "${line.trim()}"`);

}

Copilot · 2025-10-07T09:33:43Z

packages/issuance/test/package.json

+    "eslint": "catalog:",
+    "eslint-plugin-no-only-tests": "catalog:",
+    "ethers": "catalog:",
+    "forge-std": "https://github.com/foundry-rs/forge-std/tarball/v1.9.7",


Using a tarball URL instead of a semantic version for forge-std makes dependency management less predictable and could lead to issues with package integrity verification. Consider using a proper version specification if this dependency is available through npm/yarn registries.

Suggested change

"forge-std": "https://github.com/foundry-rs/forge-std/tarball/v1.9.7",

"forge-std": "^1.9.7",

RembrandtK · 2025-10-07T11:49:58Z

See also: graph-improvement-proposals PR#80 GIP-0079: Indexer Rewards Eligibility Oracle (REO)

feat: add Rewards Eligibility Oracle (REO)

fed7913

RembrandtK mentioned this pull request Oct 7, 2025

Rewards Eligibility Oracle (REO) #1221

Closed

RembrandtK requested a review from Copilot October 7, 2025 07:29

Copilot AI reviewed Oct 7, 2025

View reviewed changes

RembrandtK changed the title ~~[Placeholder] Rewards Eligibility Oracle (REO)~~ [Preview] Rewards Eligibility Oracle (REO) Oct 7, 2025

RembrandtK requested a review from Copilot October 7, 2025 07:54

Copilot AI reviewed Oct 7, 2025

View reviewed changes

RembrandtK self-assigned this Oct 7, 2025

RembrandtK added 3 commits October 7, 2025 09:42

feat: add test:coverage script to packages/contracts

e198a3b

chore: not sure if this file format is stable or keeps changing

61e32c9

RembrandtK requested a review from Copilot October 7, 2025 09:31

Copilot AI reviewed Oct 7, 2025

View reviewed changes

RembrandtK mentioned this pull request Oct 7, 2025

GIP-0079: Indexer Rewards Eligibility Oracle (REO) graphprotocol/graph-improvement-proposals#80

Open

-    } catch {
-      // Ignore reset errors
+    } catch (err) {
+      // Errors during reset to default values (e.g., roles not present, already set values) are non-fatal and can be safely ignored.
+      // Logging for debugging purposes:
+      console.warn('Non-fatal error during RewardsEligibilityOracle state reset:', err);

	"forge-std": "https://github.com/foundry-rs/forge-std/tarball/v1.9.7",
	"forge-std": "^1.9.7",

[Preview] Rewards Eligibility Oracle (REO) #1235

Are you sure you want to change the base?

[Preview] Rewards Eligibility Oracle (REO) #1235

Conversation

RembrandtK commented Oct 7, 2025

Uh oh!

socket-security bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

openzeppelin-code bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

[Preview] Rewards Eligibility Oracle (REO)

🚨 Report Summary

Uh oh!

codecov bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

RembrandtK commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

socket-security bot commented Oct 7, 2025 •

edited

Loading

openzeppelin-code bot commented Oct 7, 2025 •

edited

Loading

codecov bot commented Oct 7, 2025 •

edited

Loading

RembrandtK commented Oct 7, 2025 •

edited

Loading