-
Notifications
You must be signed in to change notification settings - Fork 258
fix: single node script #2031
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
fix: single node script #2031
Conversation
WalkthroughThe Changes
Sequence Diagram(s)(omitted — changes are script-level workflow updates without complex multi-component runtime interaction warranting a sequence diagram) Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
c9b8492 to
f42c99e
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: 1
🧹 Nitpick comments (2)
_build/single-node.sh (2)
16-17: Consider making cleanup optional or adding a warning.The unconditional
rm -rf ~/.akashis destructive and removes all previous data without warning. While appropriate for a development script, consider either:
- Adding a confirmation prompt
- Making cleanup optional via a flag
- Adding a clear comment warning users about data loss
🔎 Example: Optional cleanup with flag
+CLEAN=${CLEAN:-false} + +if [ "$CLEAN" = "true" ]; then + echo "Cleaning up previous data..." + rm -rf ~/.akash +fi -# Clean up previous data -rm -rf ~/.akashThen users can run:
CLEAN=true ./_build/single-node.sh <chain-id> <account>
21-21: Consider using a descriptive moniker instead of reusing chain-id.The command uses
$CHAINIDfor both the moniker (first positional argument) and the--chain-idflag. While functional, monikers are typically human-readable node names (e.g., "local-validator") distinct from the chain identifier.🔎 Suggested improvement
-akash genesis init "$CHAINID" --chain-id "$CHAINID" +akash genesis init "local-validator" --chain-id "$CHAINID"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
_build/single-node.sh
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-08T15:59:15.441Z
Learnt from: troian
Repo: akash-network/node PR: 2007
File: make/releasing.mk:43-45
Timestamp: 2025-10-08T15:59:15.441Z
Learning: In make/releasing.mk, the $(AKASH) target is intentionally marked as .PHONY to force a rebuild every time, even though it produces the akash binary file. This is a deliberate design choice by the maintainers.
Applied to files:
_build/single-node.sh
🧬 Code graph analysis (1)
_build/single-node.sh (1)
script/upgrades.sh (1)
init(317-410)
🔇 Additional comments (5)
_build/single-node.sh (5)
20-20: LGTM!Increasing the initial token amount to 100 billion is reasonable for a development environment.
23-24: LGTM!The updated
akash genesis add-accountcommands correctly follow the new CLI structure.
25-25: LGTM!The updated
akash genesis gentxcommand with--min-self-delegation="1"correctly follows the new CLI structure. The minimal self-delegation is appropriate for a development environment.
26-26: LGTM!The updated
akash genesis collectcommand correctly follows the new CLI structure.
35-35: LGTM!The
akash startcommand with--pruning=nothingis appropriate for a development environment.
chainzero
left a comment
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.
Approve PR to update all genesis-related commands to use the new akash genesis prefix (init, add-account, collect) required by the updated Akash CLI
|
I merged this PR without Arthurs approve due to not important change. |
Description
./_build/single-node.sh is not working due to cli changes.
I fixed that script and improved it a bit.
Testing
Blocks are produced and I I can query the node:
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.md