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

feat [WIP DO NOT MERGE]: make blocks go brrr #83

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

jcstein
Copy link
Member

@jcstein jcstein commented Dec 19, 2023

Overview

Loom demonstration

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

Summary by CodeRabbit

  • New Features

    • Implemented faster block times by adjusting timeout settings for the consensus process.
    • Enabled key indexing for improved query performance.
  • Configuration Changes

    • Changed node mode to "validator" to support network security and consensus.

Copy link

coderabbitai bot commented Dec 19, 2023

Walkthrough

The changes involve tweaking the configuration of the celestia-app to enable faster consensus and block production by reducing various timeout settings. The node is also reconfigured to operate as a validator and improved for transaction indexing.

Changes

File Change Summary
entrypoint.sh Reduced timeout values, enabled key indexing, changed node mode to "validator" in config.toml.

🐇✨
In the realm of code, under digital skies,
A swift change rolls in, where speed multiplies.
Blocks fast as a hop, validators take flight,
A rabbit's quick script sets the blockchain alight. 🚀💫

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 X ?


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • You can reply to a review comment made by CodeRabbit.
  • You can tag CodeRabbit on specific lines of code or files in the PR by tagging @coderabbitai in a comment.
  • You can tag @coderabbitai in a PR comment and ask one-off questions about the PR and the codebase. Use quoted replies to pass the context for follow-up questions.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Copy link

@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.

Review Status

Actionable comments generated: 6

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 856ef2e and a8e8ad4.
Files selected for processing (1)
  • entrypoint.sh (1 hunks)
Additional comments: 1
entrypoint.sh (1)
  • 33-48: The changes to the config.toml file are significant and could impact the stability and security of the consensus process. Reducing timeouts too much may lead to increased network instability, especially in a distributed network where nodes have varying response times.
  • Verify the impact of these new timeout settings on the network's stability and performance.
  • Ensure that the changes are tested under realistic network conditions to prevent potential consensus issues.

Comment on lines 33 to 48
# If you encounter: `sed: -I or -i may not be used with stdin` on MacOS you can mitigate by installing gnu-sed
# https://gist.github.com/andre3k1/e3a1a7133fded5de5a9ee99c87c6fa0d?permalink_comment_id=3082272#gistcomment-3082272
sed -i'.bak' 's#"tcp://127.0.0.1:26657"#"tcp://0.0.0.0:26657"#g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_commit\s*=.*/timeout_commit = "1s"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_propose\s*=.*/timeout_propose = "1s"/g' ~/.celestia-app/config/config.toml

# Adjusting consensus timeouts for faster block times
sed -i'.bak' 's/^timeout_commit\s*=.*/timeout_commit = "100ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_propose\s*=.*/timeout_propose = "100ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_propose_delta\s*=.*/timeout_propose_delta = "50ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_prevote\s*=.*/timeout_prevote = "50ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_prevote_delta\s*=.*/timeout_prevote_delta = "25ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_precommit\s*=.*/timeout_precommit = "50ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_precommit_delta\s*=.*/timeout_precommit_delta = "25ms"/g' ~/.celestia-app/config/config.toml

# Other configurations
sed -i'.bak' 's/index_all_keys = false/index_all_keys = true/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/mode = "full"/mode = "validator"/g' ~/.celestia-app/config/config.toml
Copy link

Choose a reason for hiding this comment

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

Consider replacing hardcoded paths and values with environment variables or configuration files to increase the script's flexibility and maintainability.

Comment on lines 33 to 49
# If you encounter: `sed: -I or -i may not be used with stdin` on MacOS you can mitigate by installing gnu-sed
# https://gist.github.com/andre3k1/e3a1a7133fded5de5a9ee99c87c6fa0d?permalink_comment_id=3082272#gistcomment-3082272
sed -i'.bak' 's#"tcp://127.0.0.1:26657"#"tcp://0.0.0.0:26657"#g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_commit\s*=.*/timeout_commit = "1s"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_propose\s*=.*/timeout_propose = "1s"/g' ~/.celestia-app/config/config.toml

# Adjusting consensus timeouts for faster block times
sed -i'.bak' 's/^timeout_commit\s*=.*/timeout_commit = "100ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_propose\s*=.*/timeout_propose = "100ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_propose_delta\s*=.*/timeout_propose_delta = "50ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_prevote\s*=.*/timeout_prevote = "50ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_prevote_delta\s*=.*/timeout_prevote_delta = "25ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_precommit\s*=.*/timeout_precommit = "50ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_precommit_delta\s*=.*/timeout_precommit_delta = "25ms"/g' ~/.celestia-app/config/config.toml

# Other configurations
sed -i'.bak' 's/index_all_keys = false/index_all_keys = true/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/mode = "full"/mode = "validator"/g' ~/.celestia-app/config/config.toml

Copy link

Choose a reason for hiding this comment

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

Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [9-14]

Deleting the $APP_PATH directory is a destructive operation. It's recommended to either back up this directory before deletion or add a confirmation prompt to prevent accidental data loss.


Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [50-58]

Hardcoding a private key in the script poses a significant security risk. It's crucial to remove this and instead use a secure method for handling keys, such as environment variables or a secure key management system.


Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [70-85]

The loop that waits for the genesis block lacks error handling. If the genesis block isn't found within the maximum number of attempts, the script should handle this case gracefully, possibly with a descriptive error message and a non-zero exit status.


Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [87-91]

The script conditionally sets the CELESTIA_NAMESPACE variable but does not explain its purpose or expected values. Adding comments to document this would improve code clarity and maintainability.

Comment on lines 35 to 48
sed -i'.bak' 's#"tcp://127.0.0.1:26657"#"tcp://0.0.0.0:26657"#g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_commit\s*=.*/timeout_commit = "1s"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_propose\s*=.*/timeout_propose = "1s"/g' ~/.celestia-app/config/config.toml

# Adjusting consensus timeouts for faster block times
sed -i'.bak' 's/^timeout_commit\s*=.*/timeout_commit = "100ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_propose\s*=.*/timeout_propose = "100ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_propose_delta\s*=.*/timeout_propose_delta = "50ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_prevote\s*=.*/timeout_prevote = "50ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_prevote_delta\s*=.*/timeout_prevote_delta = "25ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_precommit\s*=.*/timeout_precommit = "50ms"/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/^timeout_precommit_delta\s*=.*/timeout_precommit_delta = "25ms"/g' ~/.celestia-app/config/config.toml

# Other configurations
sed -i'.bak' 's/index_all_keys = false/index_all_keys = true/g' ~/.celestia-app/config/config.toml
sed -i'.bak' 's/mode = "full"/mode = "validator"/g' ~/.celestia-app/config/config.toml
Copy link

Choose a reason for hiding this comment

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

The script creates backup files with the .bak extension but does not include logic to clean them up. Consider adding a cleanup step to remove these files after successful execution to avoid cluttering the filesystem.

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.

None yet

1 participant