Skip to content

Service Quality Oracle #1214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 6 commits into
base: baseline-upgrade
Choose a base branch
from
Draft

Service Quality Oracle #1214

wants to merge 6 commits into from

Conversation

RembrandtK
Copy link
Contributor

Draft PR for Service Quality Oracle. Details to be added.

This PR is likely to be replaced by one directly merging to main, after baseline-upgrade has been merged, that will need to be audited.

- Remove restrictive rootDir setting that prevented imports from parent directories
- Set rootDir to '..' to allow access to parent contracts types
- Fixes CI build failure in packages/contracts/task TypeScript compilation

Resolves TypeScript error TS6059 where files outside rootDir were being imported
@RembrandtK RembrandtK requested a review from Copilot August 14, 2025 14:56
Copy link

socket-security bot commented Aug 14, 2025

Copilot

This comment was marked as outdated.

Copy link

openzeppelin-code bot commented Aug 14, 2025

Service Quality Oracle

Generated at commit: e882dc87fd999b18f431ff2bdf20185832147a41

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
2
4
0
15
35
56
Dependencies Critical
High
Medium
Low
Note
Total
0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

@RembrandtK RembrandtK requested a review from Copilot August 15, 2025 11:52
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a Service Quality Oracle system for The Graph protocol, implementing time-based indexer eligibility for rewards based on service quality assessments. The system operates on a "deny by default" principle where indexers must be explicitly approved by authorized oracles to receive rewards.

  • Implementation of ServiceQualityOracle contract with role-based access control and time-based approval mechanism
  • Integration with RewardsManager to check indexer eligibility before distributing rewards
  • Comprehensive test suite covering access control, oracle management, and reward distribution scenarios

Reviewed Changes

Copilot reviewed 36 out of 37 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
packages/issuance/contracts/quality/ServiceQualityOracle.sol Core oracle contract implementation with ERC-7201 storage and role-based access
packages/issuance/contracts/common/BaseUpgradeable.sol Base upgradeable contract with standardized access control patterns
packages/contracts/contracts/rewards/RewardsManager.sol Integration of service quality checks into reward distribution logic
packages/common/contracts/quality/IServiceQualityOracle.sol Interface definition for service quality oracle functionality
packages/issuance/test/tests/ServiceQualityOracle.test.ts Comprehensive test suite for oracle functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

"clean": "rm -rf build",
"test": "pnpm test:self",
"test:self": "pnpm test:hardhat",
"test:hardhat": "pnpm --filter @graphprotocol/contracts --filter @graphprotocol/issuance --filter @graphprotocol/sdk build && hardhat test tests/*.test.ts tests/**/*.test.ts",
Copy link
Preview

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test glob pattern 'tests/.test.ts tests/**/.test.ts' does not match the actual test file locations. Based on the diff, test files are in 'test/tests/' directory, so the pattern should be 'test/tests/.test.ts test/tests/**/.test.ts'.

Suggested change
"test:hardhat": "pnpm --filter @graphprotocol/contracts --filter @graphprotocol/issuance --filter @graphprotocol/sdk build && hardhat test tests/*.test.ts tests/**/*.test.ts",
"test:hardhat": "pnpm --filter @graphprotocol/contracts --filter @graphprotocol/issuance --filter @graphprotocol/sdk build && hardhat test test/tests/*.test.ts test/tests/**/*.test.ts",

Copilot uses AI. Check for mistakes.

await serviceQualityOracle.connect(governor).grantOperatorRole(governor.address)
await serviceQualityOracle.connect(governor).setValidityPeriod(validityPeriod)
// Now revoke the operator role from governor to ensure tests start with clean state
await serviceQualityOracle.connect(governor).revokeOperatorRole(governor.address)
Copy link
Preview

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method 'grantOperatorRole' does not exist on the ServiceQualityOracle contract. It should use the standard AccessControl method 'grantRole(OPERATOR_ROLE, governor.address)' instead.

Suggested change
await serviceQualityOracle.connect(governor).revokeOperatorRole(governor.address)
const OPERATOR_ROLE = await serviceQualityOracle.OPERATOR_ROLE();
await serviceQualityOracle.connect(governor).grantRole(OPERATOR_ROLE, governor.address);
await serviceQualityOracle.connect(governor).setValidityPeriod(validityPeriod)
// Now revoke the operator role from governor to ensure tests start with clean state
await serviceQualityOracle.connect(governor).revokeRole(OPERATOR_ROLE, governor.address);

Copilot uses AI. Check for mistakes.

Copy link

socket-security bot commented Aug 15, 2025

All alerts resolved. Learn more about Socket for GitHub.

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report

@RembrandtK RembrandtK self-assigned this Aug 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant