-
Notifications
You must be signed in to change notification settings - Fork 46
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
fix(subgraph): subgraph-mainfest-fix #1741
Conversation
WalkthroughThe changes involve modifications to the Changes
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-university ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Code Climate has analyzed commit accd30b and detected 0 issues on this pull request. View more on Code Climate. |
|
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
subgraph/core-neo/subgraph.yaml (1)
41-41
: Consider adding a comment explaining the receipt requirement.To improve maintainability, consider documenting why the receipt data is needed for this specific event handler.
Add a YAML comment above the receipt line:
- event: DisputeCreation(indexed uint256,indexed address) handler: handleDisputeCreation + # Receipt data required for accessing transaction details during dispute creation receipt: true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
subgraph/core-neo/subgraph.yaml
(1 hunks)subgraph/core-university/subgraph.yaml
(1 hunks)
🔇 Additional comments (3)
subgraph/core-university/subgraph.yaml (2)
41-41
: Verify the necessity of transaction receipt data.
Adding receipt: true
for the DisputeCreation event will allow access to transaction receipt data, which can impact indexing performance. Please confirm:
- Why is receipt data specifically needed for this event?
- Have you considered the indexing performance impact?
#!/bin/bash
# Description: Check for similar event handlers that might also need receipt data
# Test 1: Check if other dispute-related events in the manifest have receipt: true
echo "Checking for other dispute-related events..."
rg -A 2 "event: .*Dispute.*\n.*handler:" subgraph/core-university/subgraph.yaml
# Test 2: Check if the same event in other subgraph manifests uses receipt: true
echo "Checking for consistency across other subgraph manifests..."
fd -t f "subgraph.yaml" | xargs rg -A 2 "event: DisputeCreation.*\n.*handler:"
Line range hint 1-1
: Verify spec version upgrade requirements.
The upgrade from specVersion 0.0.3 to 0.0.4 is a breaking change. Please ensure that all required changes for 0.0.4 compatibility are implemented.
✅ Verification successful
Spec version upgrade is properly implemented
The manifest has all required components for spec version 0.0.4:
- Features section is present with
fullTextSearch
enabled - Schema file exists (symlinked to ../core/schema.graphql)
- All required sections (dataSources, mapping, etc.) are properly configured
The version is also consistent with other subgraphs in the repository, with most using 0.0.4 (only core using 0.0.5).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify if the manifest has all required elements for spec version 0.0.4
# Test 1: Check if features section is present (required in 0.0.4)
echo "Checking for features section..."
if ! grep -A 2 "features:" subgraph/core-university/subgraph.yaml; then
echo "Error: features section is missing"
exit 1
fi
# Test 2: Check if schema.graphql exists
echo "Checking for schema.graphql..."
if [ ! -f "subgraph/core-university/schema.graphql" ]; then
echo "Error: schema.graphql is missing"
exit 1
fi
Length of output: 772
Script:
#!/bin/bash
# Let's verify the manifest contents more thoroughly
# Check the entire manifest content to analyze its structure
echo "=== Manifest Content ==="
cat subgraph/core-university/subgraph.yaml
# Check if schema.graphql exists and show its presence
echo -e "\n=== Schema File Check ==="
ls -l subgraph/core-university/schema.graphql 2>/dev/null || echo "schema.graphql not found"
# Search for other subgraph manifests to compare spec versions
echo -e "\n=== Other Subgraph Manifests Spec Versions ==="
fd subgraph.yaml | xargs grep "specVersion:" || echo "No other manifests found"
Length of output: 6668
subgraph/core-neo/subgraph.yaml (1)
41-41
: Verify the necessity of accessing transaction receipts.
The addition of receipt: true
allows the handleDisputeCreation
handler to access transaction receipt data. While this is a valid configuration, it comes with a slight processing overhead.
Let's verify if the handler actually uses receipt data:
PR-Codex overview
This PR focuses on adding a
receipt: true
property to the event configurations in twosubgraph.yaml
files, which likely indicates that the subgraph should expect to receive a transaction receipt for certain events.Detailed summary
In
subgraph/core-neo/subgraph.yaml
:receipt: true
to the event configuration.In
subgraph/core-university/subgraph.yaml
:receipt: true
to the event configuration.Summary by CodeRabbit
New Features
DisputeCreation
event.specVersion
to improve compatibility and features.Bug Fixes