feat: subgraph support for shutter disputekit in devnet #1966
Conversation
WalkthroughThe changes introduce dynamic handling of dispute kit IDs throughout the subgraph code, replacing hardcoded values. Entity ID construction and event processing now extract the dispute kit ID from event data or related entities. A new data source for the Shutter dispute kit is added, and relevant function signatures are updated for flexibility and safety. Changes
Sequence Diagram(s)sequenceDiagram
participant Event
participant Utils
participant Dispute
participant Round
participant ClassicDispute
participant ClassicRound
Event->>Utils: extractDisputeKitIDFromExtraData(_extraData)
Utils-->>Event: disputeKitID
Event->>ClassicDispute: createClassicDisputeFromEvent(event, disputeKitID, roundIndex)
ClassicDispute->>ClassicDispute: Construct ID using disputeKitID
Event->>ClassicRound: createClassicRound(disputeID, choices, roundIndex, disputeKitID)
ClassicRound->>ClassicRound: Construct ID using disputeKitID
Event->>Dispute: Load Dispute by coreDisputeID
Dispute->>Round: Load Round by disputeID and roundIndex
Round-->>Event: disputeKitID for further entity ID construction
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
subgraph/core/src/entities/ClassicRound.ts (2)
6-13: Missing JSDoc / comments after signature change
createClassicRoundnow acceptsdisputeKitID, but the function header or surrounding docs were not updated. A brief comment clarifying the ID format (<disputeKitID>-<coreDisputeID>-<roundIndex>) will save future readers time.
85-97: PersistclassicRoundafter answer update only onceInside
updateChoiceFundingFromContributionEventyouanswer.save()and thenclassicRound.save()immediately after.
BecauseclassicRoundisn’t modified in this function, the lattersave()is unnecessary and costs an extra store write.- answer.save(); - classicRound.save(); + answer.save(); // classicRound untouchedsubgraph/core/src/DisputeKitClassic.ts (2)
34-38: Consider caching the extracted dispute-kit ID
extractDisputeKitIDFromExtraDatais inexpensive, yet the value never changes for this dispute.
Persisting it once in theDisputeorClassicDisputeentity (e.g.,disputeKitfield) would let subsequent handlers fetch it directly instead of re-computing / re-loadingRoundeach time, reducing store look-ups.
41-50: Repeated pattern → extract helperThe sequence
load Dispute → load current Round → read disputeKitis duplicated in several handlers (handleCommitCast,handleVoteCast,handleChoiceFunded,handleWithdrawal). Factor it into a small utility such as:export function getDisputeKitID(coreDisputeID: string): string | null { ... }to improve readability and reduce the chance of future inconsistencies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
subgraph/core/src/DisputeKitClassic.ts(8 hunks)subgraph/core/src/KlerosCore.ts(1 hunks)subgraph/core/src/entities/ClassicContribution.ts(1 hunks)subgraph/core/src/entities/ClassicDispute.ts(1 hunks)subgraph/core/src/entities/ClassicRound.ts(2 hunks)subgraph/core/src/utils.ts(1 hunks)subgraph/core/subgraph.yaml(1 hunks)subgraph/package.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
subgraph/core/src/DisputeKitClassic.ts (4)
subgraph/core/src/KlerosCore.ts (1)
handleDisputeCreation(77-97)subgraph/core/src/utils.ts (3)
extractDisputeKitIDFromExtraData(6-10)ZERO(3-3)ONE(4-4)subgraph/core/src/entities/ClassicDispute.ts (1)
createClassicDisputeFromEvent(6-15)subgraph/core/src/entities/ClassicRound.ts (1)
createClassicRound(6-25)
🪛 Biome (1.9.4)
subgraph/core/src/utils.ts
[error] 1-1: Do not shadow the global "BigInt" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (8)
subgraph/core/src/utils.ts (2)
1-1: Biome static analysis issue can be safely ignored.The static analysis reports a potential issue with shadowing the global
BigIntproperty, but this is a standard and necessary pattern in The Graph's AssemblyScript SDK. This import is correct and follows the standard practices.🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Do not shadow the global "BigInt" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
6-10: Solid utility function for extracting dispute kit IDs.The function correctly extracts the dispute kit ID by:
- Getting the last 32 bytes from the extraData
- Reversing the byte order (little-endian to big-endian)
- Converting to a BigInt and returning as string
This approach enables dynamic handling of dispute kit IDs throughout the subgraph.
subgraph/package.json (1)
3-3: Version bump is appropriate for this feature addition.The version increment from 0.15.2 to 0.16.0 follows semantic versioning principles, correctly indicating a new feature (adding support for ShutterDisputeKit).
subgraph/core/src/KlerosCore.ts (1)
159-159: Updated comment accurately reflects expanded dispute kit support.The comment now correctly indicates that the ClassicVote entity works for both the ClassicDisputeKit and the new ShutterDisputeKit, aligning with the PR's goal of supporting multiple dispute kits.
subgraph/core/src/entities/ClassicDispute.ts (2)
1-1: Added necessary Bytes import.The import of
Bytesis now required since this file interacts with the new utility function that processes byte data.
6-8: Well-implemented dynamic dispute kit ID handling.The function signature now accepts a
disputeKitIDparameter and properly uses it to construct the ClassicDispute entity ID, replacing the previously hardcoded ID prefix. This change enables consistent entity identification across multiple dispute kits.subgraph/core/src/entities/ClassicRound.ts (1)
74-86:❌ Incorrect review comment
Null-check
disputeKitIDbefore using itSame concern as in
ClassicContribution: ifcoreRound.disputeKitisnullthe derivedroundIDis malformed.- const disputeKitID = coreRound.disputeKit; + const disputeKitID = coreRound.disputeKit; + if (!disputeKitID) return; // abort – underlying data is incomplete
Here’s some targeted searching to confirm whether the
disputeKitfield onRoundcan actually benull(and whether a guard is needed):
🏁 Script executed:
#!/bin/bash # 1) Check GraphQL schema for Round entity and disputeKit nullability rg "type Round" -n -g "*.graphql" rg "disputeKit" -n -g "*.graphql" # 2) Inspect the generated TS entity for Round to see how disputeKit is typed rg "class Round" -n rg "disputeKit" -C3 -nLength of output: 96599
DisputeKitID Is Always Defined – No Guard Needed
Based on
schema.graphqland the generatedRoundentity,disputeKitis non-nullable:
- In
schema.graphql:
type Round @entity { … disputeKit: DisputeKit! … }- In
subgraph/core/src/entities/Round.ts:It’s always set before the entity is saved, and the generated AssemblyScriptround.disputeKit = roundInfo.disputeKitID.toString();get disputeKit(): stringcannot returnnull.The same pattern is used throughout (e.g. in
ClassicContribution.ts) without issue. You can safely omit the additionalif (!disputeKitID) return;check here.Likely an incorrect or invalid review comment.
subgraph/core/subgraph.yaml (1)
123-158: Validate ABI/event signature parity between Classic & Shutter
DisputeKitShutterre-uses theDisputeKitClassic.tshandlers. This will only work if every event signature in the Shutter ABI exactly matches the Classic ABI.Please double-check that:
- The ABI file at
../../contracts/deployments/arbitrumSepoliaDevnet/DisputeKitShutter.jsoncontains all six events with identical parameter ordering/types.- The
startBlock(148 194 178) predates the first emitted event; otherwise initial events will be missed.A quick grep / diff against the Classic ABI before deploying would prevent runtime failures.
| const coreDispute = Dispute.load(coreDisputeID); | ||
| if (!coreDispute) return null; | ||
|
|
||
| const roundId = `${coreDisputeID}-${coreRoundIndex}`; | ||
| const coreRound = Round.load(roundId); | ||
| if (!coreRound) return null; | ||
|
|
||
| const disputeKitID = coreRound.disputeKit; | ||
|
|
||
| const roundID = `${disputeKitID}-${coreDisputeID}-${coreRoundIndex}`; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Guard against a missing disputeKit reference
coreRound.disputeKit is assumed to be present and non-empty. If the Round entity was created before the refactor (or gets corrupted), this will evaluate to null and the constructed roundID becomes "null-<coreDisputeID>-<coreRoundIndex>", causing silent data fragmentation.
- const disputeKitID = coreRound.disputeKit;
+ const disputeKitID = coreRound.disputeKit;
+ if (!disputeKitID) {
+ // Inconsistent data – abort processing to avoid polluting the store
+ return null;
+ }Fail-fast avoids persisting wrongly-keyed entities and makes the issue visible during indexing.
🏁 Script executed:
#!/bin/bash
echo "=== GraphQL schema files ==="
find . -type f -name "*.graphql"
echo ""
echo "=== 'type Round' and 'disputeKit' in GraphQL schemas ==="
rg -n "type Round" -C2 -g "*.graphql" || true
rg -n "disputeKit" -C2 -g "*.graphql" || true
echo ""
echo "=== Locate and inspect Round entity class ==="
round_files=$(rg -l "export class Round" -g "*.ts")
echo "Found Round.ts files:"
echo "$round_files"
for file in $round_files; do
echo ""
echo "------ $file ------"
rg -n "export class Round" -C3 "$file"
rg -n "get disputeKit" -C5 "$file" || true
doneLength of output: 3678
Guard against a missing disputeKit reference
The GraphQL schema defines Round.disputeKit as non-nullable, but legacy or corrupted data may still omit it. Add a null check immediately after loading the Round to avoid constructing IDs like "null-<coreDisputeID>-<coreRoundIndex>" and silently polluting the store.
• File: subgraph/core/src/entities/ClassicContribution.ts
• Context: right after assigning disputeKitID
- const disputeKitID = coreRound.disputeKit;
+ const disputeKitID = coreRound.disputeKit;
+ if (!disputeKitID) {
+ // Inconsistent data – abort processing to avoid polluting the store
+ return null;
+ }This fail-fast guard ensures any missing disputeKit is surfaced during indexing rather than resulting in fragmented or mis-keyed entities.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const coreDispute = Dispute.load(coreDisputeID); | |
| if (!coreDispute) return null; | |
| const roundId = `${coreDisputeID}-${coreRoundIndex}`; | |
| const coreRound = Round.load(roundId); | |
| if (!coreRound) return null; | |
| const disputeKitID = coreRound.disputeKit; | |
| const roundID = `${disputeKitID}-${coreDisputeID}-${coreRoundIndex}`; | |
| const coreDispute = Dispute.load(coreDisputeID); | |
| if (!coreDispute) return null; | |
| const roundId = `${coreDisputeID}-${coreRoundIndex}`; | |
| const coreRound = Round.load(roundId); | |
| if (!coreRound) return null; | |
| const disputeKitID = coreRound.disputeKit; | |
| if (!disputeKitID) { | |
| // Inconsistent data – abort processing to avoid polluting the store | |
| return null; | |
| } | |
| const roundID = `${disputeKitID}-${coreDisputeID}-${coreRoundIndex}`; |
| const coreDispute = Dispute.load(coreDisputeID); | ||
| if (!coreDispute) return; | ||
|
|
||
| const roundId = `${coreDisputeID}-${coreRoundIndex}`; | ||
| const coreRound = Round.load(roundId); | ||
| if (!coreRound) return; | ||
| const disputeKitID = coreRound.disputeKit; | ||
|
|
||
| const roundID = `${disputeKitID}-${coreDisputeID}-${coreRoundIndex}`; |
There was a problem hiding this comment.
Defensive check for disputeKitID
Replicating the guard here avoids malformed IDs during withdrawals.
const disputeKitID = coreRound.disputeKit;
-
+ if (!disputeKitID) return;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const coreDispute = Dispute.load(coreDisputeID); | |
| if (!coreDispute) return; | |
| const roundId = `${coreDisputeID}-${coreRoundIndex}`; | |
| const coreRound = Round.load(roundId); | |
| if (!coreRound) return; | |
| const disputeKitID = coreRound.disputeKit; | |
| const roundID = `${disputeKitID}-${coreDisputeID}-${coreRoundIndex}`; | |
| const coreDispute = Dispute.load(coreDisputeID); | |
| if (!coreDispute) return; | |
| const roundId = `${coreDisputeID}-${coreRoundIndex}`; | |
| const coreRound = Round.load(roundId); | |
| if (!coreRound) return; | |
| const disputeKitID = coreRound.disputeKit; | |
| if (!disputeKitID) return; | |
| const roundID = `${disputeKitID}-${coreDisputeID}-${coreRoundIndex}`; |
| const coreDispute = Dispute.load(coreDisputeID); | ||
| if (!coreDispute) return; | ||
|
|
||
| const roundId = `${coreDisputeID}-${coreRoundIndex}`; | ||
| const coreRound = Round.load(roundId); | ||
| if (!coreRound) return; | ||
| const disputeKitID = coreRound.disputeKit; | ||
|
|
||
| const roundID = `${disputeKitID}-${coreDisputeID}-${coreRoundIndex}`; |
There was a problem hiding this comment.
Early exit when disputeKitID missing
Same defensive check as earlier – without it, roundID may start with "null-".
const disputeKitID = coreRound.disputeKit;
-
+ if (!disputeKitID) return;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const coreDispute = Dispute.load(coreDisputeID); | |
| if (!coreDispute) return; | |
| const roundId = `${coreDisputeID}-${coreRoundIndex}`; | |
| const coreRound = Round.load(roundId); | |
| if (!coreRound) return; | |
| const disputeKitID = coreRound.disputeKit; | |
| const roundID = `${disputeKitID}-${coreDisputeID}-${coreRoundIndex}`; | |
| const coreDispute = Dispute.load(coreDisputeID); | |
| if (!coreDispute) return; | |
| const roundId = `${coreDisputeID}-${coreRoundIndex}`; | |
| const coreRound = Round.load(roundId); | |
| if (!coreRound) return; | |
| const disputeKitID = coreRound.disputeKit; | |
| if (!disputeKitID) return; | |
| const roundID = `${disputeKitID}-${coreDisputeID}-${coreRoundIndex}`; |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
subgraph/core/src/entities/ClassicDispute.ts (1)
5-14: Consider adding documentation for improved maintainability.While the code changes are well-implemented, adding function-level documentation that explains the purpose of the new parameters would improve maintainability, especially for new developers who might work on this codebase in the future.
+/** + * Creates a ClassicDispute entity from a DisputeCreation event + * @param event The DisputeCreation event + * @param disputeKitID The ID of the dispute kit (e.g., "1" for Classic, "2" for Shutter) + * @param roundIndex The initial round index for this dispute + */ export function createClassicDisputeFromEvent(event: DisputeCreation, disputeKitID: string, roundIndex: BigInt): void { const coreDisputeID = event.params._coreDisputeID.toString(); const classicDispute = new ClassicDispute(`${disputeKitID}-${coreDisputeID}`); classicDispute.coreDispute = coreDisputeID; classicDispute.currentLocalRoundIndex = roundIndex; classicDispute.numberOfChoices = event.params._numberOfChoices; classicDispute.extraData = event.params._extraData; classicDispute.timestamp = event.block.timestamp; classicDispute.save(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
subgraph/core/src/DisputeKitClassic.ts(8 hunks)subgraph/core/src/entities/ClassicDispute.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- subgraph/core/src/DisputeKitClassic.ts
🧰 Additional context used
🪛 Biome (1.9.4)
subgraph/core/src/entities/ClassicDispute.ts
[error] 1-1: Do not shadow the global "BigInt" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: SonarCloud
🔇 Additional comments (4)
subgraph/core/src/entities/ClassicDispute.ts (4)
1-1: Note on static analysis warning about BigInt import.The static analysis warning about shadowing the global "BigInt" property can be safely ignored in this context. This is standard practice in The Graph's subgraph development, where we import the specialized
BigInttype from the graph-ts library.🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Do not shadow the global "BigInt" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
5-5: Good enhancement: Added parameters for flexible dispute kit handling.Adding the
disputeKitIDandroundIndexparameters to the function signature is a good enhancement that improves code flexibility by removing hardcoded values. This change aligns with the PR objective to support multiple dispute kits, specifically the DisputeKitShutter.
7-7: Proper update of entity ID to use dynamic dispute kit ID.Replacing the previously hardcoded prefix with a dynamic
disputeKitIDparameter is a necessary change to support multiple dispute kits. This ensures unique IDs across different dispute kit implementations while maintaining a consistent entity structure.
9-9: Good improvement: Dynamic initialization of currentLocalRoundIndex.Using the provided
roundIndexparameter to initializecurrentLocalRoundIndexinstead of a hardcoded value improves flexibility and supports different dispute kit implementations that might start with different round indices.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
subgraph/core/src/DisputeKitClassic.ts (2)
38-52: Same guard needed inhandleCommitCastThe previous review already highlighted the risk of generating IDs that start with
"null-"; the safeguard still hasn’t been added here.
Please short-circuit whendisputeKitIDis falsy.
73-82: Repeat of missingdisputeKitIDvalidation inhandleVoteCastThis block is vulnerable to the exact same issue as above – add the early exit to prevent malformed IDs.
| const coreDispute = Dispute.load(coreDisputeID); | ||
| if (!coreDispute) return; | ||
|
|
||
| const roundId = `${coreDisputeID}-${coreRoundIndex}`; | ||
| const coreRound = Round.load(roundId); | ||
| if (!coreRound) return; | ||
| const disputeKitID = coreRound.disputeKit; | ||
|
|
||
| const roundID = `${disputeKitID}-${coreDisputeID}-${coreRoundIndex}`; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing defensive check in handleWithdrawal
coreRound.disputeKit can be empty for legacy rounds or if an earlier handler failed to set it.
Add the same guard used elsewhere to avoid "null-" IDs and the subsequent failed ClassicRound.load.
|
|
||
| const localDispute = ClassicDispute.load(`${disputeKitID}-${coreDisputeID}`); | ||
| if (!localDispute) return; | ||
| const newRoundIndex = localDispute.currentLocalRoundIndex.plus(ONE); | ||
| const numberOfChoices = localDispute.numberOfChoices; | ||
| localDispute.currentLocalRoundIndex = newRoundIndex; | ||
| localDispute.save(); | ||
| createClassicRound(coreDisputeID, numberOfChoices, newRoundIndex); | ||
|
|
||
| if (BigInt.fromString(disputeKitID) === newDisputeKitID) { | ||
| const newRoundIndex = localDispute.currentLocalRoundIndex.plus(ONE); | ||
| const numberOfChoices = localDispute.numberOfChoices; | ||
| localDispute.currentLocalRoundIndex = newRoundIndex; | ||
| localDispute.save(); | ||
| createClassicRound(coreDisputeID, numberOfChoices, newRoundIndex, disputeKitID); | ||
| } |
There was a problem hiding this comment.
Incorrect BigInt comparison – use .equals() instead of ===
BigInt is a reference type in AssemblyScript; === compares object identity, not numeric value, and will always return false for two distinct instances representing the same number.
- if (BigInt.fromString(disputeKitID) === newDisputeKitID) {
+ if (BigInt.fromString(disputeKitID).equals(newDisputeKitID)) {Without this fix, the new round will never be created after an appeal when the dispute kit stays the same, breaking the appeal flow.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const localDispute = ClassicDispute.load(`${disputeKitID}-${coreDisputeID}`); | |
| if (!localDispute) return; | |
| const newRoundIndex = localDispute.currentLocalRoundIndex.plus(ONE); | |
| const numberOfChoices = localDispute.numberOfChoices; | |
| localDispute.currentLocalRoundIndex = newRoundIndex; | |
| localDispute.save(); | |
| createClassicRound(coreDisputeID, numberOfChoices, newRoundIndex); | |
| if (BigInt.fromString(disputeKitID) === newDisputeKitID) { | |
| const newRoundIndex = localDispute.currentLocalRoundIndex.plus(ONE); | |
| const numberOfChoices = localDispute.numberOfChoices; | |
| localDispute.currentLocalRoundIndex = newRoundIndex; | |
| localDispute.save(); | |
| createClassicRound(coreDisputeID, numberOfChoices, newRoundIndex, disputeKitID); | |
| } | |
| const localDispute = ClassicDispute.load(`${disputeKitID}-${coreDisputeID}`); | |
| if (!localDispute) return; | |
| if (BigInt.fromString(disputeKitID).equals(newDisputeKitID)) { | |
| const newRoundIndex = localDispute.currentLocalRoundIndex.plus(ONE); | |
| const numberOfChoices = localDispute.numberOfChoices; | |
| localDispute.currentLocalRoundIndex = newRoundIndex; | |
| localDispute.save(); | |
| createClassicRound(coreDisputeID, numberOfChoices, newRoundIndex, disputeKitID); | |
| } |
| const disputeKitID = extractDisputeKitIDFromExtraData(event.params._extraData); | ||
|
|
||
| const disputeKitClassic = DisputeKitClassic.bind(event.address); | ||
| const klerosCore = KlerosCore.bind(disputeKitClassic.core()); | ||
| const totalRounds = klerosCore.getNumberOfRounds(event.params._coreDisputeID); | ||
| const newRoundIndex = totalRounds.minus(ONE); | ||
|
|
||
| createClassicDisputeFromEvent(event, disputeKitID, newRoundIndex); | ||
| createClassicRound(disputeID, event.params._numberOfChoices, newRoundIndex, disputeKitID); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a null / empty check for disputeKitID before composing entity IDs
extractDisputeKitIDFromExtraData() can legitimately fail (e.g. malformed or zero-length extraData) and return an empty string.
Using that value immediately creates IDs such as "null-123" which later break .load() look-ups and triggers hard-to-trace “entity not found” errors.
- const disputeKitID = extractDisputeKitIDFromExtraData(event.params._extraData);
+ const disputeKitID = extractDisputeKitIDFromExtraData(event.params._extraData);
+ if (!disputeKitID) {
+ // Guard against malformed events – nothing to index
+ return;
+ }This defensive guard is especially important now that the same code path indexes multiple kits.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const disputeKitID = extractDisputeKitIDFromExtraData(event.params._extraData); | |
| const disputeKitClassic = DisputeKitClassic.bind(event.address); | |
| const klerosCore = KlerosCore.bind(disputeKitClassic.core()); | |
| const totalRounds = klerosCore.getNumberOfRounds(event.params._coreDisputeID); | |
| const newRoundIndex = totalRounds.minus(ONE); | |
| createClassicDisputeFromEvent(event, disputeKitID, newRoundIndex); | |
| createClassicRound(disputeID, event.params._numberOfChoices, newRoundIndex, disputeKitID); | |
| } | |
| const disputeKitID = extractDisputeKitIDFromExtraData(event.params._extraData); | |
| if (!disputeKitID) { | |
| // Guard against malformed events – nothing to index | |
| return; | |
| } | |
| const disputeKitClassic = DisputeKitClassic.bind(event.address); | |
| const klerosCore = KlerosCore.bind(disputeKitClassic.core()); | |
| const totalRounds = klerosCore.getNumberOfRounds(event.params._coreDisputeID); | |
| const newRoundIndex = totalRounds.minus(ONE); | |
| createClassicDisputeFromEvent(event, disputeKitID, newRoundIndex); | |
| createClassicRound(disputeID, event.params._numberOfChoices, newRoundIndex, disputeKitID); | |
| } |
…ata sources not deployed e.g. DisputeKitShutter on testnet
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
subgraph/core/subgraph.template.yaml (2)
132-139: Consolidate shared entity and ABI definitions
The Classic and Shutter dispute kit data sources share identicalentitiesandabisblocks. To reduce duplication acrosssubgraph.template.yamlfiles, consider extracting these common definitions into a reusable snippet or templating function.
155-158: Clarify mapping file reuse
The Shutter subgraph references./src/DisputeKitClassic.tsas its mapping file, which could be confusing. Consider renaming or aliasing the AssemblyScript file (e.g.,DisputeKitShutter.ts) to improve discoverability and maintainability.subgraph/core-neo/subgraph.template.yaml (2)
137-143: Extract common configuration blocks
Classic and Shutter kits in the Neo manifest duplicate the same handlers and entity definitions. Consider templating or script-generating these sections to prevent drift and simplify future additions.
152-153: Disambiguate mapping file naming
ReferencingDisputeKitClassic.tsfor the Shutter data source may confuse future readers. A mapping file namedDisputeKitShutter.tswould better reflect its purpose.subgraph/scripts/update.sh (4)
3-3: Separate assignment for clarity
TheSCRIPT_DIRassignment embeds command substitution within the declaration. For readability and to avoid masking errors, consider splitting this into separate declaration and assignment steps.
13-13: Quote command substitution to prevent splitting
The unquoted$(yq ...)in thebasenameinvocation may split on whitespace in paths. Wrap it in quotes to ensure the full string is treated as a single argument.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 13-13: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 13-13: Quote this to prevent word splitting.
(SC2046)
49-52: Quote loop variables to avoid globbing
In thefor f in $(yq …)loop and subsequentsed/yqcalls, unquoted$fcan lead to word splitting or globbing. Use an array or quote$fto handle filenames robustly.
79-79: Quote array addition to prevent splitting
The expressionnotDeployedSourceIndices+=($i)may misbehave if$iever contains whitespace. PrefernotDeployedSourceIndices+=("$i")to ensure each index is added correctly.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 79-79: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
subgraph/core-university/subgraph.template.yaml (2)
7-28: Document placeholder replacement process
The fieldsnetwork,address, andstartBlockare left as_PLACEHOLDER_. It would be helpful to add a top‐of‐file comment or reference a script in your CI/docs to explain how these placeholders should be populated for different environments.
84-117: Add DisputeKitShutter data source template
This PR adds support for the Shutter DisputeKit on Devnet. Consider including a template section here, similar toDisputeKitClassic, so users can plug in the Shutter contract details:+ - kind: ethereum + name: DisputeKitShutter + network: _PLACEHOLDER_ + source: + address: "_PLACEHOLDER_" + abi: DisputeKitShutter + startBlock: _PLACEHOLDER_ + mapping: + kind: ethereum/events + apiVersion: 0.0.7 + language: wasm/assemblyscript + entities: + - ShutterDispute + - ShutterRound + abis: + - name: DisputeKitShutter + file: ../../contracts/deployments/_PLACEHOLDER_/DisputeKitShutterUniversity.json + - name: KlerosCore + file: ../../contracts/deployments/_PLACEHOLDER_/KlerosCoreUniversity.json + eventHandlers: + - event: DisputeCreation(indexed uint256,uint256,uint8) + handler: handleShutterDisputeCreation + # Add other Shutter-specific events as needed + file: ./src/DisputeKitShutter.ts
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (10)
subgraph/core-neo/subgraph.template.yaml(1 hunks)subgraph/core-neo/subgraph.yaml(1 hunks)subgraph/core-university/subgraph.template.yaml(1 hunks)subgraph/core-university/subgraph.yaml(3 hunks)subgraph/core/subgraph.template.yaml(1 hunks)subgraph/core/subgraph.yaml(2 hunks)subgraph/dispute-template-registry/subgraph.template.yaml(1 hunks)subgraph/dispute-template-registry/subgraph.yaml(1 hunks)subgraph/scripts/all.sh(1 hunks)subgraph/scripts/update.sh(3 hunks)
✅ Files skipped from review due to trivial changes (3)
- subgraph/core-neo/subgraph.yaml
- subgraph/dispute-template-registry/subgraph.yaml
- subgraph/dispute-template-registry/subgraph.template.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- subgraph/core/subgraph.yaml
🧰 Additional context used
🪛 Shellcheck (0.10.0)
subgraph/scripts/update.sh
[warning] 13-13: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 13-13: Quote this to prevent word splitting.
(SC2046)
[warning] 79-79: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
🔇 Additional comments (11)
subgraph/core/subgraph.template.yaml (1)
123-131: Add DisputeKitShutter data source
The newDisputeKitShuttersection correctly mirrors the Classic kit configuration, enabling event handling for the Shutter dispute kit in Devnet. Ensure that the shared mapping inDisputeKitClassic.tsdynamically dispatches based on the extracted disputeKitID from event data.subgraph/core-neo/subgraph.template.yaml (1)
119-127: Introduce DisputeKitShutter for Neo subgraph
TheDisputeKitShutterdata source is correctly added alongside the Classic kit, maintaining consistency with the core manifest. This ensures the Neo deployment can index Shutter dispute kit events.subgraph/scripts/all.sh (1)
13-17: Include new subgraphs in the build loop
Thefor subgraph in core core-neo core-university drtupdate ensures that the newly addedcore-neoandcore-universitymanifests are built and deployed alongside existing ones.subgraph/core-university/subgraph.yaml (2)
68-70: Update PolicyRegistry address and startBlock
ThePolicyRegistrycontract address andstartBlockforarbitrum-sepoliahave been updated to the University deployment values. Please confirm these match the intended on-chain deployment.
123-127: Update EvidenceModule address and startBlock
TheEvidenceModuleaddressandstartBlockare now configured for the University environment. Recommend verifying against the actual deployment to ensure accurate indexing.subgraph/core-university/subgraph.template.yaml (6)
1-5: ValidatespecVersionand schema path
Ensure that the specifiedspecVersion: 0.0.5is supported by your Graph Node setup and that./schema.graphqlcorrectly resolves at runtime.
29-35: Verify ABI file paths
Confirm that the relative paths underabis:(e.g.,../../contracts/deployments/_PLACEHOLDER_/SortitionModuleUniversity.json) match your mono-repo structure and that the placeholders are replaced correctly during deployment.
36-62: Confirm event signatures and handler mappings
Double-check that eachevent:signature exactly matches the on-chain ABI (includingindexedparameters and their order) and that the AssemblyScript mapping functions (e.g.,handleDisputeCreation) exist with the correct signatures.
63-83: Review PolicyRegistry configuration
ThePolicyRegistrydata source currently only indexes theCourtentity viaPolicyUpdateevents. Verify whether other entities or events fromPolicyRegistryshould be captured in this subgraph.
118-138: Check EvidenceModule entity alignment
Make sure that the entitiesClassicEvidenceGroupandClassicEvidencecorrespond exactly to whathandleEvidenceEventemits in AssemblyScript, and that no additional fields or groups are missing.
139-160: Standardize SortitionModule event handlers
Verify the signatures ofStakeLockedandStakeSetagainst the on-chain contract ABI (parameter types/order), and ensure the handlershandleStakeLockedandhandleStakeSetare implemented accordingly.
…or-multiple-dispute-kit-above-id-1
|
you can test this here: https://api.studio.thegraph.com/query/108432/kleros-v2-devnet/version/latest/graphql?que[…]pute+%7B%0A++++++++id%0A++++++%7D%0A++++%7D%0A++%7D%0A%7D
This only supports Devnet.
For Testnet support: would need a script adjustment to tweak the DisputeKitShutter address and deployment block in the subgraph.yaml file (since Devnet and Testnet share the same subgraph.yaml file)
For Neo support: it would be just adding the DisputeKitShutter address and deployment block in its subgraph.yaml file, since that file is independent from Devnet and Testnet so doesn't need script adjustment, add it like:
PR-Codex overview
This PR focuses on updating the
subgraphconfigurations and improving the handling of disputes and contributions within the Kleros system. It includes version updates, new data sources, and modifications to entity handling to accommodate additional dispute kits.Detailed summary
versioninpackage.jsonfrom0.15.2to0.16.0.core-neoandcore-universitysubgraphs.ClassicVoteentity handling forClassicDisputeKitandShutterDisputeKit.extractDisputeKitIDFromExtraDatafunction inutils.ts.subgraph.yamlfiles to include new data sources and configurations.createClassicDisputeFromEventto acceptdisputeKitIDandroundIndex.DisputeKitClassic.tsto support new dispute kits.ClassicContributionandClassicRoundentities to handle dispute kit IDs.SortitionModulefor better stake management.Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation
Chores