-
Notifications
You must be signed in to change notification settings - Fork 264
Audit of message passing tutorial #1616
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
base: main
Are you sure you want to change the base?
Audit of message passing tutorial #1616
Conversation
✅ Deploy Preview for docs-optimism ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe changes restructure and clarify the message-passing tutorials for cross-chain communication. Two relay tutorial entries ("relay-messages-cast" and "relay-messages-viem") were removed from the main tutorials metadata. The primary message-passing tutorial was reorganized for clarity, with improved prerequisite and network setup sections, standardized environment variable usage, and clearer distinctions between Supersim and Devnet environments. The Solidity contract examples were updated for correct imports and interface usage. The manual relay tutorial was expanded to include both shell script ( Sequence Diagram(s)Manual Cross-Chain Message Relay (New Flow)sequenceDiagram
participant User
participant ChainA (L2A)
participant ChainB (L2B)
participant ManualRelayScript
User->>ChainA (L2A): Send setGreeting via GreetingSender
ChainA (L2A)-->>User: Transaction receipt (message sent)
User->>ManualRelayScript: Run relay script (cast or JS)
ManualRelayScript->>ChainA (L2A): Fetch message from outbox
ManualRelayScript->>ChainB (L2B): Relay message to Greeter contract
ChainB (L2B)-->>ManualRelayScript: Transaction receipt (greeting updated)
ManualRelayScript-->>User: Print updated greeting
Comparison: Old vs. New Tutorial FlowsequenceDiagram
participant User
participant ChainA (L2A)
participant ChainB (L2B)
participant ViemScript
participant ManualRelayScript
%% Old flow with viem (deleted)
User->>ViemScript: Run viem relay script
ViemScript->>ChainA (L2A): Send setGreeting via GreetingSender
ViemScript->>ChainB (L2B): setGreeting directly
ChainB (L2B)-->>ViemScript: Read greeting
%% New flow (manual relay)
User->>ChainA (L2A): Send setGreeting via GreetingSender
ChainA (L2A)-->>User: Transaction receipt
User->>ManualRelayScript: Run relay script (cast or JS)
ManualRelayScript->>ChainA (L2A): Fetch message from outbox
ManualRelayScript->>ChainB (L2B): Relay message to Greeter contract
ChainB (L2B)-->>ManualRelayScript: Transaction receipt
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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
Documentation and Community
|
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: 2
🧹 Nitpick comments (8)
pages/interop/tutorials/message-passing/manual-relay.mdx (4)
98-99
: Avoid first-person pronouns.
Replace “We are going to use a Node project.” with an imperative instruction to maintain a consistent, formal tone.- 1. Initialize a new Node project. + 1. Set up a new Node.js project.
137-139
: Add missing comma for readability.
Insert a comma after “To relay a message” for grammatical correctness.- To relay a message we need the information in the receipt. + To relay a message, we need the information in the receipt.🧰 Tools
🪛 LanguageTool
[grammar] ~137-~137: Possible typo. Did you mean “A” or “we”?
Context: ...the OP-Stack actions. On wallet A we need the public actions, those that onl...(DT_PRP)
162-163
: Refine the instruction phrasing.
Change “Run JavaScript program, and see that the message is relayed.” to a clearer imperative without superfluous commas.- 2. Run JavaScript program, and see that the message is relayed. + 2. Run the JavaScript program and verify that the message is relayed.🧰 Tools
2. Run JavaScript program, and see that the message is re...🪛 LanguageTool
[uncategorized] ~163-~163: You might be missing the article “the” here.
Context: ...ipt for it.(AI_EN_LECTOR_MISSING_DETERMINER_THE)
186-188
: Remove stray backtick.
There’s an extra backtick after “Run this”. Remove it to fix the markdown.- Run this` script: + Run this script:
public/tutorials/setup-for-manual-relay.sh (4)
1-2
: Add strict error handling.
Includeset -e
after the shebang to ensure the script exits on the first error.#! /bin/sh +set -e
4-5
: Handle potentialcd
failures.
Guard directory changes to avoid silent failures.- mkdir -p manual-relay/onchain - cd manual-relay/onchain + mkdir -p manual-relay/onchain + cd manual-relay/onchain || exit 1🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 5-5: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
13-14
: Handle potentialcd
failures.
Add an exit guard when changing into thelib
directory.- cd lib + cd lib || exit 1🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 13-13: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
7-7
: Clarify the sample private key.
This hard-coded private key is for local testing only. Add a warning to prevent accidental use in production.- PRIVATE_KEY=0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80 + # WARNING: This private key is for local testing only. Do not use in production. + PRIVATE_KEY=0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80🧰 Tools
🪛 Gitleaks (8.26.0)
7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (9)
pages/interop/tutorials/_meta.json
(0 hunks)pages/interop/tutorials/message-passing.mdx
(8 hunks)pages/interop/tutorials/message-passing/_meta.json
(1 hunks)pages/interop/tutorials/message-passing/manual-relay.mdx
(5 hunks)public/tutorials/GreetingSender.sol
(2 hunks)public/tutorials/app.mts
(0 hunks)public/tutorials/manual-relay.mjs
(2 hunks)public/tutorials/setup-for-manual-relay.sh
(3 hunks)words.txt
(1 hunks)
💤 Files with no reviewable changes (2)
- pages/interop/tutorials/_meta.json
- public/tutorials/app.mts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.mdx`: "ALWAYS review Markdown content THOROUGHLY with the following criteria: - First, check the frontmatter section at the top of the file: 1. For regular pages, ensure AL...
**/*.mdx
: "ALWAYS review Markdown content THOROUGHLY with the following criteria:
- First, check the frontmatter section at the top of the file:
- For regular pages, ensure ALL these fields are present and not empty:
--- title: [non-empty] lang: [non-empty] description: [non-empty] topic: [non-empty] personas: [non-empty array] categories: [non-empty array] content_type: [valid type] ---
- For landing pages (index.mdx or files with ), only these fields are required:
--- title: [non-empty] lang: [non-empty] description: [non-empty] topic: [non-empty] ---
- If any required fields are missing or empty, comment:
'This file appears to be missing required metadata. Please check keywords.config.yaml for valid options and add the required fields manually. You can validate your changes by running:pnpm validate-metadata ```'
- Use proper nouns in place of personal pronouns like 'We' and 'Our' to maintain consistency in communal documentation.
- Avoid gender-specific language and use the imperative form.
- Monitor capitalization for emphasis. Avoid using all caps, italics, or bold for emphasis.
- Ensure proper nouns are capitalized in sentences.
- Apply the Oxford comma.
- Use proper title case for buttons, tab names, page names, and links. Sentence case should be used for body content and short phrases, even in links.
- Use correct spelling and grammar at all times (IMPORTANT).
- For H1, H2, and H3 headers:
- Use sentence case, capitalizing only the first word.
- Preserve the capitalization of proper nouns, technical terms, and acronyms as defined in the 'nouns.txt' file located in the root directory of the project.
- Do not automatically lowercase words that appear in the 'nouns.txt' file, regardless of their position in the header.
- Flag any headers that seem to inconsistently apply these rules for manual review.
- When reviewing capitalization, always refer to the 'nouns.txt' file for the correct capitalization of proper nouns and technical terms specific to the project.
"
pages/interop/tutorials/message-passing/manual-relay.mdx
pages/interop/tutorials/message-passing.mdx
🪛 LanguageTool
pages/interop/tutorials/message-passing/manual-relay.mdx
[grammar] ~137-~137: Possible typo. Did you mean “A” or “we”?
Context: ...the OP-Stack actions. On wallet A we need the public actions, those that onl...
(DT_PRP)
[typographical] ~143-~143: It seems that a comma is missing.
Context: ...c1b0e9c ``` To relay a message we need the information in the receipt....
(IN_ORDER_TO_VB_COMMA)
[uncategorized] ~163-~163: You might be missing the article “the” here.
Context: ...ipt for it.
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~171-~171: A comma might be missing here.
Context: ...hat messages were relayed by a specific transaction you can use this code: ```javascript...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
🪛 Gitleaks (8.26.0)
pages/interop/tutorials/message-passing.mdx
148-148: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
public/tutorials/setup-for-manual-relay.sh
7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Shellcheck (0.10.0)
public/tutorials/setup-for-manual-relay.sh
[warning] 5-5: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 13-13: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - docs-optimism
- GitHub Check: Header rules - docs-optimism
- GitHub Check: Pages changed - docs-optimism
🔇 Additional comments (15)
words.txt (1)
97-97
: Minor style adjustment notedThe position of "Devnets" has been moved in the word list, maintaining alphabetical order. This aligns with the standardization of terminology across the documentation.
public/tutorials/GreetingSender.sol (1)
4-6
: Import paths correctly updated to use absolute pathsThe import statements have been updated to use absolute paths from the
@eth-optimism/contracts-bedrock
package instead of relative paths. This change makes the contract more maintainable and aligns with modern Solidity development practices.pages/interop/tutorials/message-passing/_meta.json (1)
2-2
: Metadata key appropriately renamedThe key was renamed from "relay-with-cast" to "manual-relay" to better reflect the expanded tutorial content that now includes both shell script (cast) and JavaScript API methods for manual relaying.
public/tutorials/manual-relay.mjs (6)
8-8
: Import for chain configurations updatedThe import statement has been updated to import chains from the
@eth-optimism/viem/chains
package, which is appropriate for the supersim environment.
16-17
: Environment variable name standardizedThe private key environment variable has been renamed from
PRIV_KEY
toPRIVATE_KEY
, following a more standard naming convention. This improves consistency across the tutorials.
24-24
: Wallet client extensions commented outThe wallet action extensions for L2 have been commented out. This might be intentional, but it's worth verifying if these extensions are needed for the relay functionality.
These extensions appear to be partially commented out without explanation. Please verify if these wallet extensions are needed for the relay functionality to work properly, or if there's a specific reason they're commented out.
Also applies to: 31-31
35-35
: Environment variable names standardizedThe contract address environment variables have been renamed from
GREETER_X_ADDR
toGREETER_X_ADDRESS
, providing more consistency in naming conventions.Also applies to: 41-41
57-59
: Improved feedback with intermediate greeting checkAdded code to check and log the greeting state before relaying the transaction, which provides better visibility into the process and improves the tutorial's educational value.
73-74
: Variable naming improved for clarityThe final greeting variable was renamed from
greeting2
togreeting3
, which better reflects the sequence of operations (after the first two greeting checks). This improves code readability.pages/interop/tutorials/message-passing/manual-relay.mdx (2)
1-22
: Frontmatter completeness check passed.
All required metadata fields are present and non-empty.
114-117
: Clarify environment variable export.
The commandexport GREETER_A_ADDRESS GREETER_B_ADDRESS PRIVATE_KEY
does not assign values. Consider showing how to set or source the variables before exporting, for example:- export GREETER_A_ADDRESS GREETER_B_ADDRESS PRIVATE_KEY + export GREETER_A_ADDRESS=<value> GREETER_B_ADDRESS=<value> PRIVATE_KEY=<value>pages/interop/tutorials/message-passing.mdx (4)
1-23
: Frontmatter completeness check passed.
All required metadata fields are present and correctly populated.
48-54
: Review manual-relay content.
The “What you’ll learn” section still includes “How to relay messages manually between chains”, but this tutorial focuses on on-chain message passing. Consider removing or updating this bullet to avoid confusion.
67-69
: Align required tools with content.
The tutorial no longer uses a TypeScript off-chain app or Viem. Please remove or update these tool entries to reflect only the on-chain workflow.
71-75
: Update the “What You’ll Build” list.
Since the TypeScript relay application is now a separate tutorial, remove the related bullet to avoid confusion.
import { readFileSync } from 'fs'; | ||
|
||
const greeterData = JSON.parse(readFileSync('../onchain/out/Greeter.sol/Greeter.json')) | ||
const greetingSenderData = JSON.parse(readFileSync('../onchain/out/Greeter.sol/Greeter.json')) | ||
|
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.
Improved file loading with dynamic imports
The script now uses readFileSync
to dynamically load the contract JSON data instead of using static imports. This approach is more flexible for tutorials.
However, there seems to be an issue:
The greetingSenderData
is loading the Greeter.json file instead of a GreetingSender.json file. This will cause incorrect ABI loading for the GreetingSender contract.
-const greetingSenderData = JSON.parse(readFileSync('../onchain/out/Greeter.sol/Greeter.json'))
+const greetingSenderData = JSON.parse(readFileSync('../onchain/out/GreetingSender.sol/GreetingSender.json'))
📝 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.
import { readFileSync } from 'fs'; | |
const greeterData = JSON.parse(readFileSync('../onchain/out/Greeter.sol/Greeter.json')) | |
const greetingSenderData = JSON.parse(readFileSync('../onchain/out/Greeter.sol/Greeter.json')) | |
import { readFileSync } from 'fs'; | |
const greeterData = JSON.parse(readFileSync('../onchain/out/Greeter.sol/Greeter.json')) | |
const greetingSenderData = JSON.parse(readFileSync('../onchain/out/GreetingSender.sol/GreetingSender.json')) |
🤖 Prompt for AI Agents
In public/tutorials/manual-relay.mjs around lines 11 to 15, the
greetingSenderData variable incorrectly loads Greeter.json instead of
GreetingSender.json, causing wrong ABI data. Update the readFileSync path for
greetingSenderData to point to
'../onchain/out/GreetingSender.sol/GreetingSender.json' to correctly load the
GreetingSender contract JSON.
import { Callout, Steps, Tabs } from 'nextra/components' | ||
import { InteropCallout } from '@/components/WipCallout' | ||
import { AutorelayCallout } from '@/components/AutorelayCallout' |
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.
🛠️ Refactor suggestion
Remove unused import.
AutorelayCallout
is imported but never used in the document. Please remove it to keep the code clean.
- import { InteropCallout } from '@/components/WipCallout'
- import { AutorelayCallout } from '@/components/AutorelayCallout'
+ import { InteropCallout } from '@/components/WipCallout'
📝 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.
import { Callout, Steps, Tabs } from 'nextra/components' | |
import { InteropCallout } from '@/components/WipCallout' | |
import { AutorelayCallout } from '@/components/AutorelayCallout' | |
import { Callout, Steps, Tabs } from 'nextra/components' | |
import { InteropCallout } from '@/components/WipCallout' |
🤖 Prompt for AI Agents
In pages/interop/tutorials/message-passing.mdx around lines 25 to 27, the import
statement includes AutorelayCallout which is not used anywhere in the file.
Remove the import of AutorelayCallout from the import list to clean up unused
code.
Description
Tests
None, but I did make sure everything runs.
Additional context
N/A
Metadata
Replacing #1581