-
Notifications
You must be signed in to change notification settings - Fork 52
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
feat: add DA overview and bring local-da into tutorials #509
Conversation
WalkthroughThe pull request introduces several modifications to the VitePress site configuration and tutorial documents related to Data Availability (DA). It adds new sidebar entries for the "Tutorials" section, updates existing links, and modifies the Changes
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
CodeRabbit Configuration File (
|
|
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
🧹 Outside diff range and nitpick comments (12)
tutorials/da/local-da.md (3)
8-13
: Consider enhancing the introduction with more context.While the introduction provides good prerequisite information, it could be improved by:
- Briefly explaining what local-da is and its benefits
- Specifying the version of local-da being used
Consider expanding the introduction like this:
-This tutorial serves as a comprehensive guide for using the [local-da](https://github.com/rollkit/local-da) with your chain. +This tutorial serves as a comprehensive guide for using [local-da](https://github.com/rollkit/local-da) - a lightweight, local Data Availability (DA) layer implementation perfect for testing and development. This allows you to develop and test your chain without connecting to production DA networks.
28-52
: Enhance running instructions with more context.The running instructions could be improved by:
- Explaining the differences between Quick Start and Wordle Chain configurations
- Using a more generic timestamp in the log example
Consider these improvements:
## Run your rollup -Start your rollup node with the following command, ensuring to include the DA address flag: +Start your rollup node with one of the following commands, depending on your setup: ::: code-group ```sh [Quick Start] +# Basic configuration for testing rollkit start --rollkit.da_address http://localhost:7980+# Advanced configuration with aggregator and custom rollup ID rollkit start \ --rollkit.aggregator \ --rollkit.da_address http://localhost:7980 \ --rollkit.sequencer_rollup_id wordle
:::
You should see the following log message indicating that your rollup is connected to the local DA network:
-I[2024-11-15|14:54:19.842] DA server is already running module=main address=http://localhost:7980 +I[<timestamp>] DA server is already running module=main address=http://localhost:7980--- `53-55`: **Enhance summary with next steps and troubleshooting.** The summary could be more helpful by including: 1. Next steps for users 2. Common troubleshooting tips 3. Links to related resources Consider expanding like this: ```diff ## Summary -By following these steps, you will set up a local DA network node and configure your rollup to post data to it. This setup is useful for testing and development in a controlled environment. +By following these steps, you will set up a local DA network node and configure your rollup to post data to it. This setup is useful for testing and development in a controlled environment. + +### Next Steps +- Learn how to [monitor your local DA node](/monitoring) +- Explore [advanced configuration options](/configuration) +- Set up [multiple rollup nodes](/advanced/multiple-nodes) + +### Troubleshooting +If you encounter issues: +1. Ensure the local DA node is running: `curl http://localhost:7980/health` +2. Check logs: `journalctl -u local-da -f` +3. Visit our [troubleshooting guide](/troubleshooting) for more help
tutorials/da/overview.md (2)
7-11
: Define "DA" on first use for clarityConsider expanding "DA" to "Data Availability (DA)" on first use to improve readability for newcomers.
-# DA +# Data Availability (DA)
39-39
: Update the log timestampThe log example shows a future date (2024-11-15). Consider using a past date or removing the timestamp if it's not crucial to the example.
-I[2024-11-15|14:09:41.735] Starting mock DA server module=main address=http://localhost:26658 +I[2023-11-15|14:09:41.735] Starting mock DA server module=main address=http://localhost:26658tutorials/da/celestia-da.md (1)
Line range hint
92-102
: Consider enhancing namespace generation guidanceWhile the OpenSSL command for namespace generation is helpful, consider adding:
- A note about namespace uniqueness importance
- An example of what the final namespace should look like after replacement
tutorials/da/avail-da.md (5)
Line range hint
89-93
: Fix the localhost URL schemeThe URL
https://localhost:8000
is using HTTPS which is unlikely for a local development setup. It should probably behttp://localhost:8000
instead.-DA_BLOCK_HEIGHT=$(curl https://localhost:8000/v1/latest_block | jq -r '.result.block.header.height') +DA_BLOCK_HEIGHT=$(curl http://localhost:8000/v1/latest_block | jq -r '.result.block.header.height')
Line range hint
106-112
: Add error handling to the startup commandThe current command doesn't handle potential failures gracefully. Consider adding error handling and validation.
- gmd start \ - --rollkit.aggregator \ - --rollkit.da_address="grpc://localhost:3000" \ - --rollkit.da_start_height $DA_BLOCK_HEIGHT \ - --minimum-gas-prices="0.1stake" + if [ -z "$DA_BLOCK_HEIGHT" ]; then + echo "Error: Failed to get DA block height" + exit 1 + fi + + gmd start \ + --rollkit.aggregator \ + --rollkit.da_address="grpc://localhost:3000" \ + --rollkit.da_start_height "$DA_BLOCK_HEIGHT" \ + --minimum-gas-prices="0.1stake" || { + echo "Error: Failed to start gmd" + exit 1 + }
Line range hint
29-30
: Fix malformed Markdown linkThe faucet link has double parentheses which breaks the Markdown formatting.
-- To fund your wallet address for using Turing Testnet: get AVAIL tokens from [the faucet]((https://faucet.avail.tools/)) ++ To fund your wallet address for using Turing Testnet: get AVAIL tokens from [the faucet](https://faucet.avail.tools/)
Line range hint
117-120
: Improve the gas price section integrationThe gas price information box seems abruptly placed. Consider:
- Moving it to the configuration section
- Expanding on the relationship between DA network gas prices and rollup gas prices
- Adding examples of typical gas price configurations
Warning message needs more context and alternative path
The tutorial appears to be actively maintained and contains detailed instructions, but the warning message should be enhanced since:
- The tutorial contains complete implementation steps and recent references (e.g., Turing testnet)
- The content seems well-structured and comprehensive
- Other files in the repository use warning blocks effectively with more context
Suggested improvements for the warning message:
- Specify which parts of the tutorial are broken (e.g., specific commands, configurations)
- Add a temporary workaround if available (e.g., using local development setup)
- Include the last known working version/configuration
- Link to the documentation for alternative DA solutions mentioned in the repository
🔗 Analysis chain
Line range hint
4-8
: Consider adding more context to the warning messageWhile it's good to warn users about the broken tutorial, consider:
- Specifying which parts are broken
- Adding an estimated timeline for the fix
- Providing alternative approaches if available
This helps users make an informed decision about whether to proceed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any updates or discussions on the linked issue gh issue view rollkit/avail-da#57 --json title,state,commentsLength of output: 107
Script:
#!/bin/bash # Let's check if there are any other similar warning messages in the repository rg -l ":::warning" --type md # Also check the content of the tutorial file to understand the context better cat tutorials/da/avail-da.mdLength of output: 5100
.vitepress/config.ts (1)
Line range hint
144-159
: Fix the Open Graph description meta tag.There's a syntax error in setting the
og:description
meta tag content.Apply this diff to fix the issue:
{ name: "og:description", - content: pageData.frontmatter.layout === `${pageData.description}`, + content: pageData.description, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
.vitepress/config.ts
(1 hunks)tutorials/da/avail-da.md
(1 hunks)tutorials/da/celestia-da.md
(2 hunks)tutorials/da/local-da.md
(1 hunks)tutorials/da/overview.md
(1 hunks)
🧰 Additional context used
🪛 Markdownlint
tutorials/da/overview.md
22-22: Column: 1
Hard tabs
(MD010, no-hard-tabs)
23-23: Column: 1
Hard tabs
(MD010, no-hard-tabs)
25-25: Column: 1
Hard tabs
(MD010, no-hard-tabs)
26-26: Column: 1
Hard tabs
(MD010, no-hard-tabs)
🔇 Additional comments (10)
tutorials/da/local-da.md (2)
1-7
: LGTM! File setup follows VitePress best practices.
The setup properly handles markdown linting exceptions and imports constants for version management.
14-23
: Security considerations for installation and network configuration.
The current installation method and network configuration have several security considerations:
- The curl | bash pattern can be unsafe without checksum verification
- The exposed port 7980 might need firewall configuration
Consider:
- Adding SHA256 checksum verification
- Adding firewall configuration guidance
- Warning about running in production environments
Example improvement:
To set up a local DA network node on your machine, run the following script to install and start the local DA node:
+
+> ⚠️ Warning: This installation method is intended for development environments only. For production deployments, please follow security best practices and verify checksums.
+
```bash-vue
curl -sSL https://rollkit.dev/install-local-da.sh | bash -s {{constants.localDALatestTag}}
-This script will build and run the node, which will then listen on port 7980
.
+This script will build and run the node, which will then listen on port 7980
. Ensure this port is only accessible locally if you're running in a shared environment:
+
+bash +sudo ufw allow from 127.0.0.1 to any port 7980 +
</details>
<details>
<summary>tutorials/da/overview.md (2)</summary>
`48-50`: **LGTM!**
The DA layers section is well-organized and uses consistent link formatting.
---
`30-30`: **Verify the relative link paths**
The links `[Celestia](celestia-da)` and `[Avail](avail-da)` should be verified to ensure they correctly point to the intended documentation pages.
<details>
<summary>✅ Verification successful</summary>
**Links verified and correctly configured**
The relative links `[Celestia](celestia-da)` and `[Avail](avail-da)` are properly configured as both target files exist in the same directory:
- `tutorials/da/celestia-da.md`
- `tutorials/da/avail-da.md`
</details>
<details>
<summary>🏁 Scripts executed</summary>
The following scripts were executed for the analysis:
Script:
```shell
#!/bin/bash
# Description: Verify that the linked files exist
fd --type f "celestia-da.md|avail-da.md" tutorials/da/
Length of output: 109
tutorials/da/celestia-da.md (4)
59-76
: Well-structured network-specific commands!
The code group organization effectively separates commands for different networks, making it clear which command to use for each network type.
Line range hint 11-13
: Verify the quick start tutorial reference
The introduction references the quick start tutorial. Let's verify this link exists.
#!/bin/bash
# Search for the quick start tutorial file
fd --type f "quick-start.md$" tutorials/
Line range hint 127-134
: Verify block explorer links
Let's verify that all block explorer links are accessible and current.
#!/bin/bash
# Extract and verify all URLs in the explorer section
rg -o 'https://[^ )"]+' | while read -r url; do
curl --head --silent --fail "$url" || echo "Failed: $url"
done
5-5
: Verify the constants import path
The import path has been updated to use a relative path. Let's verify this path exists and is correct.
✅ Verification successful
Let me try another approach to verify the constants import path.
Since the file .vitepress/constants/constants.js
exists at the exact path referenced in the import statement ../../.vitepress/constants/constants.js
, the import path is correct.
Import path is valid ✅
The relative import path ../../.vitepress/constants/constants.js
correctly resolves to the existing constants file from the location of tutorials/da/celestia-da.md
.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the constants file exists at the specified location
fd --type f "constants.js$" .vitepress/constants/
Length of output: 430874
Script:
#!/bin/bash
# Check if the constants file exists and its content
fd -t f "constants.js$" .vitepress/
Length of output: 71
tutorials/da/avail-da.md (1)
Line range hint 47-54
: Consider versioning the config.yaml template
The link to the config.yaml template points to the main branch which could change over time. Consider:
- Pointing to a specific version/tag
- Including the essential configuration inline
- Adding validation for the config values
.vitepress/config.ts (1)
226-240
: LGTM! The DA section structure is well-organized.
The sidebar configuration follows a clear hierarchical structure for DA tutorials, with a logical progression from overview to specific implementations.
Let's verify that all referenced markdown files exist:
✅ Verification successful
All referenced DA tutorial markdown files exist and are properly organized
The verification confirms that all markdown files referenced in the sidebar configuration are present in the correct location:
- tutorials/da/overview.md
- tutorials/da/local-da.md
- tutorials/da/celestia-da.md
- tutorials/da/avail-da.md
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of DA tutorial files
# Check for required markdown files
for file in overview local-da celestia-da avail-da; do
echo "Checking for tutorials/da/$file.md"
fd -t f "$file.md$" tutorials/da
done
Length of output: 815
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
🧹 Outside diff range and nitpick comments (1)
.vitepress/config.ts (1)
Line range hint
152-169
: Fix Open Graph description metadata logicThe current implementation incorrectly compares
layout
withdescription
. Theog:description
meta tag should follow similar conditional logic as the title.Apply this fix:
pageData.frontmatter.head.push([ "meta", { name: "og:title", content: pageData.frontmatter.layout === "home" ? `Rollkit` : `${pageData.title} | Rollkit`, }, { name: "og:description", - content: pageData.frontmatter.layout === `${pageData.description}`, + content: pageData.frontmatter.layout === "home" + ? "The unstoppable stack" + : pageData.description || "The unstoppable stack", }, ]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.vitepress/config.ts
(1 hunks)
🔇 Additional comments (1)
.vitepress/config.ts (1)
226-240
: LGTM: Sidebar structure aligns with documentation reorganization
The changes effectively organize the DA-related tutorials under a consistent path structure /tutorials/da/*
and add new entries for the overview and local-da guide.
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.
utACK
Overview
This PR pulls in the local-da how to guide into the DA section and adds a DA overview to explain the go-da interface.
Summary by CodeRabbit