Refactor veasacn subgraphs: support for single subgraph deployment on multiple inbox or outbox. #413
Conversation
Merge dev branch into veascan subgraphs upgrade
WalkthroughThis pull request updates two subgraph projects by revising package metadata, schema definitions, scripts, and event handling logic. In the inbox project, the package version is bumped, a new update script is added, and a new GraphQL Changes
Sequence Diagram(s)sequenceDiagram
participant E as Event
participant I as VeaInboxHandler
participant IE as Inbox Entity
participant S as Snapshot Entity
E->>I: MessageSent event
I->>IE: Load or create Inbox (by address)
I->>S: getCurrentSnapshot(inboxAddress)
S-->>I: Return snapshot data
I->>IE: useNextMessageIndex(inboxAddress)
sequenceDiagram
participant E as Event
participant O as VeaOutboxHandler
participant OE as Outbox Entity
participant C as Claim/Challenge Processor
E->>O: Claimed/Challenged/Verified event
O->>OE: Load or create Outbox (by address)
O->>C: Process claim/challenge using event address
C-->>O: Return updated data
Possibly related PRs
Suggested labels
Suggested reviewers
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 veascan ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
veascan-subgraph-inbox/subgraph.yaml (2)
8-33:⚠️ Potential issueCheck entity name "Refs" vs. "Ref" in the schema.
The subgraph references- Refsfor the entity list, but the schema defines atype Ref @entity. This discrepancy may lead to runtime errors unless there's a dedicated alias. Confirm that the entity name is spelled exactly as in the schema.
35-61:⚠️ Potential issueNew VeaInboxArbToEthDevnet data source is well-configured.
The addition of this data source and associated event handlers matches the new contract address. However, ensure the same "Refs" vs. "Ref" mismatch is resolved here too.
🧹 Nitpick comments (12)
veascan-subgraph-inbox/package.json (1)
13-13: Addition of Update Script Command.
The new script"update:arbitrum-sepolia": "./scripts/update.sh arbitrumSepolia arbitrum-sepolia"is added to facilitate updating the subgraph configuration for the Arbitrum Sepolia network. Please verify that theupdate.shscript accepts these two parameters and that they correctly map to the necessary environment values.veascan-subgraph-inbox/schema.graphql (2)
25-27: Linking Snapshot to Inbox.
TheSnapshottype is now extended with aninbox: Inbox!field. This new relationship will enable queries to easily reference the source inbox. Make sure that the mapping logic correctly assigns anInboxwhen processing snapshots.
51-54: Extending Ref Entity with Inbox Association.
Similarly, theReftype now includes aninbox: Inbox!field, which improves traceability between references and their originating inboxes. Ensure that any resolver or handler logic which formerly assumed a different structure is updated accordingly.veascan-subgraph-outbox/package.json (1)
13-14: Simplified Update Script Commands.
Both"update:sepolia"and"update:chiado"commands have been simplified by removing extra parameters (e.g.ArbToEthDevnetandArbToGnosisDevnet). This streamlining should reduce confusion and potential misconfiguration—verify that the new commands correctly trigger the intended update process.veascan-subgraph-outbox/scripts/update.sh (2)
5-17: Review of the Update Function Implementation.
Theupdatefunction is structured clearly with local variable declarations for input parameters. Note the use of inline environment variable assignment withyqto update the network field.
Consider refactoring the JSON parsing to use redirection (e.g. usingjq '.address' < "$f") instead ofcat "$f" | jq ...for slight performance improvements.
23-23: Quote Command Substitution for Backup Filename.
The backup command:cp "$SCRIPT_DIR"/../subgraph.yaml "$SCRIPT_DIR"/../subgraph.yaml.bak.$(date +%s)may be susceptible to word splitting. To be safe, consider quoting the command substitution as shown below:
-cp "$SCRIPT_DIR"/../subgraph.yaml "$SCRIPT_DIR"/../subgraph.yaml.bak.$(date +%s) +cp "$SCRIPT_DIR"/../subgraph.yaml "$SCRIPT_DIR"/../subgraph.yaml.bak."$(date +%s)"veascan-subgraph-inbox/scripts/update.sh (2)
10-14: ABI File Path Update Enhancement.
The script now computes the contract file path relative to thesubgraph.yamlfile and uses it to update the mapping’s ABI path. This refactoring improves maintainability. Just confirm that all expected contract file paths (especially when combined with an optional suffix) are handled correctly.
38-45: Backup and Update Loop: Consider Quoting for Safety.
The loop backing upsubgraph.yamland iterating over data sources is clear and functional. Similar to the outbox script, it is advisable to quote the date substitution to avoid any word splitting issues. For example:-cp "$SCRIPT_DIR"/../subgraph.yaml "$SCRIPT_DIR"/../subgraph.yaml.bak.$(date +%s) +cp "$SCRIPT_DIR"/../subgraph.yaml "$SCRIPT_DIR"/../subgraph.yaml.bak."$(date +%s)"veascan-subgraph-inbox/src/VeaInbox.ts (2)
49-100: Consider refactoring default Snapshot creation.
The logic for creating a newSnapshotif one does not exist is solid. However, there's notable duplication when initializing default fields (lines 61-74 and 81-98). You could extract this initialization into a helper to keep the function DRY.- snapshot = new Snapshot(inboxAddress.toHexString() + "-" + ref.currentSnapshotIndex.toString()); - snapshot.inbox = inboxAddress; - snapshot.numberMessages = BigInt.fromI32(0); - ... + snapshot = createDefaultSnapshot(inboxAddress, ref.currentSnapshotIndex);
159-245: Iterating backward to find a matching epoch could be slow for large histories.
While this logic works, consider indexing snapshots by epoch to load the relevant snapshot directly, rather than searching from the current index downward. This optimizes performance for many snapshots.Also, if AssemblyScript supports optional chaining, you may simplify checks like
if (snapshot && snapshot.epoch)toif (snapshot?.epoch).🧰 Tools
🪛 Biome (1.9.4)
[error] 190-190: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
veascan-subgraph-outbox/src/VeaOutbox.ts (2)
44-44: Looping backward through claims.Repeatedly loading claims in reverse can become expensive at scale. Consider indexing claims by epoch for more direct lookups.
76-76: Decrementing loop for verification.Similar performance consideration applies here as in the challenge logic. If grows large, consider direct indexing by epoch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
veascan-subgraph-inbox/package.json(2 hunks)veascan-subgraph-inbox/schema.graphql(3 hunks)veascan-subgraph-inbox/scripts/update.sh(1 hunks)veascan-subgraph-inbox/src/VeaInbox.ts(5 hunks)veascan-subgraph-inbox/subgraph.yaml(2 hunks)veascan-subgraph-outbox/package.json(2 hunks)veascan-subgraph-outbox/schema.graphql(2 hunks)veascan-subgraph-outbox/scripts/update.sh(1 hunks)veascan-subgraph-outbox/src/VeaOutbox.ts(5 hunks)veascan-subgraph-outbox/subgraph.yaml(2 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
veascan-subgraph-outbox/scripts/update.sh (1)
veascan-subgraph-inbox/scripts/update.sh (1)
update(5-17)
veascan-subgraph-inbox/scripts/update.sh (1)
veascan-subgraph-outbox/scripts/update.sh (1)
update(5-27)
veascan-subgraph-outbox/src/VeaOutbox.ts (1)
veascan-web/src/gql/graphql.ts (5)
Claim(132-145)Challenge(36-44)Verification(1019-1026)Message(349-360)Ref(641-649)
🪛 Shellcheck (0.10.0)
veascan-subgraph-inbox/scripts/update.sh
[warning] 23-23: Quote this to prevent word splitting.
(SC2046)
🪛 Biome (1.9.4)
veascan-subgraph-inbox/src/VeaInbox.ts
[error] 3-3: 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)
[error] 190-190: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
veascan-subgraph-outbox/src/VeaOutbox.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)
[error] 80-80: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (41)
veascan-subgraph-inbox/package.json (1)
3-3: Version Update Validated.
The package version is updated to"0.2.1", which aligns with the improvements and deployment strategy outlined in the PR objectives.veascan-subgraph-inbox/schema.graphql (1)
9-12: New Inbox Entity Introduced.
A newInboxtype with the immutable directive is added with anidand a derived fieldmessages. This establishes a central entity for grouping related snapshots.veascan-subgraph-outbox/package.json (1)
3-3: Outbox Package Version Update.
The version is updated to"0.2.3", keeping the versioning in step with new functionalities. This is consistent with the update in the inbox subgraph.veascan-subgraph-inbox/scripts/update.sh (2)
5-9: Enhanced Update Function Signature and Parameter Handling.
The function declaration now includes a descriptive comment detailing its parameters (file, dataSourceIndex, graphNetwork), and variables are declared locally. This improves clarity and minimizes side effects.
19-26: Improved Extraction of Address and Start Block.
Transitioning to usingjqdirectly (withoutcat) for parsing JSON makes the extraction ofaddressandblockNumbermore efficient and concise.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 23-23: Quote this to prevent word splitting.
(SC2046)
veascan-subgraph-outbox/schema.graphql (7)
3-6: All added fields for Claim look good.
These additional fields (outbox,epoch,stateroot, andbridger) consistently match their annotated data types and improve clarity on the relationships and data stored.
16-19: New Outbox entity is well-defined.
Using@derivedFrom(field: "outbox")onclaimskeeps the relationship clear and allows easy bidirectional referencing.
26-26: Challenger field annotation is coherent.
The comment clarifies that this field holds an address in bytes form.
34-34: Caller field annotation is coherent.
Similar to thechallengerfield, this helps clarify that it's an address.
40-40: New outbox field in Message promotes consistent linking.
Associating a message with itsOutboxhelps unify the subgraph data model.
43-43: Relayer annotation is understandable.
Indicating an address forrelayeris in line with typical usage patterns.
49-49: New outbox field in Ref merges data contexts elegantly.
The addition of anoutboxreference inRefharmonizes with the newOutboxentity and fosters a more connected schema.veascan-subgraph-inbox/src/VeaInbox.ts (4)
1-7: Importing BigInt from @graphprotocol/graph-ts is standard usage.
The static analysis warning about overshadowing the global BigInt appears to be a false positive, as AssemblyScript builds for The Graph typically use this import.🧰 Tools
🪛 Biome (1.9.4)
[error] 3-3: 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)
26-47: handleMessageSent logic is coherent.
Creating or loading anInboxentity and then generating aMessagewith a composite ID (<inboxAddress>-<messageIndex>) ensures uniqueness. The data extraction steps for_to,_data, and_msgSenderare correct.
102-117: useNextMessageIndex function is clear and correct.
Incrementingref.nextMessageIndexand returning the old value is a suitable pattern for retrieving unique sequential message IDs.
120-157: handleSnapshotSaved changes appear logically consistent.
Loading theInboxif missing, retrieving theepochPeriod, and properly updating or creating the nextSnapshotis well handled. This ensures continuity of snapshot indexing.veascan-subgraph-outbox/src/VeaOutbox.ts (19)
1-1: Acknowledge static analysis caution regarding BigInt shadowing.Importing BigInt from
@graphprotocol/graph-tsis standard in The Graph environment and should not cause issues. The raised warning is likely a false positive for this context.🧰 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)
7-7: New import reference.Referencing
VeaOutboxArbToEthDevnetis consistent with the updated contract name. No concerns here.
14-14: Introducing Outbox entity import.Importing
Outboxaligns well with your new entity usage. Looks good.
18-22: Creating and persisting Outbox if missing.Good approach to ensure the
Outboxentity is initialized on-the-fly. No issues spotted.
23-26: Constructing unique claim ID per outbox.Using
event.addressandclaimIndexto generate a uniqueClaimID is a solid pattern. This prevents collisions across multiple outbox instances.
39-39: Loading Ref entity from address.Invoking
getRefensures that indexing logic remains consistent for each outbox. No concerns.
46-47: Dynamic claim loading by index.This logic is straightforward and properly concatenates the outbox address with numeric index. No issues.
58-58: Retrieving challenge index.Reusing a pattern parallel to the claim index logic is consistent. No concerns.
59-61: Constructing and creating Challenge entity.Correctly forming a unique challenge ID with the outbox address and index. This is a solid approach.
72-72: Retrieving reference in handleVerified.Using
getRefconsistently aligns verification with the correct outbox state.
78-79: Loading Claim by computed ID.Identical pattern as before—safe and consistent. No issues.
80-80: Conditional check on an optional claim.Consider using optional chaining
claim?.epoch.equals(...)if AssemblyScript fully supports it. Otherwise, this straightforward null check and property usage is fine.🧰 Tools
🪛 Biome (1.9.4)
[error] 80-80: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
81-82: Marking claim as verified.Simple and effective logic to update and save verification state. No issues.
83-83: Empty line.No additional remarks needed here.
84-85: Creating Verification entity linked to Claim.Reusing the claim’s ID ensures a 1:1 relationship between Claim and Verification. This approach looks good if only one verification per claim is expected.
107-108: Incrementing claim index.
useClaimIndexcleanly incrementstotalClaimsinRef. Implementation is correct.
115-116: Incrementing challenge index.
useChallengeIndexmirrors the claim index logic. No issues found.
123-125: Fetching or initializing Ref entity.Straightforward load-and-create pattern ensures you always have a valid
Ref. No issues found.
128-129: Setting outbox on newly created Ref.Linking the
Refentity to the outbox address ensures consistent referencing. Looks good.veascan-subgraph-outbox/subgraph.yaml (6)
6-6: Renaming the data source.Renaming to
VeaOutboxArbToEthTestnethelps convey the environment and purpose more clearly. No issues.
9-11: Updated address, ABI, and start block for Testnet source.Please verify the correctness of the contract address and block number to ensure accurate indexing and event coverage.
22-23: Refreshing ABI name and file references.This reconfiguration matches the new contract naming scheme. Approved.
24-32: Event handlers for the Testnet data source.Handlers (
handleChallenged,handleClaimed, etc.) align with your updated mapping code. No concerns.
33-33: Mapping file reference.Points to
./src/VeaOutbox.ts, consistent with the updated handlers. Good job.
34-51: New Devnet data source.Nice addition for the Devnet environment. Ensure that the address (
0xb1f5125b52CE23D3763AC1C9ACEf0668825A66c0) andstartBlock(7825233) are correct for your needs.
jaybuidl
dismissed stale reviews from coderabbitai[bot] and themself
via
7b10419
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
veascan-web/package.json (1)
75-75: Dependency Addition: Verify svgo Version CompatibilityThe addition of
"svgo": "^3.3.2"to thedevDependenciesis appropriate for optimizing SVG assets during development. Please ensure that this version is fully compatible with your build tools and workflows, and confirm that no breaking changes have been introduced compared to previous versions used in the project.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
veascan-web/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: dependency-review
- GitHub Check: test
- GitHub Check: Analyze (javascript)
veascan-subgraph-inbox:Inboxwith relation toSnapshotandRef.Snapshot.idupdated value : inbox.address-snapshotIndexMessage.idupdated value: inbox.address-messageIndexveascan-subgraph-outbox:Inboxwith relation toClaim,MessageandRef.Claim.idupdated value : inbox.address- claimIndexMessage.idupdated value: inbox.address-messageIndexPR-Codex overview
This PR introduces several updates and enhancements to the
veascanproject, including new entities, updated scripts, and modifications to the GraphQL schema to support improved functionality for theInboxandOutboxcomponents.Detailed summary
svgodependency toveascan-web/package.json.veascan-subgraph-inboxandveascan-subgraph-outbox.update:arbitrum-sepoliainveascan-subgraph-inbox/package.json.InboxandOutboxentities to GraphQL schemas.InboxandOutbox.subgraph.yamlfiles to reflect new addresses and ABIs.update.shscripts for better handling of parameters and paths.VeaOutbox.tsandVeaInbox.tsfor improved clarity and functionality, including the use of composite IDs.Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores