fix: metrics, reset chain state, restart rollup #593
Conversation
WalkthroughDocumentation and shell scripts have been updated to reflect new binary names, directory structures, and command-line flag conventions. Notable changes include replacing Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ShellScript
participant ChainBinary
User->>ShellScript: Run initialization script
ShellScript->>ChainBinary: Start chain with updated flags (dot notation)
ChainBinary-->>ShellScript: Chain process starts with correct config
ShellScript-->>User: Outputs status and instructions
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 (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 focuses on updating and correcting documentation guides for Rollkit, specifically addressing metrics, resetting chain state, and restarting rollups. The changes aim to improve accuracy by reflecting current command usage, directory structures, and flag names, and by streamlining information presentation by linking to external specifications where appropriate.
Highlights
- Metrics Documentation Update: The detailed list of available metrics has been replaced with a link to the comprehensive Technical Specifications, ensuring the documentation remains up-to-date without manual maintenance of metric tables.
- Chain State Reset Guide Refinement: The guide for resetting chain state has been updated to reflect the
testappbinary and its associated.testappdirectory structure. It now clarifies that only thedatadirectory's content needs to be deleted, and introduces thetestapp unsafe-cleancommand as an alternative. - Rollup Restart Guide Corrections: The guide for restarting rollups has been updated to use the
testappbinary in examples and corrects the--rollkit.da_namespaceflag to--rollkit.da.namespacefor accuracy. - Command-line Flag Standardization: Various initialization scripts have been updated to use the standardized dot-notation for Rollkit DA-related flags (e.g.,
--rollkit.da.namespaceinstead of--rollkit.da_namespace), improving consistency across the codebase.
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. ↩
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
public/gm/init-mocha-testnet.sh (1)
57-58: Fix SC2140 & maintain flag consistency (same as Arabica script)Apply the same quoting refactor suggested for
init-arabica-testnet.shto avoid broken restart script and make maintenance easier.
🧹 Nitpick comments (5)
guides/restart-rollup.md (1)
42-43: Tighten wording & drop redundant “first”The sentence reads a bit clumsily and triggers a style warning (
FIRST_BEGAN). Consider:-It is important to include any additional flags that you used when you first started your rollup. +It is important to include any additional flags that you used when you started your rollup.public/gm/init-mocha-testnet.sh (1)
60-60: Quote variable expansionsMirror the quoting fix for
$AUTH_TOKEN/$DA_BLOCK_HEIGHT.public/gm/init-mainnet.sh (1)
60-60: Quote variable expansionsWrap
$AUTH_TOKENand$DA_BLOCK_HEIGHTin double quotes in the livegmd startcommand.guides/metrics.md (1)
23-26: Minor Markdown nit – blank line before fenced blockFor consistency with the rest of the doc, add a blank line before the fenced code block:
-1. Accessing the metrics endpoint directly: -```bash +1. Accessing the metrics endpoint directly: + +```bashguides/reset-state.md (1)
65-71: Offer an explicit one-liner for manual clean-upSome users may prefer a direct
rmover the helper sub-command. Consider adding:rm -rf $HOME/.testapp/data/*right before or after the
testapp unsafe-cleansnippet.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
guides/metrics.md(1 hunks)guides/reset-state.md(2 hunks)guides/restart-rollup.md(3 hunks)public/gm/init-arabica-testnet.sh(1 hunks)public/gm/init-local.sh(1 hunks)public/gm/init-mainnet.sh(1 hunks)public/gm/init-mocha-testnet.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
guides/reset-state.md (2)
Learnt from: assafmo
PR: rollkit/docs#390
File: guides/assets/hyperlane-use-tia-for-gas/relayer.json:1-5
Timestamp: 2024-10-11T19:05:20.591Z
Learning: In the file `guides/assets/hyperlane-use-tia-for-gas/relayer.json`, the settings `"allowLocalCheckpointSyncers": "true"` and `"gasPaymentEnforcement": [{ "type": "none" }]` are intentional.
Learnt from: assafmo
PR: rollkit/docs#390
File: guides/assets/hyperlane-use-tia-for-gas/relayer.json:1-5
Timestamp: 2024-10-08T09:25:31.642Z
Learning: In the file `guides/assets/hyperlane-use-tia-for-gas/relayer.json`, the settings `"allowLocalCheckpointSyncers": "true"` and `"gasPaymentEnforcement": [{ "type": "none" }]` are intentional.
🪛 LanguageTool
guides/restart-rollup.md
[style] ~42-~42: This phrase is redundant. Consider writing “started”.
Context: ...additional flags that you used when you first started your rollup. For example, if you used t...
(FIRST_BEGAN)
🪛 Shellcheck (0.10.0)
public/gm/init-arabica-testnet.sh
[warning] 57-57: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(SC2140)
public/gm/init-mocha-testnet.sh
[warning] 57-57: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(SC2140)
public/gm/init-mainnet.sh
[warning] 57-57: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(SC2140)
public/gm/init-local.sh
[warning] 93-93: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(SC2140)
🔇 Additional comments (5)
public/gm/init-arabica-testnet.sh (1)
60-60: Quote variables to survive spaces & subshells
$AUTH_TOKENcan legally contain spaces. Wrap expansions:-gmd start --rollkit.aggregator --rollkit.da_auth_token=$AUTH_TOKEN ... +gmd start --rollkit.aggregator --rollkit.da_auth_token="$AUTH_TOKEN" ...guides/metrics.md (1)
15-16: Link may cover only Block-Manager metricsThe URL points to
block-manager.html#metrics, which currently lists Block Manager metrics only. If other subsystems (P2P, Sequencer, etc.) expose metrics, consider linking to a higher-level index or adding multiple links.public/gm/init-local.sh (1)
95-96: Flag migration looks goodNice catch updating
--rollkit.aggregatorand the DA flags to their dot-notation counterparts. Aligns this script with the rest of the repo.guides/reset-state.md (2)
21-33: Consistent binary & directory rename acknowledgedThe switch from
.rollkit→.testappandrollkit start→testapp startis clear and matches other docs. No issues spotted.
139-145: Verify the newcometsub-commandPrevious Cosmos-SDK builds expose
tendermint unsafe-reset-all; recent versions sometimes alias this tocomet, but support isn’t universal.Please confirm that the built
gmdbinary indeed provides:gmd comet unsafe-reset-alland update the guide if the correct invocation remains
tendermint unsafe-reset-all.
| echo "gmd start --rollkit.aggregator --rollkit.da.auth_token=\$AUTH_TOKEN --rollkit.da.namespace 00000000000000000000000000000000000000000008e5f679bf7116cb --rollkit.da.start_height \$DA_BLOCK_HEIGHT --rpc.laddr tcp://127.0.0.1:36657 --grpc.address 127.0.0.1:9290 --p2p.laddr \"0.0.0.0:36656\" --minimum-gas-prices="0.025stake"" >> restart-mainnet.sh | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Broken quoting in generated restart script
The restart script suffers from the same SC2140 pattern. Refactor with a here-doc as shown for the Arabica script.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 57-57: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(SC2140)
🤖 Prompt for AI Agents
In public/gm/init-mainnet.sh around lines 57 to 58, the echo command appending
to restart-mainnet.sh has broken quoting causing SC2140 issues. Refactor this by
replacing the echo with a here-doc to properly handle the complex quoting and
variable expansions, similar to the approach used in the Arabica script,
ensuring the generated restart script is correctly formatted and free of quoting
errors.
Overview
Updated guides:
Summary by CodeRabbit
Documentation
Chores