-
Notifications
You must be signed in to change notification settings - Fork 263
Fixing the errors in the message passing tutorial #1367
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
Conversation
✅ Deploy Preview for docs-optimism ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
📝 WalkthroughWalkthroughThis pull request updates the Sequence Diagram(s)sequenceDiagram
participant WalletA as walletA (Chain A)
participant GreeterA as Greeter Contract (Chain A)
participant Interop as L2ToL2CrossDomainMessenger
participant WalletB as walletB (Chain B)
participant GreeterB as Greeter Contract (Chain B)
WalletA->>GreeterA: setGreeting(greeting)
GreeterA-->>WalletA: emit CrossDomainSetGreeting(sender, chainId, greeting)
WalletB->>GreeterB: setGreeting(greeting)
GreeterB-->>WalletB: confirm update
WalletA->>Interop: send interop message to walletB
Interop->>WalletB: relay message
WalletB->>GreeterB: update greeting based on cross-chain message
GreeterB-->>WalletB: confirm cross-domain update
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
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: 0
🧹 Nitpick comments (7)
public/tutorials/app_v2.mts (6)
1-20
: Unused import "defineChain".The
defineChain
import is never used. Consider removing it to keep the codebase clean and avoid confusion.-import { - createWalletClient, - http, - defineChain, - publicActions, - getContract, - Address, -} from 'viem' +import { + createWalletClient, + http, + publicActions, + getContract, + Address, +} from 'viem'
23-30
: Evaluate error handling for wallet initialization.Creating walletA could fail if network connectivity or configuration is invalid. Wrap in a try/catch or ensure robust error handling upstream.
31-38
: Duplicate pattern in wallet creation.The code for
walletB
is almost identical towalletA
. Consider abstracting common wallet creation logic into a helper function to reduce repetition.
51-57
: Consider adding error checks after write operations.The code sets the greeting on chain B and logs the result. If transaction confirmation fails, the script may crash. Add error handling or logs to detect failures.
70-72
: Consider timeouts or checks for stuck transactions.
waitForTransactionReceipt
can hang if a transaction is stuck. Implement a timeout or a status check to exit gracefully if the chain does not confirm in time.
73-75
: Clarify chain naming in console logs.The log says "Chain A Greeting," but the code is reading from the greeter on chain B. Consider renaming for clarity if the final greeting is on chain B.
pages/stack/interop/tutorials/message-passing.mdx (1)
96-101
: Avoid first-person references."At this point we are relying on the autorelay functionality..." should avoid "we." Rephrase to maintain consistency with documentation guidelines on pronouns.
-At this point we are relying on the autorelay functionality of Supersim. +At this point, the tutorial relies on the autorelay functionality of Supersim.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pages/stack/interop/tutorials/message-passing.mdx
(5 hunks)public/tutorials/app_v2.mts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.mdx`: "ALWAYS review Markdown content THOROUGHLY with ...
**/*.mdx
: "ALWAYS review Markdown content THOROUGHLY with the following criteria:
- 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/stack/interop/tutorials/message-passing.mdx
🔇 Additional comments (11)
public/tutorials/app_v2.mts (4)
21-22
: Validate environment variables before use.If
process.env.PRIV_KEY
is undefined or incorrectly set, this code may fail. Consider validating environment variables to prevent runtime errors.
39-49
: Check for missing addresses and ABIs.Ensure
GREETER_B_ADDR
andGREETER_A_ADDR
, as well as the JSON ABIs, are present at runtime. Otherwise, the script may throw errors.
58-61
: Warning: No fallback logic if greeting transaction fails.When setting a greeting via chain A, there is no fallback or attempt in case the transaction is dropped or rejected. This can lead to silent failures.
62-69
: Safeguard empty sentMessage array.
createInteropSentL2ToL2Messages
may return an empty array if no messages are found. VerifysentMessages[0]
is not undefined before using it.pages/stack/interop/tutorials/message-passing.mdx (7)
106-106
: Command snippet LGTM.The command to run Supersim with the
--interop.autorelay
flag appears correct and consistent with the tutorial flow.
110-115
: Table rendering LGTM.The ChainID table is clear, and the alignment is correct for readability.
151-152
: Reference snippet LGTM.The snippet reference for
Greeter.sol
is properly linked, ensuring governed and tracked code for the tutorial.
451-452
: Reference snippet LGTM.The snippet reference for
app.mts
is properly included, making the tutorial steps cohesive.
557-560
: Table structure LGTM.The new table referencing contract addresses aligns well with the rest of the tutorial style and uses parallel columns.
561-562
: Instruction clarity is good."To modify the program to relay messages on devnet..." is concise, and the direct steps are well-labeled.
565-569
: Table format for updated lines LGTM.The line references for
interopAlpha0
andinteropAlpha1
are easy to follow for manual modifications.
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: 1
🧹 Nitpick comments (3)
pages/stack/interop/tutorials/message-passing.mdx (3)
109-113
: Improve table header capitalization.The table headers should follow title case according to the coding guidelines.
-| Role | ChainID | RPC URL | -| -------- | ------: | --------------------------------------------- | +| Role | Chain ID | RPC URL | +| -------- | -------: | --------------------------------------------- |
554-557
: Fix table formatting and header capitalization.The table has inconsistent column widths and incorrect header capitalization.
-| Contract | Network | Address | -| --------------- | ------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Contract | Network | Address | +| ------------ | ---------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- |
563-569
: Fix table formatting and improve readability.The table has inconsistent column alignment and could be more readable.
-| Line number | New content | -| ----------: | ------------------------------------------------------------------------- | +| Line Number | New Content | +| -----------:| --------------------------------------------------------------------------|
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pages/stack/interop/tutorials/message-passing.mdx
(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.mdx`: "ALWAYS review Markdown content THOROUGHLY with ...
**/*.mdx
: "ALWAYS review Markdown content THOROUGHLY with the following criteria:
- 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/stack/interop/tutorials/message-passing.mdx
🔇 Additional comments (2)
pages/stack/interop/tutorials/message-passing.mdx (2)
97-99
: LGTM! Clear warning about manual message execution.The warning callout effectively alerts users about the requirement for manual message execution when using devnet.
604-608
: LGTM! Clear next steps section.The next steps section provides clear guidance for users to continue their learning journey.
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
🧹 Nitpick comments (2)
pages/stack/interop/tutorials/message-passing.mdx (2)
97-99
: Enhance the warning message for clarity.The warning callout should be more specific about the consequences of not manually sending executing messages on devnet.
- If you attempt to run these steps with the [devnet](../tools/devnet), you *must* Send the executing message yourself, as explained [here](#implement-manual-message-relaying). + When using the [devnet](../tools/devnet), messages will not be automatically relayed. You *must* manually send executing messages as explained in the [manual message relaying section](#implement-manual-message-relaying), otherwise cross-chain communication will fail.
98-98
: Fix capitalization in warning message.The word "Send" should not be capitalized mid-sentence.
- If you attempt to run these steps with the [devnet](../tools/devnet), you *must* Send the executing message yourself, as explained [here](#implement-manual-message-relaying). + If you attempt to run these steps with the [devnet](../tools/devnet), you *must* send the executing message yourself, as explained [here](#implement-manual-message-relaying).
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pages/stack/interop/tutorials/message-passing.mdx
(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.mdx`: "ALWAYS review Markdown content THOROUGHLY with ...
**/*.mdx
: "ALWAYS review Markdown content THOROUGHLY with the following criteria:
- 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/stack/interop/tutorials/message-passing.mdx
🔇 Additional comments (2)
pages/stack/interop/tutorials/message-passing.mdx (2)
449-450
: Fix incorrect language identifier in code blocks.The code blocks use
typescript
identifier for TypeScript code.Also applies to: 498-499
571-571
: Clarify ETH requirements for both chains.The instruction should specify that ETH is needed on both Interop Devnet chains.
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: 1
🧹 Nitpick comments (5)
public/tutorials/app_v2.mts (5)
20-20
: Validate environment variable usage.
The code directly usesprocess.env.PRIV_KEY
without handling the scenario where it might be undefined, which could lead to runtime errors if the environment variable is not properly set.Consider handling the case where
PRIV_KEY
is missing by throwing an error or providing a user-friendly message. For example:-const account = privateKeyToAccount(process.env.PRIV_KEY as `0x${string}`) +if (!process.env.PRIV_KEY) { + throw new Error('Environment variable PRIV_KEY is missing. Please set it before running this script.') +} +const account = privateKeyToAccount(process.env.PRIV_KEY as `0x${string}`)
22-37
: Refactor to avoid duplication when creating wallet clients.
BothwalletA
andwalletB
use similar chain-specific code. DRY (Don’t Repeat Yourself) practices recommend extracting shared logic into a helper function or factory to reduce duplication and potential refactoring overhead.-const walletA = createWalletClient({ - chain: supersimL2A, - transport: http(), - account -}).extend(publicActions) - .extend(publicActionsL2()) - .extend(walletActionsL2()) - -const walletB = createWalletClient({ - chain: supersimL2B, - transport: http(), - account -}).extend(publicActions) - .extend(publicActionsL2()) - .extend(walletActionsL2()) +function createLayer2Wallet(chain, account) { + return createWalletClient({ + chain, + transport: http(), + account + }) + .extend(publicActions) + .extend(publicActionsL2()) + .extend(walletActionsL2()) +} + +const walletA = createLayer2Wallet(supersimL2A, account) +const walletB = createLayer2Wallet(supersimL2B, account)
38-48
: Ensure contract addresses are set and valid.
Bothprocess.env.GREETER_B_ADDR
andprocess.env.GREETER_A_ADDR
must be valid addresses. The code does not verify they are defined or valid. If they are undefined or invalid, calls togetContract
will fail at runtime.-if (!process.env.GREETER_B_ADDR) { - console.warn('GREETER_B_ADDR is not set. The contract address might be invalid.') -} -if (!process.env.GREETER_A_ADDR) { - console.warn('GREETER_A_ADDR is not set. The contract address might be invalid.') -} +if (!process.env.GREETER_B_ADDR) { + throw new Error('Environment variable GREETER_B_ADDR is missing or invalid.') +} +if (!process.env.GREETER_A_ADDR) { + throw new Error('Environment variable GREETER_A_ADDR is missing or invalid.') +}
50-56
: Consider adding error handling for transaction failures.
When callinggreeter.write.setGreeting
and waiting for the receipt, a transaction failure might remain silent. You might want to wrap it in a try/catch to handle potential errors more gracefully.-const txnBHash = await greeter.write.setGreeting(["Greeting directly to chain B"]) -await walletB.waitForTransactionReceipt({hash: txnBHash}) -const greeting1 = await greeter.read.greet() -console.log(`Chain B Greeting: ${greeting1}`) +try { + const txnBHash = await greeter.write.setGreeting(["Greeting directly to chain B"]) + await walletB.waitForTransactionReceipt({ hash: txnBHash }) + const greeting1 = await greeter.read.greet() + console.log(`Chain B Greeting: ${greeting1}`) +} catch (err) { + console.error('Failed to set greeting on Chain B:', err) +}
57-60
: Apply similar error handling for Chain A transaction.
ThegreetingSender.write.setGreeting
call is also unguarded. Consider applying the same try/catch pattern to ensure any transaction failures are handled.
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
🧹 Nitpick comments (1)
pages/stack/interop/tutorials/message-passing.mdx (1)
571-585
: Enhance clarity of ETH requirements.Consider the following improvements:
- Clarify that ETH is needed on both devnet chains.
- Provide a more specific estimate for the ETH bridging wait time.
- 2. Set `PRIV_KEY` to the private key of an address that has [Sepolia ETH](https://cloud.google.com/application/web3/faucet/ethereum/sepolia). + 2. Set `PRIV_KEY` to the private key of an address that has Sepolia ETH. You'll need ETH on both devnet chains: + - Bridge ETH to Devnet 0 (`interopAlpha0`) + - Bridge ETH to Devnet 1 (`interopAlpha1`) + You can obtain Sepolia ETH from the [faucet](https://cloud.google.com/application/web3/faucet/ethereum/sepolia). - Wait a few minutes until you can see the ETH [on your explorer](https://sid.testnet.routescan.io/). + Wait approximately 5-10 minutes for the bridging to complete. You can verify the ETH balance [on your explorer](https://sid.testnet.routescan.io/).
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pages/stack/interop/tutorials/message-passing.mdx
(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.mdx`: "ALWAYS review Markdown content THOROUGHLY with ...
**/*.mdx
: "ALWAYS review Markdown content THOROUGHLY with the following criteria:
- 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/stack/interop/tutorials/message-passing.mdx
🔇 Additional comments (4)
pages/stack/interop/tutorials/message-passing.mdx (4)
97-99
: LGTM! Clear and helpful warning callout.The warning effectively alerts users about the manual message execution requirement on devnet, with a helpful link to the implementation details.
109-113
: LGTM! Well-structured table format.The table is properly formatted with clear column headers and consistent alignment.
449-450
: Fix incorrect language identifier in code blocks.The code blocks are using
solidity
identifier for TypeScript code, which should betypescript
.Also applies to: 498-499
554-557
: LGTM! Comprehensive contract reference table.The table clearly presents contract addresses with helpful links to block explorers.
Description
Fixing multiple errors.