Skip to content
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

Tracing Deposits and Withdrawals #966

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Tracing Deposits and Withdrawals #966

wants to merge 14 commits into from

Conversation

krofax
Copy link
Contributor

@krofax krofax commented Oct 8, 2024

Description

This tutorial explains how you can use the Viem race a deposit or withdrawal transaction between L1 and L2. It deprecates the Optimism SDK.

Tests

Additional context

Metadata

@krofax krofax requested a review from a team as a code owner October 8, 2024 14:30
Copy link

netlify bot commented Oct 8, 2024

Deploy Preview for docs-optimism ready!

Name Link
🔨 Latest commit 7257426
🔍 Latest deploy log https://app.netlify.com/sites/docs-optimism/deploys/6717dda61195e00008a14024
😎 Deploy Preview https://deploy-preview-966--docs-optimism.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@krofax krofax requested a review from brokewhale October 8, 2024 14:34
Copy link
Contributor

coderabbitai bot commented Oct 8, 2024

Caution

Review failed

The head commit changed during the review from 1043a5d to 7257426.

Walkthrough

The changes involve updating documentation and code to replace references to the "Optimism SDK" with the "viem" library. This includes revising a tutorial to guide users on utilizing the viem library for tracing transactions between Layer 1 and Layer 2. Additionally, the JavaScript script has been modified to use the viem library for creating public clients and retrieving transaction data. The words.txt file has also been updated to standardize case sensitivity and introduce new terms related to the viem library.

Changes

File Path Change Summary
pages/builders/app-developers/tutorials/sdk-trace-txns.mdx Revised to replace "Optimism SDK" with "viem" library, updated tutorial content and installation commands.
public/tutorials/sdk-trace-txns.js Transitioned from @eth-optimism/sdk and ethers to viem; updated transaction handling and renamed variables.
words.txt Standardized case sensitivity of identifiers and added new terms related to the viem library.

Possibly related issues

Possibly related PRs

  • Added banner and notice page for SDK deprecation  #928: This PR discusses the deprecation of the Optimism SDK and the transition to the viem library, which directly relates to the changes made in this PR.
  • Make tiny updates to the fjord calculator #965: This PR includes updates to the documentation regarding the deprecation of the Optimism SDK and the transition to the viem library, aligning with the changes made in the main PR.
  • superchainERC20 #986: This PR introduces the SuperchainERC20 Token Standard, which is relevant as it may involve the use of the viem library for interoperability, connecting to the broader context of the main PR's focus on transitioning from the Optimism SDK to viem.

Suggested reviewers

  • hamdiallam
  • bradleycamacho

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Outside diff range and nitpick comments (7)
pages/builders/app-developers/tutorials/sdk-trace-txns.mdx (6)

12-14: LGTM: Introduction updated to reflect the use of viem library.

The introduction has been appropriately updated to mention the viem library. However, consider adding a brief explanation of why viem is being used instead of the Optimism SDK to provide context for the change.

Consider adding a sentence explaining the reason for switching to viem, such as:
"We now recommend using viem as it provides improved performance and a more streamlined API for interacting with Ethereum networks."


22-23: LGTM: Project setup instructions updated for viem.

The project setup instructions have been correctly updated to use the viem library. However, there's a minor inconsistency in the installation step.

In the installation step, the comment mentions installing both viem and ethers.js, but the command only installs viem. Consider either removing the mention of ethers.js or adding it to the installation command if it's needed for the tutorial.

Also applies to: 40-43


49-55: LGTM: RPC URL section updated for viem and new networks.

The section has been appropriately updated to mention the viem library's getTransactionReceipt function and the new network URLs for Sepolia and OP Sepolia.

Consider adding a brief explanation of why Sepolia and OP Sepolia are being used (e.g., "We're using Sepolia and OP Sepolia as they are the current testnet environments for Ethereum and Optimism, respectively.").


105-133: LGTM: Deposit tracing steps updated for viem.

The deposit tracing steps have been appropriately updated to use viem functions instead of the Optimism SDK. The instructions are clear and provide a good guide for users.

Consider adding brief comments explaining what each step does, especially for the log parsing step. This would help users understand the purpose of each operation in the tracing process.


138-165: LGTM: Withdrawal tracing steps updated for viem.

The withdrawal tracing steps have been appropriately updated to use viem functions instead of the Optimism SDK. The instructions are clear and provide a good guide for users.

Similar to the deposit tracing section, consider adding brief comments explaining what each step does, especially for the log parsing step. This would help users understand the purpose of each operation in the withdrawal tracing process.


Line range hint 1-172: Overall: Excellent update to use viem library.

The tutorial has been successfully updated to use the viem library instead of the Optimism SDK. The changes are consistent throughout the file, maintaining the tutorial's structure and flow while providing clear instructions for using viem to trace deposits and withdrawals.

To further improve the tutorial:

  1. Consider adding a brief introduction explaining why viem is now recommended over the Optimism SDK.
  2. Add more explanatory comments to the code snippets to help users understand each step of the process.
  3. Ensure that all linked tutorials and resources are also updated or include notes about any differences in the libraries used.
public/tutorials/sdk-trace-txns.js (1)

9-10: Add error handling for missing RPC URLs.

It's a good practice to check if l1RpcUrl and l2RpcUrl are defined to prevent runtime errors if the environment variables are not set.

Consider adding error handling:

 const l1RpcUrl = process.env.L1_RPC_URL;
 const l2RpcUrl = process.env.L2_RPC_URL;

+if (!l1RpcUrl || !l2RpcUrl) {
+  throw new Error('L1_RPC_URL and L2_RPC_URL environment variables must be set.');
+}
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 4444b66 and f03401c.

📒 Files selected for processing (2)
  • pages/builders/app-developers/tutorials/sdk-trace-txns.mdx (2 hunks)
  • public/tutorials/sdk-trace-txns.js (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
pages/builders/app-developers/tutorials/sdk-trace-txns.mdx (1)

Pattern **/*.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. Use bold for prominence instead of all caps or italics.
  • Ensure proper nouns are capitalized in sentences.
  • Apply the Oxford comma.
  • Use proper title case for headers, 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).
    "
🔇 Additional comments (2)
pages/builders/app-developers/tutorials/sdk-trace-txns.mdx (2)

4-4: LGTM: Updated description accurately reflects the tutorial content.

The description has been appropriately updated to mention the viem library instead of the Optimism SDK, which aligns with the changes made throughout the tutorial.


70-72: LGTM: Dependency import updated to use viem.

The import statement has been correctly updated to use the viem library instead of the Optimism SDK.

pages/builders/app-developers/tutorials/sdk-trace-txns.mdx Outdated Show resolved Hide resolved
pages/builders/app-developers/tutorials/sdk-trace-txns.mdx Outdated Show resolved Hide resolved
public/tutorials/sdk-trace-txns.js Show resolved Hide resolved
Copy link
Member

@bradleycamacho bradleycamacho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still needs a tech review so I'm happy to review again later, but this look great to me! Just make sure to update the headers to comply with the new style

@ZakAyesh
Copy link
Contributor

For all of these tutorials usually there is a step for running the command mkdir op-sample-project if we assume that developers go through a couple of these they may have trouble creating the directories since they will have to overwrite their old one. Can we name the directories something based on the content of the tutorial. Like for this one:

mkdir op-tracing-tutorial

@krofax
Copy link
Contributor Author

krofax commented Oct 18, 2024

For all of these tutorials usually there is a step for running the command mkdir op-sample-project if we assume that developers go through a couple of these they may have trouble creating the directories since they will have to overwrite their old one. Can we name the directories something based on the content of the tutorial. Like for this one:

mkdir op-tracing-tutorial

@ZakAyesh Updated the tut, can you take a second review?

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between f03401c and 665c798.

📒 Files selected for processing (1)
  • public/tutorials/sdk-trace-txns.js (1 hunks)
🧰 Additional context used

public/tutorials/sdk-trace-txns.js Show resolved Hide resolved
public/tutorials/sdk-trace-txns.js Show resolved Hide resolved
Copy link
Contributor

@brokewhale brokewhale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the chains are not imported, they should

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Outside diff range and nitpick comments (2)
pages/builders/app-developers/tutorials/sdk-trace-txns.mdx (1)

49-55: LGTM with a minor suggestion.

The changes accurately reflect the use of viem's getTransactionReceipt function and provide appropriate RPC URLs for test networks. The explanation about using an RPC provider that supports indexed event queries is helpful for users.

Consider adding a brief note explaining that these are test network URLs and that users should use mainnet URLs for production environments.

pages/builders/app-developers/transactions/estimates.mdx (1)

Line range hint 1-24: Overall adherence to coding guidelines is commendable.

The document generally follows the provided coding guidelines, including:

  • Use of proper nouns instead of personal pronouns
  • Appropriate capitalization for emphasis
  • Correct capitalization of proper nouns in sentences
  • Consistent use of the Oxford comma
  • Proper title case for headers
  • Correct spelling and grammar

To ensure complete compliance, a final proofreading of the entire document is recommended.

Consider performing a final proofreading pass to catch any minor inconsistencies that may have been overlooked.

🧰 Tools
🪛 LanguageTool

[style] ~26-~26: ‘exactly the same’ might be wordy. Consider a shorter alternative.
Context: ...> A transaction's execution gas fee is exactly the same fee that you would pay for the same tra...

(EN_WORDINESS_PREMIUM_EXACTLY_THE_SAME)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 665c798 and 4a5abc0.

📒 Files selected for processing (2)
  • pages/builders/app-developers/transactions/estimates.mdx (1 hunks)
  • pages/builders/app-developers/tutorials/sdk-trace-txns.mdx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
pages/builders/app-developers/transactions/estimates.mdx (1)

Pattern **/*.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. Use bold for prominence instead of all caps or italics.
  • Ensure proper nouns are capitalized in sentences.
  • Apply the Oxford comma.
  • Use proper title case for headers, 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).
    "
pages/builders/app-developers/tutorials/sdk-trace-txns.mdx (1)

Pattern **/*.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. Use bold for prominence instead of all caps or italics.
  • Ensure proper nouns are capitalized in sentences.
  • Apply the Oxford comma.
  • Use proper title case for headers, 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).
    "
🔇 Additional comments (14)
pages/builders/app-developers/tutorials/sdk-trace-txns.mdx (14)

4-4: LGTM: Description updated correctly.

The description has been appropriately updated to reflect the use of the viem library instead of the Optimism SDK, accurately representing the content of the updated tutorial.


10-12: LGTM: Introduction updated correctly.

The introduction has been appropriately updated to reflect the use of the viem library instead of the Optimism SDK. The link to the viem library documentation is correctly provided, which is helpful for users who want to learn more about the library.


22-23: LGTM: Project setup instructions updated correctly.

The project setup instructions have been appropriately updated to reflect the use of the viem library instead of the Optimism SDK. This change is consistent with the overall update of the tutorial.


59-64: LGTM: Helpful Node REPL instructions added.

The addition of instructions for starting and using the Node REPL is beneficial, especially for users who may be new to this environment. The explanation is clear and concise, which will help users follow the tutorial more easily.


70-72: LGTM: Viem import statement added correctly.

The import statement for viem has been correctly added, which is consistent with the use of viem throughout the updated tutorial.


76-81: LGTM: RPC URL import instructions added.

The instructions for importing RPC URLs have been clearly added and are consistent with the earlier introduction of these URLs. The use of environment variables for managing configuration is a good practice that has been correctly implemented here.


86-88: LGTM: Deposit transaction hash setup instructions added.

The instructions for setting up a deposit transaction hash have been clearly added. The explanation about deposit tracing being based on the transaction hash is helpful for users' understanding. The flexibility to use a custom transaction hash is a good feature for users following the tutorial.


93-95: LGTM: Withdrawal transaction hash setup instructions added.

The instructions for setting up a withdrawal transaction hash have been clearly added. The explanation about withdrawal tracing being based on the transaction hash is helpful and consistent with the previous deposit transaction hash setup. The flexibility to use a custom transaction hash is maintained, which is beneficial for users following the tutorial.


100-100: LGTM: RPC provider creation updated to use viem.

The code snippet for creating RPC providers has been correctly updated to use viem's createPublicClient function. The creation of separate clients for L1 and L2 is appropriate and consistent with the tutorial's purpose of tracing transactions across both layers.


105-119: LGTM: Deposit tracing instructions updated to use viem.

The instructions for tracing a deposit have been appropriately updated to use viem functions. The step-by-step guidance is clear and demonstrates the correct use of viem's getTransactionReceipt function. These changes are consistent with the overall update to use viem instead of the Optimism SDK throughout the tutorial.


124-127: LGTM: Deposit log parsing instructions added.

The addition of instructions for parsing deposit logs is a valuable inclusion. It demonstrates how to use viem's functions to extract detailed information from the transaction logs, which is helpful for users who need to perform more in-depth analysis of their deposits.


131-133: LGTM: Deposit transaction retrieval updated to use viem.

The instructions for getting the deposit transaction have been correctly updated to use viem. The changes are consistent with the overall update of the tutorial, and they clearly demonstrate how to query for the L2 transaction that executed the deposit using viem's functions.


138-151: LGTM: Withdrawal tracing instructions updated to use viem.

The instructions for tracing a withdrawal have been appropriately updated to use viem functions. The step-by-step guidance is clear and demonstrates the correct use of viem's functions for getting withdrawal status and transaction receipt. These changes are consistent with the overall update to use viem instead of the Optimism SDK throughout the tutorial.


156-159: LGTM: Withdrawal log parsing instructions added.

The addition of instructions for parsing withdrawal logs is a valuable inclusion. It demonstrates how to use viem's functions to extract detailed information from the transaction logs, which is helpful for users who need to perform more in-depth analysis of their withdrawals.

pages/builders/app-developers/tutorials/sdk-trace-txns.mdx Outdated Show resolved Hide resolved
pages/builders/app-developers/tutorials/sdk-trace-txns.mdx Outdated Show resolved Hide resolved
pages/builders/app-developers/transactions/estimates.mdx Outdated Show resolved Hide resolved

```js file=<rootDir>/public/tutorials/sdk-trace-txns.js#L10-L11 hash=02141d8cb077764665c61fc48e18ed04
```
```js file=<rootDir>/public/tutorials/sdk-trace-txns.js#L9-L10 hash=014f5d17eddb45fc081c7db9f33361b4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Earlier in the tutorial we exported the RPC URL as an environment variable, should we just use that rather than declaring a new variable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it seems the L2 RPC is missing a variable here which is necessary for the code below

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants