-
Notifications
You must be signed in to change notification settings - Fork 57
clean: re-organize the DA and Sequencing folders #590
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
Warning Rate limit exceeded@pthmas has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 23 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 (1)
WalkthroughThe sidebar configuration and documentation structure were reorganized. New documentation was added for data availability, sequencing, and execution under the "Learn" section. Several guides were removed or consolidated, including deletion of specific DA and sequencing guides. The sidebar now features clearer grouping of related topics and updated navigation paths. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Sidebar
participant Docs
User->>Sidebar: Navigates documentation
Sidebar->>Docs: Shows grouped sections (Learn, How To Guides)
Docs-->>User: Presents new/updated content (Data Availability, Sequencing, Execution)
Possibly related PRs
Suggested reviewers
Poem
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
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.
Summary of Changes
Hello @pthmas, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly reorganizes the project's documentation by creating a new 'learn' section for conceptual topics like Data Availability and Sequencing. It aims to improve the clarity and discoverability of information by centralizing key knowledge and providing dedicated guides for core components.
Highlights
- Documentation Reorganization: Core documentation on Data Availability (DA) and Sequencing has been restructured, moving conceptual and detailed content into a new
/learn
section for better accessibility and clarity. - New DA Overview: A new
learn/data-availability.md
file has been added, providing a comprehensive overview of Data Availability within the Rollkit framework. - New Single Sequencer Guide: A dedicated guide for the single sequencer model has been introduced at
learn/sequencing/single.md
, detailing its functionality, benefits, and considerations. - Navigation Updates: The sidebar navigation in
.vitepress/config.ts
has been updated to reflect the new documentation structure, including new top-level 'Data Availability' and 'Sequencing' entries under the 'Learn' section, and updated links for existing guides.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
|
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.
Code Review
This pull request reorganizes documentation for Data Availability and Sequencing by moving content from guides
to a new learn
section and adding new explanatory pages. The changes improve the structure of the documentation. A critical typo in the VitePress configuration would lead to broken links, and there are also a few medium-severity issues in the new markdown files, including a typo and inconsistent link formatting.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
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
♻️ Duplicate comments (2)
learn/sequencing/overview.md (1)
28-32
: Internal links still include “.md” and point to non-existent filesThis was flagged previously. VitePress resolves extension-less links automatically and it keeps URLs clean.
Additionally,based.md
andforced-inclusion.md
don’t exist yet, so these links will break even if the extension is removed.-* [single-sequencer](/learn/sequencing/single.md) ... -* [based-sequencer](/learn/sequencing/based.md) ... -* [forced-inclusion-sequencer](/learn/sequencing/forced-inclusion.md) ... +* [single-sequencer](/learn/sequencing/single) ... +* [based-sequencer](/learn/sequencing/based) _(coming soon)_ +* [forced-inclusion-sequencer](/learn/sequencing/forced-inclusion) _(coming soon)_learn/data-availability.md (1)
39-41
: Prefer extension-less internal linksSame point raised earlier: drop “.md” to keep URLs stable and clean.
- [Set up a local DA](/guides/da/local-da.md) - [Set up Celestia DA](/guides/da/celestia-da.md) + [Set up a local DA](/guides/da/local-da) + [Set up Celestia DA](/guides/da/celestia-da)
🧹 Nitpick comments (2)
.vitepress/config.ts (1)
235-236
: Nit: sentence-case article for consistencySidebar items generally use sentence-case (“Build a chain”, “Quick start guide”).
Change “Deploy A Local DA” → “Deploy a local DA” for consistency.- text: "Deploy A Local DA", + text: "Deploy a Local DA",learn/sequencing/single.md (1)
16-18
: Cross-link to DA documentation for better discoverabilitySince the DA layer is first mentioned here, linking to the new Data-Availability page improves navigation.
- - The sequencer posts the block data to the configured DA layer (e.g., Celestia, Avail, etc.). + - The sequencer posts the block data to the configured [data-availability layer](/learn/data-availability) (e.g., Celestia, Avail, etc.).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.vitepress/config.ts
(3 hunks)guides/da/overview.md
(0 hunks)guides/sequencing/single.md
(0 hunks)learn/data-availability.md
(1 hunks)learn/sequencing/overview.md
(1 hunks)learn/sequencing/single.md
(1 hunks)
💤 Files with no reviewable changes (2)
- guides/sequencing/single.md
- guides/da/overview.md
🧰 Additional context used
🪛 LanguageTool
learn/data-availability.md
[style] ~30-~30: Consider using a more concise synonym.
Context: ...llkit enables rollups to be deployed in a variety of environments, from local devnets to sca...
(A_VARIETY_OF)
🔇 Additional comments (1)
.vitepress/config.ts (1)
198-211
: No broken links in “Sequencing” sidebar
Verified that all four pages—overview
,single
,based
, andforced-inclusion
—exist underlearn/sequencing
. All sidebar links are valid.
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 (5)
learn/execution/abci.md (5)
3-3
: Tighten wording by dropping filler phraseThe phrase “This means you can use a variety of” is verbose and can be trimmed without losing meaning.
-Rollkit is designed to be modular and flexible, allowing different execution layers to be plugged in via the ABCI (Application Blockchain Interface) protocol. This means you can use a variety of ABCI-compatible applications as the execution environment for your rollup. +Rollkit is designed to be modular and flexible, allowing different execution layers to be plugged in via the ABCI (Application Blockchain Interface) protocol, so any ABCI-compatible application can serve as the execution environment for your rollup.
7-11
: Use relative links & reference concrete integration guideExternal links are useful, but an internal “Cosmos SDK integration” guide (if present or planned) would be more actionable and future-proof. Consider:
-### Cosmos SDK App -A Cosmos SDK-based application can serve as the execution layer for a Rollkit rollup. This allows you to leverage the rich ecosystem and features of the Cosmos SDK, including modules for staking, governance, IBC, and more. - -- [Cosmos SDK Documentation](https://docs.cosmos.network/) +### Cosmos SDK App +A Cosmos SDK-based application can serve as the execution layer for a Rollkit rollup, letting you reuse modules for staking, governance, IBC and more. + +- [Official Cosmos SDK docs](https://docs.cosmos.network/) +- [Rollkit × Cosmos SDK integration guide](../execution/cosmos-sdk.md) <!-- add/ensure this doc exists -->
12-16
: Mirror structure & clarify smart-contract angleMinor restructuring improves parallelism with the previous subsection.
-### CosmWasm -CosmWasm is a smart contract platform built for the Cosmos ecosystem. It is ABCI-compatible and can be integrated as the execution layer in Rollkit, enabling your rollup to support WebAssembly (Wasm) smart contracts. - -- [CosmWasm Documentation](https://docs.cosmwasm.com/) +### CosmWasm +CosmWasm is a smart-contract platform for the Cosmos ecosystem. Because it exposes an ABCI interface, it can act as the execution layer in Rollkit, enabling Wasm smart-contracts on your rollup. + +- [Official CosmWasm docs](https://docs.cosmwasm.com/) +- [Rollkit × CosmWasm integration guide](../execution/cosmwasm.md)
17-22
: Consider adding a simple sequence diagramThe bullet list is clear, but a visual (Mermaid) sequence diagram would convey the ABCI message flow at a glance.
Example insertion:
```mermaid sequenceDiagram participant User participant Rollkit participant ExecutionLayer User->>Rollkit: Submit Tx Rollkit->>ExecutionLayer: DeliverTx (ABCI) ExecutionLayer-->>Rollkit: Response Rollkit-->>User: Tx resultHelps readers who are new to ABCI quickly grasp the interaction. --- `25-29`: **Add a “Limitations” or “Gotchas” callout** The Benefits section is great, but balancing it with limitations (e.g., ABCI overhead, consensus-state sync requirements, version-compatibility) sets proper expectations and pre-empts support questions. A simple admonition block could work: ```markdown ::: warning Limitations ABCI adds an extra process boundary, so latency is higher than monolithic nodes. Ensure version compatibility between Rollkit and the execution layer during upgrades. :::
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
learn/execution/abci.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
learn/execution/abci.md
[style] ~3-~3: Consider using a more concise synonym.
Context: ...rface) protocol. This means you can use a variety of ABCI-compatible applications as the exe...
(A_VARIETY_OF)
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.
needs some refinement, the information isnt 100% true in all the cases. the execution seems to account for what was there before not what is here today. we should make sure it represents all there is
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.
left some comments, its looking better
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
♻️ Duplicate comments (1)
learn/execution.md (1)
11-18
: Clarify ABCI scope and add experimental status disclaimerPast feedback asked to keep ABCI confined to Cosmos SDK. Spell that out explicitly, and warn that the Go ABCI adapter is still under active development to set reader expectations.
-Rollkit natively supports Cosmos SDK-based applications as the execution layer for a rollup via the ABCI (Application Blockchain Interface) protocol. +Rollkit natively supports Cosmos SDK–based applications as the execution layer via the ABCI (Application Blockchain Interface) protocol **(Cosmos SDK only; other execution layers do _not_ use ABCI)**. @@ -- [Rollkit ABCI Adapter](https://github.com/rollkit/go-execution-abci) +- [Rollkit ABCI Adapter (early preview)](https://github.com/rollkit/go-execution-abci) — API subject to change
🧹 Nitpick comments (3)
learn/execution.md (2)
5-5
: Tighten wording & fix compound-modifier hyphenationThe phrase is verbose and the compound modifier “Reth compatible” should be hyphenated.
-This means you can use a variety of Cosmos SDK or Reth compatible applications as the execution environment for your rollup. +This means you can plug in Cosmos SDK- or Reth-compatible applications as the execution environment for your rollup.
19-26
: Flag Reth integration as experimental & link to docsReth support is rapidly evolving. A short note avoids misleading readers.
-Reth is a high-performance Ethereum execution client written in Rust. Rollkit can integrate Reth as an execution layer, enabling Ethereum-compatible rollups to process EVM transactions and maintain Ethereum-like state. +Reth is a high-performance Ethereum execution client written in Rust. **Rollkit’s Reth integration is experimental** but already enables Ethereum-compatible rollups to process EVM transactions and maintain Ethereum-like state.learn/sequencing/single.md (1)
41-45
: Consider adding “Limitations” sectionReaders benefit from understanding trade-offs (e.g., censorship risk, liveness tied to one node). A short “Limitations” section would balance the “Advantages” list.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
learn/execution.md
(1 hunks)learn/sequencing/single.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
learn/execution.md
[style] ~5-~5: Consider using a more concise synonym.
Context: ...xecution layer. This means you can use a variety of Cosmos SDK or Reth compatible applicati...
(A_VARIETY_OF)
1987f5b
to
a32996b
Compare
learn/sequencing/single.md
Outdated
## Advantages | ||
|
||
- **Simplicity:** Easy to set up and operate, making it ideal for development, testing, and small-scale deployments. | ||
- **Low Latency:** Fast block production and transaction inclusion, since there is no consensus overhead among multiple sequencers. | ||
|
||
## Use Cases | ||
|
||
- Production rollups seeking simplicity and performance | ||
- Prototyping and development | ||
- Private or permissioned rollups | ||
- Projects that value deterministic ordering and operational control |
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.
these sections read weird. use cases feels too naive, it should be its own section with case studies.
advantages would be good to call something else as its unclear what the comparison is on
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.
Let's remove the use cases for now and write a proper section later.
Rephrased the Advantages to add comparisons.
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
♻️ Duplicate comments (1)
learn/execution.md (1)
11-13
: Clarify ABCI scope to avoid earlier confusion.Previous review feedback noted that ABCI is specific to Cosmos SDK. Consider explicitly stating that ABCI support is only for Cosmos-SDK execution layers to avoid implying other environments must implement it.
Example:
-Rollkit natively supports Cosmos SDK-based applications as the execution layer for a rollup via the ABCI (Application Blockchain Interface) protocol. +For Cosmos-SDK execution layers, Rollkit communicates via the ABCI (Application Blockchain Interface) protocol.
🧹 Nitpick comments (4)
learn/execution.md (4)
3-3
: Link to a movingmain
branch is brittle – pin to a tagged commit or docs site.Linking directly to
core/execution/execution.go
on themain
branch risks future 404s or the code fragment drifting. Prefer:-[see execution.go](https://github.com/rollkit/rollkit/blob/main/core/execution/execution.go) +[see execution.go](https://github.com/rollkit/rollkit/blob/<commit-sha>/core/execution/execution.go)or point to the rendered API docs instead.
5-5
: Minor wording tweak for conciseness.Static-analysis flagged “a variety of … applications”. “Various” is shorter and reads better:
-This means you can use a variety of Cosmos SDK or Reth compatible applications as the execution environment for your rollup: +This means you can use various Cosmos SDK- or Reth-compatible applications as the execution environment for your rollup:
21-22
: Flag Reth integration as experimental / fast-moving.Reth is still under heavy development. A short disclaimer helps set expectations:
-Reth is a high-performance Ethereum execution client written in Rust. Rollkit can integrate Reth as an execution layer, enabling Ethereum-compatible rollups to process EVM transactions and maintain Ethereum-like state. +Reth is a high-performance Ethereum execution client written in Rust. *Integration with Rollkit is currently experimental* but already enables Ethereum-compatible rollups to process EVM transactions and maintain Ethereum-like state.
28-34
: Add a simple sequence or architecture diagram to cement the mental model.The bullet list is helpful, but a diagram (mermaid or image) showing Rollkit <-> Execution-Layer calls will greatly improve comprehension, especially for new users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
learn/execution.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
learn/execution.md
[style] ~5-~5: Consider using a more concise synonym.
Context: ...xecution layer. This means you can use a variety of Cosmos SDK or Reth compatible applicati...
(A_VARIETY_OF)
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)
learn/execution.md (2)
3-6
: Tighten the wording & drop the colon spliceThe second sentence is a run-on joined by a colon and can be trimmed for clarity:
-This means you can use a variety of Cosmos SDK or Reth compatible applications as the execution environment for your rollup: choose the execution environment that best fits your use case. +Therefore, you can plug in any Cosmos SDK- or Reth-compatible application that best fits your rollup’s requirements.
17-25
: Add caveat on Reth integration maturityReth support is still experimental and behind the
lumen
repo. Mentioning this helps set correct expectations and avoid premature production adoption:-Reth is a high-performance Ethereum execution client written in Rust. Rollkit can integrate Reth as an execution layer, enabling Ethereum-compatible rollups to process EVM transactions and maintain Ethereum-like state. +Reth is a high-performance Ethereum execution client written in Rust. Rollkit **can** integrate Reth as an execution layer (currently experimental), enabling Ethereum-compatible rollups to process EVM transactions and maintain Ethereum-like state.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
learn/execution.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
learn/execution.md
[style] ~5-~5: Consider using a more concise synonym.
Context: ...xecution layer. This means you can use a variety of Cosmos SDK or Reth compatible applicati...
(A_VARIETY_OF)
🔇 Additional comments (1)
learn/execution.md (1)
11-15
: Re-verify the “via the ABCI protocol” claimPast feedback (see previous review thread) indicated confusion about whether Rollkit actually communicates with Cosmos-SDK apps over ABCI or through its own adapter layer. If the adapter implements a superset or a different transport, we risk misleading readers.
Please double-check the integration path and rephrase if it is not literally ABCI:
-Rollkit natively supports Cosmos SDK-based applications as the execution layer for a rollup via the ABCI (Application Blockchain Interface) protocol. +Rollkit natively supports Cosmos SDK-based applications through its ABCI-compatible adapter layer.
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.
left one comment, other than that nice job
Co-authored-by: Marko <marko@baricevic.me>
Overview
guides/overview.md
file as it became a duplicate.Summary by CodeRabbit
New Features
Documentation