-
Notifications
You must be signed in to change notification settings - Fork 58
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe documentation was reorganized by introducing a new "Learn" section for "Data Availability" and "Sequencing," moving and expanding related guides. Sidebar navigation was updated to reflect these changes. Several old guide files were deleted, and new, more detailed explanatory documents were added under the "learn" directory. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Docs Sidebar
participant Learn Section
User->>Docs Sidebar: Navigates documentation
Docs Sidebar->>Learn Section: Shows "Data Availability" and "Sequencing"
User->>Learn Section: Selects topic (e.g., Single Sequencer)
Learn Section->>User: Presents detailed explanation
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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)
learn/data-availability.md
Outdated
|
||
Rollkit is designed to be DA-agnostic, meaning it can integrate with different data availability layers depending on your needs. The main options are: | ||
|
||
- **Mock Data Availability (Mock DA):** |
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.
we can remove this section since its an internal mock
@@ -0,0 +1,41 @@ | |||
# Data Availability in Rollkit |
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.
lets link out to some DA articles so people can get a deeper explanation of DA
learn/data-availability.md
Outdated
- If data is unavailable, users cannot verify the rollup's state or submit fraud proofs. | ||
|
||
- **Interoperability:** | ||
- By supporting multiple DA layers, Rollkit enables rollups to be deployed in a variety of environments, from local devnets to scalable, decentralized networks. |
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.
while we do allow supporting multiple DA layers we will only support one. in the future we may remove the interface in order to not allow switching.
learn/data-availability.md
Outdated
## Best Practices | ||
|
||
- **Use Mock or Local DA only for development and testing.** | ||
- **For production, always use a decentralized DA layer that implements the Rollkit DA interface.** |
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.
dont think we need this as we should either point people to local DA or celestia testnets. would be good to add links on how to connect to arabica, mocha and/or mainnet
learn/execution/abci.md
Outdated
@@ -0,0 +1,29 @@ | |||
# ABCI-Compatible Execution Layers in Rollkit | |||
|
|||
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. |
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.
we have a general purpose execution inteface, we should touch on the general purpose interface and then mention abci and reth as two options we support by default
learn/sequencing/overview.md
Outdated
* [based-sequencer](/learn/sequencing/based.md) - A more decentralized model where multiple sequencers work together to order transactions and produce blocks, improving censorship resistance (Not available yet). | ||
|
||
* [forced-inclusion-sequencer](/learn/sequencing/forced-inclusion.md) - A model that ensures all transactions are included in the rollup, even if they are not ordered by the sequencer, providing strong guarantees against censorship. (Not available yet). |
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 will be an option on single so its not really its own sequencing model.
learn/sequencing/overview.md
Outdated
* [based-sequencer](/learn/sequencing/based.md) - A more decentralized model where multiple sequencers work together to order transactions and produce blocks, improving censorship resistance (Not available yet). | ||
|
||
* [forced-inclusion-sequencer](/learn/sequencing/forced-inclusion.md) - A model that ensures all transactions are included in the rollup, even if they are not ordered by the sequencer, providing strong guarantees against censorship. (Not available yet). |
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 description isnt 100%, id just remove mention of based for now
learn/sequencing/single.md
Outdated
- **Centralization:** The sequencer is a single point of control. However, this is often acceptable for many applications, especially in early stages or permissioned environments. | ||
- **No Fault Tolerance:** If the sequencer is unavailable, the rollup cannot make progress. High-availability setups and monitoring can mitigate this risk. |
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.
lets remove these
learn/sequencing/single.md
Outdated
## Future-Proofing | ||
|
||
While the single sequencer model is robust and production-ready, Rollkit is designed to support more advanced sequencing architectures in the future. Projects can seamlessly upgrade to decentralized sequencing when the technology and their needs mature. |
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.
if we dont have anything to point users to we shouldnt add sections like this
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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
learn/data-availability.md (1)
37-41
: Omit .md extension in internal links for cleaner URLs
Same issue flagged in earlier review; suggestion still applies.-[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)
learn/data-availability.md (2)
3-4
: Fix grammar & clarify DA’s relationship to RollkitThe opening sentence is awkward, uses incorrect apostrophes, and repeats “Rollkit.” It also implies DA is a built-in component rather than an external requirement, contradicting earlier feedback.
-Data availability (DA) is a core of Rollkit's. Rollkit utilize's data availability ensures that all transaction data and block information required to verify the rollup's state is accessible to anyone running a node or light client. +Data availability (DA) is critical to Rollkit-based rollups. By relying on a DA layer, Rollkit ensures that all transaction and block data required to verify the rollup state is accessible to anyone running a node or light client.
29-31
: Tighten wording“To be deployed in a variety of environments” → “to be deployed in various environments” is shorter and avoids the flagged “a variety of.”
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
learn/data-availability.md
(1 hunks)
🧰 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)
learn/data-availability.md (1)
9-13
: Confirm whether the Mock DA section should stayA previous reviewer noted that Mock DA is strictly an internal test harness and suggested removing this section. If the intention is to keep it, consider explicitly stating that it is unsupported outside CI/dev and may be removed in the future.
42c8ad4
to
62b3fbe
Compare
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 (3)
learn/data-availability.md (2)
37-38
: Drop “.md” extension from internal linksVitePress resolves page links automatically; omitting the extension keeps URLs cleaner and future-proof.
-[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)
3-3
: Refine intro for accuracy & grammar“Data availability (DA) is a core of Rollkit's” is factually misleading (DA is an external requirement, not an internal component) and contains grammatical issues (
utilize's
). Consider:-Data availability (DA) is a core of Rollkit's. Rollkit utilize's data availability ensures that all transaction data and block information required to verify the rollup's state is accessible to anyone running a node or light client. +Rollkit relies on data availability (DA) to ensure that all transaction and block data needed to verify a rollup’s state is accessible to any full or light node.learn/execution/abci.md (1)
3-6
: Mention Reth & tighten phrasingThe earlier review asked to “touch on the general-purpose interface and then mention ABCI and Reth as two options.” Reth (Ethereum execution) is still missing, and the sentence can be more concise.
-Rollkit is designed to be modular and flexible, allowing different execution layers to be plugged in via the ABCI (Application Blockchain Interface) protocol. Rollkit defines a general-purpose execution interface ([see execution.go](https://github.com/rollkit/rollkit/blob/main/core/execution/execution.go)) that enables developers to integrate any ABCI-compatible application as the rollup's execution layer. - -This means you can use a variety of ABCI-compatible applications as the execution environment for your rollup. +Rollkit exposes a general-purpose execution interface ([execution.go](https://github.com/rollkit/rollkit/blob/main/core/execution/execution.go)). +Any ABCI-compatible application—or the Ethereum-focused **Reth** engine—can plug in as the rollup’s execution layer.
🧹 Nitpick comments (2)
learn/data-availability.md (1)
31-31
: Remove redundant wording“Use Local DA only for development and testing locally.” → “Use Local DA only for development and testing.”
The trailing locally is redundant.learn/execution/abci.md (1)
9-18
: Add Reth to “Supported Execution Layers”Reth is officially maintained and ABCI-compatible via the execution interface; listing it here will prevent confusion.
### Supported Execution Layers @@ ### CosmWasm ... + +### Reth (Ethereum Execution) +Reth provides an Ethereum-native execution environment for Rollkit rollups that want EVM compatibility. + +- [Reth Repository](https://github.com/paradigmxyz/reth)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.vitepress/config.ts
(3 hunks)learn/data-availability.md
(1 hunks)learn/execution/abci.md
(1 hunks)learn/sequencing/overview.md
(1 hunks)learn/sequencing/single.md
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- learn/sequencing/overview.md
- learn/sequencing/single.md
- .vitepress/config.ts
🧰 Additional context used
🪛 LanguageTool
learn/execution/abci.md
[style] ~5-~5: Consider using a more concise synonym.
Context: ...xecution layer. This means you can use a variety of ABCI-compatible applications as the exe...
(A_VARIETY_OF)
Overview
guides/overview.md
file as it became a duplicate.Summary by CodeRabbit
New Features
Documentation