-
Notifications
You must be signed in to change notification settings - Fork 263
Missing lints & interop ideas #1381
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. |
Warning Rate limit exceeded@bradleycamacho has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 12 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis pull request adds a new "Ideas" card to the Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Cards Component
participant R as Router
participant P as Ideas Page
U->>C: Click "Ideas" card
C->>R: Navigate to /stack/interop/ideas
R->>P: Request ideas page content
P-->>R: Return content of ideas page
R-->>U: Display Superchain interoperability ideas document
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 (3)
pages/stack/interop.mdx (1)
39-40
: New "Ideas" card added to the interop navigation
The addition of<Card title="Ideas" href="/stack/interop/ideas" />
integrates neatly with the existing Cards component. For visual consistency, consider whether an icon should be added, as most of the other cards include one.pages/app-developers/transactions/parameters.mdx (1)
64-64
: Style refinement for the Tenderly gas price line
The sentence on line 64 regarding Tenderly's [tenderly_gasPrice
] method may trigger a static analysis warning about adverb repetition. Review the sentence to ensure that no redundant adverbs are present. For example, if "alternatively" appears more than once, consider removing the duplicate for clarity.🧰 Tools
🪛 LanguageTool
[style] ~64-~64: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...ternatively, you can rely on Tenderly's [tenderly_gasPrice
](https://docs.tenderly.co/nod...(ADVERB_REPETITION_PREMIUM)
pages/stack/interop/tutorials/message-passing.mdx (1)
516-520
: Handling transaction receipt and confirmation logic
The code snippet from<rootDir>/public/tutorials/app_v2.mts#L61-L63
emphasizes waiting for the transaction receipt after relaying a message. Verify that this logic robustly handles asynchronous behavior and potential delays in block confirmation. Consider including error handling should the receipt not be received within an expected timeframe.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
pages/app-developers/tools/build/faucets.mdx
(1 hunks)pages/app-developers/transactions/parameters.mdx
(2 hunks)pages/stack/interop.mdx
(1 hunks)pages/stack/interop/_meta.json
(1 hunks)pages/stack/interop/tutorials/message-passing.mdx
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- pages/app-developers/tools/build/faucets.mdx
🧰 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.mdx
pages/app-developers/transactions/parameters.mdx
pages/stack/interop/tutorials/message-passing.mdx
🪛 LanguageTool
pages/app-developers/transactions/parameters.mdx
[style] ~64-~64: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...ternatively, you can rely on Tenderly's [tenderly_gasPrice
](https://docs.tenderly.co/nod...
(ADVERB_REPETITION_PREMIUM)
⏰ 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 (6)
pages/stack/interop/_meta.json (1)
10-10
: New metadata key added for interoperability ideas
The"ideas": "Interop ideas"
key is introduced in a consistent manner alongside the other keys. The ordering and naming follow the established pattern, which should help with linking the new Ideas card in the UI.pages/app-developers/transactions/parameters.mdx (1)
31-52
: Enhanced steps for base fee configuration
The updated<Steps>
block now clearly delineates the sequential process:
• Retrieving the latest block,
• Extracting the base fee and gas used,
• Predicting the next base fee, and
• Selecting the base fee per gas.Each header (e.g., "Retrieve the latest block", "Predict the next base fee") is clear and instructive. Verify that the specified percentages (10% adjustments and 2x multipliers) match the intended fee mechanism parameters for OP Mainnet.
pages/stack/interop/tutorials/message-passing.mdx (4)
503-507
: Verify updated code snippet from app_v2.mts (lines 11–15)
The inclusion of the updated code snippet is clear. Please ensure that the referenced snippet from<rootDir>/public/tutorials/app_v2.mts#L11-L15
reflects the intended changes to the cross-chain messaging tutorial.
508-509
: Review the import and initialization snippet
The code snippet from<rootDir>/public/tutorials/app_v2.mts#L22-L28
is used to import from the@eth-optimism/viem
package. Confirm that this update aligns with the intended interoperability enhancements and that all associated dependencies are properly updated.
511-514
: Extension with OP-Stack actions
The snippet from<rootDir>/public/tutorials/app_v2.mts#L59
demonstrates extending wallets with both Viem public actions and OP-Stack actions. This addition is important for robust interchain interactions. Ensure that the implementation correctly integrates the intended OP-Stack functionalities.
522-527
: Retrieving and processing single relayed message
The snippet from<rootDir>/public/tutorials/app_v2.mts#L64-L70
illustrates handling the case where a single message is expected even though multiple might be returned. Confirm that the logic correctly selects the first message and that this approach will suffice for the intended use case. Clarify in the documentation that the system currently assumes a one-message scenario.
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)
158-187
: Clarify and Refine Deployment Command ExplanationThe added
<details>
block detailing the deployment command is very helpful and greatly improves clarity. There is a minor grammatical issue at line 181 where it reads “the when the command line includes backticks…” – consider changing it to “when the command line includes backticks…” for better readability.
503-529
: Enhance the Manual Relaying Logic Documentation in app_v2.mtsThe detailed explanation in this section is thorough—it outlines importing from the
@eth-optimism/viem
package, extending wallet functions, encoding the relay message, and waiting for the relay receipt. To further strengthen this section, consider adding a brief note on error handling (for example, what to do if the transaction receipt is delayed or fails to confirm) or linking to a troubleshooting guide.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pages/stack/interop/message-passing.mdx
(2 hunks)pages/stack/interop/tutorials/message-passing.mdx
(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- pages/stack/interop/message-passing.mdx
🧰 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
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Redirect rules - docs-optimism
- GitHub Check: Header rules - docs-optimism
- GitHub Check: Pages changed - docs-optimism
- GitHub Check: pr-workflow
🔇 Additional comments (1)
pages/stack/interop/tutorials/message-passing.mdx (1)
225-236
: Accurate and Clear Explanation for GreetingSender FunctionalityThis explanation block clearly describes how the
GreetingSender
encodes a call tosetGreeting
and sends it to a cross-chain contract. The information is concise and supports the reader’s understanding of the process.
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: 3
🧹 Nitpick comments (4)
pages/stack/interop/ideas.mdx (4)
1-5
: YAML front matter review: Refine description language.
The front matter is well structured, but consider replacing “up until now” with “until now” for a more formal tone in the description.
9-9
: Call to action revision: Clarify language and maintain tone.
The sentence “It’s time to reimagine the onchain future together, reimagine your application with Superchain Interop and hack on Superchain Interop devnets.” could benefit from slight rephrasing. Consider hyphenating “onchain” as “on-chain” (if that aligns with project style) and revising for clarity.
11-11
: Header formatting: Hyphenate compound adjectives and apply sentence case.
The header “## Cutting edge Superchain ideas” should use a hyphen for the compound adjective and sentence case. A revised version would be: “## Cutting-edge superchain ideas”.
15-15
: Call-to-action clarity: Refine the invitation text.
The call to action is clear, but ensure consistency in tone by avoiding informal contractions. Also, verify that the link text and follow-up instructions align with overall documentation style.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pages/stack/interop/ideas.mdx
(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/ideas.mdx
🪛 LanguageTool
pages/stack/interop/ideas.mdx
[grammar] ~7-~7: Did you mean “until” or “up to”?
Context: ...vity and composability has been limited up until now. We’ve mastered wallets, DeFi, L2s,...
(UP_UNTIL)
[uncategorized] ~13-~13: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...e Superchain ideas Hack on the various cutting edge applications that are uniquely enabled ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[duplication] ~41-~41: Possible typo: you repeated a word.
Context: ...grations from a chain in the Superchain to to deposit/withdraw to any chain in the Su...
(ENGLISH_WORD_REPEAT_RULE)
[misspelling] ~42-~42: This word is normally spelled as one.
Context: ...entivizer Agent | P2 | A multi-chain agent that drips rewards to users via a...
(EN_COMPOUNDS_MULTI_CHAIN)
⏰ 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 (1)
pages/stack/interop/ideas.mdx (1)
17-21
: Helpful tools section: Verify link text and punctuation.
The “Helpful Tools:” list is concise and functional. Ensure that the link texts use proper sentence case if that is the established style. Overall, this section meets the documentation standards.
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.
Looks good, I just left a few comments we should address before publishing
|
||
# Superchain interoperability ideas | ||
|
||
Crypto used to be full of wild ideas—where are they now? Creativity and composability has been limited up until now. We've mastered wallets, DeFi, L2s, and stables. It's time to bring back the bold. Discover net-new use cases that are uniquely enabled by Superchain Interop, and not just bridge abstraction. |
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.
Crypto used to be full of wild ideas—where are they now? Creativity and composability has been limited up until now. We've mastered wallets, DeFi, L2s, and stables. It's time to bring back the bold. Discover net-new use cases that are uniquely enabled by Superchain Interop, and not just bridge abstraction. | |
Crypto used to be full of wild ideas—where are they now? Creativity and composability has been limited up until now. We've made leaps and bounds in many domains, but it's time to bring back the bold. Discover net-new use cases that are uniquely enabled by Superchain Interop, and not just bridge abstraction. |
Maybe its the cynic in me, but our industry has a long way to go. I think we can reframe the second sentence. Mainly because sentiment around wallets and even L2s is still poor
|
||
### Interop ideas | ||
|
||
| Use Case | Idea | Priority | Description | | |
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.
This markdown table has an extra column
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.
I think we might also want to consider the priority column
| | SuperArbitrage | P1 | Build a trading bot that uses Superchain interop to arbitrage DEX pools on ChainA and ChainB | | | ||
| | Predict | P1 | Prediction market that uses Superchain interop to determine whether a set of actions happens on a set of chainsEg Will Uniswap USDC-ETH on the Superchain see more volume than Velodrome USDC-ETH today? | | | ||
| Identity | Attestations | P1 | Use an attestation on chainA to interact with a smart contract on chainB Superchain attestations | | | ||
| | SuperGotchi | P3 | A Tamagotchi-like game where the pets live as NFTs onchain, and transform based on activity on ChainA and ChainB | | |
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.
Use case: Gaming
|
||
### Interop ideas | ||
|
||
| Use Case | Idea | Priority | Description | | |
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.
I think this is from an internal doc I gave Amanda - can we remove the priority column and remove the word "Super" from all these and remove acronomys.
Superswap = Swap
eg CDP = Collateralized debt position
@@ -0,0 +1,45 @@ | |||
--- | |||
title: Superchain interoperability ideas |
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.
The console is actually being updated to have this info with the associated UI designs. If we follow DRY - should we just link out there?
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.
@zainbacchus sorry for the delay on this; there was an issue in our backlog to create this page so I just knocked it out. If it already exists on the console page, maybe we should just close this PR? Also happy to make your requested edits, but it seems a bit redundant. |
@bradleycamacho sorry for the duplicate effort, but I agree this does seem to be a redundancy so I'm going to go ahead and close this |
Closes #471